agent: move cgroup hierarchy flag up to /ecs root

README.md

@@ -176,7 +176,7 @@ additional details on each available environment variable.
 | `ECS_ENABLE_CONTAINER_METADATA` | `true` | When `true`, the agent will create a file describing the container's metadata and the file can be located and consumed by using the container enviornment variable `$ECS_CONTAINER_METADATA_FILE` | `false` | `false` |
 | `ECS_HOST_DATA_DIR` | `/var/lib/ecs` | The source directory on the host from which ECS_DATADIR is mounted. We use this to determine the source mount path for container metadata files in the case the ECS Agent is running as a container. We do not use this value in Windows because the ECS Agent is not running as container in Windows. | `/var/lib/ecs` | `Not used` |
 | `ECS_ENABLE_TASK_CPU_MEM_LIMIT` | `true` | Whether to enable task-level cpu and memory limits | `true` | `false` |
-| `ECS_CGROUP_MEMORY_SUBSYSTEM_PATH` | `/sys/fs/cgroup/memory/` | The root path of the cgroup memory subsystem  | `/sys/fs/cgroup/memory/` | Not applicable |
+| `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 |
 
 ### Persistence
 

agent/acs/update_handler/updater_test.go

@@ -1,4 +1,4 @@
-// Copyright 2014-2016 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// Copyright 2014-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License"). You may
 // not use this file except in compliance with the License. A copy of the
@@ -105,7 +105,8 @@ func TestPerformUpdateWithUpdatesDisabled(t *testing.T) {
 		Reason:            ptr("Updates are disabled").(*string),
 	}})
 
-	u.performUpdateHandler(statemanager.NewNoopStateManager(), engine.NewTaskEngine(cfg, nil, nil, nil, nil, nil, nil, nil))(&ecsacs.PerformUpdateMessage{
+	u.performUpdateHandler(statemanager.NewNoopStateManager(),
+		engine.NewTaskEngine(cfg, nil, nil, nil, nil, nil, nil, nil))(&ecsacs.PerformUpdateMessage{
 		ClusterArn:           ptr("cluster").(*string),
 		ContainerInstanceArn: ptr("containerInstance").(*string),
 		MessageId:            ptr("mid").(*string),

agent/app/agent.go

@@ -1,4 +1,4 @@
-// Copyright 2014-2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// Copyright 2014-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License"). You may
 // not use this file except in compliance with the License. A copy of the
@@ -145,9 +145,6 @@ func newAgent(
 		metadataManager = containermetadata.NewManager(dockerClient, cfg)
 	}
 
-	resource := resources.New()
-	resource.ApplyConfigDependencies(cfg)
-
 	return &ecsAgent{
 		ctx:               ctx,
 		ec2MetadataClient: ec2MetadataClient,
@@ -166,7 +163,7 @@ func newAgent(
 		}),
 		os:                 oswrapper.New(),
 		metadataManager:    metadataManager,
-		resource:           resource,
+		resource:           resources.New(),
 		terminationHandler: sighandlers.StartDefaultTerminationHandler,
 	}, nil
 }
@@ -228,6 +225,7 @@ func (agent *ecsAgent) doStart(containerChangeEventStream *eventstream.EventStre
 
 	// Conditionally create '/ecs' cgroup root
 	if agent.cfg.TaskCPUMemLimit.Enabled() {
+		agent.resource.ApplyConfigDependencies(agent.cfg)
 		err = agent.resource.Init()
 		// When task CPU and memory limits are enabled, all tasks are placed
 		// under the '/ecs' cgroup root.
@@ -310,8 +308,9 @@ func (agent *ecsAgent) newTaskEngine(containerChangeEventStream *eventstream.Eve
 
 	if !agent.cfg.Checkpoint {
 		seelog.Info("Checkpointing not enabled; a new container instance will be created each time the agent is run")
-		return engine.NewTaskEngine(agent.cfg, agent.dockerClient,
-			credentialsManager, containerChangeEventStream, imageManager, state, agent.metadataManager, agent.resource), "", nil
+		return engine.NewTaskEngine(agent.cfg, agent.dockerClient, credentialsManager,
+			containerChangeEventStream, imageManager, state,
+			agent.metadataManager, agent.resource), "", nil
 	}
 
 	// We try to set these values by loading the existing state file first

agent/app/agent_test.go

@@ -170,6 +170,8 @@ func TestDoStartTaskLimitsFail(t *testing.T) {
 		saveableOptionFactory.EXPECT().AddSaveable(gomock.Any(), gomock.Any()).AnyTimes(),
 		stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(), gomock.Any(),
 			gomock.Any(), gomock.Any()).Return(statemanager.NewNoopStateManager(), nil),
+
+		resource.EXPECT().ApplyConfigDependencies(gomock.Any()).AnyTimes(),
 		resource.EXPECT().Init().Return(errors.New("test error")),
 	)
 

agent/app/agent_unix_test.go

@@ -466,6 +466,7 @@ func TestDoStartCgroupInitHappyPath(t *testing.T) {
 	mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes()
 
 	gomock.InOrder(
+		mockResource.EXPECT().ApplyConfigDependencies(gomock.Any()),
 		mockResource.EXPECT().Init().Return(nil),
 		mockCredentialsProvider.EXPECT().Retrieve().Return(credentials.Value{}, nil),
 		dockerClient.EXPECT().SupportedVersions().Return(nil),
@@ -527,6 +528,7 @@ func TestDoStartCgroupInitErrorPath(t *testing.T) {
 	imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1)
 	mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes()
 
+	mockResource.EXPECT().ApplyConfigDependencies(gomock.Any())
 	mockResource.EXPECT().Init().Return(errors.New("cgroup init error"))
 
 	cfg := getTestConfig()

agent/config/config.go

@@ -1,4 +1,4 @@
-// Copyright 2014-2016 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// Copyright 2014-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License"). You may
 // not use this file except in compliance with the License. A copy of the
@@ -280,7 +280,7 @@ func environmentConfig() (Config, error) {
 
 	dockerStopTimeout := getDockerStopTimeout()
 
-	cgroupMemorySubsystemPath := os.Getenv("ECS_CGROUP_MEMORY_SUBSYSTEM_PATH")
+	cgroupPath := os.Getenv("ECS_CGROUP_PATH")
 
 	taskCleanupWaitDuration := parseEnvVariableDuration("ECS_ENGINE_TASK_CLEANUP_WAIT_DURATION")
 
@@ -390,7 +390,7 @@ func environmentConfig() (Config, error) {
 		ContainerMetadataEnabled:         containerMetadataEnabled,
 		DataDirOnHost:                    dataDirOnHost,
 		OverrideAWSLogsExecutionRole:     overrideAWSLogsExecutionRoleEnabled,
-		CgroupMemorySubsystemPath:        cgroupMemorySubsystemPath,
+		CgroupPath:                       cgroupPath,
 	}, err
 }
 

agent/config/config_unix.go

@@ -1,5 +1,5 @@
 // +build !windows
-// Copyright 2014-2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// Copyright 2014-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License"). You may
 // not use this file except in compliance with the License. A copy of the
@@ -30,7 +30,7 @@ const (
 
 	// Default cgroup memory system root path, this is the default used if the
 	// path has not been configured through ECS_CGROUP_MEMORY_SUBSYSTEM_PATH
-	defaultCgroupMemorySubsystemPath = "/sys/fs/cgroup/memory/"
+	defaultCgroupPath = "/sys/fs/cgroup"
 )
 
 // DefaultConfig returns the default configuration for Linux
@@ -59,7 +59,7 @@ func DefaultConfig() Config {
 		AWSVPCBlockInstanceMetdata:  false,
 		ContainerMetadataEnabled:    false,
 		TaskCPUMemLimit:             DefaultEnabled,
-		CgroupMemorySubsystemPath:   defaultCgroupMemorySubsystemPath,
+		CgroupPath:                  defaultCgroupPath,
 	}
 }
 

agent/config/types.go

@@ -195,5 +195,5 @@ type Config struct {
 	// driver authentication over the task's execution role
 	OverrideAWSLogsExecutionRole bool
 
-	CgroupMemorySubsystemPath string
+	CgroupPath string
 }

agent/resources/resources_linux.go

@@ -30,14 +30,15 @@ import (
 )
 
 const (
+	memorySubsystem    = "/memory"
 	memoryUseHierarchy = "memory.use_hierarchy"
 )
 
 // cgroupWrapper implements the Resource interface
 type cgroupWrapper struct {
-	control             cgroup.Control
-	ioutil              ioutilwrapper.IOUtil
-	memorySubsystemPath string
+	control    cgroup.Control
+	ioutil     ioutilwrapper.IOUtil
+	cgroupPath string
 }
 
 // New is used to return an object that implements the Resource interface
@@ -68,7 +69,7 @@ func (c *cgroupWrapper) Cleanup(task *api.Task) error {
 }
 
 func (c *cgroupWrapper) ApplyConfigDependencies(cfg *config.Config) {
-	c.memorySubsystemPath = cfg.CgroupMemorySubsystemPath
+	c.cgroupPath = cfg.CgroupPath
 }
 
 // cgroupInit is used to create the root '/ecs/ cgroup
@@ -77,7 +78,21 @@ func (c *cgroupWrapper) cgroupInit() error {
 		seelog.Debugf("Cgroup at %s already exists, skipping creation", config.DefaultTaskCgroupPrefix)
 		return nil
 	}
-	return c.control.Init()
+	err := c.control.Init()
+
+	if err != nil {
+		return err
+	}
+
+	// echo 1 > memory.use_hierarchy
+	cgroupRoot := c.cgroupPath
+
+	err = c.ioutil.WriteFile(filepath.Join(cgroupRoot, memorySubsystem, config.DefaultTaskCgroupPrefix, memoryUseHierarchy), []byte(strconv.Itoa(1)), os.FileMode(0))
+	if err != nil {
+		return errors.Wrapf(err, "resource: setup cgroup: unable to set use hierarchy flag")
+	}
+
+	return nil
 }
 
 // setupCgroup is used to create the task cgroup
@@ -109,14 +124,6 @@ func (c *cgroupWrapper) setupCgroup(task *api.Task) error {
 		return errors.Wrapf(err, "resource: setup cgroup: unable to create cgroup at %s for task: %s", cgroupRoot, task.Arn)
 	}
 
-	// echo 1 > memory.use_hierarchy
-	memorySubsystem := c.memorySubsystemPath
-
-	err = c.ioutil.WriteFile(filepath.Join(memorySubsystem, cgroupRoot, memoryUseHierarchy), []byte(strconv.Itoa(1)), os.FileMode(0))
-	if err != nil {
-		return errors.Wrapf(err, "resource: setup cgroup: unable to set use hierarchy flag for task: %s", task.Arn)
-	}
-
 	return nil
 }
 

agent/resources/resources_linux_test.go

@@ -43,13 +43,17 @@ func TestInitHappyPath(t *testing.T) {
 
 	mockControl := mock_cgroup.NewMockControl(ctrl)
 	mockIO := mock_ioutilwrapper.NewMockIOUtil(ctrl)
+	cgroupPath := fmt.Sprintf("/sys/fs/cgroup/memory/ecs/memory.use_hierarchy")
 
 	gomock.InOrder(
 		mockControl.EXPECT().Exists(gomock.Any()).Return(false),
 		mockControl.EXPECT().Init().Return(nil),
+		mockIO.EXPECT().WriteFile(cgroupPath, gomock.Any(), gomock.Any()).Return(nil),
 	)
 
+	cfg := config.DefaultConfig()
 	resource := newResources(mockControl, mockIO)
+	resource.ApplyConfigDependencies(&cfg)
 	assert.NoError(t, resource.Init())
 }
 
@@ -89,21 +93,14 @@ func TestSetupHappyPath(t *testing.T) {
 	mockControl := mock_cgroup.NewMockControl(ctrl)
 	mockCgroup := mock_cgroups.NewMockCgroup(ctrl)
 	mockIO := mock_ioutilwrapper.NewMockIOUtil(ctrl)
-
 	task := testdata.LoadTask("sleep5TaskCgroup")
-	taskid, err := task.GetID()
-	assert.NoError(t, err)
-	cgroupPath := fmt.Sprintf("/sys/fs/cgroup/memory/ecs/%s/memory.use_hierarchy", taskid)
 
 	gomock.InOrder(
 		mockControl.EXPECT().Exists(gomock.Any()).Return(false),
 		mockControl.EXPECT().Create(gomock.Any()).Return(mockCgroup, nil),
-		mockIO.EXPECT().WriteFile(cgroupPath, gomock.Any(), gomock.Any()).Return(nil),
 	)
 
-	cfg := config.DefaultConfig()
 	resource := newResources(mockControl, mockIO)
-	resource.ApplyConfigDependencies(&cfg)
 	assert.NoError(t, resource.Setup(task))
 }