From 611fb25926153e6ac9b9992eee2ffb8ec96b369a Mon Sep 17 00:00:00 2001 From: Yifan Gu Date: Tue, 21 Apr 2015 13:02:50 -0700 Subject: [PATCH] kubelet: Refactor GetDockerVersion(). Remove GetDockerServerVersion() from DockerContainerCommandRunner interface, replaced with runtime.Version(). Also added Version type in runtime for version comparision. --- pkg/kubelet/container/runtime.go | 13 +++++++-- pkg/kubelet/dockertools/docker.go | 6 ++-- pkg/kubelet/dockertools/docker_test.go | 6 ++-- pkg/kubelet/dockertools/manager.go | 40 ++++++++++++++++++++++++++ pkg/kubelet/kubelet.go | 11 ++++--- pkg/kubelet/kubelet_test.go | 4 --- pkg/kubelet/server.go | 19 ++++++------ pkg/kubelet/server_test.go | 19 ++++++------ 8 files changed, 81 insertions(+), 37 deletions(-) diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index 65c60f95844..c9d736d837c 100644 --- a/pkg/kubelet/container/runtime.go +++ b/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() (map[string]string, error) + // Version returns the version information of the container runtime. + 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). diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index 2f0a7d1c655..1156dbd1426 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/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 } @@ -479,7 +480,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 } diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index 2b57a5820a5..514c2ceafa1 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/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} - version, err := runner.GetDockerServerVersion() + manager := &DockerManager{client: fakeDocker} + version, err := manager.Version() if err != nil { t.Errorf("got error while getting docker server version - %s", err) } diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 4e8b15e1a28..32d41aca438 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/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 +} diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index c58189628ab..86c719d7839 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1658,13 +1658,12 @@ func (kl *Kubelet) syncLoop(updates <-chan PodUpdate, handler SyncHandler) { } } -// Returns Docker version for this Kubelet. -func (kl *Kubelet) GetDockerVersion() (docker.APIVersion, error) { - if kl.dockerClient == nil { - return nil, fmt.Errorf("no Docker client") +// Returns the container runtime version for this Kubelet. +func (kl *Kubelet) GetContainerRuntimeVersion() (kubecontainer.Version, error) { + if kl.containerManager == nil { + return nil, fmt.Errorf("no container runtime") } - dockerRunner := dockertools.NewDockerContainerCommandRunner(kl.dockerClient) - return dockerRunner.GetDockerServerVersion() + return kl.containerManager.Version() } func (kl *Kubelet) validatePodPhase(podStatus *api.PodStatus) error { diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 5fc8e740410..409aebd7480 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/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 diff --git a/pkg/kubelet/server.go b/pkg/kubelet/server.go index 4b98f0232cb..e261e0ff8c3 100644 --- a/pkg/kubelet/server.go +++ b/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) { - return fmt.Errorf("Docker version is too old (%v)", version.String()) + // Verify the docker version. + 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 } diff --git a/pkg/kubelet/server_test.go b/pkg/kubelet/server_test.go index 712ffdd9b2b..778cf720346 100644 --- a/pkg/kubelet/server_test.go +++ b/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) { - return fk.dockerVersionFunc() +func (fk *fakeKubelet) GetContainerRuntimeVersion() (kubecontainer.Version, error) { + 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) { - return docker.NewAPIVersion("1.15") + fw.fakeKubelet.containerVersionFunc = func() (kubecontainer.Version, error) { + 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 - fw.fakeKubelet.dockerVersionFunc = func() (docker.APIVersion, error) { - return docker.NewAPIVersion("1.1") + //Test with old container runtime version + fw.fakeKubelet.containerVersionFunc = func() (kubecontainer.Version, error) { + return dockertools.NewVersion("1.1") } resp, err = http.Get(fw.testHTTPServer.URL + "/healthz")