From 6ef726650fd4fe14be35177cb4575720d0f8ffcd Mon Sep 17 00:00:00 2001 From: Utsa Bhattacharjya Date: Sat, 23 Feb 2019 22:54:14 -0800 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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 4/4] 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) + }() + +}