diff --git a/agent/app/agent.go b/agent/app/agent.go index f8760b648c..39c4e5c517 100644 --- a/agent/app/agent.go +++ b/agent/app/agent.go @@ -1097,26 +1097,24 @@ func (agent *ecsAgent) startACSSession( return exitcodes.ExitSuccess } -// validateRequiredVersion validates docker version. +// verifyRequiredDockerVersion validates docker version. // Minimum docker version supported is 1.9.0, maps to api version 1.21 // see https://docs.docker.com/develop/sdk/#api-version-matrix func (agent *ecsAgent) verifyRequiredDockerVersion() (int, bool) { - supportedVersions := agent.dockerClient.SupportedVersions() + supportedVersions := dockerclient.SupportedVersionsExtended(agent.dockerClient.SupportedVersions) if len(supportedVersions) == 0 { seelog.Critical("Could not get supported docker versions.") return exitcodes.ExitError, false } - // if api version 1.21 is supported, it means docker version is at least 1.9.0 for _, version := range supportedVersions { - if version == dockerclient.Version_1_21 { + if version == dockerclient.MinDockerAPIVersion { return -1, true } } - // api 1.21 is not supported, docker version is older than 1.9.0 - seelog.Criticalf("Required minimum docker API verion %s is not supported", - dockerclient.Version_1_21) + seelog.Criticalf("Required minimum docker API version %s is not supported", + dockerclient.MinDockerAPIVersion) return exitcodes.ExitTerminal, false } diff --git a/agent/app/agent_capability.go b/agent/app/agent_capability.go index cb35c7c23b..aef94844aa 100644 --- a/agent/app/agent_capability.go +++ b/agent/app/agent_capability.go @@ -206,14 +206,14 @@ func (agent *ecsAgent) capabilities() ([]*ecs.Attribute, error) { } supportedVersions := make(map[dockerclient.DockerVersion]bool) - // Determine API versions to report as supported. Supported versions are also used for capability-enablement, except - // logging drivers. - for _, version := range agent.dockerClient.SupportedVersions() { + // Determine API versions to report as supported via com.amazonaws.ecs.capability.docker-remote-api.X.XX capabilities + // and for determining which features we support that depend on specific docker API versions + for _, version := range dockerclient.SupportedVersionsExtended(agent.dockerClient.SupportedVersions) { capabilities = appendNameOnlyAttribute(capabilities, capabilityPrefix+"docker-remote-api."+string(version)) supportedVersions[version] = true } - capabilities = agent.appendLoggingDriverCapabilities(capabilities) + capabilities = agent.appendLoggingDriverCapabilities(capabilities, supportedVersions) if agent.cfg.SELinuxCapable.Enabled() { capabilities = appendNameOnlyAttribute(capabilities, capabilityPrefix+"selinux") @@ -315,7 +315,6 @@ func (agent *ecsAgent) appendDockerDependentCapabilities(capabilities []*ecs.Att capabilities = appendNameOnlyAttribute(capabilities, capabilityPrefix+"ecr-auth") capabilities = appendNameOnlyAttribute(capabilities, attributePrefix+"execution-role-ecr-pull") } - 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") @@ -339,17 +338,10 @@ func (agent *ecsAgent) appendGMSADomainlessCapabilities(capabilities []*ecs.Attr return capabilities } -func (agent *ecsAgent) appendLoggingDriverCapabilities(capabilities []*ecs.Attribute) []*ecs.Attribute { - knownVersions := make(map[dockerclient.DockerVersion]struct{}) - // Determine known API versions. Known versions are used exclusively for logging-driver enablement, since none of - // the structural API elements change. - for _, version := range agent.dockerClient.KnownVersions() { - knownVersions[version] = struct{}{} - } - +func (agent *ecsAgent) appendLoggingDriverCapabilities(capabilities []*ecs.Attribute, supportedVersions map[dockerclient.DockerVersion]bool) []*ecs.Attribute { for _, loggingDriver := range agent.cfg.AvailableLoggingDrivers { requiredVersion := dockerclient.LoggingDriverMinimumVersion[loggingDriver] - if _, ok := knownVersions[requiredVersion]; ok { + if _, ok := supportedVersions[requiredVersion]; ok { capabilities = appendNameOnlyAttribute(capabilities, capabilityPrefix+"logging-driver."+string(loggingDriver)) } } diff --git a/agent/app/agent_capability_test.go b/agent/app/agent_capability_test.go index 8aa03db662..1fbf303180 100644 --- a/agent/app/agent_capability_test.go +++ b/agent/app/agent_capability_test.go @@ -108,10 +108,6 @@ func TestCapabilities(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, dockerclient.Version_1_18, - }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - dockerclient.Version_1_18, dockerclient.Version_1_19, }), // CNI plugins are platform dependent. @@ -255,11 +251,6 @@ func getCapabilitiesWithConfig(cfg *config.Config, t *testing.T) []*ecs.Attribut dockerclient.Version_1_17, dockerclient.Version_1_18, }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - dockerclient.Version_1_18, - dockerclient.Version_1_19, - }), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -295,7 +286,6 @@ func TestCapabilitiesECR(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_19, }) - client.EXPECT().KnownVersions().Return(nil) mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil) client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil) @@ -352,7 +342,6 @@ func TestCapabilitiesTaskIAMRoleForSupportedDockerVersion(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_19, }) - client.EXPECT().KnownVersions().Return(nil) mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil) client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil) @@ -407,7 +396,6 @@ func TestCapabilitiesTaskIAMRoleForUnSupportedDockerVersion(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_18, }) - client.EXPECT().KnownVersions().Return(nil) mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil) client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil) @@ -462,7 +450,6 @@ func TestCapabilitiesTaskIAMRoleNetworkHostForSupportedDockerVersion(t *testing. client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_19, }) - client.EXPECT().KnownVersions().Return(nil) mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil) client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil) @@ -517,7 +504,6 @@ func TestCapabilitiesTaskIAMRoleNetworkHostForUnSupportedDockerVersion(t *testin client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_18, }) - client.EXPECT().KnownVersions().Return(nil) mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil) client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil) @@ -591,11 +577,6 @@ func TestAWSVPCBlockInstanceMetadataWhenTaskENIIsDisabled(t *testing.T) { dockerclient.Version_1_17, dockerclient.Version_1_18, }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - dockerclient.Version_1_18, - dockerclient.Version_1_19, - }), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -660,7 +641,6 @@ func TestCapabilitiesExecutionRoleAWSLogs(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, }) - client.EXPECT().KnownVersions().Return(nil) // CNI plugins are platform dependent. // Therefore, for any version query for any plugin return an error cniClient.EXPECT().Version(gomock.Any()).Return("v1", errors.New("some error happened")) @@ -727,7 +707,6 @@ func TestCapabilitiesTaskResourceLimit(t *testing.T) { gomock.InOrder( client.EXPECT().SupportedVersions().Return(versionList), - client.EXPECT().KnownVersions().Return(versionList), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -781,7 +760,6 @@ func TestCapabilitesTaskResourceLimitDisabledByMissingDockerVersion(t *testing.T gomock.InOrder( client.EXPECT().SupportedVersions().Return(versionList), - client.EXPECT().KnownVersions().Return(versionList), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -834,7 +812,6 @@ func TestCapabilitesTaskResourceLimitErrorCase(t *testing.T) { gomock.InOrder( client.EXPECT().SupportedVersions().Return(versionList), - client.EXPECT().KnownVersions().Return(versionList), ) ctx, cancel := context.WithCancel(context.TODO()) // Cancel the context to cancel async routines @@ -904,7 +881,6 @@ func TestCapabilitiesIncreasedTaskCPULimit(t *testing.T) { gomock.InOrder( client.EXPECT().SupportedVersions().Return(versionList), - client.EXPECT().KnownVersions().Return(versionList), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -947,7 +923,6 @@ func TestCapabilitiesContainerHealth(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_24, }) - client.EXPECT().KnownVersions().Return(nil) mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil) client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil) @@ -998,7 +973,6 @@ func TestCapabilitiesContainerHealthDisabled(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_24, }) - client.EXPECT().KnownVersions().Return(nil) mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil) client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil) @@ -1058,7 +1032,6 @@ func TestCapabilitesListPluginsErrorCase(t *testing.T) { gomock.InOrder( client.EXPECT().SupportedVersions().Return(versionList), - client.EXPECT().KnownVersions().Return(versionList), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil, errors.New("listPlugins error happened")), @@ -1106,7 +1079,6 @@ func TestCapabilitesScanPluginsErrorCase(t *testing.T) { gomock.InOrder( client.EXPECT().SupportedVersions().Return(versionList), - client.EXPECT().KnownVersions().Return(versionList), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return(nil, errors.New("Scan plugins error happened")), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -1224,7 +1196,6 @@ func TestCapabilitiesExecuteCommand(t *testing.T) { gomock.InOrder( client.EXPECT().SupportedVersions().Return(versionList), - client.EXPECT().KnownVersions().Return(versionList), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return(nil, errors.New("Scan plugins error happened")), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -1310,10 +1281,6 @@ func TestCapabilitiesNoServiceConnect(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, dockerclient.Version_1_18, - }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - dockerclient.Version_1_18, dockerclient.Version_1_19, }), // CNI plugins are platform dependent. diff --git a/agent/app/agent_capability_unix_test.go b/agent/app/agent_capability_unix_test.go index 1e531581ec..d95a58e204 100644 --- a/agent/app/agent_capability_unix_test.go +++ b/agent/app/agent_capability_unix_test.go @@ -88,10 +88,6 @@ func TestVolumeDriverCapabilitiesUnix(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, dockerclient.Version_1_18, - }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - dockerclient.Version_1_18, dockerclient.Version_1_19, }), cniClient.EXPECT().Version(ecscni.VPCENIPluginName).Return("v1", nil), @@ -183,9 +179,6 @@ func TestNvidiaDriverCapabilitiesUnix(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - }), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -269,9 +262,6 @@ func TestEmptyNvidiaDriverCapabilitiesUnix(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - }), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -349,9 +339,6 @@ func TestENITrunkingCapabilitiesUnix(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - }), cniClient.EXPECT().Version(ecscni.VPCENIPluginName).Return("v1", nil), cniClient.EXPECT().Version(ecscni.ECSBranchENIPluginName).Return("v2", nil), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), @@ -444,9 +431,6 @@ func TestNoENITrunkingCapabilitiesUnix(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - }), cniClient.EXPECT().Version(ecscni.VPCENIPluginName).Return("v1", nil), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), @@ -527,9 +511,6 @@ func TestPIDAndIPCNamespaceSharingCapabilitiesUnix(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - }), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -606,9 +587,6 @@ func TestPIDAndIPCNamespaceSharingCapabilitiesNoPauseContainer(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - }), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -683,9 +661,6 @@ func TestAppMeshCapabilitiesUnix(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - }), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -769,9 +744,6 @@ func TestTaskEIACapabilitiesNoOptimizedCPU(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - }), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -828,9 +800,6 @@ func TestTaskEIACapabilitiesWithOptimizedCPU(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - }), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -884,9 +853,6 @@ func TestCapabilitiesUnix(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - }), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -969,9 +935,6 @@ func TestFirelensConfigCapabilitiesUnix(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - }), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), client.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), diff --git a/agent/app/agent_capability_windows_test.go b/agent/app/agent_capability_windows_test.go index 850bc7c0fb..7fd9c32043 100644 --- a/agent/app/agent_capability_windows_test.go +++ b/agent/app/agent_capability_windows_test.go @@ -69,10 +69,6 @@ func TestVolumeDriverCapabilitiesWindows(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, dockerclient.Version_1_18, - }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - dockerclient.Version_1_18, dockerclient.Version_1_19, }), cniClient.EXPECT().Version(ecscni.ECSVPCENIPluginExecutable).Return("v1", nil), @@ -82,6 +78,7 @@ func TestVolumeDriverCapabilitiesWindows(t *testing.T) { capabilityPrefix + "privileged-container", capabilityPrefix + "docker-remote-api.1.17", capabilityPrefix + "docker-remote-api.1.18", + capabilityPrefix + "docker-remote-api.1.19", capabilityPrefix + "logging-driver.json-file", capabilityPrefix + "logging-driver.syslog", capabilityPrefix + "logging-driver.journald", @@ -159,10 +156,6 @@ func TestSupportedCapabilitiesWindows(t *testing.T) { client.EXPECT().SupportedVersions().Return([]dockerclient.DockerVersion{ dockerclient.Version_1_17, dockerclient.Version_1_18, - }), - client.EXPECT().KnownVersions().Return([]dockerclient.DockerVersion{ - dockerclient.Version_1_17, - dockerclient.Version_1_18, dockerclient.Version_1_19, }), cniClient.EXPECT().Version(ecscni.ECSVPCENIPluginExecutable).Return("v1", nil), @@ -172,6 +165,7 @@ func TestSupportedCapabilitiesWindows(t *testing.T) { capabilityPrefix + "privileged-container", capabilityPrefix + "docker-remote-api.1.17", capabilityPrefix + "docker-remote-api.1.18", + capabilityPrefix + "docker-remote-api.1.19", capabilityPrefix + "logging-driver.json-file", capabilityPrefix + "logging-driver.syslog", capabilityPrefix + "logging-driver.journald", @@ -221,7 +215,6 @@ func TestSupportedCapabilitiesWindows(t *testing.T) { capabilities, err := agent.capabilities() assert.NoError(t, err) - assert.Equal(t, len(expectedCapabilities), len(capabilities)) for _, expected := range expectedCapabilities { assert.Contains(t, capabilities, &ecs.Attribute{ Name: expected.Name, diff --git a/agent/app/agent_test.go b/agent/app/agent_test.go index 02079e3688..0215c6afa8 100644 --- a/agent/app/agent_test.go +++ b/agent/app/agent_test.go @@ -21,7 +21,6 @@ import ( "errors" "fmt" "sort" - "sync" "testing" "time" @@ -80,9 +79,8 @@ var notFoundErr = awserr.NewRequestFailure(awserr.Error(awserr.New("NotFound", " var badReqErr = awserr.NewRequestFailure(awserr.Error(awserr.New("BadRequest", "", errors.New(""))), 400, "") var serverErr = awserr.NewRequestFailure(awserr.Error(awserr.New("InternalServerError", "", errors.New(""))), 500, "") var apiVersions = []dockerclient.DockerVersion{ - dockerclient.Version_1_21, - dockerclient.Version_1_22, - dockerclient.Version_1_23} + dockerclient.MinDockerAPIVersion, +} var capabilities []*ecs.Attribute var testHostCPU = int64(1024) var testHostMEMORY = int64(1024) @@ -249,13 +247,11 @@ func TestDoStartRegisterContainerInstanceErrorTerminal(t *testing.T) { mockDaemonManagers := map[string]dm.DaemonManager{md.EbsCsiDriver: mockDaemonManager} mockDaemonManager.EXPECT().IsLoaded(gomock.Any()).Return(true, nil).AnyTimes() mockDaemonManager.EXPECT().LoadImage(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes() gomock.InOrder( - dockerClient.EXPECT().SupportedVersions().Return(apiVersions), client.EXPECT().GetHostResources().Return(testHostResource, nil), mockCredentialsProvider.EXPECT().Retrieve().Return(aws_credentials.Value{}, nil), - dockerClient.EXPECT().SupportedVersions().Return(nil), - dockerClient.EXPECT().KnownVersions().Return(nil), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{""}, nil), dockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -313,13 +309,11 @@ func TestDoStartRegisterContainerInstanceErrorNonTerminal(t *testing.T) { mockDaemonManagers := map[string]dm.DaemonManager{md.EbsCsiDriver: mockDaemonManager} mockDaemonManager.EXPECT().IsLoaded(gomock.Any()).Return(true, nil).AnyTimes() mockDaemonManager.EXPECT().LoadImage(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes() gomock.InOrder( - dockerClient.EXPECT().SupportedVersions().Return(apiVersions), client.EXPECT().GetHostResources().Return(testHostResource, nil), mockCredentialsProvider.EXPECT().Retrieve().Return(aws_credentials.Value{}, nil), - dockerClient.EXPECT().SupportedVersions().Return(nil), - dockerClient.EXPECT().KnownVersions().Return(nil), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{""}, nil), dockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -444,10 +438,7 @@ func testDoStartHappyPathWithConditions(t *testing.T, blackholed bool, warmPools ec2MetadataClient.EXPECT().InstanceID().Return(instanceID, nil) } - var discoverEndpointsInvoked sync.WaitGroup - discoverEndpointsInvoked.Add(2) mockMobyPlugins := mock_mobypkgwrapper.NewMockPlugins(ctrl) - dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).AnyTimes() mockCredentialsProvider := app_mocks.NewMockProvider(ctrl) containermetadata := mock_containermetadata.NewMockManager(ctrl) mockPauseLoader := mock_loader.NewMockLoader(ctrl) @@ -468,28 +459,28 @@ func testDoStartHappyPathWithConditions(t *testing.T, blackholed bool, warmPools return []*md.ManagedDaemon{}, nil } + waitC := make(chan bool, 3) imageManager.EXPECT().AddImageToCleanUpExclusionList(gomock.Eq("service_connect_agent:v1")).Times(1) imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1) dockerClient.EXPECT().ListContainers(gomock.Any(), gomock.Any(), gomock.Any()).Return( dockerapi.ListContainersResponse{}).AnyTimes() + dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes() + dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).Return("20.10.15", nil).AnyTimes() client.EXPECT().DiscoverPollEndpoint(gomock.Any()).Do(func(x interface{}) { - // Ensures that the test waits until acs session has bee started - discoverEndpointsInvoked.Done() + // Ensures that the test waits until acs session has been started + waitC <- true }).Return("poll-endpoint", nil) client.EXPECT().DiscoverPollEndpoint(gomock.Any()).Return("acs-endpoint", nil).AnyTimes() client.EXPECT().DiscoverTelemetryEndpoint(gomock.Any()).Do(func(x interface{}) { - // Ensures that the test waits until telemetry session has bee started - discoverEndpointsInvoked.Done() + // Ensures that the test waits until telemetry session has been started + waitC <- true }).Return("telemetry-endpoint", nil) client.EXPECT().DiscoverTelemetryEndpoint(gomock.Any()).Return( "tele-endpoint", nil).AnyTimes() gomock.InOrder( - dockerClient.EXPECT().SupportedVersions().Return(apiVersions), client.EXPECT().GetHostResources().Return(testHostResource, nil), mockCredentialsProvider.EXPECT().Retrieve().Return(aws_credentials.Value{}, nil), - dockerClient.EXPECT().SupportedVersions().Return(nil), - dockerClient.EXPECT().KnownVersions().Return(nil), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{""}, nil), dockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -535,17 +526,30 @@ func testDoStartHappyPathWithConditions(t *testing.T, blackholed bool, warmPools serviceconnectManager: mockServiceConnectManager, } - var agentW sync.WaitGroup - agentW.Add(1) go func() { agent.doStart(eventstream.NewEventStream("events", ctx), credentialsManager, dockerstate.NewTaskEngineState(), imageManager, client, execCmdMgr) - agentW.Done() + waitC <- true }() - discoverEndpointsInvoked.Wait() + timeoutC := time.After(time.Second * 30) + for i := 0; i < 2; i++ { + select { + case <-waitC: + // expect to receive 2 times from this channel + case <-timeoutC: + ctrl.Finish() + // cancel the loop + i = 4 + } + } cancel() - agentW.Wait() + // wait for agent to exit + select { + case <-waitC: + case <-time.After(time.Second * 10): + t.Logf("Timed out waiting for agent to exit") + } assertMetadata(t, data.AgentVersionKey, version.Version, dataClient) assertMetadata(t, data.AvailabilityZoneKey, availabilityZone, dataClient) @@ -991,8 +995,7 @@ func TestReregisterContainerInstanceHappyPath(t *testing.T) { gomock.InOrder( mockCredentialsProvider.EXPECT().Retrieve().Return(aws_credentials.Value{}, nil), - mockDockerClient.EXPECT().SupportedVersions().Return(nil), - mockDockerClient.EXPECT().KnownVersions().Return(nil), + mockDockerClient.EXPECT().SupportedVersions().Return(apiVersions), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{""}, nil), mockDockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -1051,8 +1054,7 @@ func TestReregisterContainerInstanceInstanceTypeChanged(t *testing.T) { gomock.InOrder( mockCredentialsProvider.EXPECT().Retrieve().Return(aws_credentials.Value{}, nil), - mockDockerClient.EXPECT().SupportedVersions().Return(nil), - mockDockerClient.EXPECT().KnownVersions().Return(nil), + mockDockerClient.EXPECT().SupportedVersions().Return(apiVersions), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{""}, nil), mockDockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -1113,8 +1115,7 @@ func TestReregisterContainerInstanceAttributeError(t *testing.T) { gomock.InOrder( mockCredentialsProvider.EXPECT().Retrieve().Return(aws_credentials.Value{}, nil), - mockDockerClient.EXPECT().SupportedVersions().Return(nil), - mockDockerClient.EXPECT().KnownVersions().Return(nil), + mockDockerClient.EXPECT().SupportedVersions().Return(apiVersions), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), mockDockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -1173,8 +1174,7 @@ func TestReregisterContainerInstanceNonTerminalError(t *testing.T) { gomock.InOrder( mockCredentialsProvider.EXPECT().Retrieve().Return(aws_credentials.Value{}, nil), - mockDockerClient.EXPECT().SupportedVersions().Return(nil), - mockDockerClient.EXPECT().KnownVersions().Return(nil), + mockDockerClient.EXPECT().SupportedVersions().Return(apiVersions), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), mockDockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -1234,8 +1234,7 @@ func TestRegisterContainerInstanceWhenContainerInstanceARNIsNotSetHappyPath(t *t gomock.InOrder( mockCredentialsProvider.EXPECT().Retrieve().Return(aws_credentials.Value{}, nil), - mockDockerClient.EXPECT().SupportedVersions().Return(nil), - mockDockerClient.EXPECT().KnownVersions().Return(nil), + mockDockerClient.EXPECT().SupportedVersions().Return(apiVersions), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), mockDockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -1293,8 +1292,7 @@ func TestRegisterContainerInstanceWhenContainerInstanceARNIsNotSetCanRetryError( retriableError := apierrors.NewRetriableError(apierrors.NewRetriable(true), errors.New("error")) gomock.InOrder( mockCredentialsProvider.EXPECT().Retrieve().Return(aws_credentials.Value{}, nil), - mockDockerClient.EXPECT().SupportedVersions().Return(nil), - mockDockerClient.EXPECT().KnownVersions().Return(nil), + mockDockerClient.EXPECT().SupportedVersions().Return(apiVersions), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), mockDockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -1352,8 +1350,7 @@ func TestRegisterContainerInstanceWhenContainerInstanceARNIsNotSetCannotRetryErr cannotRetryError := apierrors.NewRetriableError(apierrors.NewRetriable(false), errors.New("error")) gomock.InOrder( mockCredentialsProvider.EXPECT().Retrieve().Return(aws_credentials.Value{}, nil), - mockDockerClient.EXPECT().SupportedVersions().Return(nil), - mockDockerClient.EXPECT().KnownVersions().Return(nil), + mockDockerClient.EXPECT().SupportedVersions().Return(apiVersions), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), mockDockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -1410,8 +1407,7 @@ func TestRegisterContainerInstanceWhenContainerInstanceARNIsNotSetAttributeError gomock.InOrder( mockCredentialsProvider.EXPECT().Retrieve().Return(aws_credentials.Value{}, nil), - mockDockerClient.EXPECT().SupportedVersions().Return(nil), - mockDockerClient.EXPECT().KnownVersions().Return(nil), + mockDockerClient.EXPECT().SupportedVersions().Return(apiVersions), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), mockDockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), @@ -1468,12 +1464,11 @@ func TestRegisterContainerInstanceInvalidParameterTerminalError(t *testing.T) { mockDaemonManager.EXPECT().IsLoaded(gomock.Any()).Return(true, nil).AnyTimes() mockDaemonManager.EXPECT().LoadImage(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes() + gomock.InOrder( - dockerClient.EXPECT().SupportedVersions().Return(apiVersions), client.EXPECT().GetHostResources().Return(testHostResource, nil), mockCredentialsProvider.EXPECT().Retrieve().Return(aws_credentials.Value{}, nil), - dockerClient.EXPECT().SupportedVersions().Return(nil), - dockerClient.EXPECT().KnownVersions().Return(nil), mockMobyPlugins.EXPECT().Scan().AnyTimes().Return([]string{}, nil), dockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return([]string{}, nil), diff --git a/agent/app/agent_unix_test.go b/agent/app/agent_unix_test.go index 4683e86844..01ba8e679c 100644 --- a/agent/app/agent_unix_test.go +++ b/agent/app/agent_unix_test.go @@ -90,7 +90,7 @@ func TestDoStartTaskENIHappyPath(t *testing.T) { // invoked via go routines, which will lead to occasional test failues mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes() dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).AnyTimes() - dockerClient.EXPECT().SupportedVersions().Return(apiVersions) + dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes() dockerClient.EXPECT().ListContainers(gomock.Any(), gomock.Any(), gomock.Any()).Return( dockerapi.ListContainersResponse{}).AnyTimes() imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1) @@ -136,8 +136,6 @@ func TestDoStartTaskENIHappyPath(t *testing.T) { cniClient.EXPECT().Capabilities(ecscni.ECSAppMeshPluginName).Return(cniCapabilities, nil), cniClient.EXPECT().Capabilities(ecscni.ECSBranchENIPluginName).Return(cniCapabilities, nil), mockCredentialsProvider.EXPECT().Retrieve().Return(credentials.Value{}, nil), - dockerClient.EXPECT().SupportedVersions().Return(nil), - dockerClient.EXPECT().KnownVersions().Return(nil), cniClient.EXPECT().Version(ecscni.VPCENIPluginName).Return("v1", nil), cniClient.EXPECT().Version(ecscni.ECSBranchENIPluginName).Return("v2", nil), mockMobyPlugins.EXPECT().Scan().Return([]string{}, nil), @@ -453,7 +451,7 @@ func TestDoStartCgroupInitHappyPath(t *testing.T) { ec2MetadataClient := mock_ec2.NewMockEC2MetadataClient(ctrl) dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).AnyTimes() - dockerClient.EXPECT().SupportedVersions().Return(apiVersions) + dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes() imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1) mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes() ec2MetadataClient.EXPECT().PrimaryENIMAC().Return("mac", nil) @@ -482,8 +480,6 @@ func TestDoStartCgroupInitHappyPath(t *testing.T) { gomock.InOrder( mockControl.EXPECT().Init().Return(nil), mockCredentialsProvider.EXPECT().Retrieve().Return(credentials.Value{}, nil), - dockerClient.EXPECT().SupportedVersions().Return(nil), - dockerClient.EXPECT().KnownVersions().Return(nil), mockMobyPlugins.EXPECT().Scan().Return([]string{}, nil), dockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]string{}, nil), @@ -558,7 +554,7 @@ func TestDoStartCgroupInitErrorPath(t *testing.T) { discoverEndpointsInvoked.Add(2) dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).AnyTimes() - dockerClient.EXPECT().SupportedVersions().Return(apiVersions) + dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes() imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1) mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes() mockPauseLoader.EXPECT().LoadImage(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() @@ -632,7 +628,7 @@ func TestDoStartGPUManagerHappyPath(t *testing.T) { containerChangeEvents := make(chan dockerapi.DockerContainerChangeEvent) dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).AnyTimes() - dockerClient.EXPECT().SupportedVersions().Return(apiVersions) + dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes() imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1) mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes() ec2MetadataClient.EXPECT().PrimaryENIMAC().Return("mac", nil) @@ -662,8 +658,6 @@ func TestDoStartGPUManagerHappyPath(t *testing.T) { gomock.InOrder( mockGPUManager.EXPECT().Initialize().Return(nil), mockCredentialsProvider.EXPECT().Retrieve().Return(credentials.Value{}, nil), - dockerClient.EXPECT().SupportedVersions().Return(nil), - dockerClient.EXPECT().KnownVersions().Return(nil), mockMobyPlugins.EXPECT().Scan().Return([]string{}, nil), dockerClient.EXPECT().ListPluginsWithFilters(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]string{}, nil), @@ -741,7 +735,7 @@ func TestDoStartGPUManagerInitError(t *testing.T) { discoverEndpointsInvoked.Add(2) dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).AnyTimes() - dockerClient.EXPECT().SupportedVersions().Return(apiVersions) + dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes() imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1) mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes() mockGPUManager.EXPECT().Initialize().Return(errors.New("init error")) @@ -797,7 +791,7 @@ func TestDoStartTaskENIPauseError(t *testing.T) { // invoked via go routines, which will lead to occasional test failures mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes() dockerClient.EXPECT().Version(gomock.Any(), gomock.Any()).AnyTimes() - dockerClient.EXPECT().SupportedVersions().Return(apiVersions) + dockerClient.EXPECT().SupportedVersions().Return(apiVersions).AnyTimes() dockerClient.EXPECT().ListContainers(gomock.Any(), gomock.Any(), gomock.Any()).Return( dockerapi.ListContainersResponse{}).AnyTimes() imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1) diff --git a/agent/dockerclient/dockerapi_compare_versions_test.go b/agent/dockerclient/dockerapi_compare_versions_test.go index 46e077ef9a..8e1f948ed4 100644 --- a/agent/dockerclient/dockerapi_compare_versions_test.go +++ b/agent/dockerclient/dockerapi_compare_versions_test.go @@ -117,6 +117,16 @@ func TestDockerVersionMatches(t *testing.T) { selector: ">=1.9,<=1.9", expectedOutput: true, }, + { + version: "1.24", + selector: ">=1.24", + expectedOutput: true, + }, + { + version: "1.25", + selector: ">=1.24", + expectedOutput: true, + }, } for i, testCase := range testCases { diff --git a/agent/dockerclient/sdkclientfactory/sdkclientfactory.go b/agent/dockerclient/sdkclientfactory/sdkclientfactory.go index dc9f099404..584efd0aa0 100644 --- a/agent/dockerclient/sdkclientfactory/sdkclientfactory.go +++ b/agent/dockerclient/sdkclientfactory/sdkclientfactory.go @@ -15,6 +15,7 @@ package sdkclientfactory import ( "context" + "time" "github.com/aws/amazon-ecs-agent/agent/dockerclient" "github.com/aws/amazon-ecs-agent/agent/dockerclient/sdkclient" @@ -119,65 +120,32 @@ func (f *factory) getClient(version dockerclient.DockerVersion) (sdkclient.Clien // findDockerVersions loops over all known API versions and finds which ones // are supported by the docker daemon on the host func findDockerVersions(ctx context.Context, endpoint string) map[dockerclient.DockerVersion]sdkclient.Client { - // if the client version returns a MinAPIVersion, use it to return all the - // Docker clients between MinAPIVersion and APIVersion, else try pinging - // the clients in getKnownAPIVersions - var minAPIVersion, apiVersion string - // get a Docker client with the default supported version - client, err := newVersionedClient(endpoint, string(minDockerAPIVersion)) - if err == nil { - derivedCtx, cancel := context.WithTimeout(ctx, dockerclient.VersionTimeout) - defer cancel() - serverVersion, err := client.ServerVersion(derivedCtx) - if err == nil { - apiVersion = serverVersion.APIVersion - // check if the docker.Version has MinAPIVersion value - if serverVersion.MinAPIVersion != "" { - minAPIVersion = serverVersion.MinAPIVersion - } - } - } clients := make(map[dockerclient.DockerVersion]sdkclient.Client) + var minVersion dockerclient.DockerVersion for _, version := range dockerclient.GetKnownAPIVersions() { - dockerClient, err := getDockerClientForVersion(endpoint, string(version), minAPIVersion, apiVersion, ctx) + dockerClient, err := newVersionedClient(endpoint, string(version)) if err != nil { log.Infof("Unable to get Docker client for version %s: %v", version, err) continue } - clients[version] = dockerClient - } - return clients -} - -func getDockerClientForVersion( - endpoint string, - version string, - minAPIVersion string, - apiVersion string, - derivedCtx context.Context) (sdkclient.Client, error) { - if minAPIVersion != "" && apiVersion != "" { - lessThanMinCheck := "<" + minAPIVersion - moreThanMaxCheck := ">" + apiVersion - minVersionCheck, err := dockerclient.DockerAPIVersion(version).Matches(lessThanMinCheck) - if err != nil { - return nil, errors.Wrapf(err, "version detection using MinAPIVersion: unable to get min version: %s", minAPIVersion) - } - maxVersionCheck, err := dockerclient.DockerAPIVersion(version).Matches(moreThanMaxCheck) + ctx, cancel := context.WithTimeout(ctx, time.Second*5) + defer cancel() + _, err = dockerClient.ServerVersion(ctx) if err != nil { - return nil, errors.Wrapf(err, "version detection using MinAPIVersion: unable to get max version: %s", apiVersion) + log.Infof("Unable to get Docker client for version %s: %v", version, err) + continue } - // Do not add the version when it is outside minVersion to maxVersion - if minVersionCheck || maxVersionCheck { - return nil, errors.Errorf("version detection using MinAPIVersion: unsupported version: %s", version) + clients[version] = dockerClient + if minVersion == "" { + minVersion = version } } - client, err := newVersionedClient(endpoint, string(version)) - if err != nil { - return nil, errors.Wrapf(err, "version detection check: unable to create Docker client for version: %s", version) - } - _, err = client.Ping(derivedCtx) - if err != nil { - return nil, errors.Wrapf(err, "version detection check: failed to ping with Docker version: %s", string(version)) + if minVersion.Compare(dockerclient.MinDockerAPIVersion) == 1 { + // If the min supported docker api version is greater than the current min + // version, then set the min version to this new value. + // Only do it if greater to avoid "downgrading" the min version, such as + // from 1.21 to 1.17. + dockerclient.SetMinDockerAPIVersion(minVersion) } - return client, nil + return clients } diff --git a/agent/dockerclient/sdkclientfactory/sdkclientfactory_test.go b/agent/dockerclient/sdkclientfactory/sdkclientfactory_test.go index bc6664d6d7..8a202a87d4 100644 --- a/agent/dockerclient/sdkclientfactory/sdkclientfactory_test.go +++ b/agent/dockerclient/sdkclientfactory/sdkclientfactory_test.go @@ -138,31 +138,6 @@ func TestFindSupportedAPIVersionsFromMinAPIVersions(t *testing.T) { } } -func TestCompareDockerVersionsWithMinAPIVersion(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - minAPIVersion := "1.12" - apiVersion := "1.32" - versions := []string{"1.11", "1.33"} - rightVersion := "1.25" - ctx, cancel := context.WithCancel(context.TODO()) - defer cancel() - - for _, version := range versions { - _, err := getDockerClientForVersion("endpoint", version, minAPIVersion, apiVersion, ctx) - assert.EqualError(t, err, "version detection using MinAPIVersion: unsupported version: "+version) - } - - mockClients := make(map[string]*mock_sdkclient.MockClient) - newVersionedClient = func(endpoint, version string) (sdkclient.Client, error) { - mockClients[version] = mock_sdkclient.NewMockClient(ctrl) - mockClients[version].EXPECT().Ping(gomock.Any()) - return mockClients[version], nil - } - client, _ := getDockerClientForVersion("endpoint", rightVersion, minAPIVersion, apiVersion, ctx) - assert.Equal(t, mockClients[rightVersion], client) -} - func TestGetClientCached(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() diff --git a/agent/dockerclient/sdkclientfactory/sdkclientfactory_windows_test.go b/agent/dockerclient/sdkclientfactory/sdkclientfactory_windows_test.go index 23aba51900..26e0c07a4a 100644 --- a/agent/dockerclient/sdkclientfactory/sdkclientfactory_windows_test.go +++ b/agent/dockerclient/sdkclientfactory/sdkclientfactory_windows_test.go @@ -36,7 +36,7 @@ func TestGetClientMinimumVersion(t *testing.T) { newVersionedClient = func(endpoint, version string) (sdkclient.Client, error) { mockClient := mock_sdkclient.NewMockClient(ctrl) - if version == string(minDockerAPIVersion) { + if version == string(dockerclient.MinDockerAPIVersion) { mockClient = expectedClient } mockClient.EXPECT().ServerVersion(gomock.Any()).Return(docker.Version{}, nil).AnyTimes() @@ -44,8 +44,8 @@ func TestGetClientMinimumVersion(t *testing.T) { return mockClient, nil } - // Ensure a call to GetClient with a version below the minDockerAPIVersion - // is replaced by the minDockerAPIVersion + // Ensure a call to GetClient with a version below the dockerclient.MinDockerAPIVersion + // is replaced by the dockerclient.MinDockerAPIVersion ctx, cancel := context.WithCancel(context.TODO()) defer cancel() factory := NewFactory(ctx, expectedEndpoint) @@ -64,7 +64,7 @@ func TestFindClientAPIVersion(t *testing.T) { for _, version := range getAgentSupportedDockerVersions() { if isWindowsReplaceableVersion(version) { - version = minDockerAPIVersion + version = dockerclient.MinDockerAPIVersion } mockClient.EXPECT().ClientVersion().Return(string(version)) assert.Equal(t, version, factory.FindClientAPIVersion(mockClient)) diff --git a/agent/dockerclient/sdkclientfactory/versionsupport_unix.go b/agent/dockerclient/sdkclientfactory/versionsupport_unix.go index 9117a7d729..3f115e2cfb 100644 --- a/agent/dockerclient/sdkclientfactory/versionsupport_unix.go +++ b/agent/dockerclient/sdkclientfactory/versionsupport_unix.go @@ -21,11 +21,6 @@ import ( "github.com/aws/amazon-ecs-agent/agent/dockerclient/sdkclient" ) -const ( - // minDockerAPIVersion is the min Docker API version supported by agent - minDockerAPIVersion = dockerclient.Version_1_17 -) - // GetClient on linux will simply return the cached client from the map func (f *factory) GetClient(version dockerclient.DockerVersion) (sdkclient.Client, error) { return f.getClient(version) @@ -33,27 +28,10 @@ func (f *factory) GetClient(version dockerclient.DockerVersion) (sdkclient.Clien // getAgentSupportedDockerVersions returns a list of agent-supported Docker versions for linux func getAgentSupportedDockerVersions() []dockerclient.DockerVersion { - return []dockerclient.DockerVersion{ - dockerclient.Version_1_17, - dockerclient.Version_1_18, - dockerclient.Version_1_19, - dockerclient.Version_1_20, - dockerclient.Version_1_21, - dockerclient.Version_1_22, - dockerclient.Version_1_23, - dockerclient.Version_1_24, - dockerclient.Version_1_25, - dockerclient.Version_1_26, - dockerclient.Version_1_27, - dockerclient.Version_1_28, - dockerclient.Version_1_29, - dockerclient.Version_1_30, - dockerclient.Version_1_31, - dockerclient.Version_1_32, - } + return dockerclient.GetKnownAPIVersions() } // getDefaultVersion will return the default Docker API version for linux func GetDefaultVersion() dockerclient.DockerVersion { - return dockerclient.Version_1_21 + return dockerclient.MinDockerAPIVersion } diff --git a/agent/dockerclient/sdkclientfactory/versionsupport_windows.go b/agent/dockerclient/sdkclientfactory/versionsupport_windows.go index afc83054a2..dd91331d5f 100644 --- a/agent/dockerclient/sdkclientfactory/versionsupport_windows.go +++ b/agent/dockerclient/sdkclientfactory/versionsupport_windows.go @@ -21,15 +21,12 @@ import ( "github.com/aws/amazon-ecs-agent/agent/dockerclient/sdkclient" ) -// minDockerAPIVersion is the min Docker API version supported by agent -const minDockerAPIVersion = dockerclient.Version_1_24 - // GetClient will replace some versions of Docker on Windows. We need this because // agent assumes that it can always call older versions of the docker API. func (f *factory) GetClient(version dockerclient.DockerVersion) (sdkclient.Client, error) { for _, v := range getWindowsReplaceableVersions() { if v == version { - version = minDockerAPIVersion + version = dockerclient.MinDockerAPIVersion break } } @@ -71,5 +68,5 @@ func getAgentSupportedDockerVersions() []dockerclient.DockerVersion { // getDefaultVersion returns agent's default version of the Docker API func GetDefaultVersion() dockerclient.DockerVersion { - return minDockerAPIVersion + return dockerclient.MinDockerAPIVersion } diff --git a/agent/dockerclient/versions.go b/agent/dockerclient/versions.go index c9e9913e55..b2aff7f2f1 100644 --- a/agent/dockerclient/versions.go +++ b/agent/dockerclient/versions.go @@ -13,8 +13,21 @@ package dockerclient +import ( + "fmt" + "sync" + + "github.com/aws/amazon-ecs-agent/ecs-agent/logger" +) + type DockerVersion string +var ( + // MinDockerAPIVersion is the min Docker API version supported by agent + MinDockerAPIVersion = Version_1_21 + MinDockerAPIVersionMu sync.Mutex +) + const ( Version_1_17 DockerVersion = "1.17" Version_1_18 DockerVersion = "1.18" @@ -32,12 +45,69 @@ const ( Version_1_30 DockerVersion = "1.30" Version_1_31 DockerVersion = "1.31" Version_1_32 DockerVersion = "1.32" + Version_1_33 DockerVersion = "1.33" + Version_1_34 DockerVersion = "1.34" + Version_1_35 DockerVersion = "1.35" + Version_1_36 DockerVersion = "1.36" + Version_1_37 DockerVersion = "1.37" + Version_1_38 DockerVersion = "1.38" + Version_1_39 DockerVersion = "1.39" + Version_1_40 DockerVersion = "1.40" + Version_1_41 DockerVersion = "1.41" + Version_1_42 DockerVersion = "1.42" + Version_1_43 DockerVersion = "1.43" + Version_1_44 DockerVersion = "1.44" ) func (d DockerVersion) String() string { return string(d) } +// Compare returns 0 if versions are equal +// returns 1 if this version is greater than rhs +// returns -1 if this version if less than rhs +func (d DockerVersion) Compare(rhs DockerVersion) int { + if d == rhs { + return 0 + } + dockerApiVersion := DockerAPIVersion(d) + lessThan, err := dockerApiVersion.Matches(fmt.Sprintf("<%s", rhs)) + if err != nil { + return 0 + } + if lessThan { + return -1 + } + return 1 +} + +// SetMinDockerAPIVersion sets the minimum/default docker API version that the +// ECS agent will use. +func SetMinDockerAPIVersion(v DockerVersion) { + MinDockerAPIVersionMu.Lock() + defer MinDockerAPIVersionMu.Unlock() + if MinDockerAPIVersion == v { + return + } + logger.Info("Setting minimum docker API version", logger.Fields{ + "previousMinAPIVersion": MinDockerAPIVersion, + "newMinAPIVersion": v, + }) + MinDockerAPIVersion = v +} + +// GetSupportedDockerAPIVersion takes in a DockerVersion and: +// 1. if the input version is supported, then the same version is returned. +// 2. if the input version is less than the minimum supported version, then the minimum supported version is returned. +// 3. if the input version is invalid, log a warning and return the minimum supported version. +func GetSupportedDockerAPIVersion(version DockerVersion) DockerVersion { + cmp := version.Compare(MinDockerAPIVersion) + if cmp == 1 { + return version + } + return MinDockerAPIVersion +} + // GetKnownAPIVersions returns all of the API versions that we know about. // It doesn't care if the version is supported by Docker or ECS agent func GetKnownAPIVersions() []DockerVersion { @@ -58,5 +128,17 @@ func GetKnownAPIVersions() []DockerVersion { Version_1_30, Version_1_31, Version_1_32, + Version_1_33, + Version_1_34, + Version_1_35, + Version_1_36, + Version_1_37, + Version_1_38, + Version_1_39, + Version_1_40, + Version_1_41, + Version_1_42, + Version_1_43, + Version_1_44, } } diff --git a/agent/dockerclient/versions_test.go b/agent/dockerclient/versions_test.go new file mode 100644 index 0000000000..f867d000c8 --- /dev/null +++ b/agent/dockerclient/versions_test.go @@ -0,0 +1,100 @@ +//go:build unit +// +build 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 dockerclient + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGetSupportedDockerAPIVersion(t *testing.T) { + oldVersionResult := GetSupportedDockerAPIVersion(Version_1_17) + assert.Equal(t, MinDockerAPIVersion, oldVersionResult) + newVersionResult := GetSupportedDockerAPIVersion(Version_1_32) + assert.Equal(t, Version_1_32, newVersionResult) + invalidVersionResult := GetSupportedDockerAPIVersion(DockerVersion("Foo.Bar")) + assert.Equal(t, MinDockerAPIVersion, invalidVersionResult) +} + +func TestCompare(t *testing.T) { + testCases := []struct { + lhs DockerVersion + rhs DockerVersion + expectedResult int + }{ + { + lhs: Version_1_17, + rhs: Version_1_17, + expectedResult: 0, + }, + { + lhs: Version_1_44, + rhs: Version_1_44, + expectedResult: 0, + }, + { + lhs: Version_1_17, + rhs: Version_1_33, + expectedResult: -1, + }, + { + lhs: Version_1_33, + rhs: Version_1_43, + expectedResult: -1, + }, + { + lhs: Version_1_34, + rhs: Version_1_35, + expectedResult: -1, + }, + { + lhs: Version_1_30, + rhs: Version_1_17, + expectedResult: 1, + }, + { + lhs: Version_1_18, + rhs: Version_1_17, + expectedResult: 1, + }, + { + lhs: Version_1_44, + rhs: Version_1_35, + expectedResult: 1, + }, + { + lhs: DockerVersion("Foo.Bar"), + rhs: Version_1_35, + expectedResult: 0, + }, + { + lhs: Version_1_35, + rhs: DockerVersion("foobar"), + expectedResult: 0, + }, + { + lhs: DockerVersion("Foo.Bar"), + rhs: DockerVersion("Foo.Bar"), + expectedResult: 0, + }, + } + for _, tc := range testCases { + actual := tc.lhs.Compare(tc.rhs) + assert.Equal(t, tc.expectedResult, actual) + } +} diff --git a/agent/dockerclient/versions_unix.go b/agent/dockerclient/versions_unix.go new file mode 100644 index 0000000000..ef0675338f --- /dev/null +++ b/agent/dockerclient/versions_unix.go @@ -0,0 +1,49 @@ +//go:build !windows +// +build !windows + +// 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 dockerclient + +import "github.com/aws/amazon-ecs-agent/ecs-agent/logger" + +// SupportedVersionsExtended takes in a function that returns supported docker API versions. +// It returns a list of "extended" API versions that may not be directly supported +// but their functionality is supported. +// For example, in Docker Engine 25.x API versions 1.17-1.23 were deprecated to be used +// directly, although their functionality is still supported. +func SupportedVersionsExtended(supportedVersionsFn func() []DockerVersion) []DockerVersion { + supportedAPIVersions := supportedVersionsFn() + // Check if MinDockerAPIVersion is greater than or equal to 1.24 + // This version is used because this is the earliest API version supported when docker + // deprecated early API versions. See https://docs.docker.com/engine/deprecated/ + cmp := MinDockerAPIVersion.Compare(Version_1_24) + + if cmp == 1 || cmp == 0 { + // extend list of supported versions to include earlier known versions of docker api + extendedAPIVersions := []DockerVersion{} + knownAPIVersions := GetKnownAPIVersions() + for _, knownAPIVersion := range knownAPIVersions { + if knownAPIVersion.Compare(MinDockerAPIVersion) < 0 { + extendedAPIVersions = append(extendedAPIVersions, knownAPIVersion) + } + } + supportedAPIVersions = append(extendedAPIVersions, supportedAPIVersions...) + } + + logger.Debug("Extended supported docker API versions", logger.Fields{ + "supportedAPIVersions": supportedAPIVersions, + }) + return supportedAPIVersions +} diff --git a/agent/dockerclient/versions_unix_test.go b/agent/dockerclient/versions_unix_test.go new file mode 100644 index 0000000000..1d963ae875 --- /dev/null +++ b/agent/dockerclient/versions_unix_test.go @@ -0,0 +1,119 @@ +//go:build !windows && unit +// +build !windows,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 dockerclient + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func setTestMinDockerAPIVersion(minVersion DockerVersion) func() { + origMin := MinDockerAPIVersion + SetMinDockerAPIVersion(minVersion) + return func() { + SetMinDockerAPIVersion(origMin) + } +} + +func TestSupportedVersionsExtended(t *testing.T) { + defer setTestMinDockerAPIVersion(Version_1_19)() + supportedVersionsTestFunc := func() []DockerVersion { + return []DockerVersion{ + Version_1_19, + Version_1_20, + Version_1_21, + Version_1_22, + Version_1_23, + Version_1_24, + } + } + actualSupportedVersions := SupportedVersionsExtended(supportedVersionsTestFunc) + expectedSupportedVersions := []DockerVersion{ + Version_1_19, + Version_1_20, + Version_1_21, + Version_1_22, + Version_1_23, + Version_1_24, + } + + assert.Equal(t, + expectedSupportedVersions, + actualSupportedVersions, + ) +} + +func TestSupportedVersionsExtended_1_24(t *testing.T) { + // Min version 1.24 triggers the 'extended' docker versions to be added + defer setTestMinDockerAPIVersion(Version_1_24)() + supportedVersionsTestFunc := func() []DockerVersion { + return []DockerVersion{ + Version_1_24, + Version_1_25, + Version_1_26, + } + } + actualSupportedVersions := SupportedVersionsExtended(supportedVersionsTestFunc) + expectedSupportedVersions := []DockerVersion{ + Version_1_17, + Version_1_18, + Version_1_19, + Version_1_20, + Version_1_21, + Version_1_22, + Version_1_23, + Version_1_24, + Version_1_25, + Version_1_26, + } + + assert.Equal(t, + expectedSupportedVersions, + actualSupportedVersions, + ) +} + +func TestSupportedVersionsExtended_1_26(t *testing.T) { + // Min version greater thanm 1.24 triggers the 'extended' docker versions to be added + defer setTestMinDockerAPIVersion(Version_1_26)() + supportedVersionsTestFunc := func() []DockerVersion { + return []DockerVersion{ + Version_1_26, + Version_1_27, + } + } + actualSupportedVersions := SupportedVersionsExtended(supportedVersionsTestFunc) + expectedSupportedVersions := []DockerVersion{ + Version_1_17, + Version_1_18, + Version_1_19, + Version_1_20, + Version_1_21, + Version_1_22, + Version_1_23, + Version_1_24, + Version_1_25, + Version_1_26, + Version_1_27, + } + + assert.Equal(t, + expectedSupportedVersions, + actualSupportedVersions, + ) +} diff --git a/agent/dockerclient/versions_windows.go b/agent/dockerclient/versions_windows.go new file mode 100644 index 0000000000..1bbafd5cdf --- /dev/null +++ b/agent/dockerclient/versions_windows.go @@ -0,0 +1,23 @@ +//go:build windows +// +build windows + +// 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 dockerclient + +// SupportVersionsExtended is a no-op on windows as windows uses the getWindowsReplaceableVersions +// function for this. +func SupportedVersionsExtended(supportedVersionsFn func() []DockerVersion) []DockerVersion { + return supportedVersionsFn() +} diff --git a/agent/engine/docker_task_engine.go b/agent/engine/docker_task_engine.go index 92a7fcd230..d2c9ecf6d9 100644 --- a/agent/engine/docker_task_engine.go +++ b/agent/engine/docker_task_engine.go @@ -1505,7 +1505,14 @@ func (engine *DockerTaskEngine) createContainer(task *apitask.Task, container *a }) client := engine.client if container.DockerConfig.Version != nil { - client = client.WithVersion(dockerclient.DockerVersion(*container.DockerConfig.Version)) + minVersion := dockerclient.GetSupportedDockerAPIVersion(dockerclient.DockerVersion(*container.DockerConfig.Version)) + logger.Debug("CreateContainer: overriding docker API version in task payload", logger.Fields{ + field.TaskID: task.GetID(), + field.Container: container.Name, + "usingDockerAPIVersion": minVersion, + "payloadDockerAPIVersion": *container.DockerConfig.Version, + }) + client = client.WithVersion(minVersion) } dockerContainerName := "" @@ -1812,7 +1819,14 @@ func (engine *DockerTaskEngine) startContainer(task *apitask.Task, container *ap }) client := engine.client if container.DockerConfig.Version != nil { - client = client.WithVersion(dockerclient.DockerVersion(*container.DockerConfig.Version)) + minVersion := dockerclient.GetSupportedDockerAPIVersion(dockerclient.DockerVersion(*container.DockerConfig.Version)) + logger.Debug("StartContainer: overriding docker API version in task payload", logger.Fields{ + field.TaskID: task.GetID(), + field.Container: container.Name, + "usingDockerAPIVersion": minVersion, + "payloadDockerAPIVersion": *container.DockerConfig.Version, + }) + client = client.WithVersion(minVersion) } dockerID, err := engine.getDockerID(task, container) diff --git a/agent/engine/engine_sudo_linux_integ_test.go b/agent/engine/engine_sudo_linux_integ_test.go index 02fef36d8f..269e7ad0d2 100644 --- a/agent/engine/engine_sudo_linux_integ_test.go +++ b/agent/engine/engine_sudo_linux_integ_test.go @@ -607,7 +607,8 @@ func verifyExecCmdAgentExpectedMounts(t *testing.T, ctx context.Context, client *sdkClient.Client, testTaskId, containerId, containerName, testExecCmdHostVersionedBinDir, testConfigFileName, testLogConfigFileName string) { - inspectState, _ := client.ContainerInspect(ctx, containerId) + inspectState, err := client.ContainerInspect(ctx, containerId) + require.NoError(t, err) expectedMounts := []struct { source string