From db9817136c493fb1ae9c390fb347df7436919fb7 Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Fri, 26 Jul 2019 10:14:23 -0400 Subject: [PATCH 1/3] Allow unbounded ram tasks on Windows Fixes #1144 --- README.md | 1 + agent/api/task/task_windows.go | 11 +++++++ agent/api/task/task_windows_test.go | 47 +++++++++++++++++++++++++++++ agent/config/config_windows.go | 3 ++ agent/config/types_windows.go | 3 ++ 5 files changed, 65 insertions(+) diff --git a/README.md b/README.md index 46a0168635c..9fbceab7ac1 100644 --- a/README.md +++ b/README.md @@ -170,6 +170,7 @@ additional details on each available environment variable. | `ECS_CGROUP_PATH` | `/sys/fs/cgroup` | The root cgroup path that is expected by the ECS agent. This is the path that accessible from the agent mount. | `/sys/fs/cgroup` | Not applicable | | `ECS_CGROUP_CPU_PERIOD` | `10ms` | CGroups CPU period for task level limits. This value should be between 8ms to 100ms | `100ms` | Not applicable | | `ECS_ENABLE_CPU_UNBOUNDED_WINDOWS_WORKAROUND` | `true` | When `true`, ECS will allow CPU unbounded(CPU=`0`) tasks to run along with CPU bounded tasks in Windows. | Not applicable | `false` | +| `ECS_ENABLE_RAM_UNBOUNDED_WINDOWS_WORKAROUND` | `true` | When `true`, ECS will allow tasks with no memory reservation to run along with memory bounded tasks in Windows. To run a memory unbounded task, omit the memory hard limit and set any memory reservation, it will be ignored. | Not applicable | `false` | | `ECS_TASK_METADATA_RPS_LIMIT` | `100,150` | Comma separated integer values for steady state and burst throttle limits for task metadata endpoint | `40,60` | `40,60` | | `ECS_SHARED_VOLUME_MATCH_FULL_CONFIG` | `true` | When `true`, ECS Agent will compare name, driver options, and labels to make sure volumes are identical. When `false`, Agent will short circuit shared volume comparison if the names match. This is the default Docker behavior. If a volume is shared across instances, this should be set to `false`. | `false` | `false`| | `ECS_CONTAINER_INSTANCE_PROPAGATE_TAGS_FROM` | `ec2_instance` | If `ec2_instance` is specified, existing tags defined on the container instance will be registered to Amazon ECS and will be discoverable using the `ListTagsForResource` API. Using this requires that the IAM role associated with the container instance have the `ec2:DescribeTags` action allowed. | `none` | `none` | diff --git a/agent/api/task/task_windows.go b/agent/api/task/task_windows.go index 85556d6a99c..d96a6a473e9 100644 --- a/agent/api/task/task_windows.go +++ b/agent/api/task/task_windows.go @@ -42,6 +42,9 @@ type PlatformFields struct { // CpuUnbounded determines whether a mix of unbounded and bounded CPU tasks // are allowed to run in the instance CpuUnbounded bool `json:"cpuUnbounded"` + // RamUnbounded determines whether a mix of unbounded and bounded Memory tasks + // are allowed to run in the instance + RamUnbounded bool `json:"ramUnbounded"` } var cpuShareScaleFactor = runtime.NumCPU() * cpuSharesPerCore @@ -51,6 +54,7 @@ func (task *Task) adjustForPlatform(cfg *config.Config) { task.downcaseAllVolumePaths() platformFields := PlatformFields{ CpuUnbounded: cfg.PlatformVariables.CPUUnbounded, + RamUnbounded: cfg.PlatformVariables.RAMUnbounded, } task.PlatformFields = platformFields } @@ -119,6 +123,13 @@ func (task *Task) platformHostConfigOverride(hostConfig *dockercontainer.HostCon hostConfig.CPUPercent = minimumCPUPercent } hostConfig.CPUShares = 0 + + if hostConfig.Memory <= 0 && task.PlatformFields.RamUnbounded { + // As of version 17.06.2-ee-6 of docker. MemoryReservation is not supported on windows. This ensures that + // this parameter is not passed, allowing to launch a container without a hard limit. + hostConfig.MemoryReservation = 0 + } + return nil } diff --git a/agent/api/task/task_windows_test.go b/agent/api/task/task_windows_test.go index 8842aa9ff92..e95842cadda 100644 --- a/agent/api/task/task_windows_test.go +++ b/agent/api/task/task_windows_test.go @@ -35,6 +35,9 @@ import ( const ( minDockerClientAPIVersion = dockerclient.Version_1_24 + + nonZeroMemoryReservationValue = 1 + expectedMemoryReservationValue = 0 ) func TestPostUnmarshalWindowsCanonicalPaths(t *testing.T) { @@ -254,6 +257,50 @@ func TestCPUPercentBasedOnUnboundedEnabled(t *testing.T) { } } +func TestWindowsMemoryReservationOption(t *testing.T) { + // Testing sending a task to windows overriding MemoryReservation value + rawHostConfigInput := docker.HostConfig{ + MemoryReservation: nonZeroMemoryReservationValue, + } + + rawHostConfig, err := json.Marshal(&rawHostConfigInput) + if err != nil { + t.Fatal(err) + } + + testTask := &Task{ + Arn: "arn:aws:ecs:us-east-1:012345678910:task/c09f0188-7f87-4b0f-bfc3-16296622b6fe", + Family: "myFamily", + Version: "1", + Containers: []*Container{ + { + Name: "c1", + DockerConfig: DockerConfig{ + HostConfig: strptr(string(rawHostConfig)), + }, + }, + }, + PlatformFields: PlatformFields{ + RamUnbounded: false, + }, + } + + // With RamUnbounded set to false, MemoryReservation is not overridden + config, configErr := testTask.DockerHostConfig(testTask.Containers[0], dockerMap(testTask), defaultDockerClientAPIVersion) + if configErr != nil { + t.Fatal(configErr) + } + assert.EqualValues(t, nonZeroMemoryReservationValue, config.MemoryReservation) + + // With RamUnbounded set to true, tasks with no memory hard limit will have their memory reservation set to zero + testTask.PlatformFields.RamUnbounded = true + config, configErr = testTask.DockerHostConfig(testTask.Containers[0], dockerMap(testTask), defaultDockerClientAPIVersion) + if configErr != nil { + t.Fatal(configErr) + } + assert.EqualValues(t, expectedMemoryReservationValue, config.MemoryReservation) +} + func TestGetCanonicalPath(t *testing.T) { testcases := []struct { name string diff --git a/agent/config/config_windows.go b/agent/config/config_windows.go index 7f4371b6858..149a3e5870e 100644 --- a/agent/config/config_windows.go +++ b/agent/config/config_windows.go @@ -59,6 +59,7 @@ func DefaultConfig() Config { dataDir := filepath.Join(ecsRoot, "data") platformVariables := PlatformVariables{ CPUUnbounded: false, + RAMUnbounded: false, } return Config{ DockerEndpoint: "npipe:////./pipe/docker_engine", @@ -118,8 +119,10 @@ func (cfg *Config) platformOverrides() { cfg.TaskCPUMemLimit = ExplicitlyDisabled cpuUnbounded := utils.ParseBool(os.Getenv("ECS_ENABLE_CPU_UNBOUNDED_WINDOWS_WORKAROUND"), false) + ramUnbounded := utils.ParseBool(os.Getenv("ECS_ENABLE_RAM_UNBOUNDED_WINDOWS_WORKAROUND"), false) platformVariables := PlatformVariables{ CPUUnbounded: cpuUnbounded, + RAMUnbounded: ramUnbounded, } cfg.PlatformVariables = platformVariables } diff --git a/agent/config/types_windows.go b/agent/config/types_windows.go index 4df58e7f9d3..8afe8b03b8c 100644 --- a/agent/config/types_windows.go +++ b/agent/config/types_windows.go @@ -19,4 +19,7 @@ type PlatformVariables struct { // CPUUnbounded specifies if agent can run a mix of CPU bounded and // unbounded tasks for windows CPUUnbounded bool + // RAMUnbounded specifies if agent can run a mix of Memory bounded and + // unbounded tasks for windows + RAMUnbounded bool } From d0eedb0f15d6b2f5201def85f284785eba73c1ec Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Fri, 26 Jul 2019 10:46:32 -0400 Subject: [PATCH 2/3] Fix failing windows tests --- agent/api/task/task_windows_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/agent/api/task/task_windows_test.go b/agent/api/task/task_windows_test.go index e95842cadda..f143c6d1773 100644 --- a/agent/api/task/task_windows_test.go +++ b/agent/api/task/task_windows_test.go @@ -259,8 +259,10 @@ func TestCPUPercentBasedOnUnboundedEnabled(t *testing.T) { func TestWindowsMemoryReservationOption(t *testing.T) { // Testing sending a task to windows overriding MemoryReservation value - rawHostConfigInput := docker.HostConfig{ - MemoryReservation: nonZeroMemoryReservationValue, + rawHostConfigInput := dockercontainer.HostConfig{ + Resources: dockercontainer.Resources{ + MemoryReservation: nonZeroMemoryReservationValue, + }, } rawHostConfig, err := json.Marshal(&rawHostConfigInput) @@ -272,10 +274,10 @@ func TestWindowsMemoryReservationOption(t *testing.T) { Arn: "arn:aws:ecs:us-east-1:012345678910:task/c09f0188-7f87-4b0f-bfc3-16296622b6fe", Family: "myFamily", Version: "1", - Containers: []*Container{ + Containers: []*apicontainer.Container{ { Name: "c1", - DockerConfig: DockerConfig{ + DockerConfig: apicontainer.DockerConfig{ HostConfig: strptr(string(rawHostConfig)), }, }, From 541f0d5359035d1fb020c93db1565b618bfa2255 Mon Sep 17 00:00:00 2001 From: adnan khan Date: Thu, 17 Oct 2019 15:36:11 -0400 Subject: [PATCH 3/3] api,config: rename unbounded memory config rename the unbounded memory config for windows and add update unit tests --- README.md | 2 +- agent/api/task/task_windows.go | 10 +++++----- agent/api/task/task_windows_test.go | 18 ++++++++---------- agent/config/config_windows.go | 11 ++++++----- agent/config/config_windows_test.go | 17 +++++++++++++++++ agent/config/types_windows.go | 4 ++-- 6 files changed, 39 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 9fbceab7ac1..37ed13de30e 100644 --- a/README.md +++ b/README.md @@ -170,7 +170,7 @@ additional details on each available environment variable. | `ECS_CGROUP_PATH` | `/sys/fs/cgroup` | The root cgroup path that is expected by the ECS agent. This is the path that accessible from the agent mount. | `/sys/fs/cgroup` | Not applicable | | `ECS_CGROUP_CPU_PERIOD` | `10ms` | CGroups CPU period for task level limits. This value should be between 8ms to 100ms | `100ms` | Not applicable | | `ECS_ENABLE_CPU_UNBOUNDED_WINDOWS_WORKAROUND` | `true` | When `true`, ECS will allow CPU unbounded(CPU=`0`) tasks to run along with CPU bounded tasks in Windows. | Not applicable | `false` | -| `ECS_ENABLE_RAM_UNBOUNDED_WINDOWS_WORKAROUND` | `true` | When `true`, ECS will allow tasks with no memory reservation to run along with memory bounded tasks in Windows. To run a memory unbounded task, omit the memory hard limit and set any memory reservation, it will be ignored. | Not applicable | `false` | +| `ECS_ENABLE_MEMORY_UNBOUNDED_WINDOWS_WORKAROUND` | `true` | When `true`, ECS will ignore the memory reservation parameter (soft limit) to run along with memory bounded tasks in Windows. To run a memory unbounded task, omit the memory hard limit and set any memory reservation, it will be ignored. | Not applicable | `false` | | `ECS_TASK_METADATA_RPS_LIMIT` | `100,150` | Comma separated integer values for steady state and burst throttle limits for task metadata endpoint | `40,60` | `40,60` | | `ECS_SHARED_VOLUME_MATCH_FULL_CONFIG` | `true` | When `true`, ECS Agent will compare name, driver options, and labels to make sure volumes are identical. When `false`, Agent will short circuit shared volume comparison if the names match. This is the default Docker behavior. If a volume is shared across instances, this should be set to `false`. | `false` | `false`| | `ECS_CONTAINER_INSTANCE_PROPAGATE_TAGS_FROM` | `ec2_instance` | If `ec2_instance` is specified, existing tags defined on the container instance will be registered to Amazon ECS and will be discoverable using the `ListTagsForResource` API. Using this requires that the IAM role associated with the container instance have the `ec2:DescribeTags` action allowed. | `none` | `none` | diff --git a/agent/api/task/task_windows.go b/agent/api/task/task_windows.go index d96a6a473e9..8bb9e3f1775 100644 --- a/agent/api/task/task_windows.go +++ b/agent/api/task/task_windows.go @@ -42,9 +42,9 @@ type PlatformFields struct { // CpuUnbounded determines whether a mix of unbounded and bounded CPU tasks // are allowed to run in the instance CpuUnbounded bool `json:"cpuUnbounded"` - // RamUnbounded determines whether a mix of unbounded and bounded Memory tasks + // MemoryUnbounded determines whether a mix of unbounded and bounded Memory tasks // are allowed to run in the instance - RamUnbounded bool `json:"ramUnbounded"` + MemoryUnbounded bool `json:"memoryUnbounded"` } var cpuShareScaleFactor = runtime.NumCPU() * cpuSharesPerCore @@ -53,8 +53,8 @@ var cpuShareScaleFactor = runtime.NumCPU() * cpuSharesPerCore func (task *Task) adjustForPlatform(cfg *config.Config) { task.downcaseAllVolumePaths() platformFields := PlatformFields{ - CpuUnbounded: cfg.PlatformVariables.CPUUnbounded, - RamUnbounded: cfg.PlatformVariables.RAMUnbounded, + CpuUnbounded: cfg.PlatformVariables.CPUUnbounded, + MemoryUnbounded: cfg.PlatformVariables.MemoryUnbounded, } task.PlatformFields = platformFields } @@ -124,7 +124,7 @@ func (task *Task) platformHostConfigOverride(hostConfig *dockercontainer.HostCon } hostConfig.CPUShares = 0 - if hostConfig.Memory <= 0 && task.PlatformFields.RamUnbounded { + if hostConfig.Memory <= 0 && task.PlatformFields.MemoryUnbounded { // As of version 17.06.2-ee-6 of docker. MemoryReservation is not supported on windows. This ensures that // this parameter is not passed, allowing to launch a container without a hard limit. hostConfig.MemoryReservation = 0 diff --git a/agent/api/task/task_windows_test.go b/agent/api/task/task_windows_test.go index f143c6d1773..584431078eb 100644 --- a/agent/api/task/task_windows_test.go +++ b/agent/api/task/task_windows_test.go @@ -283,23 +283,21 @@ func TestWindowsMemoryReservationOption(t *testing.T) { }, }, PlatformFields: PlatformFields{ - RamUnbounded: false, + MemoryUnbounded: false, }, } - // With RamUnbounded set to false, MemoryReservation is not overridden + // With MemoryUnbounded set to false, MemoryReservation is not overridden config, configErr := testTask.DockerHostConfig(testTask.Containers[0], dockerMap(testTask), defaultDockerClientAPIVersion) - if configErr != nil { - t.Fatal(configErr) - } + + assert.Nil(t, configErr) assert.EqualValues(t, nonZeroMemoryReservationValue, config.MemoryReservation) - // With RamUnbounded set to true, tasks with no memory hard limit will have their memory reservation set to zero - testTask.PlatformFields.RamUnbounded = true + // With MemoryUnbounded set to true, tasks with no memory hard limit will have their memory reservation set to zero + testTask.PlatformFields.MemoryUnbounded = true config, configErr = testTask.DockerHostConfig(testTask.Containers[0], dockerMap(testTask), defaultDockerClientAPIVersion) - if configErr != nil { - t.Fatal(configErr) - } + + assert.Nil(t, configErr) assert.EqualValues(t, expectedMemoryReservationValue, config.MemoryReservation) } diff --git a/agent/config/config_windows.go b/agent/config/config_windows.go index 149a3e5870e..0e8bed016ab 100644 --- a/agent/config/config_windows.go +++ b/agent/config/config_windows.go @@ -58,8 +58,8 @@ func DefaultConfig() Config { ecsRoot := filepath.Join(programData, "Amazon", "ECS") dataDir := filepath.Join(ecsRoot, "data") platformVariables := PlatformVariables{ - CPUUnbounded: false, - RAMUnbounded: false, + CPUUnbounded: false, + MemoryUnbounded: false, } return Config{ DockerEndpoint: "npipe:////./pipe/docker_engine", @@ -119,10 +119,11 @@ func (cfg *Config) platformOverrides() { cfg.TaskCPUMemLimit = ExplicitlyDisabled cpuUnbounded := utils.ParseBool(os.Getenv("ECS_ENABLE_CPU_UNBOUNDED_WINDOWS_WORKAROUND"), false) - ramUnbounded := utils.ParseBool(os.Getenv("ECS_ENABLE_RAM_UNBOUNDED_WINDOWS_WORKAROUND"), false) + memoryUnbounded := utils.ParseBool(os.Getenv("ECS_ENABLE_MEMORY_UNBOUNDED_WINDOWS_WORKAROUND"), false) + platformVariables := PlatformVariables{ - CPUUnbounded: cpuUnbounded, - RAMUnbounded: ramUnbounded, + CPUUnbounded: cpuUnbounded, + MemoryUnbounded: memoryUnbounded, } cfg.PlatformVariables = platformVariables } diff --git a/agent/config/config_windows_test.go b/agent/config/config_windows_test.go index 4da165a0881..293fc592973 100644 --- a/agent/config/config_windows_test.go +++ b/agent/config/config_windows_test.go @@ -113,3 +113,20 @@ func TestCPUUnboundedWindowsDisabled(t *testing.T) { assert.NoError(t, err) assert.False(t, cfg.PlatformVariables.CPUUnbounded) } + +func TestMemoryUnboundedSet(t *testing.T) { + defer setTestRegion()() + defer setTestEnv("ECS_ENABLE_MEMORY_UNBOUNDED_WINDOWS_WORKAROUND", "true")() + cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient()) + cfg.platformOverrides() + assert.NoError(t, err) + assert.True(t, cfg.PlatformVariables.MemoryUnbounded) +} + +func TestMemoryUnboundedWindowsDisabled(t *testing.T) { + defer setTestRegion()() + cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient()) + cfg.platformOverrides() + assert.NoError(t, err) + assert.False(t, cfg.PlatformVariables.MemoryUnbounded) +} diff --git a/agent/config/types_windows.go b/agent/config/types_windows.go index 8afe8b03b8c..9060db332bc 100644 --- a/agent/config/types_windows.go +++ b/agent/config/types_windows.go @@ -19,7 +19,7 @@ type PlatformVariables struct { // CPUUnbounded specifies if agent can run a mix of CPU bounded and // unbounded tasks for windows CPUUnbounded bool - // RAMUnbounded specifies if agent can run a mix of Memory bounded and + // MemoryUnbounded specifies if agent can run a mix of Memory bounded and // unbounded tasks for windows - RAMUnbounded bool + MemoryUnbounded bool }