From fff8a7c371dfe2eec98f0f7c51372c4eb2c04635 Mon Sep 17 00:00:00 2001 From: Ken Robertson Date: Fri, 22 Jan 2016 12:54:23 -0800 Subject: [PATCH 1/2] kubelet: Update engine version parsing to handle semantic versioning This updates the dockertools.dockerVersion to use a semantic versioning library to more gracefully support engine versions which include additional version fields. Previously, go-dockerclient's APIVersion struct was use which only handles plain numeric x.y.z version strings. With #19675, the library was now used on the Docker engine string, however it is possible for the engine string to include include additional information for beta, rc, or distro specific builds. This PR also enables the TestDockerRuntimeVersion test which was previously just a FIXME and updates it to pass, and be used to test the version string that cause #20005. This negates the need for fsouza/go-dockerclient#451, since even with that change, if a user was running Docker 1.10.0-rc1, this would cause the kubelet to report it as simply 1.10.0. --- pkg/kubelet/dockertools/manager.go | 50 +++++++++++++++++----- pkg/kubelet/kubelet_test.go | 68 ++++++++++++++++++++++++------ 2 files changed, 94 insertions(+), 24 deletions(-) diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index f19a6566797..3d272e2cc47 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -31,6 +31,7 @@ import ( "sync" "time" + "github.com/coreos/go-semver/semver" docker "github.com/fsouza/go-dockerclient" "github.com/golang/glog" "github.com/golang/groupcache/lru" @@ -944,18 +945,47 @@ func (dm *DockerManager) podInfraContainerChanged(pod *api.Pod, podInfraContaine return podInfraContainerStatus.Hash != kubecontainer.HashContainer(expectedPodInfraContainer), nil } -type dockerVersion docker.APIVersion - -func NewVersion(input string) (dockerVersion, error) { - version, err := docker.NewAPIVersion(input) - return dockerVersion(version), err +// dockerVersion implementes 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 (dv dockerVersion) String() string { +func newDockerVersion(version string) (dockerVersion, error) { + sem, err := semver.NewVersion(version) + if err != nil { + return dockerVersion{}, err + } + return dockerVersion{sem}, nil +} + +func (r dockerVersion) Compare(other string) (int, error) { + v, err := semver.NewVersion(other) + if err != nil { + return -1, err + } + + if r.LessThan(*v) { + return -1, nil + } + if v.LessThan(*r.Version) { + return 1, nil + } + return 0, nil +} + +// dockerVersion implementes kubecontainer.Version interface by implementing +// Compare() and String() on top og go-dockerclient's APIVersion. This version +// string doesn't conform to semantic versioning, as it is only "x.y" +type dockerAPIVersion docker.APIVersion + +func (dv dockerAPIVersion) String() string { return docker.APIVersion(dv).String() } -func (dv dockerVersion) Compare(other string) (int, error) { +func (dv dockerAPIVersion) Compare(other string) (int, error) { a := docker.APIVersion(dv) b, err := docker.NewAPIVersion(other) if err != nil { @@ -981,12 +1011,12 @@ func (dm *DockerManager) Version() (kubecontainer.Version, error) { } engineVersion := env.Get("Version") - version, err := docker.NewAPIVersion(engineVersion) + version, err := newDockerVersion(engineVersion) if err != nil { glog.Errorf("docker: failed to parse docker server version %q: %v", engineVersion, err) return nil, fmt.Errorf("docker: failed to parse docker server version %q: %v", engineVersion, err) } - return dockerVersion(version), nil + return version, nil } func (dm *DockerManager) APIVersion() (kubecontainer.Version, error) { @@ -1001,7 +1031,7 @@ func (dm *DockerManager) APIVersion() (kubecontainer.Version, error) { glog.Errorf("docker: failed to parse docker api version %q: %v", apiVersion, err) return nil, fmt.Errorf("docker: failed to parse docker api version %q: %v", apiVersion, err) } - return dockerVersion(version), nil + return dockerAPIVersion(version), nil } // The first version of docker that supports exec natively is 1.3.0 == API 1.15 diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 0831c6de6fb..f34e72d765c 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -2719,26 +2719,59 @@ func TestUpdateNewNodeOutOfDiskStatusWithTransitionFrequency(t *testing.T) { } } -// FIXME: Enable me.. -func testDockerRuntimeVersion(t *testing.T) { +func TestDockerRuntimeVersion(t *testing.T) { testKubelet := newTestKubelet(t) kubelet := testKubelet.kubelet fakeRuntime := testKubelet.fakeRuntime fakeRuntime.RuntimeType = "docker" - fakeRuntime.VersionInfo = "1.5.0" - fakeRuntime.APIVersionInfo = "1.18" + fakeRuntime.VersionInfo = "1.10.0-rc1-fc24" + fakeRuntime.APIVersionInfo = "1.22" kubeClient := testKubelet.fakeKubeClient kubeClient.ReactionChain = testclient.NewSimpleFake(&api.NodeList{Items: []api.Node{ - {ObjectMeta: api.ObjectMeta{Name: testKubeletHostname}}, + { + ObjectMeta: api.ObjectMeta{Name: testKubeletHostname}, + Spec: api.NodeSpec{}, + Status: api.NodeStatus{ + Conditions: []api.NodeCondition{ + { + Type: api.NodeOutOfDisk, + Status: api.ConditionFalse, + Reason: "KubeletHasSufficientDisk", + Message: fmt.Sprintf("kubelet has sufficient disk space available"), + LastHeartbeatTime: unversioned.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + LastTransitionTime: unversioned.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + { + Type: api.NodeReady, + Status: api.ConditionTrue, + Reason: "KubeletReady", + Message: fmt.Sprintf("kubelet is posting ready status"), + LastHeartbeatTime: unversioned.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + LastTransitionTime: unversioned.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + }, + Capacity: api.ResourceList{ + api.ResourceCPU: *resource.NewMilliQuantity(3000, resource.DecimalSI), + api.ResourceMemory: *resource.NewQuantity(20E9, resource.BinarySI), + api.ResourcePods: *resource.NewQuantity(0, resource.DecimalSI), + }, + Allocatable: api.ResourceList{ + api.ResourceCPU: *resource.NewMilliQuantity(2800, resource.DecimalSI), + api.ResourceMemory: *resource.NewQuantity(19900E6, resource.BinarySI), + api.ResourcePods: *resource.NewQuantity(0, resource.DecimalSI), + }, + }, + }, }}).ReactionChain + mockCadvisor := testKubelet.fakeCadvisor + mockCadvisor.On("Start").Return(nil) machineInfo := &cadvisorapi.MachineInfo{ MachineID: "123", SystemUUID: "abc", BootID: "1b3", NumCores: 2, - MemoryCapacity: 1024, + MemoryCapacity: 20E9, } - mockCadvisor := testKubelet.fakeCadvisor mockCadvisor.On("MachineInfo").Return(machineInfo, nil) versionInfo := &cadvisorapi.VersionInfo{ KernelVersion: "3.16.0-0.bpo.4-amd64", @@ -2779,13 +2812,18 @@ func testDockerRuntimeVersion(t *testing.T) { BootID: "1b3", KernelVersion: "3.16.0-0.bpo.4-amd64", OSImage: "Debian GNU/Linux 7 (wheezy)", - ContainerRuntimeVersion: "docker://1.5.0", + ContainerRuntimeVersion: "docker://1.10.0-rc1-fc24", KubeletVersion: version.Get().String(), KubeProxyVersion: version.Get().String(), }, Capacity: api.ResourceList{ api.ResourceCPU: *resource.NewMilliQuantity(2000, resource.DecimalSI), - api.ResourceMemory: *resource.NewQuantity(1024, resource.BinarySI), + api.ResourceMemory: *resource.NewQuantity(20E9, resource.BinarySI), + api.ResourcePods: *resource.NewQuantity(0, resource.DecimalSI), + }, + Allocatable: api.ResourceList{ + api.ResourceCPU: *resource.NewMilliQuantity(1800, resource.DecimalSI), + api.ResourceMemory: *resource.NewQuantity(19900E6, resource.BinarySI), api.ResourcePods: *resource.NewQuantity(0, resource.DecimalSI), }, Addresses: []api.NodeAddress{ @@ -2805,6 +2843,7 @@ func testDockerRuntimeVersion(t *testing.T) { }, } + kubelet.runtimeState = newRuntimeState(maxWaitForContainerRuntime, false, "", kubelet.isContainerRuntimeVersionCompatible) kubelet.updateRuntimeUp() if err := kubelet.updateNodeStatus(); err != nil { t.Errorf("unexpected error: %v", err) @@ -2836,13 +2875,14 @@ func testDockerRuntimeVersion(t *testing.T) { t.Errorf("unexpected node condition order. NodeReady should be last.") } - if !reflect.DeepEqual(expectedNode, updatedNode) { - t.Errorf("unexpected objects: %s", util.ObjectDiff(expectedNode, updatedNode)) + if !api.Semantic.DeepEqual(expectedNode, updatedNode) { + t.Errorf("expected \n%v\n, got \n%v", expectedNode, updatedNode) } // Downgrade docker version, node should be NotReady fakeRuntime.RuntimeType = "docker" - fakeRuntime.VersionInfo = "1.17" + fakeRuntime.VersionInfo = "1.5.0" + fakeRuntime.APIVersionInfo = "1.17" kubelet.updateRuntimeUp() if err := kubelet.updateNodeStatus(); err != nil { t.Errorf("unexpected error: %v", err) @@ -2858,8 +2898,8 @@ func testDockerRuntimeVersion(t *testing.T) { if !ok { t.Errorf("unexpected object type") } - if updatedNode.Status.Conditions[0].Reason != "KubeletNotReady" && - !strings.Contains(updatedNode.Status.Conditions[0].Message, "container runtime version is older than") { + if updatedNode.Status.Conditions[1].Reason != "KubeletNotReady" && + !strings.Contains(updatedNode.Status.Conditions[1].Message, "container runtime version is older than") { t.Errorf("unexpect NodeStatus due to container runtime version") } } From 66c99d5e9c01c5eef9b4ce195564e1221ba75379 Mon Sep 17 00:00:00 2001 From: Ken Robertson Date: Fri, 22 Jan 2016 17:14:01 -0800 Subject: [PATCH 2/2] kubelet: Update FakeDockerClient used in integration tests This updates the mock for the docker client used in integration tests to include the engine version in its VersionInfo response. --- pkg/kubelet/dockertools/docker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index 5418233464f..8d499c4db70 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -275,7 +275,7 @@ func getDockerClient(dockerEndpoint string) (*docker.Client, error) { func ConnectToDockerOrDie(dockerEndpoint string) DockerInterface { if dockerEndpoint == "fake://" { return &FakeDockerClient{ - VersionInfo: docker.Env{"ApiVersion=1.18"}, + VersionInfo: docker.Env{"ApiVersion=1.18", "Version=1.6.0"}, } } client, err := getDockerClient(dockerEndpoint)