Skip to content

Commit

Permalink
Search cached images when Agent failed to pull images for essentail c…
Browse files Browse the repository at this point in the history 
…ontainers

and ECS_PULL_DEPENDENT_CONTAINERS_UPFRONT=true. The task will be stopped
if no cached image can be found.
  • Loading branch information
@chienhanlin
chienhanlin committed Jan 30, 2021
1 parent f4fcfe9 commit 3068aec
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 32 deletions.
34 changes: 33 additions & 1 deletion agent/engine/docker_task_engine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -840,6 +840,13 @@ func (engine *DockerTaskEngine) pullContainer(task *apitask.Task, container *api

}

// No pull image is required, the cached image will be used.
// Add the container that uses the cached image to the pulled container state.
dockerContainer := &apicontainer.DockerContainer{
Container: container,
}
engine.state.AddPulledContainer(dockerContainer, task)

// No pull image is required, just update container reference and use cached image.
engine.updateContainerReference(false, container, task.Arn)
// Return the metadata without any error
Expand Down Expand Up @@ -958,12 +965,37 @@ func (engine *DockerTaskEngine) pullAndUpdateContainerReference(task *apitask.Ta
return metadata
}
pullSucceeded := metadata.Error == nil
if pullSucceeded {
findCachedImage := false
if !pullSucceeded {
// If Agent failed to pull an image when
// 1. DependentContainersPullUpfront is enabled
// 2. ImagePullBehavior is not set to always
// search the image in local cached images
if engine.cfg.DependentContainersPullUpfront.Enabled() && engine.cfg.ImagePullBehavior != config.ImagePullAlwaysBehavior {
if _, err := engine.client.InspectImage(container.Image); err != nil {
seelog.Errorf("Task engine [%s]: failed to find cached image %s for container %s",
task.Arn, container.Image, container.Name)
// Stop the task if the container is an essential container,
// and the image is not available in both remote and local caches
if container.IsEssential() {
task.SetDesiredStatus(apitaskstatus.TaskStopped)
engine.emitTaskEvent(task, fmt.Sprintf("%s: %s", metadata.Error.ErrorName(), metadata.Error.Error()))
}
return dockerapi.DockerContainerMetadata{Error: metadata.Error}
}
seelog.Infof("Task engine [%s]: found cached image %s, use it directly for container %s",
task.Arn, container.Image, container.Name)
findCachedImage = true
}
}

if pullSucceeded || findCachedImage {
dockerContainer := &apicontainer.DockerContainer{
Container: container,
}
engine.state.AddPulledContainer(dockerContainer, task)
}

engine.updateContainerReference(pullSucceeded, container, task.Arn)
return metadata
}
Expand Down
141 changes: 110 additions & 31 deletions agent/engine/docker_task_engine_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container"
apicontainerstatus "github.com/aws/amazon-ecs-agent/agent/api/container/status"
apieni "github.com/aws/amazon-ecs-agent/agent/api/eni"
apierrors "github.com/aws/amazon-ecs-agent/agent/api/errors"
apitask "github.com/aws/amazon-ecs-agent/agent/api/task"
apitaskstatus "github.com/aws/amazon-ecs-agent/agent/api/task/status"
"github.com/aws/amazon-ecs-agent/agent/asm"
Expand Down Expand Up @@ -1685,41 +1686,119 @@ func TestUpdateContainerReference(t *testing.T) {
}

