Skip to content
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.

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

Bump Docker supported version to 18.09 #72823

Merged

Conversation

@dims
Copy link
Member

commented Jan 11, 2019

For 1.14, let's support Docker 18.09 for kubeadm

Change-Id: Ib8d4d9dd3cb51cf4780623389a4bcb101d3c8fa7

What type of PR is this?
/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Allow users to use Docker 18.09 with kubeadm
@dims

This comment has been minimized.

Copy link
Member Author

commented Jan 11, 2019

/sig cluster-lifecycle

@dims

This comment has been minimized.

Copy link
Member Author

commented Jan 11, 2019

/priority important-soon

@neolit123
Copy link
Member

left a comment

Bump Docker supported version to 18.09
For 1.14, let's switch to latest Docker 18.09

Change-Id: Ib8d4d9dd3cb51cf4780623389a4bcb101d3c8fa7

@dims dims force-pushed the dims:bump-version-of-docker-to-18.09 branch from 9db4f4d to 62fc6df Jan 14, 2019

@dims

This comment has been minimized.

Copy link
Member Author

commented Jan 14, 2019

/test pull-kubernetes-e2e-kops-aws

@dims

This comment has been minimized.

Copy link
Member Author

commented Jan 15, 2019

/assign @luxas @ixdy

@neolit123

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

/hold
cc @rosti

since Docker 18.09, containerd gets bundled with it and this causes my old CRI autodetect code to error out with "too many CRIs detected"...

looks like we shouldn't enable this just yet.

@rosti

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

/hold
cc @rosti

since Docker 18.09, containerd gets bundled with it and this causes my old CRI autodetect code to error out with "too many CRIs detected"...

looks like we shouldn't enable this just yet.

The code is not in yet. It's in #69366 and has been sitting in there since the beginning of October.

@neolit123

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

The code is not in yet. It's in #69366 and has been sitting in there since the beginning of October.

ah, ok.
so should we bump Docker in this current PR regardless?

@dims

This comment has been minimized.

Copy link
Member Author

commented Jan 15, 2019

@neolit123 @rosti #69366 is marked WIP and needs rebase etc.

Does kubeadm fail with only docker 18.09 is present? (and the intention of the deployer is to use it with docker), if the answer is no, then we should just merge this PR so we can start throwing CI resources at this docker version to see what fails. Don't want to delay that process of validating newer versions to late in the cycle.

@neolit123

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

/hold cancel

Does kubeadm fail with only docker 18.09 is present?

the problem is that the related PR attempts to add something that we want (eventually), yet for some reason Docker 18.09 started bundling containerd, so it will need an alternative detection method.

this decision to bundle containerd is quite odd.

@rosti

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

+1 for the cancel on hold. The above PR is already updated to take Docker 18.09 into account and it should work.

@luxas
luxas approved these changes Jan 15, 2019
Copy link
Member

left a comment

/lgtm
/approve

I'm kind of hesitant towards just bumping a number and not really knowing what this means wrt e.g. performance tests, etc. but I guess we kind of have no other choice at the moment 😕

@k8s-ci-robot k8s-ci-robot added the lgtm label Jan 15, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jan 15, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, luxas, neolit123

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

The pull request process is described here

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 9c48ca4 into kubernetes:master Jan 15, 2019

18 checks passed

cla/linuxfoundation dims authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-godeps Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.