Consolidate docker API version definition

Refactor to only have 1 definition of the API version, since
there are two places that the API version is defined.

Update behavior of `docker.New` to where, if empty string is specified
the default version would be used.

executors/docker/consts.go

@@ -2,7 +2,6 @@ package docker
 
 import "time"
 
-const DockerAPIVersion = "1.18"
 const dockerLabelPrefix = "com.gitlab.gitlab-runner"
 
 const prebuiltImageName = "gitlab/gitlab-runner-helper"

executors/docker/executor_docker.go

@@ -1155,7 +1155,7 @@ func (e *executor) overwriteEntrypoint(image *common.Image) []string {
 }
 
 func (e *executor) connectDocker() (err error) {
-	client, err := docker_helpers.New(e.Config.Docker.DockerCredentials, DockerAPIVersion)
+	client, err := docker_helpers.New(e.Config.Docker.DockerCredentials, "")
 	if err != nil {
 		return err
 	}

executors/docker/executor_docker_command_test.go

@@ -17,7 +17,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab-runner/common"
-	"gitlab.com/gitlab-org/gitlab-runner/executors/docker"
 	"gitlab.com/gitlab-org/gitlab-runner/helpers"
 	"gitlab.com/gitlab-org/gitlab-runner/helpers/docker"
 )
@@ -210,8 +209,7 @@ func TestDockerCommandDisableEntrypointOverwrite(t *testing.T) {
 }
 
 func isDockerOlderThan17_07(t *testing.T) bool {
-	client, err := docker_helpers.New(
-		docker_helpers.DockerCredentials{}, docker.DockerAPIVersion)
+	client, err := docker_helpers.New(docker_helpers.DockerCredentials{}, "")
 	require.NoError(t, err, "should be able to connect to docker")
 
 	types, err := client.Info(context.Background())
@@ -736,7 +734,7 @@ func getDockerCredentials(id string) (credentials docker_helpers.DockerCredentia
 }
 
 func waitForDocker(credentials docker_helpers.DockerCredentials) error {
-	client, err := docker_helpers.New(credentials, docker.DockerAPIVersion)
+	client, err := docker_helpers.New(credentials, "")
 	if err != nil {
 		return err
 	}

helpers/docker/official_docker_client.go

@@ -10,14 +10,17 @@ import (
 	"time"
 
 	"github.com/docker/docker/api/types"
-	container "github.com/docker/docker/api/types/container"
-	network "github.com/docker/docker/api/types/network"
+	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/client"
 	"github.com/docker/go-connections/tlsconfig"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/net/context"
 )
 
+// The default API version used to create a new docker client.
+const DefaultAPIVersion = "1.18"
+
 // IsErrNotFound checks whether a returned error is due to an image or container
 // not being found. Proxies the docker implementation.
 func IsErrNotFound(err error) bool {
@@ -189,7 +192,8 @@ func (c *officialDockerClient) Close() error {
 	return nil
 }
 
-// New attempts to create a new Docker client of the specified version.
+// New attempts to create a new Docker client of the specified version. If the
+// specified version is empty, it will use the default version.
 //
 // If no host is given in the DockerCredentials, it will attempt to look up
 // details from the environment. If that fails, it will use the default
@@ -204,6 +208,10 @@ func New(c DockerCredentials, apiVersion string) (Client, error) {
 		c.Host = client.DefaultDockerHost
 	}
 
+	if apiVersion == "" {
+		apiVersion = DefaultAPIVersion
+	}
+
 	return newOfficialDockerClient(c, apiVersion)
 }
 

helpers/docker/official_docker_client_test.go

@@ -10,8 +10,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-const dockerAPIVersion = "1.18"
-
 func prepareDockerClientAndFakeServer(t *testing.T, handler http.HandlerFunc) (Client, *httptest.Server) {
 	server := httptest.NewServer(handler)
 
@@ -20,7 +18,7 @@ func prepareDockerClientAndFakeServer(t *testing.T, handler http.HandlerFunc) (C
 		TLSVerify: false,
 	}
 
-	client, err := New(credentials, dockerAPIVersion)
+	client, err := New(credentials, "")
 	require.NoError(t, err)
 
 	return client, server
@@ -37,5 +35,33 @@ func TestWrapError(t *testing.T) {
 
 	_, err := client.Info(ctx)
 	require.Error(t, err, "The request should respond with an error")
-	assert.Regexp(t, "\\(official_docker_client_test.go:38:\\d+s\\)", err.Error())
+	assert.Regexp(t, "\\(official_docker_client_test.go:\\d\\d:\\d+s\\)", err.Error())
+}
+
+func TestNew_Version(t *testing.T) {
+	cases := []struct {
+		version         string
+		host            string
+		expectedVersion string
+	}{
+		{
+			version:         "0.11",
+			expectedVersion: "0.11",
+		},
+		{
+			version:         "",
+			expectedVersion: DefaultAPIVersion,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.expectedVersion, func(t *testing.T) {
+			client, err := New(DockerCredentials{}, c.version)
+			require.NoError(t, err)
+
+			test, ok := client.(*officialDockerClient)
+			assert.True(t, ok)
+			assert.Equal(t, c.expectedVersion, test.client.ClientVersion())
+		})
+	}
 }