Merge pull request #2558 from tiborvass/19.03-expenv-panic

[19.03] Fix bug with panic when DOCKER_CLI_EXPERIMENTAL environment variable is incorrect

Jenkinsfile

@@ -1,13 +1,47 @@
-wrappedNode(label: 'linux && x86_64', cleanWorkspace: true) {
-  timeout(time: 60, unit: 'MINUTES') {
-    stage "Git Checkout"
-    checkout scm
+pipeline {
+    agent {
+        label "linux && x86_64"
+    }
 
-    stage "Run end-to-end test suite"
-    sh "docker version"
-    sh "docker info"
-    sh "E2E_UNIQUE_ID=clie2e${BUILD_NUMBER} \
-        IMAGE_TAG=clie2e${BUILD_NUMBER} \
-        DOCKER_BUILDKIT=1 make -f docker.Makefile test-e2e"
-  }
+    options {
+        timeout(time: 60, unit: 'MINUTES')
+    }
+
+    stages {
+        stage("Docker info") {
+            steps {
+                sh "docker version"
+                sh "docker info"
+            }
+        }
+        stage("e2e (non-experimental) - stable engine") {
+            steps {
+                sh "E2E_UNIQUE_ID=clie2e${BUILD_NUMBER} \
+                    IMAGE_TAG=clie2e${BUILD_NUMBER} \
+                    DOCKER_BUILDKIT=1 make -f docker.Makefile test-e2e-non-experimental"
+            }
+        }
+        stage("e2e (non-experimental) - 18.09 engine") {
+            steps {
+                sh "E2E_ENGINE_VERSION=18.09-dind \
+                  E2E_UNIQUE_ID=clie2e${BUILD_NUMBER} \
+                  IMAGE_TAG=clie2e${BUILD_NUMBER} \
+                  DOCKER_BUILDKIT=1 make -f docker.Makefile test-e2e-non-experimental"
+            }
+        }
+        stage("e2e (experimental)") {
+            steps {
+                sh "E2E_UNIQUE_ID=clie2e${BUILD_NUMBER} \
+                    IMAGE_TAG=clie2e${BUILD_NUMBER} \
+                    DOCKER_BUILDKIT=1 make -f docker.Makefile test-e2e-experimental"
+            }
+        }
+        stage("e2e (ssh connhelper)") {
+            steps {
+                sh "E2E_UNIQUE_ID=clie2e${BUILD_NUMBER} \
+                    IMAGE_TAG=clie2e${BUILD_NUMBER} \
+                    DOCKER_BUILDKIT=1 make -f docker.Makefile test-e2e-connhelper-ssh"
+            }
+        }
+    }
 }

appveyor.yml

@@ -4,7 +4,7 @@ clone_folder: c:\gopath\src\github.com\docker\cli
 
 environment:
   GOPATH: c:\gopath
-  GOVERSION: 1.13.10
+  GOVERSION: 1.13.11
   DEPVERSION: v0.4.1
 
 install:

cli/command/cli.go

@@ -9,7 +9,6 @@ import (
 	"runtime"
 	"strconv"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/docker/cli/cli/config"
@@ -140,19 +139,18 @@ func (cli *DockerCli) loadConfigFile() {
 	cli.configFile = cliconfig.LoadDefaultConfigFile(cli.err)
 }
 
-var fetchServerInfo sync.Once
-
 // ServerInfo returns the server version details for the host this client is
 // connected to
 func (cli *DockerCli) ServerInfo() ServerInfo {
-	fetchServerInfo.Do(cli.initializeFromClient)
 	return cli.serverInfo
 }
 
 // ClientInfo returns the client details for the cli
 func (cli *DockerCli) ClientInfo() ClientInfo {
 	if cli.clientInfo == nil {
-		_ = cli.loadClientInfo()
+		if err := cli.loadClientInfo(); err != nil {
+			panic(err)
+		}
 	}
 	return *cli.clientInfo
 }
@@ -280,6 +278,12 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize
 			return err
 		}
 	}
+	cli.initializeFromClient()
+
+	if err := cli.loadClientInfo(); err != nil {
+		return err
+	}
+
 	return nil
 }
 

cli/connhelper/connhelper.go

@@ -34,7 +34,7 @@ func GetConnectionHelper(daemonURL string) (*ConnectionHelper, error) {
 		}
 		return &ConnectionHelper{
 			Dialer: func(ctx context.Context, network, addr string) (net.Conn, error) {
-				return commandconn.New(ctx, "ssh", append(sp.Args(), []string{"--", "docker", "system", "dial-stdio"}...)...)
+				return commandconn.New(ctx, "ssh", sp.Args("docker", "system", "dial-stdio")...)
 			},
 			Host: "http://docker",
 		}, nil

cli/connhelper/ssh/ssh.go

@@ -49,8 +49,8 @@ type Spec struct {
 	Port string
 }
 
