engine: introduce a new env var to distinct pull image behavior

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Changelog
 
 ## 1.17.4-dev
+* Feature - Configurable container image pull behavior [#1348](https://github.com/aws/amazon-ecs-agent/pull/1348)
 * Bug - Fixed a bug where Docker Version() API never returns by adding a timeout [#1363](https://github.com/aws/amazon-ecs-agent/pull/1363)
 * Bug - Fixed a bug where tasks could get stuck waiting for execution of CNI plugin [#1358](https://github.com/aws/amazon-ecs-agent/pull/1358)
 

README.md

@@ -169,6 +169,7 @@ additional details on each available environment variable.
 | `ECS_IMAGE_CLEANUP_INTERVAL` | 30m | The time interval between automated image cleanup cycles. If set to less than 10 minutes, the value is ignored. | 30m | 30m |
 | `ECS_IMAGE_MINIMUM_CLEANUP_AGE` | 30m | The minimum time interval between when an image is pulled and when it can be considered for automated image cleanup. | 1h | 1h |
 | `ECS_NUM_IMAGES_DELETE_PER_CYCLE` | 5 | The maximum number of images to delete in a single automated image cleanup cycle. If set to less than 1, the value is ignored. | 5 | 5 |
+| `ECS_IMAGE_PULL_BEHAVIOR` | <default | always | once | prefer-cached > | The behavior used to customize the pull image process. If `default` is specified, the image will be pulled remotely, if the pull fails then the cached image in the instance will be used. If `always` is specified, the image will be pulled remotely, if the pull fails then the task will fail. If `once` is specified, the image will be pulled remotely if it has not been pulled before or if the image was removed by image cleanup, otherwise the cached image in the instance will be used. If `prefer-cached` is specified, the image will be pulled remotely if there is no cached image, otherwise the cached image in the instance will be used. | default | default |
 | `ECS_INSTANCE_ATTRIBUTES` | `{"stack": "prod"}` | These attributes take effect only during initial registration. After the agent has joined an ECS cluster, use the PutAttributes API action to add additional attributes. For more information, see [Amazon ECS Container Agent Configuration](http://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-agent-config.html) in the Amazon ECS Developer Guide.| `{}` | `{}` |
 | `ECS_ENABLE_TASK_ENI` | `false` | Whether to enable task networking for task to be launched with its own network interface | `false` | Not applicable |
 | `ECS_CNI_PLUGINS_PATH` | `/ecs/cni` | The path where the cni binary file is located | `/amazon-ecs-cni-plugins` | Not applicable |

agent/config/config.go

@@ -65,7 +65,7 @@ const (
 	// image cleanup.
 	DefaultNumImagesToDeletePerCycle = 5
 
-	//DefaultImageDeletionAge specifies the default value for minimum amount of elapsed time after an image
+	// DefaultImageDeletionAge specifies the default value for minimum amount of elapsed time after an image
 	// has been pulled before it can be deleted.
 	DefaultImageDeletionAge = 1 * time.Hour
 
@@ -102,6 +102,25 @@ const (
 	DefaultTaskMetadataBurstRate = 60
 )
 
+const (
+	// ImagePullDefaultBehavior specifies the behavior that if an image pull API call fails,
+	// agent tries to start from the Docker image cache anyway, assuming that the image has not changed.
+	ImagePullDefaultBehavior ImagePullBehaviorType = iota
+
+	// ImagePullAlwaysBehavior specifies the behavior that if an image pull API call fails,
+	// the task fails instead of using cached image.
+	ImagePullAlwaysBehavior
+
+	// ImagePullOnceBehavior specifies the behavior that agent will only attempt to pull
+	// the same image once, once an image is pulled, local image cache will be used
+	// for all the containers.
+	ImagePullOnceBehavior
+
+	// ImagePullPreferCachedBehavior specifies the behavior that agent will only attempt to pull
+	// the image if there is no cached image.
+	ImagePullPreferCachedBehavior
+)
+
 var (
 	// DefaultPauseContainerImageName is the name of the pause container image. The linker's
 	// load flags are used to populate this value from the Makefile
@@ -386,6 +405,7 @@ func environmentConfig() (Config, error) {
 		MinimumImageDeletionAge:          parseEnvVariableDuration("ECS_IMAGE_MINIMUM_CLEANUP_AGE"),
 		ImageCleanupInterval:             parseEnvVariableDuration("ECS_IMAGE_CLEANUP_INTERVAL"),
 		NumImagesToDeletePerCycle:        parseNumImagesToDeletePerCycle(),
+		ImagePullBehavior:                parseImagePullBehavior(),
 		InstanceAttributes:               instanceAttributes,
 		CNIPluginsPath:                   os.Getenv("ECS_CNI_PLUGINS_PATH"),
 		AWSVPCBlockInstanceMetdata:       utils.ParseBool(os.Getenv("ECS_AWSVPC_BLOCK_IMDS"), false),

agent/config/config_test.go

@@ -81,6 +81,7 @@ func TestEnvironmentConfig(t *testing.T) {
 	defer setTestEnv("ECS_IMAGE_CLEANUP_INTERVAL", "2h")()
 	defer setTestEnv("ECS_IMAGE_MINIMUM_CLEANUP_AGE", "30m")()
 	defer setTestEnv("ECS_NUM_IMAGES_DELETE_PER_CYCLE", "2")()
+	defer setTestEnv("ECS_IMAGE_PULL_BEHAVIOR", "always")()
 	defer setTestEnv("ECS_INSTANCE_ATTRIBUTES", "{\"my_attribute\": \"testing\"}")()
 	defer setTestEnv("ECS_ENABLE_TASK_ENI", "true")()
 	defer setTestEnv("ECS_TASK_METADATA_RPS_LIMIT", "1000,1100")()
@@ -112,6 +113,7 @@ func TestEnvironmentConfig(t *testing.T) {
 	assert.Equal(t, (30 * time.Minute), conf.MinimumImageDeletionAge)
 	assert.Equal(t, (2 * time.Hour), conf.ImageCleanupInterval)
 	assert.Equal(t, 2, conf.NumImagesToDeletePerCycle)
+	assert.Equal(t, ImagePullAlwaysBehavior, conf.ImagePullBehavior)
 	assert.Equal(t, "testing", conf.InstanceAttributes["my_attribute"])
 	assert.Equal(t, (90 * time.Second), conf.TaskCleanupWaitDuration)
 	serializedAdditionalLocalRoutesJSON, err := json.Marshal(conf.AWSVPCAdditionalLocalRoutes)
@@ -369,6 +371,56 @@ func TestImageCleanupMinimumNumImagesToDeletePerCycle(t *testing.T) {
 	assert.Equal(t, cfg.NumImagesToDeletePerCycle, DefaultNumImagesToDeletePerCycle, "Wrong value for NumImagesToDeletePerCycle")
 }
 
+func TestInvalidImagePullBehavior(t *testing.T) {
+	defer setTestRegion()()
+	defer setTestEnv("ECS_IMAGE_PULL_BEHAVIOR", "invalid")()
+	cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient())
+	assert.NoError(t, err)
+	assert.Equal(t, cfg.ImagePullBehavior, ImagePullDefaultBehavior, "Wrong value for ImagePullBehavior")
+}
+
+func TestParseImagePullBehavior(t *testing.T) {
+	testcases := []struct {
+		name                      string
+		envVarVal                 string
+		expectedImagePullBehavior ImagePullBehaviorType
+	}{
+		{
+			name:                      "default agent behavior",
+			envVarVal:                 "default",
+			expectedImagePullBehavior: ImagePullDefaultBehavior,
+		},
+		{
+			name:                      "always agent behavior",
+			envVarVal:                 "always",
+			expectedImagePullBehavior: ImagePullAlwaysBehavior,
+		},
+		{
+			name:                      "once agent behavior",
+			envVarVal:                 "once",
+			expectedImagePullBehavior: ImagePullOnceBehavior,
+		},
+		{
+			name:                      "prefer-cached agent behavior",
+			envVarVal:                 "prefer-cached",
+			expectedImagePullBehavior: ImagePullPreferCachedBehavior,
+		},
+		{
+			name:                      "invalid agent behavior",
+			envVarVal:                 "invalid",
+			expectedImagePullBehavior: ImagePullDefaultBehavior,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			defer setTestRegion()()
+			defer setTestEnv("ECS_IMAGE_PULL_BEHAVIOR", tc.envVarVal)()
+			assert.Equal(t, parseImagePullBehavior(), tc.expectedImagePullBehavior, "Wrong value for ImagePullBehavior")
+		})
+	}
+}
+
 func TestTaskResourceLimitsOverride(t *testing.T) {
 	defer setTestRegion()()
 	defer setTestEnv("ECS_ENABLE_TASK_CPU_MEM_LIMIT", "false")()

agent/config/parse.go

@@ -117,6 +117,22 @@ func parseNumImagesToDeletePerCycle() int {
 	return numImagesToDeletePerCycle
 }
 
+func parseImagePullBehavior() ImagePullBehaviorType {
+	ImagePullBehaviorString := os.Getenv("ECS_IMAGE_PULL_BEHAVIOR")
+	switch ImagePullBehaviorString {
+	case "always":
+		return ImagePullAlwaysBehavior
+	case "once":
+		return ImagePullOnceBehavior
+	case "prefer-cached":
+		return ImagePullPreferCachedBehavior
+	default:
+		// Use the default image pull behavior when ECS_IMAGE_PULL_BEHAVIOR is
+		// "default" or not valid
+		return ImagePullDefaultBehavior
+	}
+}
+
 func parseInstanceAttributes(errs []error) (map[string]string, []error) {
 	var instanceAttributes map[string]string
 	instanceAttributesEnv := os.Getenv("ECS_INSTANCE_ATTRIBUTES")

agent/config/types.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
@@ -20,6 +20,10 @@ import (
 	cnitypes "github.com/containernetworking/cni/pkg/types"
 )
 
+// ImagePullBehaviorType is an enum variable type corresponding to different agent pull
+// behaviors including default, always, never and once.
+type ImagePullBehaviorType int8
+
 type Config struct {
 	// DEPRECATED
 	// ClusterArn is the Name or full ARN of a Cluster to register into. It has
@@ -149,6 +153,10 @@ type Config struct {
 	// when Agent performs cleanup
 	NumImagesToDeletePerCycle int
 
+	// ImagePullBehavior specifies the agent's behavior for pulling image and loading
+	// local Docker image cache
+	ImagePullBehavior ImagePullBehaviorType
+
 	// InstanceAttributes contains key/value pairs representing
 	// attributes to be associated with this instance within the
 	// ECS service and used to influence behavior such as launch

agent/dockerclient/dockerapi/docker_client_test.go

@@ -441,7 +441,7 @@ func TestStartContainerTimeout(t *testing.T) {
 	wait.Add(1)
 	mockDocker.EXPECT().StartContainerWithContext("id", nil, gomock.Any()).Do(func(x, y, z interface{}) {
 		wait.Wait() // wait until timeout happens
-	})
+	}).MaxTimes(1)
 	mockDocker.EXPECT().InspectContainerWithContext("id", gomock.Any()).Return(nil, errors.New("test error")).AnyTimes()
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()

agent/engine/common_test.go

@@ -136,7 +136,7 @@ func validateContainerRunWorkflow(t *testing.T,
 	imageManager.EXPECT().AddAllImageStates(gomock.Any()).AnyTimes()
 	client.EXPECT().PullImage(container.Image, nil).Return(dockerapi.DockerContainerMetadata{})
 	imageManager.EXPECT().RecordContainerReference(container).Return(nil)
-	imageManager.EXPECT().GetImageStateFromImageName(gomock.Any()).Return(nil)
+	imageManager.EXPECT().GetImageStateFromImageName(gomock.Any()).Return(nil, false)
 	client.EXPECT().APIVersion().Return(defaultDockerClientAPIVersion, nil)
 	dockerConfig, err := task.DockerConfig(container, defaultDockerClientAPIVersion)
 	if err != nil {

agent/engine/docker_image_manager.go

@@ -14,13 +14,12 @@
 package engine
 
 import (
+	"context"
 	"fmt"
 	"sort"
 	"sync"
 	"time"
 
-	"context"
-
 	"github.com/aws/amazon-ecs-agent/agent/api"
 	"github.com/aws/amazon-ecs-agent/agent/config"
 	"github.com/aws/amazon-ecs-agent/agent/dockerclient"
@@ -41,7 +40,7 @@ type ImageManager interface {
 	RecordContainerReference(container *api.Container) error
 	RemoveContainerReferenceFromImageState(container *api.Container) error
 	AddAllImageStates(imageStates []*image.ImageState)
-	GetImageStateFromImageName(containerImageName string) *image.ImageState
+	GetImageStateFromImageName(containerImageName string) (*image.ImageState, bool)
 	StartImageCleanupProcess(ctx context.Context)
 	SetSaver(stateManager statemanager.Saver)
 }
@@ -59,6 +58,7 @@ type dockerImageManager struct {
 	minimumAgeBeforeDeletion         time.Duration
 	numImagesToDelete                int
 	imageCleanupTimeInterval         time.Duration
+	imagePullBehavior                config.ImagePullBehaviorType
 }
 
 // ImageStatesForDeletion is used for implementing the sort interface
@@ -72,6 +72,7 @@ func NewImageManager(cfg *config.Config, client dockerapi.DockerClient, state do
 		minimumAgeBeforeDeletion: cfg.MinimumImageDeletionAge,
 		numImagesToDelete:        cfg.NumImagesToDeletePerCycle,
 		imageCleanupTimeInterval: cfg.ImageCleanupInterval,
+		imagePullBehavior:        cfg.ImagePullBehavior,
 	}
 }
 
@@ -267,6 +268,12 @@ func (imageManager *dockerImageManager) removeExistingImageNameOfDifferentID(con
 }
 
 func (imageManager *dockerImageManager) StartImageCleanupProcess(ctx context.Context) {
+	// If the image pull behavior is prefer cached, don't clean up the image,
+	// because the cached image is needed.
+	if imageManager.imagePullBehavior == config.ImagePullPreferCachedBehavior {
+		seelog.Info("Pull behavior is set to always use cache. Disabling cleanup")
+		return
+	}
 	// passing the cleanup interval as argument which would help during testing
 	imageManager.performPeriodicImageCleanup(ctx, imageManager.imageCleanupTimeInterval)
 }
@@ -369,15 +376,15 @@ func (imageManager *dockerImageManager) deleteImage(ctx context.Context, imageID
 	}
 }
 
-func (imageManager *dockerImageManager) GetImageStateFromImageName(containerImageName string) *image.ImageState {
+func (imageManager *dockerImageManager) GetImageStateFromImageName(containerImageName string) (*image.ImageState, bool) {
 	imageManager.updateLock.Lock()
 	defer imageManager.updateLock.Unlock()
 	for _, imageState := range imageManager.getAllImageStates() {
 		for _, imageName := range imageState.Image.Names {
 			if imageName == containerImageName {
-				return imageState
+				return imageState, true
 			}
 		}
 	}
-	return nil
+	return nil, false
 }