From 1788efa01e3e10ddce0b39e35653ce900d1072f3 Mon Sep 17 00:00:00 2001 From: Peng Yin Date: Tue, 30 Jan 2018 00:28:30 +0000 Subject: [PATCH] test: combine the test with table driven tests --- agent/api/task.go | 2 + agent/engine/docker_task_engine.go | 4 +- agent/engine/docker_task_engine_test.go | 256 ++++++++---------- .../tests/functionaltests_test.go | 1 + 4 files changed, 111 insertions(+), 152 deletions(-) diff --git a/agent/api/task.go b/agent/api/task.go index 5aa79f4eb03..36ef2e28512 100644 --- a/agent/api/task.go +++ b/agent/api/task.go @@ -1104,6 +1104,8 @@ func (task *Task) GetID() (string, error) { return resourceSplit[1], nil } +// RecordExecutionStoppedAt checks if this is an essential container stopped +// and set the task executionStoppedAt timestamps func (task *Task) RecordExecutionStoppedAt(container *Container) { if !container.Essential { return diff --git a/agent/engine/docker_task_engine.go b/agent/engine/docker_task_engine.go index 6ff8d2d02b9..40e4cda9ffb 100644 --- a/agent/engine/docker_task_engine.go +++ b/agent/engine/docker_task_engine.go @@ -37,7 +37,6 @@ import ( utilsync "github.com/aws/amazon-ecs-agent/agent/utils/sync" "github.com/aws/amazon-ecs-agent/agent/utils/ttime" - "github.com/aws/aws-sdk-go/aws" "github.com/cihub/seelog" "github.com/pkg/errors" "golang.org/x/net/context" @@ -291,6 +290,7 @@ func (engine *DockerTaskEngine) synchronizeState() { engine.saver.Save() } +// updateContainerMetadata sets the container metadata from the docker inspect func updateContainerMetadata(metadata *DockerContainerMetadata, container *api.Container, task *api.Task) { container.SetCreatedAt(metadata.CreatedAt) container.SetStartedAt(metadata.StartedAt) @@ -332,7 +332,7 @@ func (engine *DockerTaskEngine) synchronizeContainerStatus(container *api.Docker seelog.Warnf("Task engine [%s]: could not find matching container for expected name [%s]: %v", task.Arn, container.DockerName, err) } else { - // update the container metadata in case the container status/metadata changed during agent restart + // update the container metadata in case the container was created during agent restart metadata := metadataFromContainer(describedContainer) updateContainerMetadata(&metadata, container.Container, task) container.DockerID = describedContainer.ID diff --git a/agent/engine/docker_task_engine_test.go b/agent/engine/docker_task_engine_test.go index f8c0fd3521a..a91ff9ea134 100644 --- a/agent/engine/docker_task_engine_test.go +++ b/agent/engine/docker_task_engine_test.go @@ -1721,169 +1721,125 @@ func TestHandleDockerHealthEvent(t *testing.T) { assert.Equal(t, testContainer.Health.Status, api.ContainerHealthy) } -func TestCreatedContainerMetadataUpdateOnRestart(t *testing.T) { - ctx, cancel := context.WithCancel(context.TODO()) - defer cancel() - ctrl, client, _, taskEngine, _, imageManager, _ := mocks(t, ctx, &defaultConfig) - defer ctrl.Finish() - - dockerID := "dockerID" - dockerContainer := &api.DockerContainer{ - DockerName: "c1", - Container: &api.Container{ - MountPoints: []api.MountPoint{ - { - SourceVolume: "empty", - ContainerPath: "container", - }, - }, - }, - } +func TestContainerMetadataUpdatedOnRestart(t *testing.T) { + dockerID := "dockerID_created" labels := map[string]string{ "name": "metadata", } - created := time.Now() - volumes := map[string]string{ - "container": "tmp", - } - emptyVolume := &api.EmptyHostVolume{} - task := &api.Task{ - Volumes: []api.TaskVolume{ - { - Name: "empty", - Volume: emptyVolume, - }, + testCases := []struct { + stage string + status api.ContainerStatus + created time.Time + started time.Time + finished time.Time + portBindings []api.PortBinding + exitCode *int + err DockerStateError + }{ + { + stage: "created", + status: api.ContainerCreated, + created: time.Now(), }, - } - - gomock.InOrder( - client.EXPECT().InspectContainer("c1", gomock.Any()).Return(&docker.Container{ - ID: dockerID, - Config: &docker.Config{ - Labels: labels, + { + stage: "started", + status: api.ContainerRunning, + started: time.Now(), + portBindings: []api.PortBinding{ + { + ContainerPort: 80, + HostPort: 80, + BindIP: "0.0.0.0/0", + Protocol: api.TransportProtocolTCP, + }, }, - Created: created, - Volumes: volumes, - }, nil), - imageManager.EXPECT().RecordContainerReference(dockerContainer.Container), - ) - taskEngine.(*DockerTaskEngine).synchronizeContainerStatus(dockerContainer, task) - assert.Equal(t, dockerID, dockerContainer.DockerID) - assert.Equal(t, created, dockerContainer.Container.GetCreatedAt()) - assert.Equal(t, labels, dockerContainer.Container.GetLabels()) - assert.Equal(t, "tmp", task.Volumes[0].Volume.SourcePath()) -} - -func TestStartedContainerMetadataUpdateOnRestart(t *testing.T) { - ctx, cancel := context.WithCancel(context.TODO()) - defer cancel() - ctrl, client, _, taskEngine, _, imageManager, _ := mocks(t, ctx, &defaultConfig) - defer ctrl.Finish() - - dockerID := "1234" - dockerContainer := &api.DockerContainer{ - DockerID: dockerID, - DockerName: "c1", - Container: &api.Container{}, - } - - portBindings := []api.PortBinding{ + }, { - ContainerPort: 80, - HostPort: 80, - BindIP: "0.0.0.0/0", - Protocol: api.TransportProtocolTCP, + stage: "stopped", + finished: time.Now(), + exitCode: aws.Int(1), }, - } - labels := map[string]string{ - "name": "metadata", - } - startedAt := time.Now() - gomock.InOrder( - client.EXPECT().DescribeContainer(dockerID).Return(api.ContainerRunning, - DockerContainerMetadata{ - Labels: labels, - DockerID: dockerID, - StartedAt: startedAt, - PortBindings: portBindings, - }), - imageManager.EXPECT().RecordContainerReference(dockerContainer.Container), - ) - taskEngine.(*DockerTaskEngine).synchronizeContainerStatus(dockerContainer, nil) - assert.Equal(t, startedAt, dockerContainer.Container.GetStartedAt()) - assert.Equal(t, labels, dockerContainer.Container.GetLabels()) - assert.Equal(t, uint16(80), dockerContainer.Container.KnownPortBindings[0].ContainerPort) -} - -func TestStoppedContainerMetadataUpdateOnRestart(t *testing.T) { - ctx, cancel := context.WithCancel(context.TODO()) - defer cancel() - ctrl, client, _, taskEngine, _, imageManager, _ := mocks(t, ctx, &defaultConfig) - defer ctrl.Finish() - - dockerID := "1234" - dockerContainer := &api.DockerContainer{ - DockerID: dockerID, - DockerName: "c1", - Container: &api.Container{ - Essential: true, + { + stage: "failed", + status: api.ContainerStopped, + err: NewDockerStateError("error"), + exitCode: aws.Int(1), }, } - task := &api.Task{} - labels := map[string]string{ - "name": "metadata", - } - finishedAt := time.Now() - gomock.InOrder( - client.EXPECT().DescribeContainer(dockerID).Return(api.ContainerStopped, - DockerContainerMetadata{ - Labels: labels, - DockerID: dockerID, - FinishedAt: finishedAt, - ExitCode: aws.Int(1), - }), - imageManager.EXPECT().RecordContainerReference(dockerContainer.Container), - ) - taskEngine.(*DockerTaskEngine).synchronizeContainerStatus(dockerContainer, task) - assert.Equal(t, finishedAt, dockerContainer.Container.GetFinishedAt()) - assert.Equal(t, labels, dockerContainer.Container.GetLabels()) - assert.Equal(t, 1, aws.IntValue(dockerContainer.Container.GetKnownExitCode())) - assert.False(t, task.GetExecutionStoppedAt().IsZero()) -} + for _, tc := range testCases { + t.Run(fmt.Sprintf("Agent restarted during container: %s", tc.stage), func(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() -func TestErroredContainerMetadataUpdateOnRestart(t *testing.T) { - ctx, cancel := context.WithCancel(context.TODO()) - defer cancel() - ctrl, client, _, taskEngine, _, _, _ := mocks(t, ctx, &defaultConfig) - defer ctrl.Finish() + ctrl, client, _, taskEngine, _, imageManager, _ := mocks(t, ctx, &defaultConfig) + defer ctrl.Finish() + dockerContainer := &api.DockerContainer{ + DockerID: dockerID, + DockerName: fmt.Sprintf("docker%s", tc.stage), + Container: &api.Container{}, + } + task := &api.Task{} - dockerID := "1234" - dockerContainer := &api.DockerContainer{ - DockerID: dockerID, - DockerName: "c1", - Container: &api.Container{}, - } - task := &api.Task{} + if tc.stage == "created" { + dockerContainer.Container.MountPoints = []api.MountPoint{ + { + SourceVolume: "empty", + ContainerPath: "container", + }, + } + dockerContainer.DockerID = "" + task.Volumes = []api.TaskVolume{ + { + Name: "empty", + Volume: &api.EmptyHostVolume{}, + }, + } + client.EXPECT().InspectContainer(dockerContainer.DockerName, gomock.Any()).Return(&docker.Container{ + ID: dockerID, + Config: &docker.Config{ + Labels: labels, + }, + Created: tc.created, + Volumes: map[string]string{ + "container": "tmp", + }, + }, nil) + imageManager.EXPECT().RecordContainerReference(dockerContainer.Container).AnyTimes() + } else { + client.EXPECT().DescribeContainer(dockerID).Return(tc.status, DockerContainerMetadata{ + Labels: labels, + DockerID: dockerID, + CreatedAt: tc.created, + StartedAt: tc.started, + FinishedAt: tc.finished, + PortBindings: tc.portBindings, + ExitCode: tc.exitCode, + Error: tc.err, + }) + imageManager.EXPECT().RecordContainerReference(dockerContainer.Container).AnyTimes() + } - labels := map[string]string{ - "name": "metadata", + taskEngine.(*DockerTaskEngine).synchronizeContainerStatus(dockerContainer, task) + assert.Equal(t, labels, dockerContainer.Container.GetLabels()) + assert.Equal(t, tc.created, dockerContainer.Container.GetCreatedAt()) + assert.Equal(t, tc.started, dockerContainer.Container.GetStartedAt()) + assert.Equal(t, tc.finished, dockerContainer.Container.GetFinishedAt()) + if tc.stage == "created" { + assert.Equal(t, "tmp", task.Volumes[0].Volume.SourcePath()) + } + if tc.stage == "started" { + assert.Equal(t, uint16(80), dockerContainer.Container.KnownPortBindings[0].ContainerPort) + } + if tc.stage == "finished" { + assert.False(t, task.GetExecutionStoppedAt().IsZero()) + assert.Equal(t, tc.exitCode, dockerContainer.Container.GetKnownExitCode()) + } + if tc.stage == "failed" { + assert.Equal(t, tc.exitCode, dockerContainer.Container.GetKnownExitCode()) + assert.NotNil(t, dockerContainer.Container.ApplyingError) + } + }) } - finishedAt := time.Now() - gomock.InOrder( - client.EXPECT().DescribeContainer(dockerID).Return(api.ContainerStopped, - DockerContainerMetadata{ - Labels: labels, - DockerID: dockerID, - FinishedAt: finishedAt, - Error: NewDockerStateError("failed"), - ExitCode: aws.Int(1), - }), - ) - taskEngine.(*DockerTaskEngine).synchronizeContainerStatus(dockerContainer, task) - assert.Equal(t, finishedAt, dockerContainer.Container.GetFinishedAt()) - assert.Equal(t, labels, dockerContainer.Container.GetLabels()) - assert.Equal(t, 1, aws.IntValue(dockerContainer.Container.GetKnownExitCode())) - assert.Error(t, dockerContainer.Container.ApplyingError) } diff --git a/agent/functional_tests/tests/functionaltests_test.go b/agent/functional_tests/tests/functionaltests_test.go index 3b714eb08f1..9feb7ffb3fd 100644 --- a/agent/functional_tests/tests/functionaltests_test.go +++ b/agent/functional_tests/tests/functionaltests_test.go @@ -27,6 +27,7 @@ import ( ecsapi "github.com/aws/amazon-ecs-agent/agent/ecs_client/model/ecs" . "github.com/aws/amazon-ecs-agent/agent/functional_tests/util" + "github.com/aws/aws-sdk-go/aws/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/cloudwatchlogs" "github.com/stretchr/testify/assert"