-// Args returns args except "ssh" itself and "-- ..."
-func (sp *Spec) Args() []string {
+// Args returns args except "ssh" itself combined with optional additional command args
+func (sp *Spec) Args(add ...string) []string {
 	var args []string
 	if sp.User != "" {
 		args = append(args, "-l", sp.User)
@@ -58,6 +58,7 @@ func (sp *Spec) Args() []string {
 	if sp.Port != "" {
 		args = append(args, "-p", sp.Port)
 	}
-	args = append(args, sp.Host)
+	args = append(args, "--", sp.Host)
+	args = append(args, add...)
 	return args
 }

cli/connhelper/ssh/ssh_test.go

@@ -16,7 +16,7 @@ func TestParseURL(t *testing.T) {
 		{
 			url: "ssh://foo",
 			expectedArgs: []string{
-				"foo",
+				"--", "foo",
 			},
 		},
 		{
@@ -24,7 +24,7 @@ func TestParseURL(t *testing.T) {
 			expectedArgs: []string{
 				"-l", "me",
 				"-p", "10022",
-				"foo",
+				"--", "foo",
 			},
 		},
 		{
@@ -53,12 +53,14 @@ func TestParseURL(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
-		sp, err := ParseURL(tc.url)
-		if tc.expectedError == "" {
-			assert.NilError(t, err)
-			assert.Check(t, is.DeepEqual(tc.expectedArgs, sp.Args()))
-		} else {
-			assert.ErrorContains(t, err, tc.expectedError)
-		}
+		t.Run(tc.url, func(t *testing.T) {
+			sp, err := ParseURL(tc.url)
+			if tc.expectedError == "" {
+				assert.NilError(t, err)
+				assert.Check(t, is.DeepEqual(tc.expectedArgs, sp.Args()))
+			} else {
+				assert.ErrorContains(t, err, tc.expectedError)
+			}
+		})
 	}
 }

docker.Makefile

@@ -16,12 +16,13 @@ LINTER_IMAGE_NAME = docker-cli-lint$(IMAGE_TAG)
 CROSS_IMAGE_NAME = docker-cli-cross$(IMAGE_TAG)
 VALIDATE_IMAGE_NAME = docker-cli-shell-validate$(IMAGE_TAG)
 E2E_IMAGE_NAME = docker-cli-e2e$(IMAGE_TAG)
+E2E_ENGINE_VERSION ?=
 CACHE_VOLUME_NAME := docker-cli-dev-cache
 ifeq ($(DOCKER_CLI_GO_BUILD_CACHE),y)
 DOCKER_CLI_MOUNTS += -v "$(CACHE_VOLUME_NAME):/root/.cache/go-build"
 endif
 VERSION = $(shell cat VERSION)
-ENVVARS = -e VERSION=$(VERSION) -e GITCOMMIT -e PLATFORM -e TESTFLAGS -e TESTDIRS -e GOOS -e GOARCH -e GOARM
+ENVVARS = -e VERSION=$(VERSION) -e GITCOMMIT -e PLATFORM -e TESTFLAGS -e TESTDIRS -e GOOS -e GOARCH -e GOARM -e TEST_ENGINE_VERSION=$(E2E_ENGINE_VERSION)
 
 # build docker image (dockerfiles/Dockerfile.build)
 .PHONY: build_docker_image
@@ -145,7 +146,7 @@ test-e2e-experimental: build_e2e_image # run experimental e2e tests
 
 .PHONY: test-e2e-non-experimental
 test-e2e-non-experimental: build_e2e_image # run non-experimental e2e tests
-	docker run --rm -v /var/run/docker.sock:/var/run/docker.sock $(ENVVARS) $(E2E_IMAGE_NAME)
+	docker run --rm -v /var/run/docker.sock:/var/run/docker.sock $(ENVVARS) -e TEST_ENGINE_VERSION=$(E2E_ENGINE_VERSION) $(E2E_IMAGE_NAME)
 
 .PHONY: test-e2e-connhelper-ssh
 test-e2e-connhelper-ssh: build_e2e_image # run experimental SSH-connection helper e2e tests

dockerfiles/Dockerfile.binary-native

@@ -1,4 +1,4 @@
-ARG GO_VERSION=1.13.10
+ARG GO_VERSION=1.13.11
 
 FROM    golang:${GO_VERSION}-alpine
 

dockerfiles/Dockerfile.cross

@@ -1,4 +1,4 @@
-ARG GO_VERSION=1.13.10
+ARG GO_VERSION=1.13.11
 
 FROM	dockercore/golang-cross:${GO_VERSION}
 ENV     DISABLE_WARN_OUTSIDE_CONTAINER=1

dockerfiles/Dockerfile.dev

@@ -1,4 +1,4 @@
-ARG GO_VERSION=1.13.10
+ARG GO_VERSION=1.13.11
 
 FROM    golang:${GO_VERSION}-alpine
 

dockerfiles/Dockerfile.e2e

@@ -1,4 +1,4 @@
-ARG GO_VERSION=1.13.10
+ARG GO_VERSION=1.13.11
 
 # Use Debian based image as docker-compose requires glibc.
 FROM golang:${GO_VERSION}-buster

dockerfiles/Dockerfile.lint

@@ -1,6 +1,6 @@
 # syntax=docker/dockerfile:1.1.3-experimental
 
-ARG GO_VERSION=1.13.10
+ARG GO_VERSION=1.13.11
 ARG GOLANGCI_LINTER_SHA="v1.21.0"
 
 FROM    golang:${GO_VERSION}-alpine AS build