From eb9560b87034506504a2ddac2d308596fd1e4a6a Mon Sep 17 00:00:00 2001 From: haikuoliu Date: Mon, 26 Mar 2018 10:45:28 -0700 Subject: [PATCH] config/engine: distinct startContainerTimouts for windows/linux Test the start container time cost on windows instances with various parameters (instance type, container memory, container cpu, etc.), and set the default startContainerTimeouts value for windows accordingly. Also make the startContainerTimeouts configurable (via ECS_CONTAINER_START_TIMEOUT environment variable) by users. --- CHANGELOG.md | 1 + README.md | 1 + agent/config/config.go | 37 ++++++++++- agent/config/config_test.go | 65 +++++++++++++++---- agent/config/config_unix.go | 8 ++- agent/config/config_unix_test.go | 3 +- agent/config/config_windows.go | 8 ++- agent/config/config_windows_test.go | 3 +- agent/config/types.go | 5 +- agent/engine/common_test.go | 5 +- agent/engine/docker_client.go | 5 +- agent/engine/docker_client_test.go | 6 +- agent/engine/docker_events_buffer_test.go | 2 +- .../engine/docker_image_manager_integ_test.go | 2 +- agent/engine/docker_image_manager_test.go | 2 +- .../docker_image_manager_unix_integ_test.go | 2 +- ...docker_image_manager_windows_integ_test.go | 2 +- agent/engine/docker_task_engine.go | 3 +- agent/engine/docker_task_engine_test.go | 16 ++--- agent/engine/engine_integ_test.go | 2 +- agent/engine/engine_unix_integ_test.go | 2 +- agent/engine/engine_windows_integ_test.go | 2 +- agent/engine/errors_test.go | 2 +- 23 files changed, 138 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 434c48c079f..de1f33a51f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## 1.17.3-dev +* Enhancement - Distinct startContainerTimeouts for windows/linux, introduce a new environment variable `ECS_CONTAINER_START_TIMEOUT` to make it configurable [#1321](https://github.com/aws/amazon-ecs-agent/pull/1321) * Enhancement - Add support for containers to inhereit ENI private DNS hostnames for `awsvpc` tasks [#1278](https://github.com/aws/amazon-ecs-agent/pull/1278) * Enhancement - Expose task definition family and task definition revision in container metadata file [#1295](https://github.com/aws/amazon-ecs-agent/pull/1295) * Enhancement - Fail image pulls if there's inactivity during image pull progress [#1290](https://github.com/aws/amazon-ecs-agent/pull/1290) diff --git a/README.md b/README.md index 7f5d18c3641..6dbee12a576 100644 --- a/README.md +++ b/README.md @@ -162,6 +162,7 @@ additional details on each available environment variable. | `ECS_APPARMOR_CAPABLE` | `true` | Whether AppArmor is available on the container instance. | `false` | `false` | | `ECS_ENGINE_TASK_CLEANUP_WAIT_DURATION` | 10m | Time to wait to delete containers for a stopped task. If set to less than 1 minute, the value is ignored. | 3h | 3h | | `ECS_CONTAINER_STOP_TIMEOUT` | 10m | Time to wait for the container to exit normally before being forcibly killed. | 30s | 30s | +| `ECS_CONTAINER_START_TIMEOUT` | 10m | Timeout before giving up on starting a container. | 3m | 8m | | `ECS_ENABLE_TASK_IAM_ROLE` | `true` | Whether to enable IAM Roles for Tasks on the Container Instance | `false` | `false` | | `ECS_ENABLE_TASK_IAM_ROLE_NETWORK_HOST` | `true` | Whether to enable IAM Roles for Tasks when launched with `host` network mode on the Container Instance | `false` | `false` | | `ECS_DISABLE_IMAGE_CLEANUP` | `true` | Whether to disable automated image cleanup for the ECS Agent. | `false` | `false` | diff --git a/agent/config/config.go b/agent/config/config.go index c66208f9367..61d8ff429f5 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -55,8 +55,8 @@ const ( // clean up task's containers. DefaultTaskCleanupWaitDuration = 3 * time.Hour - // DefaultDockerStopTimeout specifies the value for container stop timeout duration - DefaultDockerStopTimeout = 30 * time.Second + // defaultDockerStopTimeout specifies the value for container stop timeout duration + defaultDockerStopTimeout = 30 * time.Second // DefaultImageCleanupTimeInterval specifies the default value for image cleanup duration. It is used to // remove the images pulled by agent. @@ -288,6 +288,8 @@ func environmentConfig() (Config, error) { dockerStopTimeout := getDockerStopTimeout() + containerStartTimeout := getContainerStartTimeout() + cgroupPath := os.Getenv("ECS_CGROUP_PATH") taskCleanupWaitDuration := parseEnvVariableDuration("ECS_ENGINE_TASK_CLEANUP_WAIT_DURATION") @@ -385,6 +387,7 @@ func environmentConfig() (Config, error) { TaskIAMRoleEnabled: taskIAMRoleEnabled, TaskCPUMemLimit: taskCPUMemLimitEnabled, DockerStopTimeout: dockerStopTimeout, + ContainerStartTimeout: containerStartTimeout, CredentialsAuditLogFile: credentialsAuditLogFile, CredentialsAuditLogDisabled: credentialsAuditLogDisabled, TaskIAMRoleEnabledForNetworkHost: taskIAMRoleEnabledForNetworkHost, @@ -423,12 +426,34 @@ func getDockerStopTimeout() time.Duration { parsedStopTimeout := parseEnvVariableDuration("ECS_CONTAINER_STOP_TIMEOUT") if parsedStopTimeout >= minimumDockerStopTimeout { dockerStopTimeout = parsedStopTimeout + // if the ECS_CONTAINER_STOP_TIMEOUT is invalid or empty, then the parsedStopTimeout + // will be 0, in this case we should return a 0, + // because the DockerStopTimeout will merge with the DefaultDockerStopTimeout, + // only when the DockerStopTimeout is empty } else if parsedStopTimeout != 0 { + // if the configured ECS_CONTAINER_STOP_TIMEOUT is smaller than minimumDockerStopTimeout, + // DockerStopTimeout will be set to minimumDockerStopTimeout + // if the ECS_CONTAINER_STOP_TIMEOUT is 0, empty or an invalid value, then DockerStopTimeout + // will be set to defaultDockerStopTimeout during the config merge operation + dockerStopTimeout = minimumDockerStopTimeout seelog.Warnf("Discarded invalid value for docker stop timeout, parsed as: %v", parsedStopTimeout) } return dockerStopTimeout } +func getContainerStartTimeout() time.Duration { + var containerStartTimeout time.Duration + parsedStartTimeout := parseEnvVariableDuration("ECS_CONTAINER_START_TIMEOUT") + if parsedStartTimeout >= minimumContainerStartTimeout { + containerStartTimeout = parsedStartTimeout + // do the parsedStartTimeout != 0 check for the same reason as in getDockerStopTimeout() + } else if parsedStartTimeout != 0 { + containerStartTimeout = minimumContainerStartTimeout + seelog.Warnf("Discarded invalid value for container start timeout, parsed as: %v", parsedStartTimeout) + } + return containerStartTimeout +} + func getTaskCPUMemLimitEnabled() Conditional { var taskCPUMemLimitEnabled Conditional taskCPUMemLimitConfigString := os.Getenv("ECS_ENABLE_TASK_CPU_MEM_LIMIT") @@ -563,7 +588,11 @@ func (cfg *Config) validateAndOverrideBounds() error { } if cfg.DockerStopTimeout < minimumDockerStopTimeout { - return fmt.Errorf("Invalid negative DockerStopTimeout: %v", cfg.DockerStopTimeout.String()) + return fmt.Errorf("config: invalid value for docker container stop timeout: %v", cfg.DockerStopTimeout.String()) + } + + if cfg.ContainerStartTimeout < minimumContainerStartTimeout { + return fmt.Errorf("config: invalid value for docker container start timeout: %v", cfg.ContainerStartTimeout.String()) } var badDrivers []string for _, driver := range cfg.AvailableLoggingDrivers { @@ -618,6 +647,7 @@ func (cfg *Config) String() string { "ReservedMem: %v, "+ "TaskCleanupWaitDuration: %v, "+ "DockerStopTimeout: %v, "+ + "ContainerStartTimeout: %v, "+ "TaskCPUMemLimit: %v, "+ "%s", cfg.Cluster, @@ -630,6 +660,7 @@ func (cfg *Config) String() string { cfg.ReservedMemory, cfg.TaskCleanupWaitDuration, cfg.DockerStopTimeout, + cfg.ContainerStartTimeout, cfg.TaskCPUMemLimit, cfg.platformString(), ) diff --git a/agent/config/config_test.go b/agent/config/config_test.go index d62c4c0349d..8d1a4e19951 100644 --- a/agent/config/config_test.go +++ b/agent/config/config_test.go @@ -1,4 +1,4 @@ -// Copyright 2014-2016 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 @@ -69,6 +69,7 @@ func TestEnvironmentConfig(t *testing.T) { defer setTestEnv("ECS_RESERVED_PORTS_UDP", "[42,99]")() defer setTestEnv("ECS_RESERVED_MEMORY", "20")() defer setTestEnv("ECS_CONTAINER_STOP_TIMEOUT", "60s")() + defer setTestEnv("ECS_CONTAINER_START_TIMEOUT", "5m")() defer setTestEnv("ECS_AVAILABLE_LOGGING_DRIVERS", "[\""+string(dockerclient.SyslogDriver)+"\"]")() defer setTestEnv("ECS_SELINUX_CAPABLE", "true")() defer setTestEnv("ECS_APPARMOR_CAPABLE", "true")() @@ -95,8 +96,10 @@ func TestEnvironmentConfig(t *testing.T) { assert.Contains(t, conf.ReservedPortsUDP, uint16(42)) assert.Contains(t, conf.ReservedPortsUDP, uint16(99)) assert.Equal(t, uint16(20), conf.ReservedMemory) - expectedDuration, _ := time.ParseDuration("60s") - assert.Equal(t, expectedDuration, conf.DockerStopTimeout) + expectedDurationDockerStopTimeout, _ := time.ParseDuration("60s") + assert.Equal(t, expectedDurationDockerStopTimeout, conf.DockerStopTimeout) + expectedDurationContainerStartTimeout, _ := time.ParseDuration("5m") + assert.Equal(t, expectedDurationContainerStartTimeout, conf.ContainerStartTimeout) assert.Equal(t, []dockerclient.LoggingDriver{dockerclient.SyslogDriver}, conf.AvailableLoggingDrivers) assert.True(t, conf.PrivilegedDisabled) assert.True(t, conf.SELinuxCapable, "Wrong value for SELinuxCapable") @@ -195,23 +198,61 @@ func TestCheckpointWithoutECSDataDir(t *testing.T) { func TestInvalidFormatDockerStopTimeout(t *testing.T) { defer setTestRegion()() defer setTestEnv("ECS_CONTAINER_STOP_TIMEOUT", "invalid")() - conf, err := environmentConfig() + ctrl := gomock.NewController(t) + mockEc2Metadata := mock_ec2.NewMockEC2MetadataClient(ctrl) + conf, err := NewConfig(mockEc2Metadata) assert.NoError(t, err) - assert.Zero(t, conf.DockerStopTimeout, "Wrong value for DockerStopTimeout") + assert.Equal(t, conf.DockerStopTimeout, defaultDockerStopTimeout, "Wrong value for DockerStopTimeout") } -func TestInvalideValueDockerStopTimeout(t *testing.T) { +func TestZeroValueDockerStopTimeout(t *testing.T) { + defer setTestRegion()() + defer setTestEnv("ECS_CONTAINER_STOP_TIMEOUT", "0s")() + ctrl := gomock.NewController(t) + mockEc2Metadata := mock_ec2.NewMockEC2MetadataClient(ctrl) + conf, err := NewConfig(mockEc2Metadata) + assert.NoError(t, err) + assert.Equal(t, conf.DockerStopTimeout, defaultDockerStopTimeout, "Wrong value for DockerStopTimeout") +} + +func TestInvalidValueDockerStopTimeout(t *testing.T) { defer setTestRegion()() defer setTestEnv("ECS_CONTAINER_STOP_TIMEOUT", "-10s")() - conf, err := environmentConfig() + ctrl := gomock.NewController(t) + mockEc2Metadata := mock_ec2.NewMockEC2MetadataClient(ctrl) + conf, err := NewConfig(mockEc2Metadata) assert.NoError(t, err) - assert.Zero(t, conf.DockerStopTimeout) + assert.Equal(t, conf.DockerStopTimeout, minimumDockerStopTimeout, "Wrong value for DockerStopTimeout") } -func TestInvalidDockerStopTimeout(t *testing.T) { - conf := DefaultConfig() - conf.DockerStopTimeout = -1 * time.Second - assert.Error(t, conf.validateAndOverrideBounds(), "Should be error with negative DockerStopTimeout") +func TestInvalidFormatContainerStartTimeout(t *testing.T) { + defer setTestRegion()() + defer setTestEnv("ECS_CONTAINER_START_TIMEOUT", "invalid")() + ctrl := gomock.NewController(t) + mockEc2Metadata := mock_ec2.NewMockEC2MetadataClient(ctrl) + conf, err := NewConfig(mockEc2Metadata) + assert.NoError(t, err) + assert.Equal(t, conf.ContainerStartTimeout, defaultContainerStartTimeout, "Wrong value for ContainerStartTimeout") +} + +func TestZeroValueContainerStartTimeout(t *testing.T) { + defer setTestRegion()() + defer setTestEnv("ECS_CONTAINER_START_TIMEOUT", "0s")() + ctrl := gomock.NewController(t) + mockEc2Metadata := mock_ec2.NewMockEC2MetadataClient(ctrl) + conf, err := NewConfig(mockEc2Metadata) + assert.NoError(t, err) + assert.Equal(t, conf.ContainerStartTimeout, defaultContainerStartTimeout, "Wrong value for ContainerStartTimeout") +} + +func TestInvalidValueContainerStartTimeout(t *testing.T) { + defer setTestRegion()() + defer setTestEnv("ECS_CONTAINER_START_TIMEOUT", "-10s")() + ctrl := gomock.NewController(t) + mockEc2Metadata := mock_ec2.NewMockEC2MetadataClient(ctrl) + conf, err := NewConfig(mockEc2Metadata) + assert.NoError(t, err) + assert.Equal(t, conf.ContainerStartTimeout, minimumContainerStartTimeout, "Wrong value for ContainerStartTimeout") } func TestInvalidFormatParseEnvVariableUint16(t *testing.T) { diff --git a/agent/config/config_unix.go b/agent/config/config_unix.go index 98db5c06004..217288aa7fd 100644 --- a/agent/config/config_unix.go +++ b/agent/config/config_unix.go @@ -16,6 +16,7 @@ package config import ( "fmt" + "time" "github.com/aws/amazon-ecs-agent/agent/engine/dockerclient" ) @@ -31,6 +32,10 @@ const ( // Default cgroup memory system root path, this is the default used if the // path has not been configured through ECS_CGROUP_PATH defaultCgroupPath = "/sys/fs/cgroup" + // defaultContainerStartTimeout specifies the value for container start timeout duration + defaultContainerStartTimeout = 3 * time.Minute + // minimumContainerStartTimeout specifies the minimum value for starting a container + minimumContainerStartTimeout = 45 * time.Second ) // DefaultConfig returns the default configuration for Linux @@ -45,7 +50,8 @@ func DefaultConfig() Config { ReservedMemory: 0, AvailableLoggingDrivers: []dockerclient.LoggingDriver{dockerclient.JSONFileDriver, dockerclient.NoneDriver}, TaskCleanupWaitDuration: DefaultTaskCleanupWaitDuration, - DockerStopTimeout: DefaultDockerStopTimeout, + DockerStopTimeout: defaultDockerStopTimeout, + ContainerStartTimeout: defaultContainerStartTimeout, CredentialsAuditLogFile: defaultCredentialsAuditLogFile, CredentialsAuditLogDisabled: false, ImageCleanupDisabled: false, diff --git a/agent/config/config_unix_test.go b/agent/config/config_unix_test.go index 68d71f0941c..8d585cd879d 100644 --- a/agent/config/config_unix_test.go +++ b/agent/config/config_unix_test.go @@ -1,5 +1,5 @@ // +build !windows -// 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 @@ -41,6 +41,7 @@ func TestConfigDefault(t *testing.T) { assert.Equal(t, 5, len(cfg.ReservedPorts), "Default reserved ports set incorrectly") assert.Equal(t, uint16(0), cfg.ReservedMemory, "Default reserved memory set incorrectly") assert.Equal(t, 30*time.Second, cfg.DockerStopTimeout, "Default docker stop container timeout set incorrectly") + assert.Equal(t, 3*time.Minute, cfg.ContainerStartTimeout, "Default docker start container timeout set incorrectly") assert.False(t, cfg.PrivilegedDisabled, "Default PrivilegedDisabled set incorrectly") assert.Equal(t, []dockerclient.LoggingDriver{dockerclient.JSONFileDriver, dockerclient.NoneDriver}, cfg.AvailableLoggingDrivers, "Default logging drivers set incorrectly") diff --git a/agent/config/config_windows.go b/agent/config/config_windows.go index 548f8ec1782..a1aa1f1b654 100644 --- a/agent/config/config_windows.go +++ b/agent/config/config_windows.go @@ -16,6 +16,7 @@ package config import ( "os" + "time" "path/filepath" "github.com/aws/amazon-ecs-agent/agent/engine/dockerclient" @@ -41,6 +42,10 @@ const ( dnsPort = 53 // NetBIOS over TCP/IP netBIOSPort = 139 + // defaultContainerStartTimeout specifies the value for container start timeout duration + defaultContainerStartTimeout = 8 * time.Minute + // minimumContainerStartTimeout specifies the minimum value for starting a container + minimumContainerStartTimeout = 2 * time.Minute ) // DefaultConfig returns the default configuration for Windows @@ -73,7 +78,8 @@ func DefaultConfig() Config { ReservedMemory: 0, AvailableLoggingDrivers: []dockerclient.LoggingDriver{dockerclient.JSONFileDriver, dockerclient.NoneDriver, dockerclient.AWSLogsDriver}, TaskCleanupWaitDuration: DefaultTaskCleanupWaitDuration, - DockerStopTimeout: DefaultDockerStopTimeout, + DockerStopTimeout: defaultDockerStopTimeout, + ContainerStartTimeout: defaultContainerStartTimeout, CredentialsAuditLogFile: filepath.Join(ecsRoot, defaultCredentialsAuditLogFile), CredentialsAuditLogDisabled: false, ImageCleanupDisabled: false, diff --git a/agent/config/config_windows_test.go b/agent/config/config_windows_test.go index 087f503f807..64529a48988 100644 --- a/agent/config/config_windows_test.go +++ b/agent/config/config_windows_test.go @@ -1,5 +1,5 @@ // !build windows -// 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 @@ -38,6 +38,7 @@ func TestConfigDefault(t *testing.T) { assert.Equal(t, 10, len(cfg.ReservedPorts), "Default reserved ports set incorrectly") assert.Equal(t, uint16(0), cfg.ReservedMemory, "Default reserved memory set incorrectly") assert.Equal(t, 30*time.Second, cfg.DockerStopTimeout, "Default docker stop container timeout set incorrectly") + assert.Equal(t, 8*time.Minute, cfg.ContainerStartTimeout, "Default docker start container timeout set incorrectly") assert.False(t, cfg.PrivilegedDisabled, "Default PrivilegedDisabled set incorrectly") assert.Equal(t, []dockerclient.LoggingDriver{dockerclient.JSONFileDriver, dockerclient.NoneDriver, dockerclient.AWSLogsDriver}, cfg.AvailableLoggingDrivers, "Default logging drivers set incorrectly") diff --git a/agent/config/types.go b/agent/config/types.go index b9a7ec5a315..7f3465e3700 100644 --- a/agent/config/types.go +++ b/agent/config/types.go @@ -85,10 +85,13 @@ type Config struct { // other than containers managed by ECS ReservedMemory uint16 - // DockerStopTimeout specifies the amount time before a SIGKILL is issued to + // DockerStopTimeout specifies the amount of time before a SIGKILL is issued to // containers managed by ECS DockerStopTimeout time.Duration + // ContainerStartTimeout specifies the amount of time to wait to start a container + ContainerStartTimeout time.Duration + // AvailableLoggingDrivers specifies the logging drivers available for use // with Docker. If not set, it defaults to ["json-file","none"]. AvailableLoggingDrivers []dockerclient.LoggingDriver diff --git a/agent/engine/common_test.go b/agent/engine/common_test.go index 3ea13572cad..414be8b0480 100644 --- a/agent/engine/common_test.go +++ b/agent/engine/common_test.go @@ -24,6 +24,7 @@ import ( "github.com/aws/amazon-ecs-agent/agent/api" "github.com/aws/amazon-ecs-agent/agent/credentials" + "github.com/aws/amazon-ecs-agent/agent/config" "github.com/aws/amazon-ecs-agent/agent/engine/dockerclient" "github.com/aws/amazon-ecs-agent/agent/statechange" "github.com/aws/amazon-ecs-agent/agent/utils/ttime/mocks" @@ -162,8 +163,8 @@ func validateContainerRunWorkflow(t *testing.T, containerEventsWG.Done() }() }).Return(DockerContainerMetadata{DockerID: containerID}) - - client.EXPECT().StartContainer(containerID, startContainerTimeout).Do( + defaultConfig := config.DefaultConfig() + client.EXPECT().StartContainer(containerID, defaultConfig.ContainerStartTimeout).Do( func(id string, timeout time.Duration) { containerEventsWG.Add(1) go func() { diff --git a/agent/engine/docker_client.go b/agent/engine/docker_client.go index d69ad1dc2cf..9deafba31e0 100644 --- a/agent/engine/docker_client.go +++ b/agent/engine/docker_client.go @@ -67,7 +67,6 @@ const ( LoadImageTimeout = 10 * time.Minute pullImageTimeout = 2 * time.Hour createContainerTimeout = 4 * time.Minute - startContainerTimeout = 3 * time.Minute stopContainerTimeout = 30 * time.Second removeContainerTimeout = 5 * time.Minute inspectContainerTimeout = 30 * time.Second @@ -531,8 +530,8 @@ func (dg *dockerGoClient) createContainer(ctx context.Context, func (dg *dockerGoClient) StartContainer(id string, timeout time.Duration) DockerContainerMetadata { // Create a context that times out after the 'timeout' duration - // This is defined by the const 'startContainerTimeout'. Injecting the 'timeout' - // makes it easier to write tests. + // This is defined by the const 'ContainerStartTimeout' in config. Injecting + // the 'timeout' makes it easier to write tests. // Eventually, the context should be initialized from a parent root context // instead of TODO. ctx, cancel := context.WithTimeout(context.TODO(), timeout) diff --git a/agent/engine/docker_client_test.go b/agent/engine/docker_client_test.go index 0cba63ab902..bd32b20acf5 100644 --- a/agent/engine/docker_client_test.go +++ b/agent/engine/docker_client_test.go @@ -462,7 +462,7 @@ func TestStartContainer(t *testing.T) { mockDocker.EXPECT().StartContainerWithContext("id", nil, gomock.Any()).Return(nil), mockDocker.EXPECT().InspectContainerWithContext("id", gomock.Any()).Return(&docker.Container{ID: "id"}, nil), ) - metadata := client.StartContainer("id", startContainerTimeout) + metadata := client.StartContainer("id", defaultConfig.ContainerStartTimeout) if metadata.Error != nil { t.Error("Did not expect error") } @@ -804,7 +804,7 @@ func TestUsesVersionedClient(t *testing.T) { mockDocker.EXPECT().StartContainerWithContext(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockDocker.EXPECT().InspectContainerWithContext(gomock.Any(), gomock.Any()).Return(nil, errors.New("err")) - vclient.StartContainer("foo", startContainerTimeout) + vclient.StartContainer("foo", defaultConfig.ContainerStartTimeout) } func TestUnavailableVersionError(t *testing.T) { @@ -823,7 +823,7 @@ func TestUnavailableVersionError(t *testing.T) { factory.EXPECT().GetClient(dockerclient.DockerVersion("1.21")).Times(1).Return(nil, errors.New("Cannot get client")) - metadata := vclient.StartContainer("foo", startContainerTimeout) + metadata := vclient.StartContainer("foo", defaultConfig.ContainerStartTimeout) if metadata.Error == nil { t.Fatal("Expected error, didn't get one") diff --git a/agent/engine/docker_events_buffer_test.go b/agent/engine/docker_events_buffer_test.go index e3bf5f07cac..26618d20e2e 100644 --- a/agent/engine/docker_events_buffer_test.go +++ b/agent/engine/docker_events_buffer_test.go @@ -1,5 +1,5 @@ // +build !integration -// Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// Copyright 2017 - 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 diff --git a/agent/engine/docker_image_manager_integ_test.go b/agent/engine/docker_image_manager_integ_test.go index cc0af4a2d91..64869815813 100644 --- a/agent/engine/docker_image_manager_integ_test.go +++ b/agent/engine/docker_image_manager_integ_test.go @@ -1,5 +1,5 @@ // +build integration -// 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 diff --git a/agent/engine/docker_image_manager_test.go b/agent/engine/docker_image_manager_test.go index 8c40d1c4616..a6c439b8e1f 100644 --- a/agent/engine/docker_image_manager_test.go +++ b/agent/engine/docker_image_manager_test.go @@ -1,5 +1,5 @@ // +build !integration -// 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 diff --git a/agent/engine/docker_image_manager_unix_integ_test.go b/agent/engine/docker_image_manager_unix_integ_test.go index 01ebef9f459..7b4a8e2b620 100644 --- a/agent/engine/docker_image_manager_unix_integ_test.go +++ b/agent/engine/docker_image_manager_unix_integ_test.go @@ -1,5 +1,5 @@ // +build !windows,integration -// Copyright 2014-2016 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 diff --git a/agent/engine/docker_image_manager_windows_integ_test.go b/agent/engine/docker_image_manager_windows_integ_test.go index a0e3f114b36..1c1346296fe 100644 --- a/agent/engine/docker_image_manager_windows_integ_test.go +++ b/agent/engine/docker_image_manager_windows_integ_test.go @@ -1,5 +1,5 @@ // +build windows,integration -// Copyright 2014-2016 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 diff --git a/agent/engine/docker_task_engine.go b/agent/engine/docker_task_engine.go index 183ffbf0da0..87ea7367ebf 100644 --- a/agent/engine/docker_task_engine.go +++ b/agent/engine/docker_task_engine.go @@ -862,7 +862,8 @@ func (engine *DockerTaskEngine) startContainer(task *api.Task, container *api.Co }, } } - dockerContainerMD := client.StartContainer(dockerContainer.DockerID, startContainerTimeout) + dockerContainerMD := client.StartContainer(dockerContainer.DockerID, engine.cfg.ContainerStartTimeout) + // Get metadata through container inspection and available task information then write this to the metadata file // Performs this in the background to avoid delaying container start diff --git a/agent/engine/docker_task_engine_test.go b/agent/engine/docker_task_engine_test.go index aa403d0b205..860ba32bd89 100644 --- a/agent/engine/docker_task_engine_test.go +++ b/agent/engine/docker_task_engine_test.go @@ -351,7 +351,7 @@ func TestTaskWithSteadyStateResourcesProvisioned(t *testing.T) { }() }).Return(DockerContainerMetadata{DockerID: containerID + ":" + pauseContainer.Name}), // Ensure that the pause container is started after it's created - client.EXPECT().StartContainer(containerID+":"+pauseContainer.Name, startContainerTimeout).Do( + client.EXPECT().StartContainer(containerID+":"+pauseContainer.Name, defaultConfig.ContainerStartTimeout).Do( func(id string, timeout time.Duration) { containerEventsWG.Add(1) go func() { @@ -379,7 +379,7 @@ func TestTaskWithSteadyStateResourcesProvisioned(t *testing.T) { }() }).Return(DockerContainerMetadata{DockerID: containerID + ":" + sleepContainer.Name}), // Next, the sleep container is started - client.EXPECT().StartContainer(containerID+":"+sleepContainer.Name, startContainerTimeout).Do( + client.EXPECT().StartContainer(containerID+":"+sleepContainer.Name, defaultConfig.ContainerStartTimeout).Do( func(id string, timeout time.Duration) { containerEventsWG.Add(1) go func() { @@ -521,7 +521,7 @@ func TestStartTimeoutThenStart(t *testing.T) { go func() { eventStream <- createDockerEvent(api.ContainerCreated) }() }).Return(DockerContainerMetadata{DockerID: containerID}) - client.EXPECT().StartContainer(containerID, startContainerTimeout).Return(DockerContainerMetadata{ + client.EXPECT().StartContainer(containerID, defaultConfig.ContainerStartTimeout).Return(DockerContainerMetadata{ Error: &DockerTimeoutError{}, }) } @@ -790,7 +790,7 @@ func TestTaskTransitionWhenStopContainerTimesout(t *testing.T) { }).Return(DockerContainerMetadata{DockerID: containerID}) gomock.InOrder( - client.EXPECT().StartContainer(containerID, startContainerTimeout).Do( + client.EXPECT().StartContainer(containerID, defaultConfig.ContainerStartTimeout).Do( func(id string, timeout time.Duration) { go func() { eventStream <- createDockerEvent(api.ContainerRunning) @@ -886,7 +886,7 @@ func TestTaskTransitionWhenStopContainerReturnsUnretriableError(t *testing.T) { }() }).Return(DockerContainerMetadata{DockerID: containerID}), // Simulate successful start container - client.EXPECT().StartContainer(containerID, startContainerTimeout).Do( + client.EXPECT().StartContainer(containerID, defaultConfig.ContainerStartTimeout).Do( func(id string, timeout time.Duration) { containerEventsWG.Add(1) go func() { @@ -955,7 +955,7 @@ func TestTaskTransitionWhenStopContainerReturnsTransientErrorBeforeSucceeding(t client.EXPECT().CreateContainer(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return( DockerContainerMetadata{DockerID: containerID}), // Simulate successful start container - client.EXPECT().StartContainer(containerID, startContainerTimeout).Return( + client.EXPECT().StartContainer(containerID, defaultConfig.ContainerStartTimeout).Return( DockerContainerMetadata{DockerID: containerID}), // StopContainer errors out a couple of times client.EXPECT().StopContainer(containerID, gomock.Any()).Return(containerStoppingError).Times(2), @@ -1073,7 +1073,7 @@ func TestPauseContainerHappyPath(t *testing.T) { assert.True(t, ok) assert.Equal(t, api.PauseContainerName, name) }).Return(DockerContainerMetadata{DockerID: "pauseContainerID"}), - dockerClient.EXPECT().StartContainer(pauseContainerID, startContainerTimeout).Return( + dockerClient.EXPECT().StartContainer(pauseContainerID, defaultConfig.ContainerStartTimeout).Return( DockerContainerMetadata{DockerID: "pauseContainerID"}), dockerClient.EXPECT().InspectContainer(gomock.Any(), gomock.Any()).Return( &docker.Container{ @@ -1091,7 +1091,7 @@ func TestPauseContainerHappyPath(t *testing.T) { dockerClient.EXPECT().APIVersion().Return(defaultDockerClientAPIVersion, nil) dockerClient.EXPECT().CreateContainer(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(DockerContainerMetadata{DockerID: containerID}) - dockerClient.EXPECT().StartContainer(containerID, startContainerTimeout).Return( + dockerClient.EXPECT().StartContainer(containerID, defaultConfig.ContainerStartTimeout).Return( DockerContainerMetadata{DockerID: containerID}) cleanup := make(chan time.Time) diff --git a/agent/engine/engine_integ_test.go b/agent/engine/engine_integ_test.go index 845674167fb..71fd253a9b5 100644 --- a/agent/engine/engine_integ_test.go +++ b/agent/engine/engine_integ_test.go @@ -1,5 +1,5 @@ // +build integration -// 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 diff --git a/agent/engine/engine_unix_integ_test.go b/agent/engine/engine_unix_integ_test.go index 8248e6482c1..66baf6fc228 100644 --- a/agent/engine/engine_unix_integ_test.go +++ b/agent/engine/engine_unix_integ_test.go @@ -1,6 +1,6 @@ // +build !windows,integration -// 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 diff --git a/agent/engine/engine_windows_integ_test.go b/agent/engine/engine_windows_integ_test.go index a4a56acb363..af74ef7cf43 100644 --- a/agent/engine/engine_windows_integ_test.go +++ b/agent/engine/engine_windows_integ_test.go @@ -1,6 +1,6 @@ // +build windows,integration -// 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 diff --git a/agent/engine/errors_test.go b/agent/engine/errors_test.go index 59d07f34dfe..075f7d59c90 100644 --- a/agent/engine/errors_test.go +++ b/agent/engine/errors_test.go @@ -1,5 +1,5 @@ // +build !integration -// 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