diff --git a/CHANGELOG.md b/CHANGELOG.md index 318494b77c0..84f5693bb97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 1.42.0 +* Feature - Support for sub second precision in FluentD [#2538](https://github.com/aws/amazon-ecs-agent/pull/2538). +* Bug - Fixed a bug that caused configured values for ImageCleanupExclusionList +to be ignored in some situations [#2513](https://github.com/aws/amazon-ecs-agent/pull/2513) + ## 1.41.1 * Bug - Fixed a bug [#2476](https://github.com/aws/amazon-ecs-agent/issues/2476) where HostPort is not present in ECS Task Metadata Endpoint response with bridge network type [#2495](https://github.com/aws/amazon-ecs-agent/pull/2495) diff --git a/VERSION b/VERSION index f86fb9cbcf1..a50908ca3da 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.41.1 +1.42.0 diff --git a/agent/Gopkg.lock b/agent/Gopkg.lock index 696fa0b8428..b6a25ac7297 100644 --- a/agent/Gopkg.lock +++ b/agent/Gopkg.lock @@ -121,7 +121,7 @@ revision = "c220ac4f01b8a301edcb9c9c05d7600249138cfa" [[projects]] - digest = "1:7e981651f8cc87275ad60b2c48927f13cd5e55815d8db0d44bfd899e476ddd32" + digest = "1:1a07bbfee1d0534e8dda4773948e6dcd3a061ea7ab047ce04619476946226483" name = "github.com/containernetworking/cni" packages = [ "libcni", @@ -132,8 +132,16 @@ "pkg/version", ] pruneopts = "UT" - revision = "7d76556571b6cf1ab90d7026a73092ac8d5e0c23" - version = "v0.7.0" + revision = "4cfb7b568922a3c79a23e438dc52fe537fc9687e" + version = "v0.7.1" + +[[projects]] + digest = "1:b657713432ed8677bfb1467dc3da20de35d25438286f5d4530cbff4ab0e82af1" + name = "github.com/containernetworking/plugins" + packages = ["pkg/ns"] + pruneopts = "UT" + revision = "ad10b6fa91aacd720f1f9ab94341a97a82a24965" + version = "v0.8.6" [[projects]] digest = "1:6bf7f66890675dcbc7952f7afd044a2a46a8bd3143f837c46fd3ccd15da383d7" @@ -543,6 +551,7 @@ "github.com/containernetworking/cni/libcni", "github.com/containernetworking/cni/pkg/types", "github.com/containernetworking/cni/pkg/types/current", + "github.com/containernetworking/plugins/pkg/ns", "github.com/deniswernert/udev", "github.com/didip/tollbooth", "github.com/docker/docker/api/types", diff --git a/agent/Gopkg.toml b/agent/Gopkg.toml index 250c7c091db..27b9da695ba 100644 --- a/agent/Gopkg.toml +++ b/agent/Gopkg.toml @@ -80,7 +80,6 @@ required = ["github.com/golang/mock/mockgen/model"] [[constraint]] name = "github.com/vishvananda/netlink" revision ="fe3b5664d23a11b52ba59bece4ff29c52772a56b" - [[constraint]] name = "github.com/didip/tollbooth" version = "3.0.2" @@ -96,3 +95,7 @@ required = ["github.com/golang/mock/mockgen/model"] [[constraint]] branch = "master" name = "github.com/awslabs/go-config-generator-for-fluentd-and-fluentbit" + +[[constraint]] + name = "github.com/containernetworking/plugins" + version = "0.8.6" diff --git a/agent/acs/handler/task_manifest_handler.go b/agent/acs/handler/task_manifest_handler.go index 79095e6dd54..dcb62e3ea40 100644 --- a/agent/acs/handler/task_manifest_handler.go +++ b/agent/acs/handler/task_manifest_handler.go @@ -52,7 +52,6 @@ func newTaskManifestHandler(ctx context.Context, // Create a cancelable context from the parent context derivedContext, cancel := context.WithCancel(ctx) - return taskManifestHandler{ messageBufferTaskManifest: make(chan *ecsacs.TaskManifestMessage), messageBufferTaskManifestAck: make(chan string), @@ -187,7 +186,7 @@ func (taskManifestHandler *taskManifestHandler) sendTaskStopVerificationMessage( // compares the list of tasks received in the task manifest message and tasks running on the the instance // It returns all the task that are running on the instance but not present in task manifest message task list -func compareTasks(receivedTaskList []*ecsacs.TaskIdentifier, runningTaskList []*apitask.Task) []*ecsacs.TaskIdentifier { +func compareTasks(receivedTaskList []*ecsacs.TaskIdentifier, runningTaskList []*apitask.Task, clusterARN string) []*ecsacs.TaskIdentifier { tasksToBeKilled := make([]*ecsacs.TaskIdentifier, 0) for _, runningTask := range runningTaskList { // For every task running on the instance check if the task is present in receivedTaskList with the DesiredState @@ -204,8 +203,9 @@ func compareTasks(receivedTaskList []*ecsacs.TaskIdentifier, runningTaskList []* } if !taskPresent { tasksToBeKilled = append(tasksToBeKilled, &ecsacs.TaskIdentifier{ - DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), - TaskArn: aws.String(runningTask.Arn), + DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), + TaskArn: aws.String(runningTask.Arn), + TaskClusterArn: aws.String(clusterARN), }) } } @@ -240,6 +240,7 @@ func (taskManifestHandler *taskManifestHandler) handleTaskManifestSingleMessage( message *ecsacs.TaskManifestMessage) error { taskListManifestHandler := message.Tasks seqNumberFromMessage := *message.Timeline + clusterARN := *message.ClusterArn agentLatestSequenceNumber := *taskManifestHandler.latestSeqNumberTaskManifest // Check if the sequence number of message received is more than the one stored in Agent @@ -255,7 +256,7 @@ func (taskManifestHandler *taskManifestHandler) handleTaskManifestSingleMessage( return err } - tasksToKill := compareTasks(taskListManifestHandler, runningTasksOnInstance) + tasksToKill := compareTasks(taskListManifestHandler, runningTasksOnInstance, clusterARN) // Update messageId so that it can be compared to the messageId in TaskStopVerificationAck message taskManifestHandler.setMessageId(*message.MessageId) diff --git a/agent/acs/handler/task_manifest_handler_test.go b/agent/acs/handler/task_manifest_handler_test.go index f80c64a087a..4ca495da447 100644 --- a/agent/acs/handler/task_manifest_handler_test.go +++ b/agent/acs/handler/task_manifest_handler_test.go @@ -68,8 +68,8 @@ func TestManifestHandlerKillAllTasks(t *testing.T) { //Task that needs to be stopped, sent back by agent taskIdentifierFinal := []*ecsacs.TaskIdentifier{ - {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn1")}, - {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn2")}, + {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn1"), TaskClusterArn: aws.String(cluster)}, + {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn2"), TaskClusterArn: aws.String(cluster)}, } taskStopVerificationMessage := &ecsacs.TaskStopVerificationMessage{ @@ -107,7 +107,7 @@ func TestManifestHandlerKillAllTasks(t *testing.T) { ClusterArn: aws.String(cluster), ContainerInstanceArn: aws.String(containerInstanceArn), Tasks: []*ecsacs.TaskIdentifier{ - {DesiredStatus: aws.String("STOPPED"), TaskArn: aws.String("arn-long")}, + {DesiredStatus: aws.String("STOPPED"), TaskArn: aws.String("arn-long"), TaskClusterArn: aws.String(cluster)}, }, Timeline: aws.Int64(testSeqNum), } @@ -164,8 +164,8 @@ func TestManifestHandlerKillFewTasks(t *testing.T) { //Task that needs to be stopped, sent back by agent taskIdentifierFinal := []*ecsacs.TaskIdentifier{ - {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn2")}, - {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn3")}, + {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn2"), TaskClusterArn: aws.String(cluster)}, + {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn3"), TaskClusterArn: aws.String(cluster)}, } taskStopVerificationMessage := &ecsacs.TaskStopVerificationMessage{ @@ -202,12 +202,14 @@ func TestManifestHandlerKillFewTasks(t *testing.T) { ContainerInstanceArn: aws.String(containerInstanceArn), Tasks: []*ecsacs.TaskIdentifier{ { - DesiredStatus: aws.String(apitaskstatus.TaskRunningString), - TaskArn: aws.String("arn1"), + DesiredStatus: aws.String(apitaskstatus.TaskRunningString), + TaskArn: aws.String("arn1"), + TaskClusterArn: aws.String(cluster), }, { - DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), - TaskArn: aws.String("arn2"), + DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), + TaskArn: aws.String("arn2"), + TaskClusterArn: aws.String(cluster), }, }, Timeline: aws.Int64(testSeqNum), @@ -355,20 +357,21 @@ func TestManifestHandlerDifferentTaskLists(t *testing.T) { // tasks that suppose to be running taskIdentifierInitial := ecsacs.TaskIdentifier{ - DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), - TaskArn: aws.String("arn1"), + DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), + TaskArn: aws.String("arn1"), + TaskClusterArn: aws.String(cluster), } //Task that needs to be stopped, sent back by agent taskIdentifierAckFinal := []*ecsacs.TaskIdentifier{ - {DesiredStatus: aws.String(apitaskstatus.TaskRunningString), TaskArn: aws.String("arn1")}, - {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn2")}, + {DesiredStatus: aws.String(apitaskstatus.TaskRunningString), TaskArn: aws.String("arn1"), TaskClusterArn: aws.String(cluster)}, + {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn2"), TaskClusterArn: aws.String(cluster)}, } //Task that needs to be stopped, sent back by agent taskIdentifierMessage := []*ecsacs.TaskIdentifier{ - {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn1")}, - {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn2")}, + {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn1"), TaskClusterArn: aws.String(cluster)}, + {DesiredStatus: aws.String(apitaskstatus.TaskStoppedString), TaskArn: aws.String("arn2"), TaskClusterArn: aws.String(cluster)}, } taskStopVerificationMessage := &ecsacs.TaskStopVerificationMessage{ @@ -518,7 +521,7 @@ func TestCompareTasksDifferentTasks(t *testing.T) { {Arn: "arn1", DesiredStatusUnsafe: apitaskstatus.TaskRunning}, } - compareTaskList := compareTasks(receivedTaskList, taskList) + compareTaskList := compareTasks(receivedTaskList, taskList, "test-cluster-arn") assert.Equal(t, 2, len(compareTaskList)) } @@ -540,7 +543,7 @@ func TestCompareTasksSameTasks(t *testing.T) { {Arn: "arn1", DesiredStatusUnsafe: apitaskstatus.TaskRunning}, } - compareTaskList := compareTasks(receivedTaskList, taskList) + compareTaskList := compareTasks(receivedTaskList, taskList, "test-cluster-arn") assert.Equal(t, 0, len(compareTaskList)) } diff --git a/agent/acs/update_handler/updater.go b/agent/acs/update_handler/updater.go index 611691875d5..bf8c621f7c2 100644 --- a/agent/acs/update_handler/updater.go +++ b/agent/acs/update_handler/updater.go @@ -106,7 +106,7 @@ func (u *updater) stageUpdateHandler() func(req *ecsacs.StageUpdateMessage) { u.reset() } - if !u.config.UpdatesEnabled { + if !u.config.UpdatesEnabled.Enabled() { nack("Updates are disabled") return } @@ -225,7 +225,7 @@ func (u *updater) performUpdateHandler(state dockerstate.TaskEngineState, dataCl seelog.Debug("Got perform update request") - if !u.config.UpdatesEnabled { + if !u.config.UpdatesEnabled.Enabled() { reason := "Updates are disabled" seelog.Errorf("Nacking PerformUpdate; reason: %s", reason) u.acs.MakeRequest(&ecsacs.NackRequest{ diff --git a/agent/acs/update_handler/updater_test.go b/agent/acs/update_handler/updater_test.go index f002d1a73ee..9568e31dde6 100644 --- a/agent/acs/update_handler/updater_test.go +++ b/agent/acs/update_handler/updater_test.go @@ -50,7 +50,7 @@ func ptr(i interface{}) interface{} { func mocks(t *testing.T, cfg *config.Config) (*updater, *gomock.Controller, *config.Config, *mock_client.MockClientServer, *mock_http.MockRoundTripper) { if cfg == nil { cfg = &config.Config{ - UpdatesEnabled: true, + UpdatesEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, UpdateDownloadDir: filepath.Clean("/tmp/test/"), } } @@ -91,7 +91,7 @@ func mockOS() func() { } func TestStageUpdateWithUpdatesDisabled(t *testing.T) { u, ctrl, _, mockacs, _ := mocks(t, &config.Config{ - UpdatesEnabled: false, + UpdatesEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}, }) defer ctrl.Finish() @@ -116,7 +116,7 @@ func TestStageUpdateWithUpdatesDisabled(t *testing.T) { func TestPerformUpdateWithUpdatesDisabled(t *testing.T) { u, ctrl, cfg, mockacs, _ := mocks(t, &config.Config{ - UpdatesEnabled: false, + UpdatesEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}, }) defer ctrl.Finish() diff --git a/agent/api/task/task.go b/agent/api/task/task.go index 41f74a342ef..5170c8eb4b1 100644 --- a/agent/api/task/task.go +++ b/agent/api/task/task.go @@ -298,7 +298,7 @@ func (task *Task) initializeVolumes(cfg *config.Config, dockerClient dockerapi.D if err != nil { return apierrors.NewResourceInitError(task.Arn, err) } - err = task.initializeDockerVolumes(cfg.SharedVolumeMatchFullConfig, dockerClient, ctx) + err = task.initializeDockerVolumes(cfg.SharedVolumeMatchFullConfig.Enabled(), dockerClient, ctx) if err != nil { return apierrors.NewResourceInitError(task.Arn, err) } diff --git a/agent/api/task/task_linux_test.go b/agent/api/task/task_linux_test.go index d069ffa70e3..bd95c66ebf3 100644 --- a/agent/api/task/task_linux_test.go +++ b/agent/api/task/task_linux_test.go @@ -568,7 +568,7 @@ func TestPostUnmarshalWithCPULimitsFail(t *testing.T) { ResourcesMapUnsafe: make(map[string][]taskresource.TaskResource), } cfg := config.Config{ - TaskCPUMemLimit: config.ExplicitlyEnabled, + TaskCPUMemLimit: config.BooleanDefaultTrue{Value: config.ExplicitlyEnabled}, } assert.Error(t, task.PostUnmarshalTask(&cfg, nil, nil, nil, nil)) assert.Equal(t, 0, len(task.GetResources())) diff --git a/agent/api/task/task_windows.go b/agent/api/task/task_windows.go index 3d24c5cf399..1b771cd57c6 100644 --- a/agent/api/task/task_windows.go +++ b/agent/api/task/task_windows.go @@ -47,10 +47,10 @@ const ( type PlatformFields struct { // CpuUnbounded determines whether a mix of unbounded and bounded CPU tasks // are allowed to run in the instance - CpuUnbounded bool `json:"cpuUnbounded"` + CpuUnbounded config.BooleanDefaultFalse `json:"cpuUnbounded"` // MemoryUnbounded determines whether a mix of unbounded and bounded Memory tasks // are allowed to run in the instance - MemoryUnbounded bool `json:"memoryUnbounded"` + MemoryUnbounded config.BooleanDefaultFalse `json:"memoryUnbounded"` } var cpuShareScaleFactor = runtime.NumCPU() * cpuSharesPerCore @@ -130,7 +130,7 @@ func (task *Task) platformHostConfigOverride(hostConfig *dockercontainer.HostCon } hostConfig.CPUShares = 0 - if hostConfig.Memory <= 0 && task.PlatformFields.MemoryUnbounded { + if hostConfig.Memory <= 0 && task.PlatformFields.MemoryUnbounded.Enabled() { // As of version 17.06.2-ee-6 of docker. MemoryReservation is not supported on windows. This ensures that // this parameter is not passed, allowing to launch a container without a hard limit. hostConfig.MemoryReservation = 0 @@ -144,7 +144,7 @@ func (task *Task) platformHostConfigOverride(hostConfig *dockercontainer.HostCon // want. Instead, we convert 0 to 2 to be closer to expected behavior. The // reason for 2 over 1 is that 1 is an invalid value (Linux's choice, not Docker's). func (task *Task) dockerCPUShares(containerCPU uint) int64 { - if containerCPU <= 1 && !task.PlatformFields.CpuUnbounded { + if containerCPU <= 1 && !task.PlatformFields.CpuUnbounded.Enabled() { seelog.Debugf( "Converting CPU shares to allowed minimum of 2 for task arn: [%s] and cpu shares: %d", task.Arn, containerCPU) diff --git a/agent/api/task/task_windows_test.go b/agent/api/task/task_windows_test.go index 4b88cbb8f60..716ed59bb5a 100644 --- a/agent/api/task/task_windows_test.go +++ b/agent/api/task/task_windows_test.go @@ -110,7 +110,7 @@ func TestPostUnmarshalWindowsCanonicalPaths(t *testing.T) { seqNum := int64(42) task, err := TaskFromACS(&taskFromAcs, &ecsacs.PayloadMessage{SeqNum: &seqNum}) assert.Nil(t, err, "Should be able to handle acs task") - cfg := config.Config{TaskCPUMemLimit: config.ExplicitlyDisabled} + cfg := config.Config{TaskCPUMemLimit: config.BooleanDefaultTrue{Value: config.ExplicitlyDisabled}} task.PostUnmarshalTask(&cfg, nil, nil, nil, nil) for _, container := range task.Containers { // remove v3 endpoint from each container because it's randomly generated @@ -211,43 +211,43 @@ func TestCPUPercentBasedOnUnboundedEnabled(t *testing.T) { cpuShareScaleFactor := runtime.NumCPU() * cpuSharesPerCore testcases := []struct { cpu int64 - cpuUnbounded bool + cpuUnbounded config.BooleanDefaultFalse cpuPercent int64 }{ { cpu: 0, - cpuUnbounded: true, + cpuUnbounded: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, cpuPercent: 0, }, { cpu: 1, - cpuUnbounded: true, + cpuUnbounded: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, cpuPercent: 1, }, { cpu: 0, - cpuUnbounded: false, + cpuUnbounded: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}, cpuPercent: 1, }, { cpu: 1, - cpuUnbounded: false, + cpuUnbounded: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}, cpuPercent: 1, }, { cpu: 100, - cpuUnbounded: true, + cpuUnbounded: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, cpuPercent: 100 * percentageFactor / int64(cpuShareScaleFactor), }, { cpu: 100, - cpuUnbounded: false, + cpuUnbounded: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}, cpuPercent: 100 * percentageFactor / int64(cpuShareScaleFactor), }, } for _, tc := range testcases { t.Run(fmt.Sprintf("container cpu-%d,cpu unbounded tasks enabled- %t,expected cpu percent-%d", - tc.cpu, tc.cpuUnbounded, tc.cpuPercent), func(t *testing.T) { + tc.cpu, tc.cpuUnbounded.Enabled(), tc.cpuPercent), func(t *testing.T) { testTask := &Task{ Containers: []*apicontainer.Container{ { @@ -295,7 +295,7 @@ func TestWindowsMemoryReservationOption(t *testing.T) { }, }, PlatformFields: PlatformFields{ - MemoryUnbounded: false, + MemoryUnbounded: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}, }, } @@ -307,7 +307,7 @@ func TestWindowsMemoryReservationOption(t *testing.T) { assert.EqualValues(t, nonZeroMemoryReservationValue, cfg.MemoryReservation) // With MemoryUnbounded set to true, tasks with no memory hard limit will have their memory reservation set to zero - testTask.PlatformFields.MemoryUnbounded = true + testTask.PlatformFields.MemoryUnbounded = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} cfg, configErr = testTask.DockerHostConfig(testTask.Containers[0], dockerMap(testTask), defaultDockerClientAPIVersion, &config.Config{}) diff --git a/agent/api/task/taskvolume_test.go b/agent/api/task/taskvolume_test.go index 25553229ae3..436891213ed 100644 --- a/agent/api/task/taskvolume_test.go +++ b/agent/api/task/taskvolume_test.go @@ -127,7 +127,7 @@ func TestMarshalTaskVolumesEFS(t *testing.T) { if runtime.GOOS == "windows" { // windows task defs have a special 'cpu/memory unbounded' field added. // see https://github.com/aws/amazon-ecs-agent/pull/1227 - expectedTaskDef = fmt.Sprintf(expectedTaskDef, `{"cpuUnbounded": false, "memoryUnbounded": false}`) + expectedTaskDef = fmt.Sprintf(expectedTaskDef, `{"cpuUnbounded": null, "memoryUnbounded": null}`) } else { expectedTaskDef = fmt.Sprintf(expectedTaskDef, "{}") } @@ -272,7 +272,7 @@ func TestInitializeLocalDockerVolume(t *testing.T) { } func TestInitializeSharedProvisionedVolume(t *testing.T) { - sharedVolumeMatchFullConfig := true + sharedVolumeMatchFullConfig := config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ctrl := gomock.NewController(t) defer ctrl.Finish() dockerClient := mock_dockerapi.NewMockDockerClient(ctrl) @@ -304,7 +304,7 @@ func TestInitializeSharedProvisionedVolume(t *testing.T) { // Expect the volume already exists on the instance dockerClient.EXPECT().InspectVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(dockerapi.SDKVolumeResponse{}) - err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig, dockerClient, nil) + err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig.Enabled(), dockerClient, nil) assert.NoError(t, err) assert.Len(t, testTask.ResourcesMapUnsafe, 0, "no volume resource should be provisioned by agent") @@ -392,7 +392,7 @@ func TestInitializeEFSVolume_WrongVolumeType(t *testing.T) { } func TestInitializeSharedProvisionedVolumeError(t *testing.T) { - sharedVolumeMatchFullConfig := true + sharedVolumeMatchFullConfig := config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ctrl := gomock.NewController(t) defer ctrl.Finish() dockerClient := mock_dockerapi.NewMockDockerClient(ctrl) @@ -424,12 +424,12 @@ func TestInitializeSharedProvisionedVolumeError(t *testing.T) { // Expect the volume does not exists on the instance dockerClient.EXPECT().InspectVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(dockerapi.SDKVolumeResponse{Error: errors.New("volume not exist")}) - err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig, dockerClient, nil) + err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig.Enabled(), dockerClient, nil) assert.Error(t, err, "volume not found for auto-provisioned resource should cause task to fail") } func TestInitializeSharedNonProvisionedVolume(t *testing.T) { - sharedVolumeMatchFullConfig := true + sharedVolumeMatchFullConfig := config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ctrl := gomock.NewController(t) defer ctrl.Finish() dockerClient := mock_dockerapi.NewMockDockerClient(ctrl) @@ -466,7 +466,7 @@ func TestInitializeSharedNonProvisionedVolume(t *testing.T) { Labels: map[string]string{"test": "test"}, }, }) - err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig, dockerClient, nil) + err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig.Enabled(), dockerClient, nil) assert.NoError(t, err) assert.Len(t, testTask.ResourcesMapUnsafe, 0, "no volume resource should be provisioned by agent") @@ -474,7 +474,7 @@ func TestInitializeSharedNonProvisionedVolume(t *testing.T) { } func TestInitializeSharedNonProvisionedVolumeValidateNameOnly(t *testing.T) { - sharedVolumeMatchFullConfig := false + sharedVolumeMatchFullConfig := config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled} ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -514,7 +514,7 @@ func TestInitializeSharedNonProvisionedVolumeValidateNameOnly(t *testing.T) { Labels: nil, }, }) - err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig, dockerClient, nil) + err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig.Enabled(), dockerClient, nil) assert.NoError(t, err) assert.Len(t, testTask.ResourcesMapUnsafe, 0, "no volume resource should be provisioned by agent") @@ -522,7 +522,7 @@ func TestInitializeSharedNonProvisionedVolumeValidateNameOnly(t *testing.T) { } func TestInitializeSharedAutoprovisionVolumeNotFoundError(t *testing.T) { - sharedVolumeMatchFullConfig := true + sharedVolumeMatchFullConfig := config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ctrl := gomock.NewController(t) defer ctrl.Finish() dockerClient := mock_dockerapi.NewMockDockerClient(ctrl) @@ -553,14 +553,14 @@ func TestInitializeSharedAutoprovisionVolumeNotFoundError(t *testing.T) { } dockerClient.EXPECT().InspectVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(dockerapi.SDKVolumeResponse{Error: errors.New("not found")}) - err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig, dockerClient, nil) + err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig.Enabled(), dockerClient, nil) assert.NoError(t, err) assert.Len(t, testTask.ResourcesMapUnsafe, 1, "volume resource should be provisioned by agent") assert.Len(t, testTask.Containers[0].TransitionDependenciesMap, 1, "volume resource should be in the container dependency map") } func TestInitializeSharedAutoprovisionVolumeNotMatchError(t *testing.T) { - sharedVolumeMatchFullConfig := true + sharedVolumeMatchFullConfig := config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ctrl := gomock.NewController(t) defer ctrl.Finish() dockerClient := mock_dockerapi.NewMockDockerClient(ctrl) @@ -595,12 +595,12 @@ func TestInitializeSharedAutoprovisionVolumeNotMatchError(t *testing.T) { Labels: map[string]string{"test": "test"}, }, }) - err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig, dockerClient, nil) + err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig.Enabled(), dockerClient, nil) assert.Error(t, err, "volume resource details not match should cause task fail") } func TestInitializeSharedAutoprovisionVolumeTimeout(t *testing.T) { - sharedVolumeMatchFullConfig := true + sharedVolumeMatchFullConfig := config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ctrl := gomock.NewController(t) defer ctrl.Finish() dockerClient := mock_dockerapi.NewMockDockerClient(ctrl) @@ -633,12 +633,12 @@ func TestInitializeSharedAutoprovisionVolumeTimeout(t *testing.T) { dockerClient.EXPECT().InspectVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(dockerapi.SDKVolumeResponse{ Error: &dockerapi.DockerTimeoutError{}, }) - err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig, dockerClient, nil) + err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig.Enabled(), dockerClient, nil) assert.Error(t, err, "volume resource details not match should cause task fail") } func TestInitializeTaskVolume(t *testing.T) { - sharedVolumeMatchFullConfig := true + sharedVolumeMatchFullConfig := config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} testTask := &Task{ ResourcesMapUnsafe: make(map[string][]taskresource.TaskResource), Containers: []*apicontainer.Container{ @@ -663,7 +663,7 @@ func TestInitializeTaskVolume(t *testing.T) { }, } - err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig, nil, nil) + err := testTask.initializeDockerVolumes(sharedVolumeMatchFullConfig.Enabled(), nil, nil) assert.NoError(t, err) assert.Len(t, testTask.ResourcesMapUnsafe, 1, "expect the resource map has an empty volume resource") assert.Len(t, testTask.Containers[0].TransitionDependenciesMap, 1, "expect a volume resource as the container dependency") diff --git a/agent/app/agent.go b/agent/app/agent.go index 90d0ecfa826..566d4b704b7 100644 --- a/agent/app/agent.go +++ b/agent/app/agent.go @@ -152,7 +152,7 @@ func newAgent(blackholeEC2Metadata bool, acceptInsecureCert *bool) (agent, error } var dataClient data.Client - if cfg.Checkpoint { + if cfg.Checkpoint.Enabled() { dataClient, err = data.New(cfg.DataDir) if err != nil { seelog.Criticalf("Error creating data client: %v", err) @@ -164,7 +164,7 @@ func newAgent(blackholeEC2Metadata bool, acceptInsecureCert *bool) (agent, error } var metadataManager containermetadata.Manager - if cfg.ContainerMetadataEnabled { + if cfg.ContainerMetadataEnabled.Enabled() { // We use the default API client for the metadata inspect call. This version has some information // missing which means if we need those fields later we will need to change this client to // the appropriate version @@ -271,7 +271,7 @@ func (agent *ecsAgent) doStart(containerChangeEventStream *eventstream.EventStre var vpcSubnetAttributes []*ecs.Attribute // Check if Task ENI is enabled - if agent.cfg.TaskENIEnabled { + if agent.cfg.TaskENIEnabled.Enabled() { // check pause container image load if loadPauseErr != nil { if pause.IsNoSuchFileError(loadPauseErr) || pause.UnsupportedPlatform(loadPauseErr) { @@ -293,7 +293,7 @@ func (agent *ecsAgent) doStart(containerChangeEventStream *eventstream.EventStre // to not update the config to disable the TaskENIEnabled flag and // move on seelog.Warnf("Unable to detect VPC ID for the Instance, disabling Task ENI capability: %v", err) - agent.cfg.TaskENIEnabled = false + agent.cfg.TaskENIEnabled = config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled} default: // Encountered an error initializing dependencies for dealing with // ENIs for Tasks. Exit with the appropriate error code @@ -315,14 +315,14 @@ func (agent *ecsAgent) doStart(containerChangeEventStream *eventstream.EventStre } // Add container instance ARN to metadata manager - if agent.cfg.ContainerMetadataEnabled { + if agent.cfg.ContainerMetadataEnabled.Enabled() { agent.metadataManager.SetContainerInstanceARN(agent.containerInstanceARN) agent.metadataManager.SetAvailabilityZone(agent.availabilityZone) agent.metadataManager.SetHostPrivateIPv4Address(agent.getHostPrivateIPv4AddressFromEC2Metadata()) agent.metadataManager.SetHostPublicIPv4Address(agent.getHostPublicIPv4AddressFromEC2Metadata()) } - if agent.cfg.Checkpoint { + if agent.cfg.Checkpoint.Enabled() { agent.saveMetadata(data.AgentVersionKey, version.Version) agent.saveMetadata(data.AvailabilityZoneKey, agent.availabilityZone) agent.saveMetadata(data.ClusterNameKey, agent.cfg.Cluster) @@ -358,7 +358,7 @@ func (agent *ecsAgent) newTaskEngine(containerChangeEventStream *eventstream.Eve containerChangeEventStream.StartListening() - if !agent.cfg.Checkpoint { + if !agent.cfg.Checkpoint.Enabled() { seelog.Info("Checkpointing not enabled; a new container instance will be created each time the agent is run") return engine.NewTaskEngine(agent.cfg, agent.dockerClient, credentialsManager, containerChangeEventStream, imageManager, state, @@ -472,7 +472,7 @@ func (agent *ecsAgent) newStateManager( containerInstanceArn *string, savedInstanceID *string, availabilityZone *string, latestSeqNumberTaskManifest *int64) (statemanager.StateManager, error) { - if !agent.cfg.Checkpoint { + if !agent.cfg.Checkpoint.Enabled() { return statemanager.NewNoopStateManager(), nil } @@ -604,12 +604,12 @@ func (agent *ecsAgent) startAsyncRoutines( state dockerstate.TaskEngineState) { // Start of the periodic image cleanup process - if !agent.cfg.ImageCleanupDisabled { + if !agent.cfg.ImageCleanupDisabled.Enabled() { go imageManager.StartImageCleanupProcess(agent.ctx) } // Start automatic spot instance draining poller routine - if agent.cfg.SpotInstanceDrainingEnabled { + if agent.cfg.SpotInstanceDrainingEnabled.Enabled() { go agent.startSpotInstanceDrainingPoller(agent.ctx, client) } diff --git a/agent/app/agent_capability.go b/agent/app/agent_capability.go index 524dc27108b..09e0182533b 100644 --- a/agent/app/agent_capability.go +++ b/agent/app/agent_capability.go @@ -137,7 +137,7 @@ func (agent *ecsAgent) capabilities() ([]*ecs.Attribute, error) { capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+cap) } - if !agent.cfg.PrivilegedDisabled { + if !agent.cfg.PrivilegedDisabled.Enabled() { capabilities = appendNameOnlyAttribute(capabilities, capabilityPrefix+"privileged-container") } @@ -151,10 +151,10 @@ func (agent *ecsAgent) capabilities() ([]*ecs.Attribute, error) { capabilities = agent.appendLoggingDriverCapabilities(capabilities) - if agent.cfg.SELinuxCapable { + if agent.cfg.SELinuxCapable.Enabled() { capabilities = appendNameOnlyAttribute(capabilities, capabilityPrefix+"selinux") } - if agent.cfg.AppArmorCapable { + if agent.cfg.AppArmorCapable.Enabled() { capabilities = appendNameOnlyAttribute(capabilities, capabilityPrefix+"apparmor") } @@ -171,7 +171,7 @@ func (agent *ecsAgent) capabilities() ([]*ecs.Attribute, error) { // TODO: gate this on docker api version when ecs supported docker includes // credentials endpoint feature from upstream docker - if agent.cfg.OverrideAWSLogsExecutionRole { + if agent.cfg.OverrideAWSLogsExecutionRole.Enabled() { capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+"execution-role-awslogs") } @@ -224,7 +224,7 @@ func (agent *ecsAgent) appendDockerDependentCapabilities(capabilities []*ecs.Att capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+"execution-role-ecr-pull") } - if _, ok := supportedVersions[dockerclient.Version_1_24]; ok && !agent.cfg.DisableDockerHealthCheck { + if _, ok := supportedVersions[dockerclient.Version_1_24]; ok && !agent.cfg.DisableDockerHealthCheck.Enabled() { // Docker health check was added in API 1.24 capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+"container-health-check") } @@ -249,7 +249,7 @@ func (agent *ecsAgent) appendLoggingDriverCapabilities(capabilities []*ecs.Attri } func (agent *ecsAgent) appendTaskIamRoleCapabilities(capabilities []*ecs.Attribute, supportedVersions map[dockerclient.DockerVersion]bool) []*ecs.Attribute { - if agent.cfg.TaskIAMRoleEnabled { + if agent.cfg.TaskIAMRoleEnabled.Enabled() { // The "task-iam-role" capability is supported for docker v1.7.x onwards // Refer https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api.md // to lookup the table of docker supportedVersions to API supportedVersions @@ -275,20 +275,20 @@ func (agent *ecsAgent) appendTaskCPUMemLimitCapabilities(capabilities []*ecs.Att if agent.cfg.TaskCPUMemLimit.Enabled() { if _, ok := supportedVersions[dockerclient.Version_1_22]; ok { capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+capabilityTaskCPUMemLimit) - } else if agent.cfg.TaskCPUMemLimit == config.ExplicitlyEnabled { + } else if agent.cfg.TaskCPUMemLimit.Value == config.ExplicitlyEnabled { // explicitly enabled -- return an error because we cannot fulfil an explicit request return nil, errors.New("engine: Task CPU + Mem limit cannot be enabled due to unsupported Docker version") } else { // implicitly enabled -- don't register the capability, but degrade gracefully seelog.Warn("Task CPU + Mem Limit disabled due to unsupported Docker version. API version 1.22 or greater is required.") - agent.cfg.TaskCPUMemLimit = config.ExplicitlyDisabled + agent.cfg.TaskCPUMemLimit.Value = config.ExplicitlyDisabled } } return capabilities, nil } func (agent *ecsAgent) appendTaskENICapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { - if agent.cfg.TaskENIEnabled { + if agent.cfg.TaskENIEnabled.Enabled() { // The assumption here is that all of the dependencies for supporting the // Task ENI in the Agent have already been validated prior to the invocation of // the `agent.capabilities()` call @@ -302,7 +302,7 @@ func (agent *ecsAgent) appendTaskENICapabilities(capabilities []*ecs.Attribute) capabilities = append(capabilities, taskENIVersionAttribute) // We only care about AWSVPCBlockInstanceMetdata if Task ENI is enabled - if agent.cfg.AWSVPCBlockInstanceMetdata { + if agent.cfg.AWSVPCBlockInstanceMetdata.Enabled() { // If the Block Instance Metadata flag is set for AWS VPC networking mode, register a capability // indicating the same capabilities = append(capabilities, &ecs.Attribute{ diff --git a/agent/app/agent_capability_test.go b/agent/app/agent_capability_test.go index d3164cfbd4e..d3525524c5f 100644 --- a/agent/app/agent_capability_test.go +++ b/agent/app/agent_capability_test.go @@ -56,11 +56,11 @@ func TestCapabilities(t *testing.T) { dockerclient.GelfDriver, dockerclient.FluentdDriver, }, - PrivilegedDisabled: false, - SELinuxCapable: true, - AppArmorCapable: true, - TaskENIEnabled: true, - AWSVPCBlockInstanceMetdata: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}, + SELinuxCapable: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + AppArmorCapable: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + TaskENIEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + AWSVPCBlockInstanceMetdata: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, TaskCleanupWaitDuration: config.DefaultConfig().TaskCleanupWaitDuration, } @@ -190,7 +190,7 @@ func TestCapabilitiesTaskIAMRoleForSupportedDockerVersion(t *testing.T) { defer ctrl.Finish() conf := &config.Config{ - TaskIAMRoleEnabled: true, + TaskIAMRoleEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, } mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl) @@ -234,7 +234,7 @@ func TestCapabilitiesTaskIAMRoleForUnSupportedDockerVersion(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) conf := &config.Config{ - TaskIAMRoleEnabled: true, + TaskIAMRoleEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, } mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl) @@ -372,8 +372,8 @@ func TestAWSVPCBlockInstanceMetadataWhenTaskENIIsDisabled(t *testing.T) { AvailableLoggingDrivers: []dockerclient.LoggingDriver{ dockerclient.JSONFileDriver, }, - TaskENIEnabled: false, - AWSVPCBlockInstanceMetdata: true, + TaskENIEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}, + AWSVPCBlockInstanceMetdata: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, } mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl) @@ -442,8 +442,8 @@ func TestCapabilitiesExecutionRoleAWSLogs(t *testing.T) { client := mock_dockerapi.NewMockDockerClient(ctrl) cniClient := mock_ecscni.NewMockCNIClient(ctrl) conf := &config.Config{ - OverrideAWSLogsExecutionRole: true, - TaskENIEnabled: true, + OverrideAWSLogsExecutionRole: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + TaskENIEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, } mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl) @@ -486,7 +486,7 @@ func TestCapabilitiesExecutionRoleAWSLogs(t *testing.T) { func TestCapabilitiesTaskResourceLimit(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - conf := &config.Config{TaskCPUMemLimit: config.ExplicitlyEnabled} + conf := &config.Config{TaskCPUMemLimit: config.BooleanDefaultTrue{Value: config.ExplicitlyEnabled}} client := mock_dockerapi.NewMockDockerClient(ctrl) versionList := []dockerclient.DockerVersion{dockerclient.Version_1_22} @@ -529,7 +529,7 @@ func TestCapabilitiesTaskResourceLimit(t *testing.T) { func TestCapabilitesTaskResourceLimitDisabledByMissingDockerVersion(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - conf := &config.Config{TaskCPUMemLimit: config.DefaultEnabled} + conf := &config.Config{TaskCPUMemLimit: config.BooleanDefaultTrue{Value: config.NotSet}} client := mock_dockerapi.NewMockDockerClient(ctrl) versionList := []dockerclient.DockerVersion{dockerclient.Version_1_19} @@ -572,7 +572,7 @@ func TestCapabilitesTaskResourceLimitDisabledByMissingDockerVersion(t *testing.T func TestCapabilitesTaskResourceLimitErrorCase(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - conf := &config.Config{TaskCPUMemLimit: config.ExplicitlyEnabled} + conf := &config.Config{TaskCPUMemLimit: config.BooleanDefaultTrue{Value: config.ExplicitlyEnabled}} client := mock_dockerapi.NewMockDockerClient(ctrl) versionList := []dockerclient.DockerVersion{dockerclient.Version_1_19} @@ -661,7 +661,7 @@ func TestCapabilitiesContainerHealthDisabled(t *testing.T) { defer cancel() agent := &ecsAgent{ ctx: ctx, - cfg: &config.Config{DisableDockerHealthCheck: true}, + cfg: &config.Config{DisableDockerHealthCheck: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}}, dockerClient: client, pauseLoader: mockPauseLoader, mobyPlugins: mockMobyPlugins, diff --git a/agent/app/agent_capability_unix.go b/agent/app/agent_capability_unix.go index 6c4e6858db4..4012aeb1e7a 100644 --- a/agent/app/agent_capability_unix.go +++ b/agent/app/agent_capability_unix.go @@ -88,7 +88,7 @@ func (agent *ecsAgent) appendNvidiaDriverVersionAttribute(capabilities []*ecs.At } func (agent *ecsAgent) appendENITrunkingCapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { - if !agent.cfg.ENITrunkingEnabled { + if !agent.cfg.ENITrunkingEnabled.Enabled() { return capabilities } capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+taskENITrunkingAttributeSuffix) diff --git a/agent/app/agent_capability_unix_test.go b/agent/app/agent_capability_unix_test.go index c5ad803cf85..2af3fd73472 100644 --- a/agent/app/agent_capability_unix_test.go +++ b/agent/app/agent_capability_unix_test.go @@ -58,11 +58,11 @@ func TestVolumeDriverCapabilitiesUnix(t *testing.T) { dockerclient.GelfDriver, dockerclient.FluentdDriver, }, - PrivilegedDisabled: false, - SELinuxCapable: true, - AppArmorCapable: true, - TaskENIEnabled: true, - AWSVPCBlockInstanceMetdata: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}, + SELinuxCapable: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + AppArmorCapable: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + TaskENIEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + AWSVPCBlockInstanceMetdata: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, TaskCleanupWaitDuration: config.DefaultConfig().TaskCleanupWaitDuration, } @@ -146,7 +146,7 @@ func TestNvidiaDriverCapabilitiesUnix(t *testing.T) { mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ - PrivilegedDisabled: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, GPUSupportEnabled: true, } @@ -215,7 +215,7 @@ func TestEmptyNvidiaDriverCapabilitiesUnix(t *testing.T) { mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ - PrivilegedDisabled: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, GPUSupportEnabled: true, } @@ -283,9 +283,9 @@ func TestENITrunkingCapabilitiesUnix(t *testing.T) { mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ - PrivilegedDisabled: true, - TaskENIEnabled: true, - ENITrunkingEnabled: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + TaskENIEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + ENITrunkingEnabled: config.BooleanDefaultTrue{Value: config.ExplicitlyEnabled}, } mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(true, nil) @@ -366,9 +366,9 @@ func TestNoENITrunkingCapabilitiesUnix(t *testing.T) { mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ - PrivilegedDisabled: true, - TaskENIEnabled: true, - ENITrunkingEnabled: false, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + TaskENIEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + ENITrunkingEnabled: config.BooleanDefaultTrue{Value: config.ExplicitlyDisabled}, } mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(true, nil) @@ -439,7 +439,7 @@ func TestPIDAndIPCNamespaceSharingCapabilitiesUnix(t *testing.T) { mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ - PrivilegedDisabled: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, } mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(true, nil) @@ -506,7 +506,7 @@ func TestPIDAndIPCNamespaceSharingCapabilitiesNoPauseContainer(t *testing.T) { mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ - PrivilegedDisabled: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, } mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(false, errors.New("mock error")) @@ -572,7 +572,7 @@ func TestAppMeshCapabilitiesUnix(t *testing.T) { mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ - PrivilegedDisabled: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, } mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(true, nil) @@ -646,7 +646,7 @@ func TestTaskEIACapabilitiesNoOptimizedCPU(t *testing.T) { mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ - PrivilegedDisabled: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, } mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(true, nil) @@ -689,7 +689,7 @@ func TestTaskEIACapabilitiesWithOptimizedCPU(t *testing.T) { mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ - PrivilegedDisabled: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, } utils.OpenFile = func(path string) (*os.File, error) { @@ -738,7 +738,7 @@ func TestCapabilitiesUnix(t *testing.T) { mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ - PrivilegedDisabled: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, VolumePluginCapabilities: []string{capabilityEFSAuth}, } @@ -811,7 +811,7 @@ func TestFirelensConfigCapabilitiesUnix(t *testing.T) { mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) mockPauseLoader := mock_pause.NewMockLoader(ctrl) conf := &config.Config{ - PrivilegedDisabled: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, } mockPauseLoader.EXPECT().IsLoaded(gomock.Any()).Return(true, nil) diff --git a/agent/app/agent_capability_windows_test.go b/agent/app/agent_capability_windows_test.go index 4c96c81953d..d60dc841aea 100644 --- a/agent/app/agent_capability_windows_test.go +++ b/agent/app/agent_capability_windows_test.go @@ -50,11 +50,11 @@ func TestVolumeDriverCapabilitiesWindows(t *testing.T) { dockerclient.GelfDriver, dockerclient.FluentdDriver, }, - PrivilegedDisabled: false, - SELinuxCapable: true, - AppArmorCapable: true, - TaskENIEnabled: true, - AWSVPCBlockInstanceMetdata: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}, + SELinuxCapable: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + AppArmorCapable: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + TaskENIEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + AWSVPCBlockInstanceMetdata: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, TaskCleanupWaitDuration: config.DefaultConfig().TaskCleanupWaitDuration, } @@ -138,11 +138,11 @@ func TestSupportedCapabilitiesWindows(t *testing.T) { dockerclient.GelfDriver, dockerclient.FluentdDriver, }, - PrivilegedDisabled: false, - SELinuxCapable: true, - AppArmorCapable: true, - TaskENIEnabled: true, - AWSVPCBlockInstanceMetdata: true, + PrivilegedDisabled: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}, + SELinuxCapable: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + AppArmorCapable: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + TaskENIEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + AWSVPCBlockInstanceMetdata: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, TaskCleanupWaitDuration: config.DefaultConfig().TaskCleanupWaitDuration, } diff --git a/agent/app/agent_compatibility_linux.go b/agent/app/agent_compatibility_linux.go index 37c4a2d2ece..f5e92c4a90f 100644 --- a/agent/app/agent_compatibility_linux.go +++ b/agent/app/agent_compatibility_linux.go @@ -52,12 +52,12 @@ func (agent *ecsAgent) checkCompatibility(engine engine.TaskEngine) error { return nil } - if agent.cfg.TaskCPUMemLimit == config.ExplicitlyEnabled { + if agent.cfg.TaskCPUMemLimit.Value == config.ExplicitlyEnabled { return errors.New("App: unable to load old tasks because TaskCPUMemLimits setting is incompatible with old state.") } seelog.Warn("App: disabling TaskCPUMemLimit.") - agent.cfg.TaskCPUMemLimit = config.ExplicitlyDisabled + agent.cfg.TaskCPUMemLimit.Value = config.ExplicitlyDisabled return nil } diff --git a/agent/app/agent_compatibility_linux_test.go b/agent/app/agent_compatibility_linux_test.go index ba853a3020f..ff98b53fa4d 100644 --- a/agent/app/agent_compatibility_linux_test.go +++ b/agent/app/agent_compatibility_linux_test.go @@ -38,8 +38,8 @@ func TestCompatibilityEnabledSuccess(t *testing.T) { stateManager := mock_statemanager.NewMockStateManager(ctrl) cfg := getTestConfig() - cfg.Checkpoint = true - cfg.TaskCPUMemLimit = config.DefaultEnabled + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} + cfg.TaskCPUMemLimit = config.BooleanDefaultTrue{Value: config.NotSet} agent := &ecsAgent{ cfg: &cfg, @@ -66,14 +66,14 @@ func TestCompatibilityEnabledSuccess(t *testing.T) { assert.True(t, cfg.TaskCPUMemLimit.Enabled()) } -func TestCompatibilityDefaultEnabledFail(t *testing.T) { +func TestCompatibilityNotSetFail(t *testing.T) { ctrl, creds, _, images, _, _, stateManagerFactory, saveableOptionFactory := setup(t) defer ctrl.Finish() stateManager := mock_statemanager.NewMockStateManager(ctrl) cfg := getTestConfig() - cfg.Checkpoint = true - cfg.TaskCPUMemLimit = config.DefaultEnabled + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} + cfg.TaskCPUMemLimit = config.BooleanDefaultTrue{Value: config.NotSet} dataClient, cleanup := newTestDataClient(t) defer cleanup() @@ -113,8 +113,8 @@ func TestCompatibilityExplicitlyEnabledFail(t *testing.T) { stateManager := mock_statemanager.NewMockStateManager(ctrl) cfg := getTestConfig() - cfg.Checkpoint = true - cfg.TaskCPUMemLimit = config.ExplicitlyEnabled + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} + cfg.TaskCPUMemLimit = config.BooleanDefaultTrue{Value: config.ExplicitlyEnabled} dataClient, cleanup := newTestDataClient(t) defer cleanup() diff --git a/agent/app/agent_test.go b/agent/app/agent_test.go index e148c9d03d6..5512b75d5ff 100644 --- a/agent/app/agent_test.go +++ b/agent/app/agent_test.go @@ -108,7 +108,7 @@ func TestDoStartMinimumSupportedDockerVersionTerminal(t *testing.T) { ) cfg := getTestConfig() - cfg.Checkpoint = true + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ctx, cancel := context.WithCancel(context.TODO()) // Cancel the context to cancel async routines defer cancel() @@ -134,7 +134,7 @@ func TestDoStartMinimumSupportedDockerVersionError(t *testing.T) { ) cfg := getTestConfig() - cfg.Checkpoint = true + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ctx, cancel := context.WithCancel(context.TODO()) // Cancel the context to cancel async routines defer cancel() @@ -174,7 +174,7 @@ func TestDoStartNewTaskEngineError(t *testing.T) { ) cfg := getTestConfig() - cfg.Checkpoint = true + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ctx, cancel := context.WithCancel(context.TODO()) // Cancel the context to cancel async routines defer cancel() @@ -220,7 +220,7 @@ func TestDoStartRegisterContainerInstanceErrorTerminal(t *testing.T) { mockEC2Metadata.EXPECT().OutpostARN().Return("", nil) cfg := getTestConfig() - cfg.TaskCPUMemLimit = config.ExplicitlyDisabled + cfg.TaskCPUMemLimit.Value = config.ExplicitlyDisabled ctx, cancel := context.WithCancel(context.TODO()) // Cancel the context to cancel async routines defer cancel() @@ -342,8 +342,8 @@ func TestDoStartHappyPath(t *testing.T) { ) cfg := getTestConfig() - cfg.ContainerMetadataEnabled = true - cfg.Checkpoint = true + cfg.ContainerMetadataEnabled = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} cfg.Cluster = clusterName ctx, cancel := context.WithCancel(context.TODO()) @@ -401,7 +401,7 @@ func TestNewTaskEngineRestoreFromCheckpointNoEC2InstanceIDToLoadHappyPath(t *tes mockPauseLoader := mock_pause.NewMockLoader(ctrl) cfg := getTestConfig() - cfg.Checkpoint = true + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} expectedInstanceID := "inst-1" gomock.InOrder( saveableOptionFactory.EXPECT().AddSaveable("TaskEngine", gomock.Any()).Return(nil), @@ -455,7 +455,7 @@ func TestNewTaskEngineRestoreFromCheckpointPreviousEC2InstanceIDLoadedHappyPath( mockPauseLoader := mock_pause.NewMockLoader(ctrl) cfg := getTestConfig() - cfg.Checkpoint = true + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} expectedInstanceID := "inst-1" gomock.InOrder( @@ -520,7 +520,7 @@ func TestNewTaskEngineRestoreFromCheckpointClusterIDMismatch(t *testing.T) { mockPauseLoader := mock_pause.NewMockLoader(ctrl) cfg := getTestConfig() - cfg.Checkpoint = true + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} cfg.Cluster = "default" ec2InstanceID := "inst-1" @@ -577,7 +577,7 @@ func TestNewTaskEngineRestoreFromCheckpointNewStateManagerError(t *testing.T) { defer ctrl.Finish() cfg := getTestConfig() - cfg.Checkpoint = true + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ec2MetadataClient := mock_ec2.NewMockEC2MetadataClient(ctrl) mockPauseLoader := mock_pause.NewMockLoader(ctrl) @@ -624,7 +624,7 @@ func TestNewTaskEngineRestoreFromCheckpointStateLoadError(t *testing.T) { stateManager := mock_statemanager.NewMockStateManager(ctrl) cfg := getTestConfig() - cfg.Checkpoint = true + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ec2MetadataClient := mock_ec2.NewMockEC2MetadataClient(ctrl) mockPauseLoader := mock_pause.NewMockLoader(ctrl) @@ -671,7 +671,7 @@ func TestNewTaskEngineRestoreFromCheckpoint(t *testing.T) { ec2MetadataClient := mock_ec2.NewMockEC2MetadataClient(ctrl) cfg := getTestConfig() - cfg.Checkpoint = true + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} cfg.Cluster = testCluster mockPauseLoader := mock_pause.NewMockLoader(ctrl) @@ -1429,7 +1429,7 @@ func TestSaveMetadata(t *testing.T) { func getTestConfig() config.Config { cfg := config.DefaultConfig() - cfg.TaskCPUMemLimit = config.ExplicitlyDisabled + cfg.TaskCPUMemLimit.Value = config.ExplicitlyDisabled return cfg } diff --git a/agent/app/agent_unix.go b/agent/app/agent_unix.go index 9e23ba8c29c..2d39f04b1d8 100644 --- a/agent/app/agent_unix.go +++ b/agent/app/agent_unix.go @@ -146,7 +146,7 @@ func (agent *ecsAgent) verifyCNIPluginsCapabilities() error { // Check if we can get capabilities from each plugin for _, plugin := range awsVPCCNIPlugins { // skip verifying branch cni plugin if eni trunking is not enabled - if plugin == ecscni.ECSBranchENIPluginName && agent.cfg != nil && !agent.cfg.ENITrunkingEnabled { + if plugin == ecscni.ECSBranchENIPluginName && agent.cfg != nil && !agent.cfg.ENITrunkingEnabled.Enabled() { continue } @@ -219,11 +219,11 @@ func (agent *ecsAgent) cgroupInit() error { if err == nil { return nil } - if agent.cfg.TaskCPUMemLimit == config.ExplicitlyEnabled { + if agent.cfg.TaskCPUMemLimit.Value == config.ExplicitlyEnabled { return errors.Wrapf(err, "unable to setup '/ecs' cgroup") } seelog.Warnf("Disabling TaskCPUMemLimit because agent is unabled to setup '/ecs' cgroup: %v", err) - agent.cfg.TaskCPUMemLimit = config.ExplicitlyDisabled + agent.cfg.TaskCPUMemLimit.Value = config.ExplicitlyDisabled return nil } diff --git a/agent/app/agent_unix_test.go b/agent/app/agent_unix_test.go index 534c0b1017a..0cf338bc254 100644 --- a/agent/app/agent_unix_test.go +++ b/agent/app/agent_unix_test.go @@ -140,8 +140,8 @@ func TestDoStartTaskENIHappyPath(t *testing.T) { ) cfg := getTestConfig() - cfg.TaskENIEnabled = true - cfg.ENITrunkingEnabled = true + cfg.TaskENIEnabled = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} + cfg.ENITrunkingEnabled = config.BooleanDefaultTrue{Value: config.ExplicitlyEnabled} ctx, cancel := context.WithCancel(context.TODO()) // Cancel the context to cancel async routines agent := &ecsAgent{ @@ -276,7 +276,7 @@ func TestQueryCNIPluginsCapabilitiesHappyPath(t *testing.T) { agent := &ecsAgent{ cniClient: cniClient, cfg: &config.Config{ - ENITrunkingEnabled: true, + ENITrunkingEnabled: config.BooleanDefaultTrue{Value: config.ExplicitlyEnabled}, }, } assert.NoError(t, agent.verifyCNIPluginsCapabilities()) @@ -516,7 +516,7 @@ func TestDoStartCgroupInitErrorPath(t *testing.T) { mockControl.EXPECT().Init().Return(errors.New("test error")) cfg := getTestConfig() - cfg.TaskCPUMemLimit = config.ExplicitlyEnabled + cfg.TaskCPUMemLimit.Value = config.ExplicitlyEnabled ctx, cancel := context.WithCancel(context.TODO()) // Cancel the context to cancel async routines @@ -712,8 +712,8 @@ func TestDoStartTaskENIPauseError(t *testing.T) { mockPauseLoader.EXPECT().LoadImage(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("error")).AnyTimes() cfg := getTestConfig() - cfg.TaskENIEnabled = true - cfg.ENITrunkingEnabled = true + cfg.TaskENIEnabled = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} + cfg.ENITrunkingEnabled = config.BooleanDefaultTrue{Value: config.ExplicitlyEnabled} ctx, _ := context.WithCancel(context.TODO()) agent := &ecsAgent{ ctx: ctx, diff --git a/agent/app/agent_windows_test.go b/agent/app/agent_windows_test.go index a865ec6b99d..bc91a1d3c98 100644 --- a/agent/app/agent_windows_test.go +++ b/agent/app/agent_windows_test.go @@ -291,8 +291,8 @@ func TestDoStartTaskLimitsFail(t *testing.T) { defer ctrl.Finish() cfg := getTestConfig() - cfg.Checkpoint = true - cfg.TaskCPUMemLimit = config.ExplicitlyEnabled + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} + cfg.TaskCPUMemLimit.Value = config.ExplicitlyEnabled ctx, cancel := context.WithCancel(context.TODO()) // Cancel the context to cancel async routines defer cancel() diff --git a/agent/app/data_test.go b/agent/app/data_test.go index 8dc570df8e6..74fdc0d2007 100644 --- a/agent/app/data_test.go +++ b/agent/app/data_test.go @@ -92,7 +92,7 @@ func TestLoadDataNoPreviousState(t *testing.T) { defer cleanup() cfg := getTestConfig() - cfg.Checkpoint = true + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Do( func(cfg *config.Config, options ...statemanager.Option) { @@ -128,7 +128,7 @@ func TestLoadDataLoadFromBoltDB(t *testing.T) { populateBoltDB(dataClient, t) cfg := getTestConfig() - cfg.Checkpoint = true + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ctx, cancel := context.WithCancel(context.TODO()) // Cancel the context to cancel async routines @@ -159,7 +159,7 @@ func TestLoadDataLoadFromStateFile(t *testing.T) { generateStateFile(stateManager, t) cfg := getTestConfig() - cfg.Checkpoint = true + cfg.Checkpoint = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Do( func(cfg *config.Config, options ...statemanager.Option) { diff --git a/agent/config/conditional.go b/agent/config/conditional.go index 60205366068..b6f8be049b8 100644 --- a/agent/config/conditional.go +++ b/agent/config/conditional.go @@ -24,17 +24,61 @@ const ( _ Conditional = iota ExplicitlyEnabled ExplicitlyDisabled - DefaultEnabled + NotSet ) +type BooleanDefaultTrue struct { + Value Conditional +} + // Enabled is a convenience function for when consumers don't care if the value is implicit or explicit -func (b Conditional) Enabled() bool { - return b == ExplicitlyEnabled || b == DefaultEnabled +func (b BooleanDefaultTrue) Enabled() bool { + return b.Value == ExplicitlyEnabled || b.Value == NotSet +} + +// MarshalJSON is used to serialize the type to json, per the Marshaller interface +func (b BooleanDefaultTrue) MarshalJSON() ([]byte, error) { + switch b.Value { + case ExplicitlyEnabled: + return json.Marshal(true) + case ExplicitlyDisabled: + return json.Marshal(false) + default: + return json.Marshal(nil) + } +} + +// UnmarshalJSON is used to deserialize json types into Conditional, per the Unmarshaller interface +func (b *BooleanDefaultTrue) UnmarshalJSON(jsonData []byte) error { + jsonString := string(jsonData) + jsonBool, err := strconv.ParseBool(jsonString) + if err != nil && jsonString != "null" { + return err + } + + if jsonString == "" || jsonString == "null" { + b.Value = NotSet + } else if jsonBool { + b.Value = ExplicitlyEnabled + } else { + b.Value = ExplicitlyDisabled + } + + return nil +} + +type BooleanDefaultFalse struct { + Value Conditional +} + +/// Enabled is a convenience function for when consumers don't care if the value is implicit or explicit +func (b BooleanDefaultFalse) Enabled() bool { + return b.Value == ExplicitlyEnabled } // MarshalJSON is used to serialize the type to json, per the Marshaller interface -func (b Conditional) MarshalJSON() ([]byte, error) { - switch b { +func (b BooleanDefaultFalse) MarshalJSON() ([]byte, error) { + switch b.Value { case ExplicitlyEnabled: return json.Marshal(true) case ExplicitlyDisabled: @@ -45,7 +89,7 @@ func (b Conditional) MarshalJSON() ([]byte, error) { } // UnmarshalJSON is used to deserialize json types into Conditional, per the Unmarshaller interface -func (b *Conditional) UnmarshalJSON(jsonData []byte) error { +func (b *BooleanDefaultFalse) UnmarshalJSON(jsonData []byte) error { jsonString := string(jsonData) jsonBool, err := strconv.ParseBool(jsonString) if err != nil && jsonString != "null" { @@ -53,11 +97,11 @@ func (b *Conditional) UnmarshalJSON(jsonData []byte) error { } if jsonString == "" || jsonString == "null" { - *b = DefaultEnabled + b.Value = NotSet } else if jsonBool { - *b = ExplicitlyEnabled + b.Value = ExplicitlyEnabled } else { - *b = ExplicitlyDisabled + b.Value = ExplicitlyDisabled } return nil diff --git a/agent/config/conditional_test.go b/agent/config/conditional_test.go index 9343b5300dd..8f8dbcf9bf2 100644 --- a/agent/config/conditional_test.go +++ b/agent/config/conditional_test.go @@ -22,32 +22,32 @@ import ( "github.com/stretchr/testify/assert" ) -func TestConditional(t *testing.T) { - x := DefaultEnabled - y := ExplicitlyEnabled - z := ExplicitlyDisabled +func TestBooleanDefaultTrue(t *testing.T) { + x := BooleanDefaultTrue{Value: NotSet} + y := BooleanDefaultTrue{Value: ExplicitlyEnabled} + z := BooleanDefaultTrue{Value: ExplicitlyDisabled} - assert.True(t, x.Enabled(), "DefaultEnabled is enabled") + assert.True(t, x.Enabled(), "NotSet is enabled") assert.True(t, y.Enabled(), "ExplicitlyEnabled is enabled") assert.False(t, z.Enabled(), "ExplicitlyDisabled is not enabled") } -func TestConditionalImplements(t *testing.T) { - assert.Implements(t, (*json.Marshaler)(nil), (Conditional)(1)) - assert.Implements(t, (*json.Unmarshaler)(nil), (*Conditional)(nil)) +func TestBooleanDefaultTrueImplements(t *testing.T) { + assert.Implements(t, (*json.Marshaler)(nil), BooleanDefaultTrue{Value: NotSet}) + assert.Implements(t, (*json.Unmarshaler)(nil), (*BooleanDefaultTrue)(nil)) } // main conversion cases for Conditional marshalling var cases = []struct { - defaultBool Conditional + defaultBool BooleanDefaultTrue bytes []byte }{ - {ExplicitlyEnabled, []byte("true")}, - {ExplicitlyDisabled, []byte("false")}, - {DefaultEnabled, []byte("null")}, + {BooleanDefaultTrue{Value: ExplicitlyEnabled}, []byte("true")}, + {BooleanDefaultTrue{Value: ExplicitlyDisabled}, []byte("false")}, + {BooleanDefaultTrue{Value: NotSet}, []byte("null")}, } -func TestConditionalMarshal(t *testing.T) { +func TestBooleanDefaultTrueMarshal(t *testing.T) { for _, tc := range cases { t.Run(string(tc.bytes), func(t *testing.T) { m, err := json.Marshal(tc.defaultBool) @@ -57,10 +57,10 @@ func TestConditionalMarshal(t *testing.T) { } } -func TestConditionalUnmarshal(t *testing.T) { +func TestBooleanDefaultTrueUnmarshal(t *testing.T) { for _, tc := range cases { t.Run(string(tc.bytes), func(t *testing.T) { - var target Conditional + var target BooleanDefaultTrue err := json.Unmarshal(tc.bytes, &target) assert.NoError(t, err) assert.Equal(t, tc.defaultBool, target) diff --git a/agent/config/config.go b/agent/config/config.go index 20f2f6846e4..65b57970182 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -194,8 +194,16 @@ func (cfg *Config) Merge(rhs Config) *Config { for i := 0; i < left.NumField(); i++ { leftField := left.Field(i) - if utils.ZeroOrNil(leftField.Interface()) { - leftField.Set(reflect.ValueOf(right.Field(i).Interface())) + switch leftField.Interface().(type) { + case BooleanDefaultFalse, BooleanDefaultTrue: + str, _ := json.Marshal(reflect.ValueOf(leftField.Interface()).Interface()) + if string(str) == "null" { + leftField.Set(reflect.ValueOf(right.Field(i).Interface())) + } + default: + if utils.ZeroOrNil(leftField.Interface()) { + leftField.Set(reflect.ValueOf(right.Field(i).Interface())) + } } } @@ -346,7 +354,7 @@ func (cfg *Config) validateAndOverrideBounds() error { } func (cfg *Config) pollMetricsOverrides() { - if cfg.PollMetrics { + if cfg.PollMetrics.Enabled() { if cfg.PollingMetricsWaitDuration < minimumPollingMetricsWaitDuration { seelog.Warnf("ECS_POLLING_METRICS_WAIT_DURATION parsed value (%s) is less than the minimum of %s. Setting polling interval to minimum.", cfg.PollingMetricsWaitDuration, minimumPollingMetricsWaitDuration) @@ -507,26 +515,26 @@ func environmentConfig() (Config, error) { Checkpoint: parseCheckpoint(dataDir), EngineAuthType: os.Getenv("ECS_ENGINE_AUTH_TYPE"), EngineAuthData: NewSensitiveRawMessage([]byte(os.Getenv("ECS_ENGINE_AUTH_DATA"))), - UpdatesEnabled: utils.ParseBool(os.Getenv("ECS_UPDATES_ENABLED"), false), + UpdatesEnabled: parseBooleanDefaultFalseConfig("ECS_UPDATES_ENABLED"), UpdateDownloadDir: os.Getenv("ECS_UPDATE_DOWNLOAD_DIR"), - DisableMetrics: utils.ParseBool(os.Getenv("ECS_DISABLE_METRICS"), false), + DisableMetrics: parseBooleanDefaultFalseConfig("ECS_DISABLE_METRICS"), ReservedMemory: parseEnvVariableUint16("ECS_RESERVED_MEMORY"), AvailableLoggingDrivers: parseAvailableLoggingDrivers(), - PrivilegedDisabled: utils.ParseBool(os.Getenv("ECS_DISABLE_PRIVILEGED"), false), - SELinuxCapable: utils.ParseBool(os.Getenv("ECS_SELINUX_CAPABLE"), false), - AppArmorCapable: utils.ParseBool(os.Getenv("ECS_APPARMOR_CAPABLE"), false), + PrivilegedDisabled: parseBooleanDefaultFalseConfig("ECS_DISABLE_PRIVILEGED"), + SELinuxCapable: parseBooleanDefaultFalseConfig("ECS_SELINUX_CAPABLE"), + AppArmorCapable: parseBooleanDefaultFalseConfig("ECS_APPARMOR_CAPABLE"), TaskCleanupWaitDuration: parseEnvVariableDuration("ECS_ENGINE_TASK_CLEANUP_WAIT_DURATION"), - TaskENIEnabled: utils.ParseBool(os.Getenv("ECS_ENABLE_TASK_ENI"), false), - TaskIAMRoleEnabled: utils.ParseBool(os.Getenv("ECS_ENABLE_TASK_IAM_ROLE"), false), - DeleteNonECSImagesEnabled: utils.ParseBool(os.Getenv("ECS_ENABLE_UNTRACKED_IMAGE_CLEANUP"), false), - TaskCPUMemLimit: parseTaskCPUMemLimitEnabled(), + TaskENIEnabled: parseBooleanDefaultFalseConfig("ECS_ENABLE_TASK_ENI"), + TaskIAMRoleEnabled: parseBooleanDefaultFalseConfig("ECS_ENABLE_TASK_IAM_ROLE"), + DeleteNonECSImagesEnabled: parseBooleanDefaultFalseConfig("ECS_ENABLE_UNTRACKED_IMAGE_CLEANUP"), + TaskCPUMemLimit: parseBooleanDefaultTrueConfig("ECS_ENABLE_TASK_CPU_MEM_LIMIT"), DockerStopTimeout: parseDockerStopTimeout(), ContainerStartTimeout: parseContainerStartTimeout(), ImagePullInactivityTimeout: parseImagePullInactivityTimeout(), CredentialsAuditLogFile: os.Getenv("ECS_AUDIT_LOGFILE"), CredentialsAuditLogDisabled: utils.ParseBool(os.Getenv("ECS_AUDIT_LOGFILE_DISABLED"), false), TaskIAMRoleEnabledForNetworkHost: utils.ParseBool(os.Getenv("ECS_ENABLE_TASK_IAM_ROLE_NETWORK_HOST"), false), - ImageCleanupDisabled: utils.ParseBool(os.Getenv("ECS_DISABLE_IMAGE_CLEANUP"), false), + ImageCleanupDisabled: parseBooleanDefaultFalseConfig("ECS_DISABLE_IMAGE_CLEANUP"), MinimumImageDeletionAge: parseEnvVariableDuration("ECS_IMAGE_MINIMUM_CLEANUP_AGE"), NonECSMinimumImageDeletionAge: parseEnvVariableDuration("NON_ECS_IMAGE_MINIMUM_CLEANUP_AGE"), ImageCleanupInterval: parseEnvVariableDuration("ECS_IMAGE_CLEANUP_INTERVAL"), @@ -536,26 +544,26 @@ func environmentConfig() (Config, error) { ImageCleanupExclusionList: parseImageCleanupExclusionList("ECS_EXCLUDE_UNTRACKED_IMAGE"), InstanceAttributes: instanceAttributes, CNIPluginsPath: os.Getenv("ECS_CNI_PLUGINS_PATH"), - AWSVPCBlockInstanceMetdata: utils.ParseBool(os.Getenv("ECS_AWSVPC_BLOCK_IMDS"), false), + AWSVPCBlockInstanceMetdata: parseBooleanDefaultFalseConfig("ECS_AWSVPC_BLOCK_IMDS"), AWSVPCAdditionalLocalRoutes: additionalLocalRoutes, - ContainerMetadataEnabled: utils.ParseBool(os.Getenv("ECS_ENABLE_CONTAINER_METADATA"), false), + ContainerMetadataEnabled: parseBooleanDefaultFalseConfig("ECS_ENABLE_CONTAINER_METADATA"), DataDirOnHost: os.Getenv("ECS_HOST_DATA_DIR"), - OverrideAWSLogsExecutionRole: utils.ParseBool(os.Getenv("ECS_ENABLE_AWSLOGS_EXECUTIONROLE_OVERRIDE"), false), + OverrideAWSLogsExecutionRole: parseBooleanDefaultFalseConfig("ECS_ENABLE_AWSLOGS_EXECUTIONROLE_OVERRIDE"), CgroupPath: os.Getenv("ECS_CGROUP_PATH"), TaskMetadataSteadyStateRate: steadyStateRate, TaskMetadataBurstRate: burstRate, - SharedVolumeMatchFullConfig: utils.ParseBool(os.Getenv("ECS_SHARED_VOLUME_MATCH_FULL_CONFIG"), false), + SharedVolumeMatchFullConfig: parseBooleanDefaultFalseConfig("ECS_SHARED_VOLUME_MATCH_FULL_CONFIG"), ContainerInstanceTags: containerInstanceTags, ContainerInstancePropagateTagsFrom: parseContainerInstancePropagateTagsFrom(), - PollMetrics: utils.ParseBool(os.Getenv("ECS_POLL_METRICS"), true), + PollMetrics: parseBooleanDefaultTrueConfig("ECS_POLL_METRICS"), PollingMetricsWaitDuration: parseEnvVariableDuration("ECS_POLLING_METRICS_WAIT_DURATION"), - DisableDockerHealthCheck: utils.ParseBool(os.Getenv("ECS_DISABLE_DOCKER_HEALTH_CHECK"), false), + DisableDockerHealthCheck: parseBooleanDefaultFalseConfig("ECS_DISABLE_DOCKER_HEALTH_CHECK"), GPUSupportEnabled: utils.ParseBool(os.Getenv("ECS_ENABLE_GPU_SUPPORT"), false), InferentiaSupportEnabled: utils.ParseBool(os.Getenv("ECS_ENABLE_INF_SUPPORT"), false), NvidiaRuntime: os.Getenv("ECS_NVIDIA_RUNTIME"), TaskMetadataAZDisabled: utils.ParseBool(os.Getenv("ECS_DISABLE_TASK_METADATA_AZ"), false), CgroupCPUPeriod: parseCgroupCPUPeriod(), - SpotInstanceDrainingEnabled: utils.ParseBool(os.Getenv("ECS_ENABLE_SPOT_INSTANCE_DRAINING"), false), + SpotInstanceDrainingEnabled: parseBooleanDefaultFalseConfig("ECS_ENABLE_SPOT_INSTANCE_DRAINING"), GMSACapable: parseGMSACapability(), VolumePluginCapabilities: parseVolumePluginCapabilities(), }, err diff --git a/agent/config/config_test.go b/agent/config/config_test.go index 3ac2d9e29bd..a2219c5e251 100644 --- a/agent/config/config_test.go +++ b/agent/config/config_test.go @@ -43,19 +43,29 @@ func TestMerge(t *testing.T) { assert.Equal(t, "us-west-2", conf1.AWSRegion, "Incorrect region") } -// TestBooleanMerge isn't supported with current config/merge implementation, as there is no -// way to differentiate intentionally set false config value from defaulted false value because it's not set. -// Leaving it here as valid test case, but we'll need to refactor config implementation to enable this. -func TestBooleanMerge(t *testing.T) { - t.Skip("not implemented") +// TODO: we need to move all boolean configs to use BooleanDefaultTrue/False +// Below TestBooleanMerge* tests pass for one that's been migrated. +func TestBooleanMergeFalseNotOverridden(t *testing.T) { conf1 := &Config{Cluster: "Foo"} - conf2 := Config{Cluster: "ignored", DeleteNonECSImagesEnabled: false} - conf3 := Config{AWSRegion: "us-west-2", DeleteNonECSImagesEnabled: true} + conf2 := Config{Cluster: "ignored", DeleteNonECSImagesEnabled: BooleanDefaultFalse{Value: ExplicitlyDisabled}} + conf3 := Config{AWSRegion: "us-west-2", DeleteNonECSImagesEnabled: BooleanDefaultFalse{Value: ExplicitlyEnabled}} conf1.Merge(conf2).Merge(conf3) assert.Equal(t, "Foo", conf1.Cluster, "The cluster should not have been overridden") - assert.Equal(t, false, conf1.DeleteNonECSImagesEnabled, "The DeleteNonECSImagesEnabled should not have been overridden") + assert.Equal(t, ExplicitlyDisabled, conf1.DeleteNonECSImagesEnabled.Value, "The DeleteNonECSImagesEnabled should not have been overridden") + assert.Equal(t, "us-west-2", conf1.AWSRegion, "Incorrect region") +} + +func TestBooleanMergeNotSetOverridden(t *testing.T) { + conf1 := &Config{Cluster: "Foo"} + conf2 := Config{Cluster: "ignored", DeleteNonECSImagesEnabled: BooleanDefaultFalse{Value: NotSet}} + conf3 := Config{AWSRegion: "us-west-2", DeleteNonECSImagesEnabled: BooleanDefaultFalse{Value: ExplicitlyDisabled}} + + conf1.Merge(conf2).Merge(conf3) + + assert.Equal(t, "Foo", conf1.Cluster, "The cluster should not have been overridden") + assert.Equal(t, ExplicitlyDisabled, conf1.DeleteNonECSImagesEnabled.Value, "The DeleteNonECSImagesEnabled should have been overridden") assert.Equal(t, "us-west-2", conf1.AWSRegion, "Incorrect region") } @@ -154,17 +164,17 @@ func TestEnvironmentConfig(t *testing.T) { expectedDurationContainerStartTimeout, _ := time.ParseDuration("5m") assert.Equal(t, expectedDurationContainerStartTimeout, conf.ContainerStartTimeout) assert.Equal(t, []dockerclient.LoggingDriver{dockerclient.SyslogDriver}, conf.AvailableLoggingDrivers) - assert.True(t, conf.PrivilegedDisabled) - assert.True(t, conf.SELinuxCapable, "Wrong value for SELinuxCapable") - assert.True(t, conf.AppArmorCapable, "Wrong value for AppArmorCapable") - assert.True(t, conf.TaskIAMRoleEnabled, "Wrong value for TaskIAMRoleEnabled") - assert.True(t, conf.DeleteNonECSImagesEnabled, "Wrong value for DeleteNonECSImagesEnabled") + assert.True(t, conf.PrivilegedDisabled.Enabled()) + assert.True(t, conf.SELinuxCapable.Enabled(), "Wrong value for SELinuxCapable") + assert.True(t, conf.AppArmorCapable.Enabled(), "Wrong value for AppArmorCapable") + assert.True(t, conf.TaskIAMRoleEnabled.Enabled(), "Wrong value for TaskIAMRoleEnabled") + assert.Equal(t, ExplicitlyEnabled, conf.DeleteNonECSImagesEnabled.Value, "Wrong value for DeleteNonECSImagesEnabled") assert.True(t, conf.TaskIAMRoleEnabledForNetworkHost, "Wrong value for TaskIAMRoleEnabledForNetworkHost") - assert.True(t, conf.ImageCleanupDisabled, "Wrong value for ImageCleanupDisabled") - assert.True(t, conf.PollMetrics, "Wrong value for PollMetrics") + assert.True(t, conf.ImageCleanupDisabled.Enabled(), "Wrong value for ImageCleanupDisabled") + assert.True(t, conf.PollMetrics.Enabled(), "Wrong value for PollMetrics") expectedDurationPollingMetricsWaitDuration, _ := time.ParseDuration("10s") assert.Equal(t, expectedDurationPollingMetricsWaitDuration, conf.PollingMetricsWaitDuration) - assert.True(t, conf.TaskENIEnabled, "Wrong value for TaskNetwork") + assert.True(t, conf.TaskENIEnabled.Enabled(), "Wrong value for TaskNetwork") assert.Equal(t, (30 * time.Minute), conf.MinimumImageDeletionAge) assert.Equal(t, (30 * time.Minute), conf.NonECSMinimumImageDeletionAge) assert.Equal(t, (2 * time.Hour), conf.ImageCleanupInterval) @@ -177,15 +187,15 @@ func TestEnvironmentConfig(t *testing.T) { assert.NoError(t, err, "should marshal additional local routes") assert.Equal(t, additionalLocalRoutesJSON, string(serializedAdditionalLocalRoutesJSON)) assert.Equal(t, "/etc/ecs/", conf.DataDirOnHost, "Wrong value for DataDirOnHost") - assert.True(t, conf.ContainerMetadataEnabled, "Wrong value for ContainerMetadataEnabled") + assert.True(t, conf.ContainerMetadataEnabled.Enabled(), "Wrong value for ContainerMetadataEnabled") assert.Equal(t, 1000, conf.TaskMetadataSteadyStateRate) assert.Equal(t, 1100, conf.TaskMetadataBurstRate) - assert.True(t, conf.SharedVolumeMatchFullConfig, "Wrong value for SharedVolumeMatchFullConfig") + assert.True(t, conf.SharedVolumeMatchFullConfig.Enabled(), "Wrong value for SharedVolumeMatchFullConfig") assert.True(t, conf.GPUSupportEnabled, "Wrong value for GPUSupportEnabled") assert.Equal(t, "nvidia", conf.NvidiaRuntime) assert.True(t, conf.TaskMetadataAZDisabled, "Wrong value for TaskMetadataAZDisabled") assert.Equal(t, 10*time.Millisecond, conf.CgroupCPUPeriod) - assert.False(t, conf.SpotInstanceDrainingEnabled) + assert.False(t, conf.SpotInstanceDrainingEnabled.Enabled()) assert.Equal(t, []string{"efsAuth"}, conf.VolumePluginCapabilities) } @@ -218,9 +228,9 @@ func TestConfigBoolean(t *testing.T) { defer setTestEnv("ECS_ENABLE_SPOT_INSTANCE_DRAINING", "true")() cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient()) assert.NoError(t, err) - assert.True(t, cfg.DisableMetrics) - assert.True(t, cfg.DisableDockerHealthCheck) - assert.True(t, cfg.SpotInstanceDrainingEnabled) + assert.True(t, cfg.DisableMetrics.Enabled()) + assert.True(t, cfg.DisableDockerHealthCheck.Enabled()) + assert.True(t, cfg.SpotInstanceDrainingEnabled.Enabled()) } func TestBadLoggingDriverSerialization(t *testing.T) { @@ -255,27 +265,27 @@ func TestInvalidLoggingDriver(t *testing.T) { func TestDefaultPollMetricsWithoutECSDataDir(t *testing.T) { conf, err := environmentConfig() assert.NoError(t, err) - assert.True(t, conf.PollMetrics) + assert.True(t, conf.PollMetrics.Enabled()) } func TestDefaultCheckpointWithoutECSDataDir(t *testing.T) { conf, err := environmentConfig() assert.NoError(t, err) - assert.False(t, conf.Checkpoint) + assert.False(t, conf.Checkpoint.Enabled()) } func TestDefaultCheckpointWithECSDataDir(t *testing.T) { defer setTestEnv("ECS_DATADIR", "/some/dir")() conf, err := environmentConfig() assert.NoError(t, err) - assert.True(t, conf.Checkpoint) + assert.True(t, conf.Checkpoint.Enabled()) } func TestCheckpointWithoutECSDataDir(t *testing.T) { defer setTestEnv("ECS_CHECKPOINT", "true")() conf, err := environmentConfig() assert.NoError(t, err) - assert.True(t, conf.Checkpoint) + assert.True(t, conf.Checkpoint.Enabled()) } func TestInvalidFormatDockerStopTimeout(t *testing.T) { @@ -472,7 +482,7 @@ func TestTaskIAMRoleEnabled(t *testing.T) { defer setTestEnv("ECS_ENABLE_TASK_IAM_ROLE", "true")() cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient()) assert.NoError(t, err) - assert.True(t, cfg.TaskIAMRoleEnabled, "Wrong value for TaskIAMRoleEnabled") + assert.True(t, cfg.TaskIAMRoleEnabled.Enabled(), "Wrong value for TaskIAMRoleEnabled") } func TestDeleteNonECSImagesEnabled(t *testing.T) { @@ -480,7 +490,7 @@ func TestDeleteNonECSImagesEnabled(t *testing.T) { defer setTestEnv("ECS_ENABLE_UNTRACKED_IMAGE_CLEANUP", "true")() cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient()) assert.NoError(t, err) - assert.True(t, cfg.DeleteNonECSImagesEnabled, "Wrong value for DeleteNonECSImagesEnabled") + assert.Equal(t, ExplicitlyEnabled, cfg.DeleteNonECSImagesEnabled.Value, "Wrong value for DeleteNonECSImagesEnabled") } func TestTaskIAMRoleForHostNetworkEnabled(t *testing.T) { @@ -537,7 +547,7 @@ func TestSharedVolumeMatchFullConfigEnabled(t *testing.T) { defer setTestEnv("ECS_SHARED_VOLUME_MATCH_FULL_CONFIG", "true")() cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient()) assert.NoError(t, err) - assert.True(t, cfg.SharedVolumeMatchFullConfig, "Wrong value for SharedVolumeMatchFullConfig") + assert.True(t, cfg.SharedVolumeMatchFullConfig.Enabled(), "Wrong value for SharedVolumeMatchFullConfig") } func TestParseImagePullBehavior(t *testing.T) { @@ -588,7 +598,7 @@ func TestTaskResourceLimitsOverride(t *testing.T) { cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient()) assert.NoError(t, err) assert.False(t, cfg.TaskCPUMemLimit.Enabled(), "Task cpu and memory limits should be overridden to false") - assert.Equal(t, ExplicitlyDisabled, cfg.TaskCPUMemLimit, "Task cpu and memory limits should be explicitly set") + assert.Equal(t, ExplicitlyDisabled, cfg.TaskCPUMemLimit.Value, "Task cpu and memory limits should be explicitly set") } func TestAWSVPCBlockInstanceMetadata(t *testing.T) { @@ -596,7 +606,7 @@ func TestAWSVPCBlockInstanceMetadata(t *testing.T) { defer setTestRegion()() cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient()) assert.NoError(t, err) - assert.True(t, cfg.AWSVPCBlockInstanceMetdata) + assert.True(t, cfg.AWSVPCBlockInstanceMetdata.Enabled()) } func TestInvalidAWSVPCAdditionalLocalRoutes(t *testing.T) { @@ -610,7 +620,7 @@ func TestAWSLogsExecutionRole(t *testing.T) { setTestEnv("ECS_ENABLE_AWSLOGS_EXECUTIONROLE_OVERRIDE", "true") conf, err := environmentConfig() assert.NoError(t, err) - assert.True(t, conf.OverrideAWSLogsExecutionRole) + assert.True(t, conf.OverrideAWSLogsExecutionRole.Enabled()) } func TestTaskMetadataRPSLimits(t *testing.T) { diff --git a/agent/config/config_unix.go b/agent/config/config_unix.go index ac5f5d0f3b1..085846aea8d 100644 --- a/agent/config/config_unix.go +++ b/agent/config/config_unix.go @@ -50,7 +50,7 @@ func DefaultConfig() Config { ReservedPortsUDP: []uint16{}, DataDir: "/data/", DataDirOnHost: "/var/lib/ecs", - DisableMetrics: false, + DisableMetrics: BooleanDefaultFalse{Value: ExplicitlyDisabled}, ReservedMemory: 0, AvailableLoggingDrivers: []dockerclient.LoggingDriver{dockerclient.JSONFileDriver, dockerclient.NoneDriver}, TaskCleanupWaitDuration: DefaultTaskCleanupWaitDuration, @@ -58,7 +58,7 @@ func DefaultConfig() Config { ContainerStartTimeout: defaultContainerStartTimeout, CredentialsAuditLogFile: defaultCredentialsAuditLogFile, CredentialsAuditLogDisabled: false, - ImageCleanupDisabled: false, + ImageCleanupDisabled: BooleanDefaultFalse{Value: ExplicitlyDisabled}, MinimumImageDeletionAge: DefaultImageDeletionAge, NonECSMinimumImageDeletionAge: DefaultNonECSImageDeletionAge, ImageCleanupInterval: DefaultImageCleanupTimeInterval, @@ -69,16 +69,16 @@ func DefaultConfig() Config { PauseContainerTarballPath: pauseContainerTarballPath, PauseContainerImageName: DefaultPauseContainerImageName, PauseContainerTag: DefaultPauseContainerTag, - AWSVPCBlockInstanceMetdata: false, - ContainerMetadataEnabled: false, - TaskCPUMemLimit: DefaultEnabled, + AWSVPCBlockInstanceMetdata: BooleanDefaultFalse{Value: ExplicitlyDisabled}, + ContainerMetadataEnabled: BooleanDefaultFalse{Value: ExplicitlyDisabled}, + TaskCPUMemLimit: BooleanDefaultTrue{Value: NotSet}, CgroupPath: defaultCgroupPath, TaskMetadataSteadyStateRate: DefaultTaskMetadataSteadyStateRate, TaskMetadataBurstRate: DefaultTaskMetadataBurstRate, - SharedVolumeMatchFullConfig: false, // only requiring shared volumes to match on name, which is default docker behavior + SharedVolumeMatchFullConfig: BooleanDefaultFalse{Value: ExplicitlyDisabled}, // only requiring shared volumes to match on name, which is default docker behavior ContainerInstancePropagateTagsFrom: ContainerInstancePropagateTagsFromNoneType, PrometheusMetricsEnabled: false, - PollMetrics: false, + PollMetrics: BooleanDefaultTrue{Value: ExplicitlyDisabled}, PollingMetricsWaitDuration: DefaultPollingMetricsWaitDuration, NvidiaRuntime: DefaultNvidiaRuntime, CgroupCPUPeriod: defaultCgroupCPUPeriod, @@ -92,8 +92,8 @@ func (cfg *Config) platformOverrides() { cfg.ReservedPorts = append(cfg.ReservedPorts, AgentPrometheusExpositionPort) } - if cfg.TaskENIEnabled { // when task networking is enabled, eni trunking is enabled by default - cfg.ENITrunkingEnabled = utils.ParseBool(os.Getenv("ECS_ENABLE_HIGH_DENSITY_ENI"), true) + if cfg.TaskENIEnabled.Enabled() { // when task networking is enabled, eni trunking is enabled by default + cfg.ENITrunkingEnabled = parseBooleanDefaultTrueConfig("ECS_ENABLE_HIGH_DENSITY_ENI") } } diff --git a/agent/config/config_unix_test.go b/agent/config/config_unix_test.go index e350751c4bc..aebac421270 100644 --- a/agent/config/config_unix_test.go +++ b/agent/config/config_unix_test.go @@ -38,34 +38,34 @@ func TestConfigDefault(t *testing.T) { assert.Equal(t, "unix:///var/run/docker.sock", cfg.DockerEndpoint, "Default docker endpoint set incorrectly") assert.Equal(t, "/data/", cfg.DataDir, "Default datadir set incorrectly") - assert.False(t, cfg.DisableMetrics, "Default disablemetrics set incorrectly") + assert.False(t, cfg.DisableMetrics.Enabled(), "Default disablemetrics set incorrectly") assert.Equal(t, 5, len(cfg.ReservedPorts), "Default reserved ports set incorrectly") assert.Equal(t, uint16(0), cfg.ReservedMemory, "Default reserved memory set incorrectly") assert.Equal(t, 30*time.Second, cfg.DockerStopTimeout, "Default docker stop container timeout set incorrectly") assert.Equal(t, 3*time.Minute, cfg.ContainerStartTimeout, "Default docker start container timeout set incorrectly") - assert.False(t, cfg.PrivilegedDisabled, "Default PrivilegedDisabled set incorrectly") + assert.False(t, cfg.PrivilegedDisabled.Enabled(), "Default PrivilegedDisabled set incorrectly") assert.Equal(t, []dockerclient.LoggingDriver{dockerclient.JSONFileDriver, dockerclient.NoneDriver}, cfg.AvailableLoggingDrivers, "Default logging drivers set incorrectly") assert.Equal(t, 3*time.Hour, cfg.TaskCleanupWaitDuration, "Default task cleanup wait duration set incorrectly") - assert.False(t, cfg.TaskENIEnabled, "TaskENIEnabled set incorrectly") - assert.False(t, cfg.TaskIAMRoleEnabled, "TaskIAMRoleEnabled set incorrectly") + assert.False(t, cfg.TaskENIEnabled.Enabled(), "TaskENIEnabled set incorrectly") + assert.False(t, cfg.TaskIAMRoleEnabled.Enabled(), "TaskIAMRoleEnabled set incorrectly") assert.False(t, cfg.TaskIAMRoleEnabledForNetworkHost, "TaskIAMRoleEnabledForNetworkHost set incorrectly") - assert.Equal(t, DefaultEnabled, cfg.TaskCPUMemLimit, "TaskCPUMemLimit should be DefaultEnabled") + assert.Equal(t, NotSet, cfg.TaskCPUMemLimit.Value, "TaskCPUMemLimit should be NotSet") assert.False(t, cfg.CredentialsAuditLogDisabled, "CredentialsAuditLogDisabled set incorrectly") assert.Equal(t, defaultCredentialsAuditLogFile, cfg.CredentialsAuditLogFile, "CredentialsAuditLogFile is set incorrectly") - assert.False(t, cfg.ImageCleanupDisabled, "ImageCleanupDisabled default is set incorrectly") + assert.False(t, cfg.ImageCleanupDisabled.Enabled(), "ImageCleanupDisabled default is set incorrectly") assert.Equal(t, DefaultImageDeletionAge, cfg.MinimumImageDeletionAge, "MinimumImageDeletionAge default is set incorrectly") assert.Equal(t, DefaultNonECSImageDeletionAge, cfg.NonECSMinimumImageDeletionAge, "NonECSMinimumImageDeletionAge default is set incorrectly") assert.Equal(t, DefaultImageCleanupTimeInterval, cfg.ImageCleanupInterval, "ImageCleanupInterval default is set incorrectly") assert.Equal(t, DefaultNumImagesToDeletePerCycle, cfg.NumImagesToDeletePerCycle, "NumImagesToDeletePerCycle default is set incorrectly") assert.Equal(t, defaultCNIPluginsPath, cfg.CNIPluginsPath, "CNIPluginsPath default is set incorrectly") - assert.False(t, cfg.AWSVPCBlockInstanceMetdata, "AWSVPCBlockInstanceMetdata default is incorrectly set") + assert.False(t, cfg.AWSVPCBlockInstanceMetdata.Enabled(), "AWSVPCBlockInstanceMetdata default is incorrectly set") assert.Equal(t, "/var/lib/ecs", cfg.DataDirOnHost, "Default DataDirOnHost set incorrectly") assert.Equal(t, DefaultTaskMetadataSteadyStateRate, cfg.TaskMetadataSteadyStateRate, "Default TaskMetadataSteadyStateRate is set incorrectly") assert.Equal(t, DefaultTaskMetadataBurstRate, cfg.TaskMetadataBurstRate, "Default TaskMetadataBurstRate is set incorrectly") - assert.False(t, cfg.SharedVolumeMatchFullConfig, "Default SharedVolumeMatchFullConfig set incorrectly") + assert.False(t, cfg.SharedVolumeMatchFullConfig.Enabled(), "Default SharedVolumeMatchFullConfig set incorrectly") assert.Equal(t, defaultCgroupCPUPeriod, cfg.CgroupCPUPeriod, "CFS cpu period set incorrectly") } @@ -121,7 +121,7 @@ func TestConfigFromFile(t *testing.T) { assert.NoError(t, err) assert.Equal(t, expectedLocalRoute.IP, cfg.AWSVPCAdditionalLocalRoutes[0].IP, "should match expected route IP") assert.Equal(t, expectedLocalRoute.Mask, cfg.AWSVPCAdditionalLocalRoutes[0].Mask, "should match expected route Mask") - assert.Equal(t, ExplicitlyEnabled, cfg.TaskCPUMemLimit, "TaskCPUMemLimit should be explicitly enabled") + assert.Equal(t, ExplicitlyEnabled, cfg.TaskCPUMemLimit.Value, "TaskCPUMemLimit should be explicitly enabled") } // TestDockerAuthMergeFromFile tests docker auth read from file correctly after merge @@ -201,7 +201,7 @@ func TestENITrunkingEnabled(t *testing.T) { require.NoError(t, err) cfg.platformOverrides() - assert.True(t, cfg.ENITrunkingEnabled, "ENI trunking should be enabled") + assert.True(t, cfg.ENITrunkingEnabled.Enabled(), "ENI trunking should be enabled") } // TestENITrunkingDisabled tests that when task networking is enabled, eni trunking can be disabled @@ -213,7 +213,7 @@ func TestENITrunkingDisabled(t *testing.T) { defer setTestEnv("ECS_ENABLE_HIGH_DENSITY_ENI", "false")() cfg.platformOverrides() - assert.False(t, cfg.ENITrunkingEnabled, "ENI trunking should be disabled") + assert.False(t, cfg.ENITrunkingEnabled.Enabled(), "ENI trunking should be disabled") } // setupFileConfiguration create a temp file store the configuration diff --git a/agent/config/config_windows.go b/agent/config/config_windows.go index 82818e5a75a..3e61b85756f 100644 --- a/agent/config/config_windows.go +++ b/agent/config/config_windows.go @@ -58,8 +58,8 @@ func DefaultConfig() Config { ecsRoot := filepath.Join(programData, "Amazon", "ECS") dataDir := filepath.Join(ecsRoot, "data") platformVariables := PlatformVariables{ - CPUUnbounded: false, - MemoryUnbounded: false, + CPUUnbounded: BooleanDefaultFalse{Value: ExplicitlyDisabled}, + MemoryUnbounded: BooleanDefaultFalse{Value: ExplicitlyDisabled}, } return Config{ DockerEndpoint: "npipe:////./pipe/docker_engine", @@ -89,19 +89,19 @@ func DefaultConfig() Config { ImagePullInactivityTimeout: defaultImagePullInactivityTimeout, CredentialsAuditLogFile: filepath.Join(ecsRoot, defaultCredentialsAuditLogFile), CredentialsAuditLogDisabled: false, - ImageCleanupDisabled: false, + ImageCleanupDisabled: BooleanDefaultFalse{Value: ExplicitlyDisabled}, MinimumImageDeletionAge: DefaultImageDeletionAge, NonECSMinimumImageDeletionAge: DefaultNonECSImageDeletionAge, ImageCleanupInterval: DefaultImageCleanupTimeInterval, NumImagesToDeletePerCycle: DefaultNumImagesToDeletePerCycle, NumNonECSContainersToDeletePerCycle: DefaultNumNonECSContainersToDeletePerCycle, - ContainerMetadataEnabled: false, - TaskCPUMemLimit: ExplicitlyDisabled, + ContainerMetadataEnabled: BooleanDefaultFalse{Value: ExplicitlyDisabled}, + TaskCPUMemLimit: BooleanDefaultTrue{Value: ExplicitlyDisabled}, PlatformVariables: platformVariables, TaskMetadataSteadyStateRate: DefaultTaskMetadataSteadyStateRate, TaskMetadataBurstRate: DefaultTaskMetadataBurstRate, - SharedVolumeMatchFullConfig: false, //only requiring shared volumes to match on name, which is default docker behavior - PollMetrics: false, + SharedVolumeMatchFullConfig: BooleanDefaultFalse{Value: ExplicitlyDisabled}, //only requiring shared volumes to match on name, which is default docker behavior + PollMetrics: BooleanDefaultTrue{Value: ExplicitlyDisabled}, PollingMetricsWaitDuration: DefaultPollingMetricsWaitDuration, GMSACapable: true, } @@ -110,7 +110,7 @@ func DefaultConfig() Config { func (cfg *Config) platformOverrides() { // Enabling task IAM roles for Windows requires the credential proxy to run on port 80, // so we reserve this port by default when that happens. - if cfg.TaskIAMRoleEnabled { + if cfg.TaskIAMRoleEnabled.Enabled() { if cfg.ReservedPorts == nil { cfg.ReservedPorts = []uint16{} } @@ -118,10 +118,10 @@ func (cfg *Config) platformOverrides() { } // ensure TaskResourceLimit is disabled - cfg.TaskCPUMemLimit = ExplicitlyDisabled + cfg.TaskCPUMemLimit.Value = ExplicitlyDisabled - cpuUnbounded := utils.ParseBool(os.Getenv("ECS_ENABLE_CPU_UNBOUNDED_WINDOWS_WORKAROUND"), false) - memoryUnbounded := utils.ParseBool(os.Getenv("ECS_ENABLE_MEMORY_UNBOUNDED_WINDOWS_WORKAROUND"), false) + cpuUnbounded := parseBooleanDefaultFalseConfig("ECS_ENABLE_CPU_UNBOUNDED_WINDOWS_WORKAROUND") + memoryUnbounded := parseBooleanDefaultFalseConfig("ECS_ENABLE_MEMORY_UNBOUNDED_WINDOWS_WORKAROUND") platformVariables := PlatformVariables{ CPUUnbounded: cpuUnbounded, diff --git a/agent/config/config_windows_test.go b/agent/config/config_windows_test.go index 659ce74df6a..399b35b69cd 100644 --- a/agent/config/config_windows_test.go +++ b/agent/config/config_windows_test.go @@ -35,31 +35,31 @@ func TestConfigDefault(t *testing.T) { assert.Equal(t, "npipe:////./pipe/docker_engine", cfg.DockerEndpoint, "Default docker endpoint set incorrectly") assert.Equal(t, `C:\ProgramData\Amazon\ECS\data`, cfg.DataDir, "Default datadir set incorrectly") - assert.False(t, cfg.DisableMetrics, "Default disablemetrics set incorrectly") + assert.False(t, cfg.DisableMetrics.Enabled(), "Default disablemetrics set incorrectly") assert.Equal(t, 11, len(cfg.ReservedPorts), "Default reserved ports set incorrectly") assert.Equal(t, uint16(0), cfg.ReservedMemory, "Default reserved memory set incorrectly") assert.Equal(t, 30*time.Second, cfg.DockerStopTimeout, "Default docker stop container timeout set incorrectly") assert.Equal(t, 8*time.Minute, cfg.ContainerStartTimeout, "Default docker start container timeout set incorrectly") - assert.False(t, cfg.PrivilegedDisabled, "Default PrivilegedDisabled set incorrectly") + assert.False(t, cfg.PrivilegedDisabled.Enabled(), "Default PrivilegedDisabled set incorrectly") assert.Equal(t, []dockerclient.LoggingDriver{dockerclient.JSONFileDriver, dockerclient.NoneDriver, dockerclient.AWSLogsDriver}, cfg.AvailableLoggingDrivers, "Default logging drivers set incorrectly") assert.Equal(t, 3*time.Hour, cfg.TaskCleanupWaitDuration, "Default task cleanup wait duration set incorrectly") - assert.False(t, cfg.TaskIAMRoleEnabled, "TaskIAMRoleEnabled set incorrectly") + assert.False(t, cfg.TaskIAMRoleEnabled.Enabled(), "TaskIAMRoleEnabled set incorrectly") assert.False(t, cfg.TaskIAMRoleEnabledForNetworkHost, "TaskIAMRoleEnabledForNetworkHost set incorrectly") assert.False(t, cfg.CredentialsAuditLogDisabled, "CredentialsAuditLogDisabled set incorrectly") assert.Equal(t, `C:\ProgramData\Amazon\ECS\log\audit.log`, cfg.CredentialsAuditLogFile, "CredentialsAuditLogFile is set incorrectly") - assert.False(t, cfg.ImageCleanupDisabled, "ImageCleanupDisabled default is set incorrectly") + assert.False(t, cfg.ImageCleanupDisabled.Enabled(), "ImageCleanupDisabled default is set incorrectly") assert.Equal(t, DefaultImageDeletionAge, cfg.MinimumImageDeletionAge, "MinimumImageDeletionAge default is set incorrectly") assert.Equal(t, DefaultNonECSImageDeletionAge, cfg.NonECSMinimumImageDeletionAge, "NonECSMinimumImageDeletionAge default is set incorrectly") assert.Equal(t, DefaultImageCleanupTimeInterval, cfg.ImageCleanupInterval, "ImageCleanupInterval default is set incorrectly") assert.Equal(t, DefaultNumImagesToDeletePerCycle, cfg.NumImagesToDeletePerCycle, "NumImagesToDeletePerCycle default is set incorrectly") assert.Equal(t, `C:\ProgramData\Amazon\ECS\data`, cfg.DataDirOnHost, "Default DataDirOnHost set incorrectly") - assert.False(t, cfg.PlatformVariables.CPUUnbounded, "CPUUnbounded should be false by default") + assert.False(t, cfg.PlatformVariables.CPUUnbounded.Enabled(), "CPUUnbounded should be false by default") assert.Equal(t, DefaultTaskMetadataSteadyStateRate, cfg.TaskMetadataSteadyStateRate, "Default TaskMetadataSteadyStateRate is set incorrectly") assert.Equal(t, DefaultTaskMetadataBurstRate, cfg.TaskMetadataBurstRate, "Default TaskMetadataBurstRate is set incorrectly") - assert.False(t, cfg.SharedVolumeMatchFullConfig, "Default SharedVolumeMatchFullConfig set incorrectly") + assert.False(t, cfg.SharedVolumeMatchFullConfig.Enabled(), "Default SharedVolumeMatchFullConfig set incorrectly") } func TestConfigIAMTaskRolesReserves80(t *testing.T) { @@ -104,7 +104,7 @@ func TestCPUUnboundedSet(t *testing.T) { cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient()) cfg.platformOverrides() assert.NoError(t, err) - assert.True(t, cfg.PlatformVariables.CPUUnbounded) + assert.True(t, cfg.PlatformVariables.CPUUnbounded.Enabled()) } func TestCPUUnboundedWindowsDisabled(t *testing.T) { @@ -112,7 +112,7 @@ func TestCPUUnboundedWindowsDisabled(t *testing.T) { cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient()) cfg.platformOverrides() assert.NoError(t, err) - assert.False(t, cfg.PlatformVariables.CPUUnbounded) + assert.False(t, cfg.PlatformVariables.CPUUnbounded.Enabled()) } func TestMemoryUnboundedSet(t *testing.T) { @@ -121,7 +121,7 @@ func TestMemoryUnboundedSet(t *testing.T) { cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient()) cfg.platformOverrides() assert.NoError(t, err) - assert.True(t, cfg.PlatformVariables.MemoryUnbounded) + assert.True(t, cfg.PlatformVariables.MemoryUnbounded.Enabled()) } func TestMemoryUnboundedWindowsDisabled(t *testing.T) { @@ -129,5 +129,5 @@ func TestMemoryUnboundedWindowsDisabled(t *testing.T) { cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient()) cfg.platformOverrides() assert.NoError(t, err) - assert.False(t, cfg.PlatformVariables.MemoryUnbounded) + assert.False(t, cfg.PlatformVariables.MemoryUnbounded.Enabled()) } diff --git a/agent/config/parse.go b/agent/config/parse.go index ef1165af42e..8a557313fd6 100644 --- a/agent/config/parse.go +++ b/agent/config/parse.go @@ -23,20 +23,23 @@ import ( "time" "github.com/aws/amazon-ecs-agent/agent/dockerclient" - "github.com/aws/amazon-ecs-agent/agent/utils" "github.com/cihub/seelog" cnitypes "github.com/containernetworking/cni/pkg/types" ) -func parseCheckpoint(dataDir string) bool { - var checkPoint bool +func parseCheckpoint(dataDir string) BooleanDefaultFalse { + checkPoint := parseBooleanDefaultFalseConfig("ECS_CHECKPOINT") if dataDir != "" { // if we have a directory to checkpoint to, default it to be on - checkPoint = utils.ParseBool(os.Getenv("ECS_CHECKPOINT"), true) + if checkPoint.Value == NotSet { + checkPoint.Value = ExplicitlyEnabled + } } else { // if the directory is not set, default to checkpointing off for // backwards compatibility - checkPoint = utils.ParseBool(os.Getenv("ECS_CHECKPOINT"), false) + if checkPoint.Value == NotSet { + checkPoint.Value = ExplicitlyDisabled + } } return checkPoint } @@ -201,20 +204,24 @@ func parseAdditionalLocalRoutes(errs []error) ([]cnitypes.IPNet, []error) { return additionalLocalRoutes, errs } -func parseTaskCPUMemLimitEnabled() Conditional { - var taskCPUMemLimitEnabled Conditional - taskCPUMemLimitConfigString := os.Getenv("ECS_ENABLE_TASK_CPU_MEM_LIMIT") +func parseBooleanDefaultFalseConfig(envVarName string) BooleanDefaultFalse { + boolDefaultFalseCofig := BooleanDefaultFalse{Value: NotSet} + configString := os.Getenv(envVarName) + err := json.Unmarshal([]byte(configString), &boolDefaultFalseCofig) + if err != nil { + seelog.Warnf("Invalid format for \"%s\", expected an integer. err %v", envVarName, err) + } + return boolDefaultFalseCofig +} - // We only want to set taskCPUMemLimit if it is explicitly set to true or false. - // We can do this by checking against the ParseBool default - if taskCPUMemLimitConfigString != "" { - if utils.ParseBool(taskCPUMemLimitConfigString, false) { - taskCPUMemLimitEnabled = ExplicitlyEnabled - } else { - taskCPUMemLimitEnabled = ExplicitlyDisabled - } +func parseBooleanDefaultTrueConfig(envVarName string) BooleanDefaultTrue { + boolDefaultTrueCofig := BooleanDefaultTrue{Value: NotSet} + configString := os.Getenv(envVarName) + err := json.Unmarshal([]byte(configString), &boolDefaultTrueCofig) + if err != nil { + seelog.Warnf("Invalid format for \"%s\", expected an integer. err %v", envVarName, err) } - return taskCPUMemLimitEnabled + return boolDefaultTrueCofig } func parseTaskMetadataThrottles() (int, int) { diff --git a/agent/config/types.go b/agent/config/types.go index a13461aeda2..3fd18da7540 100644 --- a/agent/config/types.go +++ b/agent/config/types.go @@ -68,7 +68,7 @@ type Config struct { // Checkpoint configures whether data should be periodically to a checkpoint // file, in DataDir, such that on instance or agent restarts it will resume // as the same ContainerInstance. It defaults to false. - Checkpoint bool + Checkpoint BooleanDefaultFalse // EngineAuthType configures what type of data is in EngineAuthData. // Supported types, right now, can be found in the dockerauth package: https://godoc.org/github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerauth @@ -79,7 +79,7 @@ type Config struct { // UpdatesEnabled specifies whether updates should be applied to this agent. // Default true - UpdatesEnabled bool + UpdatesEnabled BooleanDefaultFalse // UpdateDownloadDir specifies where new agent versions should be placed // within the container in order for the external updating process to // correctly handle them. @@ -87,11 +87,11 @@ type Config struct { // DisableMetrics configures whether task utilization metrics should be // sent to the ECS telemetry endpoint - DisableMetrics bool + DisableMetrics BooleanDefaultFalse // PollMetrics configures whether metrics are constantly streamed for each container or // polled on interval instead. - PollMetrics bool + PollMetrics BooleanDefaultTrue // PollingMetricsWaitDuration configures how long a container should wait before polling metrics // again when PollMetrics is set to true @@ -99,7 +99,7 @@ type Config struct { // DisableDockerHealthCheck configures whether container health feature was enabled // on the instance - DisableDockerHealthCheck bool + DisableDockerHealthCheck BooleanDefaultFalse // ReservedMemory specifies the amount of memory (in MB) to reserve for things // other than containers managed by ECS @@ -121,15 +121,15 @@ type Config struct { // PrivilegedDisabled specified whether the Agent is capable of launching // tasks with privileged containers - PrivilegedDisabled bool + PrivilegedDisabled BooleanDefaultFalse // SELinxuCapable specifies whether the Agent is capable of using SELinux // security options - SELinuxCapable bool + SELinuxCapable BooleanDefaultFalse // AppArmorCapable specifies whether the Agent is capable of using AppArmor // security options - AppArmorCapable bool + AppArmorCapable BooleanDefaultFalse // TaskCleanupWaitDuration specifies the time to wait after a task is stopped // until cleanup of task resources is started. @@ -137,13 +137,13 @@ type Config struct { // TaskIAMRoleEnabled specifies if the Agent is capable of launching // tasks with IAM Roles. - TaskIAMRoleEnabled bool + TaskIAMRoleEnabled BooleanDefaultFalse // DeleteNonECSImagesEnabled specifies if the Agent can delete the cached, unused non-ecs images. - DeleteNonECSImagesEnabled bool + DeleteNonECSImagesEnabled BooleanDefaultFalse // TaskCPUMemLimit specifies if Agent can launch a task with a hierarchical cgroup - TaskCPUMemLimit Conditional + TaskCPUMemLimit BooleanDefaultTrue // CredentialsAuditLogFile specifies the path/filename of the audit log. CredentialsAuditLogFile string @@ -157,15 +157,15 @@ type Config struct { // TaskENIEnabled specifies if the Agent is capable of launching task within // defined EC2 networks - TaskENIEnabled bool + TaskENIEnabled BooleanDefaultFalse // ENITrunkingEnabled specifies if the Agent is enabled to launch awsvpc // task with ENI Trunking - ENITrunkingEnabled bool + ENITrunkingEnabled BooleanDefaultTrue // ImageCleanupDisabled specifies whether the Agent will periodically perform // automated image cleanup - ImageCleanupDisabled bool + ImageCleanupDisabled BooleanDefaultFalse // MinimumImageDeletionAge specifies the minimum time since it was pulled // before it can be deleted @@ -222,7 +222,7 @@ type Config struct { // AWSVPCBlockInstanceMetdata specifies if InstanceMetadata endpoint should be blocked // for tasks that are launched with network mode "awsvpc" when ECS_AWSVPC_BLOCK_IMDS=true - AWSVPCBlockInstanceMetdata bool + AWSVPCBlockInstanceMetdata BooleanDefaultFalse // OverrideAWSVPCLocalIPv4Address overrides the local IPv4 address chosen // for a task using the `awsvpc` networking mode. Using this configuration @@ -238,11 +238,11 @@ type Config struct { // ContainerMetadataEnabled specifies if the agent should provide a metadata // file for containers. - ContainerMetadataEnabled bool + ContainerMetadataEnabled BooleanDefaultFalse // OverrideAWSLogsExecutionRole is config option used to enable awslogs // driver authentication over the task's execution role - OverrideAWSLogsExecutionRole bool + OverrideAWSLogsExecutionRole BooleanDefaultFalse // CgroupPath is the path expected by the agent, defaults to // '/sys/fs/cgroup' @@ -261,7 +261,7 @@ type Config struct { // provisioned volume, if false (default). If true, we perform deep comparison including driver options // and labels. For comparing shared volume across 2 instances, this should be set to false as docker's // default behavior is to match name only, and does not propagate driver options and labels through volume drivers. - SharedVolumeMatchFullConfig bool + SharedVolumeMatchFullConfig BooleanDefaultFalse // NoIID when set to true, specifies that the agent should not register the instance // with instance identity document. This is required in order to accomodate scenarios in @@ -307,7 +307,7 @@ type Config struct { // If the instance is not spot then the poller will still run but it will never receive a termination notice. // Defaults to false. // see https://docs.aws.amazon.com/AmazonECS/latest/developerguide/container-instance-draining.html - SpotInstanceDrainingEnabled bool + SpotInstanceDrainingEnabled BooleanDefaultFalse // GMSACapable is the config option to indicate if gMSA is supported. // It should be enabled by default only if the container instance is part of a valid active directory domain. diff --git a/agent/config/types_windows.go b/agent/config/types_windows.go index c6b3977d163..5d893daf1e6 100644 --- a/agent/config/types_windows.go +++ b/agent/config/types_windows.go @@ -18,8 +18,8 @@ package config type PlatformVariables struct { // CPUUnbounded specifies if agent can run a mix of CPU bounded and // unbounded tasks for windows - CPUUnbounded bool + CPUUnbounded BooleanDefaultFalse // MemoryUnbounded specifies if agent can run a mix of Memory bounded and // unbounded tasks for windows - MemoryUnbounded bool + MemoryUnbounded BooleanDefaultFalse } diff --git a/agent/dockerclient/dockerapi/docker_client.go b/agent/dockerclient/dockerapi/docker_client.go index 9d66d3205db..5151e8f9e4f 100644 --- a/agent/dockerclient/dockerapi/docker_client.go +++ b/agent/dockerclient/dockerapi/docker_client.go @@ -1322,7 +1322,7 @@ func (dg *dockerGoClient) Stats(ctx context.Context, id string, inactivityTimeou } var resp types.ContainerStats - if !dg.config.PollMetrics { + if !dg.config.PollMetrics.Enabled() { // Streaming metrics is the default behavior seelog.Infof("DockerGoClient: Starting streaming metrics for container %s", id) go func() { diff --git a/agent/ecscni/mocks_libcni/libcni_mocks.go b/agent/ecscni/mocks_libcni/libcni_mocks.go index db394f60b65..23c4638c751 100644 --- a/agent/ecscni/mocks_libcni/libcni_mocks.go +++ b/agent/ecscni/mocks_libcni/libcni_mocks.go @@ -27,30 +27,30 @@ import ( gomock "github.com/golang/mock/gomock" ) -// MockCNI is a mock of CNI interface +// MockCNI is a mock of CNI interface. type MockCNI struct { ctrl *gomock.Controller recorder *MockCNIMockRecorder } -// MockCNIMockRecorder is the mock recorder for MockCNI +// MockCNIMockRecorder is the mock recorder for MockCNI. type MockCNIMockRecorder struct { mock *MockCNI } -// NewMockCNI creates a new mock instance +// NewMockCNI creates a new mock instance. func NewMockCNI(ctrl *gomock.Controller) *MockCNI { mock := &MockCNI{ctrl: ctrl} mock.recorder = &MockCNIMockRecorder{mock} return mock } -// EXPECT returns an object that allows the caller to indicate expected use +// EXPECT returns an object that allows the caller to indicate expected use. func (m *MockCNI) EXPECT() *MockCNIMockRecorder { return m.recorder } -// AddNetwork mocks base method +// AddNetwork mocks base method. func (m *MockCNI) AddNetwork(arg0 context.Context, arg1 *libcni.NetworkConfig, arg2 *libcni.RuntimeConf) (types.Result, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AddNetwork", arg0, arg1, arg2) @@ -59,13 +59,13 @@ func (m *MockCNI) AddNetwork(arg0 context.Context, arg1 *libcni.NetworkConfig, a return ret0, ret1 } -// AddNetwork indicates an expected call of AddNetwork +// AddNetwork indicates an expected call of AddNetwork. func (mr *MockCNIMockRecorder) AddNetwork(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddNetwork", reflect.TypeOf((*MockCNI)(nil).AddNetwork), arg0, arg1, arg2) } -// AddNetworkList mocks base method +// AddNetworkList mocks base method. func (m *MockCNI) AddNetworkList(arg0 context.Context, arg1 *libcni.NetworkConfigList, arg2 *libcni.RuntimeConf) (types.Result, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AddNetworkList", arg0, arg1, arg2) @@ -74,13 +74,13 @@ func (m *MockCNI) AddNetworkList(arg0 context.Context, arg1 *libcni.NetworkConfi return ret0, ret1 } -// AddNetworkList indicates an expected call of AddNetworkList +// AddNetworkList indicates an expected call of AddNetworkList. func (mr *MockCNIMockRecorder) AddNetworkList(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddNetworkList", reflect.TypeOf((*MockCNI)(nil).AddNetworkList), arg0, arg1, arg2) } -// CheckNetwork mocks base method +// CheckNetwork mocks base method. func (m *MockCNI) CheckNetwork(arg0 context.Context, arg1 *libcni.NetworkConfig, arg2 *libcni.RuntimeConf) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CheckNetwork", arg0, arg1, arg2) @@ -88,13 +88,13 @@ func (m *MockCNI) CheckNetwork(arg0 context.Context, arg1 *libcni.NetworkConfig, return ret0 } -// CheckNetwork indicates an expected call of CheckNetwork +// CheckNetwork indicates an expected call of CheckNetwork. func (mr *MockCNIMockRecorder) CheckNetwork(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckNetwork", reflect.TypeOf((*MockCNI)(nil).CheckNetwork), arg0, arg1, arg2) } -// CheckNetworkList mocks base method +// CheckNetworkList mocks base method. func (m *MockCNI) CheckNetworkList(arg0 context.Context, arg1 *libcni.NetworkConfigList, arg2 *libcni.RuntimeConf) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CheckNetworkList", arg0, arg1, arg2) @@ -102,13 +102,13 @@ func (m *MockCNI) CheckNetworkList(arg0 context.Context, arg1 *libcni.NetworkCon return ret0 } -// CheckNetworkList indicates an expected call of CheckNetworkList +// CheckNetworkList indicates an expected call of CheckNetworkList. func (mr *MockCNIMockRecorder) CheckNetworkList(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckNetworkList", reflect.TypeOf((*MockCNI)(nil).CheckNetworkList), arg0, arg1, arg2) } -// DelNetwork mocks base method +// DelNetwork mocks base method. func (m *MockCNI) DelNetwork(arg0 context.Context, arg1 *libcni.NetworkConfig, arg2 *libcni.RuntimeConf) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DelNetwork", arg0, arg1, arg2) @@ -116,13 +116,13 @@ func (m *MockCNI) DelNetwork(arg0 context.Context, arg1 *libcni.NetworkConfig, a return ret0 } -// DelNetwork indicates an expected call of DelNetwork +// DelNetwork indicates an expected call of DelNetwork. func (mr *MockCNIMockRecorder) DelNetwork(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DelNetwork", reflect.TypeOf((*MockCNI)(nil).DelNetwork), arg0, arg1, arg2) } -// DelNetworkList mocks base method +// DelNetworkList mocks base method. func (m *MockCNI) DelNetworkList(arg0 context.Context, arg1 *libcni.NetworkConfigList, arg2 *libcni.RuntimeConf) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DelNetworkList", arg0, arg1, arg2) @@ -130,13 +130,29 @@ func (m *MockCNI) DelNetworkList(arg0 context.Context, arg1 *libcni.NetworkConfi return ret0 } -// DelNetworkList indicates an expected call of DelNetworkList +// DelNetworkList indicates an expected call of DelNetworkList. func (mr *MockCNIMockRecorder) DelNetworkList(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DelNetworkList", reflect.TypeOf((*MockCNI)(nil).DelNetworkList), arg0, arg1, arg2) } -// GetNetworkCachedResult mocks base method +// GetNetworkCachedConfig mocks base method. +func (m *MockCNI) GetNetworkCachedConfig(arg0 *libcni.NetworkConfig, arg1 *libcni.RuntimeConf) ([]byte, *libcni.RuntimeConf, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNetworkCachedConfig", arg0, arg1) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(*libcni.RuntimeConf) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// GetNetworkCachedConfig indicates an expected call of GetNetworkCachedConfig. +func (mr *MockCNIMockRecorder) GetNetworkCachedConfig(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetworkCachedConfig", reflect.TypeOf((*MockCNI)(nil).GetNetworkCachedConfig), arg0, arg1) +} + +// GetNetworkCachedResult mocks base method. func (m *MockCNI) GetNetworkCachedResult(arg0 *libcni.NetworkConfig, arg1 *libcni.RuntimeConf) (types.Result, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetNetworkCachedResult", arg0, arg1) @@ -145,13 +161,44 @@ func (m *MockCNI) GetNetworkCachedResult(arg0 *libcni.NetworkConfig, arg1 *libcn return ret0, ret1 } -// GetNetworkCachedResult indicates an expected call of GetNetworkCachedResult +// GetNetworkCachedResult indicates an expected call of GetNetworkCachedResult. func (mr *MockCNIMockRecorder) GetNetworkCachedResult(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetworkCachedResult", reflect.TypeOf((*MockCNI)(nil).GetNetworkCachedResult), arg0, arg1) } -// ValidateNetwork mocks base method +// GetNetworkListCachedConfig mocks base method. +func (m *MockCNI) GetNetworkListCachedConfig(arg0 *libcni.NetworkConfigList, arg1 *libcni.RuntimeConf) ([]byte, *libcni.RuntimeConf, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNetworkListCachedConfig", arg0, arg1) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(*libcni.RuntimeConf) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// GetNetworkListCachedConfig indicates an expected call of GetNetworkListCachedConfig. +func (mr *MockCNIMockRecorder) GetNetworkListCachedConfig(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetworkListCachedConfig", reflect.TypeOf((*MockCNI)(nil).GetNetworkListCachedConfig), arg0, arg1) +} + +// GetNetworkListCachedResult mocks base method. +func (m *MockCNI) GetNetworkListCachedResult(arg0 *libcni.NetworkConfigList, arg1 *libcni.RuntimeConf) (types.Result, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNetworkListCachedResult", arg0, arg1) + ret0, _ := ret[0].(types.Result) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetNetworkListCachedResult indicates an expected call of GetNetworkListCachedResult. +func (mr *MockCNIMockRecorder) GetNetworkListCachedResult(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetworkListCachedResult", reflect.TypeOf((*MockCNI)(nil).GetNetworkListCachedResult), arg0, arg1) +} + +// ValidateNetwork mocks base method. func (m *MockCNI) ValidateNetwork(arg0 context.Context, arg1 *libcni.NetworkConfig) ([]string, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ValidateNetwork", arg0, arg1) @@ -160,13 +207,13 @@ func (m *MockCNI) ValidateNetwork(arg0 context.Context, arg1 *libcni.NetworkConf return ret0, ret1 } -// ValidateNetwork indicates an expected call of ValidateNetwork +// ValidateNetwork indicates an expected call of ValidateNetwork. func (mr *MockCNIMockRecorder) ValidateNetwork(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidateNetwork", reflect.TypeOf((*MockCNI)(nil).ValidateNetwork), arg0, arg1) } -// ValidateNetworkList mocks base method +// ValidateNetworkList mocks base method. func (m *MockCNI) ValidateNetworkList(arg0 context.Context, arg1 *libcni.NetworkConfigList) ([]string, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ValidateNetworkList", arg0, arg1) @@ -175,7 +222,7 @@ func (m *MockCNI) ValidateNetworkList(arg0 context.Context, arg1 *libcni.Network return ret0, ret1 } -// ValidateNetworkList indicates an expected call of ValidateNetworkList +// ValidateNetworkList indicates an expected call of ValidateNetworkList. func (mr *MockCNIMockRecorder) ValidateNetworkList(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidateNetworkList", reflect.TypeOf((*MockCNI)(nil).ValidateNetworkList), arg0, arg1) diff --git a/agent/ecscni/plugin.go b/agent/ecscni/plugin.go index 19595318603..f2c68a017bc 100644 --- a/agent/ecscni/plugin.go +++ b/agent/ecscni/plugin.go @@ -99,7 +99,7 @@ func (client *cniClient) setupNS(ctx context.Context, cfg *Config) (*current.Res var bridgeResult cnitypes.Result runtimeConfig := libcni.RuntimeConf{ ContainerID: cfg.ContainerID, - NetNS: fmt.Sprintf(netnsFormat, cfg.ContainerPID), + NetNS: fmt.Sprintf(NetnsFormat, cfg.ContainerPID), } // Execute all CNI network configurations serially, in the given order. @@ -162,7 +162,7 @@ func (client *cniClient) cleanupNS(ctx context.Context, cfg *Config) error { runtimeConfig := libcni.RuntimeConf{ ContainerID: cfg.ContainerID, - NetNS: fmt.Sprintf(netnsFormat, cfg.ContainerPID), + NetNS: fmt.Sprintf(NetnsFormat, cfg.ContainerPID), } // Execute all CNI network configurations serially, in the reverse order. @@ -197,7 +197,7 @@ func (client *cniClient) ReleaseIPResource(ctx context.Context, cfg *Config, tim runtimeConfig := libcni.RuntimeConf{ ContainerID: cfg.ContainerID, - NetNS: fmt.Sprintf(netnsFormat, cfg.ContainerPID), + NetNS: fmt.Sprintf(NetnsFormat, cfg.ContainerPID), } seelog.Debugf("[ECSCNI] Releasing the ip resource from ipam db, id: [%s], ip: [%v]", cfg.ID, cfg.IPAMV4Address) diff --git a/agent/ecscni/types.go b/agent/ecscni/types.go index c48f3d23fde..01b5f8968a8 100644 --- a/agent/ecscni/types.go +++ b/agent/ecscni/types.go @@ -34,11 +34,11 @@ const ( // defaultAppMeshIfName is the default name of app mesh to setup iptable rules // for app mesh container. IfName is mandatory field to invoke CNI plugin. defaultAppMeshIfName = "aws-appmesh" - // netnsFormat is used to construct the path to cotainer network namespace - netnsFormat = "/host/proc/%s/ns/net" // ecsSubnet is the available ip addresses to use for task networking ecsSubnet = "169.254.172.0/22" + // NetnsFormat is used to construct the path to cotainer network namespace + NetnsFormat = "/host/proc/%s/ns/net" // ECSIPAMPluginName is the binary of the ipam plugin ECSIPAMPluginName = "ecs-ipam" // ECSBridgePluginName is the binary of the bridge plugin diff --git a/agent/engine/common_integ_test.go b/agent/engine/common_integ_test.go index 8d331384ffc..042cd1c110d 100644 --- a/agent/engine/common_integ_test.go +++ b/agent/engine/common_integ_test.go @@ -46,7 +46,7 @@ import ( func defaultTestConfigIntegTest() *config.Config { cfg, _ := config.NewConfig(ec2.NewBlackholeEC2MetadataClient()) - cfg.TaskCPUMemLimit = config.ExplicitlyDisabled + cfg.TaskCPUMemLimit.Value = config.ExplicitlyDisabled cfg.ImagePullBehavior = config.ImagePullPreferCachedBehavior return cfg } diff --git a/agent/engine/docker_image_manager.go b/agent/engine/docker_image_manager.go index 7857647dae6..3031cbbadef 100644 --- a/agent/engine/docker_image_manager.go +++ b/agent/engine/docker_image_manager.go @@ -63,7 +63,7 @@ type dockerImageManager struct { imageCleanupTimeInterval time.Duration imagePullBehavior config.ImagePullBehaviorType imageCleanupExclusionList []string - deleteNonECSImagesEnabled bool + deleteNonECSImagesEnabled config.BooleanDefaultFalse nonECSContainerCleanupWaitDuration time.Duration numNonECSContainersToDelete int nonECSMinimumAgeBeforeDeletion time.Duration @@ -381,7 +381,7 @@ func (imageManager *dockerImageManager) removeUnusedImages(ctx context.Context) break } } - if imageManager.deleteNonECSImagesEnabled { + if imageManager.deleteNonECSImagesEnabled.Enabled() { // remove nonecs containers imageManager.removeNonECSContainers(ctx) // remove nonecs images diff --git a/agent/engine/docker_image_manager_test.go b/agent/engine/docker_image_manager_test.go index 66456d52a9b..945ab44ba7d 100644 --- a/agent/engine/docker_image_manager_test.go +++ b/agent/engine/docker_image_manager_test.go @@ -1113,7 +1113,7 @@ func TestNonECSImageAndContainersCleanupRemoveImage(t *testing.T) { numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, numNonECSContainersToDelete: 10, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, - deleteNonECSImagesEnabled: true, + deleteNonECSImagesEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, nonECSContainerCleanupWaitDuration: time.Hour * 3, } imageManager.SetDataClient(data.NewNoopClient()) @@ -1169,7 +1169,7 @@ func TestNonECSImageAndContainersCleanupRemoveImage_OneImageThreeTags(t *testing numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, numNonECSContainersToDelete: 10, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, - deleteNonECSImagesEnabled: true, + deleteNonECSImagesEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, nonECSContainerCleanupWaitDuration: time.Hour * 3, } imageManager.SetDataClient(data.NewNoopClient()) @@ -1229,7 +1229,7 @@ func TestNonECSImageAndContainersCleanupRemoveImage_DontDeleteExcludedImage(t *t numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, numNonECSContainersToDelete: 10, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, - deleteNonECSImagesEnabled: true, + deleteNonECSImagesEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, nonECSContainerCleanupWaitDuration: time.Hour * 3, imageCleanupExclusionList: []string{"tester"}, } @@ -1287,7 +1287,7 @@ func TestNonECSImageAndContainerCleanupRemoveImage_DontDeleteNotOldEnoughImage(t numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, numNonECSContainersToDelete: 10, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, - deleteNonECSImagesEnabled: true, + deleteNonECSImagesEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, nonECSContainerCleanupWaitDuration: time.Hour * 3, nonECSMinimumAgeBeforeDeletion: time.Hour * 100, } @@ -1341,7 +1341,7 @@ func TestNonECSImageAndContainerCleanupRemoveImage_DeleteOldEnoughImage(t *testi numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, numNonECSContainersToDelete: 10, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, - deleteNonECSImagesEnabled: true, + deleteNonECSImagesEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, nonECSContainerCleanupWaitDuration: time.Hour * 3, nonECSMinimumAgeBeforeDeletion: time.Hour * 3, } @@ -1396,7 +1396,7 @@ func TestNonECSImageAndContainersCleanupRemoveImage_DontDeleteECSImages(t *testi numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, numNonECSContainersToDelete: 10, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, - deleteNonECSImagesEnabled: true, + deleteNonECSImagesEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, nonECSContainerCleanupWaitDuration: time.Hour * 3, } imageManager.SetDataClient(data.NewNoopClient()) @@ -1460,7 +1460,7 @@ func TestNonECSImageAndContainers_RemoveDeadContainer(t *testing.T) { numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, numNonECSContainersToDelete: 10, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, - deleteNonECSImagesEnabled: true, + deleteNonECSImagesEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, nonECSContainerCleanupWaitDuration: time.Hour * 3, } imageManager.SetDataClient(data.NewNoopClient()) @@ -1517,7 +1517,7 @@ func TestNonECSImageAndContainersCleanup_RemoveOldCreatedContainer(t *testing.T) numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, numNonECSContainersToDelete: 10, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, - deleteNonECSImagesEnabled: true, + deleteNonECSImagesEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, nonECSContainerCleanupWaitDuration: time.Hour * 3, } imageManager.SetDataClient(data.NewNoopClient()) @@ -1573,7 +1573,7 @@ func TestNonECSImageAndContainersCleanup_DontRemoveContainerWithInvalidFinishedT numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, numNonECSContainersToDelete: 10, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, - deleteNonECSImagesEnabled: true, + deleteNonECSImagesEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, nonECSContainerCleanupWaitDuration: time.Hour * 3, } imageManager.SetDataClient(data.NewNoopClient()) @@ -1630,7 +1630,7 @@ func TestNonECSImageAndContainersCleanup_DoNotRemoveNewlyCreatedContainer(t *tes numImagesToDelete: config.DefaultNumImagesToDeletePerCycle, numNonECSContainersToDelete: 10, imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval, - deleteNonECSImagesEnabled: true, + deleteNonECSImagesEnabled: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, nonECSContainerCleanupWaitDuration: time.Hour * 3, } imageManager.SetDataClient(data.NewNoopClient()) diff --git a/agent/engine/docker_task_engine.go b/agent/engine/docker_task_engine.go index 71419f80382..ae9b7896777 100644 --- a/agent/engine/docker_task_engine.go +++ b/agent/engine/docker_task_engine.go @@ -79,14 +79,15 @@ const ( engineConnectRetryJitterMultiplier = 0.20 engineConnectRetryDelayMultiplier = 1.5 // logDriverTypeFirelens is the log driver type for containers that want to use the firelens container to send logs. - logDriverTypeFirelens = "awsfirelens" - logDriverTypeFluentd = "fluentd" - logDriverTag = "tag" - logDriverFluentdAddress = "fluentd-address" - dataLogDriverPath = "/data/firelens/" - logDriverAsyncConnect = "fluentd-async-connect" - dataLogDriverSocketPath = "/socket/fluent.sock" - socketPathPrefix = "unix://" + logDriverTypeFirelens = "awsfirelens" + logDriverTypeFluentd = "fluentd" + logDriverTag = "tag" + logDriverFluentdAddress = "fluentd-address" + dataLogDriverPath = "/data/firelens/" + logDriverAsyncConnect = "fluentd-async-connect" + logDriverSubSecondPrecision = "fluentd-sub-second-precision" + dataLogDriverSocketPath = "/socket/fluent.sock" + socketPathPrefix = "unix://" // fluentTagDockerFormat is the format for the log tag, which is "containerName-firelens-taskID" fluentTagDockerFormat = "%s-firelens-%s" @@ -470,7 +471,7 @@ func (engine *DockerTaskEngine) synchronizeContainerStatus(container *apicontain seelog.Warnf("Task engine [%s]: unable to add container reference to image state: %v", task.Arn, err) } - if engine.cfg.ContainerMetadataEnabled && !container.Container.IsMetadataFileUpdated() { + if engine.cfg.ContainerMetadataEnabled.Enabled() && !container.Container.IsMetadataFileUpdated() { go engine.updateMetadataFile(task, container) } } @@ -533,7 +534,7 @@ func (engine *DockerTaskEngine) sweepTask(task *apitask.Task) { } // Clean metadata directory for task - if engine.cfg.ContainerMetadataEnabled { + if engine.cfg.ContainerMetadataEnabled.Enabled() { err := engine.metadataManager.Clean(task.Arn) if err != nil { seelog.Warnf("Task engine [%s]: clean task metadata failed: %v", task.Arn, err) @@ -1079,7 +1080,7 @@ func (engine *DockerTaskEngine) createContainer(task *apitask.Task, container *a // Create metadata directory and file then populate it with common metadata of all containers of this task // Afterwards add this directory to the container's mounts if file creation was successful - if engine.cfg.ContainerMetadataEnabled && !container.IsInternal() { + if engine.cfg.ContainerMetadataEnabled.Enabled() && !container.IsInternal() { info, infoErr := engine.client.Info(engine.ctx, dockerclient.InfoTimeout) if infoErr != nil { seelog.Warnf("Task engine [%s]: unable to get docker info : %v", @@ -1120,6 +1121,7 @@ func getFirelensLogConfig(task *apitask.Task, container *apicontainer.Container, logConfig.Config[logDriverTag] = tag logConfig.Config[logDriverFluentdAddress] = fluentd logConfig.Config[logDriverAsyncConnect] = strconv.FormatBool(true) + logConfig.Config[logDriverSubSecondPrecision] = strconv.FormatBool(true) seelog.Debugf("Applying firelens log config for container %s: %v", container.Name, logConfig) return logConfig } @@ -1156,7 +1158,7 @@ func (engine *DockerTaskEngine) startContainer(task *apitask.Task, container *ap // TODO: Add a state to the apicontainer.Container for the status of the metadata file (Whether it needs update) and // add logic to engine state restoration to do a metadata update for containers that are running after the agent was restarted if dockerContainerMD.Error == nil && - engine.cfg.ContainerMetadataEnabled && + engine.cfg.ContainerMetadataEnabled.Enabled() && !container.IsInternal() { go func() { err := engine.metadataManager.Update(engine.ctx, dockerContainer.DockerID, task, container.Name) @@ -1284,7 +1286,7 @@ func (engine *DockerTaskEngine) buildCNIConfigFromTaskContainer( containerInspectOutput *types.ContainerJSON, includeIPAMConfig bool) (*ecscni.Config, error) { cniConfig := &ecscni.Config{ - BlockInstanceMetadata: engine.cfg.AWSVPCBlockInstanceMetdata, + BlockInstanceMetadata: engine.cfg.AWSVPCBlockInstanceMetdata.Enabled(), MinSupportedCNIVersion: config.DefaultMinSupportedCNIVersion, } if engine.cfg.OverrideAWSVPCLocalIPv4Address != nil && diff --git a/agent/engine/docker_task_engine_linux_test.go b/agent/engine/docker_task_engine_linux_test.go index 9c3c802a752..8d392a00329 100644 --- a/agent/engine/docker_task_engine_linux_test.go +++ b/agent/engine/docker_task_engine_linux_test.go @@ -61,7 +61,7 @@ const ( func init() { defaultConfig = config.DefaultConfig() - defaultConfig.TaskCPUMemLimit = config.ExplicitlyDisabled + defaultConfig.TaskCPUMemLimit.Value = config.ExplicitlyDisabled } // TestResourceContainerProgression tests the container progression based on a @@ -161,7 +161,7 @@ func TestDeleteTask(t *testing.T) { require.NoError(t, dataClient.SaveTask(task)) cfg := defaultConfig - cfg.TaskCPUMemLimit = config.ExplicitlyEnabled + cfg.TaskCPUMemLimit.Value = config.ExplicitlyEnabled mockState := mock_dockerstate.NewMockTaskEngineState(ctrl) taskEngine := &DockerTaskEngine{ @@ -217,7 +217,7 @@ func TestDeleteTaskBranchENIEnabled(t *testing.T) { task.ResourcesMapUnsafe = make(map[string][]taskresource.TaskResource) task.AddResource("cgroup", cgroupResource) cfg := defaultConfig - cfg.TaskCPUMemLimit = config.ExplicitlyEnabled + cfg.TaskCPUMemLimit.Value = config.ExplicitlyEnabled mockState := mock_dockerstate.NewMockTaskEngineState(ctrl) taskEngine := &DockerTaskEngine{ @@ -279,14 +279,14 @@ func TestTaskCPULimitHappyPath(t *testing.T) { metadataCreateError error metadataUpdateError error metadataCleanError error - taskCPULimit config.Conditional + taskCPULimit config.BooleanDefaultTrue }{ { name: "Task CPU Limit Succeeds", metadataCreateError: nil, metadataUpdateError: nil, metadataCleanError: nil, - taskCPULimit: config.ExplicitlyEnabled, + taskCPULimit: config.BooleanDefaultTrue{Value: config.ExplicitlyEnabled}, }, } @@ -294,7 +294,7 @@ func TestTaskCPULimitHappyPath(t *testing.T) { t.Run(tc.name, func(t *testing.T) { metadataConfig := defaultConfig metadataConfig.TaskCPUMemLimit = tc.taskCPULimit - metadataConfig.ContainerMetadataEnabled = true + metadataConfig.ContainerMetadataEnabled = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ctx, cancel := context.WithCancel(context.TODO()) defer cancel() ctrl, client, mockTime, taskEngine, credentialsManager, imageManager, metadataManager := mocks( diff --git a/agent/engine/docker_task_engine_test.go b/agent/engine/docker_task_engine_test.go index 9f86b2e9c50..bf424fe41bc 100644 --- a/agent/engine/docker_task_engine_test.go +++ b/agent/engine/docker_task_engine_test.go @@ -135,7 +135,7 @@ var ( func init() { defaultConfig = config.DefaultConfig() - defaultConfig.TaskCPUMemLimit = config.ExplicitlyDisabled + defaultConfig.TaskCPUMemLimit.Value = config.ExplicitlyDisabled } func getCreatedContainerName() string { @@ -211,8 +211,8 @@ func TestBatchContainerHappyPath(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { metadataConfig := defaultConfig - metadataConfig.TaskCPUMemLimit = tc.taskCPULimit - metadataConfig.ContainerMetadataEnabled = true + metadataConfig.TaskCPUMemLimit.Value = tc.taskCPULimit + metadataConfig.ContainerMetadataEnabled = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ctx, cancel := context.WithCancel(context.TODO()) defer cancel() ctrl, client, mockTime, taskEngine, credentialsManager, imageManager, metadataManager := mocks( @@ -725,7 +725,7 @@ func TestCreateContainerMetadata(t *testing.T) { defer ctrl.Finish() taskEngine, _ := privateTaskEngine.(*DockerTaskEngine) - taskEngine.cfg.ContainerMetadataEnabled = true + taskEngine.cfg.ContainerMetadataEnabled = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} sleepTask := testdata.LoadTask("sleep5") sleepContainer, _ := sleepTask.ContainerByName("sleep5") @@ -1376,7 +1376,7 @@ func TestStopPauseContainerCleanupDelay(t *testing.T) { defer cancel() cfg := config.DefaultConfig() - cfg.TaskCPUMemLimit = config.ExplicitlyDisabled + cfg.TaskCPUMemLimit.Value = config.ExplicitlyDisabled cfg.ENIPauseContainerCleanupDelaySeconds = expectedDelaySeconds delayedChan := make(chan time.Duration, 1) @@ -1663,7 +1663,7 @@ func TestUpdateContainerReference(t *testing.T) { // during task engine init, metadata file updated func TestMetadataFileUpdatedAgentRestart(t *testing.T) { conf := &defaultConfig - conf.ContainerMetadataEnabled = true + conf.ContainerMetadataEnabled = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} ctx, cancel := context.WithCancel(context.TODO()) defer cancel() ctrl, client, _, privateTaskEngine, _, imageManager, metadataManager := mocks(t, ctx, conf) @@ -1672,7 +1672,7 @@ func TestMetadataFileUpdatedAgentRestart(t *testing.T) { var metadataUpdateWG sync.WaitGroup metadataUpdateWG.Add(1) taskEngine, _ := privateTaskEngine.(*DockerTaskEngine) - assert.True(t, taskEngine.cfg.ContainerMetadataEnabled, "ContainerMetadataEnabled set to false.") + assert.True(t, taskEngine.cfg.ContainerMetadataEnabled.Enabled(), "ContainerMetadataEnabled set to false.") taskEngine._time = nil state := taskEngine.State() @@ -1904,7 +1904,7 @@ func TestTaskWaitForHostResourceOnRestart(t *testing.T) { taskNotStarted.Arn = "task_Not_started" conf := &defaultConfig - conf.ContainerMetadataEnabled = false + conf.ContainerMetadataEnabled = config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled} ctx, cancel := context.WithCancel(context.TODO()) defer cancel() ctrl, client, _, privateTaskEngine, _, imageManager, _ := mocks(t, ctx, conf) @@ -2846,6 +2846,7 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) { expectedLogConfigTag string expectedLogConfigFluentAddress string expectedFluentdAsyncConnect string + expectedSubSecondPrecision string expectedIPAddress string expectedPort string }{ @@ -2855,6 +2856,7 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) { expectedLogConfigType: logDriverTypeFluentd, expectedLogConfigTag: taskName + "-firelens-" + taskID, expectedFluentdAsyncConnect: strconv.FormatBool(true), + expectedSubSecondPrecision: strconv.FormatBool(true), expectedLogConfigFluentAddress: socketPathPrefix + filepath.Join(defaultConfig.DataDirOnHost, dataLogDriverPath, taskID, dataLogDriverSocketPath), expectedIPAddress: envVarBridgeMode, expectedPort: envVarPort, @@ -2865,6 +2867,7 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) { expectedLogConfigType: logDriverTypeFluentd, expectedLogConfigTag: taskName + "-firelens-" + taskID, expectedFluentdAsyncConnect: strconv.FormatBool(true), + expectedSubSecondPrecision: strconv.FormatBool(true), expectedLogConfigFluentAddress: socketPathPrefix + filepath.Join(defaultConfig.DataDirOnHost, dataLogDriverPath, taskID, dataLogDriverSocketPath), expectedIPAddress: envVarBridgeMode, expectedPort: envVarPort, @@ -2875,6 +2878,7 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) { expectedLogConfigType: logDriverTypeFluentd, expectedLogConfigTag: taskName + "-firelens-" + taskID, expectedFluentdAsyncConnect: strconv.FormatBool(true), + expectedSubSecondPrecision: strconv.FormatBool(true), expectedLogConfigFluentAddress: socketPathPrefix + filepath.Join(defaultConfig.DataDirOnHost, dataLogDriverPath, taskID, dataLogDriverSocketPath), expectedIPAddress: envVarAWSVPCMode, expectedPort: envVarPort, @@ -2899,6 +2903,7 @@ func TestCreateContainerAddFirelensLogDriverConfig(t *testing.T) { assert.Equal(t, tc.expectedLogConfigTag, hostConfig.LogConfig.Config["tag"]) assert.Equal(t, tc.expectedLogConfigFluentAddress, hostConfig.LogConfig.Config["fluentd-address"]) assert.Equal(t, tc.expectedFluentdAsyncConnect, hostConfig.LogConfig.Config["fluentd-async-connect"]) + assert.Equal(t, tc.expectedSubSecondPrecision, hostConfig.LogConfig.Config["fluentd-sub-second-precision"]) assert.Contains(t, config.Env, tc.expectedIPAddress) assert.Contains(t, config.Env, tc.expectedPort) }) diff --git a/agent/engine/engine_integ_test.go b/agent/engine/engine_integ_test.go index bd21a036c7c..791164e60e2 100644 --- a/agent/engine/engine_integ_test.go +++ b/agent/engine/engine_integ_test.go @@ -221,7 +221,7 @@ func TestHostVolumeMount(t *testing.T) { func TestSweepContainer(t *testing.T) { cfg := defaultTestConfigIntegTest() cfg.TaskCleanupWaitDuration = 1 * time.Minute - cfg.ContainerMetadataEnabled = true + cfg.ContainerMetadataEnabled = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} taskEngine, done, _ := setup(cfg, nil, t) defer done() diff --git a/agent/engine/engine_sudo_linux_integ_test.go b/agent/engine/engine_sudo_linux_integ_test.go index 8acd2427d87..6e61e24abd0 100644 --- a/agent/engine/engine_sudo_linux_integ_test.go +++ b/agent/engine/engine_sudo_linux_integ_test.go @@ -65,7 +65,7 @@ const ( func TestStartStopWithCgroup(t *testing.T) { cfg := defaultTestConfigIntegTest() cfg.TaskCleanupWaitDuration = 1 * time.Second - cfg.TaskCPUMemLimit = config.ExplicitlyEnabled + cfg.TaskCPUMemLimit.Value = config.ExplicitlyEnabled cfg.CgroupPath = "/cgroup" taskEngine, done, _ := setup(cfg, nil, t) diff --git a/agent/engine/engine_windows_integ_test.go b/agent/engine/engine_windows_integ_test.go index 4ff7f73f90d..fe0b807ed66 100644 --- a/agent/engine/engine_windows_integ_test.go +++ b/agent/engine/engine_windows_integ_test.go @@ -701,7 +701,7 @@ func TestGMSATaskFile(t *testing.T) { func defaultTestConfigIntegTestGMSA() *config.Config { cfg, _ := config.NewConfig(ec2.NewBlackholeEC2MetadataClient()) - cfg.TaskCPUMemLimit = config.ExplicitlyDisabled + cfg.TaskCPUMemLimit.Value = config.ExplicitlyDisabled cfg.ImagePullBehavior = config.ImagePullPreferCachedBehavior cfg.GMSACapable = true diff --git a/agent/engine/task_manager_test.go b/agent/engine/task_manager_test.go index 7419411089e..a1d0b3a2c73 100644 --- a/agent/engine/task_manager_test.go +++ b/agent/engine/task_manager_test.go @@ -1390,7 +1390,7 @@ func TestCleanupTaskWithResourceHappyPath(t *testing.T) { defer ctrl.Finish() cfg := getTestConfig() - cfg.TaskCPUMemLimit = config.ExplicitlyEnabled + cfg.TaskCPUMemLimit.Value = config.ExplicitlyEnabled ctx, cancel := context.WithCancel(context.TODO()) defer cancel() taskEngine := &DockerTaskEngine{ @@ -1451,7 +1451,7 @@ func TestCleanupTaskWithResourceErrorPath(t *testing.T) { defer ctrl.Finish() cfg := getTestConfig() - cfg.TaskCPUMemLimit = config.ExplicitlyEnabled + cfg.TaskCPUMemLimit.Value = config.ExplicitlyEnabled ctx, cancel := context.WithCancel(context.TODO()) defer cancel() taskEngine := &DockerTaskEngine{ @@ -1961,7 +1961,7 @@ func TestStartVolumeResourceTransitionsEmpty(t *testing.T) { func getTestConfig() config.Config { cfg := config.DefaultConfig() - cfg.TaskCPUMemLimit = config.ExplicitlyDisabled + cfg.TaskCPUMemLimit.Value = config.ExplicitlyDisabled return cfg } diff --git a/agent/handlers/task_server_setup_test.go b/agent/handlers/task_server_setup_test.go index 23c3dfb3a6f..02ff13b3daf 100644 --- a/agent/handlers/task_server_setup_test.go +++ b/agent/handlers/task_server_setup_test.go @@ -44,6 +44,7 @@ import ( v3 "github.com/aws/amazon-ecs-agent/agent/handlers/v3" v4 "github.com/aws/amazon-ecs-agent/agent/handlers/v4" mock_audit "github.com/aws/amazon-ecs-agent/agent/logger/audit/mocks" + "github.com/aws/amazon-ecs-agent/agent/stats" mock_stats "github.com/aws/amazon-ecs-agent/agent/stats/mock" "github.com/aws/aws-sdk-go/aws" "github.com/docker/docker/api/types" @@ -727,7 +728,7 @@ func TestV2ContainerStats(t *testing.T) { dockerStats.NumProcs = 2 gomock.InOrder( state.EXPECT().GetTaskByIPAddress(remoteIP).Return(taskARN, true), - statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, nil), + statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, &stats.NetworkStatsPerSec{}, nil), ) server := taskServerSetup(credentials.NewManager(), auditLog, state, ecsClient, clusterName, statsEngine, config.DefaultTaskMetadataSteadyStateRate, config.DefaultTaskMetadataBurstRate, "", containerInstanceArn) @@ -776,7 +777,7 @@ func TestV2TaskStats(t *testing.T) { gomock.InOrder( state.EXPECT().GetTaskByIPAddress(remoteIP).Return(taskARN, true), state.EXPECT().ContainerMapByArn(taskARN).Return(containerMap, true), - statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, nil), + statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, &stats.NetworkStatsPerSec{}, nil), ) server := taskServerSetup(credentials.NewManager(), auditLog, state, ecsClient, clusterName, statsEngine, config.DefaultTaskMetadataSteadyStateRate, config.DefaultTaskMetadataBurstRate, "", containerInstanceArn) @@ -1005,7 +1006,7 @@ func TestV3TaskStats(t *testing.T) { gomock.InOrder( state.EXPECT().TaskARNByV3EndpointID(v3EndpointID).Return(taskARN, true), state.EXPECT().ContainerMapByArn(taskARN).Return(containerMap, true), - statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, nil), + statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, &stats.NetworkStatsPerSec{}, nil), ) server := taskServerSetup(credentials.NewManager(), auditLog, state, ecsClient, clusterName, statsEngine, config.DefaultTaskMetadataSteadyStateRate, config.DefaultTaskMetadataBurstRate, "", containerInstanceArn) @@ -1038,7 +1039,7 @@ func TestV3ContainerStats(t *testing.T) { gomock.InOrder( state.EXPECT().TaskARNByV3EndpointID(v3EndpointID).Return(taskARN, true), state.EXPECT().DockerIDByV3EndpointID(v3EndpointID).Return(containerID, true), - statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, nil), + statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, &stats.NetworkStatsPerSec{}, nil), ) server := taskServerSetup(credentials.NewManager(), auditLog, state, ecsClient, clusterName, statsEngine, config.DefaultTaskMetadataSteadyStateRate, config.DefaultTaskMetadataBurstRate, "", containerInstanceArn) @@ -1337,7 +1338,7 @@ func TestV4TaskStats(t *testing.T) { gomock.InOrder( state.EXPECT().TaskARNByV3EndpointID(v3EndpointID).Return(taskARN, true), state.EXPECT().ContainerMapByArn(taskARN).Return(containerMap, true), - statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, nil), + statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, &stats.NetworkStatsPerSec{}, nil), ) server := taskServerSetup(credentials.NewManager(), auditLog, state, ecsClient, clusterName, statsEngine, config.DefaultTaskMetadataSteadyStateRate, config.DefaultTaskMetadataBurstRate, "", containerInstanceArn) @@ -1370,7 +1371,7 @@ func TestV4ContainerStats(t *testing.T) { gomock.InOrder( state.EXPECT().TaskARNByV3EndpointID(v3EndpointID).Return(taskARN, true), state.EXPECT().DockerIDByV3EndpointID(v3EndpointID).Return(containerID, true), - statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, nil), + statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, &stats.NetworkStatsPerSec{}, nil), ) server := taskServerSetup(credentials.NewManager(), auditLog, state, ecsClient, clusterName, statsEngine, config.DefaultTaskMetadataSteadyStateRate, config.DefaultTaskMetadataBurstRate, "", containerInstanceArn) diff --git a/agent/handlers/v2/stats_response.go b/agent/handlers/v2/stats_response.go index 43676cba9a6..9009519c82b 100644 --- a/agent/handlers/v2/stats_response.go +++ b/agent/handlers/v2/stats_response.go @@ -36,7 +36,7 @@ func NewTaskStatsResponse(taskARN string, resp := make(map[string]*types.StatsJSON) for _, dockerContainer := range containerMap { containerID := dockerContainer.DockerID - dockerStats, err := statsEngine.ContainerDockerStats(taskARN, containerID) + dockerStats, _, err := statsEngine.ContainerDockerStats(taskARN, containerID) if err != nil { seelog.Warnf("V2 task stats response: Unable to get stats for container '%s' for task '%s': %v", containerID, taskARN, err) diff --git a/agent/handlers/v2/stats_response_test.go b/agent/handlers/v2/stats_response_test.go index e6b96584b42..83b7ef0e07c 100644 --- a/agent/handlers/v2/stats_response_test.go +++ b/agent/handlers/v2/stats_response_test.go @@ -18,6 +18,8 @@ package v2 import ( "testing" + "github.com/aws/amazon-ecs-agent/agent/stats" + apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" mock_dockerstate "github.com/aws/amazon-ecs-agent/agent/engine/dockerstate/mocks" mock_stats "github.com/aws/amazon-ecs-agent/agent/stats/mock" @@ -42,7 +44,7 @@ func TestTaskStatsResponseSuccess(t *testing.T) { } gomock.InOrder( state.EXPECT().ContainerMapByArn(taskARN).Return(containerMap, true), - statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, nil), + statsEngine.EXPECT().ContainerDockerStats(taskARN, containerID).Return(dockerStats, &stats.NetworkStatsPerSec{}, nil), ) resp, err := NewTaskStatsResponse(taskARN, state, statsEngine) diff --git a/agent/handlers/v2/task_container_stats_handler.go b/agent/handlers/v2/task_container_stats_handler.go index 32470430570..65f21da9e3d 100644 --- a/agent/handlers/v2/task_container_stats_handler.go +++ b/agent/handlers/v2/task_container_stats_handler.go @@ -92,7 +92,7 @@ func WriteContainerStatsResponse(w http.ResponseWriter, taskARN string, containerID string, statsEngine stats.Engine) { - dockerStats, err := statsEngine.ContainerDockerStats(taskARN, containerID) + dockerStats, _, err := statsEngine.ContainerDockerStats(taskARN, containerID) if err != nil { errResponseJSON, err := json.Marshal("Unable to get container stats for: " + containerID) if e := utils.WriteResponseIfMarshalError(w, err); e != nil { diff --git a/agent/handlers/v4/container_stats_handler.go b/agent/handlers/v4/container_stats_handler.go index 4a0ff3c1bba..6da7478356b 100644 --- a/agent/handlers/v4/container_stats_handler.go +++ b/agent/handlers/v4/container_stats_handler.go @@ -20,7 +20,6 @@ import ( "github.com/aws/amazon-ecs-agent/agent/engine/dockerstate" "github.com/aws/amazon-ecs-agent/agent/handlers/utils" - v2 "github.com/aws/amazon-ecs-agent/agent/handlers/v2" v3 "github.com/aws/amazon-ecs-agent/agent/handlers/v3" "github.com/aws/amazon-ecs-agent/agent/stats" "github.com/cihub/seelog" @@ -54,6 +53,33 @@ func ContainerStatsHandler(state dockerstate.TaskEngineState, statsEngine stats. seelog.Infof("V4 container stats handler: writing response for container '%s'", containerID) // v4 handler shares the same container states response format with v2 handler. - v2.WriteContainerStatsResponse(w, taskArn, containerID, statsEngine) + WriteV4ContainerStatsResponse(w, taskArn, containerID, statsEngine) } } + +// WriteContainerStatsResponse writes the container stats to response writer. +func WriteV4ContainerStatsResponse(w http.ResponseWriter, + taskARN string, + containerID string, + statsEngine stats.Engine) { + dockerStats, network_rate_stats, err := statsEngine.ContainerDockerStats(taskARN, containerID) + if err != nil { + errResponseJSON, err := json.Marshal("Unable to get container stats for: " + containerID) + if e := utils.WriteResponseIfMarshalError(w, err); e != nil { + return + } + utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeContainerStats) + return + } + + containerStatsResponse := StatsResponse{ + StatsJSON: dockerStats, + Network_rate_stats: network_rate_stats, + } + + responseJSON, err := json.Marshal(containerStatsResponse) + if e := utils.WriteResponseIfMarshalError(w, err); e != nil { + return + } + utils.WriteJSONToResponse(w, http.StatusOK, responseJSON, utils.RequestTypeContainerStats) +} diff --git a/agent/handlers/v4/stats_response.go b/agent/handlers/v4/stats_response.go new file mode 100644 index 00000000000..ebaa914ca09 --- /dev/null +++ b/agent/handlers/v4/stats_response.go @@ -0,0 +1,63 @@ +// Copyright 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 v4 + +import ( + "github.com/aws/amazon-ecs-agent/agent/engine/dockerstate" + "github.com/aws/amazon-ecs-agent/agent/stats" + "github.com/cihub/seelog" + "github.com/docker/docker/api/types" + "github.com/pkg/errors" +) + +// StatsResponse is the v4 Stats response. It augments the v4 Stats response +// with the docker stats. +type StatsResponse struct { + *types.StatsJSON + Network_rate_stats *stats.NetworkStatsPerSec `json:"network_rate_stats,omitempty"` +} + +// NewV4TaskStatsResponse returns a new v4 task stats response object +func NewV4TaskStatsResponse(taskARN string, + state dockerstate.TaskEngineState, + statsEngine stats.Engine) (map[string]StatsResponse, error) { + + containerMap, ok := state.ContainerMapByArn(taskARN) + if !ok { + return nil, errors.Errorf( + "v4 task stats response: unable to lookup containers for task %s", + taskARN) + } + + resp := make(map[string]StatsResponse) + for _, dockerContainer := range containerMap { + containerID := dockerContainer.DockerID + dockerStats, network_rate_stats, err := statsEngine.ContainerDockerStats(taskARN, containerID) + if err != nil { + seelog.Warnf("V4 task stats response: Unable to get stats for container '%s' for task '%s': %v", + containerID, taskARN, err) + resp[containerID] = StatsResponse{} + continue + } + + statsResponse := StatsResponse{ + StatsJSON: dockerStats, + Network_rate_stats: network_rate_stats, + } + + resp[containerID] = statsResponse + } + + return resp, nil +} diff --git a/agent/handlers/v4/task_stats_handler.go b/agent/handlers/v4/task_stats_handler.go index 0b8db86d7a7..6536bf9e5db 100644 --- a/agent/handlers/v4/task_stats_handler.go +++ b/agent/handlers/v4/task_stats_handler.go @@ -20,7 +20,6 @@ import ( "github.com/aws/amazon-ecs-agent/agent/engine/dockerstate" "github.com/aws/amazon-ecs-agent/agent/handlers/utils" - v2 "github.com/aws/amazon-ecs-agent/agent/handlers/v2" v3 "github.com/aws/amazon-ecs-agent/agent/handlers/v3" "github.com/aws/amazon-ecs-agent/agent/stats" "github.com/cihub/seelog" @@ -40,7 +39,31 @@ func TaskStatsHandler(state dockerstate.TaskEngineState, statsEngine stats.Engin return } seelog.Infof("V4 tasks stats handler: writing response for task '%s'", taskArn) - // v4 handler shares with the same task response format with v2 handler - v2.WriteTaskStatsResponse(w, taskArn, state, statsEngine) + WriteV4TaskStatsResponse(w, taskArn, state, statsEngine) } } + +// WriteV4TaskStatsResponse writes the task stats to response writer. +func WriteV4TaskStatsResponse(w http.ResponseWriter, + taskARN string, + state dockerstate.TaskEngineState, + statsEngine stats.Engine) { + + taskStatsResponse, err := NewV4TaskStatsResponse(taskARN, state, statsEngine) + if err != nil { + seelog.Warnf("Unable to get task stats for task '%s': %v", taskARN, err) + errResponseJSON, err := json.Marshal("Unable to get task stats for: " + taskARN) + if e := utils.WriteResponseIfMarshalError(w, err); e != nil { + return + } + utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeTaskStats) + return + } + + responseJSON, err := json.Marshal(taskStatsResponse) + if e := utils.WriteResponseIfMarshalError(w, err); e != nil { + return + } + seelog.Infof("V4 Stats response json is %v", responseJSON) + utils.WriteJSONToResponse(w, http.StatusOK, responseJSON, utils.RequestTypeTaskStats) +} diff --git a/agent/stats/container.go b/agent/stats/container.go index 11dc85231eb..2880791431d 100644 --- a/agent/stats/container.go +++ b/agent/stats/container.go @@ -27,7 +27,8 @@ import ( "github.com/cihub/seelog" ) -func newStatsContainer(dockerID string, client dockerapi.DockerClient, resolver resolver.ContainerMetadataResolver, cfg *config.Config) (*StatsContainer, error) { +func newStatsContainer(dockerID string, client dockerapi.DockerClient, resolver resolver.ContainerMetadataResolver, + cfg *config.Config) (*StatsContainer, error) { dockerContainer, err := resolver.ResolveContainer(dockerID) if err != nil { return nil, err @@ -50,7 +51,7 @@ func newStatsContainer(dockerID string, client dockerapi.DockerClient, resolver func (container *StatsContainer) StartStatsCollection() { // queue will be sized to hold enough stats for 4 publishing intervals. var queueSize int - if container.config != nil && container.config.PollMetrics { + if container.config != nil && container.config.PollMetrics.Enabled() { pollingInterval := container.config.PollingMetricsWaitDuration.Seconds() queueSize = int(config.DefaultContainerMetricsPublishInterval.Seconds() / pollingInterval * 4) } else { @@ -126,6 +127,11 @@ func (container *StatsContainer) processStatsStream() error { } return nil } + err := validateDockerStats(rawStat) + if err != nil { + return err + } + if err := container.statsQueue.Add(rawStat); err != nil { seelog.Warnf("Container [%s]: error converting stats for container: %v", dockerID, err) } diff --git a/agent/stats/engine.go b/agent/stats/engine.go index 6f9fb951aba..58fcb33cb2b 100644 --- a/agent/stats/engine.go +++ b/agent/stats/engine.go @@ -18,6 +18,7 @@ package stats import ( "context" "fmt" + "strconv" "sync" "time" @@ -65,12 +66,13 @@ type DockerContainerMetadataResolver struct { // defined to make testing easier. type Engine interface { GetInstanceMetrics() (*ecstcs.MetricsMetadata, []*ecstcs.TaskMetric, error) - ContainerDockerStats(taskARN string, containerID string) (*types.StatsJSON, error) + ContainerDockerStats(taskARN string, containerID string) (*types.StatsJSON, *NetworkStatsPerSec, error) GetTaskHealthMetrics() (*ecstcs.HealthMetadata, []*ecstcs.TaskHealth, error) } // DockerStatsEngine is used to monitor docker container events and to report -// utlization metrics of the same. +// utilization metrics of the same. + type DockerStatsEngine struct { ctx context.Context stopEngine context.CancelFunc @@ -87,6 +89,7 @@ type DockerStatsEngine struct { tasksToHealthCheckContainers map[string]map[string]*StatsContainer // tasksToDefinitions maps task arns to task definition name and family metadata objects. tasksToDefinitions map[string]*taskDefinition + taskToTaskStats map[string]*StatsTask } // ResolveTask resolves the api task object, given container id. @@ -102,6 +105,18 @@ func (resolver *DockerContainerMetadataResolver) ResolveTask(dockerID string) (* return task, nil } +func (resolver *DockerContainerMetadataResolver) ResolveTaskByARN(taskArn string) (*apitask.Task, error) { + if resolver.dockerTaskEngine == nil { + return nil, fmt.Errorf("docker task engine uninitialized") + } + task, found := resolver.dockerTaskEngine.State().TaskByArn(taskArn) + if !found { + return nil, fmt.Errorf("could not map task arn to task: %s", taskArn) + } + return task, nil + +} + // ResolveContainer resolves the api container object, given container id. func (resolver *DockerContainerMetadataResolver) ResolveContainer(dockerID string) (*apicontainer.DockerContainer, error) { if resolver.dockerTaskEngine == nil { @@ -125,6 +140,7 @@ func NewDockerStatsEngine(cfg *config.Config, client dockerapi.DockerClient, con tasksToContainers: make(map[string]map[string]*StatsContainer), tasksToHealthCheckContainers: make(map[string]map[string]*StatsContainer), tasksToDefinitions: make(map[string]*taskDefinition), + taskToTaskStats: make(map[string]*StatsTask), containerChangeEventStream: containerChangeEventStream, } } @@ -139,6 +155,7 @@ func (engine *DockerStatsEngine) synchronizeState() error { for _, containerID := range listContainersResponse.DockerIDs { engine.addAndStartStatsContainer(containerID) } + return nil } @@ -146,17 +163,38 @@ func (engine *DockerStatsEngine) synchronizeState() error { func (engine *DockerStatsEngine) addAndStartStatsContainer(containerID string) { engine.lock.Lock() defer engine.lock.Unlock() - statsContainer, err := engine.addContainerUnsafe(containerID) + statsContainer, statsTaskContainer, err := engine.addContainerUnsafe(containerID) if err != nil { seelog.Debugf("Adding container to stats watch list failed, container: %s, err: %v", containerID, err) return } - if engine.config.DisableMetrics || statsContainer == nil { + if engine.config.DisableMetrics.Enabled() || statsContainer == nil { return } statsContainer.StartStatsCollection() + + task, err := engine.resolver.ResolveTask(containerID) + if err != nil { + return + } + + dockerContainer, errResolveContainer := engine.resolver.ResolveContainer(containerID) + if errResolveContainer != nil { + seelog.Debugf("Could not map container ID to container, container: %s, err: %s", containerID, err) + return + } + + if task.IsNetworkModeAWSVPC() { + // Start stats collector only for pause container + if statsTaskContainer != nil && dockerContainer.Container.Type == apicontainer.ContainerCNIPause { + statsTaskContainer.StartStatsCollection() + } else { + seelog.Debugf("stats task container is nil, cannot start task stats collection") + } + } + } // MustInit initializes fields of the DockerStatsEngine object. @@ -181,7 +219,6 @@ func (engine *DockerStatsEngine) MustInit(ctx context.Context, taskEngine ecseng if err != nil { return fmt.Errorf("Failed to subscribe to container change event stream, err %v", err) } - err = engine.synchronizeState() if err != nil { seelog.Warnf("Synchronize the container state failed, err: %v", err) @@ -191,7 +228,7 @@ func (engine *DockerStatsEngine) MustInit(ctx context.Context, taskEngine ecseng return nil } -// Shutdown cleans up the resources after the statas engine. +// Shutdown cleans up the resources after the stats engine. func (engine *DockerStatsEngine) Shutdown() { engine.stopEngine() engine.Disable() @@ -228,49 +265,83 @@ func (engine *DockerStatsEngine) removeAll() { } } +func (engine *DockerStatsEngine) addToStatsTaskMapUnsafe(task *apitask.Task, dockerContainerName string, + containerType apicontainer.ContainerType) { + var statsTaskContainer *StatsTask + if task.IsNetworkModeAWSVPC() && containerType == apicontainer.ContainerCNIPause { + // Excluding the pause container + numberOfContainers := len(task.Containers) - 1 + var taskExists bool + statsTaskContainer, taskExists = engine.taskToTaskStats[task.Arn] + if !taskExists { + containerInspect, err := engine.client.InspectContainer(engine.ctx, dockerContainerName, + dockerclient.InspectContainerTimeout) + if err != nil { + return + } + containerpid := strconv.Itoa(containerInspect.State.Pid) + statsTaskContainer, err = newStatsTaskContainer(task.Arn, containerpid, numberOfContainers, + engine.resolver, engine.config.PollingMetricsWaitDuration) + if err != nil { + return + } + engine.taskToTaskStats[task.Arn] = statsTaskContainer + } else { + statsTaskContainer.TaskMetadata.NumberContainers = numberOfContainers + } + } +} + // addContainerUnsafe adds a container to the map of containers being watched. -func (engine *DockerStatsEngine) addContainerUnsafe(dockerID string) (*StatsContainer, error) { +func (engine *DockerStatsEngine) addContainerUnsafe(dockerID string) (*StatsContainer, *StatsTask, error) { // Make sure that this container belongs to a task and that the task // is not terminal. task, err := engine.resolver.ResolveTask(dockerID) if err != nil { - return nil, errors.Wrapf(err, "could not map container to task, ignoring container: %s", dockerID) + return nil, nil, errors.Wrapf(err, "could not map container to task, ignoring container: %s", dockerID) } if len(task.Arn) == 0 || len(task.Family) == 0 { - return nil, errors.Errorf("stats add container: invalid task fields, arn: %s, familiy: %s", task.Arn, task.Family) + return nil, nil, errors.Errorf("stats add container: invalid task fields, arn: %s, familiy: %s", task.Arn, task.Family) } if task.GetKnownStatus().Terminal() { - return nil, errors.Errorf("stats add container: task is terminal, ignoring container: %s, task: %s", dockerID, task.Arn) + return nil, nil, errors.Errorf("stats add container: task is terminal, ignoring container: %s, task: %s", dockerID, task.Arn) } statsContainer, err := newStatsContainer(dockerID, engine.client, engine.resolver, engine.config) if err != nil { - return nil, errors.Wrapf(err, "could not map docker container ID to container, ignoring container: %s", dockerID) + return nil, nil, errors.Wrapf(err, "could not map docker container ID to container, ignoring container: %s", dockerID) } seelog.Debugf("Adding container to stats watch list, id: %s, task: %s", dockerID, task.Arn) engine.tasksToDefinitions[task.Arn] = &taskDefinition{family: task.Family, version: task.Version} + dockerContainer, errResolveContainer := engine.resolver.ResolveContainer(dockerID) + if errResolveContainer != nil { + seelog.Debugf("Could not map container ID to container, container: %s, err: %s", dockerID, err) + } + watchStatsContainer := false - if !engine.config.DisableMetrics { + if !engine.config.DisableMetrics.Enabled() { // Adding container to the map for collecting stats - watchStatsContainer = engine.addToStatsContainerMapUnsafe(task.Arn, dockerID, statsContainer, engine.containerMetricsMapUnsafe) + watchStatsContainer = engine.addToStatsContainerMapUnsafe(task.Arn, dockerID, statsContainer, + engine.containerMetricsMapUnsafe) + if errResolveContainer == nil { + engine.addToStatsTaskMapUnsafe(task, dockerContainer.DockerName, dockerContainer.Container.Type) + } } - if dockerContainer, err := engine.resolver.ResolveContainer(dockerID); err != nil { - seelog.Debugf("Could not map container ID to container, container: %s, err: %s", dockerID, err) - } else if dockerContainer.Container.HealthStatusShouldBeReported() { + if errResolveContainer == nil && dockerContainer.Container.HealthStatusShouldBeReported() { // Track the container health status engine.addToStatsContainerMapUnsafe(task.Arn, dockerID, statsContainer, engine.healthCheckContainerMapUnsafe) seelog.Debugf("Adding container to stats health check watch list, id: %s, task: %s", dockerID, task.Arn) } if !watchStatsContainer { - return nil, nil + return nil, nil, nil } - return statsContainer, nil + return statsContainer, engine.taskToTaskStats[task.Arn], nil } func (engine *DockerStatsEngine) containerMetricsMapUnsafe() map[string]map[string]*StatsContainer { @@ -568,7 +639,7 @@ func newDockerContainerMetadataResolver(taskEngine ecsengine.TaskEngine) (*Docke func (engine *DockerStatsEngine) taskContainerMetricsUnsafe(taskArn string) ([]*ecstcs.ContainerMetric, error) { containerMap, taskExists := engine.tasksToContainers[taskArn] if !taskExists { - return nil, fmt.Errorf("Task not found") + return nil, fmt.Errorf("task not found") } var containerMetrics []*ecstcs.ContainerMetric @@ -593,6 +664,7 @@ func (engine *DockerStatsEngine) taskContainerMetricsUnsafe(taskArn string) ([]* seelog.Infof("cloudwatch metrics for container %v not collected, reason (memory): %v", dockerID, err) continue } + containerMetric := &ecstcs.ContainerMetric{ ContainerName: &container.containerMetadata.Name, CpuStatsSet: cpuStatsSet, @@ -610,9 +682,8 @@ func (engine *DockerStatsEngine) taskContainerMetricsUnsafe(taskArn string) ([]* if err != nil { seelog.Warnf("Task not found for container ID: %s", dockerID) } else { - // send network stats for default/bridge/nat network modes - if !task.IsNetworkModeAWSVPC() && - container.containerMetadata.NetworkMode != hostNetworkMode && + // send network stats for default/bridge/nat/awsvpc network modes + if !task.IsNetworkModeAWSVPC() && container.containerMetadata.NetworkMode != hostNetworkMode && container.containerMetadata.NetworkMode != noneNetworkMode { networkStatsSet, err := container.statsQueue.GetNetworkStatsSet() if err != nil { @@ -621,9 +692,19 @@ func (engine *DockerStatsEngine) taskContainerMetricsUnsafe(taskArn string) ([]* } else { containerMetric.NetworkStatsSet = networkStatsSet } + } else if task.IsNetworkModeAWSVPC() { + taskStatsMap, taskExistsInTaskStats := engine.taskToTaskStats[taskArn] + if !taskExistsInTaskStats { + return nil, fmt.Errorf("task not found") + } + networkStats, err := taskStatsMap.StatsQueue.GetNetworkStatsSet() + if err != nil { + seelog.Warnf("error getting network stats: %v, task: %v", err, taskArn) + } else { + containerMetric.NetworkStatsSet = networkStats + } } } - containerMetrics = append(containerMetrics, containerMetric) } @@ -667,19 +748,41 @@ func (engine *DockerStatsEngine) resetStatsUnsafe() { } // ContainerDockerStats returns the last stored raw docker stats object for a container -func (engine *DockerStatsEngine) ContainerDockerStats(taskARN string, containerID string) (*types.StatsJSON, error) { +func (engine *DockerStatsEngine) ContainerDockerStats(taskARN string, containerID string) (*types.StatsJSON, *NetworkStatsPerSec, error) { engine.lock.RLock() defer engine.lock.RUnlock() containerIDToStatsContainer, ok := engine.tasksToContainers[taskARN] + taskToTaskStats := engine.taskToTaskStats if !ok { - return nil, errors.Errorf("stats engine: task '%s' for container '%s' not found", + return nil, nil, errors.Errorf("stats engine: task '%s' for container '%s' not found", taskARN, containerID) } container, ok := containerIDToStatsContainer[containerID] if !ok { - return nil, errors.Errorf("stats engine: container not found: %s", containerID) + return nil, nil, errors.Errorf("stats engine: container not found: %s", containerID) + } + containerStats := container.statsQueue.GetLastStat() + containerNetworkRateStats := container.statsQueue.GetLastNetworkStatPerSec() + + // Insert network stats in container stats + task, err := engine.resolver.ResolveTaskByARN(taskARN) + if err != nil { + return nil, nil, errors.Errorf("stats engine: task '%s' not found", + taskARN) + } + + if task.IsNetworkModeAWSVPC() { + taskStats, ok := taskToTaskStats[taskARN] + if ok { + taskNetworkStats := taskStats.StatsQueue.GetLastStat().Networks + containerStats.Networks = taskNetworkStats + containerNetworkRateStats = taskStats.StatsQueue.GetLastNetworkStatPerSec() + } else { + seelog.Warnf("Network stats not found for container %s", containerID) + } } - return container.statsQueue.GetLastStat(), nil + + return containerStats, containerNetworkRateStats, nil } diff --git a/agent/stats/engine_integ_test.go b/agent/stats/engine_integ_test.go index 683249102f9..85ba1ff3071 100644 --- a/agent/stats/engine_integ_test.go +++ b/agent/stats/engine_integ_test.go @@ -342,7 +342,7 @@ func TestStatsEngineWithNewContainers(t *testing.T) { func TestStatsEngineWithNewContainersWithPolling(t *testing.T) { // additional config fields to use polling instead of stream - cfg.PollMetrics = true + cfg.PollMetrics = config.BooleanDefaultTrue{Value: config.ExplicitlyEnabled} cfg.PollingMetricsWaitDuration = 1 * time.Second // Create a new docker client with new config dockerClientForNewContainersWithPolling, _ := dockerapi.NewDockerGoClient(sdkClientFactory, &cfg, ctx) @@ -422,7 +422,7 @@ func TestStatsEngineWithNewContainersWithPolling(t *testing.T) { validateEmptyTaskHealthMetrics(t, engine) // reset cfg, currently cfg is shared by all tests. - cfg.PollMetrics = false + cfg.PollMetrics = config.BooleanDefaultTrue{Value: config.ExplicitlyDisabled} cfg.PollingMetricsWaitDuration = config.DefaultPollingMetricsWaitDuration } diff --git a/agent/stats/engine_test.go b/agent/stats/engine_test.go index c4d19a2f753..352655df97c 100644 --- a/agent/stats/engine_test.go +++ b/agent/stats/engine_test.go @@ -26,6 +26,7 @@ import ( apieni "github.com/aws/amazon-ecs-agent/agent/api/eni" apitask "github.com/aws/amazon-ecs-agent/agent/api/task" apitaskstatus "github.com/aws/amazon-ecs-agent/agent/api/task/status" + "github.com/aws/amazon-ecs-agent/agent/config" "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi" mock_dockerapi "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi/mocks" mock_resolver "github.com/aws/amazon-ecs-agent/agent/stats/resolver/mock" @@ -181,6 +182,7 @@ func TestStatsEngineMetadataInStatsSets(t *testing.T) { }, }, nil) mockDockerClient.EXPECT().Stats(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + resolver.EXPECT().ResolveTaskByARN(gomock.Any()).Return(t1, nil).AnyTimes() engine := NewDockerStatsEngine(&cfg, nil, eventStream("TestStatsEngineMetadataInStatsSets")) ctx, cancel := context.WithCancel(context.TODO()) @@ -223,7 +225,7 @@ func TestStatsEngineMetadataInStatsSets(t *testing.T) { t.Errorf("Error validating metadata: %v", err) } - dockerStat, err := engine.ContainerDockerStats("t1", "c1") + dockerStat, _, err := engine.ContainerDockerStats("t1", "c1") assert.NoError(t, err) assert.Equal(t, ts2, dockerStat.Read) @@ -363,7 +365,7 @@ func TestGetTaskHealthMetricsStoppedContainer(t *testing.T) { // but will track container health when metrics is disabled in agent. func TestMetricsDisabled(t *testing.T) { disableMetricsConfig := cfg - disableMetricsConfig.DisableMetrics = true + disableMetricsConfig.DisableMetrics = config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled} containerID := "containerID" mockCtrl := gomock.NewController(t) @@ -415,17 +417,20 @@ func TestSynchronizeOnRestart(t *testing.T) { statsStarted <- struct{}{} }).Return(statsChan, nil) - resolver.EXPECT().ResolveTask(containerID).Return(&apitask.Task{ + testTask := &apitask.Task{ Arn: "t1", KnownStatusUnsafe: apitaskstatus.TaskRunning, Family: "f1", - }, nil) + } + + resolver.EXPECT().ResolveTask(containerID).Return(testTask, nil).Times(2) + resolver.EXPECT().ResolveTaskByARN(gomock.Any()).Return(testTask, nil).AnyTimes() resolver.EXPECT().ResolveContainer(containerID).Return(&apicontainer.DockerContainer{ DockerID: containerID, Container: &apicontainer.Container{ HealthCheckType: "docker", }, - }, nil).Times(2) + }, nil).Times(3) err := engine.synchronizeState() assert.NoError(t, err) @@ -456,20 +461,38 @@ func testNetworkModeStats(t *testing.T, netMode string, enis []*apieni.ENI, empt defer mockCtrl.Finish() resolver := mock_resolver.NewMockContainerMetadataResolver(mockCtrl) mockDockerClient := mock_dockerapi.NewMockDockerClient(mockCtrl) - t1 := &apitask.Task{ - Arn: "t1", - Family: "f1", - ENIs: enis, - } - resolver.EXPECT().ResolveTask("c1").AnyTimes().Return(t1, nil) - resolver.EXPECT().ResolveContainer(gomock.Any()).AnyTimes().Return(&apicontainer.DockerContainer{ + + testContainer := &apicontainer.DockerContainer{ Container: &apicontainer.Container{ Name: "test", NetworkModeUnsafe: netMode, + Type: apicontainer.ContainerCNIPause, }, - }, nil) + } + + t1 := &apitask.Task{ + Arn: "t1", + Family: "f1", + ENIs: enis, + KnownStatusUnsafe: apitaskstatus.TaskRunning, + Containers: []*apicontainer.Container{ + {Name: "test"}, + {Name: "test1"}, + }, + } + + resolver.EXPECT().ResolveTask("c1").AnyTimes().Return(t1, nil) + resolver.EXPECT().ResolveTaskByARN(gomock.Any()).Return(t1, nil).AnyTimes() + + resolver.EXPECT().ResolveContainer(gomock.Any()).AnyTimes().Return(testContainer, nil) mockDockerClient.EXPECT().Stats(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + mockDockerClient.EXPECT().InspectContainer(gomock.Any(), gomock.Any(), gomock.Any()).Return(&types.ContainerJSON{ + ContainerJSONBase: &types.ContainerJSONBase{ + ID: "test", + State: &types.ContainerState{Pid: 23}, + }, + }, nil).AnyTimes() engine := NewDockerStatsEngine(&cfg, nil, eventStream("TestTaskNetworkStatsSet")) ctx, cancel := context.WithCancel(context.TODO()) defer cancel() diff --git a/agent/stats/engine_unix_test.go b/agent/stats/engine_unix_test.go index c06599d909b..39420ba9d40 100644 --- a/agent/stats/engine_unix_test.go +++ b/agent/stats/engine_unix_test.go @@ -1,4 +1,4 @@ -//+build unit +//+build linux,unit // Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. // @@ -16,9 +16,18 @@ package stats import ( + "context" "testing" + apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" apieni "github.com/aws/amazon-ecs-agent/agent/api/eni" + apitask "github.com/aws/amazon-ecs-agent/agent/api/task" + apitaskstatus "github.com/aws/amazon-ecs-agent/agent/api/task/status" + mock_dockerapi "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi/mocks" + mock_resolver "github.com/aws/amazon-ecs-agent/agent/stats/resolver/mock" + "github.com/docker/docker/api/types" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" ) func TestLinuxTaskNetworkStatsSet(t *testing.T) { @@ -36,3 +45,69 @@ func TestLinuxTaskNetworkStatsSet(t *testing.T) { testNetworkModeStats(t, tc.NetworkMode, tc.ENIs, tc.StatsEmpty) } } + +func TestNetworkModeStatsAWSVPCMode(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + resolver := mock_resolver.NewMockContainerMetadataResolver(mockCtrl) + mockDockerClient := mock_dockerapi.NewMockDockerClient(mockCtrl) + + testContainer := &apicontainer.DockerContainer{ + Container: &apicontainer.Container{ + Name: "test", + Type: apicontainer.ContainerCNIPause, + }, + } + + t1 := &apitask.Task{ + Arn: "t1", + Family: "f1", + ENIs: []*apieni.ENI{{ID: "ec2Id"}}, + KnownStatusUnsafe: apitaskstatus.TaskRunning, + Containers: []*apicontainer.Container{ + {Name: "test"}, + {Name: "test1"}, + }, + } + + resolver.EXPECT().ResolveTask("c1").AnyTimes().Return(t1, nil) + resolver.EXPECT().ResolveTaskByARN(gomock.Any()).Return(t1, nil).AnyTimes() + + resolver.EXPECT().ResolveContainer(gomock.Any()).AnyTimes().Return(testContainer, nil) + mockDockerClient.EXPECT().Stats(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + + mockDockerClient.EXPECT().InspectContainer(gomock.Any(), gomock.Any(), gomock.Any()).Return(&types.ContainerJSON{ + ContainerJSONBase: &types.ContainerJSONBase{ + ID: "test", + State: &types.ContainerState{Pid: 23}, + }, + }, nil).AnyTimes() + engine := NewDockerStatsEngine(&cfg, nil, eventStream("TestTaskNetworkStatsSet")) + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + engine.ctx = ctx + engine.resolver = resolver + engine.cluster = defaultCluster + engine.containerInstanceArn = defaultContainerInstance + engine.client = mockDockerClient + engine.addAndStartStatsContainer("c1") + ts1 := parseNanoTime("2015-02-12T21:22:05.131117533Z") + containerStats := createFakeContainerStats() + dockerStats := []*types.StatsJSON{{}, {}} + dockerStats[0].Read = ts1 + containers, _ := engine.tasksToContainers["t1"] + taskContainers, _ := engine.taskToTaskStats["t1"] + for _, statsContainer := range containers { + for i := 0; i < 2; i++ { + statsContainer.statsQueue.add(containerStats[i]) + statsContainer.statsQueue.setLastStat(dockerStats[i]) + taskContainers.StatsQueue.add(containerStats[i]) + } + } + _, taskMetrics, err := engine.GetInstanceMetrics() + assert.NoError(t, err) + assert.Len(t, taskMetrics, 1) + for _, containerMetric := range taskMetrics[0].ContainerMetrics { + assert.NotNil(t, containerMetric.NetworkStatsSet, "network stats should be non-empty") + } +} diff --git a/agent/stats/mock/engine.go b/agent/stats/mock/engine.go index 3b037f3dba5..d829b4a6325 100644 --- a/agent/stats/mock/engine.go +++ b/agent/stats/mock/engine.go @@ -21,6 +21,7 @@ package mock_stats import ( reflect "reflect" + "github.com/aws/amazon-ecs-agent/agent/stats" ecstcs "github.com/aws/amazon-ecs-agent/agent/tcs/model/ecstcs" types "github.com/docker/docker/api/types" gomock "github.com/golang/mock/gomock" @@ -50,12 +51,13 @@ func (m *MockEngine) EXPECT() *MockEngineMockRecorder { } // ContainerDockerStats mocks base method -func (m *MockEngine) ContainerDockerStats(arg0, arg1 string) (*types.StatsJSON, error) { +func (m *MockEngine) ContainerDockerStats(arg0, arg1 string) (*types.StatsJSON, *stats.NetworkStatsPerSec, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ContainerDockerStats", arg0, arg1) ret0, _ := ret[0].(*types.StatsJSON) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret1, _ := ret[1].(*stats.NetworkStatsPerSec) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // ContainerDockerStats indicates an expected call of ContainerDockerStats diff --git a/agent/stats/queue.go b/agent/stats/queue.go index b7ba3500f4d..d06e26bb107 100644 --- a/agent/stats/queue.go +++ b/agent/stats/queue.go @@ -28,14 +28,16 @@ const ( // BytesInMiB is the number of bytes in a MebiByte. BytesInMiB = 1024 * 1024 MaxCPUUsagePerc float32 = 1024 * 1024 + NanoSecToSec float32 = 1000000000 ) // Queue abstracts a queue using UsageStats slice. type Queue struct { - buffer []UsageStats - maxSize int - lastStat *types.StatsJSON - lock sync.RWMutex + buffer []UsageStats + maxSize int + lastStat *types.StatsJSON + lastNetworkStatPerSec *NetworkStatsPerSec + lock sync.RWMutex } // NewQueue creates a queue. @@ -98,9 +100,21 @@ func (queue *Queue) add(rawStat *ContainerStats) { // if we got a duplicate timestamp, set cpu percentage to the same value as the previous stat seelog.Errorf("Received a docker stat object with duplicate timestamp") stat.CPUUsagePerc = lastStat.CPUUsagePerc + if stat.NetworkStats != nil { + stat.NetworkStats.RxBytesPerSecond = lastStat.NetworkStats.RxBytesPerSecond + stat.NetworkStats.TxBytesPerSecond = lastStat.NetworkStats.TxBytesPerSecond + } } else { cpuUsageSinceLastStat := float32(rawStat.cpuUsage - lastStat.cpuUsage) stat.CPUUsagePerc = 100 * cpuUsageSinceLastStat / timeSinceLastStat + + //calculate per second Network metrics + if stat.NetworkStats != nil { + rxBytesSinceLastStat := float32(stat.NetworkStats.RxBytes - lastStat.NetworkStats.RxBytes) + txBytesSinceLastStat := float32(stat.NetworkStats.TxBytes - lastStat.NetworkStats.TxBytes) + stat.NetworkStats.RxBytesPerSecond = NanoSecToSec * (rxBytesSinceLastStat / timeSinceLastStat) + stat.NetworkStats.TxBytesPerSecond = NanoSecToSec * (txBytesSinceLastStat / timeSinceLastStat) + } } if queueLength >= queue.maxSize { @@ -113,6 +127,14 @@ func (queue *Queue) add(rawStat *ContainerStats) { seelog.Errorf("Calculated CPU usage percent (%.1f) is larger than backend maximum (%.1f). lastStatTS=%s lastStatCPUTime=%d thisStatTS=%s thisStatCPUTime=%d queueLength=%d", stat.CPUUsagePerc, MaxCPUUsagePerc, lastStat.Timestamp.Format(time.RFC3339Nano), lastStat.cpuUsage, rawStat.timestamp.Format(time.RFC3339Nano), rawStat.cpuUsage, queueLength) } + + if stat.NetworkStats != nil { + networkStatPerSec := &NetworkStatsPerSec{ + RxBytesPerSecond: stat.NetworkStats.RxBytesPerSecond, + TxBytesPerSecond: stat.NetworkStats.TxBytesPerSecond, + } + queue.lastNetworkStatPerSec = networkStatPerSec + } } queue.buffer = append(queue.buffer, stat) @@ -126,6 +148,13 @@ func (queue *Queue) GetLastStat() *types.StatsJSON { return queue.lastStat } +func (queue *Queue) GetLastNetworkStatPerSec() *NetworkStatsPerSec { + queue.lock.RLock() + defer queue.lock.RUnlock() + + return queue.lastNetworkStatPerSec +} + // GetCPUStatsSet gets the stats set for CPU utilization. func (queue *Queue) GetCPUStatsSet() (*ecstcs.CWStatsSet, error) { return queue.getCWStatsSet(getCPUUsagePerc) @@ -187,6 +216,14 @@ func (queue *Queue) GetNetworkStatsSet() (*ecstcs.NetworkStatsSet, error) { if err != nil { seelog.Warnf("Error getting network tx packets: %v", err) } + networkStatsSet.RxBytesPerSecond, err = queue.getUDoubleCWStatsSet(getNetworkRxPacketsPerSecond) + if err != nil { + seelog.Warnf("Error getting network rx bytes per second: %v", err) + } + networkStatsSet.TxBytesPerSecond, err = queue.getUDoubleCWStatsSet(getNetworkTxPacketsPerSecond) + if err != nil { + seelog.Warnf("Error getting network tx bytes per second: %v", err) + } return networkStatsSet, err } @@ -246,6 +283,20 @@ func getNetworkTxPackets(s *UsageStats) uint64 { return uint64(0) } +func getNetworkRxPacketsPerSecond(s *UsageStats) float64 { + if s.NetworkStats != nil { + return float64(s.NetworkStats.RxBytesPerSecond) + } + return float64(0) +} + +func getNetworkTxPacketsPerSecond(s *UsageStats) float64 { + if s.NetworkStats != nil { + return float64(s.NetworkStats.TxBytesPerSecond) + } + return float64(0) +} + func getCPUUsagePerc(s *UsageStats) float64 { return float64(s.CPUUsagePerc) } @@ -379,3 +430,50 @@ func (queue *Queue) getULongStatsSet(getUsageInt getUsageIntFunc) (*ecstcs.ULong OverflowSum: &overflowSum, }, nil } + +// getUDoubleCWStatsSet gets the stats set for per second network metrics +func (queue *Queue) getUDoubleCWStatsSet(getUsageFloat getUsageFloatFunc) (*ecstcs.UDoubleCWStatsSet, error) { + queue.lock.Lock() + defer queue.lock.Unlock() + + queueLength := len(queue.buffer) + if queueLength < 2 { + // Need at least 2 data points to calculate this. + return nil, fmt.Errorf("need at least 2 data points in queue to calculate CW stats set") + } + + var min, max, sum float64 + var sampleCount int64 + min = math.MaxFloat64 + max = -math.MaxFloat64 + sum = 0 + sampleCount = 0 + + for _, stat := range queue.buffer { + if stat.sent { + // don't send stats to TACS if already sent + continue + } + thisStat := getUsageFloat(&stat) + if math.IsNaN(thisStat) { + continue + } + + min = math.Min(min, thisStat) + max = math.Max(max, thisStat) + sampleCount++ + sum += thisStat + } + + // don't emit metrics when sampleCount == 0 + if sampleCount == 0 { + return nil, fmt.Errorf("need at least 1 non-NaN data points in queue to calculate CW stats set") + } + + return &ecstcs.UDoubleCWStatsSet{ + Max: &max, + Min: &min, + SampleCount: &sampleCount, + Sum: &sum, + }, nil +} diff --git a/agent/stats/queue_test.go b/agent/stats/queue_test.go index 9899ac22bc0..dc3675929e5 100644 --- a/agent/stats/queue_test.go +++ b/agent/stats/queue_test.go @@ -314,6 +314,18 @@ func validateNetStatsSet(t *testing.T, netStats *ecstcs.NetworkStatsSet, queueLe assert.Equal(t, int64(queueLen), *netStats.TxErrors.SampleCount, "incorrect TxErrors sampleCount") assert.Equal(t, int64(0), *netStats.TxErrors.Sum, "incorrect TxErrors sum") assert.Equal(t, int64(0), *netStats.TxErrors.OverflowSum, "incorrect TxErrors overlfowSum") + + assert.NotNil(t, *netStats.RxBytesPerSecond, "incorrect RxBytesPerSecond set") + assert.Equal(t, float64(1.26999248e+08), *netStats.RxBytesPerSecond.Min, "incorrect RxBytesPerSecond min") + assert.Equal(t, float64(1.373376384e+09), *netStats.RxBytesPerSecond.Max, "incorrect RxBytesPerSecond max") + assert.Equal(t, int64(queueLen), *netStats.RxBytesPerSecond.SampleCount, "incorrect RxBytesPerSecond sampleCount") + assert.Equal(t, float64(5.540383824e+09), *netStats.RxBytesPerSecond.Sum, "incorrect RxBytesPerSecond sum") + + assert.NotNil(t, *netStats.TxBytesPerSecond, "incorrect TxBytesPerSecond set") + assert.Equal(t, float64(1.26999248e+08), *netStats.TxBytesPerSecond.Min, "incorrect TxBytesPerSecond min") + assert.Equal(t, float64(1.373376384e+09), *netStats.TxBytesPerSecond.Max, "incorrect TxBytesPerSecond max") + assert.Equal(t, int64(queueLen), *netStats.TxBytesPerSecond.SampleCount, "incorrect TxBytesPerSecond sampleCount") + assert.Equal(t, float64(5.540383824e+09), *netStats.TxBytesPerSecond.Sum, "incorrect TxBytesPerSecond sum") } func TestQueueUintStats(t *testing.T) { @@ -510,3 +522,197 @@ func TestHugeCPUUsagePercentDoesntGetCapped(t *testing.T) { require.Equal(t, int64(2), *statSet.SampleCount) require.Equal(t, float64(30000001124), *statSet.Sum) } + +// If there are only 2 datapoints, and both have the same timestamp, +// then sample count will be 0 for per sec metrics and GetNetworkStats should return error +func TestPerSecNetworkStatSetFailsWhenSampleCountIsZero(t *testing.T) { + timestamps := []time.Time{ + parseNanoTime("2015-02-12T21:22:05.131117533Z"), + parseNanoTime("2015-02-12T21:22:05.131117533Z"), + } + cpuTimes := []uint64{ + 22400432, + 116499979, + } + memoryUtilizationInBytes := []uint64{ + 3649536, + 3649536, + } + + bytesReceivedTransmitted := []uint64{ + 364953689, + 364953689, + } + + queue := NewQueue(3) + + for i, time := range timestamps { + queue.add(&ContainerStats{ + cpuUsage: cpuTimes[i], + memoryUsage: memoryUtilizationInBytes[i], + networkStats: &NetworkStats{ + RxBytes: bytesReceivedTransmitted[i], + RxDropped: 0, + RxErrors: bytesReceivedTransmitted[i], + RxPackets: bytesReceivedTransmitted[i], + TxBytes: bytesReceivedTransmitted[i], + TxDropped: bytesReceivedTransmitted[i], + TxErrors: 0, + TxPackets: bytesReceivedTransmitted[i], + RxBytesPerSecond: float32(nan32()), + TxBytesPerSecond: float32(nan32()), + }, + timestamp: time}) + } + + // if we have identical timestamps and 2 datapoints + // then there will not be enough valid network stats to create + // a valid network stats set, and this function call should fail. + stats, err := queue.GetNetworkStatsSet() + require.Errorf(t, err, "Received unexpected network stats set %v", stats) +} + +// If there are only 3 datapoints in total and among them 2 are identical, then GetNetworkStats should not return error +func TestPerSecNetworkStatSetPassWithThreeDatapoints(t *testing.T) { + timestamps := []time.Time{ + parseNanoTime("2015-02-12T21:22:05.131117533Z"), + parseNanoTime("2015-02-12T21:22:05.131117533Z"), + parseNanoTime("2015-02-12T21:32:05.131117533Z"), + } + cpuTimes := []uint64{ + 22400432, + 116499979, + 115436856, + } + memoryUtilizationInBytes := []uint64{ + 3649536, + 3649536, + 3649536, + } + + bytesReceivedTransmitted := []uint64{ + 364953689, + 364953689, + 364953689, + } + + queue := NewQueue(3) + + for i, time := range timestamps { + queue.add(&ContainerStats{ + cpuUsage: cpuTimes[i], + memoryUsage: memoryUtilizationInBytes[i], + networkStats: &NetworkStats{ + RxBytes: bytesReceivedTransmitted[i], + RxDropped: 0, + RxErrors: bytesReceivedTransmitted[i], + RxPackets: bytesReceivedTransmitted[i], + TxBytes: bytesReceivedTransmitted[i], + TxDropped: bytesReceivedTransmitted[i], + TxErrors: 0, + TxPackets: bytesReceivedTransmitted[i], + RxBytesPerSecond: float32(nan32()), + TxBytesPerSecond: float32(nan32()), + }, + timestamp: time}) + } + + // if we have identical timestamps and 2 datapoints + // then there will not be enough valid network stats to create + // a valid network stats set, and this function call should fail. + stats, err := queue.GetNetworkStatsSet() + require.NoErrorf(t, err, "Received unexpected network stats set %v", stats) +} + +// If there are only 2 datapoints, and both have different timestamp, GetNetworkStats should not return error +func TestPerSecNetworkStatSetPassWithTwoDatapoints(t *testing.T) { + timestamps := []time.Time{ + parseNanoTime("2015-02-12T21:22:05.131117533Z"), + parseNanoTime("2015-02-12T21:32:05.131117533Z"), + } + cpuTimes := []uint64{ + 22400432, + 116499979, + } + memoryUtilizationInBytes := []uint64{ + 3649536, + 3649536, + } + + bytesReceivedTransmitted := []uint64{ + 364953689, + 364953689, + } + + queue := NewQueue(3) + + for i, time := range timestamps { + queue.add(&ContainerStats{ + cpuUsage: cpuTimes[i], + memoryUsage: memoryUtilizationInBytes[i], + networkStats: &NetworkStats{ + RxBytes: bytesReceivedTransmitted[i], + RxDropped: 0, + RxErrors: bytesReceivedTransmitted[i], + RxPackets: bytesReceivedTransmitted[i], + TxBytes: bytesReceivedTransmitted[i], + TxDropped: bytesReceivedTransmitted[i], + TxErrors: 0, + TxPackets: bytesReceivedTransmitted[i], + RxBytesPerSecond: float32(nan32()), + TxBytesPerSecond: float32(nan32()), + }, + timestamp: time}) + } + + // if we have identical timestamps and 2 datapoints + // then there will not be enough valid network stats to create + // a valid network stats set, and this function call should fail. + stats, err := queue.GetNetworkStatsSet() + require.NoErrorf(t, err, "Received unexpected network stats set %v", stats) +} + +// If there are only 1 datapoint, then GetNetworkStats should return error +func TestPerSecNetworkStatSetFailWithOneDatapoint(t *testing.T) { + timestamps := []time.Time{ + parseNanoTime("2015-02-12T21:22:05.131117533Z"), + } + + cpuTimes := []uint64{ + 22400432, + } + memoryUtilizationInBytes := []uint64{ + 3649536, + } + + bytesReceivedTransmitted := []uint64{ + 364953689, + } + + queue := NewQueue(3) + + for i, time := range timestamps { + queue.add(&ContainerStats{ + cpuUsage: cpuTimes[i], + memoryUsage: memoryUtilizationInBytes[i], + networkStats: &NetworkStats{ + RxBytes: bytesReceivedTransmitted[i], + RxDropped: 0, + RxErrors: bytesReceivedTransmitted[i], + RxPackets: bytesReceivedTransmitted[i], + TxBytes: bytesReceivedTransmitted[i], + TxDropped: bytesReceivedTransmitted[i], + TxErrors: 0, + TxPackets: bytesReceivedTransmitted[i], + RxBytesPerSecond: float32(nan32()), + TxBytesPerSecond: float32(nan32()), + }, + timestamp: time}) + } + + // if we have identical timestamps and 2 datapoints + // then there will not be enough valid network stats to create + // a valid network stats set, and this function call should fail. + stats, err := queue.GetNetworkStatsSet() + require.Errorf(t, err, "Received unexpected network stats set %v", stats) +} diff --git a/agent/stats/resolver/mock/resolver.go b/agent/stats/resolver/mock/resolver.go index dc1e61d7838..c6c5c24e595 100644 --- a/agent/stats/resolver/mock/resolver.go +++ b/agent/stats/resolver/mock/resolver.go @@ -26,30 +26,30 @@ import ( gomock "github.com/golang/mock/gomock" ) -// MockContainerMetadataResolver is a mock of ContainerMetadataResolver interface +// MockContainerMetadataResolver is a mock of ContainerMetadataResolver interface. type MockContainerMetadataResolver struct { ctrl *gomock.Controller recorder *MockContainerMetadataResolverMockRecorder } -// MockContainerMetadataResolverMockRecorder is the mock recorder for MockContainerMetadataResolver +// MockContainerMetadataResolverMockRecorder is the mock recorder for MockContainerMetadataResolver. type MockContainerMetadataResolverMockRecorder struct { mock *MockContainerMetadataResolver } -// NewMockContainerMetadataResolver creates a new mock instance +// NewMockContainerMetadataResolver creates a new mock instance. func NewMockContainerMetadataResolver(ctrl *gomock.Controller) *MockContainerMetadataResolver { mock := &MockContainerMetadataResolver{ctrl: ctrl} mock.recorder = &MockContainerMetadataResolverMockRecorder{mock} return mock } -// EXPECT returns an object that allows the caller to indicate expected use +// EXPECT returns an object that allows the caller to indicate expected use. func (m *MockContainerMetadataResolver) EXPECT() *MockContainerMetadataResolverMockRecorder { return m.recorder } -// ResolveContainer mocks base method +// ResolveContainer mocks base method. func (m *MockContainerMetadataResolver) ResolveContainer(arg0 string) (*container.DockerContainer, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ResolveContainer", arg0) @@ -58,13 +58,13 @@ func (m *MockContainerMetadataResolver) ResolveContainer(arg0 string) (*containe return ret0, ret1 } -// ResolveContainer indicates an expected call of ResolveContainer +// ResolveContainer indicates an expected call of ResolveContainer. func (mr *MockContainerMetadataResolverMockRecorder) ResolveContainer(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolveContainer", reflect.TypeOf((*MockContainerMetadataResolver)(nil).ResolveContainer), arg0) } -// ResolveTask mocks base method +// ResolveTask mocks base method. func (m *MockContainerMetadataResolver) ResolveTask(arg0 string) (*task.Task, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ResolveTask", arg0) @@ -73,8 +73,23 @@ func (m *MockContainerMetadataResolver) ResolveTask(arg0 string) (*task.Task, er return ret0, ret1 } -// ResolveTask indicates an expected call of ResolveTask +// ResolveTask indicates an expected call of ResolveTask. func (mr *MockContainerMetadataResolverMockRecorder) ResolveTask(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolveTask", reflect.TypeOf((*MockContainerMetadataResolver)(nil).ResolveTask), arg0) } + +// ResolveTaskByARN mocks base method. +func (m *MockContainerMetadataResolver) ResolveTaskByARN(arg0 string) (*task.Task, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ResolveTaskByARN", arg0) + ret0, _ := ret[0].(*task.Task) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ResolveTaskByARN indicates an expected call of ResolveTaskByARN. +func (mr *MockContainerMetadataResolverMockRecorder) ResolveTaskByARN(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolveTaskByARN", reflect.TypeOf((*MockContainerMetadataResolver)(nil).ResolveTaskByARN), arg0) +} diff --git a/agent/stats/resolver/resolver.go b/agent/stats/resolver/resolver.go index a48abed7495..8491a83574f 100644 --- a/agent/stats/resolver/resolver.go +++ b/agent/stats/resolver/resolver.go @@ -24,4 +24,5 @@ import ( type ContainerMetadataResolver interface { ResolveTask(string) (*apitask.Task, error) ResolveContainer(string) (*apicontainer.DockerContainer, error) + ResolveTaskByARN(string) (*apitask.Task, error) } diff --git a/agent/stats/task_linux.go b/agent/stats/task_linux.go new file mode 100644 index 00000000000..419d61ef05a --- /dev/null +++ b/agent/stats/task_linux.go @@ -0,0 +1,257 @@ +// +build linux + +// Copyright 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 stats + +import ( + "context" + "fmt" + "time" + + apitaskstatus "github.com/aws/amazon-ecs-agent/agent/api/task/status" + "github.com/aws/amazon-ecs-agent/agent/config" + "github.com/aws/amazon-ecs-agent/agent/ecscni" + "github.com/aws/amazon-ecs-agent/agent/eni/netlinkwrapper" + "github.com/aws/amazon-ecs-agent/agent/stats/resolver" + "github.com/aws/amazon-ecs-agent/agent/utils/nswrapper" + "github.com/aws/amazon-ecs-agent/agent/utils/retry" + "github.com/containernetworking/plugins/pkg/ns" + + "github.com/cihub/seelog" + "github.com/docker/docker/api/types" + dockerstats "github.com/docker/docker/api/types" + netlinklib "github.com/vishvananda/netlink" +) + +const ( + // linkTypeDevice defines the string that's expected to be the output of + // netlink.Link.Type() method for netlink.Device type. + linkTypeDevice = "device" + linkTypeVlan = "vlan" + // encapTypeLoopback defines the string that's set for the link.Attrs.EncapType + // field for localhost devices. The EncapType field defines the link + // encapsulation method. For localhost, it's set to "loopback". + encapTypeLoopback = "loopback" +) + +// StatsTask abstracts methods to gather and aggregate network data for a task. Used only for AWSVPC mode. +type StatsTask struct { + StatsQueue *Queue + TaskMetadata *TaskMetadata + Ctx context.Context + Cancel context.CancelFunc + Resolver resolver.ContainerMetadataResolver + nswrapperinterface nswrapper.NS + netlinkinterface netlinkwrapper.NetLink + metricPublishInterval time.Duration +} + +func newStatsTaskContainer(taskARN string, containerPID string, numberOfContainers int, + resolver resolver.ContainerMetadataResolver, publishInterval time.Duration) (*StatsTask, error) { + nsAgent := nswrapper.NewNS() + netlinkclient := netlinkwrapper.New() + + ctx, cancel := context.WithCancel(context.Background()) + return &StatsTask{ + TaskMetadata: &TaskMetadata{ + TaskArn: taskARN, + ContainerPID: containerPID, + NumberContainers: numberOfContainers, + }, + Ctx: ctx, + Cancel: cancel, + Resolver: resolver, + netlinkinterface: netlinkclient, + nswrapperinterface: nsAgent, + metricPublishInterval: publishInterval, + }, nil +} + +func (task *StatsTask) StartStatsCollection() { + queueSize := int(config.DefaultContainerMetricsPublishInterval.Seconds() * 4) + task.StatsQueue = NewQueue(queueSize) + task.StatsQueue.Reset() + go task.collect() +} + +func (task *StatsTask) StopStatsCollection() { + task.Cancel() +} + +func (taskStat *StatsTask) collect() { + taskArn := taskStat.TaskMetadata.TaskArn + backoff := retry.NewExponentialBackoff(time.Second*1, time.Second*10, 0.5, 2) + + for { + err := taskStat.processStatsStream() + select { + case <-taskStat.Ctx.Done(): + seelog.Debugf("Stopping stats collection for taskStat %s", taskArn) + return + default: + if err != nil { + d := backoff.Duration() + time.Sleep(d) + seelog.Debugf("Error querying stats for task %s: %v", taskArn, err) + } + // We were disconnected from the stats stream. + // Check if the task is terminal. If it is, stop collecting metrics. + terminal, err := taskStat.terminal() + if err != nil { + // Error determining if the task is terminal. clean-up anyway. + seelog.Warnf("Error determining if the task %s is terminal, stopping stats collection: %v", + taskArn, err) + taskStat.StopStatsCollection() + } else if terminal { + seelog.Infof("Task %s is terminal, stopping stats collection", taskArn) + taskStat.StopStatsCollection() + } + } + } +} + +func (taskStat *StatsTask) processStatsStream() error { + taskArn := taskStat.TaskMetadata.TaskArn + awsvpcNetworkStats, errC := taskStat.getAWSVPCNetworkStats() + + returnError := false + for { + select { + case <-taskStat.Ctx.Done(): + seelog.Info("task context is done") + return nil + case err := <-errC: + seelog.Warnf("Error encountered processing metrics stream from host, this may affect "+ + "cloudwatch metric accuracy: %s", err) + returnError = true + case rawStat, ok := <-awsvpcNetworkStats: + if !ok { + if returnError { + return fmt.Errorf("error encountered processing metrics stream from host") + } + return nil + } + if err := taskStat.StatsQueue.Add(rawStat); err != nil { + seelog.Warnf("Task [%s]: error converting stats: %v", taskArn, err) + } + } + + } +} + +func (taskStat *StatsTask) terminal() (bool, error) { + resolvedTask, err := taskStat.Resolver.ResolveTaskByARN(taskStat.TaskMetadata.TaskArn) + if err != nil { + return false, err + } + return resolvedTask.GetKnownStatus() == apitaskstatus.TaskStopped, nil +} + +func getDevicesList(linkList []netlinklib.Link) []string { + var deviceNames []string + for _, link := range linkList { + if link.Type() != linkTypeDevice && link.Type() != linkTypeVlan { + // We only care about netlink.Device/netlink.Vlan types. Ignore other link types. + continue + } + if link.Attrs().EncapType == encapTypeLoopback { + // Ignore localhost + continue + } + deviceNames = append(deviceNames, link.Attrs().Name) + } + return deviceNames +} + +func (taskStat *StatsTask) populateNIDeviceList(containerPID string) ([]string, error) { + var err error + var deviceList []string + netNSPath := fmt.Sprintf(ecscni.NetnsFormat, containerPID) + err = taskStat.nswrapperinterface.WithNetNSPath(netNSPath, func(ns.NetNS) error { + linksInTaskNetNS, linkErr := taskStat.netlinkinterface.LinkList() + deviceNames := getDevicesList(linksInTaskNetNS) + deviceList = append(deviceList, deviceNames...) + return linkErr + }) + return deviceList, err +} + +func linkStatsToDockerStats(netLinkStats *netlinklib.LinkStatistics, numberOfContainers uint64) dockerstats.NetworkStats { + networkStats := dockerstats.NetworkStats{ + RxBytes: netLinkStats.RxBytes / numberOfContainers, + RxPackets: netLinkStats.RxPackets / numberOfContainers, + RxErrors: netLinkStats.RxErrors / numberOfContainers, + RxDropped: netLinkStats.RxDropped / numberOfContainers, + TxBytes: netLinkStats.TxBytes / numberOfContainers, + TxPackets: netLinkStats.TxPackets / numberOfContainers, + TxErrors: netLinkStats.TxErrors / numberOfContainers, + TxDropped: netLinkStats.TxDropped / numberOfContainers, + } + return networkStats +} + +func (taskStat *StatsTask) getAWSVPCNetworkStats() (<-chan *types.StatsJSON, <-chan error) { + + errC := make(chan error) + statsC := make(chan *dockerstats.StatsJSON) + if taskStat.TaskMetadata.NumberContainers > 0 { + go func() { + defer close(statsC) + statPollTicker := time.NewTicker(taskStat.metricPublishInterval) + defer statPollTicker.Stop() + for range statPollTicker.C { + if len(taskStat.TaskMetadata.DeviceName) == 0 { + var err error + taskStat.TaskMetadata.DeviceName, err = taskStat.populateNIDeviceList(taskStat.TaskMetadata.ContainerPID) + if err != nil { + errC <- err + return + } + } + networkStats := make(map[string]dockerstats.NetworkStats, len(taskStat.TaskMetadata.DeviceName)) + for _, device := range taskStat.TaskMetadata.DeviceName { + var link netlinklib.Link + err := taskStat.nswrapperinterface.WithNetNSPath(fmt.Sprintf(ecscni.NetnsFormat, + taskStat.TaskMetadata.ContainerPID), + func(ns.NetNS) error { + var linkErr error + if link, linkErr = taskStat.netlinkinterface.LinkByName(device); linkErr != nil { + return linkErr + } + return nil + }) + + if err != nil { + errC <- err + return + } + netLinkStats := link.Attrs().Statistics + networkStats[link.Attrs().Name] = linkStatsToDockerStats(netLinkStats, + uint64(taskStat.TaskMetadata.NumberContainers)) + } + + dockerStats := &types.StatsJSON{ + Networks: networkStats, + Stats: types.Stats{ + Read: time.Now(), + }, + } + statsC <- dockerStats + } + }() + } + + return statsC, errC +} diff --git a/agent/stats/task_linux_test.go b/agent/stats/task_linux_test.go new file mode 100644 index 00000000000..ea6c7fd7638 --- /dev/null +++ b/agent/stats/task_linux_test.go @@ -0,0 +1,192 @@ +// +build linux,unit + +// Copyright 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 stats + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/aws/amazon-ecs-agent/agent/utils/nswrapper/mocks" + + apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" + apitask "github.com/aws/amazon-ecs-agent/agent/api/task" + apitaskstatus "github.com/aws/amazon-ecs-agent/agent/api/task/status" + mock_netlink "github.com/aws/amazon-ecs-agent/agent/eni/netlinkwrapper/mocks" + mock_resolver "github.com/aws/amazon-ecs-agent/agent/stats/resolver/mock" + "github.com/containernetworking/plugins/pkg/ns" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + "github.com/vishvananda/netlink" +) + +func TestTaskStatsCollection(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + resolver := mock_resolver.NewMockContainerMetadataResolver(ctrl) + mockNS := mocks.NewMockNS(ctrl) + mockNetLink := mock_netlink.NewMockNetLink(ctrl) + + containerPID := "23" + taskId := "task1" + ctx, cancel := context.WithCancel(context.TODO()) + numberOfContainers := 2 + taskStats := &StatsTask{ + TaskMetadata: &TaskMetadata{ + TaskArn: taskId, + ContainerPID: containerPID, + DeviceName: []string{"device1", "device2"}, + NumberContainers: numberOfContainers, + }, + Ctx: ctx, + Cancel: cancel, + Resolver: resolver, + netlinkinterface: mockNetLink, + nswrapperinterface: mockNS, + metricPublishInterval: time.Second, + } + + testTask := &apitask.Task{ + Containers: []*apicontainer.Container{ + {Name: "c1"}, + {Name: "c2", Type: apicontainer.ContainerCNIPause}, + }, + KnownStatusUnsafe: apitaskstatus.TaskRunning, + } + resolver.EXPECT().ResolveTaskByARN(gomock.Any()).Return(testTask, nil).AnyTimes() + mockNS.EXPECT().WithNetNSPath(gomock.Any(), + gomock.Any()).Do(func(nsPath interface{}, toRun func(n ns.NetNS) error) error { + return toRun(nil) + }).AnyTimes() + mockNetLink.EXPECT().LinkByName(gomock.Any()).Return(&netlink.Device{ + LinkAttrs: netlink.LinkAttrs{ + Name: "name", + Statistics: &netlink.LinkStatistics{ + RxPackets: uint64(2), + RxBytes: uint64(50), + }, + }, + }, nil).AnyTimes() + + taskStats.StartStatsCollection() + time.Sleep(checkPointSleep) + taskStats.StopStatsCollection() + + networkStatsSet, err := taskStats.StatsQueue.GetNetworkStatsSet() + + assert.NoError(t, err) + assert.NotNil(t, networkStatsSet) + rxSum := (*networkStatsSet.RxBytes.SampleCount * (int64(50))) / int64(numberOfContainers) + assert.EqualValues(t, rxSum, *networkStatsSet.RxBytes.Sum) +} + +func TestTaskStatsCollectionError(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + resolver := mock_resolver.NewMockContainerMetadataResolver(ctrl) + mockNS := mocks.NewMockNS(ctrl) + mockNetLink := mock_netlink.NewMockNetLink(ctrl) + + containerPID := "23" + taskId := "task1" + ctx, cancel := context.WithCancel(context.TODO()) + + taskStats := &StatsTask{ + TaskMetadata: &TaskMetadata{ + TaskArn: taskId, + ContainerPID: containerPID, + DeviceName: []string{"device1", "device2"}, + NumberContainers: 2, + }, + Ctx: ctx, + Cancel: cancel, + Resolver: resolver, + netlinkinterface: mockNetLink, + nswrapperinterface: mockNS, + metricPublishInterval: time.Second, + } + + testTask := &apitask.Task{ + Containers: []*apicontainer.Container{ + {Name: "c1"}, + {Name: "c2", Type: apicontainer.ContainerCNIPause}, + }, + KnownStatusUnsafe: apitaskstatus.TaskRunning, + } + resolver.EXPECT().ResolveTaskByARN(gomock.Any()).Return(testTask, nil).AnyTimes() + err := errors.New("emit macho dwarf: elf header corrupted") + + mockNS.EXPECT().WithNetNSPath(gomock.Any(), gomock.Any()).Return(err) + mockNS.EXPECT().WithNetNSPath(gomock.Any(), + gomock.Any()).Do(func(nsPath interface{}, toRun func(n ns.NetNS) error) error { + return toRun(nil) + }).AnyTimes() + mockNetLink.EXPECT().LinkByName(gomock.Any()).Return(&netlink.Device{ + LinkAttrs: netlink.LinkAttrs{ + Name: "name", + Statistics: &netlink.LinkStatistics{ + RxPackets: uint64(2), + RxBytes: uint64(50), + }, + }, + }, nil).AnyTimes() + + taskStats.StartStatsCollection() + time.Sleep(checkPointSleep) + taskStats.StopStatsCollection() + + networkStatsSet, err := taskStats.StatsQueue.GetNetworkStatsSet() + assert.NoError(t, err) + assert.EqualValues(t, 50, *networkStatsSet.RxBytes.Sum) + assert.EqualValues(t, 2, *networkStatsSet.RxPackets.Sum) + assert.EqualValues(t, 2, *networkStatsSet.RxPackets.SampleCount) + assert.EqualValues(t, 2, *networkStatsSet.TxPackets.SampleCount) +} + +func TestGetDeviceList(t *testing.T) { + + link1 := &netlink.GenericLink{ + LinkType: linkTypeDevice, + LinkAttrs: netlink.LinkAttrs{ + Name: "link1device", + }, + } + link2 := &netlink.GenericLink{ + LinkType: linkTypeVlan, + LinkAttrs: netlink.LinkAttrs{ + Name: "link2device", + }, + } + link3 := &netlink.GenericLink{ + LinkType: "randomLinkType", + LinkAttrs: netlink.LinkAttrs{ + Name: "link3device", + }, + } + link4 := &netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + EncapType: encapTypeLoopback, + Name: "link4device", + }, + LinkType: linkTypeVlan, + } + linkList := []netlink.Link{link1, link2, link3, link4} + + deviceNames := getDevicesList(linkList) + + assert.ElementsMatch(t, []string{"link1device", "link2device"}, deviceNames) +} diff --git a/agent/stats/task_unspecified.go b/agent/stats/task_unspecified.go new file mode 100644 index 00000000000..07086610414 --- /dev/null +++ b/agent/stats/task_unspecified.go @@ -0,0 +1,43 @@ +// +build !linux + +// Copyright 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 stats + +import ( + "time" + + "github.com/aws/amazon-ecs-agent/agent/stats/resolver" + "github.com/pkg/errors" +) + +// Dummy StatsTasks +type StatsTask struct { + // AWSVPC network stats only supported on linux + TaskMetadata *TaskMetadata + StatsQueue *Queue +} + +func newStatsTaskContainer(taskARN string, containerPID string, numberOfContainers int, + resolver resolver.ContainerMetadataResolver, publishInterval time.Duration) (*StatsTask, error) { + return nil, errors.New("Unsupported platform") +} + +func (task *StatsTask) StartStatsCollection() { + // AWSVPC network stats only supported on linux +} + +func (task *StatsTask) StopStatsCollection() { + // AWSVPC network stats only supported on linux +} diff --git a/agent/stats/types.go b/agent/stats/types.go index 9fc32b0d9f7..113a4213fac 100644 --- a/agent/stats/types.go +++ b/agent/stats/types.go @@ -35,14 +35,16 @@ type ContainerStats struct { // NetworkStats contains the network stats information for a container type NetworkStats struct { - RxBytes uint64 `json:"rxBytes"` - RxDropped uint64 `json:"rxDropped"` - RxErrors uint64 `json:"rxErrors"` - RxPackets uint64 `json:"rxPackets"` - TxBytes uint64 `json:"txBytes"` - TxDropped uint64 `json:"txDropped"` - TxErrors uint64 `json:"txErrors"` - TxPackets uint64 `json:"txPackets"` + RxBytes uint64 `json:"rxBytes"` + RxDropped uint64 `json:"rxDropped"` + RxErrors uint64 `json:"rxErrors"` + RxPackets uint64 `json:"rxPackets"` + TxBytes uint64 `json:"txBytes"` + TxDropped uint64 `json:"txDropped"` + TxErrors uint64 `json:"txErrors"` + TxPackets uint64 `json:"txPackets"` + RxBytesPerSecond float32 `json:"rxBytesPerSecond"` + TxBytesPerSecond float32 `json:"txBytesPerSecond"` } // UsageStats abstracts the format in which the queue stores data. @@ -65,6 +67,15 @@ type ContainerMetadata struct { NetworkMode string `json:"-"` } +// TaskMetadata contains meta-data information for a task. +type TaskMetadata struct { + TaskArn string `json:"-"` + // ContainerPID is the PID of the pause container in the awsvpc task. + ContainerPID string `json:"-"` + DeviceName []string `json:"-"` + NumberContainers int `json:"-"` +} + // StatsContainer abstracts methods to gather and aggregate utilization data for a container. type StatsContainer struct { containerMetadata *ContainerMetadata @@ -81,3 +92,8 @@ type taskDefinition struct { family string version string } + +type NetworkStatsPerSec struct { + RxBytesPerSecond float32 `json:"rx_bytes_per_sec"` + TxBytesPerSecond float32 `json:"tx_bytes_per_sec"` +} diff --git a/agent/stats/utils.go b/agent/stats/utils.go index 9e6ec4720cc..9459dd817ab 100644 --- a/agent/stats/utils.go +++ b/agent/stats/utils.go @@ -43,5 +43,7 @@ func getNetworkStats(dockerStats *types.StatsJSON) *NetworkStats { networkStats.TxErrors += netStats.TxErrors networkStats.TxPackets += netStats.TxPackets } + networkStats.RxBytesPerSecond = float32(nan32()) + networkStats.TxBytesPerSecond = float32(nan32()) return networkStats } diff --git a/agent/stats/utils_test.go b/agent/stats/utils_test.go index 75de6dc16ae..e3ed80e9b8f 100644 --- a/agent/stats/utils_test.go +++ b/agent/stats/utils_test.go @@ -18,6 +18,7 @@ package stats import ( "encoding/json" "fmt" + "math" "testing" "github.com/docker/docker/api/types" @@ -78,4 +79,6 @@ func validateNetworkMetrics(t *testing.T, netStats *NetworkStats) { assert.Equal(t, expectedTxPackets, netStats.TxPackets) assert.Equal(t, expectedTxDropped, netStats.TxDropped) assert.Equal(t, expectedTxErrors, netStats.TxErrors) + assert.True(t, math.IsNaN(float64(netStats.RxBytesPerSecond))) + assert.True(t, math.IsNaN(float64(netStats.TxBytesPerSecond))) } diff --git a/agent/stats/utils_unix.go b/agent/stats/utils_unix.go index b6cbb15c146..db4053c4ddb 100644 --- a/agent/stats/utils_unix.go +++ b/agent/stats/utils_unix.go @@ -23,12 +23,6 @@ import ( // dockerStatsToContainerStats returns a new object of the ContainerStats object from docker stats. func dockerStatsToContainerStats(dockerStats *types.StatsJSON) (*ContainerStats, error) { - // The length of PercpuUsage represents the number of cores in an instance. - if len(dockerStats.CPUStats.CPUUsage.PercpuUsage) == 0 || numCores == uint64(0) { - seelog.Debug("Invalid container statistics reported, no cpu core usage reported") - return nil, fmt.Errorf("Invalid container statistics reported, no cpu core usage reported") - } - cpuUsage := dockerStats.CPUStats.CPUUsage.TotalUsage / numCores memoryUsage := dockerStats.MemoryStats.Usage - dockerStats.MemoryStats.Stats["cache"] storageReadBytes, storageWriteBytes := getStorageStats(dockerStats) @@ -43,6 +37,14 @@ func dockerStatsToContainerStats(dockerStats *types.StatsJSON) (*ContainerStats, }, nil } +func validateDockerStats(dockerStats *types.StatsJSON) error { + // The length of PercpuUsage represents the number of cores in an instance. + if len(dockerStats.CPUStats.CPUUsage.PercpuUsage) == 0 || numCores == uint64(0) { + return fmt.Errorf("invalid container statistics reported, no cpu core usage reported") + } + return nil +} + func getStorageStats(dockerStats *types.StatsJSON) (uint64, uint64) { // initialize block io and loop over stats to aggregate if dockerStats.BlkioStats.IoServiceBytesRecursive == nil { diff --git a/agent/stats/utils_unix_test.go b/agent/stats/utils_unix_test.go index f9c93cb28fb..d91042c1063 100644 --- a/agent/stats/utils_unix_test.go +++ b/agent/stats/utils_unix_test.go @@ -25,17 +25,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestDockerStatsToContainerStatsEmptyCpuUsageGeneratesError(t *testing.T) { - inputJsonFile, _ := filepath.Abs("./unix_test_stats.json") - jsonBytes, _ := ioutil.ReadFile(inputJsonFile) - dockerStat := &types.StatsJSON{} - json.Unmarshal([]byte(jsonBytes), dockerStat) - // empty the PercpuUsage array - dockerStat.CPUStats.CPUUsage.PercpuUsage = make([]uint64, 0) - _, err := dockerStatsToContainerStats(dockerStat) - assert.Error(t, err, "expected error converting container stats with empty PercpuUsage") -} - func TestDockerStatsToContainerStats(t *testing.T) { // numCores is a global variable in package agent/stats // which denotes the number of cpu cores @@ -57,3 +46,14 @@ func TestDockerStatsToContainerStats(t *testing.T) { assert.NotNil(t, netStats, "networkStats should not be nil") validateNetworkMetrics(t, netStats) } + +func TestDockerStatsToContainerStatsEmptyCpuUsageGeneratesError(t *testing.T) { + inputJsonFile, _ := filepath.Abs("./unix_test_stats.json") + jsonBytes, _ := ioutil.ReadFile(inputJsonFile) + dockerStat := &types.StatsJSON{} + json.Unmarshal([]byte(jsonBytes), dockerStat) + // empty the PercpuUsage array + dockerStat.CPUStats.CPUUsage.PercpuUsage = make([]uint64, 0) + err := validateDockerStats(dockerStat) + assert.Error(t, err, "expected error converting container stats with empty PercpuUsage") +} diff --git a/agent/stats/utils_windows.go b/agent/stats/utils_windows.go index f7c542e66e9..8e5c5f69678 100644 --- a/agent/stats/utils_windows.go +++ b/agent/stats/utils_windows.go @@ -17,17 +17,11 @@ package stats import ( "fmt" - "github.com/cihub/seelog" "github.com/docker/docker/api/types" ) // dockerStatsToContainerStats returns a new object of the ContainerStats object from docker stats. func dockerStatsToContainerStats(dockerStats *types.StatsJSON) (*ContainerStats, error) { - if numCores == uint64(0) { - seelog.Error("Invalid number of cpu cores acquired from the system") - return nil, fmt.Errorf("invalid number of cpu cores acquired from the system") - } - cpuUsage := (dockerStats.CPUStats.CPUUsage.TotalUsage * 100) / numCores memoryUsage := dockerStats.MemoryStats.PrivateWorkingSet networkStats := getNetworkStats(dockerStats) @@ -42,3 +36,10 @@ func dockerStatsToContainerStats(dockerStats *types.StatsJSON) (*ContainerStats, networkStats: networkStats, }, nil } + +func validateDockerStats(dockerStats *types.StatsJSON) error { + if numCores == uint64(0) { + return fmt.Errorf("invalid container statistics reported, no cpu core usage reported") + } + return nil +} diff --git a/agent/stats/utils_windows_test.go b/agent/stats/utils_windows_test.go index 75309c153cf..544292096b4 100644 --- a/agent/stats/utils_windows_test.go +++ b/agent/stats/utils_windows_test.go @@ -39,7 +39,7 @@ func TestDockerStatsToContainerStatsZeroCoresGeneratesError(t *testing.T) { }`, 100) dockerStat := &types.StatsJSON{} json.Unmarshal([]byte(jsonStat), dockerStat) - _, err := dockerStatsToContainerStats(dockerStat) + err := validateDockerStats(dockerStat) assert.Error(t, err, "expected error converting container stats with zero cpu cores") } diff --git a/agent/taskresource/envFiles/envfile.go b/agent/taskresource/envFiles/envfile.go index 79e0d7423cb..314af51f33c 100644 --- a/agent/taskresource/envFiles/envfile.go +++ b/agent/taskresource/envFiles/envfile.go @@ -398,7 +398,7 @@ func (envfile *EnvironmentFileResource) writeEnvFile(writeFunc func(file oswrapp err = tmpFile.Close() if err != nil { - seelog.Errorf("Error while closing temporary file %s created for envfile resource", tmpFile.Name(), err) + seelog.Errorf("Error while closing temporary file %s created for envfile resource", tmpFile.Name()) return err } diff --git a/agent/tcs/client/client_test.go b/agent/tcs/client/client_test.go index b4409011f65..7da2bab9d29 100644 --- a/agent/tcs/client/client_test.go +++ b/agent/tcs/client/client_test.go @@ -56,8 +56,8 @@ func (*mockStatsEngine) GetInstanceMetrics() (*ecstcs.MetricsMetadata, []*ecstcs return nil, nil, fmt.Errorf("uninitialized") } -func (*mockStatsEngine) ContainerDockerStats(taskARN string, id string) (*types.StatsJSON, error) { - return nil, fmt.Errorf("not implemented") +func (*mockStatsEngine) ContainerDockerStats(taskARN string, id string) (*types.StatsJSON, *stats.NetworkStatsPerSec, error) { + return nil, nil, fmt.Errorf("not implemented") } func (*mockStatsEngine) GetTaskHealthMetrics() (*ecstcs.HealthMetadata, []*ecstcs.TaskHealth, error) { @@ -70,8 +70,8 @@ func (*emptyStatsEngine) GetInstanceMetrics() (*ecstcs.MetricsMetadata, []*ecstc return nil, nil, fmt.Errorf("empty stats") } -func (*emptyStatsEngine) ContainerDockerStats(taskARN string, id string) (*types.StatsJSON, error) { - return nil, fmt.Errorf("not implemented") +func (*emptyStatsEngine) ContainerDockerStats(taskARN string, id string) (*types.StatsJSON, *stats.NetworkStatsPerSec, error) { + return nil, nil, fmt.Errorf("not implemented") } func (*emptyStatsEngine) GetTaskHealthMetrics() (*ecstcs.HealthMetadata, []*ecstcs.TaskHealth, error) { @@ -90,8 +90,8 @@ func (*idleStatsEngine) GetInstanceMetrics() (*ecstcs.MetricsMetadata, []*ecstcs return metadata, []*ecstcs.TaskMetric{}, nil } -func (*idleStatsEngine) ContainerDockerStats(taskARN string, id string) (*types.StatsJSON, error) { - return nil, fmt.Errorf("not implemented") +func (*idleStatsEngine) ContainerDockerStats(taskARN string, id string) (*types.StatsJSON, *stats.NetworkStatsPerSec, error) { + return nil, nil, fmt.Errorf("not implemented") } func (*idleStatsEngine) GetTaskHealthMetrics() (*ecstcs.HealthMetadata, []*ecstcs.TaskHealth, error) { @@ -118,8 +118,8 @@ func (engine *nonIdleStatsEngine) GetInstanceMetrics() (*ecstcs.MetricsMetadata, return metadata, taskMetrics, nil } -func (*nonIdleStatsEngine) ContainerDockerStats(taskARN string, id string) (*types.StatsJSON, error) { - return nil, fmt.Errorf("not implemented") +func (*nonIdleStatsEngine) ContainerDockerStats(taskARN string, id string) (*types.StatsJSON, *stats.NetworkStatsPerSec, error) { + return nil, nil, fmt.Errorf("not implemented") } func (*nonIdleStatsEngine) GetTaskHealthMetrics() (*ecstcs.HealthMetadata, []*ecstcs.TaskHealth, error) { diff --git a/agent/tcs/handler/handler.go b/agent/tcs/handler/handler.go index a0924cd4187..3bad054c245 100644 --- a/agent/tcs/handler/handler.go +++ b/agent/tcs/handler/handler.go @@ -115,7 +115,7 @@ func startSession( publishMetricsInterval time.Duration, deregisterInstanceEventStream *eventstream.EventStream) error { client := tcsclient.New(url, cfg, credentialProvider, statsEngine, - publishMetricsInterval, wsRWTimeout, cfg.DisableMetrics) + publishMetricsInterval, wsRWTimeout, cfg.DisableMetrics.Enabled()) defer client.Close() err := deregisterInstanceEventStream.Subscribe(deregisterContainerInstanceHandler, client.Disconnect) diff --git a/agent/tcs/handler/handler_test.go b/agent/tcs/handler/handler_test.go index 52d67c25d74..a5524aba4a9 100644 --- a/agent/tcs/handler/handler_test.go +++ b/agent/tcs/handler/handler_test.go @@ -32,6 +32,7 @@ import ( "github.com/aws/amazon-ecs-agent/agent/config" mock_engine "github.com/aws/amazon-ecs-agent/agent/engine/mocks" "github.com/aws/amazon-ecs-agent/agent/eventstream" + "github.com/aws/amazon-ecs-agent/agent/stats" tcsclient "github.com/aws/amazon-ecs-agent/agent/tcs/client" "github.com/aws/amazon-ecs-agent/agent/tcs/model/ecstcs" "github.com/aws/amazon-ecs-agent/agent/version" @@ -67,8 +68,8 @@ func (*mockStatsEngine) GetInstanceMetrics() (*ecstcs.MetricsMetadata, []*ecstcs return req.Metadata, req.TaskMetrics, nil } -func (*mockStatsEngine) ContainerDockerStats(taskARN string, id string) (*types.StatsJSON, error) { - return nil, fmt.Errorf("not implemented") +func (*mockStatsEngine) ContainerDockerStats(taskARN string, id string) (*types.StatsJSON, *stats.NetworkStatsPerSec, error) { + return nil, nil, fmt.Errorf("not implemented") } func (*mockStatsEngine) GetTaskHealthMetrics() (*ecstcs.HealthMetadata, []*ecstcs.TaskHealth, error) { @@ -80,8 +81,8 @@ func (*mockStatsEngine) GetTaskHealthMetrics() (*ecstcs.HealthMetadata, []*ecstc func TestDisableMetrics(t *testing.T) { params := TelemetrySessionParams{ Cfg: &config.Config{ - DisableMetrics: true, - DisableDockerHealthCheck: true, + DisableMetrics: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, + DisableDockerHealthCheck: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, }, } diff --git a/agent/tcs/handler/types.go b/agent/tcs/handler/types.go index 15642cdcd6d..5de56478d41 100644 --- a/agent/tcs/handler/types.go +++ b/agent/tcs/handler/types.go @@ -54,7 +54,7 @@ func (params *TelemetrySessionParams) time() ttime.Time { func (params *TelemetrySessionParams) isContainerHealthMetricsDisabled() (bool, error) { if params.Cfg != nil { - return params.Cfg.DisableMetrics && params.Cfg.DisableDockerHealthCheck, nil + return params.Cfg.DisableMetrics.Enabled() && params.Cfg.DisableDockerHealthCheck.Enabled(), nil } return false, errors.New("Config is empty in the tcs session parameter") } diff --git a/agent/tcs/handler/types_test.go b/agent/tcs/handler/types_test.go index 3872fc8b895..4f096ccb780 100644 --- a/agent/tcs/handler/types_test.go +++ b/agent/tcs/handler/types_test.go @@ -37,25 +37,25 @@ func TestIsMetricsDisabled(t *testing.T) { description: "Config not set should cause error", }, { - param: &TelemetrySessionParams{Cfg: &config.Config{DisableMetrics: false, DisableDockerHealthCheck: false}}, + param: &TelemetrySessionParams{Cfg: &config.Config{DisableMetrics: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}, DisableDockerHealthCheck: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}}}, result: false, err: nil, description: "No metrics was disable should return false", }, { - param: &TelemetrySessionParams{Cfg: &config.Config{DisableMetrics: true, DisableDockerHealthCheck: false}}, + param: &TelemetrySessionParams{Cfg: &config.Config{DisableMetrics: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, DisableDockerHealthCheck: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}}}, result: false, err: nil, description: "Only health metrics was disable should return false", }, { - param: &TelemetrySessionParams{Cfg: &config.Config{DisableMetrics: false, DisableDockerHealthCheck: true}}, + param: &TelemetrySessionParams{Cfg: &config.Config{DisableMetrics: config.BooleanDefaultFalse{Value: config.ExplicitlyDisabled}, DisableDockerHealthCheck: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}}}, result: false, err: nil, description: "Only telemetry metrics was disable should return false", }, { - param: &TelemetrySessionParams{Cfg: &config.Config{DisableMetrics: true, DisableDockerHealthCheck: true}}, + param: &TelemetrySessionParams{Cfg: &config.Config{DisableMetrics: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}, DisableDockerHealthCheck: config.BooleanDefaultFalse{Value: config.ExplicitlyEnabled}}}, result: true, err: nil, description: "both telemetry and health metrics were disable should return true", diff --git a/agent/tcs/model/api/api-2.json b/agent/tcs/model/api/api-2.json index 3e8656183b5..108fabba78d 100644 --- a/agent/tcs/model/api/api-2.json +++ b/agent/tcs/model/api/api-2.json @@ -198,10 +198,12 @@ "NetworkStatsSet":{ "type":"structure", "members":{ + "rxBytesPerSecond":{"shape":"UDoubleCWStatsSet"}, "rxBytes":{"shape":"ULongStatsSet"}, "rxDropped":{"shape":"ULongStatsSet"}, "rxErrors":{"shape":"ULongStatsSet"}, "rxPackets":{"shape":"ULongStatsSet"}, + "txBytesPerSecond":{"shape":"UDoubleCWStatsSet"}, "txBytes":{"shape":"ULongStatsSet"}, "txDropped":{"shape":"ULongStatsSet"}, "txErrors":{"shape":"ULongStatsSet"}, @@ -287,6 +289,25 @@ "max":10 }, "Timestamp":{"type":"timestamp"}, + "UDouble":{ + "type":"double", + "min":0 + }, + "UDoubleCWStatsSet":{ + "type":"structure", + "required":[ + "min", + "max", + "sampleCount", + "sum" + ], + "members":{ + "min":{"shape":"UDouble"}, + "max":{"shape":"UDouble"}, + "sampleCount":{"shape":"UInteger"}, + "sum":{"shape":"UDouble"} + } + }, "UInteger":{ "type":"integer", "min":0 diff --git a/agent/tcs/model/ecstcs/api.go b/agent/tcs/model/ecstcs/api.go index 4cfa913b327..b597ae8c08b 100644 --- a/agent/tcs/model/ecstcs/api.go +++ b/agent/tcs/model/ecstcs/api.go @@ -272,6 +272,8 @@ type NetworkStatsSet struct { RxBytes *ULongStatsSet `locationName:"rxBytes" type:"structure"` + RxBytesPerSecond *UDoubleCWStatsSet `locationName:"rxBytesPerSecond" type:"structure"` + RxDropped *ULongStatsSet `locationName:"rxDropped" type:"structure"` RxErrors *ULongStatsSet `locationName:"rxErrors" type:"structure"` @@ -280,6 +282,8 @@ type NetworkStatsSet struct { TxBytes *ULongStatsSet `locationName:"txBytes" type:"structure"` + TxBytesPerSecond *UDoubleCWStatsSet `locationName:"txBytesPerSecond" type:"structure"` + TxDropped *ULongStatsSet `locationName:"txDropped" type:"structure"` TxErrors *ULongStatsSet `locationName:"txErrors" type:"structure"` @@ -305,6 +309,11 @@ func (s *NetworkStatsSet) Validate() error { invalidParams.AddNested("RxBytes", err.(request.ErrInvalidParams)) } } + if s.RxBytesPerSecond != nil { + if err := s.RxBytesPerSecond.Validate(); err != nil { + invalidParams.AddNested("RxBytesPerSecond", err.(request.ErrInvalidParams)) + } + } if s.RxDropped != nil { if err := s.RxDropped.Validate(); err != nil { invalidParams.AddNested("RxDropped", err.(request.ErrInvalidParams)) @@ -325,6 +334,11 @@ func (s *NetworkStatsSet) Validate() error { invalidParams.AddNested("TxBytes", err.(request.ErrInvalidParams)) } } + if s.TxBytesPerSecond != nil { + if err := s.TxBytesPerSecond.Validate(); err != nil { + invalidParams.AddNested("TxBytesPerSecond", err.(request.ErrInvalidParams)) + } + } if s.TxDropped != nil { if err := s.TxDropped.Validate(); err != nil { invalidParams.AddNested("TxDropped", err.(request.ErrInvalidParams)) @@ -681,6 +695,54 @@ func (s *TaskMetric) Validate() error { return nil } +type UDoubleCWStatsSet struct { + _ struct{} `type:"structure"` + + // Max is a required field + Max *float64 `locationName:"max" type:"double" required:"true"` + + // Min is a required field + Min *float64 `locationName:"min" type:"double" required:"true"` + + // SampleCount is a required field + SampleCount *int64 `locationName:"sampleCount" type:"integer" required:"true"` + + // Sum is a required field + Sum *float64 `locationName:"sum" type:"double" required:"true"` +} + +// String returns the string representation +func (s UDoubleCWStatsSet) String() string { + return awsutil.Prettify(s) +} + +// GoString returns the string representation +func (s UDoubleCWStatsSet) GoString() string { + return s.String() +} + +// Validate inspects the fields of the type to determine if they are valid. +func (s *UDoubleCWStatsSet) Validate() error { + invalidParams := request.ErrInvalidParams{Context: "UDoubleCWStatsSet"} + if s.Max == nil { + invalidParams.Add(request.NewErrParamRequired("Max")) + } + if s.Min == nil { + invalidParams.Add(request.NewErrParamRequired("Min")) + } + if s.SampleCount == nil { + invalidParams.Add(request.NewErrParamRequired("SampleCount")) + } + if s.Sum == nil { + invalidParams.Add(request.NewErrParamRequired("Sum")) + } + + if invalidParams.Len() > 0 { + return invalidParams + } + return nil +} + type ULongStatsSet struct { _ struct{} `type:"structure"` diff --git a/agent/utils/nswrapper/generate_mocks_ns.go b/agent/utils/nswrapper/generate_mocks_ns.go new file mode 100644 index 00000000000..a80575efc25 --- /dev/null +++ b/agent/utils/nswrapper/generate_mocks_ns.go @@ -0,0 +1,16 @@ +// Copyright 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 nswrapper + +//go:generate mockgen -destination=mocks/nswrapper_mocks_linux.go -copyright_file=../../../scripts/copyright_file github.com/aws/amazon-ecs-agent/agent/utils/nswrapper NS diff --git a/agent/utils/nswrapper/mocks/nswrapper_mocks_linux.go b/agent/utils/nswrapper/mocks/nswrapper_mocks_linux.go new file mode 100644 index 00000000000..5bd59cef7ec --- /dev/null +++ b/agent/utils/nswrapper/mocks/nswrapper_mocks_linux.go @@ -0,0 +1,77 @@ +// Copyright 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. + +// Code generated by MockGen. DO NOT EDIT. +// Source: agent/nswrapper/ns_linux.go + +// Package mock_nswrapper is a generated GoMock package. +package mocks + +import ( + reflect "reflect" + + ns "github.com/containernetworking/plugins/pkg/ns" + gomock "github.com/golang/mock/gomock" +) + +// MockNS is a mock of NS interface. +type MockNS struct { + ctrl *gomock.Controller + recorder *MockNSMockRecorder +} + +// MockNSMockRecorder is the mock recorder for MockNS. +type MockNSMockRecorder struct { + mock *MockNS +} + +// NewMockNS creates a new mock instance. +func NewMockNS(ctrl *gomock.Controller) *MockNS { + mock := &MockNS{ctrl: ctrl} + mock.recorder = &MockNSMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockNS) EXPECT() *MockNSMockRecorder { + return m.recorder +} + +// GetNS mocks base method. +func (m *MockNS) GetNS(nspath string) (ns.NetNS, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNS", nspath) + ret0, _ := ret[0].(ns.NetNS) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetNS indicates an expected call of GetNS. +func (mr *MockNSMockRecorder) GetNS(nspath interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNS", reflect.TypeOf((*MockNS)(nil).GetNS), nspath) +} + +// WithNetNSPath mocks base method. +func (m *MockNS) WithNetNSPath(nspath string, toRun func(ns.NetNS) error) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "WithNetNSPath", nspath, toRun) + ret0, _ := ret[0].(error) + return ret0 +} + +// WithNetNSPath indicates an expected call of WithNetNSPath. +func (mr *MockNSMockRecorder) WithNetNSPath(nspath, toRun interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithNetNSPath", reflect.TypeOf((*MockNS)(nil).WithNetNSPath), nspath, toRun) +} diff --git a/agent/utils/nswrapper/ns_linux.go b/agent/utils/nswrapper/ns_linux.go new file mode 100644 index 00000000000..69947596cab --- /dev/null +++ b/agent/utils/nswrapper/ns_linux.go @@ -0,0 +1,40 @@ +// +build linux + +// Copyright 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 nswrapper + +import "github.com/containernetworking/plugins/pkg/ns" + +// NS wraps methods used from the cni/pkg/ns package +type NS interface { + GetNS(nspath string) (ns.NetNS, error) + WithNetNSPath(nspath string, toRun func(ns.NetNS) error) error +} + +type agentNS struct { +} + +// NewNS creates a new NS object +func NewNS() NS { + return &agentNS{} +} + +func (*agentNS) GetNS(nspath string) (ns.NetNS, error) { + return ns.GetNS(nspath) +} + +func (*agentNS) WithNetNSPath(nspath string, toRun func(ns.NetNS) error) error { + return ns.WithNetNSPath(nspath, toRun) +} diff --git a/agent/utils/utils_test.go b/agent/utils/utils_test.go index 461689c59c4..ce2057cdfa6 100644 --- a/agent/utils/utils_test.go +++ b/agent/utils/utils_test.go @@ -16,6 +16,7 @@ package utils import ( + "encoding/json" "errors" "sort" "testing" @@ -37,10 +38,19 @@ func TestDefaultIfBlank(t *testing.T) { assert.Equal(t, defaultValue, result) } +type dummyStruct struct { + // no contents +} + +func (d dummyStruct) MarshalJSON([]byte, error) { + json.Marshal(nil) +} + func TestZeroOrNil(t *testing.T) { type ZeroTest struct { - testInt int - TestStr string + testInt int + TestStr string + testNilJson dummyStruct } var strMap map[string]string @@ -55,6 +65,7 @@ func TestZeroOrNil(t *testing.T) { {"", true, "\"\" is the string zerovalue"}, {ZeroTest{}, true, "ZeroTest zero-value should be zero"}, {ZeroTest{TestStr: "asdf"}, false, "ZeroTest with a field populated isn't zero"}, + {ZeroTest{testNilJson: dummyStruct{}}, true, "nil is nil"}, {1, false, "1 is not 0"}, {[]uint16{1, 2, 3}, false, "[1,2,3] is not zero"}, {[]uint16{}, true, "[] is zero"}, @@ -70,6 +81,7 @@ func TestZeroOrNil(t *testing.T) { assert.Equal(t, tc.expected, ZeroOrNil(tc.param), tc.name) }) } + } func TestSlicesDeepEqual(t *testing.T) { diff --git a/agent/vendor/github.com/containernetworking/cni/libcni/api.go b/agent/vendor/github.com/containernetworking/cni/libcni/api.go index 360733e7407..0f14d3427e9 100644 --- a/agent/vendor/github.com/containernetworking/cni/libcni/api.go +++ b/agent/vendor/github.com/containernetworking/cni/libcni/api.go @@ -69,6 +69,7 @@ type CNI interface { AddNetworkList(ctx context.Context, net *NetworkConfigList, rt *RuntimeConf) (types.Result, error) CheckNetworkList(ctx context.Context, net *NetworkConfigList, rt *RuntimeConf) error DelNetworkList(ctx context.Context, net *NetworkConfigList, rt *RuntimeConf) error + GetNetworkListCachedResult(net *NetworkConfigList, rt *RuntimeConf) (types.Result, error) AddNetwork(ctx context.Context, net *NetworkConfig, rt *RuntimeConf) (types.Result, error) CheckNetwork(ctx context.Context, net *NetworkConfig, rt *RuntimeConf) error diff --git a/agent/vendor/github.com/containernetworking/cni/pkg/invoke/args.go b/agent/vendor/github.com/containernetworking/cni/pkg/invoke/args.go index 39b63972305..913528c1d59 100644 --- a/agent/vendor/github.com/containernetworking/cni/pkg/invoke/args.go +++ b/agent/vendor/github.com/containernetworking/cni/pkg/invoke/args.go @@ -15,6 +15,7 @@ package invoke import ( + "fmt" "os" "strings" ) @@ -22,6 +23,8 @@ import ( type CNIArgs interface { // For use with os/exec; i.e., return nil to inherit the // environment from this process + // For use in delegation; inherit the environment from this + // process and allow overrides AsEnv() []string } @@ -57,17 +60,17 @@ func (args *Args) AsEnv() []string { pluginArgsStr = stringify(args.PluginArgs) } - // Ensure that the custom values are first, so any value present in - // the process environment won't override them. - env = append([]string{ - "CNI_COMMAND=" + args.Command, - "CNI_CONTAINERID=" + args.ContainerID, - "CNI_NETNS=" + args.NetNS, - "CNI_ARGS=" + pluginArgsStr, - "CNI_IFNAME=" + args.IfName, - "CNI_PATH=" + args.Path, - }, env...) - return env + // Duplicated values which come first will be overrided, so we must put the + // custom values in the end to avoid being overrided by the process environments. + env = append(env, + "CNI_COMMAND="+args.Command, + "CNI_CONTAINERID="+args.ContainerID, + "CNI_NETNS="+args.NetNS, + "CNI_ARGS="+pluginArgsStr, + "CNI_IFNAME="+args.IfName, + "CNI_PATH="+args.Path, + ) + return dedupEnv(env) } // taken from rkt/networking/net_plugin.go @@ -80,3 +83,46 @@ func stringify(pluginArgs [][2]string) string { return strings.Join(entries, ";") } + +// DelegateArgs implements the CNIArgs interface +// used for delegation to inherit from environments +// and allow some overrides like CNI_COMMAND +var _ CNIArgs = &DelegateArgs{} + +type DelegateArgs struct { + Command string +} + +func (d *DelegateArgs) AsEnv() []string { + env := os.Environ() + + // The custom values should come in the end to override the existing + // process environment of the same key. + env = append(env, + "CNI_COMMAND="+d.Command, + ) + return dedupEnv(env) +} + +// dedupEnv returns a copy of env with any duplicates removed, in favor of later values. +// Items not of the normal environment "key=value" form are preserved unchanged. +func dedupEnv(env []string) []string { + out := make([]string, 0, len(env)) + envMap := map[string]string{} + + for _, kv := range env { + // find the first "=" in environment, if not, just keep it + eq := strings.Index(kv, "=") + if eq < 0 { + out = append(out, kv) + continue + } + envMap[kv[:eq]] = kv[eq+1:] + } + + for k, v := range envMap { + out = append(out, fmt.Sprintf("%s=%s", k, v)) + } + + return out +} diff --git a/agent/vendor/github.com/containernetworking/cni/pkg/invoke/delegate.go b/agent/vendor/github.com/containernetworking/cni/pkg/invoke/delegate.go index 30b4672f11f..8defe4dd398 100644 --- a/agent/vendor/github.com/containernetworking/cni/pkg/invoke/delegate.go +++ b/agent/vendor/github.com/containernetworking/cni/pkg/invoke/delegate.go @@ -16,22 +16,17 @@ package invoke import ( "context" - "fmt" "os" "path/filepath" "github.com/containernetworking/cni/pkg/types" ) -func delegateCommon(expectedCommand, delegatePlugin string, exec Exec) (string, Exec, error) { +func delegateCommon(delegatePlugin string, exec Exec) (string, Exec, error) { if exec == nil { exec = defaultExec } - if os.Getenv("CNI_COMMAND") != expectedCommand { - return "", nil, fmt.Errorf("CNI_COMMAND is not " + expectedCommand) - } - paths := filepath.SplitList(os.Getenv("CNI_PATH")) pluginPath, err := exec.FindInPath(delegatePlugin, paths) if err != nil { @@ -44,32 +39,42 @@ func delegateCommon(expectedCommand, delegatePlugin string, exec Exec) (string, // DelegateAdd calls the given delegate plugin with the CNI ADD action and // JSON configuration func DelegateAdd(ctx context.Context, delegatePlugin string, netconf []byte, exec Exec) (types.Result, error) { - pluginPath, realExec, err := delegateCommon("ADD", delegatePlugin, exec) + pluginPath, realExec, err := delegateCommon(delegatePlugin, exec) if err != nil { return nil, err } - return ExecPluginWithResult(ctx, pluginPath, netconf, ArgsFromEnv(), realExec) + // DelegateAdd will override the original "CNI_COMMAND" env from process with ADD + return ExecPluginWithResult(ctx, pluginPath, netconf, delegateArgs("ADD"), realExec) } // DelegateCheck calls the given delegate plugin with the CNI CHECK action and // JSON configuration func DelegateCheck(ctx context.Context, delegatePlugin string, netconf []byte, exec Exec) error { - pluginPath, realExec, err := delegateCommon("CHECK", delegatePlugin, exec) + pluginPath, realExec, err := delegateCommon(delegatePlugin, exec) if err != nil { return err } - return ExecPluginWithoutResult(ctx, pluginPath, netconf, ArgsFromEnv(), realExec) + // DelegateCheck will override the original CNI_COMMAND env from process with CHECK + return ExecPluginWithoutResult(ctx, pluginPath, netconf, delegateArgs("CHECK"), realExec) } // DelegateDel calls the given delegate plugin with the CNI DEL action and // JSON configuration func DelegateDel(ctx context.Context, delegatePlugin string, netconf []byte, exec Exec) error { - pluginPath, realExec, err := delegateCommon("DEL", delegatePlugin, exec) + pluginPath, realExec, err := delegateCommon(delegatePlugin, exec) if err != nil { return err } - return ExecPluginWithoutResult(ctx, pluginPath, netconf, ArgsFromEnv(), realExec) + // DelegateDel will override the original CNI_COMMAND env from process with DEL + return ExecPluginWithoutResult(ctx, pluginPath, netconf, delegateArgs("DEL"), realExec) +} + +// return CNIArgs used by delegation +func delegateArgs(action string) *DelegateArgs { + return &DelegateArgs{ + Command: action, + } } diff --git a/agent/vendor/github.com/containernetworking/plugins/LICENSE b/agent/vendor/github.com/containernetworking/plugins/LICENSE new file mode 100644 index 00000000000..8dada3edaf5 --- /dev/null +++ b/agent/vendor/github.com/containernetworking/plugins/LICENSE @@ -0,0 +1,201 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "{}" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright {yyyy} {name of copyright owner} + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License 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. diff --git a/agent/vendor/github.com/containernetworking/plugins/pkg/ns/README.md b/agent/vendor/github.com/containernetworking/plugins/pkg/ns/README.md new file mode 100644 index 00000000000..1e265c7a011 --- /dev/null +++ b/agent/vendor/github.com/containernetworking/plugins/pkg/ns/README.md @@ -0,0 +1,41 @@ +### Namespaces, Threads, and Go +On Linux each OS thread can have a different network namespace. Go's thread scheduling model switches goroutines between OS threads based on OS thread load and whether the goroutine would block other goroutines. This can result in a goroutine switching network namespaces without notice and lead to errors in your code. + +### Namespace Switching +Switching namespaces with the `ns.Set()` method is not recommended without additional strategies to prevent unexpected namespace changes when your goroutines switch OS threads. + +Go provides the `runtime.LockOSThread()` function to ensure a specific goroutine executes on its current OS thread and prevents any other goroutine from running in that thread until the locked one exits. Careful usage of `LockOSThread()` and goroutines can provide good control over which network namespace a given goroutine executes in. + +For example, you cannot rely on the `ns.Set()` namespace being the current namespace after the `Set()` call unless you do two things. First, the goroutine calling `Set()` must have previously called `LockOSThread()`. Second, you must ensure `runtime.UnlockOSThread()` is not called somewhere in-between. You also cannot rely on the initial network namespace remaining the current network namespace if any other code in your program switches namespaces, unless you have already called `LockOSThread()` in that goroutine. Note that `LockOSThread()` prevents the Go scheduler from optimally scheduling goroutines for best performance, so `LockOSThread()` should only be used in small, isolated goroutines that release the lock quickly. + +### Do() The Recommended Thing +The `ns.Do()` method provides **partial** control over network namespaces for you by implementing these strategies. All code dependent on a particular network namespace (including the root namespace) should be wrapped in the `ns.Do()` method to ensure the correct namespace is selected for the duration of your code. For example: + +```go +err = targetNs.Do(func(hostNs ns.NetNS) error { + dummy := &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Name: "dummy0", + }, + } + return netlink.LinkAdd(dummy) +}) +``` + +Note this requirement to wrap every network call is very onerous - any libraries you call might call out to network services such as DNS, and all such calls need to be protected after you call `ns.Do()`. All goroutines spawned from within the `ns.Do` will not inherit the new namespace. The CNI plugins all exit very soon after calling `ns.Do()` which helps to minimize the problem. + +When a new thread is spawned in Linux, it inherits the namespace of its parent. In versions of go **prior to 1.10**, if the runtime spawns a new OS thread, it picks the parent randomly. If the chosen parent thread has been moved to a new namespace (even temporarily), the new OS thread will be permanently "stuck in the wrong namespace", and goroutines will non-deterministically switch namespaces as they are rescheduled. + +In short, **there was no safe way to change network namespaces, even temporarily, from within a long-lived, multithreaded Go process**. If you wish to do this, you must use go 1.10 or greater. + + +### Creating network namespaces +Earlier versions of this library managed namespace creation, but as CNI does not actually utilize this feature (and it was essentially unmaintained), it was removed. If you're writing a container runtime, you should implement namespace management yourself. However, there are some gotchas when doing so, especially around handling `/var/run/netns`. A reasonably correct reference implementation, borrowed from `rkt`, can be found in `pkg/testutils/netns_linux.go` if you're in need of a source of inspiration. + + +### Further Reading + - https://github.com/golang/go/wiki/LockOSThread + - http://morsmachine.dk/go-scheduler + - https://github.com/containernetworking/cni/issues/262 + - https://golang.org/pkg/runtime/ + - https://www.weave.works/blog/linux-namespaces-and-go-don-t-mix diff --git a/agent/vendor/github.com/containernetworking/plugins/pkg/ns/ns_linux.go b/agent/vendor/github.com/containernetworking/plugins/pkg/ns/ns_linux.go new file mode 100644 index 00000000000..a34f97170e3 --- /dev/null +++ b/agent/vendor/github.com/containernetworking/plugins/pkg/ns/ns_linux.go @@ -0,0 +1,229 @@ +// Copyright 2015-2017 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License 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 ns + +import ( + "fmt" + "os" + "runtime" + "sync" + "syscall" + + "golang.org/x/sys/unix" +) + +// Returns an object representing the current OS thread's network namespace +func GetCurrentNS() (NetNS, error) { + return GetNS(getCurrentThreadNetNSPath()) +} + +func getCurrentThreadNetNSPath() string { + // /proc/self/ns/net returns the namespace of the main thread, not + // of whatever thread this goroutine is running on. Make sure we + // use the thread's net namespace since the thread is switching around + return fmt.Sprintf("/proc/%d/task/%d/ns/net", os.Getpid(), unix.Gettid()) +} + +func (ns *netNS) Close() error { + if err := ns.errorIfClosed(); err != nil { + return err + } + + if err := ns.file.Close(); err != nil { + return fmt.Errorf("Failed to close %q: %v", ns.file.Name(), err) + } + ns.closed = true + + return nil +} + +func (ns *netNS) Set() error { + if err := ns.errorIfClosed(); err != nil { + return err + } + + if err := unix.Setns(int(ns.Fd()), unix.CLONE_NEWNET); err != nil { + return fmt.Errorf("Error switching to ns %v: %v", ns.file.Name(), err) + } + + return nil +} + +type NetNS interface { + // Executes the passed closure in this object's network namespace, + // attempting to restore the original namespace before returning. + // However, since each OS thread can have a different network namespace, + // and Go's thread scheduling is highly variable, callers cannot + // guarantee any specific namespace is set unless operations that + // require that namespace are wrapped with Do(). Also, no code called + // from Do() should call runtime.UnlockOSThread(), or the risk + // of executing code in an incorrect namespace will be greater. See + // https://github.com/golang/go/wiki/LockOSThread for further details. + Do(toRun func(NetNS) error) error + + // Sets the current network namespace to this object's network namespace. + // Note that since Go's thread scheduling is highly variable, callers + // cannot guarantee the requested namespace will be the current namespace + // after this function is called; to ensure this wrap operations that + // require the namespace with Do() instead. + Set() error + + // Returns the filesystem path representing this object's network namespace + Path() string + + // Returns a file descriptor representing this object's network namespace + Fd() uintptr + + // Cleans up this instance of the network namespace; if this instance + // is the last user the namespace will be destroyed + Close() error +} + +type netNS struct { + file *os.File + closed bool +} + +// netNS implements the NetNS interface +var _ NetNS = &netNS{} + +const ( + // https://github.com/torvalds/linux/blob/master/include/uapi/linux/magic.h + NSFS_MAGIC = 0x6e736673 + PROCFS_MAGIC = 0x9fa0 +) + +type NSPathNotExistErr struct{ msg string } + +func (e NSPathNotExistErr) Error() string { return e.msg } + +type NSPathNotNSErr struct{ msg string } + +func (e NSPathNotNSErr) Error() string { return e.msg } + +func IsNSorErr(nspath string) error { + stat := syscall.Statfs_t{} + if err := syscall.Statfs(nspath, &stat); err != nil { + if os.IsNotExist(err) { + err = NSPathNotExistErr{msg: fmt.Sprintf("failed to Statfs %q: %v", nspath, err)} + } else { + err = fmt.Errorf("failed to Statfs %q: %v", nspath, err) + } + return err + } + + switch stat.Type { + case PROCFS_MAGIC, NSFS_MAGIC: + return nil + default: + return NSPathNotNSErr{msg: fmt.Sprintf("unknown FS magic on %q: %x", nspath, stat.Type)} + } +} + +// Returns an object representing the namespace referred to by @path +func GetNS(nspath string) (NetNS, error) { + err := IsNSorErr(nspath) + if err != nil { + return nil, err + } + + fd, err := os.Open(nspath) + if err != nil { + return nil, err + } + + return &netNS{file: fd}, nil +} + +func (ns *netNS) Path() string { + return ns.file.Name() +} + +func (ns *netNS) Fd() uintptr { + return ns.file.Fd() +} + +func (ns *netNS) errorIfClosed() error { + if ns.closed { + return fmt.Errorf("%q has already been closed", ns.file.Name()) + } + return nil +} + +func (ns *netNS) Do(toRun func(NetNS) error) error { + if err := ns.errorIfClosed(); err != nil { + return err + } + + containedCall := func(hostNS NetNS) error { + threadNS, err := GetCurrentNS() + if err != nil { + return fmt.Errorf("failed to open current netns: %v", err) + } + defer threadNS.Close() + + // switch to target namespace + if err = ns.Set(); err != nil { + return fmt.Errorf("error switching to ns %v: %v", ns.file.Name(), err) + } + defer func() { + err := threadNS.Set() // switch back + if err == nil { + // Unlock the current thread only when we successfully switched back + // to the original namespace; otherwise leave the thread locked which + // will force the runtime to scrap the current thread, that is maybe + // not as optimal but at least always safe to do. + runtime.UnlockOSThread() + } + }() + + return toRun(hostNS) + } + + // save a handle to current network namespace + hostNS, err := GetCurrentNS() + if err != nil { + return fmt.Errorf("Failed to open current namespace: %v", err) + } + defer hostNS.Close() + + var wg sync.WaitGroup + wg.Add(1) + + // Start the callback in a new green thread so that if we later fail + // to switch the namespace back to the original one, we can safely + // leave the thread locked to die without a risk of the current thread + // left lingering with incorrect namespace. + var innerError error + go func() { + defer wg.Done() + runtime.LockOSThread() + innerError = containedCall(hostNS) + }() + wg.Wait() + + return innerError +} + +// WithNetNSPath executes the passed closure under the given network +// namespace, restoring the original namespace afterwards. +func WithNetNSPath(nspath string, toRun func(NetNS) error) error { + ns, err := GetNS(nspath) + if err != nil { + return err + } + defer ns.Close() + return ns.Do(toRun) +} diff --git a/agent/vendor/github.com/containernetworking/plugins/plugins/main/windows/CONTRIBUTORS.md b/agent/vendor/github.com/containernetworking/plugins/plugins/main/windows/CONTRIBUTORS.md new file mode 100644 index 00000000000..ea050a8ede6 --- /dev/null +++ b/agent/vendor/github.com/containernetworking/plugins/plugins/main/windows/CONTRIBUTORS.md @@ -0,0 +1,6 @@ +# Contributors +This is the official list of the Windows CNI network plugins contributors: + - @rakelkar + - @madhanrm + - @thxCode + - @nagiesek \ No newline at end of file diff --git a/agent/version/version.go b/agent/version/version.go index 9c0bc701212..020cb04557a 100644 --- a/agent/version/version.go +++ b/agent/version/version.go @@ -22,10 +22,10 @@ package version // repository. Only the 'Version' const should change in checked-in source code // Version is the version of the Agent -const Version = "1.41.1" +const Version = "1.42.0" // GitDirty indicates the cleanliness of the git repo when this agent was built const GitDirty = true // GitShortHash is the short hash of this agent build -const GitShortHash = "0d002a15" +const GitShortHash = "1a88cff9"