Merge pull request #1014 from sharanyad/capability-detection-fix

dockerclient: Agent supported Docker versions from client min,API Versions

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
 }