dockerclient: Agent supported Docker versions from client min,API Ver…

…sions

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

@@ -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

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
 }

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)
+}

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
 	}
 

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
 	}
 

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)

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
 }