Skip to content

Commit

Permalink
fix: issues with ecs-task-runner during prototyping (#22)
Browse files Browse the repository at this point in the history
* fix: param names are kebab-case

Prior to this change, the parameter name was in camelCase, and this
casing appears to transfer over when Buildkite sets up the environment
variables for the plugin.

This change uses kebab case for the parameter, which is the casing
Buildkite expects. In turn, this transforms in UPPER_SNAKE_CASE when
the environment variables are set up.

* fix: missing newline makes config and Cloudwatch lines look contiguous

* fix: dereference the pointer to get the value of the exit code

This change fixes a missing pointer dereference so that the exit code of
the task is correctly compared, and the correct message is returned
based on the comparison.

* fix: missing newlines to keep logging legible

* chore: task runner isn't specific to migrations

Although the primary use of the task runner is for database migrations,
there isn't anything specific to the code that indicates it is only
capable of doing this.

Hence, we will keep the name of the tasks and container generic, and
leave the application up to the people who consume the plugin.
  • Loading branch information
therealvio authored Jan 19, 2025
1 parent f00405a commit b8a0e66
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 11 deletions.
2 changes: 1 addition & 1 deletion plugin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ author: https://github.com/cultureamp
requirements: []
configuration:
properties:
parameterName:
parameter-name:
type: string
script:
type: string
4 changes: 2 additions & 2 deletions src/aws/ecs.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func SubmitTask(ctx context.Context, ecsAPI EcsClientAPI, input *TaskRunnerConfi
Overrides: &types.TaskOverride{
ContainerOverrides: []types.ContainerOverride{
{
Name: aws.String("migrations-runner"),
Name: aws.String("task-runner"),
Command: input.Command,
},
},
Expand Down Expand Up @@ -109,7 +109,7 @@ func FindLogStreamFromTask(ctx context.Context, ecsClientAPI EcsClientAPI, task

// We need the logGroupName, streamPrefix, and a container name to be able to produce a FindLogStreamOutput in full
if logGroupName == "" || streamPrefix == "" {
return LogDetails{}, fmt.Errorf("cannot trace migration output: container logging is not configured on task definition: %v", response.TaskDefinition.TaskDefinitionArn)
return LogDetails{}, fmt.Errorf("cannot trace task output: container logging is not configured on task definition: %v", response.TaskDefinition.TaskDefinitionArn)
}

return LogDetails{
Expand Down
15 changes: 7 additions & 8 deletions src/plugin/task-runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
awsinternal "github.com/cultureamp/ecs-task-runner-buildkite-plugin/aws"
"github.com/cultureamp/ecs-task-runner-buildkite-plugin/buildkite"

"github.com/aws/aws-sdk-go-v2/aws"
awsconfig "github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/service/cloudwatchlogs"
"github.com/aws/aws-sdk-go-v2/service/ecs"
Expand Down Expand Up @@ -37,7 +36,7 @@ func (trp TaskRunnerPlugin) Run(ctx context.Context, fetcher ConfigFetcher) erro
}

ssmClient := ssm.NewFromConfig(cfg)
buildkite.Logf("Retrieving task configuration from: %s", config.ParameterName)
buildkite.Logf("Retrieving task configuration from: %s \n", config.ParameterName)
configuration, err := awsinternal.RetrieveConfiguration(ctx, ssmClient, config.ParameterName)
if err != nil {
return fmt.Errorf("failed to retrieve configuration: %w", err)
Expand Down Expand Up @@ -79,7 +78,7 @@ func (trp TaskRunnerPlugin) Run(ctx context.Context, fetcher ConfigFetcher) erro
}

if len(logs) > 0 {
buildkite.Logf("CloudWatch Logs for job:")
buildkite.Logf("CloudWatch Logs for job: \n")
for _, l := range logs {
if l.Timestamp != nil {
// Applying ISO 8601 format, l.Timestamp is in milliseconds, not very useful in logging
Expand All @@ -89,15 +88,15 @@ func (trp TaskRunnerPlugin) Run(ctx context.Context, fetcher ConfigFetcher) erro
}
}

// TODO: Assuming the task only has 1 container. What if there others? Like Datadog sideca
if task.Containers[0].ExitCode != aws.Int32(0) {
buildkite.LogFailuref("Task stopped with a non-zero exit code:: %d", task.Containers[0].ExitCode)
// TODO: Assuming the task only has 1 container. What if there others? Like Datadog sidecar
if *task.Containers[0].ExitCode != 0 {
buildkite.LogFailuref("Task stopped with a non-zero exit code:: %d\n", *task.Containers[0].ExitCode)
// TODO: At about here, a structured return type of "success: true/false" and "error" is returned
} else {
buildkite.Log("Task completed successfully :)")
buildkite.Log("Task completed successfully :) \n")
// TODO: At about here, a structured return type of "success: true/false" and "error" is returned
}

buildkite.Log("done.")
buildkite.Log("done. \n")
return nil
}

0 comments on commit b8a0e66

Please sign in to comment.