ecs-init support for old docker engines (pre docker 17.x) and future …

…docker engines (when api 1.25 is deprecated).

ecs-init/docker/dependencies.go

@@ -30,14 +30,13 @@ import (
 )
 
 const (
-	// dockerClientAPIVersion specifies the minimum docker client API version
-	// required by ECS Init
-	// Version 1.25 is required for setting Init to true when constructing
-	// the HostConfig for creating the ECS Agent to enable the Task
-	// Networking with ENI capability
-	dockerClientAPIVersion = "1.25"
+	// enableTaskENIDockerClientAPIVersion is the docker version at which we enable
+	// the ECS_ENABLE_TASK_ENI env var and set agent's "Init" field to true.
+	enableTaskENIDockerClientAPIVersion = "1.25"
 )
 
+var dockerAPIVersion string
+
 type dockerclient interface {
 	ListImages(opts godocker.ListImagesOptions) ([]godocker.APIImages, error)
 	LoadImage(opts godocker.LoadImageOptions) error
@@ -65,13 +64,98 @@ func (client godockerClientFactory) NewVersionedClient(endpoint string, apiVersi
 	return godocker.NewVersionedClient(endpoint, apiVersionString)
 }
 
+// backupAPIVersions are API versions that have been deprecated on docker engine 25,
+// they are only checked if none of the preferredAPIVersions are available.
+// This list can be updated with time as docker engine deprecates more API versions.
+func backupAPIVersions() []string {
+	return []string{
+		"1.21",
+		"1.22",
+		"1.23",
+		"1.24",
+	}
+}
+
+// preferredAPIVersions returns the preferred list of docker client API versions.
+// When establishing the docker client for ecs-init to use, we will try these API version
+// numbers first.
+func preferredAPIVersions() []string {
+	return []string{
+		"1.25",
+		"1.26",
+		"1.27",
+		"1.28",
+		"1.29",
+		"1.30",
+		"1.31",
+		"1.32",
+		"1.33",
+		"1.34",
+		"1.35",
+		"1.36",
+		"1.37",
+		"1.38",
+		"1.39",
+		"1.40",
+		"1.41",
+		"1.42",
+		"1.43",
+		"1.44",
+	}
+}
+
+// knownAPIVersions returns all known docker API versions, in order from
+// oldest to newest.
+func knownAPIVersions() []string {
+	return append(backupAPIVersions(), preferredAPIVersions()...)
+}
+
+// dockerVersionCompare returns 0 if lhs and rhs are equal.
+// returns 1 if lhs > rhs
+// returns -1 if lhs < rhs
+func dockerVersionCompare(lhs, rhs string) int {
+	if lhs == rhs {
+		return 0
+	}
+	var lhsI int = -1
+	var rhsI int = -1
+	for i, v := range knownAPIVersions() {
+		if lhs == v {
+			lhsI = i
+		}
+		if rhs == v {
+			rhsI = i
+		}
+	}
+	if lhsI < rhsI {
+		return -1
+	}
+	return 1
+}
+
 func newDockerClient(dockerClientFactory dockerClientFactory, pingBackoff backoff.Backoff) (dockerclient, error) {
+	var dockerClient dockerclient
+	var err error
+	allAPIVersions := append(preferredAPIVersions(), backupAPIVersions()...)
+	for _, apiVersion := range allAPIVersions {
+		dockerClient, err = newVersionedDockerClient(apiVersion, dockerClientFactory, pingBackoff)
+		if err == nil {
+			log.Infof("Successfully created docker client with API version %s", apiVersion)
+			dockerAPIVersion = apiVersion
+			break
+		}
+		log.Infof("Could not create docker client with API version %s: %s", apiVersion, err)
+	}
+	return dockerClient, err
+}
+
+func newVersionedDockerClient(apiVersion string, dockerClientFactory dockerClientFactory, pingBackoff backoff.Backoff) (dockerclient, error) {
 	dockerUnixSocketSourcePath, fromEnv := config.DockerUnixSocket()
 	if !fromEnv {
 		dockerUnixSocketSourcePath = "/var/run/docker.sock"
 	}
 	client, err := dockerClientFactory.NewVersionedClient(
-		config.UnixSocketPrefix+dockerUnixSocketSourcePath, dockerClientAPIVersion)
+		config.UnixSocketPrefix+dockerUnixSocketSourcePath, apiVersion)
 	if err != nil {
 		return nil, err
 	}
@@ -157,6 +241,9 @@ func isNetworkError(err error) bool {
 func isRetryablePingError(err error) bool {
 	godockerError, ok := err.(*godocker.Error)
 	if ok {
+		if godockerError.Status == http.StatusBadRequest {
+			return false
+		}
 		return godockerError.Status != http.StatusOK
 	}
 	return false

ecs-init/docker/dependencies_test.go

@@ -1,6 +1,3 @@
-//go:build test
-// +build test
-
 // Copyright 2015-2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License"). You may
@@ -35,6 +32,7 @@ const immediately = time.Duration(-1)
 
 var netError = &url.Error{Err: &net.OpError{Op: "read", Net: "unix", Err: io.EOF}}
 var httpError = &docker.Error{Status: http.StatusInternalServerError, Message: "error"}
+var badRequestError = &docker.Error{Status: http.StatusBadRequest, Message: "Bad Request Error"}
 
 func TestIsNetworkErrorReturnsTrue(t *testing.T) {
 	assert.True(t, isNetworkError(netError), "Expect isNetworkError to return true if network error passed in")
@@ -48,6 +46,10 @@ func TestIsRetryablePingErrorReturnsTrue(t *testing.T) {
 	assert.True(t, isRetryablePingError(httpError), "Expect RetryablePingError to be true if httpError passed in")
 }
 
+func TestIsRetryableBadRequestErrorReturnsFalse(t *testing.T) {
+	assert.False(t, isRetryablePingError(badRequestError), "Expect RetryablePingError to be false if 'bad request' client error passed in")
+}
+
 func TestIsRetryablePingErrorReturnsFalse(t *testing.T) {
 	assert.False(t, isRetryablePingError(fmt.Errorf("error")), "Expect RetryablePingError to be true if non-http error passed in")
 }
@@ -100,10 +102,33 @@ func TestNewDockerClientDoesnotRetryOnPingNonNetworkError(t *testing.T) {
 	mockClientFactory := NewMockdockerClientFactory(ctrl)
 	mockBackoff := NewMockBackoff(ctrl)
 
-	gomock.InOrder(
-		mockClientFactory.EXPECT().NewVersionedClient(gomock.Any(), gomock.Any()).Return(mockDockerClient, nil),
-		mockDockerClient.EXPECT().Ping().Return(fmt.Errorf("error")),
-	)
+	calls := []*gomock.Call{}
+	for _, apiVersion := range append(preferredAPIVersions(), backupAPIVersions()...) {
+		t.Logf("Expecting NewVersionedClient call with docker api version %s", apiVersion)
+		calls = append(calls, mockClientFactory.EXPECT().NewVersionedClient(gomock.Any(), apiVersion).Return(mockDockerClient, nil))
+		calls = append(calls, mockDockerClient.EXPECT().Ping().Return(fmt.Errorf("error")))
+	}
+	gomock.InOrder(calls...)
+
+	_, err := newDockerClient(mockClientFactory, mockBackoff)
+	assert.Error(t, err, "Expect error when creating docker client with no retry")
+}
+
+func TestNewDockerClientDoesNotRetryOnHTTPBadRequestError(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	mockDockerClient := NewMockdockerclient(ctrl)
+	mockClientFactory := NewMockdockerClientFactory(ctrl)
+	mockBackoff := NewMockBackoff(ctrl)
+
+	calls := []*gomock.Call{}
+	for _, apiVersion := range append(preferredAPIVersions(), backupAPIVersions()...) {
+		t.Logf("Expecting NewVersionedClient call with docker api version %s", apiVersion)
+		calls = append(calls, mockClientFactory.EXPECT().NewVersionedClient(gomock.Any(), apiVersion).Return(mockDockerClient, nil))
+		calls = append(calls, mockDockerClient.EXPECT().Ping().Return(badRequestError))
+	}
+	gomock.InOrder(calls...)
 
 	_, err := newDockerClient(mockClientFactory, mockBackoff)
 	assert.Error(t, err, "Expect error when creating docker client with no retry")
@@ -117,14 +142,17 @@ func TestNewDockerClientGivesUpRetryingOnPingNetworkError(t *testing.T) {
 	mockClientFactory := NewMockdockerClientFactory(ctrl)
 	mockBackoff := NewMockBackoff(ctrl)
 
-	gomock.InOrder(
-		mockClientFactory.EXPECT().NewVersionedClient(gomock.Any(), gomock.Any()).Return(mockDockerClient, nil),
-		mockDockerClient.EXPECT().Ping().Return(netError),
-		mockBackoff.EXPECT().ShouldRetry().Return(true),
-		mockBackoff.EXPECT().Duration().Return(immediately),
-		mockDockerClient.EXPECT().Ping().Return(netError),
-		mockBackoff.EXPECT().ShouldRetry().Return(false),
-	)
+	calls := []*gomock.Call{}
+	for _, apiVersion := range append(preferredAPIVersions(), backupAPIVersions()...) {
+		t.Logf("Expecting NewVersionedClient call with docker api version %s", apiVersion)
+		calls = append(calls, mockClientFactory.EXPECT().NewVersionedClient(gomock.Any(), apiVersion).Return(mockDockerClient, nil))
+		calls = append(calls, mockDockerClient.EXPECT().Ping().Return(netError))
+		calls = append(calls, mockBackoff.EXPECT().ShouldRetry().Return(true))
+		calls = append(calls, mockBackoff.EXPECT().Duration().Return(immediately))
+		calls = append(calls, mockDockerClient.EXPECT().Ping().Return(netError))
+		calls = append(calls, mockBackoff.EXPECT().ShouldRetry().Return(false))
+	}
+	gomock.InOrder(calls...)
 
 	_, err := newDockerClient(mockClientFactory, mockBackoff)
 	assert.Error(t, err, "Expect error when creating docker client with no retry")
@@ -137,20 +165,20 @@ func TestNewDockerClientGivesUpRetryingOnUnavailableSocket(t *testing.T) {
 	mockClientFactory := NewMockdockerClientFactory(ctrl)
 	mockBackoff := NewMockBackoff(ctrl)
 
-	realDockerClient, dockerClientErr := godockerClientFactory{}.NewVersionedClient("unix:///a/bad/docker.sock", dockerClientAPIVersion)
+	realDockerClient, dockerClientErr := godockerClientFactory{}.NewVersionedClient("unix:///a/bad/docker.sock", "1.25")
 
 	require.False(t, dockerClientErr != nil, "there should be no errors trying to set up a docker client (intentionally bad with nonexistent socket path")
 
-	gomock.InOrder(
-		// We use the real client to ensure we're classifying errors
-		// correctly as returned by the upstream's error handling.
-		mockClientFactory.EXPECT().NewVersionedClient(gomock.Any(), gomock.Any()).Return(realDockerClient, dockerClientErr),
+	calls := []*gomock.Call{}
+	for _, apiVersion := range append(preferredAPIVersions(), backupAPIVersions()...) {
+		t.Logf("Expecting NewVersionedClient call with docker api version %s", apiVersion)
+		calls = append(calls, mockClientFactory.EXPECT().NewVersionedClient(gomock.Any(), apiVersion).Return(realDockerClient, nil))
 		// "bad connection", retries
-		mockBackoff.EXPECT().ShouldRetry().Return(true),
-		mockBackoff.EXPECT().Duration().Return(immediately),
-		// Give up
-		mockBackoff.EXPECT().ShouldRetry().Return(false),
-	)
+		calls = append(calls, mockBackoff.EXPECT().ShouldRetry().Return(true))
+		calls = append(calls, mockBackoff.EXPECT().Duration().Return(immediately))
+		calls = append(calls, mockBackoff.EXPECT().ShouldRetry().Return(false))
+	}
+	gomock.InOrder(calls...)
 
 	_, err := newDockerClient(mockClientFactory, mockBackoff)
 	require.Error(t, err, "expect an error when creating docker client")
@@ -160,3 +188,77 @@ func TestNewDockerClientGivesUpRetryingOnUnavailableSocket(t *testing.T) {
 	_, isExpectedError := err.(*url.Error)
 	assert.True(t, isExpectedError, "expect net.OpError wrapped by url.Error")
 }
+
+func TestDockerVersionCompare(t *testing.T) {
+	testCases := []struct {
+		lhs            string
+		rhs            string
+		expectedResult int
+	}{
+		{
+			lhs:            "1.21",
+			rhs:            "1.24",
+			expectedResult: -1,
+		},
+		{
+			lhs:            "1.21",
+			rhs:            "1.25",
+			expectedResult: -1,
+		},
+		{
+			lhs:            "1.21",
+			rhs:            "1.44",
+			expectedResult: -1,
+		},
+		{
+			lhs:            "1.21",
+			rhs:            "1.21",
+			expectedResult: 0,
+		},
+		{
+			lhs:            "1.25",
+			rhs:            "1.25",
+			expectedResult: 0,
+		},
+		{
+			lhs:            "1.35",
+			rhs:            "1.35",
+			expectedResult: 0,
+		},
+		{
+			lhs:            "1.44",
+			rhs:            "1.44",
+			expectedResult: 0,
+		},
+		{
+			lhs:            "1.24",
+			rhs:            "1.22",
+			expectedResult: 1,
+		},
+		{
+			lhs:            "1.25",
+			rhs:            "1.22",
+			expectedResult: 1,
+		},
+		{
+			lhs:            "1.34",
+			rhs:            "1.25",
+			expectedResult: 1,
+		},
+		{
+			lhs:            "1.44",
+			rhs:            "1.21",
+			expectedResult: 1,
+		},
+		{
+			lhs:            "1.44",
+			rhs:            "1.26",
+			expectedResult: 1,
+		},
+	}
+	for _, tc := range testCases {
+		actualResult := dockerVersionCompare(tc.lhs, tc.rhs)
+		testName := fmt.Sprintf("lhs=%s rhs=%s expectedResult=%d", tc.lhs, tc.rhs, tc.expectedResult)
+		assert.Equal(t, tc.expectedResult, actualResult, testName)
+	}
+}

ecs-init/docker/docker.go

@@ -59,7 +59,7 @@ const (
 	minBackoffDuration = time.Second
 	// maxBackoffDuration specifies the maximum backoff duration for ping to
 	// return a success response from docker socket
-	maxBackoffDuration = 5 * time.Second
+	maxBackoffDuration = 3 * time.Second
 	// backoffJitterMultiple specifies the backoff jitter multiplier
 	// coefficient when pinging the docker socket
 	backoffJitterMultiple = 0.2
@@ -327,6 +327,10 @@ func (c *client) getContainerConfig(envVarsFromFiles map[string]string) *godocke
 		// Task networking is not supported when not running on EC2. Explicitly disable since it's enabled by default.
 		envVariables["ECS_ENABLE_TASK_ENI"] = "false"
 	}
+	if dockerVersionCompare(dockerAPIVersion, enableTaskENIDockerClientAPIVersion) < 0 {
+		// Task networking is not supported before docker API version 1.25
+		envVariables["ECS_ENABLE_TASK_ENI"] = "false"
+	}
 
 	if !isPathValid(config.MountDirectoryEBS(), true) {
 		// EBS Task Attach (EBSTA) is not supported for external instances

ecs-init/docker/docker_config.go

@@ -62,7 +62,12 @@ func createHostConfig(binds []string) *godocker.HostConfig {
 		NetworkMode: networkMode,
 		UsernsMode:  usernsMode,
 		CapAdd:      caps,
-		Init:        true,
+	}
+
+	// Task ENI feature requires agent to be "init" process, so set it if docker API
+	// version is new enough for this feature.
+	if dockerVersionCompare(dockerAPIVersion, enableTaskENIDockerClientAPIVersion) >= 0 {
+		hostConfig.Init = true
 	}
 
 	if ctrdapparmor.HostSupports() {