From 7aebdb5bea0ab8d0da74efae63c791242605ea39 Mon Sep 17 00:00:00 2001 From: Sharanya Devaraj Date: Thu, 12 Oct 2017 21:59:40 +0000 Subject: [PATCH] dockerclient: Agent supported Docker versions from client min,API Versions Finding agent supported Docker versions by getting the default client version and if the min API version is present, add all the clients between min API version and API version supported, else follow current logic --- CHANGELOG.md | 4 +- .../dockerclient/dockerclientfactory.go | 78 +++++++++++++++++-- .../dockerclient/dockerclientfactory_test.go | 62 ++++++++++++++- .../dockerclientfactory_unix_test.go | 4 +- .../dockerclientfactory_windows_test.go | 6 +- .../dockerclient/versionsupport_unix.go | 4 + .../dockerclient/versionsupport_windows.go | 8 +- 7 files changed, 148 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f30803bf2c..3773ac5e123 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,14 @@ ## UNRELEASED * Feature - Support for provisioning Tasks with ENIs +* Bug - Fixed a bug where unsupported Docker API client versions could be registered +[#1014](https://github.com/aws/amazon-ecs-agent/pull/1014) ## 1.14.5 * Enhancement - Retry failed container image pull operations [#975](https://github.com/aws/amazon-ecs-agent/pull/975) * Enhancement - Set read and write timeouts for websocket connectons [#993](https://github.com/aws/amazon-ecs-agent/pull/993) * Enhancement - Add support for the SumoLogic Docker log driver plugin - [#992](https://github.com/aws/amazon-ecs-agent/pull/992) + [#992](https://github.com/aws/amazon-ecs-agent/pull/992) * Bug - Fixed a memory leak issue when submitting the task state change [#967](https://github.com/aws/amazon-ecs-agent/pull/967) * Bug - Fixed a race condition where a container can be created twice when agent restarts. [#939](https://github.com/aws/amazon-ecs-agent/pull/939) * Bug - Fixed an issue where `microsoft/windowsservercore:latest` was not diff --git a/agent/engine/dockerclient/dockerclientfactory.go b/agent/engine/dockerclient/dockerclientfactory.go index b5dd9bc3a9f..9b869b346b3 100644 --- a/agent/engine/dockerclient/dockerclientfactory.go +++ b/agent/engine/dockerclient/dockerclientfactory.go @@ -14,13 +14,23 @@ package dockerclient import ( - "errors" - "github.com/aws/amazon-ecs-agent/agent/engine/dockeriface" + "github.com/aws/amazon-ecs-agent/agent/utils" log "github.com/cihub/seelog" docker "github.com/fsouza/go-dockerclient" + "github.com/pkg/errors" ) +const ( + // minAPIVersionKey is the docker.Env key for min API version + // This is supported in Docker API versions >=1.25 + // https://docs.docker.com/engine/api/version-history/#v125-api-changes + minAPIVersionKey = "MinAPIVersion" + // apiVersionKey is the docker.Env key for API version + apiVersionKey = "ApiVersion" + // zeroPatch is a string to append patch number zero if the major minor version lacks it + zeroPatch = ".0" +) // Factory provides a collection of docker remote clients that include a // recommended client version as well as a set of alternative supported // docker clients. @@ -110,19 +120,71 @@ func (f *factory) getClient(version DockerVersion) (dockeriface.Client, error) { // findDockerVersions loops over all known API versions and finds which ones // are supported by the docker daemon on the host func findDockerVersions(endpoint string) map[DockerVersion]dockeriface.Client { + // if the client version returns a MinAPIVersion and APIVersion, then 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 { + clientVersion, err := client.Version() + if err == nil { + // check if the docker.Env obj has MinAPIVersion key + if clientVersion.Exists(minAPIVersionKey) { + minAPIVersion = clientVersion.Get(minAPIVersionKey) + } + // check if the docker.Env obj has APIVersion key + if clientVersion.Exists(apiVersionKey) { + apiVersion = clientVersion.Get(apiVersionKey) + } + } + } + clients := make(map[DockerVersion]dockeriface.Client) for _, version := range getKnownAPIVersions() { - client, err := newVersionedClient(endpoint, string(version)) + dockerClient, err := getDockerClientForVersion(endpoint, string(version), minAPIVersion, apiVersion) if err != nil { - log.Infof("Error while creating client: %v", err) + log.Infof("Unable to get Docker client for version %s: %v", version, err) continue } + clients[version] = dockerClient + } + return clients +} - err = client.Ping() +func getDockerClientForVersion( + endpoint string, + version string, + minAPIVersion string, + apiVersion string) (dockeriface.Client, error) { + if (minAPIVersion != "" && apiVersion != "") { + // Adding patch number zero to Docker versions to reuse the existing semver + // comparator + // TODO: remove this logic later when non-semver comparator is implemented + versionWithPatch := version + zeroPatch + lessThanMinCheck := "<" + minAPIVersion + zeroPatch + moreThanMaxCheck := ">" + apiVersion + zeroPatch + minVersionCheck, err := utils.Version(versionWithPatch).Matches(lessThanMinCheck) + if err != nil { + return nil, errors.Wrapf(err, "version detection using MinAPIVersion: unable to get min version: %s", minAPIVersion) + } + maxVersionCheck, err := utils.Version(versionWithPatch).Matches(moreThanMaxCheck) if err != nil { - log.Infof("Failed to ping with Docker version %s: %v", version, err) + return nil, errors.Wrapf(err, "version detection using MinAPIVersion: unable to get max version: %s", apiVersion) + } + // do not add the version when it is less than min api version or greater + // than api version + if minVersionCheck || maxVersionCheck { + return nil, errors.Errorf("version detection using MinAPIVersion: unsupported version: %s", version) } - clients[version] = client } - return clients + 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() + if err != nil { + return nil, errors.Wrapf(err, "version detection check: failed to ping with Docker version: %s", string(version)) + } + return client, nil } diff --git a/agent/engine/dockerclient/dockerclientfactory_test.go b/agent/engine/dockerclient/dockerclientfactory_test.go index fa7a444a98a..e4f3e030988 100644 --- a/agent/engine/dockerclient/dockerclientfactory_test.go +++ b/agent/engine/dockerclient/dockerclientfactory_test.go @@ -21,6 +21,7 @@ import ( "github.com/aws/amazon-ecs-agent/agent/engine/dockeriface/mocks" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + docker "github.com/fsouza/go-dockerclient" ) const expectedEndpoint = "expectedEndpoint" @@ -35,7 +36,8 @@ func TestGetDefaultClientSuccess(t *testing.T) { if version == string(getDefaultVersion()) { mockClient = expectedClient } - mockClient.EXPECT().Ping() + mockClient.EXPECT().Version().Return(&docker.Env{}, nil).AnyTimes() + mockClient.EXPECT().Ping().AnyTimes() return mockClient, nil } @@ -59,7 +61,8 @@ func TestFindSupportedAPIVersions(t *testing.T) { // Ensure that agent pings all known versions of Docker API for i := 0; i < len(allVersions); i++ { mockClients[string(allVersions[i])] = mock_dockeriface.NewMockClient(ctrl) - mockClients[string(allVersions[i])].EXPECT().Ping() + mockClients[string(allVersions[i])].EXPECT().Version().Return(&docker.Env{}, nil).AnyTimes() + mockClients[string(allVersions[i])].EXPECT().Ping().AnyTimes() } // Define the function for the mock client @@ -92,3 +95,58 @@ func TestVerifyAgentVersions(t *testing.T) { assert.True(t, isKnown(agentVersion)) } } + +func TestFindSupportedAPIVersionsFromMinAPIVersions(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + agentVersions := getAgentVersions() + allVersions := getKnownAPIVersions() + + // Set up the mocks and expectations + mockClients := make(map[string]*mock_dockeriface.MockClient) + + // Ensure that agent pings all known versions of Docker API + for i := 0; i < len(allVersions); i++ { + mockClients[string(allVersions[i])] = mock_dockeriface.NewMockClient(ctrl) + mockClients[string(allVersions[i])].EXPECT().Version().Return(&docker.Env{"MinAPIVersion=1.12","ApiVersion=1.27"}, nil).AnyTimes() + mockClients[string(allVersions[i])].EXPECT().Ping().AnyTimes() + } + + // Define the function for the mock client + // For simplicity, we will pretend all versions of docker are available + newVersionedClient = func(endpoint, version string) (dockeriface.Client, error) { + return mockClients[version], nil + } + + factory := NewFactory(expectedEndpoint) + actualVersions := factory.FindSupportedAPIVersions() + + assert.Equal(t, len(agentVersions), len(actualVersions)) + for i := 0; i < len(actualVersions); i++ { + assert.Equal(t, agentVersions[i], actualVersions[i]) + } +} + +func TestCompareDockerVersionsWithMinAPIVersion(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + minAPIVersion := "1.12" + apiVersion := "1.27" + versions := []string{"1.11", "1.29"} + rightVersion := "1.25" + + for _, version := range versions { + _, err := getDockerClientForVersion("endpoint", version, minAPIVersion, apiVersion) + assert.EqualError(t, err, "version detection using MinAPIVersion: unsupported version: " + version) + } + + mockClients := make(map[string]*mock_dockeriface.MockClient) + newVersionedClient = func(endpoint, version string) (dockeriface.Client, error) { + mockClients[version] = mock_dockeriface.NewMockClient(ctrl) + mockClients[version].EXPECT().Ping() + return mockClients[version], nil + } + client, _ := getDockerClientForVersion("endpoint", rightVersion, minAPIVersion, apiVersion) + assert.Equal(t, mockClients[rightVersion], client) +} diff --git a/agent/engine/dockerclient/dockerclientfactory_unix_test.go b/agent/engine/dockerclient/dockerclientfactory_unix_test.go index ed3af1fd0c0..9492495418e 100644 --- a/agent/engine/dockerclient/dockerclientfactory_unix_test.go +++ b/agent/engine/dockerclient/dockerclientfactory_unix_test.go @@ -21,6 +21,7 @@ import ( "github.com/aws/amazon-ecs-agent/agent/engine/dockeriface/mocks" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + docker "github.com/fsouza/go-dockerclient" ) func TestGetClientCached(t *testing.T) { @@ -29,7 +30,8 @@ func TestGetClientCached(t *testing.T) { newVersionedClient = func(endpoint, version string) (dockeriface.Client, error) { mockClient := mock_dockeriface.NewMockClient(ctrl) - mockClient.EXPECT().Ping() + mockClient.EXPECT().Version().Return(&docker.Env{}, nil).AnyTimes() + mockClient.EXPECT().Ping().AnyTimes() return mockClient, nil } diff --git a/agent/engine/dockerclient/dockerclientfactory_windows_test.go b/agent/engine/dockerclient/dockerclientfactory_windows_test.go index 142d31e0422..a2c007d3f6c 100644 --- a/agent/engine/dockerclient/dockerclientfactory_windows_test.go +++ b/agent/engine/dockerclient/dockerclientfactory_windows_test.go @@ -21,6 +21,7 @@ import ( "github.com/aws/amazon-ecs-agent/agent/engine/dockeriface/mocks" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + docker "github.com/fsouza/go-dockerclient" ) func TestGetClientMinimumVersion(t *testing.T) { @@ -31,10 +32,11 @@ func TestGetClientMinimumVersion(t *testing.T) { newVersionedClient = func(endpoint, version string) (dockeriface.Client, error) { mockClient := mock_dockeriface.NewMockClient(ctrl) - if version == string(MinDockerAPIWindows) { + if version == string(minDockerAPIVersion) { mockClient = expectedClient } - mockClient.EXPECT().Ping() + mockClient.EXPECT().Version().Return(&docker.Env{}, nil).AnyTimes() + mockClient.EXPECT().Ping().AnyTimes() return mockClient, nil } diff --git a/agent/engine/dockerclient/versionsupport_unix.go b/agent/engine/dockerclient/versionsupport_unix.go index 5dd82640b9a..37a45067329 100644 --- a/agent/engine/dockerclient/versionsupport_unix.go +++ b/agent/engine/dockerclient/versionsupport_unix.go @@ -18,6 +18,10 @@ import ( "github.com/aws/amazon-ecs-agent/agent/engine/dockeriface" ) +const ( + // minDockerAPIVersion is the min Docker API version supported by agent + minDockerAPIVersion = Version_1_17 +) // GetClient on linux will simply return the cached client from the map func (f *factory) GetClient(version DockerVersion) (dockeriface.Client, error) { return f.getClient(version) diff --git a/agent/engine/dockerclient/versionsupport_windows.go b/agent/engine/dockerclient/versionsupport_windows.go index b075ba4acd7..0b6e1693ee1 100644 --- a/agent/engine/dockerclient/versionsupport_windows.go +++ b/agent/engine/dockerclient/versionsupport_windows.go @@ -18,14 +18,14 @@ import ( "github.com/aws/amazon-ecs-agent/agent/engine/dockeriface" ) -const MinDockerAPIWindows = Version_1_24 +const minDockerAPIVersion = 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 DockerVersion) (dockeriface.Client, error) { for _, v := range getWindowsReplaceableVersions() { if v == version { - version = MinDockerAPIWindows + version = minDockerAPIVersion break } } @@ -48,10 +48,10 @@ func getWindowsReplaceableVersions() []DockerVersion { // getAgentVersions for Windows should return all of the replaceable versions plus additional versions func getAgentVersions() []DockerVersion { - return append(getWindowsReplaceableVersions(), MinDockerAPIWindows) + return append(getWindowsReplaceableVersions(), minDockerAPIVersion) } // getDefaultVersion returns agent's default version of the Docker API func getDefaultVersion() DockerVersion { - return MinDockerAPIWindows + return minDockerAPIVersion }