Merge pull request #1904 from ubhattacharjya/MergeContOrder

Merge branch 'container-ordering-feature' into dev

README.md

@@ -149,7 +149,7 @@ additional details on each available environment variable.
 | `ECS_APPARMOR_CAPABLE` | `true` | Whether AppArmor is available on the container instance. | `false` | `false` |
 | `ECS_ENGINE_TASK_CLEANUP_WAIT_DURATION` | 10m | Time to wait to delete containers for a stopped task. If set to less than 1 minute, the value is ignored.  | 3h | 3h |
 | `ECS_CONTAINER_STOP_TIMEOUT` | 10m | Instance scoped configuration for time to wait for the container to exit normally before being forcibly killed. | 30s | 30s |
-| `ECS_CONTAINER_START_TIMEOUT` | 10m | Instance scoped configuration for timeout before giving up on starting a container. | 3m | 8m |
+| `ECS_CONTAINER_START_TIMEOUT` | 10m | Timeout before giving up on starting a container. | 3m | 8m |
 | `ECS_ENABLE_TASK_IAM_ROLE` | `true` | Whether to enable IAM Roles for Tasks on the Container Instance | `false` | `false` |
 | `ECS_ENABLE_TASK_IAM_ROLE_NETWORK_HOST` | `true` | Whether to enable IAM Roles for Tasks when launched with `host` network mode on the Container Instance | `false` | `false` |
 | `ECS_DISABLE_IMAGE_CLEANUP` | `true` | Whether to disable automated image cleanup for the ECS Agent. | `false` | `false` |

agent/acs/model/api/api-2.json

@@ -138,8 +138,8 @@
     "AssociationType":{
       "type":"string",
       "enum":[
-        "gpu",
-        "elastic-inference"
+        "elastic-inference",
+        "gpu"
       ]
     },
     "Associations":{
@@ -203,7 +203,30 @@
         "healthCheckType":{"shape":"HealthCheckType"},
         "registryAuthentication":{"shape":"RegistryAuthenticationData"},
         "logsAuthStrategy":{"shape":"AuthStrategy"},
-        "secrets":{"shape":"SecretList"}
+        "secrets":{"shape":"SecretList"},
+        "dependsOn":{"shape":"ContainerDependencies"},
+        "startTimeout":{"shape":"Integer"},
+        "stopTimeout":{"shape":"Integer"}
+      }
+    },
+    "ContainerCondition":{
+      "type":"string",
+      "enum":[
+        "START",
+        "COMPLETE",
+        "SUCCESS",
+        "HEALTHY"
+      ]
+    },
+    "ContainerDependencies":{
+      "type":"list",
+      "member":{"shape":"ContainerDependency"}
+    },
+    "ContainerDependency":{
+      "type":"structure",
+      "members":{
+        "containerName":{"shape":"String"},
+        "condition":{"shape":"ContainerCondition"}
       }
     },
     "ContainerList":{
@@ -479,8 +502,7 @@
     "SecretType":{
       "type":"string",
       "enum":[
-        "ENVIRONMENT_VARIABLE",
-        "MOUNT_POINT"
+        "ENVIRONMENT_VARIABLE"
       ]
     },
     "SensitiveString":{
@@ -592,4 +614,4 @@
       ]
     }
   }
-}
+}

agent/acs/model/ecsacs/api.go

