Merge pull request #7134 from yifan-gu/version

kubelet: Refactor GetDockerVersion().

pkg/kubelet/container/runtime.go

@@ -25,11 +25,20 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/volume"
 )
 
+type Version interface {
+	// Compare compares two versions of the runtime. On success it returns -1
+	// if the version is less than the other, 1 if it is greater than the other,
+	// or 0 if they are equal.
+	Compare(other string) (int, error)
+	// String returns a string that represents the version.
+	String() string
+}
+
 // Runtime interface defines the interfaces that should be implemented
 // by a container runtime.
 type Runtime interface {
-	// Version returns a map of version information of the container runtime.
+	// Version returns the version information of the container runtime.
-	Version() (map[string]string, error)
+	Version() (Version, error)
 	// GetPods returns a list containers group by pods. The boolean parameter
 	// specifies whether the runtime returns all containers including those already
 	// exited and dead containers (used for garbage collection).

pkg/kubelet/dockertools/docker.go

@@ -120,7 +120,8 @@ type dockerContainerCommandRunner struct {
 var dockerAPIVersionWithExec, _ = docker.NewAPIVersion("1.15")
 
 // Returns the major and minor version numbers of docker server.
-func (d *dockerContainerCommandRunner) GetDockerServerVersion() (docker.APIVersion, error) {
+// TODO(yifan): Remove this once the ContainerCommandRunner is implemented by dockerManager.
+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)
@@ -136,7 +137,7 @@ func (d *dockerContainerCommandRunner) GetDockerServerVersion() (docker.APIVersi
 }
 
 func (d *dockerContainerCommandRunner) nativeExecSupportExists() (bool, error) {
-	version, err := d.GetDockerServerVersion()
+	version, err := d.getDockerServerVersion()
 	if err != nil {
 		return false, err
 	}
@@ -483,7 +484,6 @@ func ConnectToDockerOrDie(dockerEndpoint string) DockerInterface {
 // TODO(yifan): Move this to container.Runtime.
 type ContainerCommandRunner interface {
 	RunInContainer(containerID string, cmd []string) ([]byte, 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

@@ -129,10 +129,10 @@ func TestContainerManifestNaming(t *testing.T) {
 	}
 }
 
-func TestGetDockerServerVersion(t *testing.T) {
+func TestVersion(t *testing.T) {
 	fakeDocker := &FakeDockerClient{VersionInfo: docker.Env{"Version=1.1.3", "ApiVersion=1.15"}}
-	runner := dockerContainerCommandRunner{fakeDocker}
+	manager := &DockerManager{client: fakeDocker}
-	version, err := runner.GetDockerServerVersion()
+	version, err := manager.Version()
 	if err != nil {
 		t.Errorf("got error while getting docker server version - %s", err)
 	}

pkg/kubelet/dockertools/manager.go

@@ -655,3 +655,43 @@ func (dm *DockerManager) PodInfraContainerChanged(pod *api.Pod, podInfraContaine
 	}
 	return podInfraContainer.Hash != HashContainer(expectedPodInfraContainer), nil
 }
+
+type dockerVersion docker.APIVersion
+
+func NewVersion(input string) (dockerVersion, error) {
+	version, err := docker.NewAPIVersion(input)
+	return dockerVersion(version), err
+}
+
+func (dv dockerVersion) String() string {
+	return docker.APIVersion(dv).String()
+}
+
+func (dv dockerVersion) Compare(other string) (int, error) {
+	a := docker.APIVersion(dv)
+	b, err := docker.NewAPIVersion(other)
+	if err != nil {
+		return 0, err
+	}
+	if a.LessThan(b) {
+		return -1, nil
+	}
+	if a.GreaterThan(b) {
+		return 1, nil
+	}
+	return 0, nil
+}
+
+func (dm *DockerManager) Version() (kubecontainer.Version, error) {
+	env, err := dm.client.Version()
+	if err != nil {
+		return nil, fmt.Errorf("docker: failed to get docker version: %v", err)
+	}
+
+	apiVersion := env.Get("ApiVersion")
+	version, err := docker.NewAPIVersion(apiVersion)
+	if err != nil {
+		return nil, fmt.Errorf("docker: failed to parse docker server version %q: %v", apiVersion, err)
+	}
+	return dockerVersion(version), nil
+}

pkg/kubelet/kubelet.go

@@ -1658,13 +1658,12 @@ func (kl *Kubelet) syncLoop(updates <-chan PodUpdate, handler SyncHandler) {
 	}
 }
 
-// Returns Docker version for this Kubelet.
+// Returns the container runtime version for this Kubelet.
-func (kl *Kubelet) GetDockerVersion() (docker.APIVersion, error) {
+func (kl *Kubelet) GetContainerRuntimeVersion() (kubecontainer.Version, error) {
-	if kl.dockerClient == nil {
+	if kl.containerManager == nil {
-		return nil, fmt.Errorf("no Docker client")
+		return nil, fmt.Errorf("no container runtime")
 	}
-	dockerRunner := dockertools.NewDockerContainerCommandRunner(kl.dockerClient)
+	return kl.containerManager.Version()
-	return dockerRunner.GetDockerServerVersion()
 }
 
 func (kl *Kubelet) validatePodPhase(podStatus *api.PodStatus) error {

pkg/kubelet/kubelet_test.go

@@ -1597,10 +1597,6 @@ func (f *fakeContainerCommandRunner) RunInContainer(id string, cmd []string) ([]
 	return []byte{}, f.E
 }
 
-func (f *fakeContainerCommandRunner) GetDockerServerVersion() (docker.APIVersion, error) {
-	return nil, nil
-}
-
 func (f *fakeContainerCommandRunner) ExecInContainer(id string, cmd []string, in io.Reader, out, err io.WriteCloser, tty bool) error {
 	f.Cmd = cmd
 	f.ID = id

pkg/kubelet/server.go

@@ -41,7 +41,6 @@ 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"
@@ -101,7 +100,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() (docker.APIVersion, error)
+	GetContainerRuntimeVersion() (kubecontainer.Version, error)
 	GetCachedMachineInfo() (*cadvisorApi.MachineInfo, error)
 	GetPods() []*api.Pod
 	GetPodByName(namespace, name string) (*api.Pod, bool)
@@ -160,18 +159,18 @@ func (s *Server) error(w http.ResponseWriter, err error) {
 	http.Error(w, msg, http.StatusInternalServerError)
 }
 
-func isValidDockerVersion(ver docker.APIVersion) bool {
-	minAllowedVersion, _ := docker.NewAPIVersion("1.15")
-	return ver.GreaterThanOrEqualTo(minAllowedVersion)
-}
-
 func (s *Server) dockerHealthCheck(req *http.Request) error {
-	version, err := s.host.GetDockerVersion()
+	version, err := s.host.GetContainerRuntimeVersion()
 	if err != nil {
 		return errors.New("unknown Docker version")
 	}
-	if !isValidDockerVersion(version) {
+	// Verify the docker version.
-		return fmt.Errorf("Docker version is too old (%v)", version.String())
+	result, err := version.Compare("1.15")
+	if err != nil {
+		return err
+	}
+	if result < 0 {
+		return fmt.Errorf("Docker version is too old: %q", version.String())
 	}
 	return nil
 }

pkg/kubelet/server_test.go

@@ -32,10 +32,11 @@ import (
 	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	kubecontainer "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/container"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools"
 	"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"
 )
 
@@ -48,7 +49,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() (docker.APIVersion, error)
+	containerVersionFunc               func() (kubecontainer.Version, 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
@@ -72,8 +73,8 @@ func (fk *fakeKubelet) GetRootInfo(req *cadvisorApi.ContainerInfoRequest) (*cadv
 	return fk.rootInfoFunc(req)
 }
 
-func (fk *fakeKubelet) GetDockerVersion() (docker.APIVersion, error) {
+func (fk *fakeKubelet) GetContainerRuntimeVersion() (kubecontainer.Version, error) {
-	return fk.dockerVersionFunc()
+	return fk.containerVersionFunc()
 }
 
 func (fk *fakeKubelet) GetCachedMachineInfo() (*cadvisorApi.MachineInfo, error) {
@@ -450,8 +451,8 @@ func TestPodsInfo(t *testing.T) {
 
 func TestHealthCheck(t *testing.T) {
 	fw := newServerTest()
-	fw.fakeKubelet.dockerVersionFunc = func() (docker.APIVersion, error) {
+	fw.fakeKubelet.containerVersionFunc = func() (kubecontainer.Version, error) {
-		return docker.NewAPIVersion("1.15")
+		return dockertools.NewVersion("1.15")
 	}
 	fw.fakeKubelet.hostnameFunc = func() string {
 		return "127.0.0.1"
@@ -489,9 +490,9 @@ func TestHealthCheck(t *testing.T) {
 		t.Errorf("expected status code %d, got %d", http.StatusOK, resp.StatusCode)
 	}
 
-	//Test with old docker version
+	//Test with old container runtime version
-	fw.fakeKubelet.dockerVersionFunc = func() (docker.APIVersion, error) {
+	fw.fakeKubelet.containerVersionFunc = func() (kubecontainer.Version, error) {
-		return docker.NewAPIVersion("1.1")
+		return dockertools.NewVersion("1.1")
 	}
 
 	resp, err = http.Get(fw.testHTTPServer.URL + "/healthz")