Skip to content

Commit

Permalink
Add ContainerManifestPulled and TaskManifestPulled states
Browse files Browse the repository at this point in the history
  • Loading branch information
amogh09 committed Apr 9, 2024
1 parent 22af4f2 commit 8f2d145
Show file tree
Hide file tree
Showing 19 changed files with 598 additions and 110 deletions.
5 changes: 4 additions & 1 deletion agent/api/container/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions agent/api/task/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -3682,7 +3682,8 @@ func (task *Task) ToHostResources() map[string]*ecs.Resource {
func (task *Task) HasActiveContainers() bool {
for _, container := range task.Containers {
containerStatus := container.GetKnownStatus()
if containerStatus >= apicontainerstatus.ContainerPulled && containerStatus <= apicontainerstatus.ContainerResourcesProvisioned {
if containerStatus >= apicontainerstatus.ContainerManifestPulled &&
containerStatus <= apicontainerstatus.ContainerResourcesProvisioned {
return true
}
}
Expand Down
10 changes: 9 additions & 1 deletion agent/api/task/task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2843,7 +2843,15 @@ func TestPostUnmarshalTaskASMDockerAuth(t *testing.T) {
}

err := task.PostUnmarshalTask(cfg, credentialsManager, resFields, nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t,
apicontainer.ResourceDependency{
Name: asmauth.ResourceName,
RequiredStatus: resourcestatus.ResourceStatus(asmauth.ASMAuthStatusCreated),
},
task.Containers[0].
TransitionDependenciesMap[apicontainerstatus.ContainerManifestPulled].
ResourceDependencies[0])
}

func TestPostUnmarshalTaskSecret(t *testing.T) {
Expand Down
6 changes: 6 additions & 0 deletions agent/engine/dependencygraph/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
273 changes: 253 additions & 20 deletions agent/engine/dependencygraph/graph_test.go

Large diffs are not rendered by default.

19 changes: 13 additions & 6 deletions agent/engine/dependencygraph/graph_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,19 @@ func TestVerifyCgroupDependenciesResolved(t *testing.T) {
ExpectedResolved bool
}{
{
Name: "resource none,container pull depends on resource created",
TargetKnown: apicontainerstatus.ContainerStatusNone,
TargetDep: apicontainerstatus.ContainerPulled,
DependencyKnown: resourcestatus.ResourceStatus(cgroup.CgroupStatusNone),
RequiredStatus: resourcestatus.ResourceStatus(cgroup.CgroupCreated),

Name: "resource none, container pull depends on resource created",
TargetKnown: apicontainerstatus.ContainerStatusNone,
TargetDep: apicontainerstatus.ContainerPulled,
DependencyKnown: resourcestatus.ResourceStatus(cgroup.CgroupStatusNone),
RequiredStatus: resourcestatus.ResourceStatus(cgroup.CgroupCreated),
ExpectedResolved: true,
},
{
Name: "resource none, current is manifest_pulled, container pull depends on resource created",
TargetKnown: apicontainerstatus.ContainerManifestPulled,
TargetDep: apicontainerstatus.ContainerPulled,
DependencyKnown: resourcestatus.ResourceStatus(cgroup.CgroupStatusNone),
RequiredStatus: resourcestatus.ResourceStatus(cgroup.CgroupCreated),
ExpectedResolved: false,
},
{
Expand Down
14 changes: 14 additions & 0 deletions agent/engine/docker_task_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down
17 changes: 10 additions & 7 deletions agent/engine/task_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 8f2d145

Please sign in to comment.