From e36e2c8e2d5bebfaf6973735f7f25b7c98b8cbb8 Mon Sep 17 00:00:00 2001 From: Utsa Bhattacharjya Date: Mon, 28 Jan 2019 12:39:01 -0800 Subject: [PATCH 01/24] Model changes for ACS for container ordering --- agent/acs/model/api/api-2.json | 23 +- agent/acs/model/ecsacs/api.go | 20 ++ agent/api/container/container.go | 7 + agent/statemanager/state_manager.go | 3 +- agent/statemanager/state_manager_test.go | 36 +++ .../v20/containerOrdering/ecs_agent_data.json | 259 ++++++++++++++++++ 6 files changed, 346 insertions(+), 2 deletions(-) create mode 100644 agent/statemanager/testdata/v20/containerOrdering/ecs_agent_data.json diff --git a/agent/acs/model/api/api-2.json b/agent/acs/model/api/api-2.json index a5004f3da42..4e99a8fdddc 100644 --- a/agent/acs/model/api/api-2.json +++ b/agent/acs/model/api/api-2.json @@ -203,13 +203,34 @@ "healthCheckType":{"shape":"HealthCheckType"}, "registryAuthentication":{"shape":"RegistryAuthenticationData"}, "logsAuthStrategy":{"shape":"AuthStrategy"}, - "secrets":{"shape":"SecretList"} + "secrets":{"shape":"SecretList"}, + "dependsOn":{"shape": "DependsOnList"} } }, "ContainerList":{ "type":"list", "member":{"shape":"Container"} }, + "DependsOn":{ + "type":"structure", + "members":{ + "container":{"shape":"String"}, + "condition":{"shape":"ConditionType"} + } + }, + "ConditionType":{ + "type":"string", + "enum":[ + "START", + "COMPLETE", + "SUCCESS", + "HEALTHY" + ] + }, + "DependsOnList":{ + "type":"list", + "member":{"shape":"DependsOn"} + }, "DockerConfig":{ "type":"structure", "members":{ diff --git a/agent/acs/model/ecsacs/api.go b/agent/acs/model/ecsacs/api.go index 5d1b3cfcdee..2c5f3f98693 100644 --- a/agent/acs/model/ecsacs/api.go +++ b/agent/acs/model/ecsacs/api.go @@ -206,6 +206,8 @@ type Container struct { Cpu *int64 `locationName:"cpu" type:"integer"` + DependsOn []*DependsOn `locationName:"dependsOn" type:"list"` + DockerConfig *DockerConfig `locationName:"dockerConfig" type:"structure"` EntryPoint []*string `locationName:"entryPoint" type:"list"` @@ -249,6 +251,24 @@ func (s Container) GoString() string { return s.String() } +type DependsOn struct { + _ struct{} `type:"structure"` + + Condition *string `locationName:"condition" type:"string" enum:"ConditionType"` + + Container *string `locationName:"container" type:"string"` +} + +// String returns the string representation +func (s DependsOn) String() string { + return awsutil.Prettify(s) +} + +// GoString returns the string representation +func (s DependsOn) GoString() string { + return s.String() +} + type DockerConfig struct { _ struct{} `type:"structure"` diff --git a/agent/api/container/container.go b/agent/api/container/container.go index 21f2bf52fb3..d37e3a1c3c4 100644 --- a/agent/api/container/container.go +++ b/agent/api/container/container.go @@ -93,6 +93,8 @@ type HealthStatus struct { type Container struct { // Name is the name of the container specified in the task definition Name string + // DependsOn is the field which specifies the ordering for container startup and shutdown. + DependsOn []DependsOn `json:"dependsOn,omitempty"` // V3EndpointID is a container identifier used to construct v3 metadata endpoint; it's unique among // all the containers managed by the agent V3EndpointID string @@ -230,6 +232,11 @@ type Container struct { labels map[string]string } +type DependsOn struct { + Container string `json:"container"` + Condition string `json:"condition"` +} + // DockerContainer is a mapping between containers-as-docker-knows-them and // containers-as-we-know-them. // This is primarily used in DockerState, but lives here such that tasks and diff --git a/agent/statemanager/state_manager.go b/agent/statemanager/state_manager.go index 999bbb28875..e9e2b44a0a8 100644 --- a/agent/statemanager/state_manager.go +++ b/agent/statemanager/state_manager.go @@ -78,7 +78,8 @@ const ( // a) Add 'Associations' field to 'api.task.task' // b) Add 'GPUIDs' field to 'apicontainer.Container' // c) Add 'NvidiaRuntime' field to 'api.task.task' - ECSDataVersion = 19 + // 20) Add 'DependsOn' field to 'apicontainer.Container' + ECSDataVersion = 20 // ecsDataFile specifies the filename in the ECS_DATADIR ecsDataFile = "ecs_agent_data.json" diff --git a/agent/statemanager/state_manager_test.go b/agent/statemanager/state_manager_test.go index 7291487b9ff..e81d8cd69ec 100644 --- a/agent/statemanager/state_manager_test.go +++ b/agent/statemanager/state_manager_test.go @@ -326,3 +326,39 @@ func TestLoadsDataForASMSecretsTask(t *testing.T) { assert.Equal(t, "secret-value-from", secret.ValueFrom) assert.Equal(t, "asm", secret.Provider) } + +// verify that the state manager correctly loads container ordering related fields in state file +func TestLoadsDataForContainerOrdering(t *testing.T) { + cleanup, err := setupWindowsTest(filepath.Join(".", "testdata", "v20", "containerOrdering", "ecs_agent_data.json")) + require.Nil(t, err, "Failed to set up test") + defer cleanup() + cfg := &config.Config{DataDir: filepath.Join(".", "testdata", "v20", "containerOrdering")} + taskEngine := engine.NewTaskEngine(&config.Config{}, nil, nil, nil, nil, dockerstate.NewTaskEngineState(), nil, nil) + var containerInstanceArn, cluster, savedInstanceID string + var sequenceNumber int64 + stateManager, err := statemanager.NewStateManager(cfg, + statemanager.AddSaveable("TaskEngine", taskEngine), + statemanager.AddSaveable("ContainerInstanceArn", &containerInstanceArn), + statemanager.AddSaveable("Cluster", &cluster), + statemanager.AddSaveable("EC2InstanceID", &savedInstanceID), + statemanager.AddSaveable("SeqNum", &sequenceNumber), + ) + assert.NoError(t, err) + err = stateManager.Load() + assert.NoError(t, err) + assert.Equal(t, "state-file", cluster) + assert.EqualValues(t, 0, sequenceNumber) + + tasks, err := taskEngine.ListTasks() + assert.NoError(t, err) + assert.Equal(t, 1, len(tasks)) + + task := tasks[0] + assert.Equal(t, "arn:aws:ecs:us-west-2:1234567890:task/33425c99-5db7-45fb-8244-bc94d00661e4", task.Arn) + assert.Equal(t, "container-ordering-state", task.Family) + assert.Equal(t, 2, len(task.Containers)) + + dependsOn := task.Containers[1].DependsOn + assert.Equal(t, "container_1", dependsOn[0].Container) + assert.Equal(t, "START", dependsOn[0].Condition) +} diff --git a/agent/statemanager/testdata/v20/containerOrdering/ecs_agent_data.json b/agent/statemanager/testdata/v20/containerOrdering/ecs_agent_data.json new file mode 100644 index 00000000000..1df5b52de0f --- /dev/null +++ b/agent/statemanager/testdata/v20/containerOrdering/ecs_agent_data.json @@ -0,0 +1,259 @@ +{ + "Data": { + "Cluster": "state-file", + "ContainerInstanceArn": "arn:aws:ecs:us-west-2:1234567890:container-instance/46efd519-df3f-4096-8f34-faebb1747752", + "EC2InstanceID": "i-0da29eb1a8a98768b", + "TaskEngine": { + "Tasks": [ + { + "Arn": "arn:aws:ecs:us-west-2:1234567890:task/33425c99-5db7-45fb-8244-bc94d00661e4", + "Family": "container-ordering-state", + "Version": "1", + "Containers": [ + { + "Name": "container_1", + "Image": "amazonlinux:1", + "ImageID": "sha256:7f929d2604c7e504a568eac9a2523c1b9e9b15e1fcee4076e1411a552913d08e", + "Command": [ + "sleep", + "3600" + ], + "Cpu": 0, + "GPUIDs": ["0", "1"], + "Memory": 512, + "Links": null, + "volumesFrom": [], + "mountPoints": [], + "portMappings": [], + "Essential": true, + "EntryPoint": null, + "environment": { + "NVIDIA_VISIBLE_DEVICES": "0,1" + }, + "overrides": { + "command": null + }, + "dockerConfig": { + "config": "{}", + "hostConfig": "{\"CapAdd\":[],\"CapDrop\":[]}", + "version": "1.17" + }, + "registryAuthentication": null, + "secrets": [], + "LogsAuthStrategy": "", + "desiredStatus": "RUNNING", + "KnownStatus": "RUNNING", + "TransitionDependencySet": { + "1": { + "ContainerDependencies": null, + "ResourceDependencies": [ + { + "Name": "cgroup", + "RequiredStatus": 1 + } + ] + } + }, + "RunDependencies": null, + "IsInternal": "NORMAL", + "ApplyingError": { + "error": "API error (500): Get https://registry-1.docker.io/v2/library/amazonlinux/manifests/1: toomanyrequests: too many failed login attempts for username or IP address\n", + "name": "CannotPullContainerError" + }, + "SentStatus": "RUNNING", + "metadataFileUpdated": false, + "KnownExitCode": null, + "KnownPortBindings": null + }, + { + "Name": "container_2", + "DependsOn": [ + { + "Container":"container_1", + "Condition":"START" + } + ], + "Image": "amazonlinux:1", + "ImageID": "sha256:7f929d2604c7e504a568eac9a2523c1b9e9b15e1fcee4076e1411a552913d08e", + "Command": [ + "sleep", + "3600" + ], + "Cpu": 0, + "GPUIDs": [], + "Memory": 512, + "Links": null, + "volumesFrom": [], + "mountPoints": [], + "portMappings": [], + "Essential": true, + "EntryPoint": null, + "environment": {}, + "overrides": { + "command": null + }, + "dockerConfig": { + "config": "{}", + "hostConfig": "{\"CapAdd\":[],\"CapDrop\":[]}", + "version": "1.17" + }, + "registryAuthentication": null, + "secrets": [], + "LogsAuthStrategy": "", + "desiredStatus": "RUNNING", + "KnownStatus": "RUNNING", + "TransitionDependencySet": { + "1": { + "ContainerDependencies": null, + "ResourceDependencies": [ + { + "Name": "cgroup", + "RequiredStatus": 1 + } + ] + } + }, + "RunDependencies": null, + "IsInternal": "NORMAL", + "ApplyingError": { + "error": "API error (500): Get https://registry-1.docker.io/v2/library/amazonlinux/manifests/1: toomanyrequests: too many failed login attempts for username or IP address\n", + "name": "CannotPullContainerError" + }, + "SentStatus": "RUNNING", + "metadataFileUpdated": false, + "KnownExitCode": null, + "KnownPortBindings": null + } + ], + "Associations": [ + { + "Containers": ["container_1"], + "Content": { + "Encoding": "base64", + "Value": "val" + }, + "Name": "0", + "Type": "gpu" + }, + { + "Containers": ["container_1"], + "Content": { + "Encoding": "base64", + "Value": "val" + }, + "Name": "1", + "Type": "gpu" + } + ], + "resources": { + "cgroup": [ + { + "cgroupRoot": "/ecs/33425c99-5db7-45fb-8244-bc94d00661e4", + "cgroupMountPath": "/sys/fs/cgroup", + "createdAt": "0001-01-01T00:00:00Z", + "desiredStatus": "CREATED", + "knownStatus": "CREATED", + "resourceSpec": { + "cpu": { + "shares": 2 + } + } + } + ] + }, + "volumes": [], + "DesiredStatus": "RUNNING", + "KnownStatus": "RUNNING", + "KnownTime": "2018-10-04T18:05:49.121835686Z", + "PullStartedAt": "2018-10-04T18:05:34.359798761Z", + "PullStoppedAt": "2018-10-04T18:05:48.445985904Z", + "ExecutionStoppedAt": "0001-01-01T00:00:00Z", + "SentStatus": "RUNNING", + "StartSequenceNumber": 2, + "StopSequenceNumber": 0, + "executionCredentialsID": "b1a6ede6-1a9f-4ab3-a02e-bd3e51b11244", + "ENI": null, + "MemoryCPULimitsEnabled": true, + "PlatformFields": {} + } + ], + "IdToContainer": { + "8f5e6e3091f221c876103289ddabcbcdeb64acd7ac7e2d0cf4da2be2be9d8956": { + "DockerId": "8f5e6e3091f221c876103289ddabcbcdeb64acd7ac7e2d0cf4da2be2be9d8956", + "DockerName": "ecs-private-registry-state-1-container1-a68ef4b6e0fba38d3500", + "Container": { + "Name": "container_1", + "Image": "amazonlinux:1", + "ImageID": "sha256:7f929d2604c7e504a568eac9a2523c1b9e9b15e1fcee4076e1411a552913d08e", + "Command": [ + "sleep", + "3600" + ], + "Cpu": 0, + "Memory": 512, + "Links": null, + "volumesFrom": [], + "mountPoints": [], + "portMappings": [], + "Essential": true, + "EntryPoint": null, + "environment": {}, + "overrides": { + "command": null + }, + "dockerConfig": { + "config": "{}", + "hostConfig": "{\"CapAdd\":[],\"CapDrop\":[]}", + "version": "1.17" + }, + "registryAuthentication": null, + "LogsAuthStrategy": "", + "desiredStatus": "RUNNING", + "KnownStatus": "RUNNING", + "TransitionDependencySet": { + "1": { + "ContainerDependencies": null, + "ResourceDependencies": [ + { + "Name": "cgroup", + "RequiredStatus": 1 + } + ] + } + }, + "RunDependencies": null, + "IsInternal": "NORMAL", + "ApplyingError": { + "error": "API error (500): Get https://registry-1.docker.io/v2/library/amazonlinux/manifests/1: toomanyrequests: too many failed login attempts for username or IP address\n", + "name": "CannotPullContainerError" + }, + "SentStatus": "RUNNING", + "metadataFileUpdated": false, + "KnownExitCode": null, + "KnownPortBindings": null + } + } + }, + "IdToTask": { + "8f5e6e3091f221c876103289ddabcbcdeb64acd7ac7e2d0cf4da2be2be9d8956": "arn:aws:ecs:us-west-2:1234567890:task/33425c99-5db7-45fb-8244-bc94d00661e4" + }, + "ImageStates": [ + { + "Image": { + "ImageID": "sha256:7f929d2604c7e504a568eac9a2523c1b9e9b15e1fcee4076e1411a552913d08e", + "Names": [ + "amazonlinux:1" + ], + "Size": 165452304 + }, + "PulledAt": "2018-10-04T18:05:48.445644088Z", + "LastUsedAt": "2018-10-04T18:05:48.445645342Z", + "PullSucceeded": false + } + ], + "ENIAttachments": null, + "IPToTask": {} + } + }, + "Version": 20 +} From 5fbfab0a5f31092d13d222fbbb0623d545814739 Mon Sep 17 00:00:00 2001 From: Utsa Bhattacharjya Date: Wed, 30 Jan 2019 15:37:17 -0800 Subject: [PATCH 02/24] Adding adapter to have volumes/links use DependsOn field --- agent/api/task/task.go | 51 +++++++++++++++- agent/api/task/task_test.go | 97 ++++++++++++++++++++++++++++++ agent/app/agent_capability.go | 4 ++ agent/app/agent_capability_test.go | 3 + 4 files changed, 154 insertions(+), 1 deletion(-) diff --git a/agent/api/task/task.go b/agent/api/task/task.go index 5072ea8d795..ba3e4b2507f 100644 --- a/agent/api/task/task.go +++ b/agent/api/task/task.go @@ -72,6 +72,8 @@ const ( NvidiaVisibleDevicesEnvVar = "NVIDIA_VISIBLE_DEVICES" GPUAssociationType = "gpu" + ContainerOrderingStartCondition = "START" + arnResourceSections = 2 arnResourceDelimiter = "/" // networkModeNone specifies the string used to define the `none` docker networking mode @@ -250,6 +252,18 @@ func (task *Task) PostUnmarshalTask(cfg *config.Config, } } + err := task.initializeContainerOrderingForVolumes() + if err != nil { + seelog.Errorf("Task [%s]: could not initialize volumes dependency for container: %v", task.Arn, err) + return apierrors.NewResourceInitError(task.Arn, err) + } + + err = task.initializeContainerOrderingForLinks() + if err != nil { + seelog.Errorf("Task [%s]: could not initialize links dependency for container: %v", task.Arn, err) + return apierrors.NewResourceInitError(task.Arn, err) + } + if task.requiresASMDockerAuthData() { task.initializeASMAuthResource(credentialsManager, resourceFields) } @@ -262,7 +276,7 @@ func (task *Task) PostUnmarshalTask(cfg *config.Config, task.initializeASMSecretResource(credentialsManager, resourceFields) } - err := task.initializeDockerLocalVolumes(dockerClient, ctx) + err = task.initializeDockerLocalVolumes(dockerClient, ctx) if err != nil { return apierrors.NewResourceInitError(task.Arn, err) } @@ -1241,6 +1255,41 @@ func (task *Task) shouldOverrideIPCMode(container *apicontainer.Container, docke } } +func (task *Task) initializeContainerOrderingForVolumes() error { + for _, container := range task.Containers { + if len(container.VolumesFrom) > 0 { + for _, volume := range container.VolumesFrom { + if _, ok := task.ContainerByName(volume.SourceContainer); !ok { + return fmt.Errorf("could not find container with name %s", volume.SourceContainer) + } + dependOn := apicontainer.DependsOn{Container: volume.SourceContainer, Condition: ContainerOrderingStartCondition} + container.DependsOn = append(container.DependsOn, dependOn) + } + } + } + return nil +} + +func (task *Task) initializeContainerOrderingForLinks() error { + for _, container := range task.Containers { + if len(container.Links) > 0 { + for _, link := range container.Links { + linkParts := strings.Split(link, ":") + if len(linkParts) > 2 { + return fmt.Errorf("Invalid link format") + } + linkName := linkParts[0] + if _, ok := task.ContainerByName(linkName); !ok { + return fmt.Errorf("could not find container with name %s", linkName) + } + dependOn := apicontainer.DependsOn{Container: linkName, Condition: ContainerOrderingStartCondition} + container.DependsOn = append(container.DependsOn, dependOn) + } + } + } + return nil +} + func (task *Task) dockerLinks(container *apicontainer.Container, dockerContainerMap map[string]*apicontainer.DockerContainer) ([]string, error) { dockerLinkArr := make([]string, len(container.Links)) for i, link := range container.Links { diff --git a/agent/api/task/task_test.go b/agent/api/task/task_test.go index 2d613622d4f..8e9efefe303 100644 --- a/agent/api/task/task_test.go +++ b/agent/api/task/task_test.go @@ -2816,3 +2816,100 @@ func TestAssociationByTypeAndName(t *testing.T) { association, ok = task.AssociationByTypeAndName("other-type", "dev1") assert.False(t, ok) } + +func TestInitializeContainerOrderingWithLinksAndVolumesFrom(t *testing.T) { + containerWithOnlyVolume := &apicontainer.Container{ + Name: "myName", + Image: "image:tag", + VolumesFrom: []apicontainer.VolumeFrom{{SourceContainer: "myName1"}}, + } + + containerWithOnlyLink := &apicontainer.Container{ + Name: "myName1", + Image: "image:tag", + Links: []string{"myName"}, + } + + containerWithBothVolumeAndLink := &apicontainer.Container{ + Name: "myName2", + Image: "image:tag", + VolumesFrom: []apicontainer.VolumeFrom{{SourceContainer: "myName"}}, + Links: []string{"myName1"}, + } + + containerWithNoVolumeOrLink := &apicontainer.Container{ + Name: "myName3", + Image: "image:tag", + } + + task := &Task{ + Arn: "test", + ResourcesMapUnsafe: make(map[string][]taskresource.TaskResource), + Containers: []*apicontainer.Container{containerWithOnlyVolume, containerWithOnlyLink, + containerWithBothVolumeAndLink, containerWithNoVolumeOrLink}, + } + + err := task.initializeContainerOrderingForVolumes() + assert.NoError(t, err) + err = task.initializeContainerOrderingForLinks() + assert.NoError(t, err) + + containerResultWithVolume := task.Containers[0] + assert.Equal(t, "myName1", containerResultWithVolume.DependsOn[0].Container) + assert.Equal(t, ContainerOrderingStartCondition, containerResultWithVolume.DependsOn[0].Condition) + + containerResultWithLink := task.Containers[1] + assert.Equal(t, "myName", containerResultWithLink.DependsOn[0].Container) + assert.Equal(t, ContainerOrderingStartCondition, containerResultWithLink.DependsOn[0].Condition) + + containerResultWithBothVolumeAndLink := task.Containers[2] + assert.Equal(t, "myName", containerResultWithBothVolumeAndLink.DependsOn[0].Container) + assert.Equal(t, ContainerOrderingStartCondition, containerResultWithBothVolumeAndLink.DependsOn[0].Condition) + assert.Equal(t, "myName1", containerResultWithBothVolumeAndLink.DependsOn[1].Container) + assert.Equal(t, ContainerOrderingStartCondition, containerResultWithBothVolumeAndLink.DependsOn[1].Condition) + + containerResultWithNoVolumeOrLink := task.Containers[3] + assert.Equal(t, 0, len(containerResultWithNoVolumeOrLink.DependsOn)) +} + +func TestInitializeContainerOrderingWithError(t *testing.T) { + containerWithVolumeError := &apicontainer.Container{ + Name: "myName", + Image: "image:tag", + VolumesFrom: []apicontainer.VolumeFrom{{SourceContainer: "dummyContainer"}}, + } + + containerWithLinkError1 := &apicontainer.Container{ + Name: "myName1", + Image: "image:tag", + Links: []string{"dummyContainer"}, + } + + containerWithLinkError2 := &apicontainer.Container{ + Name: "myName2", + Image: "image:tag", + Links: []string{"myName:link1:link2"}, + } + + task1 := &Task{ + Arn: "test", + ResourcesMapUnsafe: make(map[string][]taskresource.TaskResource), + Containers: []*apicontainer.Container{containerWithVolumeError, containerWithLinkError1}, + } + + task2 := &Task{ + Arn: "test", + ResourcesMapUnsafe: make(map[string][]taskresource.TaskResource), + Containers: []*apicontainer.Container{containerWithVolumeError, containerWithLinkError2}, + } + + errVolume1 := task1.initializeContainerOrderingForVolumes() + assert.Error(t, errVolume1) + errLink1 := task1.initializeContainerOrderingForLinks() + assert.Error(t, errLink1) + + errVolume2 := task2.initializeContainerOrderingForVolumes() + assert.Error(t, errVolume2) + errLink2 := task2.initializeContainerOrderingForLinks() + assert.Error(t, errLink2) +} diff --git a/agent/app/agent_capability.go b/agent/app/agent_capability.go index 9fcd3324d0b..da3ca78347c 100644 --- a/agent/app/agent_capability.go +++ b/agent/app/agent_capability.go @@ -42,6 +42,7 @@ const ( capabiltyPIDAndIPCNamespaceSharing = "pid-ipc-namespace-sharing" capabilityNvidiaDriverVersionInfix = "nvidia-driver-version." capabilityECREndpoint = "ecr-endpoint" + capabilityContainerOrdering = "container-ordering" taskEIAAttributeSuffix = "task-eia" ) @@ -141,6 +142,9 @@ func (agent *ecsAgent) capabilities() ([]*ecs.Attribute, error) { // support elastic inference in agent capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+taskEIAAttributeSuffix) + // support container ordering in agent + capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+capabilityContainerOrdering) + return capabilities, nil } diff --git a/agent/app/agent_capability_test.go b/agent/app/agent_capability_test.go index dc5ce1d1b8e..b4baee1ab18 100644 --- a/agent/app/agent_capability_test.go +++ b/agent/app/agent_capability_test.go @@ -126,6 +126,9 @@ func TestCapabilities(t *testing.T) { { Name: aws.String(attributePrefix + taskEIAAttributeSuffix), }, + { + Name: aws.String(attributePrefix + capabilityContainerOrdering), + }, }...) ctx, cancel := context.WithCancel(context.TODO()) From 5c3fa512e21d5e6bf94cff9c90e3b9d4bfc83dbf Mon Sep 17 00:00:00 2001 From: Utsa Bhattacharjya Date: Thu, 7 Feb 2019 14:43:09 -0800 Subject: [PATCH 03/24] Refactor dependency graph code for volume/links dependency resolution --- agent/api/task/task.go | 5 +- agent/api/task/task_test.go | 4 +- agent/engine/dependencygraph/graph.go | 177 +++++---- agent/engine/dependencygraph/graph_test.go | 421 +++++++++++---------- 4 files changed, 336 insertions(+), 271 deletions(-) diff --git a/agent/api/task/task.go b/agent/api/task/task.go index ba3e4b2507f..0f3b0e21ed6 100644 --- a/agent/api/task/task.go +++ b/agent/api/task/task.go @@ -72,7 +72,8 @@ const ( NvidiaVisibleDevicesEnvVar = "NVIDIA_VISIBLE_DEVICES" GPUAssociationType = "gpu" - ContainerOrderingStartCondition = "START" + ContainerOrderingStartCondition = "START" + ContainerOrderingRunningCondition = "RUNNING" arnResourceSections = 2 arnResourceDelimiter = "/" @@ -1282,7 +1283,7 @@ func (task *Task) initializeContainerOrderingForLinks() error { if _, ok := task.ContainerByName(linkName); !ok { return fmt.Errorf("could not find container with name %s", linkName) } - dependOn := apicontainer.DependsOn{Container: linkName, Condition: ContainerOrderingStartCondition} + dependOn := apicontainer.DependsOn{Container: linkName, Condition: ContainerOrderingRunningCondition} container.DependsOn = append(container.DependsOn, dependOn) } } diff --git a/agent/api/task/task_test.go b/agent/api/task/task_test.go index 8e9efefe303..4a02e379180 100644 --- a/agent/api/task/task_test.go +++ b/agent/api/task/task_test.go @@ -2860,13 +2860,13 @@ func TestInitializeContainerOrderingWithLinksAndVolumesFrom(t *testing.T) { containerResultWithLink := task.Containers[1] assert.Equal(t, "myName", containerResultWithLink.DependsOn[0].Container) - assert.Equal(t, ContainerOrderingStartCondition, containerResultWithLink.DependsOn[0].Condition) + assert.Equal(t, ContainerOrderingRunningCondition, containerResultWithLink.DependsOn[0].Condition) containerResultWithBothVolumeAndLink := task.Containers[2] assert.Equal(t, "myName", containerResultWithBothVolumeAndLink.DependsOn[0].Container) assert.Equal(t, ContainerOrderingStartCondition, containerResultWithBothVolumeAndLink.DependsOn[0].Condition) assert.Equal(t, "myName1", containerResultWithBothVolumeAndLink.DependsOn[1].Container) - assert.Equal(t, ContainerOrderingStartCondition, containerResultWithBothVolumeAndLink.DependsOn[1].Condition) + assert.Equal(t, ContainerOrderingRunningCondition, containerResultWithBothVolumeAndLink.DependsOn[1].Condition) containerResultWithNoVolumeOrLink := task.Containers[3] assert.Equal(t, 0, len(containerResultWithNoVolumeOrLink.DependsOn)) diff --git a/agent/engine/dependencygraph/graph.go b/agent/engine/dependencygraph/graph.go index 05bb1c20b71..0a728a329a1 100644 --- a/agent/engine/dependencygraph/graph.go +++ b/agent/engine/dependencygraph/graph.go @@ -14,8 +14,7 @@ package dependencygraph import ( - "strings" - + "fmt" apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" apicontainerstatus "github.com/aws/amazon-ecs-agent/agent/api/container/status" apitask "github.com/aws/amazon-ecs-agent/agent/api/task" @@ -23,6 +22,7 @@ import ( "github.com/aws/amazon-ecs-agent/agent/taskresource" log "github.com/cihub/seelog" "github.com/pkg/errors" + "strings" ) var ( @@ -33,8 +33,6 @@ var ( DependentContainerNotResolvedErr = errors.New("dependency graph: dependent container not in expected state") // ContainerPastDesiredStatusErr is the error where the container status is bigger than desired status ContainerPastDesiredStatusErr = errors.New("container transition: container status is equal or greater than desired status") - volumeUnresolvedErr = errors.New("dependency graph: container volume dependency not resolved") - linkUnresolvedErr = errors.New("dependency graph: container links dependency not resolved") // ErrContainerDependencyNotResolved is when the container's dependencies // on other containers are not resolved ErrContainerDependencyNotResolved = errors.New("dependency graph: dependency on containers not resolved") @@ -90,14 +88,11 @@ func dependenciesCanBeResolved(target *apicontainer.Container, by []*apicontaine for _, cont := range by { nameMap[cont.Name] = cont } - neededVolumeContainers := make([]string, len(target.VolumesFrom)) - for i, volume := range target.VolumesFrom { - neededVolumeContainers[i] = volume.SourceContainer - } - return verifyStatusResolvable(target, nameMap, neededVolumeContainers, volumeCanResolve) && - verifyStatusResolvable(target, nameMap, linksToContainerNames(target.Links), linkCanResolve) && - verifyStatusResolvable(target, nameMap, target.SteadyStateDependencies, onSteadyStateCanResolve) + if err := verifyContainerOrderingStatusResolvable(target, nameMap, containerOrderingDependenciesCanResolve); err != nil { + return false + } + return verifyStatusResolvable(target, nameMap, target.SteadyStateDependencies, onSteadyStateCanResolve) } // DependenciesAreResolved validates that the `target` container can be @@ -130,12 +125,8 @@ func DependenciesAreResolved(target *apicontainer.Container, resourcesMap[resource.GetName()] = resource } - if !verifyStatusResolvable(target, nameMap, neededVolumeContainers, volumeIsResolved) { - return volumeUnresolvedErr - } - - if !verifyStatusResolvable(target, nameMap, linksToContainerNames(target.Links), linkIsResolved) { - return linkUnresolvedErr + if err := verifyContainerOrderingStatusResolvable(target, nameMap, containerOrderingDependenciesIsResolved); err != nil { + return err } if !verifyStatusResolvable(target, nameMap, target.SteadyStateDependencies, onSteadyStateIsResolved) { @@ -193,6 +184,32 @@ func verifyStatusResolvable(target *apicontainer.Container, existingContainers m return true } +// verifyContainerOrderingStatusResolvable validates that `target` can be resolved given that +// the dependsOn containers are resolved and there are `existingContainers` +// (map from name to container). The `resolves` function passed should return true if the named container is resolved. + +func verifyContainerOrderingStatusResolvable(target *apicontainer.Container, existingContainers map[string]*apicontainer.Container, + resolves func(*apicontainer.Container, *apicontainer.Container, string) bool) error { + + targetGoal := target.GetDesiredStatus() + if targetGoal != target.GetSteadyStateStatus() && targetGoal != apicontainerstatus.ContainerCreated { + // A container can always stop, die, or reach whatever other state it + // wants regardless of what dependencies it has + return nil + } + + for _, dependency := range target.DependsOn { + dependencyContainer, ok := existingContainers[dependency.Container] + if !ok { + return fmt.Errorf("dependency graph: container ordering dependency [%v] for target [%v] does not exist.", dependencyContainer, target) + } + if !resolves(target, dependencyContainer, dependency.Condition) { + return fmt.Errorf("dependency graph: failed to resolve the container ordering dependency [%v] for target [%v]", dependencyContainer, target) + } + } + return nil +} + func verifyTransitionDependenciesResolved(target *apicontainer.Container, existingContainers map[string]*apicontainer.Container, existingResources map[string]taskresource.TaskResource) error { @@ -236,74 +253,90 @@ func verifyResourceDependenciesResolved(target *apicontainer.Container, existing return true } -func linkCanResolve(target *apicontainer.Container, link *apicontainer.Container) bool { +func containerOrderingDependenciesCanResolve(target *apicontainer.Container, + dependsOnContainer *apicontainer.Container, + dependsOnStatus string) bool { targetDesiredStatus := target.GetDesiredStatus() - linkDesiredStatus := link.GetDesiredStatus() - if targetDesiredStatus == apicontainerstatus.ContainerCreated { - // The 'target' container desires to be moved to 'Created' state. - // Allow this only if the desired status of the linked container is - // 'Created' or if the linked container is in 'steady state' - return linkDesiredStatus == apicontainerstatus.ContainerCreated || linkDesiredStatus == link.GetSteadyStateStatus() - } else if targetDesiredStatus == target.GetSteadyStateStatus() { - // The 'target' container desires to be moved to its 'steady' state. - // Allow this only if the linked container is in 'steady state' as well - return linkDesiredStatus == link.GetSteadyStateStatus() - } - log.Errorf("Failed to resolve the desired status of the link [%v] for the target [%v]", link, target) - return false -} + dependsOnContainerDesiredStatus := dependsOnContainer.GetDesiredStatus() -func linkIsResolved(target *apicontainer.Container, link *apicontainer.Container) bool { - targetDesiredStatus := target.GetDesiredStatus() - if targetDesiredStatus == apicontainerstatus.ContainerCreated { - // The 'target' container desires to be moved to 'Created' state. - // Allow this only if the known status of the linked container is - // 'Created' or if the linked container is in 'steady state' - linkKnownStatus := link.GetKnownStatus() - return linkKnownStatus == apicontainerstatus.ContainerCreated || link.IsKnownSteadyState() - } else if targetDesiredStatus == target.GetSteadyStateStatus() { - // The 'target' container desires to be moved to its 'steady' state. - // Allow this only if the linked container is in 'steady state' as well - return link.IsKnownSteadyState() - } - log.Errorf("Failed to resolve if the link [%v] has been resolved for the target [%v]", link, target) - return false -} + switch dependsOnStatus { -func volumeCanResolve(target *apicontainer.Container, volume *apicontainer.Container) bool { - targetDesiredStatus := target.GetDesiredStatus() - if targetDesiredStatus != apicontainerstatus.ContainerCreated && targetDesiredStatus != target.GetSteadyStateStatus() { + case "START": // The 'target' container doesn't desire to move to either 'Created' or the 'steady' state, // which is not allowed - log.Errorf("Failed to resolve the desired status of the volume [%v] for the target [%v]", volume, target) + if targetDesiredStatus != apicontainerstatus.ContainerCreated && targetDesiredStatus != target.GetSteadyStateStatus() { + return false + } + // The 'target' container desires to be moved to 'Created' or the 'steady' state. + // Allow this only if the dependency container also desires to be started + // i.e it's status is any of 'Created', 'steady state' or 'Stopped' + return dependsOnContainerDesiredStatus == apicontainerstatus.ContainerCreated || + dependsOnContainerDesiredStatus == apicontainerstatus.ContainerStopped || + dependsOnContainerDesiredStatus == dependsOnContainer.GetSteadyStateStatus() + + case "RUNNING": + + if targetDesiredStatus == apicontainerstatus.ContainerCreated { + // The 'target' container desires to be moved to 'Created' state. + // Allow this only if the desired status of the dependency container is + // 'Created' or if the linked container is in 'steady state' + return dependsOnContainerDesiredStatus == apicontainerstatus.ContainerCreated || + dependsOnContainerDesiredStatus == dependsOnContainer.GetSteadyStateStatus() + + } else if targetDesiredStatus == target.GetSteadyStateStatus() { + // The 'target' container desires to be moved to its 'steady' state. + // Allow this only if the dependency container also desires to be in 'steady state' + return dependsOnContainerDesiredStatus == dependsOnContainer.GetSteadyStateStatus() + + } return false - } - // The 'target' container desires to be moved to 'Created' or the 'steady' state. - // Allow this only if the known status of the source volume container is - // any of 'Created', 'steady state' or 'Stopped' - volumeDesiredStatus := volume.GetDesiredStatus() - return volumeDesiredStatus == apicontainerstatus.ContainerCreated || - volumeDesiredStatus == volume.GetSteadyStateStatus() || - volumeDesiredStatus == apicontainerstatus.ContainerStopped + default: + return false + } } -func volumeIsResolved(target *apicontainer.Container, volume *apicontainer.Container) bool { +func containerOrderingDependenciesIsResolved(target *apicontainer.Container, + dependsOnContainer *apicontainer.Container, + dependsOnStatus string) bool { + targetDesiredStatus := target.GetDesiredStatus() - if targetDesiredStatus != apicontainerstatus.ContainerCreated && targetDesiredStatus != apicontainerstatus.ContainerRunning { + dependsOnContainerKnownStatus := dependsOnContainer.GetKnownStatus() + + switch dependsOnStatus { + + case "START": // The 'target' container doesn't desire to be moved to 'Created' or the 'steady' state. - // Do not allow it. - log.Errorf("Failed to resolve if the volume [%v] has been resolved for the target [%v]", volume, target) + // which is not allowed. + if targetDesiredStatus != apicontainerstatus.ContainerCreated && targetDesiredStatus != apicontainerstatus.ContainerRunning { + return false + } + // The 'target' container desires to be moved to 'Created' or the 'steady' state. + // Allow this only if the known status of the dependency container state is already started + // i.e it's state is any of 'Created', 'steady state' or 'Stopped' + return dependsOnContainerKnownStatus == apicontainerstatus.ContainerCreated || + dependsOnContainerKnownStatus == apicontainerstatus.ContainerStopped || + dependsOnContainerKnownStatus == dependsOnContainer.GetSteadyStateStatus() + + case "RUNNING": + + if targetDesiredStatus == apicontainerstatus.ContainerCreated { + // The 'target' container desires to be moved to 'Created' state. + // Allow this only if the known status of the linked container is + // 'Created' or if the dependency container is in 'steady state' + return dependsOnContainerKnownStatus == apicontainerstatus.ContainerCreated || dependsOnContainer.IsKnownSteadyState() + + } else if targetDesiredStatus == target.GetSteadyStateStatus() { + // The 'target' container desires to be moved to its 'steady' state. + // Allow this only if the dependency container is in 'steady state' as well + return dependsOnContainer.IsKnownSteadyState() + + } return false - } - // The 'target' container desires to be moved to 'Created' or the 'steady' state. - // Allow this only if the known status of the source volume container is - // any of 'Created', 'steady state' or 'Stopped' - knownStatus := volume.GetKnownStatus() - return knownStatus == apicontainerstatus.ContainerCreated || - knownStatus == volume.GetSteadyStateStatus() || - knownStatus == apicontainerstatus.ContainerStopped + default: + return false + } } func onSteadyStateCanResolve(target *apicontainer.Container, run *apicontainer.Container) bool { diff --git a/agent/engine/dependencygraph/graph_test.go b/agent/engine/dependencygraph/graph_test.go index fcc2322d618..1281070f53c 100644 --- a/agent/engine/dependencygraph/graph_test.go +++ b/agent/engine/dependencygraph/graph_test.go @@ -38,20 +38,18 @@ func volumeStrToVol(vols []string) []apicontainer.VolumeFrom { return ret } -func steadyStateContainer(name string, links, volumes []string, desiredState apicontainerstatus.ContainerStatus, steadyState apicontainerstatus.ContainerStatus) *apicontainer.Container { +func steadyStateContainer(name string, dependsOn []apicontainer.DependsOn, desiredState apicontainerstatus.ContainerStatus, steadyState apicontainerstatus.ContainerStatus) *apicontainer.Container { container := apicontainer.NewContainerWithSteadyState(steadyState) container.Name = name - container.Links = links - container.VolumesFrom = volumeStrToVol(volumes) + container.DependsOn = dependsOn container.DesiredStatusUnsafe = desiredState return container } -func createdContainer(name string, links, volumes []string, steadyState apicontainerstatus.ContainerStatus) *apicontainer.Container { +func createdContainer(name string, dependsOn []apicontainer.DependsOn, steadyState apicontainerstatus.ContainerStatus) *apicontainer.Container { container := apicontainer.NewContainerWithSteadyState(steadyState) container.Name = name - container.Links = links - container.VolumesFrom = volumeStrToVol(volumes) + container.DependsOn = dependsOn container.DesiredStatusUnsafe = apicontainerstatus.ContainerCreated return container } @@ -74,12 +72,12 @@ func TestValidDependencies(t *testing.T) { assert.True(t, resolveable, "One container should resolve trivially") // Webserver stack - php := steadyStateContainer("php", []string{"db"}, []string{}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - db := steadyStateContainer("db", []string{}, []string{"dbdatavolume"}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - dbdata := createdContainer("dbdatavolume", []string{}, []string{}, apicontainerstatus.ContainerRunning) - webserver := steadyStateContainer("webserver", []string{"php"}, []string{"htmldata"}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - htmldata := steadyStateContainer("htmldata", []string{}, []string{"sharedcssfiles"}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - sharedcssfiles := createdContainer("sharedcssfiles", []string{}, []string{}, apicontainerstatus.ContainerRunning) + php := steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: "RUNNING"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + db := steadyStateContainer("db", []apicontainer.DependsOn{{Container: "dbdatavolume", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + dbdata := createdContainer("dbdatavolume", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) + webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{Container: "php", Condition: "RUNNING"}, {Container: "htmldata", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{Container: "sharedcssfiles", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + sharedcssfiles := createdContainer("sharedcssfiles", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) task = &apitask.Task{ Containers: []*apicontainer.Container{ @@ -95,8 +93,8 @@ func TestValidDependenciesWithCycles(t *testing.T) { // Unresolveable: cycle task := &apitask.Task{ Containers: []*apicontainer.Container{ - steadyStateContainer("a", []string{"b"}, []string{}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), - steadyStateContainer("b", []string{"a"}, []string{}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), + steadyStateContainer("a", []apicontainer.DependsOn{{Container: "b", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), + steadyStateContainer("b", []apicontainer.DependsOn{{Container: "a", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), }, } resolveable := ValidDependencies(task) @@ -107,13 +105,14 @@ func TestValidDependenciesWithUnresolvedReference(t *testing.T) { // Unresolveable, reference doesn't exist task := &apitask.Task{ Containers: []*apicontainer.Container{ - steadyStateContainer("php", []string{"db"}, []string{}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), + steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), }, } resolveable := ValidDependencies(task) assert.False(t, resolveable, "Nonexistent reference shouldn't resolve") } + func TestDependenciesAreResolvedWhenSteadyStateIsRunning(t *testing.T) { task := &apitask.Task{ Containers: []*apicontainer.Container{ @@ -127,12 +126,12 @@ func TestDependenciesAreResolvedWhenSteadyStateIsRunning(t *testing.T) { assert.NoError(t, err, "One container should resolve trivially") // Webserver stack - php := steadyStateContainer("php", []string{"db"}, []string{}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - db := steadyStateContainer("db", []string{}, []string{"dbdatavolume"}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - dbdata := createdContainer("dbdatavolume", []string{}, []string{}, apicontainerstatus.ContainerRunning) - webserver := steadyStateContainer("webserver", []string{"php"}, []string{"htmldata"}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - htmldata := steadyStateContainer("htmldata", []string{}, []string{"sharedcssfiles"}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - sharedcssfiles := createdContainer("sharedcssfiles", []string{}, []string{}, apicontainerstatus.ContainerRunning) + php := steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: "RUNNING"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + db := steadyStateContainer("db", []apicontainer.DependsOn{{Container: "dbdatavolume", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + dbdata := createdContainer("dbdatavolume", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) + webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{Container: "php", Condition: "RUNNING"}, {Container: "htmldata", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{Container: "sharedcssfiles", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + sharedcssfiles := createdContainer("sharedcssfiles", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) task = &apitask.Task{ Containers: []*apicontainer.Container{ @@ -149,17 +148,17 @@ func TestDependenciesAreResolvedWhenSteadyStateIsRunning(t *testing.T) { err = DependenciesAreResolved(dbdata, task.Containers, "", nil, nil) assert.NoError(t, err, "data volume with no deps should resolve") - dbdata.KnownStatusUnsafe = apicontainerstatus.ContainerCreated + dbdata.SetKnownStatus(apicontainerstatus.ContainerCreated) err = DependenciesAreResolved(php, task.Containers, "", nil, nil) assert.Error(t, err, "Php shouldn't run, db is not created") - db.KnownStatusUnsafe = apicontainerstatus.ContainerCreated + db.SetKnownStatus(apicontainerstatus.ContainerCreated) err = DependenciesAreResolved(php, task.Containers, "", nil, nil) assert.Error(t, err, "Php shouldn't run, db is not running") err = DependenciesAreResolved(db, task.Containers, "", nil, nil) assert.NoError(t, err, "db should be resolved, dbdata volume is Created") - db.KnownStatusUnsafe = apicontainerstatus.ContainerRunning + db.SetKnownStatus(apicontainerstatus.ContainerRunning) err = DependenciesAreResolved(php, task.Containers, "", nil, nil) assert.NoError(t, err, "Php should resolve") @@ -182,23 +181,24 @@ func TestRunDependencies(t *testing.T) { task.Containers[1].SetDesiredStatus(apicontainerstatus.ContainerRunning) assert.Error(t, DependenciesAreResolved(c2, task.Containers, "", nil, nil), "Dependencies should not be resolved") - task.Containers[0].KnownStatusUnsafe = apicontainerstatus.ContainerRunning + task.Containers[0].SetKnownStatus(apicontainerstatus.ContainerRunning) assert.NoError(t, DependenciesAreResolved(c2, task.Containers, "", nil, nil), "Dependencies should be resolved") task.Containers[1].SetDesiredStatus(apicontainerstatus.ContainerCreated) assert.NoError(t, DependenciesAreResolved(c1, task.Containers, "", nil, nil), "Dependencies should be resolved") } + func TestRunDependenciesWhenSteadyStateIsResourcesProvisionedForOneContainer(t *testing.T) { // Webserver stack - php := steadyStateContainer("php", []string{"db"}, []string{}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - db := steadyStateContainer("db", []string{}, []string{"dbdatavolume"}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - dbdata := createdContainer("dbdatavolume", []string{}, []string{}, apicontainerstatus.ContainerRunning) - webserver := steadyStateContainer("webserver", []string{"php"}, []string{"htmldata"}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - htmldata := steadyStateContainer("htmldata", []string{}, []string{"sharedcssfiles"}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - sharedcssfiles := createdContainer("sharedcssfiles", []string{}, []string{}, apicontainerstatus.ContainerRunning) + php := steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + db := steadyStateContainer("db", []apicontainer.DependsOn{{Container: "dbdatavolume", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + dbdata := createdContainer("dbdatavolume", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) + webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{Container: "php", Condition: "START"}, {Container: "htmldata", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{Container: "sharedcssfiles", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + sharedcssfiles := createdContainer("sharedcssfiles", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) // The Pause container, being added to the webserver stack - pause := steadyStateContainer("pause", []string{}, []string{}, apicontainerstatus.ContainerResourcesProvisioned, apicontainerstatus.ContainerResourcesProvisioned) + pause := steadyStateContainer("pause", []apicontainer.DependsOn{}, apicontainerstatus.ContainerResourcesProvisioned, apicontainerstatus.ContainerResourcesProvisioned) task := &apitask.Task{ Containers: []*apicontainer.Container{ @@ -240,156 +240,6 @@ func TestRunDependenciesWhenSteadyStateIsResourcesProvisionedForOneContainer(t * assert.NoError(t, err, "Php should resolve") } -func TestVolumeCanResolve(t *testing.T) { - testcases := []struct { - TargetDesired apicontainerstatus.ContainerStatus - VolumeDesired apicontainerstatus.ContainerStatus - Resolvable bool - }{ - { - TargetDesired: apicontainerstatus.ContainerCreated, - VolumeDesired: apicontainerstatus.ContainerStatusNone, - Resolvable: false, - }, - { - TargetDesired: apicontainerstatus.ContainerCreated, - VolumeDesired: apicontainerstatus.ContainerCreated, - Resolvable: true, - }, - { - TargetDesired: apicontainerstatus.ContainerCreated, - VolumeDesired: apicontainerstatus.ContainerRunning, - Resolvable: true, - }, - { - TargetDesired: apicontainerstatus.ContainerCreated, - VolumeDesired: apicontainerstatus.ContainerStopped, - Resolvable: true, - }, - { - TargetDesired: apicontainerstatus.ContainerCreated, - VolumeDesired: apicontainerstatus.ContainerZombie, - Resolvable: false, - }, - { - TargetDesired: apicontainerstatus.ContainerRunning, - VolumeDesired: apicontainerstatus.ContainerStatusNone, - Resolvable: false, - }, - { - TargetDesired: apicontainerstatus.ContainerRunning, - VolumeDesired: apicontainerstatus.ContainerCreated, - Resolvable: true, - }, - { - TargetDesired: apicontainerstatus.ContainerRunning, - VolumeDesired: apicontainerstatus.ContainerRunning, - Resolvable: true, - }, - { - TargetDesired: apicontainerstatus.ContainerRunning, - VolumeDesired: apicontainerstatus.ContainerStopped, - Resolvable: true, - }, - { - TargetDesired: apicontainerstatus.ContainerRunning, - VolumeDesired: apicontainerstatus.ContainerZombie, - Resolvable: false, - }, - { - TargetDesired: apicontainerstatus.ContainerStatusNone, - Resolvable: false, - }, - { - TargetDesired: apicontainerstatus.ContainerStopped, - Resolvable: false, - }, - { - TargetDesired: apicontainerstatus.ContainerZombie, - Resolvable: false, - }, - } - for _, tc := range testcases { - t.Run(fmt.Sprintf("T:%s+V:%s", tc.TargetDesired.String(), tc.VolumeDesired.String()), - assertCanResolve(volumeCanResolve, tc.TargetDesired, tc.VolumeDesired, tc.Resolvable)) - } -} - -func TestVolumeIsResolved(t *testing.T) { - testcases := []struct { - TargetDesired apicontainerstatus.ContainerStatus - VolumeKnown apicontainerstatus.ContainerStatus - Resolved bool - }{ - { - TargetDesired: apicontainerstatus.ContainerCreated, - VolumeKnown: apicontainerstatus.ContainerStatusNone, - Resolved: false, - }, - { - TargetDesired: apicontainerstatus.ContainerCreated, - VolumeKnown: apicontainerstatus.ContainerCreated, - Resolved: true, - }, - { - TargetDesired: apicontainerstatus.ContainerCreated, - VolumeKnown: apicontainerstatus.ContainerRunning, - Resolved: true, - }, - { - TargetDesired: apicontainerstatus.ContainerCreated, - VolumeKnown: apicontainerstatus.ContainerStopped, - Resolved: true, - }, - { - TargetDesired: apicontainerstatus.ContainerCreated, - VolumeKnown: apicontainerstatus.ContainerZombie, - Resolved: false, - }, - { - TargetDesired: apicontainerstatus.ContainerRunning, - VolumeKnown: apicontainerstatus.ContainerStatusNone, - Resolved: false, - }, - { - TargetDesired: apicontainerstatus.ContainerRunning, - VolumeKnown: apicontainerstatus.ContainerCreated, - Resolved: true, - }, - { - TargetDesired: apicontainerstatus.ContainerRunning, - VolumeKnown: apicontainerstatus.ContainerRunning, - Resolved: true, - }, - { - TargetDesired: apicontainerstatus.ContainerRunning, - VolumeKnown: apicontainerstatus.ContainerStopped, - Resolved: true, - }, - { - TargetDesired: apicontainerstatus.ContainerRunning, - VolumeKnown: apicontainerstatus.ContainerZombie, - Resolved: false, - }, - { - TargetDesired: apicontainerstatus.ContainerStatusNone, - Resolved: false, - }, - { - TargetDesired: apicontainerstatus.ContainerStopped, - Resolved: false, - }, - { - TargetDesired: apicontainerstatus.ContainerZombie, - Resolved: false, - }, - } - for _, tc := range testcases { - t.Run(fmt.Sprintf("T:%s+V:%s", tc.TargetDesired.String(), tc.VolumeKnown.String()), - assertResolved(volumeIsResolved, tc.TargetDesired, tc.VolumeKnown, tc.Resolved)) - } -} - func TestOnSteadyStateIsResolved(t *testing.T) { testcases := []struct { TargetDesired apicontainerstatus.ContainerStatus @@ -426,19 +276,6 @@ func TestOnSteadyStateIsResolved(t *testing.T) { } } -func assertCanResolve(f func(target *apicontainer.Container, dep *apicontainer.Container) bool, targetDesired, depKnown apicontainerstatus.ContainerStatus, expectedResolvable bool) func(t *testing.T) { - return func(t *testing.T) { - target := &apicontainer.Container{ - DesiredStatusUnsafe: targetDesired, - } - dep := &apicontainer.Container{ - DesiredStatusUnsafe: depKnown, - } - resolvable := f(target, dep) - assert.Equal(t, expectedResolvable, resolvable) - } -} - func assertResolved(f func(target *apicontainer.Container, dep *apicontainer.Container) bool, targetDesired, depKnown apicontainerstatus.ContainerStatus, expectedResolved bool) func(t *testing.T) { return func(t *testing.T) { target := &apicontainer.Container{ @@ -728,3 +565,197 @@ func TestTransitionDependencyResourceNotFound(t *testing.T) { resolved := verifyTransitionDependenciesResolved(target, nil, resources) assert.Equal(t, ErrResourceDependencyNotResolved, resolved) } + +func TestContainerOrderingCanResolve(t *testing.T) { + testcases := []struct { + TargetDesired apicontainerstatus.ContainerStatus + DependencyDesired apicontainerstatus.ContainerStatus + DependencyCondition string + Resolvable bool + }{ + { + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyDesired: apicontainerstatus.ContainerStatusNone, + DependencyCondition: "START", + Resolvable: false, + }, + { + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyDesired: apicontainerstatus.ContainerStopped, + DependencyCondition: "START", + Resolvable: true, + }, + { + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyDesired: apicontainerstatus.ContainerZombie, + DependencyCondition: "START", + Resolvable: false, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerStatusNone, + DependencyCondition: "START", + Resolvable: false, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerCreated, + DependencyCondition: "START", + Resolvable: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerRunning, + DependencyCondition: "START", + Resolvable: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerStopped, + DependencyCondition: "START", + Resolvable: true, + }, + { + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyDesired: apicontainerstatus.ContainerCreated, + DependencyCondition: "RUNNING", + Resolvable: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerRunning, + DependencyCondition: "RUNNING", + Resolvable: true, + }, + { + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyDesired: apicontainerstatus.ContainerRunning, + DependencyCondition: "RUNNING", + Resolvable: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerZombie, + DependencyCondition: "RUNNING", + Resolvable: false, + }, + { + TargetDesired: apicontainerstatus.ContainerStatusNone, + DependencyDesired: apicontainerstatus.ContainerStopped, + DependencyCondition: "RUNNING", + Resolvable: false, + }, + } + for _, tc := range testcases { + t.Run(fmt.Sprintf("T:%s+V:%s", tc.TargetDesired.String(), tc.DependencyDesired.String()), + assertContainerOrderingCanResolve(containerOrderingDependenciesCanResolve, tc.TargetDesired, + tc.DependencyDesired, tc.DependencyCondition, tc.Resolvable)) + } +} + +func TestContainerOrderingIsResolved(t *testing.T) { + testcases := []struct { + TargetDesired apicontainerstatus.ContainerStatus + DependencyKnown apicontainerstatus.ContainerStatus + DependencyCondition string + Resolved bool + }{ + { + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnown: apicontainerstatus.ContainerStatusNone, + DependencyCondition: "START", + Resolved: false, + }, + { + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnown: apicontainerstatus.ContainerCreated, + DependencyCondition: "START", + Resolved: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerStopped, + DependencyCondition: "START", + Resolved: true, + }, + { + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnown: apicontainerstatus.ContainerStopped, + DependencyCondition: "START", + Resolved: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerStatusNone, + DependencyCondition: "START", + Resolved: false, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerCreated, + DependencyCondition: "START", + Resolved: true, + }, + { + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnown: apicontainerstatus.ContainerCreated, + DependencyCondition: "RUNNING", + Resolved: true, + }, + { + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnown: apicontainerstatus.ContainerRunning, + DependencyCondition: "RUNNING", + Resolved: true, + }, + { + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnown: apicontainerstatus.ContainerZombie, + DependencyCondition: "RUNNING", + Resolved: false, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerRunning, + DependencyCondition: "RUNNING", + Resolved: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerZombie, + DependencyCondition: "RUNNING", + Resolved: false, + }, + } + for _, tc := range testcases { + t.Run(fmt.Sprintf("T:%s+V:%s", tc.TargetDesired.String(), tc.DependencyKnown.String()), + assertContainerOrderingResolved(containerOrderingDependenciesIsResolved, tc.TargetDesired, tc.DependencyKnown, tc.DependencyCondition, tc.Resolved)) + } +} + +func assertContainerOrderingCanResolve(f func(target *apicontainer.Container, dep *apicontainer.Container, depCond string) bool, targetDesired, depKnown apicontainerstatus.ContainerStatus, depCond string, expectedResolvable bool) func(t *testing.T) { + return func(t *testing.T) { + target := &apicontainer.Container{ + DesiredStatusUnsafe: targetDesired, + } + dep := &apicontainer.Container{ + DesiredStatusUnsafe: depKnown, + } + resolvable := f(target, dep, depCond) + assert.Equal(t, expectedResolvable, resolvable) + } +} + +func assertContainerOrderingResolved(f func(target *apicontainer.Container, dep *apicontainer.Container, depCond string) bool, targetDesired, depKnown apicontainerstatus.ContainerStatus, depCond string, expectedResolved bool) func(t *testing.T) { + return func(t *testing.T) { + target := &apicontainer.Container{ + DesiredStatusUnsafe: targetDesired, + } + dep := &apicontainer.Container{ + KnownStatusUnsafe: depKnown, + } + resolved := f(target, dep, depCond) + assert.Equal(t, expectedResolved, resolved) + } +} + From 79bd51770754121c4be28328a0f683c2526e18c8 Mon Sep 17 00:00:00 2001 From: Adnan Khan Date: Wed, 6 Feb 2019 19:56:01 +0000 Subject: [PATCH 04/24] engine,dockerclient: per container start/stop timeouts * start timeout is governed by the agent and is the context timeout for the StartContainer api call * stop timeout is the parameter passed to StopContainer and is time the docker daemon waits after a StopContainer call to issue a SIGKILL to the container --- agent/acs/model/api/api-2.json | 6 +- agent/acs/model/ecsacs/api.go | 4 ++ agent/api/container/container.go | 20 +++++++ agent/api/container/container_test.go | 17 +++++- agent/api/task/task_test.go | 59 ++++++++++++------- agent/dockerclient/dockerapi/docker_client.go | 22 ++++--- .../dockerapi/docker_client_test.go | 1 + agent/engine/docker_task_engine.go | 18 ++++-- agent/engine/docker_task_engine_test.go | 2 +- agent/engine/engine_unix_integ_test.go | 39 ++++++++++++ 10 files changed, 150 insertions(+), 38 deletions(-) diff --git a/agent/acs/model/api/api-2.json b/agent/acs/model/api/api-2.json index 4e99a8fdddc..0c96d646e18 100644 --- a/agent/acs/model/api/api-2.json +++ b/agent/acs/model/api/api-2.json @@ -204,7 +204,9 @@ "registryAuthentication":{"shape":"RegistryAuthenticationData"}, "logsAuthStrategy":{"shape":"AuthStrategy"}, "secrets":{"shape":"SecretList"}, - "dependsOn":{"shape": "DependsOnList"} + "dependsOn":{"shape": "DependsOnList"}, + "startTimeout":{"shape":"Integer"}, + "stopTimeout":{"shape":"Integer"} } }, "ContainerList":{ @@ -613,4 +615,4 @@ ] } } -} \ No newline at end of file +} diff --git a/agent/acs/model/ecsacs/api.go b/agent/acs/model/ecsacs/api.go index 2c5f3f98693..4ae7360d5da 100644 --- a/agent/acs/model/ecsacs/api.go +++ b/agent/acs/model/ecsacs/api.go @@ -238,6 +238,10 @@ type Container struct { Secrets []*Secret `locationName:"secrets" type:"list"` + StartTimeout *int64 `locationName:"startTimeout" type:"integer"` + + StopTimeout *int64 `locationName:"stopTimeout" type:"integer"` + VolumesFrom []*VolumeFrom `locationName:"volumesFrom" type:"list"` } diff --git a/agent/api/container/container.go b/agent/api/container/container.go index 25e450df245..50f19488372 100644 --- a/agent/api/container/container.go +++ b/agent/api/container/container.go @@ -140,6 +140,12 @@ type Container struct { // LogsAuthStrategy specifies how the logs driver for the container will be // authenticated LogsAuthStrategy string + // StartTimeout specifies the time the agent waits for StartContainer api call + // before timing out + StartTimeout uint + // StopTimeout specifies the time value to be passed as StopContainer api call + StopTimeout uint + // lock is used for fields that are accessed and updated concurrently lock sync.RWMutex @@ -867,3 +873,17 @@ func (c *Container) HasSecretAsEnv() bool { } return false } + +func (c *Container) GetStartTimeout() time.Duration { + c.lock.Lock() + defer c.lock.Unlock() + + return time.Duration(c.StartTimeout) * time.Second +} + +func (c *Container) GetStopTimeout() time.Duration { + c.lock.Lock() + defer c.lock.Unlock() + + return time.Duration(c.StopTimeout) * time.Second +} diff --git a/agent/api/container/container_test.go b/agent/api/container/container_test.go index 921e09ebe19..abd93c1c116 100644 --- a/agent/api/container/container_test.go +++ b/agent/api/container/container_test.go @@ -327,7 +327,7 @@ func TestMergeEnvironmentVariables(t *testing.T) { }, { - Name: "merge single item to nil container env var map", + Name: "merge single item to nil container env var map", InContainerEnvironment: nil, InEnvVarMap: map[string]string{ "SECRET1": "secret1"}, @@ -357,7 +357,7 @@ func TestMergeEnvironmentVariables(t *testing.T) { }, { - Name: "merge nil to nil container env var map", + Name: "merge nil to nil container env var map", InContainerEnvironment: nil, InEnvVarMap: nil, OutEnvVarMap: map[string]string{}, @@ -455,3 +455,16 @@ func TestHasSecretAsEnv(t *testing.T) { } } + +func TestPerContainerTimeouts(t *testing.T) { + timeout := uint(10) + expectedTimeout := time.Duration(timeout) * time.Second + + container := Container{ + StartTimeout: timeout, + StopTimeout: timeout, + } + + assert.Equal(t, container.GetStartTimeout(), expectedTimeout) + assert.Equal(t, container.GetStopTimeout(), expectedTimeout) +} diff --git a/agent/api/task/task_test.go b/agent/api/task/task_test.go index 491fa50218a..6cfb5c9dd4f 100644 --- a/agent/api/task/task_test.go +++ b/agent/api/task/task_test.go @@ -2845,28 +2845,27 @@ func TestTaskGPUDisabled(t *testing.T) { }, }, } - assert.False(t, testTask.isGPUEnabled()) } func TestInitializeContainerOrderingWithLinksAndVolumesFrom(t *testing.T) { containerWithOnlyVolume := &apicontainer.Container{ - Name: "myName", - Image: "image:tag", + Name: "myName", + Image: "image:tag", VolumesFrom: []apicontainer.VolumeFrom{{SourceContainer: "myName1"}}, } containerWithOnlyLink := &apicontainer.Container{ Name: "myName1", Image: "image:tag", - Links: []string{"myName"}, + Links: []string{"myName"}, } containerWithBothVolumeAndLink := &apicontainer.Container{ - Name: "myName2", - Image: "image:tag", + Name: "myName2", + Image: "image:tag", VolumesFrom: []apicontainer.VolumeFrom{{SourceContainer: "myName"}}, - Links: []string{"myName1"}, + Links: []string{"myName1"}, } containerWithNoVolumeOrLink := &apicontainer.Container{ @@ -2877,8 +2876,8 @@ func TestInitializeContainerOrderingWithLinksAndVolumesFrom(t *testing.T) { task := &Task{ Arn: "test", ResourcesMapUnsafe: make(map[string][]taskresource.TaskResource), - Containers: []*apicontainer.Container{containerWithOnlyVolume, containerWithOnlyLink, - containerWithBothVolumeAndLink, containerWithNoVolumeOrLink}, + Containers: []*apicontainer.Container{containerWithOnlyVolume, containerWithOnlyLink, + containerWithBothVolumeAndLink, containerWithNoVolumeOrLink}, } err := task.initializeContainerOrderingForVolumes() @@ -2887,27 +2886,27 @@ func TestInitializeContainerOrderingWithLinksAndVolumesFrom(t *testing.T) { assert.NoError(t, err) containerResultWithVolume := task.Containers[0] - assert.Equal(t, "myName1", containerResultWithVolume.DependsOn[0].Container) - assert.Equal(t, ContainerOrderingStartCondition, containerResultWithVolume.DependsOn[0].Condition) + assert.Equal(t, "myName1", containerResultWithVolume.DependsOn[0].Container) + assert.Equal(t, ContainerOrderingStartCondition, containerResultWithVolume.DependsOn[0].Condition) containerResultWithLink := task.Containers[1] - assert.Equal(t, "myName", containerResultWithLink.DependsOn[0].Container) - assert.Equal(t, ContainerOrderingRunningCondition, containerResultWithLink.DependsOn[0].Condition) + assert.Equal(t, "myName", containerResultWithLink.DependsOn[0].Container) + assert.Equal(t, ContainerOrderingRunningCondition, containerResultWithLink.DependsOn[0].Condition) containerResultWithBothVolumeAndLink := task.Containers[2] - assert.Equal(t, "myName", containerResultWithBothVolumeAndLink.DependsOn[0].Container) - assert.Equal(t, ContainerOrderingStartCondition, containerResultWithBothVolumeAndLink.DependsOn[0].Condition) - assert.Equal(t, "myName1", containerResultWithBothVolumeAndLink.DependsOn[1].Container) - assert.Equal(t, ContainerOrderingRunningCondition, containerResultWithBothVolumeAndLink.DependsOn[1].Condition) + assert.Equal(t, "myName", containerResultWithBothVolumeAndLink.DependsOn[0].Container) + assert.Equal(t, ContainerOrderingStartCondition, containerResultWithBothVolumeAndLink.DependsOn[0].Condition) + assert.Equal(t, "myName1", containerResultWithBothVolumeAndLink.DependsOn[1].Container) + assert.Equal(t, ContainerOrderingRunningCondition, containerResultWithBothVolumeAndLink.DependsOn[1].Condition) containerResultWithNoVolumeOrLink := task.Containers[3] - assert.Equal(t, 0, len(containerResultWithNoVolumeOrLink.DependsOn)) + assert.Equal(t, 0, len(containerResultWithNoVolumeOrLink.DependsOn)) } func TestInitializeContainerOrderingWithError(t *testing.T) { containerWithVolumeError := &apicontainer.Container{ - Name: "myName", - Image: "image:tag", + Name: "myName", + Image: "image:tag", VolumesFrom: []apicontainer.VolumeFrom{{SourceContainer: "dummyContainer"}}, } @@ -2945,3 +2944,23 @@ func TestInitializeContainerOrderingWithError(t *testing.T) { errLink2 := task2.initializeContainerOrderingForLinks() assert.Error(t, errLink2) } + +func TestPostUnmarshalPerContainerTimeouts(t *testing.T) { + modelTimeout := int64(10) + expectedTimeout := uint(modelTimeout) + + taskFromACS := ecsacs.Task{ + Containers: []*ecsacs.Container{ + { + StartTimeout: aws.Int64(modelTimeout), + StopTimeout: aws.Int64(modelTimeout), + }, + }, + } + seqNum := int64(42) + task, err := TaskFromACS(&taskFromACS, &ecsacs.PayloadMessage{SeqNum: &seqNum}) + assert.Nil(t, err, "Should be able to handle acs task") + + assert.Equal(t, task.Containers[0].StartTimeout, expectedTimeout) + assert.Equal(t, task.Containers[0].StopTimeout, expectedTimeout) +} diff --git a/agent/dockerclient/dockerapi/docker_client.go b/agent/dockerclient/dockerapi/docker_client.go index 2eb0a4c17ed..6c8641b73db 100644 --- a/agent/dockerclient/dockerapi/docker_client.go +++ b/agent/dockerclient/dockerapi/docker_client.go @@ -91,6 +91,8 @@ const ( pullRetryJitterMultiplier = 0.2 ) +var ctxTimeoutStopContainer = dockerclient.StopContainerTimeout + // DockerClient interface to make testing it easier type DockerClient interface { // SupportedVersions returns a slice of the supported docker versions (or at least supposedly supported). @@ -550,8 +552,8 @@ func (dg *dockerGoClient) createContainer(ctx context.Context, return dg.containerMetadata(ctx, dockerContainer.ID) } -func (dg *dockerGoClient) StartContainer(ctx context.Context, id string, timeout time.Duration) DockerContainerMetadata { - ctx, cancel := context.WithTimeout(ctx, timeout) +func (dg *dockerGoClient) StartContainer(ctx context.Context, id string, ctxTimeout time.Duration) DockerContainerMetadata { + ctx, cancel := context.WithTimeout(ctx, ctxTimeout) defer cancel() defer metrics.MetricsEngineGlobal.RecordDockerMetric("START_CONTAINER")() // Buffered channel so in the case of timeout it takes one write, never gets @@ -566,7 +568,7 @@ func (dg *dockerGoClient) StartContainer(ctx context.Context, id string, timeout // send back the DockerTimeoutError err := ctx.Err() if err == context.DeadlineExceeded { - return DockerContainerMetadata{Error: &DockerTimeoutError{timeout, "started"}} + return DockerContainerMetadata{Error: &DockerTimeoutError{ctxTimeout, "started"}} } return DockerContainerMetadata{Error: CannotStartContainerError{err}} } @@ -654,13 +656,16 @@ func (dg *dockerGoClient) inspectContainer(ctx context.Context, dockerID string) } func (dg *dockerGoClient) StopContainer(ctx context.Context, dockerID string, timeout time.Duration) DockerContainerMetadata { - ctx, cancel := context.WithTimeout(ctx, timeout) + // ctxTimeout is sum of timeout(applied to the StopContainer api call) and a fixed constant dockerclient.StopContainerTimeout + // the context's timeout should be greater than the sigkill timout for the StopContainer call + ctxTimeout := timeout + ctxTimeoutStopContainer + ctx, cancel := context.WithTimeout(ctx, ctxTimeout) defer cancel() defer metrics.MetricsEngineGlobal.RecordDockerMetric("STOP_CONTAINER")() // Buffered channel so in the case of timeout it takes one write, never gets // read, and can still be GC'd response := make(chan DockerContainerMetadata, 1) - go func() { response <- dg.stopContainer(ctx, dockerID) }() + go func() { response <- dg.stopContainer(ctx, dockerID, timeout) }() select { case resp := <-response: return resp @@ -669,19 +674,18 @@ func (dg *dockerGoClient) StopContainer(ctx context.Context, dockerID string, ti // send back the DockerTimeoutError err := ctx.Err() if err == context.DeadlineExceeded { - return DockerContainerMetadata{Error: &DockerTimeoutError{timeout, "stopped"}} + return DockerContainerMetadata{Error: &DockerTimeoutError{ctxTimeout, "stopped"}} } return DockerContainerMetadata{Error: CannotStopContainerError{err}} } } -func (dg *dockerGoClient) stopContainer(ctx context.Context, dockerID string) DockerContainerMetadata { +func (dg *dockerGoClient) stopContainer(ctx context.Context, dockerID string, timeout time.Duration) DockerContainerMetadata { client, err := dg.sdkDockerClient() if err != nil { return DockerContainerMetadata{Error: CannotGetDockerClientError{version: dg.version, err: err}} } - - err = client.ContainerStop(ctx, dockerID, &dg.config.DockerStopTimeout) + err = client.ContainerStop(ctx, dockerID, &timeout) metadata := dg.containerMetadata(ctx, dockerID) if err != nil { seelog.Infof("DockerGoClient: error stopping container %s: %v", dockerID, err) diff --git a/agent/dockerclient/dockerapi/docker_client_test.go b/agent/dockerclient/dockerapi/docker_client_test.go index 2a487aa8b44..70db0d83bf7 100644 --- a/agent/dockerclient/dockerapi/docker_client_test.go +++ b/agent/dockerclient/dockerapi/docker_client_test.go @@ -468,6 +468,7 @@ func TestStopContainerTimeout(t *testing.T) { cfg.DockerStopTimeout = xContainerShortTimeout mockDockerSDK, client, _, _, _, done := dockerClientSetupWithConfig(t, cfg) defer done() + ctxTimeoutStopContainer = xContainerShortTimeout wait := &sync.WaitGroup{} wait.Add(1) diff --git a/agent/engine/docker_task_engine.go b/agent/engine/docker_task_engine.go index efd12e68337..f829a448dc7 100644 --- a/agent/engine/docker_task_engine.go +++ b/agent/engine/docker_task_engine.go @@ -955,7 +955,13 @@ func (engine *DockerTaskEngine) startContainer(task *apitask.Task, container *ap } } startContainerBegin := time.Now() - dockerContainerMD := client.StartContainer(engine.ctx, dockerContainer.DockerID, engine.cfg.ContainerStartTimeout) + + ctxTimeoutStartContainer := container.GetStartTimeout() + if ctxTimeoutStartContainer <= 0 { + ctxTimeoutStartContainer = engine.cfg.ContainerStartTimeout + } + + dockerContainerMD := client.StartContainer(engine.ctx, dockerContainer.DockerID, ctxTimeoutStartContainer) // 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 @@ -1095,9 +1101,13 @@ func (engine *DockerTaskEngine) stopContainer(task *apitask.Task, container *api } seelog.Infof("Task engine [%s]: cleaned pause container network namespace", task.Arn) } - // timeout is defined by the const 'stopContainerTimeout' and the 'DockerStopTimeout' in the config - timeout := engine.cfg.DockerStopTimeout + dockerclient.StopContainerTimeout - return engine.client.StopContainer(engine.ctx, dockerContainer.DockerID, timeout) + + apiTimeoutStopContainer := container.GetStopTimeout() + if apiTimeoutStopContainer <= 0 { + apiTimeoutStopContainer = engine.cfg.DockerStopTimeout + } + + return engine.client.StopContainer(engine.ctx, dockerContainer.DockerID, apiTimeoutStopContainer) } func (engine *DockerTaskEngine) removeContainer(task *apitask.Task, container *apicontainer.Container) error { diff --git a/agent/engine/docker_task_engine_test.go b/agent/engine/docker_task_engine_test.go index eccffc202f3..89aa3ae504a 100644 --- a/agent/engine/docker_task_engine_test.go +++ b/agent/engine/docker_task_engine_test.go @@ -1271,7 +1271,7 @@ func TestStopPauseContainerCleanupCalled(t *testing.T) { mockCNIClient.EXPECT().CleanupNS(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil), dockerClient.EXPECT().StopContainer(gomock.Any(), containerID, - defaultConfig.DockerStopTimeout+dockerclient.StopContainerTimeout, + defaultConfig.DockerStopTimeout, ).Return(dockerapi.DockerContainerMetadata{}), ) diff --git a/agent/engine/engine_unix_integ_test.go b/agent/engine/engine_unix_integ_test.go index bdcb4d07313..9ef01080e16 100644 --- a/agent/engine/engine_unix_integ_test.go +++ b/agent/engine/engine_unix_integ_test.go @@ -1447,3 +1447,42 @@ func TestGPUAssociationTask(t *testing.T) { go taskEngine.AddTask(&taskUpdate) verifyTaskIsStopped(stateChangeEvents, testTask) } + +func TestPerContainerStopTimeout(t *testing.T) { + // set the global stop timemout, but this should be ignored since the per container value is set + globalStopContainerTimeout := 1000 * time.Second + os.Setenv("ECS_CONTAINER_STOP_TIMEOUT", globalStopContainerTimeout.String()) + defer os.Unsetenv("ECS_CONTAINER_STOP_TIMEOUT") + cfg := defaultTestConfigIntegTest() + + taskEngine, _, _ := setup(cfg, nil, t) + + dockerTaskEngine := taskEngine.(*DockerTaskEngine) + + if dockerTaskEngine.cfg.DockerStopTimeout != globalStopContainerTimeout { + t.Errorf("Expect ECS_CONTAINER_STOP_TIMEOUT to be set to , %v", dockerTaskEngine.cfg.DockerStopTimeout) + } + + testTask := createTestTask("TestDockerStopTimeout") + testTask.Containers[0].Command = []string{"sh", "-c", "trap 'echo hello' SIGTERM; while true; do echo `date +%T`; sleep 1s; done;"} + testTask.Containers[0].Image = testBusyboxImage + testTask.Containers[0].Name = "test-docker-timeout" + testTask.Containers[0].StopTimeout = uint(testDockerStopTimeout.Seconds()) + + go dockerTaskEngine.AddTask(testTask) + + verifyContainerRunningStateChange(t, taskEngine) + verifyTaskRunningStateChange(t, taskEngine) + + startTime := ttime.Now() + dockerTaskEngine.stopContainer(testTask, testTask.Containers[0]) + + verifyContainerStoppedStateChange(t, taskEngine) + + if ttime.Since(startTime) < testDockerStopTimeout { + t.Errorf("Container stopped before the timeout: %v", ttime.Since(startTime)) + } + if ttime.Since(startTime) > testDockerStopTimeout+1*time.Second { + t.Errorf("Container should have stopped eariler, but stopped after %v", ttime.Since(startTime)) + } +} From 3265bf90061696bb1bdb235c4a99a19b7c36c99a Mon Sep 17 00:00:00 2001 From: Adnan Khan Date: Fri, 15 Feb 2019 16:38:33 -0500 Subject: [PATCH 05/24] statemanager: add container start/stop statemanger --- agent/api/task/task_test.go | 2 +- agent/statemanager/state_manager.go | 5 +- agent/statemanager/state_manager_test.go | 35 +++ .../perContainerTimeouts/ecs_agent_data.json | 202 ++++++++++++++++++ 4 files changed, 242 insertions(+), 2 deletions(-) create mode 100644 agent/statemanager/testdata/v20/perContainerTimeouts/ecs_agent_data.json diff --git a/agent/api/task/task_test.go b/agent/api/task/task_test.go index 6cfb5c9dd4f..c642ce47c01 100644 --- a/agent/api/task/task_test.go +++ b/agent/api/task/task_test.go @@ -2945,7 +2945,7 @@ func TestInitializeContainerOrderingWithError(t *testing.T) { assert.Error(t, errLink2) } -func TestPostUnmarshalPerContainerTimeouts(t *testing.T) { +func TestTaskFromACSPerContainerTimeouts(t *testing.T) { modelTimeout := int64(10) expectedTimeout := uint(modelTimeout) diff --git a/agent/statemanager/state_manager.go b/agent/statemanager/state_manager.go index e9e2b44a0a8..86f731ae38d 100644 --- a/agent/statemanager/state_manager.go +++ b/agent/statemanager/state_manager.go @@ -78,7 +78,10 @@ const ( // a) Add 'Associations' field to 'api.task.task' // b) Add 'GPUIDs' field to 'apicontainer.Container' // c) Add 'NvidiaRuntime' field to 'api.task.task' - // 20) Add 'DependsOn' field to 'apicontainer.Container' + // 20) + // a) Add 'DependsOn' field to 'apicontainer.Container' + // b) Add 'StartTime' field to 'api.container.Container' + // c) Add 'StopTime' field to 'api.container.Container' ECSDataVersion = 20 // ecsDataFile specifies the filename in the ECS_DATADIR diff --git a/agent/statemanager/state_manager_test.go b/agent/statemanager/state_manager_test.go index e81d8cd69ec..16cc45bd09d 100644 --- a/agent/statemanager/state_manager_test.go +++ b/agent/statemanager/state_manager_test.go @@ -362,3 +362,38 @@ func TestLoadsDataForContainerOrdering(t *testing.T) { assert.Equal(t, "container_1", dependsOn[0].Container) assert.Equal(t, "START", dependsOn[0].Condition) } + +func TestLoadsDataForPerContainerTimeouts(t *testing.T) { + cleanup, err := setupWindowsTest(filepath.Join(".", "testdata", "v20", "perContainerTimeouts", "ecs_agent_data.json")) + require.Nil(t, err, "Failed to set up test") + defer cleanup() + cfg := &config.Config{DataDir: filepath.Join(".", "testdata", "v20", "perContainerTimeouts")} + taskEngine := engine.NewTaskEngine(&config.Config{}, nil, nil, nil, nil, dockerstate.NewTaskEngineState(), nil, nil) + var containerInstanceArn, cluster, savedInstanceID string + var sequenceNumber int64 + stateManager, err := statemanager.NewStateManager(cfg, + statemanager.AddSaveable("TaskEngine", taskEngine), + statemanager.AddSaveable("ContainerInstanceArn", &containerInstanceArn), + statemanager.AddSaveable("Cluster", &cluster), + statemanager.AddSaveable("EC2InstanceID", &savedInstanceID), + statemanager.AddSaveable("SeqNum", &sequenceNumber), + ) + assert.NoError(t, err) + err = stateManager.Load() + assert.NoError(t, err) + assert.Equal(t, "state-file", cluster) + assert.EqualValues(t, 0, sequenceNumber) + + tasks, err := taskEngine.ListTasks() + assert.NoError(t, err) + assert.Equal(t, 1, len(tasks)) + + task := tasks[0] + assert.Equal(t, "arn:aws:ecs:us-west-2:1234567890:task/33425c99-5db7-45fb-8244-bc94d00661e4", task.Arn) + assert.Equal(t, "per-container-timeouts", task.Family) + assert.Equal(t, 1, len(task.Containers)) + + c1 := task.Containers[0] + assert.Equal(t, uint(10), c1.StartTimeout) + assert.Equal(t, uint(10), c1.StopTimeout) +} diff --git a/agent/statemanager/testdata/v20/perContainerTimeouts/ecs_agent_data.json b/agent/statemanager/testdata/v20/perContainerTimeouts/ecs_agent_data.json new file mode 100644 index 00000000000..459f2bd3cbe --- /dev/null +++ b/agent/statemanager/testdata/v20/perContainerTimeouts/ecs_agent_data.json @@ -0,0 +1,202 @@ +{ + "Data": { + "Cluster": "state-file", + "ContainerInstanceArn": "arn:aws:ecs:us-west-2:1234567890:container-instance/46efd519-df3f-4096-8f34-faebb1747752", + "EC2InstanceID": "i-0da29eb1a8a98768b", + "TaskEngine": { + "Tasks": [ + { + "Arn": "arn:aws:ecs:us-west-2:1234567890:task/33425c99-5db7-45fb-8244-bc94d00661e4", + "Family": "per-container-timeouts", + "Version": "1", + "Containers": [ + { + "Name": "container_1", + "Image": "amazonlinux:1", + "ImageID": "sha256:7f929d2604c7e504a568eac9a2523c1b9e9b15e1fcee4076e1411a552913d08e", + "Command": [ + "sleep", + "3600" + ], + "Cpu": 0, + "GPUIDs": ["0", "1"], + "Memory": 512, + "Links": null, + "volumesFrom": [], + "mountPoints": [], + "portMappings": [], + "Essential": true, + "StartTimeout":10, + "StopTimeout":10, + "EntryPoint": null, + "environment": { + "NVIDIA_VISIBLE_DEVICES": "0,1" + }, + "overrides": { + "command": null + }, + "dockerConfig": { + "config": "{}", + "hostConfig": "{\"CapAdd\":[],\"CapDrop\":[]}", + "version": "1.17" + }, + "registryAuthentication": null, + "secrets": [], + "LogsAuthStrategy": "", + "desiredStatus": "RUNNING", + "KnownStatus": "RUNNING", + "TransitionDependencySet": { + "1": { + "ContainerDependencies": null, + "ResourceDependencies": [ + { + "Name": "cgroup", + "RequiredStatus": 1 + } + ] + } + }, + "RunDependencies": null, + "IsInternal": "NORMAL", + "ApplyingError": { + "error": "API error (500): Get https://registry-1.docker.io/v2/library/amazonlinux/manifests/1: toomanyrequests: too many failed login attempts for username or IP address\n", + "name": "CannotPullContainerError" + }, + "SentStatus": "RUNNING", + "metadataFileUpdated": false, + "KnownExitCode": null, + "KnownPortBindings": null + } + ], + "Associations": [ + { + "Containers": ["container_1"], + "Content": { + "Encoding": "base64", + "Value": "val" + }, + "Name": "0", + "Type": "gpu" + }, + { + "Containers": ["container_1"], + "Content": { + "Encoding": "base64", + "Value": "val" + }, + "Name": "1", + "Type": "gpu" + } + ], + "resources": { + "cgroup": [ + { + "cgroupRoot": "/ecs/33425c99-5db7-45fb-8244-bc94d00661e4", + "cgroupMountPath": "/sys/fs/cgroup", + "createdAt": "0001-01-01T00:00:00Z", + "desiredStatus": "CREATED", + "knownStatus": "CREATED", + "resourceSpec": { + "cpu": { + "shares": 2 + } + } + } + ] + }, + "volumes": [], + "DesiredStatus": "RUNNING", + "KnownStatus": "RUNNING", + "KnownTime": "2018-10-04T18:05:49.121835686Z", + "PullStartedAt": "2018-10-04T18:05:34.359798761Z", + "PullStoppedAt": "2018-10-04T18:05:48.445985904Z", + "ExecutionStoppedAt": "0001-01-01T00:00:00Z", + "SentStatus": "RUNNING", + "StartSequenceNumber": 2, + "StopSequenceNumber": 0, + "executionCredentialsID": "b1a6ede6-1a9f-4ab3-a02e-bd3e51b11244", + "ENI": null, + "MemoryCPULimitsEnabled": true, + "PlatformFields": {} + } + ], + "IdToContainer": { + "8f5e6e3091f221c876103289ddabcbcdeb64acd7ac7e2d0cf4da2be2be9d8956": { + "DockerId": "8f5e6e3091f221c876103289ddabcbcdeb64acd7ac7e2d0cf4da2be2be9d8956", + "DockerName": "ecs-private-registry-state-1-container1-a68ef4b6e0fba38d3500", + "Container": { + "Name": "container_1", + "Image": "amazonlinux:1", + "ImageID": "sha256:7f929d2604c7e504a568eac9a2523c1b9e9b15e1fcee4076e1411a552913d08e", + "Command": [ + "sleep", + "3600" + ], + "Cpu": 0, + "Memory": 512, + "Links": null, + "volumesFrom": [], + "mountPoints": [], + "portMappings": [], + "Essential": true, + "EntryPoint": null, + "environment": {}, + "overrides": { + "command": null + }, + "dockerConfig": { + "config": "{}", + "hostConfig": "{\"CapAdd\":[],\"CapDrop\":[]}", + "version": "1.17" + }, + "registryAuthentication": null, + "LogsAuthStrategy": "", + "desiredStatus": "RUNNING", + "KnownStatus": "RUNNING", + "TransitionDependencySet": { + "1": { + "ContainerDependencies": null, + "ResourceDependencies": [ + { + "Name": "cgroup", + "RequiredStatus": 1 + } + ] + } + }, + "RunDependencies": null, + "IsInternal": "NORMAL", + "ApplyingError": { + "error": "API error (500): Get https://registry-1.docker.io/v2/library/amazonlinux/manifests/1: toomanyrequests: too many failed login attempts for username or IP address\n", + "name": "CannotPullContainerError" + }, + "SentStatus": "RUNNING", + "metadataFileUpdated": false, + "KnownExitCode": null, + "KnownPortBindings": null + } + } + }, + "IdToTask": { + "8f5e6e3091f221c876103289ddabcbcdeb64acd7ac7e2d0cf4da2be2be9d8956": "arn:aws:ecs:us-west-2:1234567890:task/33425c99-5db7-45fb-8244-bc94d00661e4" + }, + "ImageStates": [ + { + "Image": { + "ImageID": "sha256:7f929d2604c7e504a568eac9a2523c1b9e9b15e1fcee4076e1411a552913d08e", + "Names": [ + "amazonlinux:1" + ], + "Size": 165452304 + }, + "PulledAt": "2018-10-04T18:05:48.445644088Z", + "LastUsedAt": "2018-10-04T18:05:48.445645342Z", + "PullSucceeded": false + } + ], + "ENIAttachments": null, + "IPToTask": {} + } + }, + "Version": 20 +} From 6dcd3e3302da15582c27f50b5c15f7e20fc3c0ca Mon Sep 17 00:00:00 2001 From: Utsa Bhattacharjya Date: Wed, 13 Feb 2019 11:32:07 -0800 Subject: [PATCH 06/24] Add Dependency conditions of 'Complete', 'Success' and 'Healthy' 'Success' condition on a dependency container will allow a target container to start only when the dependency container has exitted successfully with exitcode 0. 'Complete' condition on a dependency container will allow a target container to start only when the dependency container has exitted. 'Healthy' condition on a dependency container will allow a target container to start only when the dependency container is reported to be healthy. --- agent/api/container/container.go | 2 +- agent/api/container/container_test.go | 2 +- agent/engine/dependencygraph/graph.go | 90 ++++-- agent/engine/dependencygraph/graph_test.go | 336 ++++++++++++++------- 4 files changed, 294 insertions(+), 136 deletions(-) diff --git a/agent/api/container/container.go b/agent/api/container/container.go index 50f19488372..9942eab8250 100644 --- a/agent/api/container/container.go +++ b/agent/api/container/container.go @@ -132,7 +132,7 @@ type Container struct { DockerConfig DockerConfig `json:"dockerConfig"` // RegistryAuthentication is the auth data used to pull image RegistryAuthentication *RegistryAuthenticationData `json:"registryAuthentication"` - // HealthCheckType is the mechnism to use for the container health check + // HealthCheckType is the mechanism to use for the container health check // currently it only supports 'DOCKER' HealthCheckType string `json:"healthCheckType,omitempty"` // Health contains the health check information of container health check diff --git a/agent/api/container/container_test.go b/agent/api/container/container_test.go index abd93c1c116..d588e109ae5 100644 --- a/agent/api/container/container_test.go +++ b/agent/api/container/container_test.go @@ -179,7 +179,7 @@ func TestSetupExecutionRoleFlag(t *testing.T) { } } -func TestSetHealtStatus(t *testing.T) { +func TestSetHealthStatus(t *testing.T) { container := Container{} // set the container status to be healthy diff --git a/agent/engine/dependencygraph/graph.go b/agent/engine/dependencygraph/graph.go index 0a728a329a1..29f8fb0c70f 100644 --- a/agent/engine/dependencygraph/graph.go +++ b/agent/engine/dependencygraph/graph.go @@ -25,6 +25,22 @@ import ( "strings" ) +const ( + // StartCondition ensures that a container progresses to next state only when dependency container has started + startCondition = "START" + // RunningCondition ensures that a container progresses to next state only when dependency container is running + runningCondition = "RUNNING" + // SuccessCondition ensures that a container progresses to next state only when + // dependency container has successfully completed with exit code 0 + successCondition = "SUCCESS" + // CompleteCondition ensures that a container progresses to next state only when dependency container has completed + completeCondition = "COMPLETE" + // StartCondition ensures that a container progresses to next state only when dependency container is healthy + healthyCondition = "HEALTHY" + // 0 is the standard exit code for success. + successExitCode = 0 +) + var ( // CredentialsNotResolvedErr is the error where a container needs to wait for // credentials before it can process by agent @@ -256,26 +272,15 @@ func verifyResourceDependenciesResolved(target *apicontainer.Container, existing func containerOrderingDependenciesCanResolve(target *apicontainer.Container, dependsOnContainer *apicontainer.Container, dependsOnStatus string) bool { + targetDesiredStatus := target.GetDesiredStatus() dependsOnContainerDesiredStatus := dependsOnContainer.GetDesiredStatus() switch dependsOnStatus { + case startCondition: + return verifyContainerOrderingStatus(dependsOnContainer) - case "START": - // The 'target' container doesn't desire to move to either 'Created' or the 'steady' state, - // which is not allowed - if targetDesiredStatus != apicontainerstatus.ContainerCreated && targetDesiredStatus != target.GetSteadyStateStatus() { - return false - } - // The 'target' container desires to be moved to 'Created' or the 'steady' state. - // Allow this only if the dependency container also desires to be started - // i.e it's status is any of 'Created', 'steady state' or 'Stopped' - return dependsOnContainerDesiredStatus == apicontainerstatus.ContainerCreated || - dependsOnContainerDesiredStatus == apicontainerstatus.ContainerStopped || - dependsOnContainerDesiredStatus == dependsOnContainer.GetSteadyStateStatus() - - case "RUNNING": - + case runningCondition: if targetDesiredStatus == apicontainerstatus.ContainerCreated { // The 'target' container desires to be moved to 'Created' state. // Allow this only if the desired status of the dependency container is @@ -287,10 +292,24 @@ func containerOrderingDependenciesCanResolve(target *apicontainer.Container, // The 'target' container desires to be moved to its 'steady' state. // Allow this only if the dependency container also desires to be in 'steady state' return dependsOnContainerDesiredStatus == dependsOnContainer.GetSteadyStateStatus() - } return false + case successCondition: + var dependencyStoppedSuccessfully bool + + if dependsOnContainer.GetKnownExitCode() != nil { + dependencyStoppedSuccessfully = dependsOnContainer.GetKnownStatus() == apicontainerstatus.ContainerStopped && + *dependsOnContainer.GetKnownExitCode() == successExitCode + } + return verifyContainerOrderingStatus(dependsOnContainer) || dependencyStoppedSuccessfully + + case completeCondition: + return verifyContainerOrderingStatus(dependsOnContainer) + + case healthyCondition: + return verifyContainerOrderingStatus(dependsOnContainer) && dependsOnContainer.HealthStatusShouldBeReported() + default: return false } @@ -304,13 +323,7 @@ func containerOrderingDependenciesIsResolved(target *apicontainer.Container, dependsOnContainerKnownStatus := dependsOnContainer.GetKnownStatus() switch dependsOnStatus { - - case "START": - // The 'target' container doesn't desire to be moved to 'Created' or the 'steady' state. - // which is not allowed. - if targetDesiredStatus != apicontainerstatus.ContainerCreated && targetDesiredStatus != apicontainerstatus.ContainerRunning { - return false - } + case startCondition: // The 'target' container desires to be moved to 'Created' or the 'steady' state. // Allow this only if the known status of the dependency container state is already started // i.e it's state is any of 'Created', 'steady state' or 'Stopped' @@ -318,27 +331,52 @@ func containerOrderingDependenciesIsResolved(target *apicontainer.Container, dependsOnContainerKnownStatus == apicontainerstatus.ContainerStopped || dependsOnContainerKnownStatus == dependsOnContainer.GetSteadyStateStatus() - case "RUNNING": - + case runningCondition: if targetDesiredStatus == apicontainerstatus.ContainerCreated { // The 'target' container desires to be moved to 'Created' state. // Allow this only if the known status of the linked container is // 'Created' or if the dependency container is in 'steady state' return dependsOnContainerKnownStatus == apicontainerstatus.ContainerCreated || dependsOnContainer.IsKnownSteadyState() - } else if targetDesiredStatus == target.GetSteadyStateStatus() { // The 'target' container desires to be moved to its 'steady' state. // Allow this only if the dependency container is in 'steady state' as well return dependsOnContainer.IsKnownSteadyState() + } + return false + case successCondition: + // The 'target' container desires to be moved to 'Created' or the 'steady' state. + // Allow this only if the known status of the dependency container state is stopped with an exit code of 0 + if dependsOnContainer.GetKnownExitCode() != nil { + return dependsOnContainerKnownStatus == apicontainerstatus.ContainerStopped && + *dependsOnContainer.GetKnownExitCode() == successExitCode } return false + case completeCondition: + // The 'target' container desires to be moved to 'Created' or the 'steady' state. + // Allow this only if the known status of the dependency container state is stopped with any exit code + return dependsOnContainerKnownStatus == apicontainerstatus.ContainerStopped && dependsOnContainer.GetKnownExitCode() != nil + + case healthyCondition: + return dependsOnContainer.HealthStatusShouldBeReported() && + dependsOnContainer.GetHealthStatus().Status == apicontainerstatus.ContainerHealthy + default: return false } } +func verifyContainerOrderingStatus(dependsOnContainer *apicontainer.Container) bool { + dependsOnContainerDesiredStatus := dependsOnContainer.GetDesiredStatus() + // The 'target' container desires to be moved to 'Created' or the 'steady' state. + // Allow this only if the dependency container also desires to be started + // i.e it's status is any of 'Created', 'steady state' or 'Stopped' + return dependsOnContainerDesiredStatus == apicontainerstatus.ContainerCreated || + dependsOnContainerDesiredStatus == apicontainerstatus.ContainerStopped || + dependsOnContainerDesiredStatus == dependsOnContainer.GetSteadyStateStatus() +} + func onSteadyStateCanResolve(target *apicontainer.Container, run *apicontainer.Container) bool { return target.GetDesiredStatus() >= apicontainerstatus.ContainerCreated && run.GetDesiredStatus() >= run.GetSteadyStateStatus() diff --git a/agent/engine/dependencygraph/graph_test.go b/agent/engine/dependencygraph/graph_test.go index 1281070f53c..96a898e6b04 100644 --- a/agent/engine/dependencygraph/graph_test.go +++ b/agent/engine/dependencygraph/graph_test.go @@ -25,7 +25,7 @@ import ( "github.com/aws/amazon-ecs-agent/agent/taskresource" "github.com/aws/amazon-ecs-agent/agent/taskresource/mocks" resourcestatus "github.com/aws/amazon-ecs-agent/agent/taskresource/status" - + "github.com/aws/aws-sdk-go/aws" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" ) @@ -112,7 +112,6 @@ func TestValidDependenciesWithUnresolvedReference(t *testing.T) { assert.False(t, resolveable, "Nonexistent reference shouldn't resolve") } - func TestDependenciesAreResolvedWhenSteadyStateIsRunning(t *testing.T) { task := &apitask.Task{ Containers: []*apicontainer.Container{ @@ -188,7 +187,6 @@ func TestRunDependencies(t *testing.T) { assert.NoError(t, DependenciesAreResolved(c1, task.Containers, "", nil, nil), "Dependencies should be resolved") } - func TestRunDependenciesWhenSteadyStateIsResourcesProvisionedForOneContainer(t *testing.T) { // Webserver stack php := steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) @@ -568,194 +566,316 @@ func TestTransitionDependencyResourceNotFound(t *testing.T) { func TestContainerOrderingCanResolve(t *testing.T) { testcases := []struct { - TargetDesired apicontainerstatus.ContainerStatus - DependencyDesired apicontainerstatus.ContainerStatus + TargetDesired apicontainerstatus.ContainerStatus + DependencyDesired apicontainerstatus.ContainerStatus + DependencyKnown apicontainerstatus.ContainerStatus DependencyCondition string - Resolvable bool + ExitCode int + Resolvable bool }{ { - TargetDesired: apicontainerstatus.ContainerCreated, - DependencyDesired: apicontainerstatus.ContainerStatusNone, - DependencyCondition: "START", - Resolvable: false, + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyDesired: apicontainerstatus.ContainerStatusNone, + DependencyCondition: startCondition, + Resolvable: false, }, { - TargetDesired: apicontainerstatus.ContainerCreated, - DependencyDesired: apicontainerstatus.ContainerStopped, - DependencyCondition: "START", - Resolvable: true, + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyDesired: apicontainerstatus.ContainerStopped, + DependencyCondition: startCondition, + Resolvable: true, }, { - TargetDesired: apicontainerstatus.ContainerCreated, - DependencyDesired: apicontainerstatus.ContainerZombie, - DependencyCondition: "START", - Resolvable: false, + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyDesired: apicontainerstatus.ContainerZombie, + DependencyCondition: startCondition, + Resolvable: false, }, { - TargetDesired: apicontainerstatus.ContainerRunning, - DependencyDesired: apicontainerstatus.ContainerStatusNone, - DependencyCondition: "START", - Resolvable: false, + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerStatusNone, + DependencyCondition: startCondition, + Resolvable: false, }, { - TargetDesired: apicontainerstatus.ContainerRunning, - DependencyDesired: apicontainerstatus.ContainerCreated, - DependencyCondition: "START", - Resolvable: true, + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerCreated, + DependencyCondition: startCondition, + Resolvable: true, }, { - TargetDesired: apicontainerstatus.ContainerRunning, - DependencyDesired: apicontainerstatus.ContainerRunning, - DependencyCondition: "START", - Resolvable: true, + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerRunning, + DependencyCondition: startCondition, + Resolvable: true, }, { - TargetDesired: apicontainerstatus.ContainerRunning, - DependencyDesired: apicontainerstatus.ContainerStopped, - DependencyCondition: "START", - Resolvable: true, + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerStopped, + DependencyCondition: startCondition, + Resolvable: true, }, { - TargetDesired: apicontainerstatus.ContainerCreated, - DependencyDesired: apicontainerstatus.ContainerCreated, - DependencyCondition: "RUNNING", - Resolvable: true, + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyDesired: apicontainerstatus.ContainerCreated, + DependencyCondition: runningCondition, + Resolvable: true, }, { - TargetDesired: apicontainerstatus.ContainerRunning, - DependencyDesired: apicontainerstatus.ContainerRunning, - DependencyCondition: "RUNNING", - Resolvable: true, + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerRunning, + DependencyCondition: runningCondition, + Resolvable: true, }, { - TargetDesired: apicontainerstatus.ContainerCreated, - DependencyDesired: apicontainerstatus.ContainerRunning, - DependencyCondition: "RUNNING", - Resolvable: true, + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyDesired: apicontainerstatus.ContainerRunning, + DependencyCondition: runningCondition, + Resolvable: true, }, { - TargetDesired: apicontainerstatus.ContainerRunning, - DependencyDesired: apicontainerstatus.ContainerZombie, - DependencyCondition: "RUNNING", - Resolvable: false, + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerZombie, + DependencyCondition: runningCondition, + Resolvable: false, }, { - TargetDesired: apicontainerstatus.ContainerStatusNone, - DependencyDesired: apicontainerstatus.ContainerStopped, - DependencyCondition: "RUNNING", - Resolvable: false, + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerStopped, + DependencyCondition: successCondition, + Resolvable: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerRunning, + DependencyCondition: successCondition, + Resolvable: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerStopped, + ExitCode: 0, + DependencyCondition: successCondition, + Resolvable: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerStopped, + ExitCode: 1, + DependencyCondition: successCondition, + Resolvable: false, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerStopped, + DependencyCondition: completeCondition, + Resolvable: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyDesired: apicontainerstatus.ContainerRunning, + DependencyCondition: completeCondition, + Resolvable: true, }, } for _, tc := range testcases { t.Run(fmt.Sprintf("T:%s+V:%s", tc.TargetDesired.String(), tc.DependencyDesired.String()), - assertContainerOrderingCanResolve(containerOrderingDependenciesCanResolve, tc.TargetDesired, - tc.DependencyDesired, tc.DependencyCondition, tc.Resolvable)) + assertContainerOrderingCanResolve(containerOrderingDependenciesCanResolve, tc.TargetDesired, tc.DependencyDesired, tc.DependencyKnown, tc.DependencyCondition, tc.ExitCode, tc.Resolvable)) } } func TestContainerOrderingIsResolved(t *testing.T) { testcases := []struct { - TargetDesired apicontainerstatus.ContainerStatus - DependencyKnown apicontainerstatus.ContainerStatus + TargetDesired apicontainerstatus.ContainerStatus + DependencyKnown apicontainerstatus.ContainerStatus DependencyCondition string - Resolved bool + Resolved bool + ExitCode int }{ { - TargetDesired: apicontainerstatus.ContainerCreated, - DependencyKnown: apicontainerstatus.ContainerStatusNone, - DependencyCondition: "START", - Resolved: false, + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnown: apicontainerstatus.ContainerStatusNone, + DependencyCondition: startCondition, + Resolved: false, }, { - TargetDesired: apicontainerstatus.ContainerCreated, - DependencyKnown: apicontainerstatus.ContainerCreated, - DependencyCondition: "START", - Resolved: true, + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnown: apicontainerstatus.ContainerCreated, + DependencyCondition: startCondition, + Resolved: true, }, { - TargetDesired: apicontainerstatus.ContainerRunning, - DependencyKnown: apicontainerstatus.ContainerStopped, - DependencyCondition: "START", - Resolved: true, + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerStopped, + DependencyCondition: startCondition, + Resolved: true, }, { - TargetDesired: apicontainerstatus.ContainerCreated, - DependencyKnown: apicontainerstatus.ContainerStopped, - DependencyCondition: "START", - Resolved: true, + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnown: apicontainerstatus.ContainerStopped, + DependencyCondition: startCondition, + Resolved: true, }, { - TargetDesired: apicontainerstatus.ContainerRunning, - DependencyKnown: apicontainerstatus.ContainerStatusNone, - DependencyCondition: "START", - Resolved: false, + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerStatusNone, + DependencyCondition: startCondition, + Resolved: false, }, { - TargetDesired: apicontainerstatus.ContainerRunning, - DependencyKnown: apicontainerstatus.ContainerCreated, - DependencyCondition: "START", - Resolved: true, + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerCreated, + DependencyCondition: startCondition, + Resolved: true, }, { - TargetDesired: apicontainerstatus.ContainerCreated, - DependencyKnown: apicontainerstatus.ContainerCreated, - DependencyCondition: "RUNNING", - Resolved: true, + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnown: apicontainerstatus.ContainerCreated, + DependencyCondition: runningCondition, + Resolved: true, }, { - TargetDesired: apicontainerstatus.ContainerCreated, - DependencyKnown: apicontainerstatus.ContainerRunning, - DependencyCondition: "RUNNING", - Resolved: true, + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnown: apicontainerstatus.ContainerRunning, + DependencyCondition: runningCondition, + Resolved: true, }, { - TargetDesired: apicontainerstatus.ContainerCreated, - DependencyKnown: apicontainerstatus.ContainerZombie, - DependencyCondition: "RUNNING", - Resolved: false, + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnown: apicontainerstatus.ContainerZombie, + DependencyCondition: runningCondition, + Resolved: false, }, { - TargetDesired: apicontainerstatus.ContainerRunning, - DependencyKnown: apicontainerstatus.ContainerRunning, - DependencyCondition: "RUNNING", - Resolved: true, + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerRunning, + DependencyCondition: runningCondition, + Resolved: true, }, { - TargetDesired: apicontainerstatus.ContainerRunning, - DependencyKnown: apicontainerstatus.ContainerZombie, - DependencyCondition: "RUNNING", - Resolved: false, + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerZombie, + DependencyCondition: runningCondition, + Resolved: false, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerStopped, + DependencyCondition: successCondition, + ExitCode: 0, + Resolved: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerStopped, + DependencyCondition: successCondition, + ExitCode: 1, + Resolved: false, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerStopped, + DependencyCondition: completeCondition, + ExitCode: 0, + Resolved: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerStopped, + DependencyCondition: completeCondition, + ExitCode: 1, + Resolved: true, + }, + { + TargetDesired: apicontainerstatus.ContainerRunning, + DependencyKnown: apicontainerstatus.ContainerRunning, + DependencyCondition: completeCondition, + Resolved: false, }, } for _, tc := range testcases { t.Run(fmt.Sprintf("T:%s+V:%s", tc.TargetDesired.String(), tc.DependencyKnown.String()), - assertContainerOrderingResolved(containerOrderingDependenciesIsResolved, tc.TargetDesired, tc.DependencyKnown, tc.DependencyCondition, tc.Resolved)) + assertContainerOrderingResolved(containerOrderingDependenciesIsResolved, tc.TargetDesired, tc.DependencyKnown, tc.DependencyCondition, tc.ExitCode, tc.Resolved)) } } -func assertContainerOrderingCanResolve(f func(target *apicontainer.Container, dep *apicontainer.Container, depCond string) bool, targetDesired, depKnown apicontainerstatus.ContainerStatus, depCond string, expectedResolvable bool) func(t *testing.T) { +func assertContainerOrderingCanResolve(f func(target *apicontainer.Container, dep *apicontainer.Container, depCond string) bool, targetDesired, depDesired, depKnown apicontainerstatus.ContainerStatus, depCond string, exitcode int, expectedResolvable bool) func(t *testing.T) { return func(t *testing.T) { target := &apicontainer.Container{ DesiredStatusUnsafe: targetDesired, } dep := &apicontainer.Container{ - DesiredStatusUnsafe: depKnown, + DesiredStatusUnsafe: depDesired, + KnownStatusUnsafe: depKnown, + KnownExitCodeUnsafe: aws.Int(exitcode), } resolvable := f(target, dep, depCond) assert.Equal(t, expectedResolvable, resolvable) } } -func assertContainerOrderingResolved(f func(target *apicontainer.Container, dep *apicontainer.Container, depCond string) bool, targetDesired, depKnown apicontainerstatus.ContainerStatus, depCond string, expectedResolved bool) func(t *testing.T) { +func assertContainerOrderingResolved(f func(target *apicontainer.Container, dep *apicontainer.Container, depCond string) bool, targetDesired, depKnown apicontainerstatus.ContainerStatus, depCond string, exitcode int, expectedResolved bool) func(t *testing.T) { return func(t *testing.T) { target := &apicontainer.Container{ DesiredStatusUnsafe: targetDesired, } dep := &apicontainer.Container{ - KnownStatusUnsafe: depKnown, + KnownStatusUnsafe: depKnown, + KnownExitCodeUnsafe: aws.Int(exitcode), } resolved := f(target, dep, depCond) assert.Equal(t, expectedResolved, resolved) } } +func TestContainerOrderingHealthyConditionIsResolved(t *testing.T) { + testcases := []struct { + TargetDesired apicontainerstatus.ContainerStatus + DependencyKnownHealthStatus apicontainerstatus.ContainerHealthStatus + HealthCheckType string + DependencyKnownHealthExitCode int + DependencyCondition string + Resolved bool + }{ + { + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnownHealthStatus: apicontainerstatus.ContainerHealthy, + HealthCheckType: "docker", + DependencyCondition: healthyCondition, + Resolved: true, + }, + { + TargetDesired: apicontainerstatus.ContainerCreated, + DependencyKnownHealthStatus: apicontainerstatus.ContainerUnhealthy, + HealthCheckType: "docker", + DependencyKnownHealthExitCode: 1, + DependencyCondition: healthyCondition, + Resolved: false, + }, + { + TargetDesired: apicontainerstatus.ContainerCreated, + Resolved: false, + }, + } + for _, tc := range testcases { + t.Run(fmt.Sprintf("T:%s+V:%s", tc.TargetDesired.String(), tc.DependencyKnownHealthStatus.String()), + assertContainerOrderingHealthyConditionResolved(containerOrderingDependenciesIsResolved, tc.TargetDesired, tc.DependencyKnownHealthStatus, tc.HealthCheckType, tc.DependencyKnownHealthExitCode, tc.DependencyCondition, tc.Resolved)) + } +} + +func assertContainerOrderingHealthyConditionResolved(f func(target *apicontainer.Container, dep *apicontainer.Container, depCond string) bool, targetDesired apicontainerstatus.ContainerStatus, depHealthKnown apicontainerstatus.ContainerHealthStatus, healthCheckEnabled string, depHealthKnownExitCode int, depCond string, expectedResolved bool) func(t *testing.T) { + return func(t *testing.T) { + target := &apicontainer.Container{ + DesiredStatusUnsafe: targetDesired, + } + dep := &apicontainer.Container{ + Health: apicontainer.HealthStatus{ + Status: depHealthKnown, + ExitCode: depHealthKnownExitCode, + }, + HealthCheckType: healthCheckEnabled, + } + resolved := f(target, dep, depCond) + assert.Equal(t, expectedResolved, resolved) + } +} From 4c57056ec0a05bd9e6a3c6648fb71359e7a77873 Mon Sep 17 00:00:00 2001 From: Derek Petersen Date: Thu, 21 Feb 2019 17:36:25 -0800 Subject: [PATCH 07/24] engine: adding poll function during progressTask Prior to this commit, we only tracked state we explicitly tried to change when the task was starting. We did not respond to the event stream or any other source of information from Docker. This means that when we are waiting for certain dependency conditions ("SUCCESS", "COMPLETE", or "HEALTHY") the task progression logic does not update the agent-internal model of container state. Since we rely on that state for determining when conditions are met, tasks would get stuck in infinite startup loops. This change adds a call to engine.checkTaskState(), which explicity updates any changed container state. We only call this function if we know that we are waiting on the aforementioned subset of dependency conditions. Co-authored-by: Utsa Bhattacharjya --- agent/engine/dependencygraph/graph.go | 26 ++++---- agent/engine/dependencygraph/graph_test.go | 36 ++++++----- agent/engine/docker_image_manager_test.go | 64 ++++++++++---------- agent/engine/task_manager.go | 69 +++++++++++++++------- agent/engine/task_manager_test.go | 12 ++-- agent/engine/task_manager_unix_test.go | 2 +- 6 files changed, 120 insertions(+), 89 deletions(-) diff --git a/agent/engine/dependencygraph/graph.go b/agent/engine/dependencygraph/graph.go index 29f8fb0c70f..4cf56f19195 100644 --- a/agent/engine/dependencygraph/graph.go +++ b/agent/engine/dependencygraph/graph.go @@ -105,7 +105,7 @@ func dependenciesCanBeResolved(target *apicontainer.Container, by []*apicontaine nameMap[cont.Name] = cont } - if err := verifyContainerOrderingStatusResolvable(target, nameMap, containerOrderingDependenciesCanResolve); err != nil { + if _, err := verifyContainerOrderingStatusResolvable(target, nameMap, containerOrderingDependenciesCanResolve); err != nil { return false } return verifyStatusResolvable(target, nameMap, target.SteadyStateDependencies, onSteadyStateCanResolve) @@ -122,9 +122,9 @@ func DependenciesAreResolved(target *apicontainer.Container, by []*apicontainer.Container, id string, manager credentials.Manager, - resources []taskresource.TaskResource) error { + resources []taskresource.TaskResource) (*apicontainer.DependsOn, error) { if !executionCredentialsResolved(target, id, manager) { - return CredentialsNotResolvedErr + return nil, CredentialsNotResolvedErr } nameMap := make(map[string]*apicontainer.Container) @@ -141,18 +141,18 @@ func DependenciesAreResolved(target *apicontainer.Container, resourcesMap[resource.GetName()] = resource } - if err := verifyContainerOrderingStatusResolvable(target, nameMap, containerOrderingDependenciesIsResolved); err != nil { - return err + if blocked, err := verifyContainerOrderingStatusResolvable(target, nameMap, containerOrderingDependenciesIsResolved); err != nil { + return blocked, err } if !verifyStatusResolvable(target, nameMap, target.SteadyStateDependencies, onSteadyStateIsResolved) { - return DependentContainerNotResolvedErr + return nil, DependentContainerNotResolvedErr } if err := verifyTransitionDependenciesResolved(target, nameMap, resourcesMap); err != nil { - return err + return nil, err } - return nil + return nil, nil } func linksToContainerNames(links []string) []string { @@ -205,25 +205,25 @@ func verifyStatusResolvable(target *apicontainer.Container, existingContainers m // (map from name to container). The `resolves` function passed should return true if the named container is resolved. func verifyContainerOrderingStatusResolvable(target *apicontainer.Container, existingContainers map[string]*apicontainer.Container, - resolves func(*apicontainer.Container, *apicontainer.Container, string) bool) error { + resolves func(*apicontainer.Container, *apicontainer.Container, string) bool) (*apicontainer.DependsOn, error) { targetGoal := target.GetDesiredStatus() if targetGoal != target.GetSteadyStateStatus() && targetGoal != apicontainerstatus.ContainerCreated { // A container can always stop, die, or reach whatever other state it // wants regardless of what dependencies it has - return nil + return nil, nil } for _, dependency := range target.DependsOn { dependencyContainer, ok := existingContainers[dependency.Container] if !ok { - return fmt.Errorf("dependency graph: container ordering dependency [%v] for target [%v] does not exist.", dependencyContainer, target) + return nil, fmt.Errorf("dependency graph: container ordering dependency [%v] for target [%v] does not exist.", dependencyContainer, target) } if !resolves(target, dependencyContainer, dependency.Condition) { - return fmt.Errorf("dependency graph: failed to resolve the container ordering dependency [%v] for target [%v]", dependencyContainer, target) + return &dependency, fmt.Errorf("dependency graph: failed to resolve the container ordering dependency [%v] for target [%v]", dependencyContainer, target) } } - return nil + return nil, nil } func verifyTransitionDependenciesResolved(target *apicontainer.Container, diff --git a/agent/engine/dependencygraph/graph_test.go b/agent/engine/dependencygraph/graph_test.go index 96a898e6b04..d8b2e798404 100644 --- a/agent/engine/dependencygraph/graph_test.go +++ b/agent/engine/dependencygraph/graph_test.go @@ -121,7 +121,7 @@ func TestDependenciesAreResolvedWhenSteadyStateIsRunning(t *testing.T) { }, }, } - err := DependenciesAreResolved(task.Containers[0], task.Containers, "", nil, nil) + _, err := DependenciesAreResolved(task.Containers[0], task.Containers, "", nil, nil) assert.NoError(t, err, "One container should resolve trivially") // Webserver stack @@ -138,28 +138,28 @@ func TestDependenciesAreResolvedWhenSteadyStateIsRunning(t *testing.T) { }, } - err = DependenciesAreResolved(php, task.Containers, "", nil, nil) + _, err = DependenciesAreResolved(php, task.Containers, "", nil, nil) assert.Error(t, err, "Shouldn't be resolved; db isn't running") - err = DependenciesAreResolved(db, task.Containers, "", nil, nil) + _, err = DependenciesAreResolved(db, task.Containers, "", nil, nil) assert.Error(t, err, "Shouldn't be resolved; dbdatavolume isn't created") - err = DependenciesAreResolved(dbdata, task.Containers, "", nil, nil) + _, err = DependenciesAreResolved(dbdata, task.Containers, "", nil, nil) assert.NoError(t, err, "data volume with no deps should resolve") dbdata.SetKnownStatus(apicontainerstatus.ContainerCreated) - err = DependenciesAreResolved(php, task.Containers, "", nil, nil) + _, err = DependenciesAreResolved(php, task.Containers, "", nil, nil) assert.Error(t, err, "Php shouldn't run, db is not created") db.SetKnownStatus(apicontainerstatus.ContainerCreated) - err = DependenciesAreResolved(php, task.Containers, "", nil, nil) + _, err = DependenciesAreResolved(php, task.Containers, "", nil, nil) assert.Error(t, err, "Php shouldn't run, db is not running") - err = DependenciesAreResolved(db, task.Containers, "", nil, nil) + _, err = DependenciesAreResolved(db, task.Containers, "", nil, nil) assert.NoError(t, err, "db should be resolved, dbdata volume is Created") db.SetKnownStatus(apicontainerstatus.ContainerRunning) - err = DependenciesAreResolved(php, task.Containers, "", nil, nil) + _, err = DependenciesAreResolved(php, task.Containers, "", nil, nil) assert.NoError(t, err, "Php should resolve") } @@ -175,16 +175,20 @@ func TestRunDependencies(t *testing.T) { SteadyStateDependencies: []string{"a"}, } task := &apitask.Task{Containers: []*apicontainer.Container{c1, c2}} + _, err := DependenciesAreResolved(c2, task.Containers, "", nil, nil) + assert.Error(t, err, "Dependencies should not be resolved") - assert.Error(t, DependenciesAreResolved(c2, task.Containers, "", nil, nil), "Dependencies should not be resolved") task.Containers[1].SetDesiredStatus(apicontainerstatus.ContainerRunning) - assert.Error(t, DependenciesAreResolved(c2, task.Containers, "", nil, nil), "Dependencies should not be resolved") + _, err = DependenciesAreResolved(c2, task.Containers, "", nil, nil) + assert.Error(t, err, "Dependencies should not be resolved") task.Containers[0].SetKnownStatus(apicontainerstatus.ContainerRunning) - assert.NoError(t, DependenciesAreResolved(c2, task.Containers, "", nil, nil), "Dependencies should be resolved") + _, err = DependenciesAreResolved(c2, task.Containers, "", nil, nil) + assert.NoError(t, err, "Dependencies should be resolved") task.Containers[1].SetDesiredStatus(apicontainerstatus.ContainerCreated) - assert.NoError(t, DependenciesAreResolved(c1, task.Containers, "", nil, nil), "Dependencies should be resolved") + _, err = DependenciesAreResolved(c1, task.Containers, "", nil, nil) + assert.NoError(t, err, "Dependencies should be resolved") } func TestRunDependenciesWhenSteadyStateIsResourcesProvisionedForOneContainer(t *testing.T) { @@ -210,11 +214,11 @@ func TestRunDependenciesWhenSteadyStateIsResourcesProvisionedForOneContainer(t * continue } container.SteadyStateDependencies = []string{"pause"} - err := DependenciesAreResolved(container, task.Containers, "", nil, nil) + _, err := DependenciesAreResolved(container, task.Containers, "", nil, nil) assert.Error(t, err, "Shouldn't be resolved; pause isn't running") } - err := DependenciesAreResolved(pause, task.Containers, "", nil, nil) + _, err := DependenciesAreResolved(pause, task.Containers, "", nil, nil) assert.NoError(t, err, "Pause container's dependencies should be resolved") // Transition pause container to RUNNING @@ -228,13 +232,13 @@ func TestRunDependenciesWhenSteadyStateIsResourcesProvisionedForOneContainer(t * } // Assert that dependencies remain unresolved until the pause container reaches // RESOURCES_PROVISIONED - err = DependenciesAreResolved(container, task.Containers, "", nil, nil) + _, err = DependenciesAreResolved(container, task.Containers, "", nil, nil) assert.Error(t, err, "Shouldn't be resolved; pause isn't running") } pause.KnownStatusUnsafe = apicontainerstatus.ContainerResourcesProvisioned // Dependecies should be resolved now that the 'pause' container has // transitioned into RESOURCES_PROVISIONED - err = DependenciesAreResolved(php, task.Containers, "", nil, nil) + _, err = DependenciesAreResolved(php, task.Containers, "", nil, nil) assert.NoError(t, err, "Php should resolve") } diff --git a/agent/engine/docker_image_manager_test.go b/agent/engine/docker_image_manager_test.go index 4c1be98587e..05992c47f3f 100644 --- a/agent/engine/docker_image_manager_test.go +++ b/agent/engine/docker_image_manager_test.go @@ -145,8 +145,8 @@ func TestRecordContainerReferenceInspectError(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -179,8 +179,8 @@ func TestRecordContainerReferenceWithNoImageName(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -406,8 +406,8 @@ func TestRemoveContainerReferenceFromImageStateWithNoReference(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -442,8 +442,8 @@ func TestGetCandidateImagesForDeletionImageNoImageState(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -462,8 +462,8 @@ func TestGetCandidateImagesForDeletionImageJustPulled(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -487,8 +487,8 @@ func TestGetCandidateImagesForDeletionImageHasContainerReference(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -528,8 +528,8 @@ func TestGetCandidateImagesForDeletionImageHasMoreContainerReferences(t *testing client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -606,8 +606,8 @@ func TestImageCleanupExclusionListWithSingleName(t *testing.T) { PulledAt: time.Now().AddDate(0, -2, 0), } imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -665,8 +665,8 @@ func TestImageCleanupExclusionListWithMultipleNames(t *testing.T) { PulledAt: time.Now().AddDate(0, -2, 0), } imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -731,8 +731,8 @@ func TestGetLeastRecentlyUsedImagesLessThanFive(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -765,8 +765,8 @@ func TestRemoveAlreadyExistingImageNameWithDifferentID(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -816,8 +816,8 @@ func TestImageCleanupHappyPath(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: 1 * time.Millisecond, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -873,8 +873,8 @@ func TestImageCleanupCannotRemoveImage(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -931,8 +931,8 @@ func TestImageCleanupRemoveImageById(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -984,8 +984,8 @@ func TestNonECSImageAndContainersCleanupRemoveImage(t *testing.T) { defer ctrl.Finish() client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, @@ -1220,8 +1220,8 @@ func TestConcurrentRemoveUnusedImages(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) imageManager := &dockerImageManager{ - client: client, - state: dockerstate.NewTaskEngineState(), + client: client, + state: dockerstate.NewTaskEngineState(), minimumAgeBeforeDeletion: config.DefaultImageDeletionAge, numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, diff --git a/agent/engine/task_manager.go b/agent/engine/task_manager.go index 81e6ebfc0b4..bfcf50b6020 100644 --- a/agent/engine/task_manager.go +++ b/agent/engine/task_manager.go @@ -47,6 +47,7 @@ const ( // and start processing the task or start another round of waiting waitForPullCredentialsTimeout = 1 * time.Minute defaultTaskSteadyStatePollInterval = 5 * time.Minute + transitionPollTime = 5 * time.Second stoppedSentWaitInterval = 30 * time.Second maxStoppedWaitTimes = 72 * time.Hour / stoppedSentWaitInterval taskUnableToTransitionToStoppedReason = "TaskStateError: Agent could not progress task's state to stopped" @@ -83,6 +84,7 @@ type acsTransition struct { type containerTransition struct { nextState apicontainerstatus.ContainerStatus actionRequired bool + blockedOn *apicontainer.DependsOn reason error } @@ -163,21 +165,21 @@ type managedTask struct { func (engine *DockerTaskEngine) newManagedTask(task *apitask.Task) *managedTask { ctx, cancel := context.WithCancel(engine.ctx) t := &managedTask{ - ctx: ctx, - cancel: cancel, - Task: task, - acsMessages: make(chan acsTransition), - dockerMessages: make(chan dockerContainerChange), - resourceStateChangeEvent: make(chan resourceStateChange), + ctx: ctx, + cancel: cancel, + Task: task, + acsMessages: make(chan acsTransition), + dockerMessages: make(chan dockerContainerChange), + resourceStateChangeEvent: make(chan resourceStateChange), engine: engine, cfg: engine.cfg, stateChangeEvents: engine.stateChangeEvents, containerChangeEventStream: engine.containerChangeEventStream, - saver: engine.saver, - credentialsManager: engine.credentialsManager, - cniClient: engine.cniClient, - taskStopWG: engine.taskStopGroup, - steadyStatePollInterval: engine.taskSteadyStatePollInterval, + saver: engine.saver, + credentialsManager: engine.credentialsManager, + cniClient: engine.cniClient, + taskStopWG: engine.taskStopGroup, + steadyStatePollInterval: engine.taskSteadyStatePollInterval, } engine.managedTasks[task.Arn] = t return t @@ -740,16 +742,36 @@ func (mtask *managedTask) progressTask() { transitionChangeEntity <- resource.GetName() }) - anyContainerTransition, contTransitions, reasons := mtask.startContainerTransitions( + anyContainerTransition, blockedDependencies, contTransitions, reasons := mtask.startContainerTransitions( func(container *apicontainer.Container, nextStatus apicontainerstatus.ContainerStatus) { mtask.engine.transitionContainer(mtask.Task, container, nextStatus) transitionChange <- struct{}{} transitionChangeEntity <- container.Name }) - if !anyContainerTransition && !anyResourceTransition { - if !mtask.waitForExecutionCredentialsFromACS(reasons) { - mtask.onContainersUnableToTransitionState() + atLeastOneTransitionStarted := anyResourceTransition || anyContainerTransition + + blockedByOrderingDependencies := len(blockedDependencies) > 0 + + // If no transitions happened and we aren't blocked by ordering dependencies, then we are possibly in a state where + // its impossible for containers to move forward. We will do an additional check to see if we are waiting for ACS + // execution credentials. If not, then we will abort the task progression. + if !atLeastOneTransitionStarted && !blockedByOrderingDependencies { + if !mtask.isWaitingForACSExecutionCredentials(reasons) { + mtask.handleContainersUnableToTransitionState() + } + return + } + + // If no containers are starting and we are blocked on ordering dependencies, we should watch for the task to change + // over time. This will update the containers if they become healthy or stop, which makes it possible for the + // conditions "HEALTHY" and "SUCCESS" to succeed. + if !atLeastOneTransitionStarted && blockedByOrderingDependencies { + go mtask.engine.checkTaskState(mtask.Task) + ctx, cancel := context.WithTimeout(context.Background(), transitionPollTime) + defer cancel() + for timeout := mtask.waitEvent(ctx.Done()); !timeout; { + timeout = mtask.waitEvent(ctx.Done()) } return } @@ -781,9 +803,9 @@ func (mtask *managedTask) progressTask() { } } -// waitForExecutionCredentialsFromACS checks if the container that can't be transitioned +// isWaitingForACSExecutionCredentials checks if the container that can't be transitioned // was caused by waiting for credentials and start waiting -func (mtask *managedTask) waitForExecutionCredentialsFromACS(reasons []error) bool { +func (mtask *managedTask) isWaitingForACSExecutionCredentials(reasons []error) bool { for _, reason := range reasons { if reason == dependencygraph.CredentialsNotResolvedErr { seelog.Debugf("Managed task [%s]: waiting for credentials to pull from ECR", mtask.Arn) @@ -803,15 +825,19 @@ func (mtask *managedTask) waitForExecutionCredentialsFromACS(reasons []error) bo // startContainerTransitions steps through each container in the task and calls // the passed transition function when a transition should occur. -func (mtask *managedTask) startContainerTransitions(transitionFunc containerTransitionFunc) (bool, map[string]apicontainerstatus.ContainerStatus, []error) { +func (mtask *managedTask) startContainerTransitions(transitionFunc containerTransitionFunc) (bool, map[string]apicontainer.DependsOn, map[string]apicontainerstatus.ContainerStatus, []error) { anyCanTransition := false var reasons []error + blocked := make(map[string]apicontainer.DependsOn) transitions := make(map[string]apicontainerstatus.ContainerStatus) for _, cont := range mtask.Containers { transition := mtask.containerNextState(cont) if transition.reason != nil { // container can't be transitioned reasons = append(reasons, transition.reason) + if transition.blockedOn != nil { + blocked[cont.Name] = *transition.blockedOn + } continue } @@ -844,7 +870,7 @@ func (mtask *managedTask) startContainerTransitions(transitionFunc containerTran go transitionFunc(cont, transition.nextState) } - return anyCanTransition, transitions, reasons + return anyCanTransition, blocked, transitions, reasons } // startResourceTransitions steps through each resource in the task and calls @@ -956,7 +982,7 @@ func (mtask *managedTask) containerNextState(container *apicontainer.Container) reason: dependencygraph.ContainerPastDesiredStatusErr, } } - if err := dependencygraph.DependenciesAreResolved(container, mtask.Containers, + if blocked, err := dependencygraph.DependenciesAreResolved(container, mtask.Containers, mtask.Task.GetExecutionCredentialsID(), mtask.credentialsManager, mtask.GetResources()); err != nil { seelog.Debugf("Managed task [%s]: can't apply state to container [%s] yet due to unresolved dependencies: %v", mtask.Arn, container.Name, err) @@ -964,6 +990,7 @@ func (mtask *managedTask) containerNextState(container *apicontainer.Container) nextState: apicontainerstatus.ContainerStatusNone, actionRequired: false, reason: err, + blockedOn: blocked, } } @@ -1018,7 +1045,7 @@ func (mtask *managedTask) resourceNextState(resource taskresource.TaskResource) } } -func (mtask *managedTask) onContainersUnableToTransitionState() { +func (mtask *managedTask) handleContainersUnableToTransitionState() { seelog.Criticalf("Managed task [%s]: task in a bad state; it's not steadystate but no containers want to transition", mtask.Arn) if mtask.GetDesiredStatus().Terminal() { diff --git a/agent/engine/task_manager_test.go b/agent/engine/task_manager_test.go index 0f40d42b1e2..f7acc4eca9d 100644 --- a/agent/engine/task_manager_test.go +++ b/agent/engine/task_manager_test.go @@ -635,7 +635,7 @@ func TestStartContainerTransitionsWhenForwardTransitionPossible(t *testing.T) { } waitForAssertions.Add(2) - canTransition, transitions, _ := task.startContainerTransitions( + canTransition, _, transitions, _ := task.startContainerTransitions( func(cont *apicontainer.Container, nextStatus apicontainerstatus.ContainerStatus) { if cont.Name == firstContainerName { assert.Equal(t, nextStatus, apicontainerstatus.ContainerRunning, "Mismatch for first container next status") @@ -697,7 +697,7 @@ func TestStartContainerTransitionsWhenForwardTransitionIsNotPossible(t *testing. engine: &DockerTaskEngine{}, } - canTransition, transitions, _ := task.startContainerTransitions( + canTransition, _, transitions, _ := task.startContainerTransitions( func(cont *apicontainer.Container, nextStatus apicontainerstatus.ContainerStatus) { t.Error("Transition function should not be called when no transitions are possible") }) @@ -765,7 +765,7 @@ func TestStartContainerTransitionsInvokesHandleContainerChange(t *testing.T) { }() go task.waitEvent(nil) - canTransition, transitions, _ := task.startContainerTransitions( + canTransition, _, transitions, _ := task.startContainerTransitions( func(cont *apicontainer.Container, nextStatus apicontainerstatus.ContainerStatus) { t.Error("Invalid code path. The transition function should not be invoked when transitioning container from CREATED -> STOPPED") }) @@ -875,7 +875,7 @@ func TestOnContainersUnableToTransitionStateForDesiredStoppedTask(t *testing.T) eventsGenerated.Done() }() - task.onContainersUnableToTransitionState() + task.handleContainersUnableToTransitionState() eventsGenerated.Wait() assert.Equal(t, task.GetDesiredStatus(), apitaskstatus.TaskStopped) @@ -897,7 +897,7 @@ func TestOnContainersUnableToTransitionStateForDesiredRunningTask(t *testing.T) }, } - task.onContainersUnableToTransitionState() + task.handleContainersUnableToTransitionState() assert.Equal(t, task.GetDesiredStatus(), apitaskstatus.TaskStopped) assert.Equal(t, task.Containers[0].GetDesiredStatus(), apicontainerstatus.ContainerStopped) } @@ -1314,7 +1314,7 @@ func TestTaskWaitForExecutionCredentials(t *testing.T) { go func() { task.acsMessages <- acsTransition{desiredStatus: apitaskstatus.TaskRunning} }() } - assert.Equal(t, tc.result, task.waitForExecutionCredentialsFromACS(tc.errs), tc.msg) + assert.Equal(t, tc.result, task.isWaitingForACSExecutionCredentials(tc.errs), tc.msg) }) } } diff --git a/agent/engine/task_manager_unix_test.go b/agent/engine/task_manager_unix_test.go index baae542e517..b025f09ccc1 100644 --- a/agent/engine/task_manager_unix_test.go +++ b/agent/engine/task_manager_unix_test.go @@ -263,7 +263,7 @@ func TestStartResourceTransitionsEmpty(t *testing.T) { ResourcesMapUnsafe: make(map[string][]taskresource.TaskResource), DesiredStatusUnsafe: apitaskstatus.TaskRunning, }, - ctx: ctx, + ctx: ctx, resourceStateChangeEvent: make(chan resourceStateChange), } mtask.Task.AddResource("cgroup", res) From 6f89b0e61efbf840affa7d7d11c8c523e5bf9946 Mon Sep 17 00:00:00 2001 From: Derek Petersen Date: Mon, 18 Feb 2019 14:44:00 -0800 Subject: [PATCH 08/24] dependencygraph: Enforce shutdown order We now apply shutdown order in any dependency case, including dependsOn directives, links, or volumes. What this means is that agent will now make a best effort attempt to shutdown containers in the inverse order they were created. For example, a container using a link for communication will wait until the linked container has terminated before terminating itself. Likewise, a container named in another container's dependsOn directive will wait until that dependent container terminates. One note about the current implementation is that the dependencies aren't assumed to be transitive, so if a chain exists such that: A -> B -> C Container "C" will shutdown before "B", but it won't check status against container "A" explicity. If A depends on C, we expect: A -> B -> C A -> C The lack of transitive dependency logic applies is consistent with startup order as of this commit. --- agent/engine/dependencygraph/graph.go | 42 ++++++++--- agent/engine/dependencygraph/graph_test.go | 82 ++++++++++++++++++++++ 2 files changed, 116 insertions(+), 8 deletions(-) diff --git a/agent/engine/dependencygraph/graph.go b/agent/engine/dependencygraph/graph.go index 4cf56f19195..2c7b6b0d767 100644 --- a/agent/engine/dependencygraph/graph.go +++ b/agent/engine/dependencygraph/graph.go @@ -57,16 +57,9 @@ var ( ErrResourceDependencyNotResolved = errors.New("dependency graph: dependency on resources not resolved") ) -// Because a container may depend on another container being created -// (volumes-from) or running (links) it makes sense to abstract it out -// to each container having dependencies on another container being in any -// particular state set. For now, these are resolved here and support only -// volume/link (created/run) - // ValidDependencies takes a task and verifies that it is possible to allow all // containers within it to reach the desired status by proceeding in some -// order. ValidDependencies is called during DockerTaskEngine.AddTask to -// verify that a startup order can exist. +// order. func ValidDependencies(task *apitask.Task) bool { unresolved := make([]*apicontainer.Container, len(task.Containers)) resolved := make([]*apicontainer.Container, 0, len(task.Containers)) @@ -152,6 +145,14 @@ func DependenciesAreResolved(target *apicontainer.Container, return nil, err } + // If the target is desired terminal and isn't stopped, we should validate that it doesn't have any containers + // that are dependent on it that need to shut down first. + if target.DesiredTerminal() && !target.KnownTerminal() { + if err := verifyShutdownOrder(target, nameMap); err != nil { + return nil, err + } + } + return nil, nil } @@ -377,6 +378,31 @@ func verifyContainerOrderingStatus(dependsOnContainer *apicontainer.Container) b dependsOnContainerDesiredStatus == dependsOnContainer.GetSteadyStateStatus() } +func verifyShutdownOrder(target *apicontainer.Container, existingContainers map[string]*apicontainer.Container) error { + // We considered adding this to the task state, but this will be at most 45 loops, + // so we err'd on the side of having less state. + missingShutdownDependencies := []string{} + + for _, existingContainer := range existingContainers { + for _, dependency := range existingContainer.DependsOn { + // If another container declares a dependency on our target, we will want to verify that the container is + // stopped. + if dependency.Container == target.Name { + if !existingContainer.KnownTerminal() { + missingShutdownDependencies = append(missingShutdownDependencies, existingContainer.Name) + } + } + } + } + + if len(missingShutdownDependencies) == 0 { + return nil + } + + return fmt.Errorf("dependency graph: target %s needs other containers stopped before it can stop: [%s]", + target.Name, strings.Join(missingShutdownDependencies, "], [")) +} + func onSteadyStateCanResolve(target *apicontainer.Container, run *apicontainer.Container) bool { return target.GetDesiredStatus() >= apicontainerstatus.ContainerCreated && run.GetDesiredStatus() >= run.GetSteadyStateStatus() diff --git a/agent/engine/dependencygraph/graph_test.go b/agent/engine/dependencygraph/graph_test.go index d8b2e798404..92d7c2e0b69 100644 --- a/agent/engine/dependencygraph/graph_test.go +++ b/agent/engine/dependencygraph/graph_test.go @@ -883,3 +883,85 @@ func assertContainerOrderingHealthyConditionResolved(f func(target *apicontainer assert.Equal(t, expectedResolved, resolved) } } + +func dependsOn(vals ...string) []apicontainer.DependsOn { + d := make([]apicontainer.DependsOn, len(vals)) + for i, val := range vals { + d[i] = apicontainer.DependsOn{Container: val} + } + return d +} + +// TestVerifyShutdownOrder validates that the shutdown graph works in the inverse order of the startup graph +// This test always uses a running target +func TestVerifyShutdownOrder(t *testing.T) { + + // dependencies aren't transitive, so we can check multiple levels within here + others := map[string]*apicontainer.Container{ + "A": { + Name: "A", + DependsOn: dependsOn("B"), + KnownStatusUnsafe: apicontainerstatus.ContainerStopped, + }, + "B": { + Name: "B", + DependsOn: dependsOn("C", "D"), + KnownStatusUnsafe: apicontainerstatus.ContainerRunning, + }, + "C": { + Name: "C", + DependsOn: dependsOn("E", "F"), + KnownStatusUnsafe: apicontainerstatus.ContainerStopped, + }, + "D": { + Name: "D", + DependsOn: dependsOn("E"), + KnownStatusUnsafe: apicontainerstatus.ContainerRunning, + }, + } + + testCases := []struct { + TestName string + TargetName string + ShouldResolve bool + }{ + { + TestName: "Dependency is already stopped", + TargetName: "F", + ShouldResolve: true, + }, + { + TestName: "Running with a running dependency", + TargetName: "D", + ShouldResolve: false, + }, + { + TestName: "One dependency running, one stopped", + TargetName: "E", + ShouldResolve: false, + }, + { + TestName: "No dependencies declared", + TargetName: "Z", + ShouldResolve: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.TestName, func(t *testing.T) { + // create a target container + target := &apicontainer.Container{ + Name: tc.TargetName, + KnownStatusUnsafe: apicontainerstatus.ContainerRunning, + DesiredStatusUnsafe: apicontainerstatus.ContainerStopped, + } + + // Validation + if tc.ShouldResolve { + assert.NoError(t, verifyShutdownOrder(target, others)) + } else { + assert.Error(t, verifyShutdownOrder(target, others)) + } + }) + } +} From fa040f41ad3eec78cab59661d3cab203678e36f8 Mon Sep 17 00:00:00 2001 From: Derek Petersen Date: Tue, 19 Feb 2019 00:01:07 -0800 Subject: [PATCH 09/24] functional tests: increase timeout on link/volume The link / volume dependency tests are now affected by shutdown order, so the tests now take longer. Previously, it would take a max of 30s (the default docker stop timeout for agent). Now, since the containers stop in order, it will take a max of 30s * n, where n is the number of containers. Increasing the test timeout is a short term fix until we have granular start/stop timeouts plumbed through the ecs service. --- .../testdata/simpletests_unix/link-volume-depencies.json | 2 +- .../testdata/simpletests_windows/datavolume-windows.json | 2 +- .../taskdefinitions/datavolume-windows/task-definition.json | 2 +- .../simpletests_unix/simpletests_generated_unix_test.go | 2 +- .../simpletests_windows/simpletests_generated_windows_test.go | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/agent/functional_tests/testdata/simpletests_unix/link-volume-depencies.json b/agent/functional_tests/testdata/simpletests_unix/link-volume-depencies.json index 51d9a0b9320..651f46cc0d2 100644 --- a/agent/functional_tests/testdata/simpletests_unix/link-volume-depencies.json +++ b/agent/functional_tests/testdata/simpletests_unix/link-volume-depencies.json @@ -3,7 +3,7 @@ "Description": "Tests that the dependency graph of task definitions is resolved correctly", "TaskDefinition": "network-link-2", "Version": ">=1.0.0", - "Timeout": "2m", + "Timeout": "5m", "ExitCodes": { "exit": 42 } diff --git a/agent/functional_tests/testdata/simpletests_windows/datavolume-windows.json b/agent/functional_tests/testdata/simpletests_windows/datavolume-windows.json index f22689ea320..d3c27e99775 100644 --- a/agent/functional_tests/testdata/simpletests_windows/datavolume-windows.json +++ b/agent/functional_tests/testdata/simpletests_windows/datavolume-windows.json @@ -3,7 +3,7 @@ "Description": "Check that basic data volumes work", "TaskDefinition": "datavolume-windows", "Version": ">=1.0.0", - "Timeout": "2m", + "Timeout": "5m", "ExitCodes": { "exit": 42 } diff --git a/agent/functional_tests/testdata/taskdefinitions/datavolume-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/datavolume-windows/task-definition.json index 85e56bac546..533e1eed215 100644 --- a/agent/functional_tests/testdata/taskdefinitions/datavolume-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/datavolume-windows/task-definition.json @@ -34,6 +34,6 @@ "sourceVolume": "test", "containerPath": "C:/data/" }], - "command": ["data volumes shouldn't need to run"] + "command": ["powershell", "-c", "exit"] }] } diff --git a/agent/functional_tests/tests/generated/simpletests_unix/simpletests_generated_unix_test.go b/agent/functional_tests/tests/generated/simpletests_unix/simpletests_generated_unix_test.go index 6888a4f8886..c8f16dc04dd 100644 --- a/agent/functional_tests/tests/generated/simpletests_unix/simpletests_generated_unix_test.go +++ b/agent/functional_tests/tests/generated/simpletests_unix/simpletests_generated_unix_test.go @@ -735,7 +735,7 @@ func TestLinkVolumeDependencies(t *testing.T) { } } - timeout, err := time.ParseDuration("2m") + timeout, err := time.ParseDuration("5m") if err != nil { t.Fatalf("Could not parse timeout: %#v", err) } diff --git a/agent/functional_tests/tests/generated/simpletests_windows/simpletests_generated_windows_test.go b/agent/functional_tests/tests/generated/simpletests_windows/simpletests_generated_windows_test.go index b9a49e75300..e27222e9d28 100644 --- a/agent/functional_tests/tests/generated/simpletests_windows/simpletests_generated_windows_test.go +++ b/agent/functional_tests/tests/generated/simpletests_windows/simpletests_generated_windows_test.go @@ -63,7 +63,7 @@ func TestDataVolume(t *testing.T) { } } - timeout, err := time.ParseDuration("2m") + timeout, err := time.ParseDuration("5m") if err != nil { t.Fatalf("Could not parse timeout: %#v", err) } From 89bb2e8578421bde44429611238ad6bcf30c0a9d Mon Sep 17 00:00:00 2001 From: Derek Petersen Date: Thu, 21 Feb 2019 13:12:50 -0800 Subject: [PATCH 10/24] dependencygraph: simplify container start logic Instead of explicitly checking against many conditions, we now validate that the expected condition has progressed beyond started This mirrors prior behavior in the codebase, and reduces cyclo complexity. --- agent/engine/dependencygraph/graph.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/agent/engine/dependencygraph/graph.go b/agent/engine/dependencygraph/graph.go index 2c7b6b0d767..d92436bac4b 100644 --- a/agent/engine/dependencygraph/graph.go +++ b/agent/engine/dependencygraph/graph.go @@ -328,9 +328,7 @@ func containerOrderingDependenciesIsResolved(target *apicontainer.Container, // The 'target' container desires to be moved to 'Created' or the 'steady' state. // Allow this only if the known status of the dependency container state is already started // i.e it's state is any of 'Created', 'steady state' or 'Stopped' - return dependsOnContainerKnownStatus == apicontainerstatus.ContainerCreated || - dependsOnContainerKnownStatus == apicontainerstatus.ContainerStopped || - dependsOnContainerKnownStatus == dependsOnContainer.GetSteadyStateStatus() + return dependsOnContainerKnownStatus >= apicontainerstatus.ContainerCreated case runningCondition: if targetDesiredStatus == apicontainerstatus.ContainerCreated { From 01dca6d2625cac1d7eb0b8f9e85804613630486b Mon Sep 17 00:00:00 2001 From: Utsa Bhattacharjya Date: Fri, 22 Feb 2019 12:03:22 -0800 Subject: [PATCH 11/24] Remove the functionality of StartTimeout as Docker API Start Timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ‘StartTimeout’ now will only serve as the the time duration after which if a container has a dependency on another container and the conditions are ‘SUCCESS’, ‘HEALTHY’ and ‘COMPLETE’, then the dependency will not be resolved. For example: • If a container A has a dependency on container B and the condition is ‘START’, the StartTimeout for container B will roughly be the time required for it to exit successfully with exit code 0 • If a container A has a dependency on container B and the condition is ‘COMPLETE’, the StartTimeout for container B will roughly be the time required for it to exit. • If a container A has a dependency on container B and the condition is ‘HEALTHY’, the StartTimeout for container B will roughly be the time required for it to emit a ‘HEALTHY’ status. If the StartTimeout exceeds in any of the above cases, container A will not be able to transition to ‘CREATED’ status. It effectively reverts the implementation of StartTimeout in commit: 79bd51770754121c4be28328a0f683c2526e18c8 --- agent/api/container/container.go | 5 +++-- agent/dockerclient/dockerapi/docker_client.go | 6 +++--- agent/engine/docker_task_engine.go | 8 +------- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/agent/api/container/container.go b/agent/api/container/container.go index 9942eab8250..e5cc798bb88 100644 --- a/agent/api/container/container.go +++ b/agent/api/container/container.go @@ -140,8 +140,9 @@ type Container struct { // LogsAuthStrategy specifies how the logs driver for the container will be // authenticated LogsAuthStrategy string - // StartTimeout specifies the time the agent waits for StartContainer api call - // before timing out + // StartTimeout specifies the time value after which if a container has a dependency + // on another container and the dependency conditions are 'SUCCESS', 'COMPLETE', 'HEALTHY', + // then that dependency will not be resolved. StartTimeout uint // StopTimeout specifies the time value to be passed as StopContainer api call StopTimeout uint diff --git a/agent/dockerclient/dockerapi/docker_client.go b/agent/dockerclient/dockerapi/docker_client.go index 6c8641b73db..7ee1eede261 100644 --- a/agent/dockerclient/dockerapi/docker_client.go +++ b/agent/dockerclient/dockerapi/docker_client.go @@ -552,8 +552,8 @@ func (dg *dockerGoClient) createContainer(ctx context.Context, return dg.containerMetadata(ctx, dockerContainer.ID) } -func (dg *dockerGoClient) StartContainer(ctx context.Context, id string, ctxTimeout time.Duration) DockerContainerMetadata { - ctx, cancel := context.WithTimeout(ctx, ctxTimeout) +func (dg *dockerGoClient) StartContainer(ctx context.Context, id string, timeout time.Duration) DockerContainerMetadata { + ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() defer metrics.MetricsEngineGlobal.RecordDockerMetric("START_CONTAINER")() // Buffered channel so in the case of timeout it takes one write, never gets @@ -568,7 +568,7 @@ func (dg *dockerGoClient) StartContainer(ctx context.Context, id string, ctxTime // send back the DockerTimeoutError err := ctx.Err() if err == context.DeadlineExceeded { - return DockerContainerMetadata{Error: &DockerTimeoutError{ctxTimeout, "started"}} + return DockerContainerMetadata{Error: &DockerTimeoutError{timeout, "started"}} } return DockerContainerMetadata{Error: CannotStartContainerError{err}} } diff --git a/agent/engine/docker_task_engine.go b/agent/engine/docker_task_engine.go index f829a448dc7..b58ed952ccd 100644 --- a/agent/engine/docker_task_engine.go +++ b/agent/engine/docker_task_engine.go @@ -955,13 +955,7 @@ func (engine *DockerTaskEngine) startContainer(task *apitask.Task, container *ap } } startContainerBegin := time.Now() - - ctxTimeoutStartContainer := container.GetStartTimeout() - if ctxTimeoutStartContainer <= 0 { - ctxTimeoutStartContainer = engine.cfg.ContainerStartTimeout - } - - dockerContainerMD := client.StartContainer(engine.ctx, dockerContainer.DockerID, ctxTimeoutStartContainer) + dockerContainerMD := client.StartContainer(engine.ctx, 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 From 5bac35e874b717de40298a1cdaf858136f005c86 Mon Sep 17 00:00:00 2001 From: Derek Petersen Date: Fri, 22 Feb 2019 14:16:24 -0800 Subject: [PATCH 12/24] engine: add ordering integration tests This is the first batch of integration tests for container ordering. The tests handle the basic use cases for each of the conditions that introduces new behavior into agent (HEALTHY,COMPLETE,SUCCESS). --- agent/engine/engine_integ_test.go | 9 + agent/engine/engine_unix_integ_test.go | 9 +- agent/engine/engine_windows_integ_test.go | 9 +- agent/engine/ordering_integ_test.go | 304 ++++++++++++++++++++ agent/engine/ordering_integ_unix_test.go | 24 ++ agent/engine/ordering_integ_windows_test.go | 24 ++ 6 files changed, 363 insertions(+), 16 deletions(-) create mode 100644 agent/engine/ordering_integ_test.go create mode 100644 agent/engine/ordering_integ_unix_test.go create mode 100644 agent/engine/ordering_integ_windows_test.go diff --git a/agent/engine/engine_integ_test.go b/agent/engine/engine_integ_test.go index 4425b86d751..b4b101f152c 100644 --- a/agent/engine/engine_integ_test.go +++ b/agent/engine/engine_integ_test.go @@ -51,6 +51,15 @@ const ( localhost = "127.0.0.1" waitForDockerDuration = 50 * time.Millisecond removeVolumeTimeout = 5 * time.Second + + alwaysHealthyHealthCheckConfig = `{ + "HealthCheck":{ + "Test":["CMD-SHELL", "echo hello"], + "Interval":100000000, + "Timeout":100000000, + "StartPeriod":100000000, + "Retries":3} + }` ) func init() { diff --git a/agent/engine/engine_unix_integ_test.go b/agent/engine/engine_unix_integ_test.go index 9ef01080e16..5ad20fb6ab8 100644 --- a/agent/engine/engine_unix_integ_test.go +++ b/agent/engine/engine_unix_integ_test.go @@ -94,14 +94,7 @@ func createTestHealthCheckTask(arn string) *apitask.Task { testTask.Containers[0].HealthCheckType = "docker" testTask.Containers[0].Command = []string{"sh", "-c", "sleep 300"} testTask.Containers[0].DockerConfig = apicontainer.DockerConfig{ - Config: aws.String(`{ - "HealthCheck":{ - "Test":["CMD-SHELL", "echo hello"], - "Interval":100000000, - "Timeout":100000000, - "StartPeriod":100000000, - "Retries":3} - }`), + Config: aws.String(alwaysHealthyHealthCheckConfig), } return testTask } diff --git a/agent/engine/engine_windows_integ_test.go b/agent/engine/engine_windows_integ_test.go index 661b9b74aaf..e18b3995ba1 100644 --- a/agent/engine/engine_windows_integ_test.go +++ b/agent/engine/engine_windows_integ_test.go @@ -101,14 +101,7 @@ func createTestHealthCheckTask(arn string) *apitask.Task { testTask.Containers[0].HealthCheckType = "docker" testTask.Containers[0].Command = []string{"powershell", "-command", "Start-Sleep -s 300"} testTask.Containers[0].DockerConfig = apicontainer.DockerConfig{ - Config: aws.String(`{ - "HealthCheck":{ - "Test":["CMD-SHELL", "echo hello"], - "Interval":100000000, - "Timeout":100000000, - "StartPeriod":100000000, - "Retries":3} - }`), + Config: aws.String(alwaysHealthyHealthCheckConfig), } return testTask } diff --git a/agent/engine/ordering_integ_test.go b/agent/engine/ordering_integ_test.go new file mode 100644 index 00000000000..39a1ce175ee --- /dev/null +++ b/agent/engine/ordering_integ_test.go @@ -0,0 +1,304 @@ +// +build integration + +// Copyright 2019 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 +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package engine + +import ( + "testing" + + apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" + "github.com/aws/aws-sdk-go/aws" +) + +// TestDependencyHealthCheck is a happy-case integration test that considers a workflow with a HEALTHY dependency +// condition. We ensure that the task can be both started and stopped. +func TestDependencyHealthCheck(t *testing.T) { + taskEngine, done, _ := setupWithDefaultConfig(t) + defer done() + + stateChangeEvents := taskEngine.StateChangeEvents() + + taskArn := "testDependencyHealth" + testTask := createTestTask(taskArn) + + parent := createTestContainerWithImageAndName(baseImageForOS, "parent") + dependency := createTestContainerWithImageAndName(baseImageForOS, "dependency") + + parent.EntryPoint = &entryPointForOS + parent.Command = []string{"exit 1"} + parent.DependsOn = []apicontainer.DependsOn{ + { + Container: "dependency", + Condition: "HEALTHY", + }, + } + + dependency.EntryPoint = &entryPointForOS + dependency.Command = []string{"sleep 30"} + dependency.HealthCheckType = apicontainer.DockerHealthCheckType + dependency.DockerConfig.Config = aws.String(alwaysHealthyHealthCheckConfig) + + testTask.Containers = []*apicontainer.Container{ + parent, + dependency, + } + + go taskEngine.AddTask(testTask) + + // Both containers should start + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerRunningStateChange(t, taskEngine) + verifyTaskIsRunning(stateChangeEvents, testTask) + + // Task should stop all at once + verifyContainerStoppedStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) + verifyTaskIsStopped(stateChangeEvents, testTask) + +} + +// TestDependencyComplete validates that the COMPLETE dependency condition will resolve when the child container exits +// with exit code 1. It ensures that the child is started and stopped before the parent starts. +func TestDependencyComplete(t *testing.T) { + taskEngine, done, _ := setupWithDefaultConfig(t) + defer done() + + stateChangeEvents := taskEngine.StateChangeEvents() + + taskArn := "testDependencyComplete" + testTask := createTestTask(taskArn) + + parent := createTestContainerWithImageAndName(baseImageForOS, "parent") + dependency := createTestContainerWithImageAndName(baseImageForOS, "dependency") + + parent.EntryPoint = &entryPointForOS + parent.Command = []string{"sleep 5 && exit 0"} + parent.DependsOn = []apicontainer.DependsOn{ + { + Container: "dependency", + Condition: "COMPLETE", + }, + } + + dependency.EntryPoint = &entryPointForOS + dependency.Command = []string{"sleep 10 && exit 1"} + dependency.Essential = false + + testTask.Containers = []*apicontainer.Container{ + parent, + dependency, + } + + go taskEngine.AddTask(testTask) + + // First container should run to completion and then exit + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) + + // Second container starts after the first stops, task becomes running + verifyContainerRunningStateChange(t, taskEngine) + verifyTaskIsRunning(stateChangeEvents, testTask) + + // Last container stops and then the task stops + verifyContainerStoppedStateChange(t, taskEngine) + verifyTaskIsStopped(stateChangeEvents, testTask) +} + +// TestDependencySuccess validates that the SUCCESS dependency condition will resolve when the child container exits +// with exit code 0. It ensures that the child is started and stopped before the parent starts. +func TestDependencySuccess(t *testing.T) { + taskEngine, done, _ := setupWithDefaultConfig(t) + defer done() + + stateChangeEvents := taskEngine.StateChangeEvents() + + taskArn := "testDependencySuccess" + testTask := createTestTask(taskArn) + + parent := createTestContainerWithImageAndName(baseImageForOS, "parent") + dependency := createTestContainerWithImageAndName(baseImageForOS, "dependency") + + parent.EntryPoint = &entryPointForOS + parent.Command = []string{"exit 0"} + parent.DependsOn = []apicontainer.DependsOn{ + { + Container: "dependency", + Condition: "SUCCESS", + }, + } + + dependency.EntryPoint = &entryPointForOS + dependency.Command = []string{"sleep 10 && exit 0"} + dependency.Essential = false + + testTask.Containers = []*apicontainer.Container{ + parent, + dependency, + } + + go taskEngine.AddTask(testTask) + + // First container should run to completion + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) + + // Second container starts after the first stops, task becomes running + verifyContainerRunningStateChange(t, taskEngine) + verifyTaskIsRunning(stateChangeEvents, testTask) + + // Last container stops and then the task stops + verifyContainerStoppedStateChange(t, taskEngine) + verifyTaskIsStopped(stateChangeEvents, testTask) +} + +// TestDependencySuccess validates that the SUCCESS dependency condition will fail when the child exits 1. This is a +// contrast to how COMPLETE behaves. Instead of starting the parent, the task should simply exit. +func TestDependencySuccessErrored(t *testing.T) { + t.Skip("TODO: this test exposes a bug. Fix the bug and then remove this skip.") + taskEngine, done, _ := setupWithDefaultConfig(t) + defer done() + + stateChangeEvents := taskEngine.StateChangeEvents() + + taskArn := "testDependencySuccessErrored" + testTask := createTestTask(taskArn) + + parent := createTestContainerWithImageAndName(baseImageForOS, "parent") + dependency := createTestContainerWithImageAndName(baseImageForOS, "dependency") + + parent.EntryPoint = &entryPointForOS + parent.Command = []string{"exit 0"} + parent.DependsOn = []apicontainer.DependsOn{ + { + Container: "dependency", + Condition: "SUCCESS", + }, + } + + dependency.EntryPoint = &entryPointForOS + dependency.Command = []string{"sleep 10 && exit 1"} + dependency.Essential = false + + testTask.Containers = []*apicontainer.Container{ + parent, + dependency, + } + + go taskEngine.AddTask(testTask) + + // First container should run to completion + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) + + // task should transition to stopped + verifyTaskIsStopped(stateChangeEvents, testTask) +} + +// TestDependencySuccessTimeout +func TestDependencySuccessTimeout(t *testing.T) { + t.Skip("TODO: this test exposes a bug. Fix the bug and then remove this skip.") + taskEngine, done, _ := setupWithDefaultConfig(t) + defer done() + + stateChangeEvents := taskEngine.StateChangeEvents() + + taskArn := "testDependencySuccessTimeout" + testTask := createTestTask(taskArn) + + parent := createTestContainerWithImageAndName(baseImageForOS, "parent") + dependency := createTestContainerWithImageAndName(baseImageForOS, "dependency") + + parent.EntryPoint = &entryPointForOS + parent.Command = []string{"exit 0"} + parent.DependsOn = []apicontainer.DependsOn{ + { + Container: "dependency", + Condition: "SUCCESS", + }, + } + + dependency.EntryPoint = &entryPointForOS + dependency.Command = []string{"sleep 15 && exit 0"} + dependency.Essential = false + + // set the timeout to be shorter than the amount of time it takes to stop + dependency.StartTimeout = 8 + + testTask.Containers = []*apicontainer.Container{ + parent, + dependency, + } + + go taskEngine.AddTask(testTask) + + // First container should run to completion + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) + + // task should transition to stopped + verifyTaskIsStopped(stateChangeEvents, testTask) +} + +// TestDependencyHealthyTimeout +func TestDependencyHealthyTimeout(t *testing.T) { + t.Skip("TODO: this test exposes a bug. Fix the bug and then remove this skip.") + taskEngine, done, _ := setupWithDefaultConfig(t) + defer done() + + stateChangeEvents := taskEngine.StateChangeEvents() + + taskArn := "testDependencyHealthyTimeout" + testTask := createTestTask(taskArn) + + parent := createTestContainerWithImageAndName(baseImageForOS, "parent") + dependency := createTestContainerWithImageAndName(baseImageForOS, "dependency") + + parent.EntryPoint = &entryPointForOS + parent.Command = []string{"exit 0"} + parent.DependsOn = []apicontainer.DependsOn{ + { + Container: "dependency", + Condition: "HEALTHY", + }, + } + + dependency.EntryPoint = &entryPointForOS + dependency.Command = []string{"sleep 30"} + dependency.HealthCheckType = apicontainer.DockerHealthCheckType + + // enter a healthcheck that will fail + dependency.DockerConfig.Config = aws.String(`{ + "HealthCheck":{ + "Test":["CMD-SHELL", "exit 1"] + } + }`) + + // set the timeout. Duration doesn't matter since healthcheck will always be unhealthy. + dependency.StartTimeout = 8 + + testTask.Containers = []*apicontainer.Container{ + parent, + dependency, + } + + go taskEngine.AddTask(testTask) + + // First container should run to completion + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) + + // task should transition to stopped + verifyTaskIsStopped(stateChangeEvents, testTask) +} diff --git a/agent/engine/ordering_integ_unix_test.go b/agent/engine/ordering_integ_unix_test.go new file mode 100644 index 00000000000..d168cf8828a --- /dev/null +++ b/agent/engine/ordering_integ_unix_test.go @@ -0,0 +1,24 @@ +// +build integration,!windows + +// Copyright 2019 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 +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package engine + +const ( + baseImageForOS = testRegistryHost + "/" + "busybox" +) + +var ( + entryPointForOS = []string{"sh", "-c"} +) diff --git a/agent/engine/ordering_integ_windows_test.go b/agent/engine/ordering_integ_windows_test.go new file mode 100644 index 00000000000..7b236aea78a --- /dev/null +++ b/agent/engine/ordering_integ_windows_test.go @@ -0,0 +1,24 @@ +// +build windows,integration + +// Copyright 2019 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 +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package engine + +const ( + baseImageForOS = "microsoft/windowsservercore" +) + +var ( + entryPointForOS = []string{"pwsh", "-c"} +) From 4ac84e4dd40766e5e7149f03a47e0ce30ce42ec1 Mon Sep 17 00:00:00 2001 From: Utsa Bhattacharjya Date: Sat, 23 Feb 2019 15:46:08 -0800 Subject: [PATCH 13/24] Dependency Condition Naming change: * "START" Dependency condition has been changed to "CREATE" as it waits for the dependency to atleast get created * "RUNNING" Dependency Condition has been changed to "START" as it waits for the dependency to get started. --- agent/api/task/task.go | 8 +-- agent/api/task/task_test.go | 8 +-- agent/engine/dependencygraph/graph.go | 16 ++--- agent/engine/dependencygraph/graph_test.go | 74 +++++++++++----------- 4 files changed, 53 insertions(+), 53 deletions(-) diff --git a/agent/api/task/task.go b/agent/api/task/task.go index b5705fcf85d..a5536fe5203 100644 --- a/agent/api/task/task.go +++ b/agent/api/task/task.go @@ -72,8 +72,8 @@ const ( NvidiaVisibleDevicesEnvVar = "NVIDIA_VISIBLE_DEVICES" GPUAssociationType = "gpu" - ContainerOrderingStartCondition = "START" - ContainerOrderingRunningCondition = "RUNNING" + ContainerOrderingCreateCondition = "CREATE" + ContainerOrderingStartCondition = "START" arnResourceSections = 2 arnResourceDelimiter = "/" @@ -1272,7 +1272,7 @@ func (task *Task) initializeContainerOrderingForVolumes() error { if _, ok := task.ContainerByName(volume.SourceContainer); !ok { return fmt.Errorf("could not find container with name %s", volume.SourceContainer) } - dependOn := apicontainer.DependsOn{Container: volume.SourceContainer, Condition: ContainerOrderingStartCondition} + dependOn := apicontainer.DependsOn{Container: volume.SourceContainer, Condition: ContainerOrderingCreateCondition} container.DependsOn = append(container.DependsOn, dependOn) } } @@ -1292,7 +1292,7 @@ func (task *Task) initializeContainerOrderingForLinks() error { if _, ok := task.ContainerByName(linkName); !ok { return fmt.Errorf("could not find container with name %s", linkName) } - dependOn := apicontainer.DependsOn{Container: linkName, Condition: ContainerOrderingRunningCondition} + dependOn := apicontainer.DependsOn{Container: linkName, Condition: ContainerOrderingStartCondition} container.DependsOn = append(container.DependsOn, dependOn) } } diff --git a/agent/api/task/task_test.go b/agent/api/task/task_test.go index c642ce47c01..64236f5e01d 100644 --- a/agent/api/task/task_test.go +++ b/agent/api/task/task_test.go @@ -2887,17 +2887,17 @@ func TestInitializeContainerOrderingWithLinksAndVolumesFrom(t *testing.T) { containerResultWithVolume := task.Containers[0] assert.Equal(t, "myName1", containerResultWithVolume.DependsOn[0].Container) - assert.Equal(t, ContainerOrderingStartCondition, containerResultWithVolume.DependsOn[0].Condition) + assert.Equal(t, ContainerOrderingCreateCondition, containerResultWithVolume.DependsOn[0].Condition) containerResultWithLink := task.Containers[1] assert.Equal(t, "myName", containerResultWithLink.DependsOn[0].Container) - assert.Equal(t, ContainerOrderingRunningCondition, containerResultWithLink.DependsOn[0].Condition) + assert.Equal(t, ContainerOrderingStartCondition, containerResultWithLink.DependsOn[0].Condition) containerResultWithBothVolumeAndLink := task.Containers[2] assert.Equal(t, "myName", containerResultWithBothVolumeAndLink.DependsOn[0].Container) - assert.Equal(t, ContainerOrderingStartCondition, containerResultWithBothVolumeAndLink.DependsOn[0].Condition) + assert.Equal(t, ContainerOrderingCreateCondition, containerResultWithBothVolumeAndLink.DependsOn[0].Condition) assert.Equal(t, "myName1", containerResultWithBothVolumeAndLink.DependsOn[1].Container) - assert.Equal(t, ContainerOrderingRunningCondition, containerResultWithBothVolumeAndLink.DependsOn[1].Condition) + assert.Equal(t, ContainerOrderingStartCondition, containerResultWithBothVolumeAndLink.DependsOn[1].Condition) containerResultWithNoVolumeOrLink := task.Containers[3] assert.Equal(t, 0, len(containerResultWithNoVolumeOrLink.DependsOn)) diff --git a/agent/engine/dependencygraph/graph.go b/agent/engine/dependencygraph/graph.go index 4cf56f19195..e94b8142851 100644 --- a/agent/engine/dependencygraph/graph.go +++ b/agent/engine/dependencygraph/graph.go @@ -26,16 +26,16 @@ import ( ) const ( - // StartCondition ensures that a container progresses to next state only when dependency container has started + // CreateCondition ensures that a container progresses to next state only when dependency container has started + createCondition = "CREATE" + // StartCondition ensures that a container progresses to next state only when dependency container is running startCondition = "START" - // RunningCondition ensures that a container progresses to next state only when dependency container is running - runningCondition = "RUNNING" // SuccessCondition ensures that a container progresses to next state only when // dependency container has successfully completed with exit code 0 successCondition = "SUCCESS" // CompleteCondition ensures that a container progresses to next state only when dependency container has completed completeCondition = "COMPLETE" - // StartCondition ensures that a container progresses to next state only when dependency container is healthy + // HealthyCondition ensures that a container progresses to next state only when dependency container is healthy healthyCondition = "HEALTHY" // 0 is the standard exit code for success. successExitCode = 0 @@ -277,10 +277,10 @@ func containerOrderingDependenciesCanResolve(target *apicontainer.Container, dependsOnContainerDesiredStatus := dependsOnContainer.GetDesiredStatus() switch dependsOnStatus { - case startCondition: + case createCondition: return verifyContainerOrderingStatus(dependsOnContainer) - case runningCondition: + case startCondition: if targetDesiredStatus == apicontainerstatus.ContainerCreated { // The 'target' container desires to be moved to 'Created' state. // Allow this only if the desired status of the dependency container is @@ -323,7 +323,7 @@ func containerOrderingDependenciesIsResolved(target *apicontainer.Container, dependsOnContainerKnownStatus := dependsOnContainer.GetKnownStatus() switch dependsOnStatus { - case startCondition: + case createCondition: // The 'target' container desires to be moved to 'Created' or the 'steady' state. // Allow this only if the known status of the dependency container state is already started // i.e it's state is any of 'Created', 'steady state' or 'Stopped' @@ -331,7 +331,7 @@ func containerOrderingDependenciesIsResolved(target *apicontainer.Container, dependsOnContainerKnownStatus == apicontainerstatus.ContainerStopped || dependsOnContainerKnownStatus == dependsOnContainer.GetSteadyStateStatus() - case runningCondition: + case startCondition: if targetDesiredStatus == apicontainerstatus.ContainerCreated { // The 'target' container desires to be moved to 'Created' state. // Allow this only if the known status of the linked container is diff --git a/agent/engine/dependencygraph/graph_test.go b/agent/engine/dependencygraph/graph_test.go index d8b2e798404..c0a710fc457 100644 --- a/agent/engine/dependencygraph/graph_test.go +++ b/agent/engine/dependencygraph/graph_test.go @@ -72,11 +72,11 @@ func TestValidDependencies(t *testing.T) { assert.True(t, resolveable, "One container should resolve trivially") // Webserver stack - php := steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: "RUNNING"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - db := steadyStateContainer("db", []apicontainer.DependsOn{{Container: "dbdatavolume", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + php := steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: startCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + db := steadyStateContainer("db", []apicontainer.DependsOn{{Container: "dbdatavolume", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) dbdata := createdContainer("dbdatavolume", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) - webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{Container: "php", Condition: "RUNNING"}, {Container: "htmldata", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{Container: "sharedcssfiles", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{Container: "php", Condition: startCondition}, {Container: "htmldata", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{Container: "sharedcssfiles", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) sharedcssfiles := createdContainer("sharedcssfiles", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) task = &apitask.Task{ @@ -93,8 +93,8 @@ func TestValidDependenciesWithCycles(t *testing.T) { // Unresolveable: cycle task := &apitask.Task{ Containers: []*apicontainer.Container{ - steadyStateContainer("a", []apicontainer.DependsOn{{Container: "b", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), - steadyStateContainer("b", []apicontainer.DependsOn{{Container: "a", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), + steadyStateContainer("a", []apicontainer.DependsOn{{Container: "b", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), + steadyStateContainer("b", []apicontainer.DependsOn{{Container: "a", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), }, } resolveable := ValidDependencies(task) @@ -105,7 +105,7 @@ func TestValidDependenciesWithUnresolvedReference(t *testing.T) { // Unresolveable, reference doesn't exist task := &apitask.Task{ Containers: []*apicontainer.Container{ - steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), + steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), }, } resolveable := ValidDependencies(task) @@ -125,11 +125,11 @@ func TestDependenciesAreResolvedWhenSteadyStateIsRunning(t *testing.T) { assert.NoError(t, err, "One container should resolve trivially") // Webserver stack - php := steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: "RUNNING"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - db := steadyStateContainer("db", []apicontainer.DependsOn{{Container: "dbdatavolume", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + php := steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: startCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + db := steadyStateContainer("db", []apicontainer.DependsOn{{Container: "dbdatavolume", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) dbdata := createdContainer("dbdatavolume", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) - webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{Container: "php", Condition: "RUNNING"}, {Container: "htmldata", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{Container: "sharedcssfiles", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{Container: "php", Condition: startCondition}, {Container: "htmldata", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{Container: "sharedcssfiles", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) sharedcssfiles := createdContainer("sharedcssfiles", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) task = &apitask.Task{ @@ -193,11 +193,11 @@ func TestRunDependencies(t *testing.T) { func TestRunDependenciesWhenSteadyStateIsResourcesProvisionedForOneContainer(t *testing.T) { // Webserver stack - php := steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - db := steadyStateContainer("db", []apicontainer.DependsOn{{Container: "dbdatavolume", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + php := steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + db := steadyStateContainer("db", []apicontainer.DependsOn{{Container: "dbdatavolume", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) dbdata := createdContainer("dbdatavolume", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) - webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{Container: "php", Condition: "START"}, {Container: "htmldata", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{Container: "sharedcssfiles", Condition: "START"}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{Container: "php", Condition: createCondition}, {Container: "htmldata", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{Container: "sharedcssfiles", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) sharedcssfiles := createdContainer("sharedcssfiles", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) // The Pause container, being added to the webserver stack pause := steadyStateContainer("pause", []apicontainer.DependsOn{}, apicontainerstatus.ContainerResourcesProvisioned, apicontainerstatus.ContainerResourcesProvisioned) @@ -580,67 +580,67 @@ func TestContainerOrderingCanResolve(t *testing.T) { { TargetDesired: apicontainerstatus.ContainerCreated, DependencyDesired: apicontainerstatus.ContainerStatusNone, - DependencyCondition: startCondition, + DependencyCondition: createCondition, Resolvable: false, }, { TargetDesired: apicontainerstatus.ContainerCreated, DependencyDesired: apicontainerstatus.ContainerStopped, - DependencyCondition: startCondition, + DependencyCondition: createCondition, Resolvable: true, }, { TargetDesired: apicontainerstatus.ContainerCreated, DependencyDesired: apicontainerstatus.ContainerZombie, - DependencyCondition: startCondition, + DependencyCondition: createCondition, Resolvable: false, }, { TargetDesired: apicontainerstatus.ContainerRunning, DependencyDesired: apicontainerstatus.ContainerStatusNone, - DependencyCondition: startCondition, + DependencyCondition: createCondition, Resolvable: false, }, { TargetDesired: apicontainerstatus.ContainerRunning, DependencyDesired: apicontainerstatus.ContainerCreated, - DependencyCondition: startCondition, + DependencyCondition: createCondition, Resolvable: true, }, { TargetDesired: apicontainerstatus.ContainerRunning, DependencyDesired: apicontainerstatus.ContainerRunning, - DependencyCondition: startCondition, + DependencyCondition: createCondition, Resolvable: true, }, { TargetDesired: apicontainerstatus.ContainerRunning, DependencyDesired: apicontainerstatus.ContainerStopped, - DependencyCondition: startCondition, + DependencyCondition: createCondition, Resolvable: true, }, { TargetDesired: apicontainerstatus.ContainerCreated, DependencyDesired: apicontainerstatus.ContainerCreated, - DependencyCondition: runningCondition, + DependencyCondition: startCondition, Resolvable: true, }, { TargetDesired: apicontainerstatus.ContainerRunning, DependencyDesired: apicontainerstatus.ContainerRunning, - DependencyCondition: runningCondition, + DependencyCondition: startCondition, Resolvable: true, }, { TargetDesired: apicontainerstatus.ContainerCreated, DependencyDesired: apicontainerstatus.ContainerRunning, - DependencyCondition: runningCondition, + DependencyCondition: startCondition, Resolvable: true, }, { TargetDesired: apicontainerstatus.ContainerRunning, DependencyDesired: apicontainerstatus.ContainerZombie, - DependencyCondition: runningCondition, + DependencyCondition: startCondition, Resolvable: false, }, { @@ -699,67 +699,67 @@ func TestContainerOrderingIsResolved(t *testing.T) { { TargetDesired: apicontainerstatus.ContainerCreated, DependencyKnown: apicontainerstatus.ContainerStatusNone, - DependencyCondition: startCondition, + DependencyCondition: createCondition, Resolved: false, }, { TargetDesired: apicontainerstatus.ContainerCreated, DependencyKnown: apicontainerstatus.ContainerCreated, - DependencyCondition: startCondition, + DependencyCondition: createCondition, Resolved: true, }, { TargetDesired: apicontainerstatus.ContainerRunning, DependencyKnown: apicontainerstatus.ContainerStopped, - DependencyCondition: startCondition, + DependencyCondition: createCondition, Resolved: true, }, { TargetDesired: apicontainerstatus.ContainerCreated, DependencyKnown: apicontainerstatus.ContainerStopped, - DependencyCondition: startCondition, + DependencyCondition: createCondition, Resolved: true, }, { TargetDesired: apicontainerstatus.ContainerRunning, DependencyKnown: apicontainerstatus.ContainerStatusNone, - DependencyCondition: startCondition, + DependencyCondition: createCondition, Resolved: false, }, { TargetDesired: apicontainerstatus.ContainerRunning, DependencyKnown: apicontainerstatus.ContainerCreated, - DependencyCondition: startCondition, + DependencyCondition: createCondition, Resolved: true, }, { TargetDesired: apicontainerstatus.ContainerCreated, DependencyKnown: apicontainerstatus.ContainerCreated, - DependencyCondition: runningCondition, + DependencyCondition: startCondition, Resolved: true, }, { TargetDesired: apicontainerstatus.ContainerCreated, DependencyKnown: apicontainerstatus.ContainerRunning, - DependencyCondition: runningCondition, + DependencyCondition: startCondition, Resolved: true, }, { TargetDesired: apicontainerstatus.ContainerCreated, DependencyKnown: apicontainerstatus.ContainerZombie, - DependencyCondition: runningCondition, + DependencyCondition: startCondition, Resolved: false, }, { TargetDesired: apicontainerstatus.ContainerRunning, DependencyKnown: apicontainerstatus.ContainerRunning, - DependencyCondition: runningCondition, + DependencyCondition: startCondition, Resolved: true, }, { TargetDesired: apicontainerstatus.ContainerRunning, DependencyKnown: apicontainerstatus.ContainerZombie, - DependencyCondition: runningCondition, + DependencyCondition: startCondition, Resolved: false, }, { From c6989196b858afffe48b7dc64d99b5fc691a9451 Mon Sep 17 00:00:00 2001 From: Utsa Bhattacharjya Date: Thu, 21 Feb 2019 17:55:50 -0800 Subject: [PATCH 14/24] Validating that the dependency container has not timed out Here, the time duration(StartTimeout) mentioned by the user for a container is expired or not is checked before resolving the dependency for target container. For example, * if a target container 'A' has dependency on container 'B' and the dependency condiiton is 'SUCCESS', then the dependency will not be resolved if B times out before exitting successfully with exit code 0. * if a target container 'A' has dependency on container 'B' and the dependency condiiton is 'COMPLETE', then the dependency will not be resolved if B times out before exitting. * if a target container 'A' has dependency on container 'B' and the dependency condiiton is 'HEALTHY', then the dependency will not be resolved if B times out before emtting 'Healthy' status. The advantage of this is that the user will get to know that something is wrong with the task if the task is stuck in pending.. --- agent/engine/dependencygraph/graph.go | 38 +++++++++++++++++ agent/engine/dependencygraph/graph_test.go | 48 ++++++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/agent/engine/dependencygraph/graph.go b/agent/engine/dependencygraph/graph.go index d92436bac4b..ec2b69e4d59 100644 --- a/agent/engine/dependencygraph/graph.go +++ b/agent/engine/dependencygraph/graph.go @@ -23,6 +23,7 @@ import ( log "github.com/cihub/seelog" "github.com/pkg/errors" "strings" + "time" ) const ( @@ -209,6 +210,7 @@ func verifyContainerOrderingStatusResolvable(target *apicontainer.Container, exi resolves func(*apicontainer.Container, *apicontainer.Container, string) bool) (*apicontainer.DependsOn, error) { targetGoal := target.GetDesiredStatus() + targetKnown := target.GetKnownStatus() if targetGoal != target.GetSteadyStateStatus() && targetGoal != apicontainerstatus.ContainerCreated { // A container can always stop, die, or reach whatever other state it // wants regardless of what dependencies it has @@ -220,9 +222,27 @@ func verifyContainerOrderingStatusResolvable(target *apicontainer.Container, exi if !ok { return nil, fmt.Errorf("dependency graph: container ordering dependency [%v] for target [%v] does not exist.", dependencyContainer, target) } + if !resolves(target, dependencyContainer, dependency.Condition) { return &dependency, fmt.Errorf("dependency graph: failed to resolve the container ordering dependency [%v] for target [%v]", dependencyContainer, target) } + + // We want to check whether the dependency container has timed out only if target has not been created yet. + // If the target is already created, then everything is normal and dependency can be and is resolved. + // However, if dependency container has already stopped, then it cannot time out. + if targetKnown < apicontainerstatus.ContainerCreated && dependencyContainer.GetKnownStatus() != apicontainerstatus.ContainerStopped { + if hasDependencyTimedOut(dependencyContainer, dependency.Condition) { + return nil, fmt.Errorf("dependency graph: container ordering dependency [%v] for target [%v] has timed out.", dependencyContainer, target) + } + } + + // We want to fail fast if the dependency container did not exit successfully' because target container + // can then never progress to its desired state when the dependency condition is 'SUCCESS' + if dependency.Condition == successCondition && dependencyContainer.GetKnownExitCode() != nil { + if !hasDependencyStoppedSuccessfully(dependencyContainer, dependency.Condition) { + return nil, fmt.Errorf("dependency graph: failed to resolve container ordering dependency [%v] for target [%v] as dependency did not exit successfully.", dependencyContainer, target) + } + } } return nil, nil } @@ -366,6 +386,24 @@ func containerOrderingDependenciesIsResolved(target *apicontainer.Container, } } +func hasDependencyTimedOut(dependOnContainer *apicontainer.Container, dependencyCondition string) bool { + if dependOnContainer.GetStartedAt().IsZero() || dependOnContainer.GetStartTimeout() <= 0 { + return false + } + switch dependencyCondition { + case successCondition, completeCondition, healthyCondition: + return time.Now().After(dependOnContainer.GetStartedAt().Add(dependOnContainer.GetStartTimeout())) + default: + return false + } +} + +func hasDependencyStoppedSuccessfully(dependency *apicontainer.Container, condition string) bool { + isDependencyStoppedSuccessfully := dependency.GetKnownStatus() == apicontainerstatus.ContainerStopped && + *dependency.GetKnownExitCode() == 0 + return isDependencyStoppedSuccessfully +} + func verifyContainerOrderingStatus(dependsOnContainer *apicontainer.Container) bool { dependsOnContainerDesiredStatus := dependsOnContainer.GetDesiredStatus() // The 'target' container desires to be moved to 'Created' or the 'steady' state. diff --git a/agent/engine/dependencygraph/graph_test.go b/agent/engine/dependencygraph/graph_test.go index 92d7c2e0b69..60d26a63dbd 100644 --- a/agent/engine/dependencygraph/graph_test.go +++ b/agent/engine/dependencygraph/graph_test.go @@ -18,6 +18,7 @@ package dependencygraph import ( "fmt" "testing" + "time" apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" apicontainerstatus "github.com/aws/amazon-ecs-agent/agent/api/container/status" @@ -965,3 +966,50 @@ func TestVerifyShutdownOrder(t *testing.T) { }) } } + +func TestStartTimeoutForContainerOrdering(t *testing.T) { + testcases := []struct { + DependencyStartedAt time.Time + DependencyStartTimeout uint + DependencyCondition string + ExpectedTimedOut bool + }{ + { + DependencyStartedAt: time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC), + DependencyStartTimeout: 10, + DependencyCondition: healthyCondition, + ExpectedTimedOut: true, + }, + { + DependencyStartedAt: time.Date(4000, 1, 1, 0, 0, 0, 0, time.UTC), + DependencyStartTimeout: 10, + DependencyCondition: successCondition, + ExpectedTimedOut: false, + }, + { + DependencyStartedAt: time.Time{}, + DependencyStartTimeout: 10, + DependencyCondition: successCondition, + ExpectedTimedOut: false, + }, + } + + for _, tc := range testcases { + t.Run(fmt.Sprintf("T:dependency time out: %v", tc.ExpectedTimedOut), + assertContainerOrderingNotTimedOut(hasDependencyTimedOut, tc.DependencyStartedAt, tc.DependencyStartTimeout, tc.DependencyCondition, tc.ExpectedTimedOut)) + } +} + +func assertContainerOrderingNotTimedOut(f func(dep *apicontainer.Container, depCond string) bool, startedAt time.Time, timeout uint, depCond string, expectedTimedOut bool) func(t *testing.T) { + return func(t *testing.T) { + dep := &apicontainer.Container{ + Name: "dep", + StartTimeout: timeout, + } + + dep.SetStartedAt(startedAt) + + timedOut := f(dep, depCond) + assert.Equal(t, expectedTimedOut, timedOut) + } +} From c065ad31199aa3ad26c4378eb710e288fff9c8d4 Mon Sep 17 00:00:00 2001 From: Derek Petersen Date: Sat, 23 Feb 2019 21:11:56 -0800 Subject: [PATCH 15/24] tests: make windows test run a bit faster * Remove need to pull 'latest' server core By removing the :latest tag from all windowsservercore containers, we will have the tests use the container thats already baked into the AMI. * Remove depdency on golang and python containers We are removing the need to use any containers other than servercore and nanoserver. This reduces the number of downloads needed and the number of builds that happen before the tests start running. * Explicit timeouts on order tests The ordering tests are broken at the moment, so we are capping them with a fixed timeout. --- agent/engine/engine_windows_integ_test.go | 4 +- agent/engine/ordering_integ_test.go | 81 +++++++++++++------ agent/engine/ordering_integ_windows_test.go | 2 +- .../task-definition.json | 2 +- .../task-definition.json | 2 +- .../awslogs-windows/task-definition.json | 2 +- .../cleanup-windows/task-definition.json | 2 +- .../task-definition.json | 2 +- .../task-definition.json | 2 +- .../datavolume-windows/task-definition.json | 6 +- .../hostname-windows/task-definition.json | 2 +- .../labels-windows/task-definition.json | 2 +- .../task-definition.json | 2 +- .../network-mode-windows/task-definition.json | 2 +- .../oom-windows/task-definition.json | 4 +- .../savedstate-windows/task-definition.json | 2 +- .../task-definition.json | 2 +- .../simple-exit-windows/task-definition.json | 2 +- .../task-definition.json | 2 +- .../task-definition.json | 2 +- .../task-local-vol/task-definition.json | 2 +- .../task-definition.json | 2 +- .../task-definition.json | 2 +- .../working-dir-windows/task-definition.json | 2 +- .../windows.dockerfile | 2 +- ...metadata-file-validator-windows.dockerfile | 2 +- ...etup-container-metadata-file-validator.ps1 | 18 +---- .../windows0.dockerfile | 2 +- .../windows1.dockerfile | 2 +- .../windows2.dockerfile | 2 +- .../windows3.dockerfile | 2 +- .../windows4.dockerfile | 2 +- .../windows5.dockerfile | 2 +- .../windows6.dockerfile | 2 +- .../windows7.dockerfile | 2 +- .../windows8.dockerfile | 2 +- .../windows9.dockerfile | 2 +- misc/netkitten/build.ps1 | 13 +-- misc/netkitten/windows.dockerfile | 3 +- misc/stats-windows/windows.dockerfile | 2 +- .../setup-v3-task-endpoint-validator.ps1 | 18 +---- ...task-endpoint-validator-windows.dockerfile | 2 +- misc/volumes-test/windows.dockerfile | 2 +- misc/windows-iam/Setup_Iam.ps1 | 20 +---- misc/windows-iam/Setup_Iam_Images.ps1 | 18 +---- misc/windows-iam/iamroles.dockerfile | 2 +- misc/windows-listen80/Setup_Listen80.ps1 | 4 +- misc/windows-listen80/listen80.dockerfile | 2 +- misc/windows-python/build.ps1 | 15 ---- misc/windows-telemetry/build.ps1 | 2 + misc/windows-telemetry/windows.dockerfile | 9 +-- scripts/run-functional-tests.ps1 | 1 - scripts/run-integ-tests.ps1 | 2 +- 53 files changed, 118 insertions(+), 174 deletions(-) delete mode 100644 misc/windows-python/build.ps1 diff --git a/agent/engine/engine_windows_integ_test.go b/agent/engine/engine_windows_integ_test.go index e18b3995ba1..c0ef0b6190d 100644 --- a/agent/engine/engine_windows_integ_test.go +++ b/agent/engine/engine_windows_integ_test.go @@ -58,7 +58,7 @@ func isDockerRunning() bool { return true } func createTestContainer() *apicontainer.Container { return &apicontainer.Container{ Name: "windows", - Image: "microsoft/windowsservercore:latest", + Image: "microsoft/windowsservercore", Essential: true, DesiredStatusUnsafe: apicontainerstatus.ContainerRunning, CPU: 512, @@ -96,7 +96,7 @@ func createTestHealthCheckTask(arn string) *apitask.Task { DesiredStatusUnsafe: apitaskstatus.TaskRunning, Containers: []*apicontainer.Container{createTestContainer()}, } - testTask.Containers[0].Image = "microsoft/nanoserver:latest" + testTask.Containers[0].Image = "microsoft/nanoserver" testTask.Containers[0].Name = "test-health-check" testTask.Containers[0].HealthCheckType = "docker" testTask.Containers[0].Command = []string{"powershell", "-command", "Start-Sleep -s 300"} diff --git a/agent/engine/ordering_integ_test.go b/agent/engine/ordering_integ_test.go index 39a1ce175ee..c2ddda5c160 100644 --- a/agent/engine/ordering_integ_test.go +++ b/agent/engine/ordering_integ_test.go @@ -20,6 +20,7 @@ import ( apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" "github.com/aws/aws-sdk-go/aws" + "time" ) // TestDependencyHealthCheck is a happy-case integration test that considers a workflow with a HEALTHY dependency @@ -57,15 +58,21 @@ func TestDependencyHealthCheck(t *testing.T) { go taskEngine.AddTask(testTask) - // Both containers should start - verifyContainerRunningStateChange(t, taskEngine) - verifyContainerRunningStateChange(t, taskEngine) - verifyTaskIsRunning(stateChangeEvents, testTask) + finished := make(chan interface{}) + go func() { + // Both containers should start + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerRunningStateChange(t, taskEngine) + verifyTaskIsRunning(stateChangeEvents, testTask) - // Task should stop all at once - verifyContainerStoppedStateChange(t, taskEngine) - verifyContainerStoppedStateChange(t, taskEngine) - verifyTaskIsStopped(stateChangeEvents, testTask) + // Task should stop all at once + verifyContainerStoppedStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) + verifyTaskIsStopped(stateChangeEvents, testTask) + close(finished) + }() + + waitFinished(t, finished, 90*time.Second) } @@ -103,17 +110,23 @@ func TestDependencyComplete(t *testing.T) { go taskEngine.AddTask(testTask) - // First container should run to completion and then exit - verifyContainerRunningStateChange(t, taskEngine) - verifyContainerStoppedStateChange(t, taskEngine) + finished := make(chan interface{}) - // Second container starts after the first stops, task becomes running - verifyContainerRunningStateChange(t, taskEngine) - verifyTaskIsRunning(stateChangeEvents, testTask) + go func() { + // First container should run to completion and then exit + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) - // Last container stops and then the task stops - verifyContainerStoppedStateChange(t, taskEngine) - verifyTaskIsStopped(stateChangeEvents, testTask) + // Second container starts after the first stops, task becomes running + verifyContainerRunningStateChange(t, taskEngine) + verifyTaskIsRunning(stateChangeEvents, testTask) + + // Last container stops and then the task stops + verifyContainerStoppedStateChange(t, taskEngine) + verifyTaskIsStopped(stateChangeEvents, testTask) + }() + + waitFinished(t, finished, 90*time.Second) } // TestDependencySuccess validates that the SUCCESS dependency condition will resolve when the child container exits @@ -150,17 +163,22 @@ func TestDependencySuccess(t *testing.T) { go taskEngine.AddTask(testTask) - // First container should run to completion - verifyContainerRunningStateChange(t, taskEngine) - verifyContainerStoppedStateChange(t, taskEngine) + finished := make(chan interface{}) + go func() { + // First container should run to completion + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) - // Second container starts after the first stops, task becomes running - verifyContainerRunningStateChange(t, taskEngine) - verifyTaskIsRunning(stateChangeEvents, testTask) + // Second container starts after the first stops, task becomes running + verifyContainerRunningStateChange(t, taskEngine) + verifyTaskIsRunning(stateChangeEvents, testTask) - // Last container stops and then the task stops - verifyContainerStoppedStateChange(t, taskEngine) - verifyTaskIsStopped(stateChangeEvents, testTask) + // Last container stops and then the task stops + verifyContainerStoppedStateChange(t, taskEngine) + verifyTaskIsStopped(stateChangeEvents, testTask) + }() + + waitFinished(t, finished, 90*time.Second) } // TestDependencySuccess validates that the SUCCESS dependency condition will fail when the child exits 1. This is a @@ -302,3 +320,14 @@ func TestDependencyHealthyTimeout(t *testing.T) { // task should transition to stopped verifyTaskIsStopped(stateChangeEvents, testTask) } + +func waitFinished(t *testing.T, finished <-chan interface{}, duration time.Duration) { + select { + case <-finished: + t.Log("Finished successfully.") + return + case <-time.After(90 * time.Second): + t.Error("timed out after: ", duration) + t.FailNow() + } +} diff --git a/agent/engine/ordering_integ_windows_test.go b/agent/engine/ordering_integ_windows_test.go index 7b236aea78a..295a989eb73 100644 --- a/agent/engine/ordering_integ_windows_test.go +++ b/agent/engine/ordering_integ_windows_test.go @@ -20,5 +20,5 @@ const ( ) var ( - entryPointForOS = []string{"pwsh", "-c"} + entryPointForOS = []string{"powershell"} ) diff --git a/agent/functional_tests/testdata/taskdefinitions/awslogs-datetime-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/awslogs-datetime-windows/task-definition.json index f503fb39098..eb18daff644 100644 --- a/agent/functional_tests/testdata/taskdefinitions/awslogs-datetime-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/awslogs-datetime-windows/task-definition.json @@ -5,7 +5,7 @@ "memory": 512, "name": "awslogs-datetime-windows", "cpu": 1024, - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "entryPoint": ["powershell"], "command": ["echo", "\"May 01, 2017 19:00:01 ECS\nMay 01, 2017 19:00:04 Agent\nRunning\nin the instance\""], "logConfiguration": { diff --git a/agent/functional_tests/testdata/taskdefinitions/awslogs-multiline-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/awslogs-multiline-windows/task-definition.json index 415b57fe2f8..e89eb61093e 100644 --- a/agent/functional_tests/testdata/taskdefinitions/awslogs-multiline-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/awslogs-multiline-windows/task-definition.json @@ -5,7 +5,7 @@ "memory": 512, "name": "awslogs-multiline-windows", "cpu": 1024, - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "entryPoint": ["powershell"], "command": ["echo", "\"INFO: ECS Agent\nRunning\nINFO: Instance\""], "logConfiguration": { diff --git a/agent/functional_tests/testdata/taskdefinitions/awslogs-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/awslogs-windows/task-definition.json index 344211c1fd5..66661f8a7c2 100644 --- a/agent/functional_tests/testdata/taskdefinitions/awslogs-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/awslogs-windows/task-definition.json @@ -5,7 +5,7 @@ "memory": 512, "name": "awslogs", "cpu": 1024, - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "logConfiguration": { "logDriver": "awslogs", "options": { diff --git a/agent/functional_tests/testdata/taskdefinitions/cleanup-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/cleanup-windows/task-definition.json index 91ab3c575ee..87c38348d16 100644 --- a/agent/functional_tests/testdata/taskdefinitions/cleanup-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/cleanup-windows/task-definition.json @@ -1,7 +1,7 @@ { "family": "ecsftest-cleanup-windows", "containerDefinitions": [{ - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "name": "cleanup-windows", "cpu": 1024, "memory": 512, diff --git a/agent/functional_tests/testdata/taskdefinitions/container-metadata-file-validator-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/container-metadata-file-validator-windows/task-definition.json index 04862dbf8b4..40e03b946cd 100644 --- a/agent/functional_tests/testdata/taskdefinitions/container-metadata-file-validator-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/container-metadata-file-validator-windows/task-definition.json @@ -24,4 +24,4 @@ } }] } - \ No newline at end of file + diff --git a/agent/functional_tests/testdata/taskdefinitions/container-metadata-file-validator/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/container-metadata-file-validator/task-definition.json index 92e29ec7577..ca136a9a8e8 100644 --- a/agent/functional_tests/testdata/taskdefinitions/container-metadata-file-validator/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/container-metadata-file-validator/task-definition.json @@ -21,4 +21,4 @@ } }] } - \ No newline at end of file + diff --git a/agent/functional_tests/testdata/taskdefinitions/datavolume-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/datavolume-windows/task-definition.json index 533e1eed215..4d6175c19be 100644 --- a/agent/functional_tests/testdata/taskdefinitions/datavolume-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/datavolume-windows/task-definition.json @@ -5,7 +5,7 @@ "host": {} }], "containerDefinitions": [{ - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "name": "exit", "cpu": 512, "memory": 256, @@ -15,7 +15,7 @@ }], "command": ["powershell", "-c", "while (1) { sleep 1; if (test-path \"C:/data/success\") { exit 42 }}; done"] }, { - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "name": "dataSource", "cpu": 512, "memory": 256, @@ -25,7 +25,7 @@ }], "command": ["powershell", "-c", "New-Item -ItemType file C:/data/success"] }, { - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "name": "data-volume-source", "cpu": 512, "memory": 256, diff --git a/agent/functional_tests/testdata/taskdefinitions/hostname-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/hostname-windows/task-definition.json index 891e029201e..a1dd5c4cfcb 100644 --- a/agent/functional_tests/testdata/taskdefinitions/hostname-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/hostname-windows/task-definition.json @@ -1,7 +1,7 @@ { "family": "ecsftest-hostname", "containerDefinitions": [{ - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "name": "exit", "cpu": 1024, "memory": 512, diff --git a/agent/functional_tests/testdata/taskdefinitions/labels-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/labels-windows/task-definition.json index 3672df20cee..0672c2cb9a1 100644 --- a/agent/functional_tests/testdata/taskdefinitions/labels-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/labels-windows/task-definition.json @@ -1,7 +1,7 @@ { "family": "ecsftest-labels-windows", "containerDefinitions": [{ - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "name": "labeled", "cpu": 1024, "memory": 512, diff --git a/agent/functional_tests/testdata/taskdefinitions/logdriver-jsonfile-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/logdriver-jsonfile-windows/task-definition.json index 6e608d52c80..2bf70504d9f 100644 --- a/agent/functional_tests/testdata/taskdefinitions/logdriver-jsonfile-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/logdriver-jsonfile-windows/task-definition.json @@ -1,7 +1,7 @@ { "family": "ecsinteg-json-file-rollover", "containerDefinitions": [{ - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "name": "exit", "memory": 512, "cpu": 1024, diff --git a/agent/functional_tests/testdata/taskdefinitions/network-mode-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/network-mode-windows/task-definition.json index 6b381ceb38d..a8835b9d404 100644 --- a/agent/functional_tests/testdata/taskdefinitions/network-mode-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/network-mode-windows/task-definition.json @@ -2,7 +2,7 @@ "family": "ecsftest-networkmode", "networkMode": "$$$$NETWORK_MODE$$$$", "containerDefinitions": [{ - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "entryPoint": ["powershell"], "command": ["sleep", "60"], "name": "network-$$$$NETWORK_MODE$$$$", diff --git a/agent/functional_tests/testdata/taskdefinitions/oom-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/oom-windows/task-definition.json index caa1bd420c1..52be1f38851 100644 --- a/agent/functional_tests/testdata/taskdefinitions/oom-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/oom-windows/task-definition.json @@ -5,8 +5,8 @@ "memory": 256, "name": "memory-overcommit", "cpu": 512, - "image": "amazon/amazon-ecs-windows-python:make", - "command": ["python", "-c", "import time; time.sleep(30); foo=' '*1024*1024*1024;"] + "image": "amazon/amazon-ecs-windows-telemetry-test:make", + "command": ["-concurrency", "10", "-memory", "1024"] }] } diff --git a/agent/functional_tests/testdata/taskdefinitions/savedstate-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/savedstate-windows/task-definition.json index ec831ba5cac..cb9fc368e9a 100644 --- a/agent/functional_tests/testdata/taskdefinitions/savedstate-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/savedstate-windows/task-definition.json @@ -1,7 +1,7 @@ { "family": "ecsftest-savedstate-windows", "containerDefinitions": [{ - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "name": "savedstate-windows", "cpu": 1024, "memory": 512, diff --git a/agent/functional_tests/testdata/taskdefinitions/secrets-environment-variables/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/secrets-environment-variables/task-definition.json index f98daf3a07e..d366d64f21e 100644 --- a/agent/functional_tests/testdata/taskdefinitions/secrets-environment-variables/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/secrets-environment-variables/task-definition.json @@ -17,4 +17,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/agent/functional_tests/testdata/taskdefinitions/simple-exit-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/simple-exit-windows/task-definition.json index b5dbec1a275..9d10d3647d0 100644 --- a/agent/functional_tests/testdata/taskdefinitions/simple-exit-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/simple-exit-windows/task-definition.json @@ -1,7 +1,7 @@ { "family": "ecsinteg-simple-exit-windows", "containerDefinitions": [{ - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "name": "exit", "cpu": 1024, "memory": 512, diff --git a/agent/functional_tests/testdata/taskdefinitions/task-elastic-inference/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/task-elastic-inference/task-definition.json index 61938d975d8..4a9fa4b1b2b 100644 --- a/agent/functional_tests/testdata/taskdefinitions/task-elastic-inference/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/task-elastic-inference/task-definition.json @@ -23,4 +23,4 @@ "deviceType": "eia1.medium" } ] -} \ No newline at end of file +} diff --git a/agent/functional_tests/testdata/taskdefinitions/task-local-vol-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/task-local-vol-windows/task-definition.json index 056b06c4533..bb69a9f7be2 100644 --- a/agent/functional_tests/testdata/taskdefinitions/task-local-vol-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/task-local-vol-windows/task-definition.json @@ -2,7 +2,7 @@ "family": "ecsftest-task-local-volume", "containerDefinitions": [ { - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "name": "exit", "cpu": 512, "memory": 256, diff --git a/agent/functional_tests/testdata/taskdefinitions/task-local-vol/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/task-local-vol/task-definition.json index f83b54c495e..045c670525d 100644 --- a/agent/functional_tests/testdata/taskdefinitions/task-local-vol/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/task-local-vol/task-definition.json @@ -32,4 +32,4 @@ } } ] -} \ No newline at end of file +} diff --git a/agent/functional_tests/testdata/taskdefinitions/task-shared-vol-read-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/task-shared-vol-read-windows/task-definition.json index 64e34d7bf3d..19723e0cd24 100644 --- a/agent/functional_tests/testdata/taskdefinitions/task-shared-vol-read-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/task-shared-vol-read-windows/task-definition.json @@ -3,7 +3,7 @@ "containerDefinitions": [ { "name": "task-shared-vol-read", - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "cpu": 512, "memory": 256, "essential": true, diff --git a/agent/functional_tests/testdata/taskdefinitions/task-shared-vol-write-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/task-shared-vol-write-windows/task-definition.json index dae08a148b9..a7aa9d288de 100644 --- a/agent/functional_tests/testdata/taskdefinitions/task-shared-vol-write-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/task-shared-vol-write-windows/task-definition.json @@ -2,7 +2,7 @@ "family": "ecsftest-task-local-volume", "containerDefinitions": [ { - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "name": "task-shared-vol-write-windows", "cpu": 512, "memory": 256, diff --git a/agent/functional_tests/testdata/taskdefinitions/working-dir-windows/task-definition.json b/agent/functional_tests/testdata/taskdefinitions/working-dir-windows/task-definition.json index 10c584389ab..87ea6631c2c 100644 --- a/agent/functional_tests/testdata/taskdefinitions/working-dir-windows/task-definition.json +++ b/agent/functional_tests/testdata/taskdefinitions/working-dir-windows/task-definition.json @@ -1,7 +1,7 @@ { "family": "ecsftest-working-dir-windows", "containerDefinitions": [{ - "image": "microsoft/windowsservercore:latest", + "image": "microsoft/windowsservercore", "name": "exit", "cpu": 1024, "memory": 512, diff --git a/misc/container-health-windows/windows.dockerfile b/misc/container-health-windows/windows.dockerfile index 83c1f8214cc..3500f4ed529 100644 --- a/misc/container-health-windows/windows.dockerfile +++ b/misc/container-health-windows/windows.dockerfile @@ -10,7 +10,7 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. diff --git a/misc/container-metadata-file-validator-windows/container-metadata-file-validator-windows.dockerfile b/misc/container-metadata-file-validator-windows/container-metadata-file-validator-windows.dockerfile index 68d01abdc6c..3391340e615 100644 --- a/misc/container-metadata-file-validator-windows/container-metadata-file-validator-windows.dockerfile +++ b/misc/container-metadata-file-validator-windows/container-metadata-file-validator-windows.dockerfile @@ -11,6 +11,6 @@ # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/nanoserver:latest +FROM microsoft/windowsservercore ADD container-metadata-file-validator-windows.exe container-metadata-file-validator-windows.exe diff --git a/misc/container-metadata-file-validator-windows/setup-container-metadata-file-validator.ps1 b/misc/container-metadata-file-validator-windows/setup-container-metadata-file-validator.ps1 index bb9fada1201..35c8ce1b1fb 100644 --- a/misc/container-metadata-file-validator-windows/setup-container-metadata-file-validator.ps1 +++ b/misc/container-metadata-file-validator-windows/setup-container-metadata-file-validator.ps1 @@ -14,23 +14,7 @@ $oldPref = $ErrorActionPreference $ErrorActionPreference = 'Stop' -Invoke-Expression ${PSScriptRoot}\..\windows-deploy\hostsetup.ps1 - # Create amazon/amazon-ecs-container-metadata-file-validator-windows for tests -$buildscript = @" -mkdir C:\md -cp C:\ecs\container-metadata-file-validator-windows.go C:\md -go build -o C:\md\container-metadata-file-validator-windows.exe C:\md\container-metadata-file-validator-windows.go -cp C:\md\container-metadata-file-validator-windows.exe C:\ecs -"@ - -$buildimage="golang:1.7-nanoserver" -docker pull $buildimage - -docker run ` - --volume ${PSScriptRoot}:C:\ecs ` - $buildimage ` - powershell ${buildscript} - +Invoke-Expression "go build -o ${PSScriptRoot}\container-metadata-file-validator-windows.exe ${PSScriptRoot}\container-metadata-file-validator-windows.go" Invoke-Expression "docker build -t amazon/amazon-ecs-container-metadata-file-validator-windows --file ${PSScriptRoot}\container-metadata-file-validator-windows.dockerfile ${PSScriptRoot}" $ErrorActionPreference = $oldPref diff --git a/misc/image-cleanup-test-images/windows0.dockerfile b/misc/image-cleanup-test-images/windows0.dockerfile index 0c74c1e1437..795a83eee69 100644 --- a/misc/image-cleanup-test-images/windows0.dockerfile +++ b/misc/image-cleanup-test-images/windows0.dockerfile @@ -10,7 +10,7 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. diff --git a/misc/image-cleanup-test-images/windows1.dockerfile b/misc/image-cleanup-test-images/windows1.dockerfile index 87ea24df410..0eea3f3b725 100644 --- a/misc/image-cleanup-test-images/windows1.dockerfile +++ b/misc/image-cleanup-test-images/windows1.dockerfile @@ -10,7 +10,7 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. diff --git a/misc/image-cleanup-test-images/windows2.dockerfile b/misc/image-cleanup-test-images/windows2.dockerfile index eecb823eac3..402fc7a0972 100644 --- a/misc/image-cleanup-test-images/windows2.dockerfile +++ b/misc/image-cleanup-test-images/windows2.dockerfile @@ -10,7 +10,7 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. diff --git a/misc/image-cleanup-test-images/windows3.dockerfile b/misc/image-cleanup-test-images/windows3.dockerfile index 0467862d05d..1e46fd28a99 100644 --- a/misc/image-cleanup-test-images/windows3.dockerfile +++ b/misc/image-cleanup-test-images/windows3.dockerfile @@ -10,7 +10,7 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. diff --git a/misc/image-cleanup-test-images/windows4.dockerfile b/misc/image-cleanup-test-images/windows4.dockerfile index badb152dad6..66687cefe5a 100644 --- a/misc/image-cleanup-test-images/windows4.dockerfile +++ b/misc/image-cleanup-test-images/windows4.dockerfile @@ -10,7 +10,7 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. diff --git a/misc/image-cleanup-test-images/windows5.dockerfile b/misc/image-cleanup-test-images/windows5.dockerfile index 73e513aac2b..96528688811 100644 --- a/misc/image-cleanup-test-images/windows5.dockerfile +++ b/misc/image-cleanup-test-images/windows5.dockerfile @@ -10,7 +10,7 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. diff --git a/misc/image-cleanup-test-images/windows6.dockerfile b/misc/image-cleanup-test-images/windows6.dockerfile index 2a925dbfc7c..7e5f49366e7 100644 --- a/misc/image-cleanup-test-images/windows6.dockerfile +++ b/misc/image-cleanup-test-images/windows6.dockerfile @@ -10,7 +10,7 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. diff --git a/misc/image-cleanup-test-images/windows7.dockerfile b/misc/image-cleanup-test-images/windows7.dockerfile index c42ca9c428a..7b8115a08fb 100644 --- a/misc/image-cleanup-test-images/windows7.dockerfile +++ b/misc/image-cleanup-test-images/windows7.dockerfile @@ -10,7 +10,7 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. diff --git a/misc/image-cleanup-test-images/windows8.dockerfile b/misc/image-cleanup-test-images/windows8.dockerfile index f87c4367476..9c889a22919 100644 --- a/misc/image-cleanup-test-images/windows8.dockerfile +++ b/misc/image-cleanup-test-images/windows8.dockerfile @@ -10,7 +10,7 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. diff --git a/misc/image-cleanup-test-images/windows9.dockerfile b/misc/image-cleanup-test-images/windows9.dockerfile index 8b243414b5e..b9a069ca3b2 100644 --- a/misc/image-cleanup-test-images/windows9.dockerfile +++ b/misc/image-cleanup-test-images/windows9.dockerfile @@ -10,7 +10,7 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. diff --git a/misc/netkitten/build.ps1 b/misc/netkitten/build.ps1 index cf053df214d..7b49f124fc8 100644 --- a/misc/netkitten/build.ps1 +++ b/misc/netkitten/build.ps1 @@ -11,16 +11,5 @@ # express or implied. See the License for the specific language governing # permissions and limitations under the License. -$buildscript = @" -mkdir C:\nk -cp C:\netkitten\netkitten.go C:\nk -go build -o C:\nk\netkitten.exe C:\nk\netkitten.go -cp C:\nk\netkitten.exe C:\netkitten -"@ - -docker run ` - --volume ${PSScriptRoot}:C:\netkitten ` - golang:1.7-windowsservercore ` - powershell ${buildscript} - +Invoke-Expression "go build -o ${PSScriptRoot}\netkitten.exe ${PSScriptRoot}\netkitten.go" docker build -t "amazon/amazon-ecs-netkitten:make" -f "${PSScriptRoot}/windows.dockerfile" ${PSScriptRoot} diff --git a/misc/netkitten/windows.dockerfile b/misc/netkitten/windows.dockerfile index 67ed5844d4b..83f0a4575e2 100644 --- a/misc/netkitten/windows.dockerfile +++ b/misc/netkitten/windows.dockerfile @@ -10,10 +10,9 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. ADD netkitten.exe C:/netkitten.exe - ENTRYPOINT ["C:\\netkitten.exe"] diff --git a/misc/stats-windows/windows.dockerfile b/misc/stats-windows/windows.dockerfile index 402ad9a72f8..d7888f802f9 100644 --- a/misc/stats-windows/windows.dockerfile +++ b/misc/stats-windows/windows.dockerfile @@ -10,7 +10,7 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. diff --git a/misc/v3-task-endpoint-validator-windows/setup-v3-task-endpoint-validator.ps1 b/misc/v3-task-endpoint-validator-windows/setup-v3-task-endpoint-validator.ps1 index 4968934ee53..e3c349994b3 100644 --- a/misc/v3-task-endpoint-validator-windows/setup-v3-task-endpoint-validator.ps1 +++ b/misc/v3-task-endpoint-validator-windows/setup-v3-task-endpoint-validator.ps1 @@ -14,23 +14,7 @@ $oldPref = $ErrorActionPreference $ErrorActionPreference = 'Stop' -Invoke-Expression ${PSScriptRoot}\..\windows-deploy\hostsetup.ps1 - # Create amazon/amazon-ecs-v3-task-endpoint-validator-windows for tests -$buildscript = @" -mkdir C:\V3 -cp C:\ecs\v3-task-endpoint-validator-windows.go C:\V3 -go build -o C:\V3\v3-task-endpoint-validator-windows.exe C:\V3\v3-task-endpoint-validator-windows.go -cp C:\V3\v3-task-endpoint-validator-windows.exe C:\ecs -"@ - -$buildimage="golang:1.7-windowsservercore" -docker pull $buildimage - -docker run ` - --volume ${PSScriptRoot}:C:\ecs ` - $buildimage ` - powershell ${buildscript} - +Invoke-Expression "go build -o ${PSScriptRoot}\v3-task-endpoint-validator-windows.exe ${PSScriptRoot}\v3-task-endpoint-validator-windows.go" Invoke-Expression "docker build -t amazon/amazon-ecs-v3-task-endpoint-validator-windows --file ${PSScriptRoot}\v3-task-endpoint-validator-windows.dockerfile ${PSScriptRoot}" $ErrorActionPreference = $oldPref diff --git a/misc/v3-task-endpoint-validator-windows/v3-task-endpoint-validator-windows.dockerfile b/misc/v3-task-endpoint-validator-windows/v3-task-endpoint-validator-windows.dockerfile index f5ce1cfc72f..49282b83dbe 100644 --- a/misc/v3-task-endpoint-validator-windows/v3-task-endpoint-validator-windows.dockerfile +++ b/misc/v3-task-endpoint-validator-windows/v3-task-endpoint-validator-windows.dockerfile @@ -11,7 +11,7 @@ # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore ADD application.ps1 application.ps1 ADD v3-task-endpoint-validator-windows.exe v3-task-endpoint-validator-windows.exe diff --git a/misc/volumes-test/windows.dockerfile b/misc/volumes-test/windows.dockerfile index 1382115de0d..50c55bb3ab8 100644 --- a/misc/volumes-test/windows.dockerfile +++ b/misc/volumes-test/windows.dockerfile @@ -11,7 +11,7 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore MAINTAINER Amazon Web Services, Inc. SHELL ["powershell", "-command"] diff --git a/misc/windows-iam/Setup_Iam.ps1 b/misc/windows-iam/Setup_Iam.ps1 index 0d019452247..0597d301ce5 100644 --- a/misc/windows-iam/Setup_Iam.ps1 +++ b/misc/windows-iam/Setup_Iam.ps1 @@ -16,23 +16,9 @@ $ErrorActionPreference = 'Stop' Invoke-Expression ${PSScriptRoot}\..\windows-deploy\hostsetup.ps1 -# Create amazon/amazon-ecs-iamrolecontainer for tests -$buildscript = @" -mkdir C:\IAM -cp C:\ecs\ec2.go C:\IAM -go get -u github.com/aws/aws-sdk-go -go get -u github.com/aws/aws-sdk-go/aws -go build -o C:\IAM\ec2.exe C:\IAM\ec2.go -cp C:\IAM\ec2.exe C:\ecs -"@ - -$buildimage="golang:1.7-windowsservercore" -docker pull $buildimage - -docker run ` - --volume ${PSScriptRoot}:C:\ecs ` - $buildimage ` - powershell ${buildscript} +Invoke-Expression "go get -u github.com/aws/aws-sdk-go" +Invoke-Expression "go get -u github.com/aws/aws-sdk-go/aws" +Invoke-Expression "go build -o ${PSScriptRoot}\ec2.exe ${PSScriptRoot}\ec2.go" Invoke-Expression "docker build -t amazon/amazon-ecs-iamrolecontainer --file ${PSScriptRoot}\iamroles.dockerfile ${PSScriptRoot}" $ErrorActionPreference = $oldPref diff --git a/misc/windows-iam/Setup_Iam_Images.ps1 b/misc/windows-iam/Setup_Iam_Images.ps1 index 310d43bd834..3e793193866 100644 --- a/misc/windows-iam/Setup_Iam_Images.ps1 +++ b/misc/windows-iam/Setup_Iam_Images.ps1 @@ -16,22 +16,10 @@ $ErrorActionPreference = 'Stop' # Create amazon/amazon-ecs-iamrolecontainer for tests -$buildscript = @" -mkdir C:\IAM -cp C:\ecs\ec2.go C:\IAM -go get -u github.com/aws/aws-sdk-go -go get -u github.com/aws/aws-sdk-go/aws -go build -o C:\IAM\ec2.exe C:\IAM\ec2.go -cp C:\IAM\ec2.exe C:\ecs -"@ +Invoke-Expression "go get -u github.com/aws/aws-sdk-go' +Invoke-Expression "go get -u github.com/aws/aws-sdk-go/aws" +Invoke-Expression "go build -o ${PSScriptRoot}\ec2.exe ${PSScriptRoot}\ec2.go" -$buildimage="golang:1.7-windowsservercore" -docker pull $buildimage - -docker run ` - --volume ${PSScriptRoot}:C:\ecs ` - $buildimage ` - powershell ${buildscript} Invoke-Expression "docker build -t amazon/amazon-ecs-iamrolecontainer --file ${PSScriptRoot}\iamroles.dockerfile ${PSScriptRoot}" $ErrorActionPreference = $oldPref diff --git a/misc/windows-iam/iamroles.dockerfile b/misc/windows-iam/iamroles.dockerfile index 11fc5875fd8..5dbf4234d38 100644 --- a/misc/windows-iam/iamroles.dockerfile +++ b/misc/windows-iam/iamroles.dockerfile @@ -11,7 +11,7 @@ # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore ADD application.ps1 application.ps1 ADD ec2.exe ec2.exe diff --git a/misc/windows-listen80/Setup_Listen80.ps1 b/misc/windows-listen80/Setup_Listen80.ps1 index 4f5732486d3..50cb5ee831e 100644 --- a/misc/windows-listen80/Setup_Listen80.ps1 +++ b/misc/windows-listen80/Setup_Listen80.ps1 @@ -10,7 +10,9 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. +$oldPref = $ErrorActionPreference +$ErrorActionPreference = 'Stop' -docker pull microsoft/windowsservercore Invoke-Expression "go build -o ${PSScriptRoot}\listen80.exe ${PSScriptRoot}\listen80.go" Invoke-Expression "docker build -t amazon/amazon-ecs-listen80 --file ${PSScriptRoot}\listen80.dockerfile ${PSScriptRoot}" +$ErrorActionPreference = $oldPref diff --git a/misc/windows-listen80/listen80.dockerfile b/misc/windows-listen80/listen80.dockerfile index 7bad06d8cb6..d544f13a806 100644 --- a/misc/windows-listen80/listen80.dockerfile +++ b/misc/windows-listen80/listen80.dockerfile @@ -11,5 +11,5 @@ # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM microsoft/windowsservercore:latest +FROM microsoft/windowsservercore ADD listen80.exe listen80.exe diff --git a/misc/windows-python/build.ps1 b/misc/windows-python/build.ps1 deleted file mode 100644 index 89a596e023d..00000000000 --- a/misc/windows-python/build.ps1 +++ /dev/null @@ -1,15 +0,0 @@ -# Copyright 2017 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 -# License is located at -# -# http://aws.amazon.com/apache2.0/ -# -# or in the "license" file accompanying this file. This file is distributed -# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -# express or implied. See the License for the specific language governing -# permissions and limitations under the License. - -docker pull python:3-windowsservercore-ltsc2016 -docker tag python:3-windowsservercore-ltsc2016 amazon/amazon-ecs-windows-python:make diff --git a/misc/windows-telemetry/build.ps1 b/misc/windows-telemetry/build.ps1 index 2c3b7143d12..ed00e3b846a 100644 --- a/misc/windows-telemetry/build.ps1 +++ b/misc/windows-telemetry/build.ps1 @@ -11,4 +11,6 @@ # express or implied. See the License for the specific language governing # permissions and limitations under the License. + +Invoke-Expression "go build -o ${PSScriptRoot}\stress.exe ${PSScriptRoot}\main.go" docker build -t "amazon/amazon-ecs-windows-telemetry-test:make" -f "${PSScriptRoot}/windows.dockerfile" ${PSScriptRoot} diff --git a/misc/windows-telemetry/windows.dockerfile b/misc/windows-telemetry/windows.dockerfile index f27985383ad..a409398ba7a 100644 --- a/misc/windows-telemetry/windows.dockerfile +++ b/misc/windows-telemetry/windows.dockerfile @@ -10,11 +10,8 @@ # on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either # express or implied. See the License for the specific language governing # permissions and limitations under the License. -FROM golang:nanoserver +FROM microsoft/windowsservercore -WORKDIR /gopath -COPY main.go . - -RUN go build -o stress main.go -ENTRYPOINT ["./stress"] +ADD stress.exe stress.exe +ENTRYPOINT ["./stress.exe"] CMD [ "-concurrency", "1000", "-memory", "1024"] diff --git a/scripts/run-functional-tests.ps1 b/scripts/run-functional-tests.ps1 index 382ad1d6814..506716aedec 100755 --- a/scripts/run-functional-tests.ps1 +++ b/scripts/run-functional-tests.ps1 @@ -14,7 +14,6 @@ Invoke-Expression "${PSScriptRoot}\..\misc\windows-iam\Setup_Iam.ps1" Invoke-Expression "${PSScriptRoot}\..\misc\windows-listen80\Setup_Listen80.ps1" Invoke-Expression "${PSScriptRoot}\..\misc\windows-telemetry\build.ps1" -Invoke-Expression "${PSScriptRoot}\..\misc\windows-python\build.ps1" Invoke-Expression "${PSScriptRoot}\..\misc\container-health-windows\build.ps1" Invoke-Expression "${PSScriptRoot}\..\misc\v3-task-endpoint-validator-windows\setup-v3-task-endpoint-validator.ps1" Invoke-Expression "${PSScriptRoot}\..\misc\container-metadata-file-validator-windows\setup-container-metadata-file-validator.ps1" diff --git a/scripts/run-integ-tests.ps1 b/scripts/run-integ-tests.ps1 index b99d2792c2d..7d333eb779e 100755 --- a/scripts/run-integ-tests.ps1 +++ b/scripts/run-integ-tests.ps1 @@ -22,7 +22,7 @@ Invoke-Expression "${PSScriptRoot}\..\misc\netkitten\build.ps1" $cwd = (pwd).Path try { cd "${PSScriptRoot}" - go test -race -tags integration -timeout=25m -v ../agent/engine ../agent/stats ../agent/app + go test -race -tags integration -timeout=35m -v ../agent/engine ../agent/stats ../agent/app $testsExitCode = $LastExitCode } finally { cd "$cwd" From 3c27ecefe514d8a400191e05ce5f8c2395a22b7e Mon Sep 17 00:00:00 2001 From: Utsa Bhattacharjya Date: Sat, 23 Feb 2019 18:28:12 -0800 Subject: [PATCH 16/24] Checking dependency resolution after timeout and successful exit check --- agent/engine/dependencygraph/graph.go | 8 ++++---- agent/engine/ordering_integ_test.go | 3 --- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/agent/engine/dependencygraph/graph.go b/agent/engine/dependencygraph/graph.go index 4b4b77e5b55..28d482be0a4 100644 --- a/agent/engine/dependencygraph/graph.go +++ b/agent/engine/dependencygraph/graph.go @@ -223,10 +223,6 @@ func verifyContainerOrderingStatusResolvable(target *apicontainer.Container, exi return nil, fmt.Errorf("dependency graph: container ordering dependency [%v] for target [%v] does not exist.", dependencyContainer, target) } - if !resolves(target, dependencyContainer, dependency.Condition) { - return &dependency, fmt.Errorf("dependency graph: failed to resolve the container ordering dependency [%v] for target [%v]", dependencyContainer, target) - } - // We want to check whether the dependency container has timed out only if target has not been created yet. // If the target is already created, then everything is normal and dependency can be and is resolved. // However, if dependency container has already stopped, then it cannot time out. @@ -243,6 +239,10 @@ func verifyContainerOrderingStatusResolvable(target *apicontainer.Container, exi return nil, fmt.Errorf("dependency graph: failed to resolve container ordering dependency [%v] for target [%v] as dependency did not exit successfully.", dependencyContainer, target) } } + + if !resolves(target, dependencyContainer, dependency.Condition) { + return &dependency, fmt.Errorf("dependency graph: failed to resolve the container ordering dependency [%v] for target [%v]", dependencyContainer, target) + } } return nil, nil } diff --git a/agent/engine/ordering_integ_test.go b/agent/engine/ordering_integ_test.go index c2ddda5c160..3d0c9a99e20 100644 --- a/agent/engine/ordering_integ_test.go +++ b/agent/engine/ordering_integ_test.go @@ -184,7 +184,6 @@ func TestDependencySuccess(t *testing.T) { // TestDependencySuccess validates that the SUCCESS dependency condition will fail when the child exits 1. This is a // contrast to how COMPLETE behaves. Instead of starting the parent, the task should simply exit. func TestDependencySuccessErrored(t *testing.T) { - t.Skip("TODO: this test exposes a bug. Fix the bug and then remove this skip.") taskEngine, done, _ := setupWithDefaultConfig(t) defer done() @@ -226,7 +225,6 @@ func TestDependencySuccessErrored(t *testing.T) { // TestDependencySuccessTimeout func TestDependencySuccessTimeout(t *testing.T) { - t.Skip("TODO: this test exposes a bug. Fix the bug and then remove this skip.") taskEngine, done, _ := setupWithDefaultConfig(t) defer done() @@ -271,7 +269,6 @@ func TestDependencySuccessTimeout(t *testing.T) { // TestDependencyHealthyTimeout func TestDependencyHealthyTimeout(t *testing.T) { - t.Skip("TODO: this test exposes a bug. Fix the bug and then remove this skip.") taskEngine, done, _ := setupWithDefaultConfig(t) defer done() From cd2b228b7a4adb8de0b3feef16c9693059838005 Mon Sep 17 00:00:00 2001 From: Derek Petersen Date: Sun, 24 Feb 2019 14:04:12 -0800 Subject: [PATCH 17/24] engine: adding timeouts to the other order tests The other order tests need timeouts so that the test fails fast instead of waiting for the test runnner to timeout. --- agent/engine/ordering_integ_test.go | 59 +++++++++++++++++++---------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/agent/engine/ordering_integ_test.go b/agent/engine/ordering_integ_test.go index 3d0c9a99e20..ed29af5129a 100644 --- a/agent/engine/ordering_integ_test.go +++ b/agent/engine/ordering_integ_test.go @@ -23,6 +23,8 @@ import ( "time" ) +const orderingTimeout = 60 * time.Second + // TestDependencyHealthCheck is a happy-case integration test that considers a workflow with a HEALTHY dependency // condition. We ensure that the task can be both started and stopped. func TestDependencyHealthCheck(t *testing.T) { @@ -72,7 +74,7 @@ func TestDependencyHealthCheck(t *testing.T) { close(finished) }() - waitFinished(t, finished, 90*time.Second) + waitFinished(t, finished, orderingTimeout) } @@ -111,7 +113,6 @@ func TestDependencyComplete(t *testing.T) { go taskEngine.AddTask(testTask) finished := make(chan interface{}) - go func() { // First container should run to completion and then exit verifyContainerRunningStateChange(t, taskEngine) @@ -124,9 +125,10 @@ func TestDependencyComplete(t *testing.T) { // Last container stops and then the task stops verifyContainerStoppedStateChange(t, taskEngine) verifyTaskIsStopped(stateChangeEvents, testTask) + close(finished) }() - waitFinished(t, finished, 90*time.Second) + waitFinished(t, finished, orderingTimeout) } // TestDependencySuccess validates that the SUCCESS dependency condition will resolve when the child container exits @@ -176,9 +178,10 @@ func TestDependencySuccess(t *testing.T) { // Last container stops and then the task stops verifyContainerStoppedStateChange(t, taskEngine) verifyTaskIsStopped(stateChangeEvents, testTask) + close(finished) }() - waitFinished(t, finished, 90*time.Second) + waitFinished(t, finished, orderingTimeout) } // TestDependencySuccess validates that the SUCCESS dependency condition will fail when the child exits 1. This is a @@ -215,12 +218,18 @@ func TestDependencySuccessErrored(t *testing.T) { go taskEngine.AddTask(testTask) - // First container should run to completion - verifyContainerRunningStateChange(t, taskEngine) - verifyContainerStoppedStateChange(t, taskEngine) + finished := make(chan interface{}) + go func() { + // First container should run to completion + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) + + // task should transition to stopped + verifyTaskIsStopped(stateChangeEvents, testTask) + close(finished) + }() - // task should transition to stopped - verifyTaskIsStopped(stateChangeEvents, testTask) + waitFinished(t, finished, orderingTimeout) } // TestDependencySuccessTimeout @@ -259,12 +268,18 @@ func TestDependencySuccessTimeout(t *testing.T) { go taskEngine.AddTask(testTask) - // First container should run to completion - verifyContainerRunningStateChange(t, taskEngine) - verifyContainerStoppedStateChange(t, taskEngine) + finished := make(chan interface{}) + go func() { + // First container should run to completion + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) + + // task should transition to stopped + verifyTaskIsStopped(stateChangeEvents, testTask) + close(finished) + }() - // task should transition to stopped - verifyTaskIsStopped(stateChangeEvents, testTask) + waitFinished(t, finished, orderingTimeout) } // TestDependencyHealthyTimeout @@ -310,12 +325,18 @@ func TestDependencyHealthyTimeout(t *testing.T) { go taskEngine.AddTask(testTask) - // First container should run to completion - verifyContainerRunningStateChange(t, taskEngine) - verifyContainerStoppedStateChange(t, taskEngine) + finished := make(chan interface{}) + go func() { + // First container should run to completion + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) + + // task should transition to stopped + verifyTaskIsStopped(stateChangeEvents, testTask) + close(finished) + }() - // task should transition to stopped - verifyTaskIsStopped(stateChangeEvents, testTask) + waitFinished(t, finished, orderingTimeout) } func waitFinished(t *testing.T, finished <-chan interface{}, duration time.Duration) { From 6ef726650fd4fe14be35177cb4575720d0f8ffcd Mon Sep 17 00:00:00 2001 From: Utsa Bhattacharjya Date: Sat, 23 Feb 2019 22:54:14 -0800 Subject: [PATCH 18/24] ACS model change --- agent/acs/model/api/api-2.json | 62 +++++++++++++++++++++----------- agent/acs/model/ecsacs/api.go | 36 +++++++++++++++---- agent/api/container/container.go | 2 +- 3 files changed, 73 insertions(+), 27 deletions(-) diff --git a/agent/acs/model/api/api-2.json b/agent/acs/model/api/api-2.json index 0c96d646e18..3e53f13406e 100644 --- a/agent/acs/model/api/api-2.json +++ b/agent/acs/model/api/api-2.json @@ -138,8 +138,8 @@ "AssociationType":{ "type":"string", "enum":[ - "gpu", - "elastic-inference" + "elastic-inference", + "gpu" ] }, "Associations":{ @@ -204,23 +204,12 @@ "registryAuthentication":{"shape":"RegistryAuthenticationData"}, "logsAuthStrategy":{"shape":"AuthStrategy"}, "secrets":{"shape":"SecretList"}, - "dependsOn":{"shape": "DependsOnList"}, + "dependsOn":{"shape":"ContainerDependencies"}, "startTimeout":{"shape":"Integer"}, "stopTimeout":{"shape":"Integer"} } }, - "ContainerList":{ - "type":"list", - "member":{"shape":"Container"} - }, - "DependsOn":{ - "type":"structure", - "members":{ - "container":{"shape":"String"}, - "condition":{"shape":"ConditionType"} - } - }, - "ConditionType":{ + "ContainerCondition":{ "type":"string", "enum":[ "START", @@ -229,9 +218,20 @@ "HEALTHY" ] }, - "DependsOnList":{ + "ContainerDependencies":{ + "type":"list", + "member":{"shape":"ContainerDependency"} + }, + "ContainerDependency":{ + "type":"structure", + "members":{ + "containerName":{"shape":"String"}, + "condition":{"shape":"ContainerCondition"} + } + }, + "ContainerList":{ "type":"list", - "member":{"shape":"DependsOn"} + "member":{"shape":"Container"} }, "DockerConfig":{ "type":"structure", @@ -455,6 +455,18 @@ "type":"list", "member":{"shape":"PortMapping"} }, + "ProxyConfiguration":{ + "type":"structure", + "members":{ + "type":{"shape":"ProxyConfigurationType"}, + "containerName":{"shape":"String"}, + "properties":{"shape":"StringMap"} + } + }, + "ProxyConfigurationType":{ + "type":"string", + "enum":["APPMESH"] + }, "RegistryAuthenticationData":{ "type":"structure", "members":{ @@ -485,7 +497,8 @@ "containerPath":{"shape":"String"}, "type":{"shape":"SecretType"}, "region":{"shape":"String"}, - "provider":{"shape":"SecretProvider"} + "provider":{"shape":"SecretProvider"}, + "target":{"shape":"SecretTarget"} } }, "SecretList":{ @@ -499,11 +512,19 @@ "asm" ] }, + "SecretTarget":{ + "type":"string", + "enum":[ + "CONTAINER", + "LOG_DRIVER" + ] + }, "SecretType":{ "type":"string", "enum":[ "ENVIRONMENT_VARIABLE", - "MOUNT_POINT" + "MOUNT_POINT", + "BOOTSTRAP" ] }, "SensitiveString":{ @@ -554,7 +575,8 @@ "memory":{"shape":"Integer"}, "associations":{"shape":"Associations"}, "pidMode":{"shape":"String"}, - "ipcMode":{"shape":"String"} + "ipcMode":{"shape":"String"}, + "proxyConfiguration":{"shape":"ProxyConfiguration"} } }, "TaskList":{ diff --git a/agent/acs/model/ecsacs/api.go b/agent/acs/model/ecsacs/api.go index 4ae7360d5da..7bb2d78abc8 100644 --- a/agent/acs/model/ecsacs/api.go +++ b/agent/acs/model/ecsacs/api.go @@ -206,7 +206,7 @@ type Container struct { Cpu *int64 `locationName:"cpu" type:"integer"` - DependsOn []*DependsOn `locationName:"dependsOn" type:"list"` + DependsOn []*ContainerDependency `locationName:"dependsOn" type:"list"` DockerConfig *DockerConfig `locationName:"dockerConfig" type:"structure"` @@ -255,21 +255,21 @@ func (s Container) GoString() string { return s.String() } -type DependsOn struct { +type ContainerDependency struct { _ struct{} `type:"structure"` - Condition *string `locationName:"condition" type:"string" enum:"ConditionType"` + Condition *string `locationName:"condition" type:"string" enum:"ContainerCondition"` - Container *string `locationName:"container" type:"string"` + ContainerName *string `locationName:"containerName" type:"string"` } // String returns the string representation -func (s DependsOn) String() string { +func (s ContainerDependency) String() string { return awsutil.Prettify(s) } // GoString returns the string representation -func (s DependsOn) GoString() string { +func (s ContainerDependency) GoString() string { return s.String() } @@ -909,6 +909,26 @@ func (s PortMapping) GoString() string { return s.String() } +type ProxyConfiguration struct { + _ struct{} `type:"structure"` + + ContainerName *string `locationName:"containerName" type:"string"` + + Properties map[string]*string `locationName:"properties" type:"map"` + + Type *string `locationName:"type" type:"string" enum:"ProxyConfigurationType"` +} + +// String returns the string representation +func (s ProxyConfiguration) String() string { + return awsutil.Prettify(s) +} + +// GoString returns the string representation +func (s ProxyConfiguration) GoString() string { + return s.String() +} + type RefreshTaskIAMRoleCredentialsInput struct { _ struct{} `type:"structure"` @@ -982,6 +1002,8 @@ type Secret struct { Region *string `locationName:"region" type:"string"` + Target *string `locationName:"target" type:"string" enum:"SecretTarget"` + Type *string `locationName:"type" type:"string" enum:"SecretType"` ValueFrom *string `locationName:"valueFrom" type:"string"` @@ -1104,6 +1126,8 @@ type Task struct { PidMode *string `locationName:"pidMode" type:"string"` + ProxyConfiguration *ProxyConfiguration `locationName:"proxyConfiguration" type:"structure"` + RoleCredentials *IAMRoleCredentials `locationName:"roleCredentials" type:"structure"` TaskDefinitionAccountId *string `locationName:"taskDefinitionAccountId" type:"string"` diff --git a/agent/api/container/container.go b/agent/api/container/container.go index e5cc798bb88..9af63565362 100644 --- a/agent/api/container/container.go +++ b/agent/api/container/container.go @@ -246,7 +246,7 @@ type Container struct { } type DependsOn struct { - Container string `json:"container"` + ContainerName string `json:"containerName"` Condition string `json:"condition"` } From 01b0505e0805334407f17d47ac306ba336a8867a Mon Sep 17 00:00:00 2001 From: Utsa Bhattacharjya Date: Sat, 23 Feb 2019 22:55:05 -0800 Subject: [PATCH 19/24] Accomodate ACS model change --- agent/acs/model/api/api-2.json | 29 ++--------------- agent/acs/model/ecsacs/api.go | 24 -------------- agent/api/task/task.go | 4 +-- agent/api/task/task_test.go | 8 ++--- agent/engine/dependencygraph/graph.go | 4 +-- agent/engine/dependencygraph/graph_test.go | 32 +++++++++---------- agent/engine/ordering_integ_test.go | 12 +++---- agent/statemanager/state_manager_test.go | 2 +- .../v20/containerOrdering/ecs_agent_data.json | 2 +- 9 files changed, 35 insertions(+), 82 deletions(-) diff --git a/agent/acs/model/api/api-2.json b/agent/acs/model/api/api-2.json index 3e53f13406e..3e3faedf8cc 100644 --- a/agent/acs/model/api/api-2.json +++ b/agent/acs/model/api/api-2.json @@ -455,18 +455,6 @@ "type":"list", "member":{"shape":"PortMapping"} }, - "ProxyConfiguration":{ - "type":"structure", - "members":{ - "type":{"shape":"ProxyConfigurationType"}, - "containerName":{"shape":"String"}, - "properties":{"shape":"StringMap"} - } - }, - "ProxyConfigurationType":{ - "type":"string", - "enum":["APPMESH"] - }, "RegistryAuthenticationData":{ "type":"structure", "members":{ @@ -497,8 +485,7 @@ "containerPath":{"shape":"String"}, "type":{"shape":"SecretType"}, "region":{"shape":"String"}, - "provider":{"shape":"SecretProvider"}, - "target":{"shape":"SecretTarget"} + "provider":{"shape":"SecretProvider"} } }, "SecretList":{ @@ -512,19 +499,10 @@ "asm" ] }, - "SecretTarget":{ - "type":"string", - "enum":[ - "CONTAINER", - "LOG_DRIVER" - ] - }, "SecretType":{ "type":"string", "enum":[ - "ENVIRONMENT_VARIABLE", - "MOUNT_POINT", - "BOOTSTRAP" + "ENVIRONMENT_VARIABLE" ] }, "SensitiveString":{ @@ -575,8 +553,7 @@ "memory":{"shape":"Integer"}, "associations":{"shape":"Associations"}, "pidMode":{"shape":"String"}, - "ipcMode":{"shape":"String"}, - "proxyConfiguration":{"shape":"ProxyConfiguration"} + "ipcMode":{"shape":"String"} } }, "TaskList":{ diff --git a/agent/acs/model/ecsacs/api.go b/agent/acs/model/ecsacs/api.go index 7bb2d78abc8..7908018ab90 100644 --- a/agent/acs/model/ecsacs/api.go +++ b/agent/acs/model/ecsacs/api.go @@ -909,26 +909,6 @@ func (s PortMapping) GoString() string { return s.String() } -type ProxyConfiguration struct { - _ struct{} `type:"structure"` - - ContainerName *string `locationName:"containerName" type:"string"` - - Properties map[string]*string `locationName:"properties" type:"map"` - - Type *string `locationName:"type" type:"string" enum:"ProxyConfigurationType"` -} - -// String returns the string representation -func (s ProxyConfiguration) String() string { - return awsutil.Prettify(s) -} - -// GoString returns the string representation -func (s ProxyConfiguration) GoString() string { - return s.String() -} - type RefreshTaskIAMRoleCredentialsInput struct { _ struct{} `type:"structure"` @@ -1002,8 +982,6 @@ type Secret struct { Region *string `locationName:"region" type:"string"` - Target *string `locationName:"target" type:"string" enum:"SecretTarget"` - Type *string `locationName:"type" type:"string" enum:"SecretType"` ValueFrom *string `locationName:"valueFrom" type:"string"` @@ -1126,8 +1104,6 @@ type Task struct { PidMode *string `locationName:"pidMode" type:"string"` - ProxyConfiguration *ProxyConfiguration `locationName:"proxyConfiguration" type:"structure"` - RoleCredentials *IAMRoleCredentials `locationName:"roleCredentials" type:"structure"` TaskDefinitionAccountId *string `locationName:"taskDefinitionAccountId" type:"string"` diff --git a/agent/api/task/task.go b/agent/api/task/task.go index a5536fe5203..a366965f3bf 100644 --- a/agent/api/task/task.go +++ b/agent/api/task/task.go @@ -1272,7 +1272,7 @@ func (task *Task) initializeContainerOrderingForVolumes() error { if _, ok := task.ContainerByName(volume.SourceContainer); !ok { return fmt.Errorf("could not find container with name %s", volume.SourceContainer) } - dependOn := apicontainer.DependsOn{Container: volume.SourceContainer, Condition: ContainerOrderingCreateCondition} + dependOn := apicontainer.DependsOn{ContainerName: volume.SourceContainer, Condition: ContainerOrderingCreateCondition} container.DependsOn = append(container.DependsOn, dependOn) } } @@ -1292,7 +1292,7 @@ func (task *Task) initializeContainerOrderingForLinks() error { if _, ok := task.ContainerByName(linkName); !ok { return fmt.Errorf("could not find container with name %s", linkName) } - dependOn := apicontainer.DependsOn{Container: linkName, Condition: ContainerOrderingStartCondition} + dependOn := apicontainer.DependsOn{ContainerName: linkName, Condition: ContainerOrderingStartCondition} container.DependsOn = append(container.DependsOn, dependOn) } } diff --git a/agent/api/task/task_test.go b/agent/api/task/task_test.go index 64236f5e01d..47737336aa9 100644 --- a/agent/api/task/task_test.go +++ b/agent/api/task/task_test.go @@ -2886,17 +2886,17 @@ func TestInitializeContainerOrderingWithLinksAndVolumesFrom(t *testing.T) { assert.NoError(t, err) containerResultWithVolume := task.Containers[0] - assert.Equal(t, "myName1", containerResultWithVolume.DependsOn[0].Container) + assert.Equal(t, "myName1", containerResultWithVolume.DependsOn[0].ContainerName) assert.Equal(t, ContainerOrderingCreateCondition, containerResultWithVolume.DependsOn[0].Condition) containerResultWithLink := task.Containers[1] - assert.Equal(t, "myName", containerResultWithLink.DependsOn[0].Container) + assert.Equal(t, "myName", containerResultWithLink.DependsOn[0].ContainerName) assert.Equal(t, ContainerOrderingStartCondition, containerResultWithLink.DependsOn[0].Condition) containerResultWithBothVolumeAndLink := task.Containers[2] - assert.Equal(t, "myName", containerResultWithBothVolumeAndLink.DependsOn[0].Container) + assert.Equal(t, "myName", containerResultWithBothVolumeAndLink.DependsOn[0].ContainerName) assert.Equal(t, ContainerOrderingCreateCondition, containerResultWithBothVolumeAndLink.DependsOn[0].Condition) - assert.Equal(t, "myName1", containerResultWithBothVolumeAndLink.DependsOn[1].Container) + assert.Equal(t, "myName1", containerResultWithBothVolumeAndLink.DependsOn[1].ContainerName) assert.Equal(t, ContainerOrderingStartCondition, containerResultWithBothVolumeAndLink.DependsOn[1].Condition) containerResultWithNoVolumeOrLink := task.Containers[3] diff --git a/agent/engine/dependencygraph/graph.go b/agent/engine/dependencygraph/graph.go index 28d482be0a4..f9853e8832d 100644 --- a/agent/engine/dependencygraph/graph.go +++ b/agent/engine/dependencygraph/graph.go @@ -218,7 +218,7 @@ func verifyContainerOrderingStatusResolvable(target *apicontainer.Container, exi } for _, dependency := range target.DependsOn { - dependencyContainer, ok := existingContainers[dependency.Container] + dependencyContainer, ok := existingContainers[dependency.ContainerName] if !ok { return nil, fmt.Errorf("dependency graph: container ordering dependency [%v] for target [%v] does not exist.", dependencyContainer, target) } @@ -423,7 +423,7 @@ func verifyShutdownOrder(target *apicontainer.Container, existingContainers map[ for _, dependency := range existingContainer.DependsOn { // If another container declares a dependency on our target, we will want to verify that the container is // stopped. - if dependency.Container == target.Name { + if dependency.ContainerName == target.Name { if !existingContainer.KnownTerminal() { missingShutdownDependencies = append(missingShutdownDependencies, existingContainer.Name) } diff --git a/agent/engine/dependencygraph/graph_test.go b/agent/engine/dependencygraph/graph_test.go index 6aab34d0f92..70053c38eb8 100644 --- a/agent/engine/dependencygraph/graph_test.go +++ b/agent/engine/dependencygraph/graph_test.go @@ -73,11 +73,11 @@ func TestValidDependencies(t *testing.T) { assert.True(t, resolveable, "One container should resolve trivially") // Webserver stack - php := steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: startCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - db := steadyStateContainer("db", []apicontainer.DependsOn{{Container: "dbdatavolume", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + php := steadyStateContainer("php", []apicontainer.DependsOn{{ContainerName: "db", Condition: startCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + db := steadyStateContainer("db", []apicontainer.DependsOn{{ContainerName: "dbdatavolume", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) dbdata := createdContainer("dbdatavolume", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) - webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{Container: "php", Condition: startCondition}, {Container: "htmldata", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{Container: "sharedcssfiles", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{ContainerName: "php", Condition: startCondition}, {ContainerName: "htmldata", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{ContainerName: "sharedcssfiles", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) sharedcssfiles := createdContainer("sharedcssfiles", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) task = &apitask.Task{ @@ -94,8 +94,8 @@ func TestValidDependenciesWithCycles(t *testing.T) { // Unresolveable: cycle task := &apitask.Task{ Containers: []*apicontainer.Container{ - steadyStateContainer("a", []apicontainer.DependsOn{{Container: "b", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), - steadyStateContainer("b", []apicontainer.DependsOn{{Container: "a", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), + steadyStateContainer("a", []apicontainer.DependsOn{{ContainerName: "b", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), + steadyStateContainer("b", []apicontainer.DependsOn{{ContainerName: "a", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), }, } resolveable := ValidDependencies(task) @@ -106,7 +106,7 @@ func TestValidDependenciesWithUnresolvedReference(t *testing.T) { // Unresolveable, reference doesn't exist task := &apitask.Task{ Containers: []*apicontainer.Container{ - steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), + steadyStateContainer("php", []apicontainer.DependsOn{{ContainerName: "db", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning), }, } resolveable := ValidDependencies(task) @@ -126,11 +126,11 @@ func TestDependenciesAreResolvedWhenSteadyStateIsRunning(t *testing.T) { assert.NoError(t, err, "One container should resolve trivially") // Webserver stack - php := steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: startCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - db := steadyStateContainer("db", []apicontainer.DependsOn{{Container: "dbdatavolume", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + php := steadyStateContainer("php", []apicontainer.DependsOn{{ContainerName: "db", Condition: startCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + db := steadyStateContainer("db", []apicontainer.DependsOn{{ContainerName: "dbdatavolume", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) dbdata := createdContainer("dbdatavolume", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) - webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{Container: "php", Condition: startCondition}, {Container: "htmldata", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{Container: "sharedcssfiles", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{ContainerName: "php", Condition: startCondition}, {ContainerName: "htmldata", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{ContainerName: "sharedcssfiles", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) sharedcssfiles := createdContainer("sharedcssfiles", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) task = &apitask.Task{ @@ -194,11 +194,11 @@ func TestRunDependencies(t *testing.T) { func TestRunDependenciesWhenSteadyStateIsResourcesProvisionedForOneContainer(t *testing.T) { // Webserver stack - php := steadyStateContainer("php", []apicontainer.DependsOn{{Container: "db", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - db := steadyStateContainer("db", []apicontainer.DependsOn{{Container: "dbdatavolume", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + php := steadyStateContainer("php", []apicontainer.DependsOn{{ContainerName: "db", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + db := steadyStateContainer("db", []apicontainer.DependsOn{{ContainerName: "dbdatavolume", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) dbdata := createdContainer("dbdatavolume", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) - webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{Container: "php", Condition: createCondition}, {Container: "htmldata", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) - htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{Container: "sharedcssfiles", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + webserver := steadyStateContainer("webserver", []apicontainer.DependsOn{{ContainerName: "php", Condition: createCondition}, {ContainerName: "htmldata", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) + htmldata := steadyStateContainer("htmldata", []apicontainer.DependsOn{{ContainerName: "sharedcssfiles", Condition: createCondition}}, apicontainerstatus.ContainerRunning, apicontainerstatus.ContainerRunning) sharedcssfiles := createdContainer("sharedcssfiles", []apicontainer.DependsOn{}, apicontainerstatus.ContainerRunning) // The Pause container, being added to the webserver stack pause := steadyStateContainer("pause", []apicontainer.DependsOn{}, apicontainerstatus.ContainerResourcesProvisioned, apicontainerstatus.ContainerResourcesProvisioned) @@ -888,7 +888,7 @@ func assertContainerOrderingHealthyConditionResolved(f func(target *apicontainer func dependsOn(vals ...string) []apicontainer.DependsOn { d := make([]apicontainer.DependsOn, len(vals)) for i, val := range vals { - d[i] = apicontainer.DependsOn{Container: val} + d[i] = apicontainer.DependsOn{ContainerName: val} } return d } diff --git a/agent/engine/ordering_integ_test.go b/agent/engine/ordering_integ_test.go index ed29af5129a..e843699e92d 100644 --- a/agent/engine/ordering_integ_test.go +++ b/agent/engine/ordering_integ_test.go @@ -43,7 +43,7 @@ func TestDependencyHealthCheck(t *testing.T) { parent.Command = []string{"exit 1"} parent.DependsOn = []apicontainer.DependsOn{ { - Container: "dependency", + ContainerName: "dependency", Condition: "HEALTHY", }, } @@ -96,7 +96,7 @@ func TestDependencyComplete(t *testing.T) { parent.Command = []string{"sleep 5 && exit 0"} parent.DependsOn = []apicontainer.DependsOn{ { - Container: "dependency", + ContainerName: "dependency", Condition: "COMPLETE", }, } @@ -149,7 +149,7 @@ func TestDependencySuccess(t *testing.T) { parent.Command = []string{"exit 0"} parent.DependsOn = []apicontainer.DependsOn{ { - Container: "dependency", + ContainerName: "dependency", Condition: "SUCCESS", }, } @@ -202,7 +202,7 @@ func TestDependencySuccessErrored(t *testing.T) { parent.Command = []string{"exit 0"} parent.DependsOn = []apicontainer.DependsOn{ { - Container: "dependency", + ContainerName: "dependency", Condition: "SUCCESS", }, } @@ -249,7 +249,7 @@ func TestDependencySuccessTimeout(t *testing.T) { parent.Command = []string{"exit 0"} parent.DependsOn = []apicontainer.DependsOn{ { - Container: "dependency", + ContainerName: "dependency", Condition: "SUCCESS", }, } @@ -299,7 +299,7 @@ func TestDependencyHealthyTimeout(t *testing.T) { parent.Command = []string{"exit 0"} parent.DependsOn = []apicontainer.DependsOn{ { - Container: "dependency", + ContainerName: "dependency", Condition: "HEALTHY", }, } diff --git a/agent/statemanager/state_manager_test.go b/agent/statemanager/state_manager_test.go index 16cc45bd09d..3dfe4332a7b 100644 --- a/agent/statemanager/state_manager_test.go +++ b/agent/statemanager/state_manager_test.go @@ -359,7 +359,7 @@ func TestLoadsDataForContainerOrdering(t *testing.T) { assert.Equal(t, 2, len(task.Containers)) dependsOn := task.Containers[1].DependsOn - assert.Equal(t, "container_1", dependsOn[0].Container) + assert.Equal(t, "container_1", dependsOn[0].ContainerName) assert.Equal(t, "START", dependsOn[0].Condition) } diff --git a/agent/statemanager/testdata/v20/containerOrdering/ecs_agent_data.json b/agent/statemanager/testdata/v20/containerOrdering/ecs_agent_data.json index 1df5b52de0f..38a27246687 100644 --- a/agent/statemanager/testdata/v20/containerOrdering/ecs_agent_data.json +++ b/agent/statemanager/testdata/v20/containerOrdering/ecs_agent_data.json @@ -69,7 +69,7 @@ "Name": "container_2", "DependsOn": [ { - "Container":"container_1", + "ContainerName":"container_1", "Condition":"START" } ], From 0c0a2e88e1a0445afb891408fe646bb7fba00998 Mon Sep 17 00:00:00 2001 From: Derek Petersen Date: Tue, 26 Feb 2019 00:36:31 -0800 Subject: [PATCH 20/24] tests: prefer cached images This should eventually make windows tests faster to run. Fixes a bug where task context cancel causes an infinite steady state loop. Previously if the context expired, waitSteady() will spin forever since the timeout no longer works. This introduces a check for context expiration earlier in the code. --- agent/engine/common_integ_test.go | 1 + agent/engine/common_unix_integ_test.go | 6 ++---- agent/engine/ordering_integ_test.go | 18 +++++++++--------- agent/engine/task_manager.go | 10 ++++++++-- agent/functional_tests/util/utils_windows.go | 1 + agent/stats/common_test.go | 1 + 6 files changed, 22 insertions(+), 15 deletions(-) diff --git a/agent/engine/common_integ_test.go b/agent/engine/common_integ_test.go index 174c4e3b8cb..d0755a69c13 100644 --- a/agent/engine/common_integ_test.go +++ b/agent/engine/common_integ_test.go @@ -41,6 +41,7 @@ import ( func defaultTestConfigIntegTest() *config.Config { cfg, _ := config.NewConfig(ec2.NewBlackholeEC2MetadataClient()) cfg.TaskCPUMemLimit = config.ExplicitlyDisabled + cfg.ImagePullBehavior = config.ImagePullPreferCachedBehavior return cfg } diff --git a/agent/engine/common_unix_integ_test.go b/agent/engine/common_unix_integ_test.go index b6d7ec8ff2d..dbf76d2e809 100644 --- a/agent/engine/common_unix_integ_test.go +++ b/agent/engine/common_unix_integ_test.go @@ -32,8 +32,6 @@ func createTestContainer() *apicontainer.Container { } func isDockerRunning() bool { - if _, err := os.Stat("/var/run/docker.sock"); err != nil { - return false - } - return true + _, err := os.Stat("/var/run/docker.sock") + return err == nil } diff --git a/agent/engine/ordering_integ_test.go b/agent/engine/ordering_integ_test.go index e843699e92d..0e1d857b2ea 100644 --- a/agent/engine/ordering_integ_test.go +++ b/agent/engine/ordering_integ_test.go @@ -23,7 +23,7 @@ import ( "time" ) -const orderingTimeout = 60 * time.Second +const orderingTimeout = 90 * time.Second // TestDependencyHealthCheck is a happy-case integration test that considers a workflow with a HEALTHY dependency // condition. We ensure that the task can be both started and stopped. @@ -40,16 +40,16 @@ func TestDependencyHealthCheck(t *testing.T) { dependency := createTestContainerWithImageAndName(baseImageForOS, "dependency") parent.EntryPoint = &entryPointForOS - parent.Command = []string{"exit 1"} + parent.Command = []string{"sleep 5 && exit 1"} parent.DependsOn = []apicontainer.DependsOn{ { ContainerName: "dependency", - Condition: "HEALTHY", + Condition: "HEALTHY", }, } dependency.EntryPoint = &entryPointForOS - dependency.Command = []string{"sleep 30"} + dependency.Command = []string{"sleep 60 && exit 0"} dependency.HealthCheckType = apicontainer.DockerHealthCheckType dependency.DockerConfig.Config = aws.String(alwaysHealthyHealthCheckConfig) @@ -97,7 +97,7 @@ func TestDependencyComplete(t *testing.T) { parent.DependsOn = []apicontainer.DependsOn{ { ContainerName: "dependency", - Condition: "COMPLETE", + Condition: "COMPLETE", }, } @@ -150,7 +150,7 @@ func TestDependencySuccess(t *testing.T) { parent.DependsOn = []apicontainer.DependsOn{ { ContainerName: "dependency", - Condition: "SUCCESS", + Condition: "SUCCESS", }, } @@ -203,7 +203,7 @@ func TestDependencySuccessErrored(t *testing.T) { parent.DependsOn = []apicontainer.DependsOn{ { ContainerName: "dependency", - Condition: "SUCCESS", + Condition: "SUCCESS", }, } @@ -250,7 +250,7 @@ func TestDependencySuccessTimeout(t *testing.T) { parent.DependsOn = []apicontainer.DependsOn{ { ContainerName: "dependency", - Condition: "SUCCESS", + Condition: "SUCCESS", }, } @@ -300,7 +300,7 @@ func TestDependencyHealthyTimeout(t *testing.T) { parent.DependsOn = []apicontainer.DependsOn{ { ContainerName: "dependency", - Condition: "HEALTHY", + Condition: "HEALTHY", }, } diff --git a/agent/engine/task_manager.go b/agent/engine/task_manager.go index bfcf50b6020..853ab4fb295 100644 --- a/agent/engine/task_manager.go +++ b/agent/engine/task_manager.go @@ -313,8 +313,14 @@ func (mtask *managedTask) waitSteady() { // steadyState returns if the task is in a steady state. Steady state is when task's desired // and known status are both RUNNING func (mtask *managedTask) steadyState() bool { - taskKnownStatus := mtask.GetKnownStatus() - return taskKnownStatus == apitaskstatus.TaskRunning && taskKnownStatus >= mtask.GetDesiredStatus() + select { + case <-mtask.ctx.Done(): + seelog.Info("Context expired. No longer steady.") + return false + default: + taskKnownStatus := mtask.GetKnownStatus() + return taskKnownStatus == apitaskstatus.TaskRunning && taskKnownStatus >= mtask.GetDesiredStatus() + } } // cleanupCredentials removes credentials for a stopped task diff --git a/agent/functional_tests/util/utils_windows.go b/agent/functional_tests/util/utils_windows.go index f1dfcbe0df7..4bd388b32b7 100644 --- a/agent/functional_tests/util/utils_windows.go +++ b/agent/functional_tests/util/utils_windows.go @@ -93,6 +93,7 @@ func RunAgent(t *testing.T, options *AgentOptions) *TestAgent { os.Setenv("ECS_AUDIT_LOGFILE", logdir+"/audit.log") os.Setenv("ECS_LOGLEVEL", "debug") os.Setenv("ECS_AVAILABLE_LOGGING_DRIVERS", `["json-file","awslogs"]`) + os.Setenv("ECS_IMAGE_PULL_BEHAVIOR", "prefer-cached") t.Log("datadir", datadir) os.Mkdir(logdir, 0755) diff --git a/agent/stats/common_test.go b/agent/stats/common_test.go index 1a3db782a75..652eeaf0a4e 100644 --- a/agent/stats/common_test.go +++ b/agent/stats/common_test.go @@ -63,6 +63,7 @@ var ( func init() { cfg.EngineAuthData = config.NewSensitiveRawMessage([]byte{}) + cfg.ImagePullBehavior = config.ImagePullPreferCachedBehavior } // eventStream returns the event stream used to receive container change events From d986f361d7c844871fea1fdbfd18e5d8b55f6bf4 Mon Sep 17 00:00:00 2001 From: Derek Petersen Date: Tue, 26 Feb 2019 22:47:58 -0800 Subject: [PATCH 21/24] Adding shutdown order test --- agent/engine/ordering_integ_test.go | 101 +++++++++++++++++++++++++++- 1 file changed, 100 insertions(+), 1 deletion(-) diff --git a/agent/engine/ordering_integ_test.go b/agent/engine/ordering_integ_test.go index 0e1d857b2ea..63f61776ecb 100644 --- a/agent/engine/ordering_integ_test.go +++ b/agent/engine/ordering_integ_test.go @@ -17,10 +17,13 @@ package engine import ( "testing" + "time" + "github.com/aws/amazon-ecs-agent/agent/api" apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" + "github.com/aws/amazon-ecs-agent/agent/api/container/status" "github.com/aws/aws-sdk-go/aws" - "time" + "github.com/stretchr/testify/assert" ) const orderingTimeout = 90 * time.Second @@ -349,3 +352,99 @@ func waitFinished(t *testing.T, finished <-chan interface{}, duration time.Durat t.FailNow() } } + +// TestShutdownOrder +func TestShutdownOrder(t *testing.T) { + taskEngine, done, _ := setupWithDefaultConfig(t) + defer done() + + stateChangeEvents := taskEngine.StateChangeEvents() + + taskArn := "testShutdownOrder" + testTask := createTestTask(taskArn) + + parent := createTestContainerWithImageAndName(baseImageForOS, "parent") + A := createTestContainerWithImageAndName(baseImageForOS, "parent") + B := createTestContainerWithImageAndName(baseImageForOS, "parent") + C := createTestContainerWithImageAndName(baseImageForOS, "parent") + + parent.EntryPoint = &entryPointForOS + parent.Command = []string{"exit 0"} + parent.Essential = true + parent.DependsOn = []apicontainer.DependsOn{ + { + ContainerName: "A", + Condition: "START", + }, + { + ContainerName: "B", + Condition: "START", + }, + { + ContainerName: "C", + Condition: "START", + }, + } + + A.EntryPoint = &entryPointForOS + A.Command = []string{"sleep 1000"} + A.DependsOn = []apicontainer.DependsOn{ + { + ContainerName: "B", + Condition: "START", + }, + } + + B.EntryPoint = &entryPointForOS + B.Command = []string{"sleep 1000"} + B.DependsOn = []apicontainer.DependsOn{ + { + ContainerName: "C", + Condition: "START", + }, + } + + C.EntryPoint = &entryPointForOS + C.Command = []string{"sleep 1000"} + + testTask.Containers = []*apicontainer.Container{ + parent, + A, + B, + C, + } + + go taskEngine.AddTask(testTask) + + finished := make(chan interface{}) + go func() { + // Everything should first progress to running + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerRunningStateChange(t, taskEngine) + verifyTaskIsRunning(stateChangeEvents, testTask) + + // The shutdown order will now proceed. Parent will exit first since it has an explicit exit command. + event := <-stateChangeEvents + assert.Equal(t, event.(api.ContainerStateChange).Status, status.ContainerStopped) + assert.Equal(t, event.(api.ContainerStateChange).ContainerName, "parent") + + // The dependency chain is A -> B -> C. We expect the inverse order to be followed for shutdown: + // C shuts down, then B, then A + expectedC := <-stateChangeEvents + assert.Equal(t, expectedC.(api.ContainerStateChange).Status, status.ContainerStopped) + assert.Equal(t, expectedC.(api.ContainerStateChange).ContainerName, "C") + + expectedB := <-stateChangeEvents + assert.Equal(t, expectedB.(api.ContainerStateChange).Status, status.ContainerStopped) + assert.Equal(t, expectedB.(api.ContainerStateChange).ContainerName, "B") + + expectedA := <-stateChangeEvents + assert.Equal(t, expectedA.(api.ContainerStateChange).Status, status.ContainerStopped) + assert.Equal(t, expectedA.(api.ContainerStateChange).ContainerName, "A") + + close(finished) + }() + +} From e34ceb75728fabf179cad6a8d3c54c0455e5591d Mon Sep 17 00:00:00 2001 From: Utsa Bhattacharjya Date: Mon, 25 Feb 2019 16:21:38 -0800 Subject: [PATCH 22/24] Adding Integ Tests for Granular Stop Timeout --- agent/engine/ordering_integ_test.go | 31 +++++----- agent/engine/ordering_integ_unix_test.go | 74 ++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 15 deletions(-) diff --git a/agent/engine/ordering_integ_test.go b/agent/engine/ordering_integ_test.go index 63f61776ecb..3c76b4fbdfc 100644 --- a/agent/engine/ordering_integ_test.go +++ b/agent/engine/ordering_integ_test.go @@ -78,7 +78,6 @@ func TestDependencyHealthCheck(t *testing.T) { }() waitFinished(t, finished, orderingTimeout) - } // TestDependencyComplete validates that the COMPLETE dependency condition will resolve when the child container exits @@ -96,7 +95,7 @@ func TestDependencyComplete(t *testing.T) { dependency := createTestContainerWithImageAndName(baseImageForOS, "dependency") parent.EntryPoint = &entryPointForOS - parent.Command = []string{"sleep 5 && exit 0"} + parent.Command = []string{"sleep 5"} parent.DependsOn = []apicontainer.DependsOn{ { ContainerName: "dependency", @@ -158,7 +157,7 @@ func TestDependencySuccess(t *testing.T) { } dependency.EntryPoint = &entryPointForOS - dependency.Command = []string{"sleep 10 && exit 0"} + dependency.Command = []string{"sleep 10"} dependency.Essential = false testTask.Containers = []*apicontainer.Container{ @@ -258,7 +257,7 @@ func TestDependencySuccessTimeout(t *testing.T) { } dependency.EntryPoint = &entryPointForOS - dependency.Command = []string{"sleep 15 && exit 0"} + dependency.Command = []string{"sleep 15"} dependency.Essential = false // set the timeout to be shorter than the amount of time it takes to stop @@ -342,17 +341,6 @@ func TestDependencyHealthyTimeout(t *testing.T) { waitFinished(t, finished, orderingTimeout) } -func waitFinished(t *testing.T, finished <-chan interface{}, duration time.Duration) { - select { - case <-finished: - t.Log("Finished successfully.") - return - case <-time.After(90 * time.Second): - t.Error("timed out after: ", duration) - t.FailNow() - } -} - // TestShutdownOrder func TestShutdownOrder(t *testing.T) { taskEngine, done, _ := setupWithDefaultConfig(t) @@ -447,4 +435,17 @@ func TestShutdownOrder(t *testing.T) { close(finished) }() + waitFinished(t, finished, orderingTimeout) +} + + +func waitFinished(t *testing.T, finished <-chan interface{}, duration time.Duration) { + select { + case <-finished: + t.Log("Finished successfully.") + return + case <-time.After(90 * time.Second): + t.Error("timed out after: ", duration) + t.FailNow() + } } diff --git a/agent/engine/ordering_integ_unix_test.go b/agent/engine/ordering_integ_unix_test.go index d168cf8828a..b392c43d42e 100644 --- a/agent/engine/ordering_integ_unix_test.go +++ b/agent/engine/ordering_integ_unix_test.go @@ -15,6 +15,13 @@ package engine +import ( + "testing" + + apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" + "github.com/stretchr/testify/assert" +) + const ( baseImageForOS = testRegistryHost + "/" + "busybox" ) @@ -22,3 +29,70 @@ const ( var ( entryPointForOS = []string{"sh", "-c"} ) + +func TestGranularStopTimeout(t *testing.T) { + taskEngine, done, _ := setupWithDefaultConfig(t) + defer done() + + stateChangeEvents := taskEngine.StateChangeEvents() + + taskArn := "TestGranularStopTimeout" + testTask := createTestTask(taskArn) + + parent := createTestContainerWithImageAndName(baseImageForOS, "parent") + dependency1 := createTestContainerWithImageAndName(baseImageForOS, "dependency1") + dependency2 := createTestContainerWithImageAndName(baseImageForOS, "dependency2") + + parent.EntryPoint = &entryPointForOS + parent.Command = []string{"sleep 30"} + parent.Essential = true + parent.DependsOn = []apicontainer.DependsOn{ + { + ContainerName: "dependency1", + Condition: "START", + }, + { + ContainerName: "dependency2", + Condition: "START", + }, + } + + dependency1.EntryPoint = &entryPointForOS + dependency1.Command = []string{"trap 'echo caught' SIGTERM; sleep 60"} + dependency1.StopTimeout = 5 + + dependency2.EntryPoint = &entryPointForOS + dependency2.Command = []string{"trap 'echo caught' SIGTERM; sleep 60"} + dependency2.StopTimeout = 50 + + testTask.Containers = []*apicontainer.Container{ + dependency1, + dependency2, + parent, + } + + go taskEngine.AddTask(testTask) + + finished := make(chan interface{}) + go func() { + + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerRunningStateChange(t, taskEngine) + verifyContainerRunningStateChange(t, taskEngine) + + verifyTaskIsRunning(stateChangeEvents, testTask) + + verifyContainerStoppedStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) + verifyContainerStoppedStateChange(t, taskEngine) + + verifyTaskIsStopped(stateChangeEvents, testTask) + + assert.Equal(t, 137, *testTask.Containers[0].GetKnownExitCode(), "Dependency1 should exit with code 137") + assert.Equal(t, 0, *testTask.Containers[1].GetKnownExitCode(), "Dependency2 should exit with code 0") + + close(finished) + }() + + waitFinished(t, finished, orderingTimeout) +} From bc3f30c1a7c80b4a42a4844ec718ad8a8b60b184 Mon Sep 17 00:00:00 2001 From: Utsa Bhattacharjya Date: Wed, 27 Feb 2019 13:09:45 -0800 Subject: [PATCH 23/24] Fix Shutdown Order test --- agent/engine/ordering_integ_test.go | 33 ++++++++++++----------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/agent/engine/ordering_integ_test.go b/agent/engine/ordering_integ_test.go index 3c76b4fbdfc..7de2939e4c0 100644 --- a/agent/engine/ordering_integ_test.go +++ b/agent/engine/ordering_integ_test.go @@ -343,6 +343,7 @@ func TestDependencyHealthyTimeout(t *testing.T) { // TestShutdownOrder func TestShutdownOrder(t *testing.T) { + shutdownOrderingTimeout := 120 * time.Second taskEngine, done, _ := setupWithDefaultConfig(t) defer done() @@ -352,30 +353,22 @@ func TestShutdownOrder(t *testing.T) { testTask := createTestTask(taskArn) parent := createTestContainerWithImageAndName(baseImageForOS, "parent") - A := createTestContainerWithImageAndName(baseImageForOS, "parent") - B := createTestContainerWithImageAndName(baseImageForOS, "parent") - C := createTestContainerWithImageAndName(baseImageForOS, "parent") + A := createTestContainerWithImageAndName(baseImageForOS, "A") + B := createTestContainerWithImageAndName(baseImageForOS, "B") + C := createTestContainerWithImageAndName(baseImageForOS, "C") parent.EntryPoint = &entryPointForOS - parent.Command = []string{"exit 0"} + parent.Command = []string{"echo hi"} parent.Essential = true parent.DependsOn = []apicontainer.DependsOn{ { ContainerName: "A", Condition: "START", }, - { - ContainerName: "B", - Condition: "START", - }, - { - ContainerName: "C", - Condition: "START", - }, } A.EntryPoint = &entryPointForOS - A.Command = []string{"sleep 1000"} + A.Command = []string{"sleep 100"} A.DependsOn = []apicontainer.DependsOn{ { ContainerName: "B", @@ -384,7 +377,7 @@ func TestShutdownOrder(t *testing.T) { } B.EntryPoint = &entryPointForOS - B.Command = []string{"sleep 1000"} + B.Command = []string{"sleep 100"} B.DependsOn = []apicontainer.DependsOn{ { ContainerName: "C", @@ -393,7 +386,7 @@ func TestShutdownOrder(t *testing.T) { } C.EntryPoint = &entryPointForOS - C.Command = []string{"sleep 1000"} + C.Command = []string{"sleep 100"} testTask.Containers = []*apicontainer.Container{ parent, @@ -419,10 +412,10 @@ func TestShutdownOrder(t *testing.T) { assert.Equal(t, event.(api.ContainerStateChange).ContainerName, "parent") // The dependency chain is A -> B -> C. We expect the inverse order to be followed for shutdown: - // C shuts down, then B, then A + // A shuts down, then B, then C expectedC := <-stateChangeEvents assert.Equal(t, expectedC.(api.ContainerStateChange).Status, status.ContainerStopped) - assert.Equal(t, expectedC.(api.ContainerStateChange).ContainerName, "C") + assert.Equal(t, expectedC.(api.ContainerStateChange).ContainerName, "A") expectedB := <-stateChangeEvents assert.Equal(t, expectedB.(api.ContainerStateChange).Status, status.ContainerStopped) @@ -430,12 +423,12 @@ func TestShutdownOrder(t *testing.T) { expectedA := <-stateChangeEvents assert.Equal(t, expectedA.(api.ContainerStateChange).Status, status.ContainerStopped) - assert.Equal(t, expectedA.(api.ContainerStateChange).ContainerName, "A") + assert.Equal(t, expectedA.(api.ContainerStateChange).ContainerName, "C") close(finished) }() - waitFinished(t, finished, orderingTimeout) + waitFinished(t, finished, shutdownOrderingTimeout) } @@ -444,7 +437,7 @@ func waitFinished(t *testing.T, finished <-chan interface{}, duration time.Durat case <-finished: t.Log("Finished successfully.") return - case <-time.After(90 * time.Second): + case <-time.After(duration): t.Error("timed out after: ", duration) t.FailNow() } From c3b851402eec15e132dab427157b8329f1fcd9d5 Mon Sep 17 00:00:00 2001 From: Derek Petersen Date: Wed, 27 Feb 2019 01:55:15 -0800 Subject: [PATCH 24/24] integ tests: allocate more cpu per container The default helper function will now allocate 1024 cpu shares or 100% cpu-percent on windows. This will enable the windows based tests to finish in more predictable ways. When Windows tests were constrained, simple tasks liek "sleep 10" were taking much longer than the expected 10 seconds. --- agent/engine/common_integ_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/engine/common_integ_test.go b/agent/engine/common_integ_test.go index d0755a69c13..262c57de662 100644 --- a/agent/engine/common_integ_test.go +++ b/agent/engine/common_integ_test.go @@ -108,8 +108,8 @@ func createTestContainerWithImageAndName(image string, name string) *apicontaine Command: []string{}, Essential: true, DesiredStatusUnsafe: apicontainerstatus.ContainerRunning, - CPU: 100, - Memory: 80, + CPU: 1024, + Memory: 128, } }