Merge pull request #7054 from ncdc/dockerclient-version

pkg/kubelet/dockertools/docker.go

@@ -117,29 +117,22 @@ type dockerContainerCommandRunner struct {
 }
 
 // The first version of docker that supports exec natively is 1.3.0 == API 1.15
-var dockerAPIVersionWithExec = []uint{1, 15}
+var dockerAPIVersionWithExec, _ = docker.NewAPIVersion("1.15")
 
 // Returns the major and minor version numbers of docker server.
-func (d *dockerContainerCommandRunner) GetDockerServerVersion() ([]uint, error) {
+func (d *dockerContainerCommandRunner) GetDockerServerVersion() (docker.APIVersion, error) {
 	env, err := d.client.Version()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get docker server version - %v", err)
 	}
-	version := []uint{}
-	for _, entry := range *env {
-		if strings.Contains(strings.ToLower(entry), "apiversion") || strings.Contains(strings.ToLower(entry), "api version") {
-			elems := strings.Split(strings.Split(entry, "=")[1], ".")
-			for _, elem := range elems {
-				val, err := strconv.ParseUint(elem, 10, 32)
-				if err != nil {
-					return nil, fmt.Errorf("failed to parse docker server version %q: %v", entry, err)
-				}
-				version = append(version, uint(val))
-			}
-			return version, nil
-		}
+
+	apiVersion := env.Get("ApiVersion")
+	version, err := docker.NewAPIVersion(apiVersion)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse docker server version %q: %v", apiVersion, err)
 	}
-	return nil, fmt.Errorf("docker server version missing from server version output - %+v", env)
+
+	return version, nil
 }
 
 func (d *dockerContainerCommandRunner) nativeExecSupportExists() (bool, error) {
@@ -147,15 +140,7 @@ func (d *dockerContainerCommandRunner) nativeExecSupportExists() (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	if len(dockerAPIVersionWithExec) != len(version) {
-		return false, fmt.Errorf("unexpected docker version format. Expecting %v format, got %v", dockerAPIVersionWithExec, version)
-	}
-	for idx, val := range dockerAPIVersionWithExec {
-		if version[idx] < val {
-			return false, nil
-		}
-	}
-	return true, nil
+	return version.GreaterThanOrEqualTo(dockerAPIVersionWithExec), nil
 }
 
 func (d *dockerContainerCommandRunner) getRunInContainerCommand(containerID string, cmd []string) (*exec.Cmd, error) {
@@ -494,7 +479,7 @@ func ConnectToDockerOrDie(dockerEndpoint string) DockerInterface {
 // TODO(yifan): Move this to container.Runtime.
 type ContainerCommandRunner interface {
 	RunInContainer(containerID string, cmd []string) ([]byte, error)
-	GetDockerServerVersion() ([]uint, error)
+	GetDockerServerVersion() (docker.APIVersion, error)
 	ExecInContainer(containerID string, cmd []string, in io.Reader, out, err io.WriteCloser, tty bool) error
 	PortForward(pod *kubecontainer.Pod, port uint16, stream io.ReadWriteCloser) error
 }

pkg/kubelet/dockertools/docker_test.go

@@ -130,26 +130,20 @@ func TestContainerManifestNaming(t *testing.T) {
 }
 
 func TestGetDockerServerVersion(t *testing.T) {
-	fakeDocker := &FakeDockerClient{VersionInfo: docker.Env{"Client version=1.2", "Server version=1.1.3", "Server API version=1.15"}}
+	fakeDocker := &FakeDockerClient{VersionInfo: docker.Env{"Version=1.1.3", "ApiVersion=1.15"}}
 	runner := dockerContainerCommandRunner{fakeDocker}
 	version, err := runner.GetDockerServerVersion()
 	if err != nil {
 		t.Errorf("got error while getting docker server version - %s", err)
 	}
-	expectedVersion := []uint{1, 15}
-	if len(expectedVersion) != len(version) {
-		t.Errorf("invalid docker server version. expected: %v, got: %v", expectedVersion, version)
-	} else {
-		for idx, val := range expectedVersion {
-			if version[idx] != val {
-				t.Errorf("invalid docker server version. expected: %v, got: %v", expectedVersion, version)
-			}
-		}
+	expectedVersion, _ := docker.NewAPIVersion("1.15")
+	if e, a := expectedVersion.String(), version.String(); e != a {
+		t.Errorf("invalid docker server version. expected: %v, got: %v", e, a)
 	}
 }
 
 func TestExecSupportExists(t *testing.T) {
-	fakeDocker := &FakeDockerClient{VersionInfo: docker.Env{"Client version=1.2", "Server version=1.3.0", "Server API version=1.15"}}
+	fakeDocker := &FakeDockerClient{VersionInfo: docker.Env{"Version=1.3.0", "ApiVersion=1.15"}}
 	runner := dockerContainerCommandRunner{fakeDocker}
 	useNativeExec, err := runner.nativeExecSupportExists()
 	if err != nil {
@@ -161,7 +155,7 @@ func TestExecSupportExists(t *testing.T) {
 }
 
 func TestExecSupportNotExists(t *testing.T) {
-	fakeDocker := &FakeDockerClient{VersionInfo: docker.Env{"Client version=1.2", "Server version=1.1.2", "Server API version=1.14"}}
+	fakeDocker := &FakeDockerClient{VersionInfo: docker.Env{"Version=1.1.2", "ApiVersion=1.14"}}
 	runner := dockerContainerCommandRunner{fakeDocker}
 	useNativeExec, _ := runner.nativeExecSupportExists()
 	if useNativeExec {

pkg/kubelet/kubelet.go

@@ -1636,7 +1636,7 @@ func (kl *Kubelet) syncLoop(updates <-chan PodUpdate, handler SyncHandler) {
 }
 
 // Returns Docker version for this Kubelet.
-func (kl *Kubelet) GetDockerVersion() ([]uint, error) {
+func (kl *Kubelet) GetDockerVersion() (docker.APIVersion, error) {
 	if kl.dockerClient == nil {
 		return nil, fmt.Errorf("no Docker client")
 	}

pkg/kubelet/kubelet_test.go

@@ -1595,7 +1595,7 @@ func (f *fakeContainerCommandRunner) RunInContainer(id string, cmd []string) ([]
 	return []byte{}, f.E
 }
 
-func (f *fakeContainerCommandRunner) GetDockerServerVersion() ([]uint, error) {
+func (f *fakeContainerCommandRunner) GetDockerServerVersion() (docker.APIVersion, error) {
 	return nil, nil
 }
 

pkg/kubelet/server.go

@@ -41,6 +41,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util/flushwriter"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util/httpstream"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util/httpstream/spdy"
+	"github.com/fsouza/go-dockerclient"
 	"github.com/golang/glog"
 	cadvisorApi "github.com/google/cadvisor/info/v1"
 	"github.com/prometheus/client_golang/prometheus"
@@ -100,7 +101,7 @@ func ListenAndServeKubeletReadOnlyServer(host HostInterface, address net.IP, por
 type HostInterface interface {
 	GetContainerInfo(podFullName string, uid types.UID, containerName string, req *cadvisorApi.ContainerInfoRequest) (*cadvisorApi.ContainerInfo, error)
 	GetRootInfo(req *cadvisorApi.ContainerInfoRequest) (*cadvisorApi.ContainerInfo, error)
-	GetDockerVersion() ([]uint, error)
+	GetDockerVersion() (docker.APIVersion, error)
 	GetCachedMachineInfo() (*cadvisorApi.MachineInfo, error)
 	GetPods() []*api.Pod
 	GetPodByName(namespace, name string) (*api.Pod, bool)
@@ -159,31 +160,18 @@ func (s *Server) error(w http.ResponseWriter, err error) {
 	http.Error(w, msg, http.StatusInternalServerError)
 }
 
-func isValidDockerVersion(ver []uint) (bool, string) {
-	minAllowedVersion := []uint{1, 15}
-	for i := 0; i < len(ver) && i < len(minAllowedVersion); i++ {
-		if ver[i] != minAllowedVersion[i] {
-			if ver[i] < minAllowedVersion[i] {
-				versions := make([]string, len(ver))
-				for i, v := range ver {
-					versions[i] = fmt.Sprint(v)
-				}
-				return false, strings.Join(versions, ".")
-			}
-			return true, ""
-		}
-	}
-	return true, ""
+func isValidDockerVersion(ver docker.APIVersion) bool {
+	minAllowedVersion, _ := docker.NewAPIVersion("1.15")
+	return ver.GreaterThanOrEqualTo(minAllowedVersion)
 }
 
 func (s *Server) dockerHealthCheck(req *http.Request) error {
-	versions, err := s.host.GetDockerVersion()
+	version, err := s.host.GetDockerVersion()
 	if err != nil {
 		return errors.New("unknown Docker version")
 	}
-	valid, version := isValidDockerVersion(versions)
-	if !valid {
-		return fmt.Errorf("Docker version is too old (%v)", version)
+	if !isValidDockerVersion(version) {
+		return fmt.Errorf("Docker version is too old (%v)", version.String())
 	}
 	return nil
 }

pkg/kubelet/server_test.go

@@ -35,6 +35,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util/httpstream"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util/httpstream/spdy"
+	"github.com/fsouza/go-dockerclient"
 	cadvisorApi "github.com/google/cadvisor/info/v1"
 )
 
@@ -47,7 +48,7 @@ type fakeKubelet struct {
 	podsFunc                           func() []*api.Pod
 	logFunc                            func(w http.ResponseWriter, req *http.Request)
 	runFunc                            func(podFullName string, uid types.UID, containerName string, cmd []string) ([]byte, error)
-	dockerVersionFunc                  func() ([]uint, error)
+	dockerVersionFunc                  func() (docker.APIVersion, error)
 	execFunc                           func(pod string, uid types.UID, container string, cmd []string, in io.Reader, out, err io.WriteCloser, tty bool) error
 	portForwardFunc                    func(name string, uid types.UID, port uint16, stream io.ReadWriteCloser) error
 	containerLogsFunc                  func(podFullName, containerName, tail string, follow bool, stdout, stderr io.Writer) error
@@ -71,7 +72,7 @@ func (fk *fakeKubelet) GetRootInfo(req *cadvisorApi.ContainerInfoRequest) (*cadv
 	return fk.rootInfoFunc(req)
 }
 
-func (fk *fakeKubelet) GetDockerVersion() ([]uint, error) {
+func (fk *fakeKubelet) GetDockerVersion() (docker.APIVersion, error) {
 	return fk.dockerVersionFunc()
 }
 
@@ -449,8 +450,8 @@ func TestPodsInfo(t *testing.T) {
 
 func TestHealthCheck(t *testing.T) {
 	fw := newServerTest()
-	fw.fakeKubelet.dockerVersionFunc = func() ([]uint, error) {
-		return []uint{1, 15}, nil
+	fw.fakeKubelet.dockerVersionFunc = func() (docker.APIVersion, error) {
+		return docker.NewAPIVersion("1.15")
 	}
 	fw.fakeKubelet.hostnameFunc = func() string {
 		return "127.0.0.1"
@@ -489,8 +490,8 @@ func TestHealthCheck(t *testing.T) {
 	}
 
 	//Test with old docker version
-	fw.fakeKubelet.dockerVersionFunc = func() ([]uint, error) {
-		return []uint{1, 1}, nil
+	fw.fakeKubelet.dockerVersionFunc = func() (docker.APIVersion, error) {
+		return docker.NewAPIVersion("1.1")
 	}
 
 	resp, err = http.Get(fw.testHTTPServer.URL + "/healthz")