Make pullImageTimeout configurable

README.md

@@ -159,6 +159,7 @@ additional details on each available environment variable.
 | `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_IMAGE_PULL_INACTIVITY_TIMEOUT` | 1m | The time to wait after docker pulls complete waiting for extraction of a container. Useful for tuning large Windows containers. | 1m | 3m |
+| `ECS_IMAGE_PULL_TIMEOUT` | 1h | The time to wait for pulling docker image. | 2h | 2h |
 | `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_ENABLE_HIGH_DENSITY_ENI` | `false` | Whether to enable high density eni feature when using task networking | `true` | Not applicable |

agent/config/config.go

@@ -87,6 +87,9 @@ const (
 	// has been created before it can be deleted
 	DefaultNonECSImageDeletionAge = 1 * time.Hour
 
+	//DefaultImagePullTimeout specifies the timeout for PullImage API.
+	DefaultImagePullTimeout = 2 * time.Hour
+
 	// minimumTaskCleanupWaitDuration specifies the minimum duration to wait before cleaning up
 	// a task's container. This is used to enforce sane values for the config.TaskCleanupWaitDuration field.
 	minimumTaskCleanupWaitDuration = 1 * time.Minute
@@ -531,6 +534,7 @@ func environmentConfig() (Config, error) {
 		DockerStopTimeout:                   parseDockerStopTimeout(),
 		ContainerStartTimeout:               parseContainerStartTimeout(),
 		ImagePullInactivityTimeout:          parseImagePullInactivityTimeout(),
+		ImagePullTimeout:                    parseEnvVariableDuration("ECS_IMAGE_PULL_TIMEOUT"),
 		CredentialsAuditLogFile:             os.Getenv("ECS_AUDIT_LOGFILE"),
 		CredentialsAuditLogDisabled:         utils.ParseBool(os.Getenv("ECS_AUDIT_LOGFILE_DISABLED"), false),
 		TaskIAMRoleEnabledForNetworkHost:    utils.ParseBool(os.Getenv("ECS_ENABLE_TASK_IAM_ROLE_NETWORK_HOST"), false),

agent/config/config_unix.go

@@ -63,6 +63,7 @@ func DefaultConfig() Config {
 		NonECSMinimumImageDeletionAge:       DefaultNonECSImageDeletionAge,
 		ImageCleanupInterval:                DefaultImageCleanupTimeInterval,
 		ImagePullInactivityTimeout:          defaultImagePullInactivityTimeout,
+		ImagePullTimeout:                    DefaultImagePullTimeout,
 		NumImagesToDeletePerCycle:           DefaultNumImagesToDeletePerCycle,
 		NumNonECSContainersToDeletePerCycle: DefaultNumNonECSContainersToDeletePerCycle,
 		CNIPluginsPath:                      defaultCNIPluginsPath,

agent/config/config_unix_test.go

@@ -67,6 +67,7 @@ func TestConfigDefault(t *testing.T) {
 		"Default TaskMetadataBurstRate is set incorrectly")
 	assert.False(t, cfg.SharedVolumeMatchFullConfig.Enabled(), "Default SharedVolumeMatchFullConfig set incorrectly")
 	assert.Equal(t, defaultCgroupCPUPeriod, cfg.CgroupCPUPeriod, "CFS cpu period set incorrectly")
+	assert.Equal(t, DefaultImagePullTimeout, cfg.ImagePullTimeout, "Default ImagePullTimeout set incorrectly")
 }
 
 // TestConfigFromFile tests the configuration can be read from file

agent/config/config_windows.go

@@ -87,6 +87,7 @@ func DefaultConfig() Config {
 		DockerStopTimeout:                   defaultDockerStopTimeout,
 		ContainerStartTimeout:               defaultContainerStartTimeout,
 		ImagePullInactivityTimeout:          defaultImagePullInactivityTimeout,
+		ImagePullTimeout:                    DefaultImagePullTimeout,
 		CredentialsAuditLogFile:             filepath.Join(ecsRoot, defaultCredentialsAuditLogFile),
 		CredentialsAuditLogDisabled:         false,
 		ImageCleanupDisabled:                BooleanDefaultFalse{Value: ExplicitlyDisabled},

agent/config/config_windows_test.go

@@ -60,6 +60,7 @@ func TestConfigDefault(t *testing.T) {
 	assert.Equal(t, DefaultTaskMetadataBurstRate, cfg.TaskMetadataBurstRate,
 		"Default TaskMetadataBurstRate is set incorrectly")
 	assert.False(t, cfg.SharedVolumeMatchFullConfig.Enabled(), "Default SharedVolumeMatchFullConfig set incorrectly")
+	assert.Equal(t, DefaultImagePullTimeout, cfg.ImagePullTimeout, "Default ImagePullTimeout set incorrectly")
 }
 
 func TestConfigIAMTaskRolesReserves80(t *testing.T) {

agent/config/types.go

@@ -115,6 +115,9 @@ type Config struct {
 	// ImagePullInactivityTimeout is here to override the amount of time to wait when pulling and extracting a container
 	ImagePullInactivityTimeout time.Duration
 
+	//ImagePullTimeout is here to override the timeout for PullImage API
+	ImagePullTimeout time.Duration
+
 	// AvailableLoggingDrivers specifies the logging drivers available for use
 	// with Docker.  If not set, it defaults to ["json-file","none"].
 	AvailableLoggingDrivers []dockerclient.LoggingDriver

agent/dockerclient/dockerapi/docker_client_test.go

@@ -141,7 +141,7 @@ func TestPullImageOutputTimeout(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
-	metadata := client.PullImage(ctx, "image", nil, dockerclient.PullImageTimeout)
+	metadata := client.PullImage(ctx, "image", nil, defaultTestConfig().ImagePullTimeout)
 	assert.Error(t, metadata.Error, "Expected error for pull timeout")
 	assert.Equal(t, "DockerTimeoutError", metadata.Error.(apierrors.NamedError).ErrorName())
 }
@@ -192,7 +192,7 @@ func TestPullImageInactivityTimeout(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
-	metadata := client.PullImage(ctx, "image", nil, dockerclient.PullImageTimeout)
+	metadata := client.PullImage(ctx, "image", nil, defaultTestConfig().ImagePullTimeout)
 	assert.Error(t, metadata.Error, "Expected error for pull inactivity timeout")
 	assert.Equal(t, "CannotPullContainerError", metadata.Error.(apierrors.NamedError).ErrorName(), "Wrong error type")
 }
@@ -210,7 +210,7 @@ func TestImagePull(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
-	metadata := client.PullImage(ctx, "image", nil, dockerclient.PullImageTimeout)
+	metadata := client.PullImage(ctx, "image", nil, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 }
 
@@ -228,7 +228,7 @@ func TestImagePullTag(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
-	metadata := client.PullImage(ctx, "image:mytag", nil, dockerclient.PullImageTimeout)
+	metadata := client.PullImage(ctx, "image:mytag", nil, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 }
 
@@ -244,7 +244,7 @@ func TestImagePullDigest(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
-	metadata := client.PullImage(ctx, "image@sha256:bc8813ea7b3603864987522f02a76101c17ad122e1c46d790efc0fca78ca7bfb", nil, dockerclient.PullImageTimeout)
+	metadata := client.PullImage(ctx, "image@sha256:bc8813ea7b3603864987522f02a76101c17ad122e1c46d790efc0fca78ca7bfb", nil, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 }
 
@@ -290,7 +290,7 @@ func TestPullImageECRSuccess(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
-	metadata := client.PullImage(ctx, image, authData, dockerclient.PullImageTimeout)
+	metadata := client.PullImage(ctx, image, authData, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 }
 
@@ -335,7 +335,7 @@ func TestPullImageECRAuthFail(t *testing.T) {
 	ecrClientFactory.EXPECT().GetClient(authData.ECRAuthData).Return(ecrClient, nil)
 	ecrClient.EXPECT().GetAuthorizationToken(gomock.Any()).Return(nil, errors.New("test error"))
 
-	metadata := client.PullImage(ctx, image, authData, dockerclient.PullImageTimeout)
+	metadata := client.PullImage(ctx, image, authData, defaultTestConfig().ImagePullTimeout)
 	assert.Error(t, metadata.Error, "expected pull to fail")
 }
 
@@ -355,7 +355,7 @@ func TestPullImageError(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
-	metadata := client.PullImage(ctx, "image", nil, dockerclient.PullImageTimeout)
+	metadata := client.PullImage(ctx, "image", nil, defaultTestConfig().ImagePullTimeout)
 	assert.Error(t, metadata.Error, "toomanyrequests: Rate exceeded")
 	assert.Equal(t, "CannotPullContainerError", metadata.Error.(apierrors.NamedError).ErrorName(), "Wrong error type")
 }
@@ -1286,19 +1286,19 @@ func TestECRAuthCacheWithoutExecutionRole(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
-	metadata := client.PullImage(ctx, image, authData, dockerclient.PullImageTimeout)
+	metadata := client.PullImage(ctx, image, authData, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 
 	// Pull from the same registry shouldn't expect ecr client call
-	metadata = client.PullImage(ctx, image+"2", authData, dockerclient.PullImageTimeout)
+	metadata = client.PullImage(ctx, image+"2", authData, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 
 	// Pull from the same registry shouldn't expect ecr client call
-	metadata = client.PullImage(ctx, image+"3", authData, dockerclient.PullImageTimeout)
+	metadata = client.PullImage(ctx, image+"3", authData, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 
 	// Pull from the same registry shouldn't expect ecr client call
-	metadata = client.PullImage(ctx, image+"4", authData, dockerclient.PullImageTimeout)
+	metadata = client.PullImage(ctx, image+"4", authData, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 }
 
@@ -1342,7 +1342,7 @@ func TestECRAuthCacheForDifferentRegistry(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
-	metadata := client.PullImage(ctx, image, authData, dockerclient.PullImageTimeout)
+	metadata := client.PullImage(ctx, image, authData, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 
 	// Pull from the different registry should expect ECR client call
@@ -1354,7 +1354,7 @@ func TestECRAuthCacheForDifferentRegistry(t *testing.T) {
 			AuthorizationToken: aws.String(base64.StdEncoding.EncodeToString([]byte(username + ":" + password))),
 			ExpiresAt:          aws.Time(time.Now().Add(10 * time.Hour)),
 		}, nil).Times(1)
-	metadata = client.PullImage(ctx, image, authData, dockerclient.PullImageTimeout)
+	metadata = client.PullImage(ctx, image, authData, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 }
 
@@ -1401,15 +1401,15 @@ func TestECRAuthCacheWithSameExecutionRole(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
-	metadata := client.PullImage(ctx, image, authData, dockerclient.PullImageTimeout)
+	metadata := client.PullImage(ctx, image, authData, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 
 	// Pull from the same registry shouldn't expect ecr client call
-	metadata = client.PullImage(ctx, image+"2", authData, dockerclient.PullImageTimeout)
+	metadata = client.PullImage(ctx, image+"2", authData, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 
 	// Pull from the same registry shouldn't expect ecr client call
-	metadata = client.PullImage(ctx, image+"3", authData, dockerclient.PullImageTimeout)
+	metadata = client.PullImage(ctx, image+"3", authData, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 }
 
@@ -1456,7 +1456,7 @@ func TestECRAuthCacheWithDifferentExecutionRole(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
-	metadata := client.PullImage(ctx, image, authData, dockerclient.PullImageTimeout)
+	metadata := client.PullImage(ctx, image, authData, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 
 	// Pull from the same registry but with different role
@@ -1470,7 +1470,7 @@ func TestECRAuthCacheWithDifferentExecutionRole(t *testing.T) {
 			AuthorizationToken: aws.String(base64.StdEncoding.EncodeToString([]byte(username + ":" + password))),
 			ExpiresAt:          aws.Time(time.Now().Add(10 * time.Hour)),
 		}, nil).Times(1)
-	metadata = client.PullImage(ctx, image, authData, dockerclient.PullImageTimeout)
+	metadata = client.PullImage(ctx, image, authData, defaultTestConfig().ImagePullTimeout)
 	assert.NoError(t, metadata.Error, "Expected pull to succeed")
 }
 

agent/dockerclient/timeout.go

@@ -17,8 +17,6 @@ import "time"
 
 // Timelimits for docker operations enforced above docker
 const (
-	// PullImageTimeout is the timeout for the PullImage API
-	PullImageTimeout = 2 * time.Hour
 	// ListImagesTimeout is the timeout for the ListImages API
 	ListImagesTimeout = 10 * time.Minute
 	// LoadImageTimeout is the timeout for the LoadImage API. It's set

agent/engine/docker_image_manager_integ_test.go

@@ -449,7 +449,7 @@ func TestImageWithSameIDAndDifferentNames(t *testing.T) {
 
 	// Pull the images needed for the test
 	if _, err = dockerClient.InspectImage(test4Image1Name); client.IsErrNotFound(err) {
-		metadata := dockerClient.PullImage(ctx, test4Image1Name, nil, dockerclient.PullImageTimeout)
+		metadata := dockerClient.PullImage(ctx, test4Image1Name, nil, defaultTestConfigIntegTest().ImagePullTimeout)
 		assert.NoError(t, metadata.Error, "Failed to pull image %s", test4Image1Name)
 	}
 

agent/engine/docker_task_engine.go

@@ -871,7 +871,7 @@ func (engine *DockerTaskEngine) pullAndUpdateContainerReference(task *apitask.Ta
 		defer container.SetASMDockerAuthConfig(types.AuthConfig{})
 	}
 
-	metadata := engine.client.PullImage(engine.ctx, container.Image, container.RegistryAuthentication, dockerclient.PullImageTimeout)
+	metadata := engine.client.PullImage(engine.ctx, container.Image, container.RegistryAuthentication, engine.cfg.ImagePullTimeout)
 
 	// Don't add internal images(created by ecs-agent) into imagemanger state
 	if container.IsInternal() {