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

Set default CPU limit as 2 and show in TMDEv2/v3/v4 task metadata #2783

Merged
merged 1 commit into from
Jan 12, 2021

Conversation

chienhanlin
Copy link
Contributor

@chienhanlin chienhanlin commented Jan 6, 2021

Summary

This PR sets default CPU limit from 0 to 2 if Null, zero, and CPU values of 1 is assigned to a container in a task definition.

Cases and the corresponding Task metadata endpoint (TMDE) task response are listed as followed:

  • Case 1: CPU limit is Null or 0 or 1 for a container in the task definition -> TMDEv2/v3/v4 task metadata shows the container with CPU limit as 2
  • Case 2: CPU limit is >= 2 for a container in the task definition -> TMDEv2/v3/v4 task metadata shows the container with CPU limit defined in the task definition

Reference of CPU limit can be found here

Implementation details

  1. Set the minimumCPUUnit and defaultCPU as 2
  2. Update container.CPU in ContainerResponse if it is less than minimumCPUUnit
  3. Update the expected CPU limit to 2 in TestTaskResponseMarshal

Testing

New tests cover the changes: no

Manual tests

Run a task definition with 5 containers with different CPU limits as follows:

  1. Container sleepWithCPUNotSpecified has CPU limit Null
  2. Container sleepWithCPU0 has CPU limit 0
  3. Container sleepWithCPU1 has CPU limit 1
  4. Container sleepWithCPU2 has CPU limit 2
  5. Container sleepWithCPU3 has CPU limit 3

All containers have Memory limit 50.

Task metadata from TMDEv3
curl ${ECS_CONTAINER_METADATA_URI}/task

{
   "Cluster":"default",
   "TaskARN":"TaskARN",
   ...
   "Containers":[
      { ..."Name":"sleepWithCPUNotSpecified", ... "Limits":{ "CPU":2, "Memory":50}, ...},
      { ..."Name":"sleepWithCPU0", ... "Limits":{ "CPU":2, "Memory":50}, ...},
      { ..."Name":"sleepWithCPU1", ... "Limits":{ "CPU":2, "Memory":50}, ...},
      { ..."Name":"sleepWithCPU2", ... "Limits":{ "CPU":2, "Memory":50}, ...},
      { ..."Name":"sleepWithCPU3", ... "Limits":{ "CPU":3, "Memory":50}, ...},
   ],
  ...
}

Task metadata from TMDEv4
curl ${ECS_CONTAINER_METADATA_URI_V4}/task

{
   "Cluster":"default",
   "TaskARN":"TaskARN",
   "LaunchType":"EC2",
   ...
   "Containers":[
      { ..."Name":"sleepWithCPUNotSpecified", ... "Limits":{ "CPU":2, "Memory":50}, ...},
      { ..."Name":"sleepWithCPU0", ... "Limits":{ "CPU":2, "Memory":50}, ...},
      { ..."Name":"sleepWithCPU1", ... "Limits":{ "CPU":2, "Memory":50}, ...},
      { ..."Name":"sleepWithCPU2", ... "Limits":{ "CPU":2, "Memory":50}, ...},
      { ..."Name":"sleepWithCPU3", ... "Limits":{ "CPU":3, "Memory":50}, ...},
   ],
}

Description for the changelog

Bug - Fixed the number of cpu units the Agent will reserve for the Linux container instances.

Licensing

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

@chienhanlin chienhanlin changed the title [WIP] Set default CPU limit as 2 and show in TMDEv2/v3/v4 task metadata Set default CPU limit as 2 and show in TMDEv2/v3/v4 task metadata Jan 6, 2021
@chienhanlin chienhanlin marked this pull request as ready for review January 6, 2021 01:39
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