Kublet.log reports error with latest Docker version

[mikebrow]

To reproduce error:
Install latest Docker release..

first terminal:

sudo PATH=$PATH KUBERNETES_PROVIDER=local EXPERIMENTAL_CRI=true CONTAINER_RUNTIME=docker hack/local-up-cluster.sh

second terminal:
cat /tmp/kubelet.log|grep illegal

E0303 10:36:09.811626 1592 container_manager_linux.go:90] Unable to parse docker version "17.03.0-ce": illegal zero-prefixed version component "03" in "17.03.0-ce"

note docker version
Client:
Version: 17.03.0-ce
API version: 1.26
Go version: go1.7.5
Git commit: 60ccb22
Built: Thu Feb 23 11:02:43 2017
OS/Arch: linux/amd64

Server:
Version: 17.03.0-ce
API version: 1.26 (minimum version 1.12)
Go version: go1.7.5
Git commit: 60ccb22
Built: Thu Feb 23 11:02:43 2017
OS/Arch: linux/amd64
Experimental: false

Kubernetes version:
Client Version: version.Info{Major:"1", Minor:"7+", GitVersion:"v1.7.0-alpha.0.746+1d97472361e611-dirty", GitCommit:"1d97472361e61106a1da1ce33da97f243e4d0441", GitTreeState:"dirty", BuildDate:"2017-03-02T22:50:43Z", GoVersion:"go1.8", Compiler:"gc", Platform:"linux/amd64"}

Environment:

• local, vm developer environment:
• "Ubuntu" "16.04.2 LTS (Xenial Xerus)"
• Linux mike-VirtualBox 4.4.0-65-generic Remove dot import syntax from registry package. #86-Ubuntu SMP Thu Feb 23 17:49:58 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

So the code is correctly reporting a semantic versioning error based on the rule that none of the segments should have a leading zero. But we don't own Docker's versioning. So... should we call ParseGeneric vs ParseSemantic (in pkg/util/version/version.go) to disable semver testing, or do we PR docker and limit the number of versions we're compatible with based on this test, or do we change this to a warning or info level information in the parse routine? Seems like overkill to error on a leading zero.

[yujuhong]

kubelet doesn't know how to handle docker's new version scheme yet. Some of the code does rely on semver versions, but they should only check the docker API version, which still follows the original scheme.

[dims]

Details on the docker versioning are here - moby/moby#31075 for the record

[yongtang]

I can help with the version fix if no one is working on this issue yet. As docker version (not API version) is used in many places, the complete fix may span to https://github.com/google/cadvisor I think.

[yujuhong]

@yongtang that'd be great!

Ignore the code in https://github.com/kubernetes/kubernetes/tree/v1.7.0-alpha.0/pkg/kubelet/dockertools, as that package is going away soon.

[derekwaynecarr]

we are getting questions about this as well. @yujuhong - if this is something you have cycles to update its appreciated.

[mkumatag]

I think we should go for modifying ParseSemantic to ParseGeneric to get away from semver check because moby/moby#31075 (comment) says docker never supported semver.

[mikebrow]

I fail to see the benefit of failing due to a leading zero rule. Seems like overkill. Suggest making the leading zero rule a warning/informational msg and return nil instead of error on the semver check. That or add a parameter to pass in a laxLeadingZero option to ParseSemantic() .. yada yada

[FrenchBen]

@mikebrow @yujuhong @yongtang Would love to land a helping hand in the patch ❤️ ; compatibility is always a concern for Docker, and we never want to see a release be a blocker for upgrades, although tracking down/keeping up how the API is being used in the wild is always challenging.

I agree that checking the API version may be a better way of doing this, instead of depending on the previous semver check.

Feel free to ping me in future issues when something like this happens.

* disclosure I work at Docker

[mkumatag]

I spent some more time and here are my observation, we are logic of comparing current docker version with containerdVersion=1.11.0. After the new version change actually it does not make sense to compare with the older format of docker version, so I agree with @FrenchBen that we need to compare api version rather than versions.

Reference :
https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/cm/container_manager_linux.go#L70
https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/cm/container_manager_linux.go#L651

[yujuhong]

Feel free to send a PR if you would like to help.

[mkumatag]

@yujuhong I've submitted code in cadvisor to expose docker API version, can you PTAL at google/cadvisor#1623 and give LGTM ?

[supereagle]

I think we should go for modifying ParseSemantic to ParseGeneric to get away from semver check

@mkumatag I have submitted the PR #44199 for docker version check in kubelet, but not for docker api version.

[yujuhong]

Thanks everyone for the help!