Merge pull request #28240 from Random-Liu/add-semver-back

Automatic merge from submit-queue

Kubelet: Add the docker semver back.

Fixes #28221.
This PR:
1) Add the semver back https://github.com/kubernetes/kubernetes/pull/20020
2) Remove the code in https://github.com/kubernetes/kubernetes/pull/20827, because docker 1.8 is not officially supported now, and we want to deprecate it. https://github.com/kubernetes/kubernetes/pull/27208
3) Add a test for docker version comparison.

XRef https://github.com/kubernetes/kubernetes/pull/28223

@yujuhong 
/cc @ingvagabund 

[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/.github/PULL_REQUEST_TEMPLATE.md?pixel)]()

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
+// dockerVersion implements kubecontainer.Version interface by implementing
-// Compare() and String(). It could contain either server version or api version.
+// Compare() and String() (which is implemented by the underlying semver.Version)
-type dockerVersion string
+// 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) {
+func (v apiVersion) Compare(other string) (int, error) {
-	if dockerversion.LessThan(string(v), other) {
+	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)
 	}
-
+	version, err := newDockerVersion(v.Version)
-	return dockerVersion(v.Version), nil
+	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.

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"