diff --git a/executors/docker/consts.go b/executors/docker/consts.go index 926d6a4b483a7767d319173fef90d3e97646ca62..058efcfe98596aaa14d238cff7643da085aa613a 100644 --- a/executors/docker/consts.go +++ b/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" diff --git a/executors/docker/executor_docker.go b/executors/docker/executor_docker.go index 11e6e598aa1cb2764a21693b445f3e7af87ae52a..31dbb20835f00952fd05e62837199ba5f5e2ff8f 100644 --- a/executors/docker/executor_docker.go +++ b/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 } diff --git a/executors/docker/executor_docker_command_test.go b/executors/docker/executor_docker_command_test.go index e046aa34d61da7d278638759ee05058b11cd6105..0b4de8dab3aeafdd7c081f1b293eb32ab469d1ef 100644 --- a/executors/docker/executor_docker_command_test.go +++ b/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 } diff --git a/helpers/docker/official_docker_client.go b/helpers/docker/official_docker_client.go index 0e172354db261e560969ffc1b7413da056424b2c..152e7aad1a5af5bbb2a68b17fc8b864b71a2e98c 100644 --- a/helpers/docker/official_docker_client.go +++ b/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) } diff --git a/helpers/docker/official_docker_client_test.go b/helpers/docker/official_docker_client_test.go index aeea0a9e241445310ea08612b847f9224356310b..3ed8e91f450891a317450612d94623b97923332f 100644 --- a/helpers/docker/official_docker_client_test.go +++ b/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()) + }) + } }