Stop task on signal #383
Merged
Stop task on signal #383
+150
−100
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
session.New is deprecated because errors are not caught until the first service request is made.
When ecs.RunTask is called with Count unset, then there should always be exactly 1 task. This commit simplifies the code under that assumption so that we do not need an array of tasks. Also I don't think some of the functions worked anyway with multiple tasks (e.g. waitExitTasks returned when the first task finished instead of the last task).
We accidentally discarded the cancel when timeout is enabled, although at this point Cancel was unused
Pass a mostly empty aws.Config{} to session.NewSession (instead of defaults.Get().Config). The defaults package “shouldn't be used directly, but session.Session instead” (https://github.com/aws/aws-sdk-go/blob/v1.44.14/aws/defaults/defaults.go#L4-L5) When we called defaults.Get(), then Config.Credentials was set to a CredChain instead of AnonymousCredentials, which prevented Session from resolving credentials (https://github.com/aws/aws-sdk-go/blob/v1.44.14/aws/session/session.go#L783-L785).
Changes in behavior:
* Read the profile name from the AWS_PROFILE environment variable instead of the nonstandard AWS_DEFAULT_PROFILE
* Read the additional fields in ~/.aws/config that credentials.resolveCredsFromProfile understands but SharedCredentialsProvider does not:
* source_profile
* credential_source
* web_identity_token_file, role_arn, role_session_name
* sso_account_id, sso_region, sso_role_name, sso_start_url
* credential_process
* role_arn
* Read the "default" profile as a fallback if the given profile does not work
When the Context is Done (e.g. timeout), cancel the AWS requests immediately rather than waiting for them to complete.
Previously, when the --timeout was hit or ecs-task was interrupted, it exited immediately without cleaning anything up. This commit makes ecs-task call StopTask, followed by waiting up to 1 minute for the task to complete, followed by waiting 10 seconds for any additional logs.
Fix tests: mock the *WithContext methods now that we are using those instead
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat: Include fractional seconds in the log messages
feat: Use Session to create Credentials (to support more credential resolutions such as
sso_account_idin~/.aws/config)feat: Stop task on timeout or signal. Previously, when the --timeout was hit or ecs-task was interrupted, it exited immediately without cleaning anything up. This commit makes ecs-task call StopTask, followed by waiting up to 1 minute for the task to complete, followed by waiting 10 seconds for any additional logs.