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

Update error prefix of v4 container stats endpoint for task lookup failure case #3794

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

amogh09
Copy link
Contributor

@amogh09 amogh09 commented Jul 11, 2023

Summary

There is a discrepancy in the prefix of error message returned by v4 container stats endpoint between task lookup failure and container lookup failure cases.

Task lookup failure

"V4 container handler: unable to get task arn from request:  unable to get task Arn from v3 endpoint ID: <v3EndpointID>"

vs

Container lookup failure

"V4 container stats handler: unable to get container ID from request: unable to get docker ID from v3 endpoint ID: <v3EndpointID>"

That is, V4 container handler vs V4 container stats handler. We consider the former a bug and this PR makes the prefix for the two cases consistent by converging to V4 container stats handler. This discrepancy does not exist in V3 container stats endpoint and it uses V3 container stats handler prefix for both cases.

Note that there are some unrelated changes in this PR which are a result of running go mod vendor for agent module. The vendor directory for agent module is out of sync.

Implementation details

  • Add error prefix strings to TMDS handlers package in ecs-agent module.
  • Update container stats and task stats handlers in ecs-agent module to use the error prefix when generating error response for lookup failure cases.
  • Remove error prefixes from errors returned by the implementation of GetContainerStats and GetTaskStats methods in agent module.

Testing

Built Agent from source, installed it on an EC2 instance, and invoked v4 container stats endpoint with a bad endpoint container ID.

Before

$ curl -s http://169.254.170.2/v4/bad/stats | jq
"V4 container handler: unable to get task arn from request: unable to get task Arn from v3 endpoint ID: bad"

After

$ curl -s http://169.254.170.2/v4/bad/stats | jq
"V4 container stats handler: unable to get task arn from request: unable to get task Arn from v3 endpoint ID: bad"

New tests cover the changes: yes

Description for the changelog

Update error prefix of v4 container stats endpoint for task lookup failure case

Licensing

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

@amogh09 amogh09 changed the title Update error prefix of v4 container stats endpoint for container lookup failure case Update error prefix of v4 container stats endpoint for task lookup failure case Jul 11, 2023
@amogh09 amogh09 marked this pull request as ready for review July 11, 2023 20:27
@amogh09 amogh09 requested a review from a team as a code owner July 11, 2023 20:27
Copy link
Contributor

@xxx0624 xxx0624 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT. Thank you for taking care of this!

@amogh09 amogh09 merged commit 42bad15 into aws:dev Jul 12, 2023
6 checks passed
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.

4 participants