Fail early when an unsupported docker API version is found (#4141)

agent/dockerclient/dockerapi/docker_client.go

@@ -115,7 +115,7 @@ type DockerClient interface {
 
 	// WithVersion returns a new DockerClient for which all operations will use the given remote api version.
 	// A default version will be used for a client not produced via this method.
-	WithVersion(dockerclient.DockerVersion) DockerClient
+	WithVersion(dockerclient.DockerVersion) (DockerClient, error)
 
 	// ContainerEvents returns a channel of DockerContainerChangeEvents. Events are placed into the channel and should
 	// be processed by the listener.
@@ -260,14 +260,17 @@ type ImagePullResponse struct {
 	Error    string `json:"error,omitempty"`
 }
 
-func (dg *dockerGoClient) WithVersion(version dockerclient.DockerVersion) DockerClient {
-	return &dockerGoClient{
+func (dg *dockerGoClient) WithVersion(version dockerclient.DockerVersion) (DockerClient, error) {
+	versionedClient := &dockerGoClient{
 		sdkClientFactory: dg.sdkClientFactory,
 		version:          version,
 		auth:             dg.auth,
 		config:           dg.config,
 		context:          dg.context,
 	}
+	// Check if the version is supported
+	_, err := versionedClient.sdkDockerClient()
+	return versionedClient, err
 }
 
 // NewDockerGoClient creates a new DockerGoClient

agent/dockerclient/dockerapi/docker_client_test.go

@@ -1256,7 +1256,12 @@ func TestUsesVersionedClient(t *testing.T) {
 	client, err := NewDockerGoClient(sdkFactory, defaultTestConfig(), ctx)
 	assert.NoError(t, err)
 
-	vclient := client.WithVersion(dockerclient.DockerVersion("1.20"))
+	sdkFactory.EXPECT().
+		GetClient(dockerclient.DockerVersion("1.20")).
+		Return(mockDockerSDK, nil)
+
+	vclient, err := client.WithVersion(dockerclient.DockerVersion("1.20"))
+	require.NoError(t, err)
 
 	sdkFactory.EXPECT().GetClient(dockerclient.DockerVersion("1.20")).Times(2).Return(mockDockerSDK, nil)
 	mockDockerSDK.EXPECT().ContainerStart(gomock.Any(), gomock.Any(), types.ContainerStartOptions{}).Return(nil)
@@ -1280,7 +1285,12 @@ func TestUnavailableVersionError(t *testing.T) {
 	client, err := NewDockerGoClient(sdkFactory, defaultTestConfig(), ctx)
 	assert.NoError(t, err)
 
-	vclient := client.WithVersion(dockerclient.DockerVersion("1.21"))
+	sdkFactory.EXPECT().
+		GetClient(dockerclient.DockerVersion("1.21")).
+		Return(nil, errors.New("Cannot get client"))
+
+	vclient, err := client.WithVersion(dockerclient.DockerVersion("1.21"))
+	require.EqualError(t, err, "Cannot get client")
 
 	sdkFactory.EXPECT().GetClient(dockerclient.DockerVersion("1.21")).Times(1).Return(nil, errors.New("Cannot get client"))
 	metadata := vclient.StartContainer(ctx, "foo", defaultTestConfig().ContainerStartTimeout)

agent/dockerclient/sdkclientfactory/sdkclientfactory.go

@@ -130,11 +130,18 @@ func findDockerVersions(ctx context.Context, endpoint string) map[dockerclient.D
 		}
 		ctx, cancel := context.WithTimeout(ctx, time.Second*5)
 		defer cancel()
-		_, err = dockerClient.ServerVersion(ctx)
+		serverVer, err := dockerClient.ServerVersion(ctx)
 		if err != nil {
 			log.Infof("Unable to get Docker client for version %s: %v", version, err)
 			continue
 		}
+		if version.Compare(dockerclient.DockerVersion(serverVer.APIVersion)) > 0 {
+			// Required client API version exceeds server's API version
+			log.Infof(
+				"Unable to get Docker client for version %s: server's API version is lower at %s",
+				version, serverVer.APIVersion)
+			continue
+		}
 		clients[version] = dockerClient
 		if minVersion == "" {
 			minVersion = version

agent/dockerclient/sdkclientfactory/sdkclientfactory_test.go

@@ -138,6 +138,54 @@ func TestFindSupportedAPIVersionsFromMinAPIVersions(t *testing.T) {
 	}
 }
 
+// Tests that sdkclientfactory.NewFactory checks that the server's API version is not lower
+// than the client's API version.
+func TestFactoryChecksServerVersion(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	allVersions := dockerclient.GetKnownAPIVersions()
+
+	// Set up the mocks and expectations
+	mockClients := make(map[string]*mock_sdkclient.MockClient)
+
+	serverAPIVersion := dockerclient.Version_1_35
+
+	// Ensure that agent pings all known versions of Docker API
+	for i := 0; i < len(allVersions); i++ {
+		mockClients[string(allVersions[i])] = mock_sdkclient.NewMockClient(ctrl)
+		mockClients[string(allVersions[i])].EXPECT().
+			ServerVersion(gomock.Any()).
+			Return(docker.Version{APIVersion: serverAPIVersion.String()}, nil)
+		mockClients[string(allVersions[i])].EXPECT().Ping(gomock.Any()).AnyTimes()
+	}
+
+	// Define the function for the mock client
+	// For simplicity, we will pretend all versions of docker are available
+	newVersionedClient = func(endpoint, version string) (sdkclient.Client, error) {
+		return mockClients[version], nil
+	}
+
+	// Count versions before serverAPIVersion
+	expectedClientCount := 0
+	for _, v := range allVersions {
+		if v.Compare(serverAPIVersion) <= 0 {
+			expectedClientCount++
+		}
+	}
+
+	ctx, cancel := context.WithCancel(context.TODO())
+	defer cancel()
+	factory := NewFactory(ctx, expectedEndpoint)
+	actualVersions := factory.FindSupportedAPIVersions()
+
+	assert.NotEmpty(t, actualVersions)
+	// Ensure that each supported version is lower than serverAPIVersion
+	for _, actualVersion := range actualVersions {
+		assert.True(t, actualVersion.Compare(serverAPIVersion) <= 0)
+	}
+}
+
 func TestGetClientCached(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()

agent/engine/docker_task_engine.go

@@ -1526,7 +1526,8 @@ func (engine *DockerTaskEngine) createContainer(task *apitask.Task, container *a
 			"usingDockerAPIVersion":   minVersion,
 			"payloadDockerAPIVersion": *container.DockerConfig.Version,
 		})
-		client = client.WithVersion(minVersion)
+		// Error in creating versioned client is dealt with later when client.APIVersion() is called
+		client, _ = client.WithVersion(minVersion)
 	}
 
 	dockerContainerName := ""
@@ -1840,7 +1841,8 @@ func (engine *DockerTaskEngine) startContainer(task *apitask.Task, container *ap
 			"usingDockerAPIVersion":   minVersion,
 			"payloadDockerAPIVersion": *container.DockerConfig.Version,
 		})
-		client = client.WithVersion(minVersion)
+		// Error in creating versioned client is dealt with later when client.StartContainer() is called
+		client, _ = client.WithVersion(minVersion)
 	}
 
 	dockerID, err := engine.getDockerID(task, container)