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

Consume TMDS init function from ecs-agent module #3663

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

amogh09
Copy link
Contributor

@amogh09 amogh09 commented Apr 28, 2023

Summary

TMDS initialization was moved to ecs-agent module in #3660. Change TMDS initialization in agent module to consume the TMDS initialization function from ecs-agent module.

Implementation details

  • Import tmds.NewServer function from ecs-agent module in agent/task_server_setup.go file and use it to initialize TMDS.
  • Replace usage of AgentCredentialsPort constant with tmds.Port constant imported from ecs-agent module.
  • Replace usage of LoggingHandler in introspection server with a LoggingHandler imported from ecs-agent module. LoggingHandler has also been moved to ecs-agent module. Remove the LoggingHandler in agent module.

Testing

Unit and functional tests are already in place to test TMDS. In addition to those, I performed some manual tests that are listed below.

  • Built Agent from source and called the following endpoints and made sure that they are available.
    • v2 - credentials, container metadata, task metadata, taskWithTags, container stats, and task stats
    • v3 - container metadata, task metadata, taskWithTags, container stats, task stats, and associations
    • v4 - container metadata, task metadata, taskWithTags, container stats, task stats, and associations
    • task protection - GetTaskProtection and UpdateTaskProtection
  • Performed stress tests on multiple endpoints and made sure that they are able to handle the same requests load as before.
  • Performed stress test with request rate matching the configured rate limit (40 rps sustained, 100 rps burst) and checked that rate limiter is not triggered for both sustained and burst cases.
  • Exceeded request rate limits and ensured that TMDS returns 429 Too Many Requests error when the limits are exceeded.
  • Checked that audit logs are output to the audit log file as before for requests to v2 credentials endpoint and when rate limits are exceeded.
  • Checked that logging middleware continues to work as before by setting the log level to DEBUG and observing Agent logs while sending requests to TMDS.
  • Checked that logging middleware for Introspection server continues to work as before by setting the log level to DEBUG and observing Agent logs while sending requests to the Introspection Server.
  • Collected profiling data for CPU and memory utilization and checked that the utilization is not obviously different than before.
  • Called taskWithTags endpoint at a high request rate (for this endpoint) of 40rps to trigger throttling and checked that write timeout kicked in for the throttled requests and cut the connection after a timeout of 5 seconds as expected.

New tests cover the changes: no

Description for the changelog

Consume TMDS init function from ecs-agent module

Licensing

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

@amogh09 amogh09 marked this pull request as ready for review April 28, 2023 18:57
@amogh09 amogh09 requested a review from a team as a code owner April 28, 2023 18:57
@@ -46,6 +45,7 @@ require (
github.com/containerd/continuity v0.3.0 // indirect
github.com/coreos/go-systemd/v22 v22.3.2 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/didip/tollbooth v4.0.2+incompatible // indirect
Copy link
Contributor

Choose a reason for hiding this comment

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

General Question!: Why was github.com/didip/tollbooth v4.0.2+incompatible moved here?

Copy link
Contributor Author

@amogh09 amogh09 Apr 28, 2023

Choose a reason for hiding this comment

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

This was done by go mod tidy. The reason is that agent module no longer has a direct dependency on tollbooth which is a library that provides request rate limiting middleware. ecs-agent module is responsible for adding the rate limiting middleware to TMDS, so agent module now has an indirect dependency on tollbooth while ecs-agent has a direct dependency as shown below.

github.com/didip/tollbooth v4.0.2+incompatible

@mye956
Copy link
Contributor

mye956 commented Apr 28, 2023

Could you also add a link to the previous PR in the description?

@amogh09
Copy link
Contributor Author

amogh09 commented Apr 28, 2023

Yes, added a link to the previous PR to the description.

@amogh09 amogh09 merged commit f41cc6a into aws:dev Apr 28, 2023
@amogh09 amogh09 deleted the consume-tmds branch April 28, 2023 21:58
@Realmonia Realmonia mentioned this pull request May 9, 2023
Realmonia pushed a commit to Realmonia/amazon-ecs-agent that referenced this pull request May 16, 2023
Realmonia pushed a commit to Realmonia/amazon-ecs-agent that referenced this pull request May 21, 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.

4 participants