@@ -206,6 +206,8 @@ type Container struct {
 
 	Cpu *int64 `locationName:"cpu" type:"integer"`
 
+	DependsOn []*ContainerDependency `locationName:"dependsOn" type:"list"`
+
 	DockerConfig *DockerConfig `locationName:"dockerConfig" type:"structure"`
 
 	EntryPoint []*string `locationName:"entryPoint" type:"list"`
@@ -236,6 +238,10 @@ type Container struct {
 
 	Secrets []*Secret `locationName:"secrets" type:"list"`
 
+	StartTimeout *int64 `locationName:"startTimeout" type:"integer"`
+
+	StopTimeout *int64 `locationName:"stopTimeout" type:"integer"`
+
 	VolumesFrom []*VolumeFrom `locationName:"volumesFrom" type:"list"`
 }
 
@@ -249,6 +255,24 @@ func (s Container) GoString() string {
 	return s.String()
 }
 
+type ContainerDependency struct {
+	_ struct{} `type:"structure"`
+
+	Condition *string `locationName:"condition" type:"string" enum:"ContainerCondition"`
+
+	ContainerName *string `locationName:"containerName" type:"string"`
+}
+
+// String returns the string representation
+func (s ContainerDependency) String() string {
+	return awsutil.Prettify(s)
+}
+
+// GoString returns the string representation
+func (s ContainerDependency) GoString() string {
+	return s.String()
+}
+
 type DockerConfig struct {
 	_ struct{} `type:"structure"`
 

agent/api/container/container.go

@@ -93,6 +93,8 @@ type HealthStatus struct {
 type Container struct {
 	// Name is the name of the container specified in the task definition
 	Name string
+	// DependsOn is the field which specifies the ordering for container startup and shutdown.
+	DependsOn []DependsOn `json:"dependsOn,omitempty"`
 	// V3EndpointID is a container identifier used to construct v3 metadata endpoint; it's unique among
 	// all the containers managed by the agent
 	V3EndpointID string
@@ -130,14 +132,21 @@ type Container struct {
 	DockerConfig DockerConfig `json:"dockerConfig"`
 	// RegistryAuthentication is the auth data used to pull image
 	RegistryAuthentication *RegistryAuthenticationData `json:"registryAuthentication"`
-	// HealthCheckType is the mechnism to use for the container health check
+	// HealthCheckType is the mechanism to use for the container health check
 	// currently it only supports 'DOCKER'
 	HealthCheckType string `json:"healthCheckType,omitempty"`
 	// Health contains the health check information of container health check
 	Health HealthStatus `json:"-"`
 	// LogsAuthStrategy specifies how the logs driver for the container will be
 	// authenticated
 	LogsAuthStrategy string
+	// StartTimeout specifies the time value after which if a container has a dependency
+	// on another container and the dependency conditions are 'SUCCESS', 'COMPLETE', 'HEALTHY',
+	// then that dependency will not be resolved.
+	StartTimeout uint
+	// StopTimeout specifies the time value to be passed as StopContainer api call
+	StopTimeout uint
+
 	// lock is used for fields that are accessed and updated concurrently
 	lock sync.RWMutex
 
@@ -236,6 +245,11 @@ type Container struct {
 	labels map[string]string
 }
 
+type DependsOn struct {
+	ContainerName string `json:"containerName"`
+	Condition     string `json:"condition"`
+}
+
 // DockerContainer is a mapping between containers-as-docker-knows-them and
 // containers-as-we-know-them.
 // This is primarily used in DockerState, but lives here such that tasks and
@@ -860,3 +874,17 @@ func (c *Container) HasSecretAsEnv() bool {
 	}
 	return false
 }
+
+func (c *Container) GetStartTimeout() time.Duration {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
+	return time.Duration(c.StartTimeout) * time.Second
+}
+
+func (c *Container) GetStopTimeout() time.Duration {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
+	return time.Duration(c.StopTimeout) * time.Second
+}

agent/api/container/container_test.go

@@ -179,7 +179,7 @@ func TestSetupExecutionRoleFlag(t *testing.T) {
 	}
 }
 
-func TestSetHealtStatus(t *testing.T) {
+func TestSetHealthStatus(t *testing.T) {
 	container := Container{}
 
 	// set the container status to be healthy
@@ -455,3 +455,16 @@ func TestHasSecretAsEnv(t *testing.T) {
 	}
 
 }
+
+func TestPerContainerTimeouts(t *testing.T) {
+	timeout := uint(10)
+	expectedTimeout := time.Duration(timeout) * time.Second
+
+	container := Container{
+		StartTimeout: timeout,
+		StopTimeout:  timeout,
+	}
+
+	assert.Equal(t, container.GetStartTimeout(), expectedTimeout)
+	assert.Equal(t, container.GetStopTimeout(), expectedTimeout)
+}

agent/api/task/task.go

@@ -72,6 +72,9 @@ const (
 	NvidiaVisibleDevicesEnvVar = "NVIDIA_VISIBLE_DEVICES"
 	GPUAssociationType         = "gpu"
 
+	ContainerOrderingCreateCondition = "CREATE"
+	ContainerOrderingStartCondition  = "START"
+
 	arnResourceSections  = 2
 	arnResourceDelimiter = "/"
 	// networkModeNone specifies the string used to define the `none` docker networking mode
@@ -250,6 +253,18 @@ func (task *Task) PostUnmarshalTask(cfg *config.Config,
 		}
 	}
 
+	err := task.initializeContainerOrderingForVolumes()
+	if err != nil {
+		seelog.Errorf("Task [%s]: could not initialize volumes dependency for container: %v", task.Arn, err)
+		return apierrors.NewResourceInitError(task.Arn, err)
+	}
+
+	err = task.initializeContainerOrderingForLinks()
+	if err != nil {
+		seelog.Errorf("Task [%s]: could not initialize links dependency for container: %v", task.Arn, err)
+		return apierrors.NewResourceInitError(task.Arn, err)
+	}
+
 	if task.requiresASMDockerAuthData() {
 		task.initializeASMAuthResource(credentialsManager, resourceFields)
 	}
@@ -262,7 +277,7 @@ func (task *Task) PostUnmarshalTask(cfg *config.Config,
 		task.initializeASMSecretResource(credentialsManager, resourceFields)
 	}
 
-	err := task.initializeDockerLocalVolumes(dockerClient, ctx)
+	err = task.initializeDockerLocalVolumes(dockerClient, ctx)
 	if err != nil {
 		return apierrors.NewResourceInitError(task.Arn, err)
 	}
@@ -1250,6 +1265,41 @@ func (task *Task) shouldOverrideIPCMode(container *apicontainer.Container, docke
 	}
 }
 
+func (task *Task) initializeContainerOrderingForVolumes() error {
+	for _, container := range task.Containers {
+		if len(container.VolumesFrom) > 0 {
+			for _, volume := range container.VolumesFrom {
+				if _, ok := task.ContainerByName(volume.SourceContainer); !ok {
+					return fmt.Errorf("could not find container with name %s", volume.SourceContainer)
+				}
+				dependOn := apicontainer.DependsOn{ContainerName: volume.SourceContainer, Condition: ContainerOrderingCreateCondition}
+				container.DependsOn = append(container.DependsOn, dependOn)
+			}
+		}
+	}
+	return nil
+}
+
+func (task *Task) initializeContainerOrderingForLinks() error {
+	for _, container := range task.Containers {
+		if len(container.Links) > 0 {
+			for _, link := range container.Links {
+				linkParts := strings.Split(link, ":")
+				if len(linkParts) > 2 {
+					return fmt.Errorf("Invalid link format")
+				}
+				linkName := linkParts[0]
+				if _, ok := task.ContainerByName(linkName); !ok {
+					return fmt.Errorf("could not find container with name %s", linkName)
+				}
+				dependOn := apicontainer.DependsOn{ContainerName: linkName, Condition: ContainerOrderingStartCondition}
+				container.DependsOn = append(container.DependsOn, dependOn)
+			}
+		}
+	}
+	return nil
+}
+
 func (task *Task) dockerLinks(container *apicontainer.Container, dockerContainerMap map[string]*apicontainer.DockerContainer) ([]string, error) {
 	dockerLinkArr := make([]string, len(container.Links))
 	for i, link := range container.Links {