Skip to content

Upgrade kubeadm's version of docker support #68495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
k8s-ci-robot merged 1 commit into from
Sep 18, 2018
Merged

Upgrade kubeadm's version of docker support #68495

k8s-ci-robot merged 1 commit into from
Sep 18, 2018
+10 −5

Conversation

@yuansisi
Copy link

@yuansisi yuansisi commented Sep 11, 2018 •
edited
Loading

What this PR does / why we need it:
I suggest that the docker version can be updated to 17.09
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

Special notes for your reviewer:

Release note:

kubeadm: update the maximum validated docker version to 18.06

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 11, 2018
@k8s-ci-robot k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. area/kubeadm sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 11, 2018
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Sep 11, 2018

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
  • If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]
Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Sep 11, 2018
@xichengliudui
Copy link
Contributor

xichengliudui commented Sep 11, 2018

/assign @fabriziopandini

@rosti
Copy link
Contributor

rosti commented Sep 11, 2018

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 11, 2018
@rosti
Copy link
Contributor

rosti commented Sep 11, 2018

We'll have to check in the issue tracker, but sticking to 17.03 was because of problems, that users were having with newer versions than this.

@fabriziopandini
Copy link
Member

fabriziopandini commented Sep 11, 2018

I'm not aware of what is the docker version supported/validated by sig-node and by kubernetes test infrastructure; I'm also a little bit concerned about changing this at this time of the cycle...
/cc @timothysc @neolit123

BTW, if we go for this change, this deserves a release note and we should plan for changing documentation as well.

@neolit123
Copy link
Member

neolit123 commented Sep 11, 2018

AFAIK SIG node have not verified 17.09 and they are not going to.
this leaves us in a spot where 17.03 is a default safe bet for us, but it presents a distro problem because 17.03 is old on some distros.

my vote would be to still recommend 17.03, but deferring to @timothysc for the final call.

@neolit123
Copy link
Member

neolit123 commented Sep 11, 2018

BTW, if we go for this change, this deserves a release note and we should plan for changing documentation as well.

💯

@rosti
Copy link
Contributor

rosti commented Sep 11, 2018

Unless SIG-node give us the go signal, we probably shouldn't do anything. The missing warning can cause trouble for users and difficult to trace problems...

On the other hand, we can investigate what SIG-node folks are doing in this case:

  • Do they simply use newer Docker versions without formerly verifying them?
  • Do they use other CRIs (containerd, CRI-O, ...)?
  • Do they stick to older Docker and Linux distro versions to be able to run it?

@yuansisi yuansisi closed this Sep 12, 2018
@rosti
Copy link
Contributor

rosti commented Sep 12, 2018

@yuansisi I hope, that we can reopen this, once SIG-node gives us the go call.

@yuansisi yuansisi reopened this Sep 12, 2018
timothysc
timothysc reviewed Sep 13, 2018
View reviewed changes
cmd/kubeadm/app/util/system/docker_validator.go Outdated
const (
dockerConfigPrefix = "DOCKER_"
maxDockerValidatedVersion = "17.03"
maxDockerValidatedVersion = "17.09"
Copy link
Contributor

@timothysc timothysc Sep 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be 18.06
/cc @kubernetes/sig-cluster-lifecycle-pr-reviews

neolit123
neolit123 reviewed Sep 13, 2018
View reviewed changes
Copy link
Member

@neolit123 neolit123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuansisi
please update the version to 18.06.

@k8s-ci-robot k8s-ci-robot added sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 18, 2018
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. area/kubectl sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 18, 2018
@yuansisi
Copy link
Author

yuansisi commented Sep 18, 2018

/assign @deads2k

@yuansisi yuansisi force-pushed the fix-20180910 branch 3 times, most recently from 821092c to 97c9fa3 Compare September 18, 2018 07:38
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 18, 2018
@neolit123
Copy link
Member

neolit123 commented Sep 18, 2018

/lgtm
we can add the missing tests later.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 18, 2018
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Sep 18, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: neolit123, timothysc, yuansisi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 9d8c9cc into kubernetes:master Sep 18, 2018
@gabrielfsousa
Copy link

gabrielfsousa commented Sep 27, 2018

docker 18.06 will be the recommend for kubernetes 1.12 ?

@neolit123
Copy link
Member

neolit123 commented Sep 27, 2018

@gabrielfsousa yes.

@gabrielfsousa
Copy link

gabrielfsousa commented Sep 27, 2018

cool !!!!

@gabrielfsousa
Copy link

gabrielfsousa commented Sep 28, 2018

the changelog still is the old one
"The validated docker versions are the same as for v1.10: 1.11.2 to 1.13.1 and 17.03.x"

@dims
Copy link
Member

dims commented Sep 28, 2018

same as etcd, same issue #69216

@micahhausler micahhausler mentioned this pull request Oct 15, 2018
Closed
@MarkusTeufelberger MarkusTeufelberger mentioned this pull request Oct 20, 2018
Closed
@warmchang warmchang mentioned this pull request Oct 30, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@detiber detiber Awaiting requested review from detiber

3 more reviewers

@timothysc timothysc timothysc approved these changes

@neolit123 neolit123 neolit123 left review comments

@dixudx dixudx dixudx left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@neolit123 neolit123

@fabriziopandini fabriziopandini

@deads2k deads2k

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubeadm area/kubectl cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

v1.12

Development

Successfully merging this pull request may close these issues.