Merge pull request #2239 from adnxn/coveord-allow-unbounded-ram-tasks-windows

Author: adnxn

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_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` |

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"`
+	// MemoryUnbounded determines whether a mix of unbounded and bounded Memory tasks
+	// are allowed to run in the instance
+	MemoryUnbounded bool `json:"memoryUnbounded"`
 }
 
 var cpuShareScaleFactor = runtime.NumCPU() * cpuSharesPerCore
@@ -50,7 +53,8 @@ var cpuShareScaleFactor = runtime.NumCPU() * cpuSharesPerCore
 func (task *Task) adjustForPlatform(cfg *config.Config) {
 	task.downcaseAllVolumePaths()
 	platformFields := PlatformFields{
-		CpuUnbounded: cfg.PlatformVariables.CPUUnbounded,
+		CpuUnbounded:    cfg.PlatformVariables.CPUUnbounded,
+		MemoryUnbounded: cfg.PlatformVariables.MemoryUnbounded,
 	}
 	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.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
+	}
+
 	return nil
 }
 

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 := dockercontainer.HostConfig{
+		Resources: dockercontainer.Resources{
+			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: []*apicontainer.Container{
+			{
+				Name: "c1",
+				DockerConfig: apicontainer.DockerConfig{
+					HostConfig: strptr(string(rawHostConfig)),
+				},
+			},
+		},
+		PlatformFields: PlatformFields{
+			MemoryUnbounded: false,
+		},
+	}
+
+	// With MemoryUnbounded set to false, MemoryReservation is not overridden
+	config, configErr := testTask.DockerHostConfig(testTask.Containers[0], dockerMap(testTask), defaultDockerClientAPIVersion)
+
+	assert.Nil(t, configErr)
+	assert.EqualValues(t, nonZeroMemoryReservationValue, config.MemoryReservation)
+
+	// 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)
+
+	assert.Nil(t, configErr)
+	assert.EqualValues(t, expectedMemoryReservationValue, config.MemoryReservation)
+}
+
 func TestGetCanonicalPath(t *testing.T) {
 	testcases := []struct {
 		name           string

agent/config/config_windows.go

@@ -58,7 +58,8 @@ func DefaultConfig() Config {
 	ecsRoot := filepath.Join(programData, "Amazon", "ECS")
 	dataDir := filepath.Join(ecsRoot, "data")
 	platformVariables := PlatformVariables{
-		CPUUnbounded: false,
+		CPUUnbounded:    false,
+		MemoryUnbounded: false,
 	}
 	return Config{
 		DockerEndpoint: "npipe:////./pipe/docker_engine",
@@ -118,8 +119,11 @@ func (cfg *Config) platformOverrides() {
 	cfg.TaskCPUMemLimit = ExplicitlyDisabled
 
 	cpuUnbounded := utils.ParseBool(os.Getenv("ECS_ENABLE_CPU_UNBOUNDED_WINDOWS_WORKAROUND"), false)
+	memoryUnbounded := utils.ParseBool(os.Getenv("ECS_ENABLE_MEMORY_UNBOUNDED_WINDOWS_WORKAROUND"), false)
+
 	platformVariables := PlatformVariables{
-		CPUUnbounded: cpuUnbounded,
+		CPUUnbounded:    cpuUnbounded,
+		MemoryUnbounded: memoryUnbounded,
 	}
 	cfg.PlatformVariables = platformVariables
 }

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)
+}

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
+	// MemoryUnbounded specifies if agent can run a mix of Memory bounded and
+	// unbounded tasks for windows
+	MemoryUnbounded bool
 }