Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip sending internal task events to ECS control plane #3772

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

chienhanlin
Copy link
Contributor

@chienhanlin chienhanlin commented Jun 28, 2023

Summary


The internal task events should not be sent to ECS control plane.

Current behavior

For a container instance that had/has run a Service Connect enabled task, ecs-agent.log will show

level=warn time=xxx msg="Could not submit task state change: [arn:::::/service-connect-relay-xxx-> RUNNING, xxx: InvalidParameterException: Invalid identifier: Identifier is for cluster arn:::::. Your cluster is xxx" module=client.go
level=error time=xxx msg="Unretriable error sending state change to ECS" xxx error="InvalidParameterException: Invalid identifier: Identifier is for cluster arn:::::. Your cluster is xxx"
level=warn time=xxx msg="TaskHandler: Event is sent with invalid parameters; just removing" xxx

after ECS Agent is updated/restarted.

Here, ECS Agent will go though a list with task events, and try to make SubmitTaskStateChange API calls to submit any un-reported events to ECS control plane. As this internal task, which is designed for setting up an AppNet relay container for Service Connect, was not reported previously, ECS Agent will now try to report it to ECS control plane. However, making a STSC call will return an error because this internal task is constructed with mock data.

See how we construct this AppNet relay internal task here.

After this PR

ECS Agent will not submit internal task events to ECS control plane after it is updated/restarted. Logs shown in the "Current behavior" section will not show up in ecs-agent.log.

Implementation details


Code path

startDrainEventsTicker <- AddStateChangeEvent <- flushBatchUnsafe <- sendChange <- submitTaskEvents <- submitFirstEvent <- taskShouldBeSent

agent/eventhandler/task_handler_types.go

  • Add a new if-else statement to avoid sending internal task event.

agent/eventhandler/task_handler_types_test.go

  • Add a new test case to TestShouldTaskEventBeSent.

Testing


Manually testing

  1. Registered an EC2 instance launched with the latest ECS optimized AMI amzn2-ami-ecs-hvm-2.0.20230606-x86_64-ebs, which has ECS Agent version 1.73.0, to an empty ECS cluster.
  2. Ran Service Connect service on the ECS cluster.
  3. Downgraded ECS Agent version to 1.71.0 on the same container instance.
  4. Updated ECS Agent using the artifact built from this PR on the same container instance.
  5. Checked ecs-agent.log, and confirmed there is no error like level=warn time=xxx msg="Could not submit task state change: [arn:::::/service-connect-relay-xxx or level=error time=xxx msg="Unretriable error sending state change to ECS" xxx error="InvalidParameterException: Invalid identifier: Identifier is for cluster arn:::::. Your cluster is xxx"

New tests cover the changes: yes

Description for the changelog


Skip sending internal task events to ECS control plane

Licensing


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@chienhanlin chienhanlin marked this pull request as ready for review June 28, 2023 21:31
@chienhanlin chienhanlin requested a review from a team as a code owner June 28, 2023 21:31
@chienhanlin chienhanlin merged commit f6bd9a9 into aws:dev Jun 29, 2023
@chienhanlin chienhanlin deleted the skipSTSCCallforInternalTask branch June 29, 2023 18:38
@mye956 mye956 mentioned this pull request Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants