From 7f6d9b3bc68d7ab22aa609b52828698c3301cd5e Mon Sep 17 00:00:00 2001 From: Random Liu Date: Wed, 29 Jun 2016 20:44:16 +0000 Subject: [PATCH] Add the semver back. --- pkg/kubelet/dockertools/docker_manager.go | 60 +++++++++++++++---- .../dockertools/docker_manager_test.go | 52 ++++++++++++++++ 2 files changed, 101 insertions(+), 11 deletions(-) diff --git a/pkg/kubelet/dockertools/docker_manager.go b/pkg/kubelet/dockertools/docker_manager.go index 28ba651e65..1a8a0c9957 100644 --- a/pkg/kubelet/dockertools/docker_manager.go +++ b/pkg/kubelet/dockertools/docker_manager.go @@ -32,10 +32,11 @@ import ( "sync" "time" + "github.com/coreos/go-semver/semver" dockertypes "github.com/docker/engine-api/types" dockercontainer "github.com/docker/engine-api/types/container" dockerstrslice "github.com/docker/engine-api/types/strslice" - dockerversion "github.com/docker/engine-api/types/versions" + dockerapiversion "github.com/docker/engine-api/types/versions" dockernat "github.com/docker/go-connections/nat" "github.com/golang/glog" cadvisorapi "github.com/google/cadvisor/info/v1" @@ -914,18 +915,52 @@ func getDockerNetworkMode(container *dockertypes.ContainerJSON) string { return "" } -// dockerVersion implementes kubecontainer.Version interface by implementing -// Compare() and String(). It could contain either server version or api version. -type dockerVersion string +// dockerVersion implements kubecontainer.Version interface by implementing +// Compare() and String() (which is implemented by the underlying semver.Version) +// TODO: this code is the same as rktVersion and may make sense to be moved to +// somewhere shared. +type dockerVersion struct { + *semver.Version +} -func (v dockerVersion) String() string { +// newDockerVersion returns a semantically versioned docker version value +func newDockerVersion(version string) (dockerVersion, error) { + sem, err := semver.NewVersion(version) + return dockerVersion{sem}, err +} + +func (r dockerVersion) String() string { + return r.Version.String() +} + +func (r dockerVersion) Compare(other string) (int, error) { + v, err := newDockerVersion(other) + if err != nil { + return -1, err + } + + if r.LessThan(*v.Version) { + return -1, nil + } + if v.Version.LessThan(*r.Version) { + return 1, nil + } + return 0, nil +} + +// apiVersion implements kubecontainer.Version interface by implementing +// Compare() and String(). It uses the compare function of engine-api to +// compare docker apiversions. +type apiVersion string + +func (v apiVersion) String() string { return string(v) } -func (v dockerVersion) Compare(other string) (int, error) { - if dockerversion.LessThan(string(v), other) { +func (v apiVersion) Compare(other string) (int, error) { + if dockerapiversion.LessThan(string(v), other) { return -1, nil - } else if dockerversion.GreaterThan(string(v), other) { + } else if dockerapiversion.GreaterThan(string(v), other) { return 1, nil } return 0, nil @@ -940,8 +975,11 @@ func (dm *DockerManager) Version() (kubecontainer.Version, error) { if err != nil { return nil, fmt.Errorf("docker: failed to get docker version: %v", err) } - - return dockerVersion(v.Version), nil + version, err := newDockerVersion(v.Version) + if err != nil { + return nil, fmt.Errorf("docker: failed to parse docker version %q: %v", v.Version, err) + } + return version, nil } func (dm *DockerManager) APIVersion() (kubecontainer.Version, error) { @@ -950,7 +988,7 @@ func (dm *DockerManager) APIVersion() (kubecontainer.Version, error) { return nil, fmt.Errorf("docker: failed to get docker version: %v", err) } - return dockerVersion(v.APIVersion), nil + return apiVersion(v.APIVersion), nil } // Status returns error if docker daemon is unhealthy, nil otherwise. diff --git a/pkg/kubelet/dockertools/docker_manager_test.go b/pkg/kubelet/dockertools/docker_manager_test.go index 97cad5e566..7274ae9797 100644 --- a/pkg/kubelet/dockertools/docker_manager_test.go +++ b/pkg/kubelet/dockertools/docker_manager_test.go @@ -2061,6 +2061,58 @@ func TestCheckVersionCompatibility(t *testing.T) { } } +func TestNewDockerVersion(t *testing.T) { + cases := []struct { + value string + out string + err bool + }{ + {value: "1", err: true}, + {value: "1.8", err: true}, + {value: "1.8.1", out: "1.8.1"}, + {value: "1.8.1-fc21.other", out: "1.8.1-fc21.other"}, + {value: "1.8.1-beta.12", out: "1.8.1-beta.12"}, + } + for _, test := range cases { + v, err := newDockerVersion(test.value) + switch { + case err != nil && test.err: + continue + case (err != nil) != test.err: + t.Errorf("error for %q: expected %t, got %v", test.value, test.err, err) + continue + } + if v.String() != test.out { + t.Errorf("unexpected parsed version %q for %q", v, test.value) + } + } +} + +func TestDockerVersionComparison(t *testing.T) { + v, err := newDockerVersion("1.10.3") + assert.NoError(t, err) + for i, test := range []struct { + version string + compare int + err bool + }{ + {version: "1.9.2", compare: 1}, + {version: "1.9.2-rc2", compare: 1}, + {version: "1.10.3", compare: 0}, + {version: "1.10.3-rc3", compare: 1}, + {version: "1.10.4", compare: -1}, + {version: "1.10.4-rc1", compare: -1}, + {version: "1.11.1", compare: -1}, + {version: "1.11.1-rc4", compare: -1}, + {version: "invalid", compare: -1, err: true}, + } { + testCase := fmt.Sprintf("test case #%d test version %q", i, test.version) + res, err := v.Compare(test.version) + assert.Equal(t, test.compare, res, testCase) + assert.Equal(t, test.err, err != nil, testCase) + } +} + func TestVersion(t *testing.T) { expectedVersion := "1.8.1" expectedAPIVersion := "1.20"