Skip to content
This repository was archived by the owner on Jul 10, 2024. It is now read-only.

Commit 60c1cba

Browse files
authored
Specify a container memory size when the task does not have one. (#241)
This was causing failures on an EC2-based cluster where only container memory sizes were being used.
1 parent 10dd11e commit 60c1cba

File tree

1 file changed

+21
-7
lines changed

1 file changed

+21
-7
lines changed

cmd/internal/ecs/add.go

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -847,10 +847,9 @@ func modifyTaskState(wf *AddWorkflow) (nextState optionals.Optional[AddWorkflowS
847847
{Name: aws.String("POSTMAN_ENV"), Value: &pEnv},
848848
}...)
849849
}
850-
input.ContainerDefinitions = append(input.ContainerDefinitions, types.ContainerDefinition{
851-
Name: aws.String("postman-lc-agent"),
852-
// TODO: Cpu and Memory should be omitted for Fargate; they take their default values for EC2 if omitted.
853-
// For now we can leave the defaults in place, but they might be a bit large for EC2.
850+
851+
agentContainer := types.ContainerDefinition{
852+
Name: aws.String("postman-lc-agent"),
854853
EntryPoint: []string{"/postman-lc-agent", "apidump", "--collection", collectionId},
855854
Environment: append(envs, []types.KeyValuePair{
856855
{Name: aws.String("POSTMAN_API_KEY"), Value: &pKey},
@@ -861,7 +860,22 @@ func modifyTaskState(wf *AddWorkflow) (nextState optionals.Optional[AddWorkflowS
861860
}...),
862861
Essential: aws.Bool(false),
863862
Image: aws.String(postmanECRImage),
864-
})
863+
}
864+
865+
// If running on EC2, a memory size is required if no task-level memory size is specified.
866+
// If running on Fargate, a task-level memory size is required, and the container-level
867+
// setting is optional.
868+
// We'll specify a memory reservation only when required, i.e., when the task-level memory
869+
// is absent.
870+
// TODO: come up with a better default value, 300MB is from internal dogfooding
871+
if input.Memory == nil {
872+
agentContainer.MemoryReservation = aws.Int32(300)
873+
}
874+
875+
// TODO: cpu share is optional on EC2 but the default is "two CPU shares" which may be too large.
876+
// It is optional in Fargate
877+
878+
input.ContainerDefinitions = append(input.ContainerDefinitions, agentContainer)
865879

866880
output, err := wf.ecsClient.RegisterTaskDefinition(wf.ctx, input)
867881
if err != nil {
@@ -871,7 +885,7 @@ func modifyTaskState(wf *AddWorkflow) (nextState optionals.Optional[AddWorkflowS
871885
printer.Infof("Please start over with a different profile, or add this permission in IAM.\n")
872886
return awf_error(errors.New("Failed to update the ECS task definition due to insufficient permissions."))
873887
}
874-
printer.Errorf("Could not register an ECS task definition. The error from the AWS library is shown below. Please send this log message to observability-support@postman.com for assistance.\n", err)
888+
printer.Errorf("Could not register an ECS task definition. The error from the AWS library is shown below. Please send this log message to observability-support@postman.com for assistance.\n%v\n", err)
875889
return awf_error(errors.Wrap(err, "Error registering task definition"))
876890
}
877891
printer.Infof("Registered task definition %q revision %d.\n",
@@ -914,7 +928,7 @@ func updateServiceState(wf *AddWorkflow) (nextState optionals.Optional[AddWorkfl
914928
wf.ecsServiceARN, uoe.OperationName)
915929
return awf_error(errors.New("Failed to update the ECS service due to insufficient permissions."))
916930
}
917-
printer.Errorf("Could not update the ECS service %q. The error from the AWS library is shown below. Please send this log message to observability-support@postman.com for assistance.\n", wf.ecsServiceARN, err)
931+
printer.Errorf("Could not update the ECS service %q. The error from the AWS library is shown below. Please send this log message to observability-support@postman.com for assistance.\n%v\n", wf.ecsServiceARN, err)
918932
return awf_error(errors.Wrapf(err, "Error updating ECS service %q", wf.ecsServiceARN))
919933
}
920934
printer.Infof("Updated service %q with new version of task definition.\n", wf.ecsService)

0 commit comments

Comments
 (0)