Add ContainerManifestPulled and TaskManifestPulled states

Author: amogh09

agent/api/container/container_test.go

@@ -96,7 +96,10 @@ func TestIsKnownSteadyState(t *testing.T) {
 func TestGetNextStateProgression(t *testing.T) {
 	// This creates a container with `iota` ContainerStatus (NONE)
 	container := &Container{}
-	// NONE should transition to PULLED
+	// NONE should transition to MANIFEST_PULLED
+	assert.Equal(t, container.GetNextKnownStateProgression(), apicontainerstatus.ContainerManifestPulled)
+	container.SetKnownStatus(apicontainerstatus.ContainerManifestPulled)
+	// MANIFEST_PULLED should transition to PULLED
 	assert.Equal(t, container.GetNextKnownStateProgression(), apicontainerstatus.ContainerPulled)
 	container.SetKnownStatus(apicontainerstatus.ContainerPulled)
 	// PULLED should transition to CREATED

agent/api/task/task.go

@@ -1052,7 +1052,7 @@ func (task *Task) initializeASMAuthResource(credentialsManager credentials.Manag
 		if container.ShouldPullWithASMAuth() {
 			container.BuildResourceDependency(asmAuthResource.GetName(),
 				resourcestatus.ResourceStatus(asmauth.ASMAuthStatusCreated),
-				apicontainerstatus.ContainerPulled)
+				apicontainerstatus.ContainerManifestPulled)
 		}
 	}
 }

agent/api/task/task_test.go

@@ -2842,8 +2842,18 @@ func TestPostUnmarshalTaskASMDockerAuth(t *testing.T) {
 		},
 	}
 
+	resourceDep := apicontainer.ResourceDependency{
+		Name:           asmauth.ResourceName,
+		RequiredStatus: resourcestatus.ResourceStatus(asmauth.ASMAuthStatusCreated),
+	}
+
 	err := task.PostUnmarshalTask(cfg, credentialsManager, resFields, nil, nil)