// TestPullAndUpdateContainerReference checks whether a container is added to task engine state when
// pullSucceeded and DependentContainersPullUpfront is enabled.
// Test # | Image availability | DependentContainersPullUpfront | ImagePullBehavior
// -----------------------------------------------------------------------------------
// 1 | remote | enabled | default
// 2 | local | enabled | default
// 3 | local | enabled | once
// 4 | local | enabled | prefer-cached
// 5 | local | enabled | always
func TestPullAndUpdateContainerReference(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()
cfg := &config.Config{
DependentContainersPullUpfront: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled},
testcases := []struct {
Name string
ImagePullUpfront config.BooleanDefaultFalse
ImagePullBehavior config.ImagePullBehaviorType
ImageState *image.ImageState
ImageInspect *types.ImageInspect
InspectImage bool
NumOfPulledContainer int
PullImageErr apierrors.NamedError
}{
{
Name: "DependentContainersPullUpfrontEnabledWithRemoteImage",
ImagePullUpfront: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled},
ImagePullBehavior: config.ImagePullDefaultBehavior,
ImageState: &image.ImageState{
Image: &image.Image{ImageID: "id"},
},
InspectImage: false,
NumOfPulledContainer: 1,
PullImageErr: nil,
},
{
Name: "DependentContainersPullUpfrontEnabledWithCachedImage",
ImagePullUpfront: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled},
ImagePullBehavior: config.ImagePullDefaultBehavior,
ImageState: nil,
ImageInspect: nil,
InspectImage: true,
NumOfPulledContainer: 1,
PullImageErr: dockerapi.CannotPullContainerError{fmt.Errorf("error")},
},
{
Name: "DependentContainersPullUpfrontEnabledAndImagePullOnceBehavior",
ImagePullUpfront: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled},
ImagePullBehavior: config.ImagePullOnceBehavior,
ImageState: nil,
ImageInspect: nil,
InspectImage: true,
NumOfPulledContainer: 1,
PullImageErr: dockerapi.CannotPullContainerError{fmt.Errorf("error")},
},
{
Name: "DependentContainersPullUpfrontEnabledAndImagePullPreferCachedBehavior",
ImagePullUpfront: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled},
ImagePullBehavior: config.ImagePullPreferCachedBehavior,
ImageState: nil,
ImageInspect: nil,
InspectImage: true,
NumOfPulledContainer: 1,
PullImageErr: dockerapi.CannotPullContainerError{fmt.Errorf("error")},
},
{
Name: "DependentContainersPullUpfrontEnabledAndImagePullAlwaysBehavior",
ImagePullUpfront: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled},
ImagePullBehavior: config.ImagePullAlwaysBehavior,
ImageState: nil,
ImageInspect: nil,
InspectImage: false,
NumOfPulledContainer: 0,
PullImageErr: dockerapi.CannotPullContainerError{fmt.Errorf("error")},
},
}
ctrl, client, _, privateTaskEngine, _, imageManager, _ := mocks(t, ctx, cfg)
defer ctrl.Finish()

taskEngine, _ := privateTaskEngine.(*DockerTaskEngine)
taskEngine._time = nil
imageName := "image"
taskArn := "taskArn"
container := &apicontainer.Container{
Type: apicontainer.ContainerNormal,
Image: imageName,
}
task := &apitask.Task{
Arn: taskArn,
Containers: []*apicontainer.Container{container},
}
imageState := &image.ImageState{
Image: &image.Image{ImageID: "id"},
}
for _, tc := range testcases {
t.Run(tc.Name, func(t *testing.T) {
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()
cfg := &config.Config{
DependentContainersPullUpfront: tc.ImagePullUpfront,
ImagePullBehavior: tc.ImagePullBehavior,
}
ctrl, client, _, privateTaskEngine, _, imageManager, _ := mocks(t, ctx, cfg)
defer ctrl.Finish()

client.EXPECT().PullImage(gomock.Any(), imageName, gomock.Any(), gomock.Any())
imageManager.EXPECT().RecordContainerReference(container)
imageManager.EXPECT().GetImageStateFromImageName(imageName).Return(imageState, true)
metadata := taskEngine.pullAndUpdateContainerReference(task, container)
pulledContainersMap, ok := taskEngine.State().PulledContainerMapByArn(taskArn)
require.True(t, ok, "no container found in the agent state")
require.Len(t, pulledContainersMap, 1)
assert.True(t, imageState.PullSucceeded, "PullSucceeded set to false")
assert.Equal(t, dockerapi.DockerContainerMetadata{}, metadata, "expected empty metadata")
taskEngine, _ := privateTaskEngine.(*DockerTaskEngine)
taskEngine._time = nil
imageName := "image"
taskArn := "taskArn"
container := &apicontainer.Container{
Type: apicontainer.ContainerNormal,
Image: imageName,
Essential: true,
}

task := &apitask.Task{
Arn: taskArn,
Containers: []*apicontainer.Container{container},
}

client.EXPECT().PullImage(gomock.Any(), imageName, nil, gomock.Any()).
Return(dockerapi.DockerContainerMetadata{Error: tc.PullImageErr})

if tc.InspectImage {
client.EXPECT().InspectImage(imageName).Return(tc.ImageInspect, nil)
}

imageManager.EXPECT().RecordContainerReference(container)
imageManager.EXPECT().GetImageStateFromImageName(imageName).Return(tc.ImageState, false)
metadata := taskEngine.pullAndUpdateContainerReference(task, container)
pulledContainersMap, _ := taskEngine.State().PulledContainerMapByArn(taskArn)
require.Len(t, pulledContainersMap, tc.NumOfPulledContainer)
assert.Equal(t, dockerapi.DockerContainerMetadata{Error: tc.PullImageErr},
metadata, "expected metadata with error")
})
}
}

// TestMetadataFileUpdatedAgentRestart checks whether metadataManager.Update(...) is
Expand Down

0 comments on commit 3068aec

Please sign in to comment.