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

Move task protection handler to ecs-agent module #3779

Merged
merged 4 commits into from
Jul 7, 2023

Conversation

amogh09
Copy link
Contributor

@amogh09 amogh09 commented Jun 30, 2023

Summary

Adding task protection endpoint handlers to ecs-agent module and consuming them in agent module. The new handlers mostly reuse the existing handlers' code except for some changes noted below.

  • The new handlers support metrics publishing. Integration of the new handlers with agent module uses a no-op metrics publisher, so there is no actual functional change in this PR.
  • Cosmetic changes to increase code reuse and readability.

Implementation details

  • Remove task protection handlers and their tests from agent module
  • Update v4 TaskResponse model (aka task metadata in v4 format) to include a new CredentialsID field for task role credentials. Task role credentials are required by task protection handlers for ECS client initialization for fulfilling task protection requests. The new field is marked with json:"-" so it won't be included in v4 task metadata response.
  • Add new task protection handlers to ecs-agent module. The new package exports the following functions -
    • TaskProtectionPath that returns a standard URI path for task protection endpoints.
    • GetTaskProtectionHandler that returns an HTTP handler for GetTaskProtection request.
    • UpdateTaskProtectionHandler that returns an HTTP handler for UpdateTaskProtection request.
  • The new handlers reuse the AgentState interface used by v4 task and container metadata handlers for getting task information from Agent.
  • Add unit tests for the new task protection handlers. Tests cover all happy and error cases.
  • Updates to TMDS setup code in agent/handlers/task_server_setup.go to consume the new handlers.
  • Update imports as necessary.
  • Update existing v4 task metadata tests in ecs-agent to ensure that task role credentials ID are not populated in the response.

Testing

Test-driven development was followed. TMDS-level tests were added to agent module for task protection endpoints in #3739 and #3740 to capture the current high-level behavior of the endpoints. The same tests are passing for this PR, so there is no regression.

Manual regression, stress, and performance tests were also performed. For manual testing, agent was build from source of this PR and deployed to an EC2 instance. Another EC2 instance was provisioned with released Agent version v1.71.2.

For manual regression testing, GetTaskProtection, UpdateTaskProtection, and v4 Task Metadata endpoints were called on both the instances and the results were compared using diff. No regression was detected.

For manual stress testing, GetTaskProtection and UpdateTaskProtection endpoints were called at a rate of 80 rps for 60 seconds (steady case) and 200 rps for 1 second (burst case). 100% of requests were successful. The request rates match the ECS rate limits for these APIs.

For manual performance testing, agent was profiled for heap and CPU usage while stress tests were being performed as explained above. Similar heap and CPU usage was seen for both the Agents.

New tests cover the changes: yes

Description for the changelog

Move task protection handlers to 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 changed the base branch from master to dev June 30, 2023 06:20
@amogh09 amogh09 marked this pull request as ready for review June 30, 2023 17:29
@amogh09 amogh09 requested a review from a team as a code owner June 30, 2023 17:29
@danehlim
Copy link
Contributor

danehlim commented Jul 5, 2023

Excellent PR description write up - it helped inform the reviewing process a lot!

@amogh09 amogh09 merged commit 235ac90 into aws:dev Jul 7, 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