-	assert.NoError(t, err)
+	require.NoError(t, err)
+	assert.Equal(t,
+		resourceDep,
+		task.Containers[0].
+			TransitionDependenciesMap[apicontainerstatus.ContainerManifestPulled].
+			ResourceDependencies[0])
 }
 
 func TestPostUnmarshalTaskSecret(t *testing.T) {

agent/engine/dependencygraph/graph.go

@@ -415,6 +415,12 @@ func containerOrderingDependenciesIsResolved(target *apicontainer.Container,
 	targetContainerKnownStatus := target.GetKnownStatus()
 	dependsOnContainerKnownStatus := dependsOnContainer.GetKnownStatus()
 
+	// Containers are allowed to transition to MANIFEST_PULLED irrespective of any container
+	// ordering dependencies.
+	if targetContainerKnownStatus < apicontainerstatus.ContainerManifestPulled {
+		return true
+	}
+
 	// The 'target' container desires to be moved to 'Created' or the 'steady' state.
 	// Allow this only if the environment variable ECS_PULL_DEPENDENT_CONTAINERS_UPFRONT is enabled and
 	// known status of the `target` container state has not reached to 'Pulled' state;

agent/engine/dependencygraph/graph_test.go

@@ -44,6 +44,14 @@ func TestVerifyCgroupDependenciesResolved(t *testing.T) {
 			DependencyKnown: resourcestatus.ResourceStatus(cgroup.CgroupStatusNone),
 			RequiredStatus:  resourcestatus.ResourceStatus(cgroup.CgroupCreated),
 
+			ExpectedResolved: true,
+		},
+		{
+			Name:             "resource none,container pull depends on resource created",
+			TargetKnown:      apicontainerstatus.ContainerManifestPulled,
+			TargetDep:        apicontainerstatus.ContainerPulled,
+			DependencyKnown:  resourcestatus.ResourceStatus(cgroup.CgroupStatusNone),
+			RequiredStatus:   resourcestatus.ResourceStatus(cgroup.CgroupCreated),
 			ExpectedResolved: false,
 		},
 		{

agent/engine/dependencygraph/graph_unix_test.go

@@ -289,6 +289,7 @@ func (engine *DockerTaskEngine) reconcileHostResources() {
 
 func (engine *DockerTaskEngine) initializeContainerStatusToTransitionFunction() {
 	containerStatusToTransitionFunction := map[apicontainerstatus.ContainerStatus]transitionApplyFunc{
+		apicontainerstatus.ContainerManifestPulled:       engine.pullContainerManifest,
 		apicontainerstatus.ContainerPulled:               engine.pullContainer,
 		apicontainerstatus.ContainerCreated:              engine.createContainer,
 		apicontainerstatus.ContainerRunning:              engine.startContainer,
@@ -1231,6 +1232,19 @@ func (engine *DockerTaskEngine) GetDaemonManagers() map[string]dm.DaemonManager
 	return engine.daemonManagers
 }
 
+// Pulls the manifest of the container image.
+func (engine *DockerTaskEngine) pullContainerManifest(
+	task *apitask.Task, container *apicontainer.Container,
+) dockerapi.DockerContainerMetadata {
+	// Currently a no-op
+	logger.Debug("Manifest pull is currently a no-op", logger.Fields{
+		field.TaskID:    task.GetID(),
+		field.Container: container.Name,
+		field.Image:     container.Image,
+	})
+	return dockerapi.DockerContainerMetadata{}
+}
+
 func (engine *DockerTaskEngine) pullContainer(task *apitask.Task, container *apicontainer.Container) dockerapi.DockerContainerMetadata {
 	switch container.Type {
 	case apicontainer.ContainerCNIPause, apicontainer.ContainerNamespacePause, apicontainer.ContainerServiceConnectRelay, apicontainer.ContainerManagedDaemon:

agent/engine/docker_task_engine.go

@@ -620,8 +620,9 @@ func (mtask *managedTask) emitTaskEvent(task *apitask.Task, reason string) {
 	}
 	if !taskKnownStatus.BackendRecognized() {
 		logger.Debug("Skipping event emission for task", logger.Fields{
-			field.TaskID: mtask.GetID(),
-			field.Error:  "status not recognized by ECS",
+			field.TaskID:      mtask.GetID(),
+			field.Error:       "status not recognized by ECS",
+			field.KnownStatus: taskKnownStatus.String(),
 		})
 		return
 	}
@@ -862,34 +863,36 @@ func (mtask *managedTask) handleEventError(containerChange dockerContainerChange
 	switch event.Status {
 	// event.Status is the desired container transition from container's known status
 	// (* -> event.Status)
-	case apicontainerstatus.ContainerPulled:
+	case apicontainerstatus.ContainerManifestPulled, apicontainerstatus.ContainerPulled:
 		// If the agent pull behavior is always or once, we receive the error because
-		// the image pull fails, the task should fail. If we don't fail task here,
+		// the image or manifest pull fails, the task should fail. If we don't fail task here,
 		// then the cached image will probably be used for creating container, and we
 		// don't want to use cached image for both cases.
 		if mtask.cfg.ImagePullBehavior == config.ImagePullAlwaysBehavior ||
 			mtask.cfg.ImagePullBehavior == config.ImagePullOnceBehavior {
-			logger.Error("Error while pulling image; moving task to STOPPED", logger.Fields{
+			logger.Error("Error while pulling image or its manifest; moving task to STOPPED", logger.Fields{
 				field.TaskID:    mtask.GetID(),
 				field.Image:     container.Image,
 				field.Container: container.Name,
 				field.Error:     event.Error,
+				field.Status:    event.Status,
 			})
 			// The task should be stopped regardless of whether this container is
 			// essential or non-essential.
 			mtask.SetDesiredStatus(apitaskstatus.TaskStopped)
 			return false
 		}
 		// If the agent pull behavior is prefer-cached, we receive the error because
-		// the image pull fails and there is no cached image in local, we don't make
+		// the image or manifest pull fails and there is no cached image in local, we don't make
 		// the task fail here, will let create container handle it instead.
 		// If the agent pull behavior is default, use local image cache directly,
 		// assuming it exists.
-		logger.Error("Error while pulling image; will try to run anyway", logger.Fields{
+		logger.Error("Error while pulling image or its manifest; will try to run anyway", logger.Fields{
 			field.TaskID:    mtask.GetID(),
 			field.Image:     container.Image,
 			field.Container: container.Name,
 			field.Error:     event.Error,
+			field.Status:    event.Status,
 		})
 		// proceed anyway
 		return true