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

Add K8s Docker runtime support deprecation release note #10371

Conversation

bmelbourne
Copy link
Contributor

@bmelbourne bmelbourne commented Dec 5, 2020

What this PR does / why we need it:
As Kubernetes will deprecate Docker runtime support from version 1.20 and will be removed in a future release (currently planned for the 1.22 release in late 2021), notify users that the default container runtime will be set to containerd from kOps 1.20.

https://github.com/kubernetes/kubernetes/tree/master/CHANGELOG/CHANGELOG-1.20.md#deprecation

Which issue(s) this PR fixes
#10356

Special notes for your reviewer:
PR #10370 raised to set default container runtime to containerd in kOps 1.20.

@k8s-ci-robot
Copy link
Contributor

Hi @bmelbourne. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 5, 2020
@k8s-ci-robot k8s-ci-robot added area/documentation size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 5, 2020
@bmelbourne bmelbourne changed the title Add K8s Docker support deprecation release note Add K8s Docker runtime support deprecation release note Dec 5, 2020
@bmelbourne
Copy link
Contributor Author

/assign @geojaz

@bmelbourne bmelbourne force-pushed the add-k8s-docker-deprecation-release-note branch from 8fa824b to 06e8b46 Compare December 6, 2020 11:09
@olemarkus
Copy link
Member

Any reason not to do this as part of #10370 ?

Also, I think we should follow the k8s deprecation here. Meaning that kOps will still use docker as default prior to k8s 1.20.

@bmelbourne
Copy link
Contributor Author

Any reason not to do this as part of #10370 ?

Also, I think we should follow the k8s deprecation here. Meaning that kOps will still use docker as default prior to k8s 1.20.

That's essentially the purpose of this deprecation release note (once merged in KOps 1.19)...we'll be notifying users that containerd will become the default container runtime from kOps 1.20 (as covered in #10370).

@olemarkus
Copy link
Member

Ah, it is for the 1.19 notes.

/cc @hakman

@bmelbourne bmelbourne force-pushed the add-k8s-docker-deprecation-release-note branch from 06e8b46 to 0d58327 Compare December 27, 2020 15:55
Copy link
Member

@geojaz geojaz left a comment

Choose a reason for hiding this comment

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

@bmelbourne Thanks! This lgtm, but I think it could use just a quick iteration.

I think we should call out that kOps won't do anything (automatically) to existing clusters running on Docker. And as long you're iterating, I would prefer the note to be something like:
As of kOps 1.20, new clusters will default to use containerd instead of Docker. Existing clusters can define their upgrade path manually. This change in default behavior is in line with the upstream deprecation of Docker in K8s 1.20.

I like this because it calls out the action item clearly at the beginning and leaves the explanation and further details for those who are interested. Also, this way, we're not trying to predict the future and possibly having to fix later :)

@bmelbourne bmelbourne force-pushed the add-k8s-docker-deprecation-release-note branch from 0d58327 to cbe15dd Compare January 1, 2021 13:10
@k8s-ci-robot
Copy link
Contributor

@bmelbourne: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

@moshevayner
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 1, 2021
@moshevayner
Copy link
Member

/retest

@moshevayner
Copy link
Member

Hey @bmelbourne
It looks like the CI/verify test is still failing, complains that it needs you to run make apimachinery.
My guess is that since your branch is 27 days old, it might've been forked off one with failing tests? 🤷🏼‍♂️
Either way, I could suggest that you try and pull master into your branch and see if it helps, or alternatively run make apimachinery, even though I'm not sure why would a release notes change cause that to fail.

@rifelpet
Copy link
Member

rifelpet commented Jan 1, 2021

The failing test is due to https://github.com/kubernetes/kops/issues I'd like to address that once and for all rather than punting this to next year. I'm hoping we can get that fixed soon so I wouldn't worry about the failing test for now

@geojaz
Copy link
Member

geojaz commented Jan 3, 2021

/label retest-not-required-docs-only

@k8s-ci-robot
Copy link
Contributor

@geojaz: The label(s) /label retest-not-required-docs-only cannot be applied. These labels are supported: api-review, community/discussion, community/maintenance, community/question, cuj/build-train-deploy, cuj/multi-user, platform/aws, platform/azure, platform/gcp, platform/minikube, platform/other, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash

In response to this:

/label retest-not-required-docs-only

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.

@geojaz
Copy link
Member

geojaz commented Jan 4, 2021

/retest

@geojaz
Copy link
Member

geojaz commented Jan 4, 2021

@bmelbourne I think the other stuff is ready now so we can do one last rebase and make apimachinery :)

@bmelbourne bmelbourne force-pushed the add-k8s-docker-deprecation-release-note branch from cbe15dd to 5102d13 Compare January 4, 2021 20:41
@bmelbourne bmelbourne force-pushed the add-k8s-docker-deprecation-release-note branch from 5102d13 to 3426a1a Compare January 4, 2021 20:49
@olemarkus
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 5, 2021
@hakman
Copy link
Member

hakman commented Jan 5, 2021

/hold pending decision in today's Office Hours call

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 5, 2021
docs/releases/1.19-NOTES.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 6, 2021
@hakman
Copy link
Member

hakman commented Jan 6, 2021

Considering that most periodic tests are passing using containerd, we decided in yesterday's Office Hours to go ahead with the deprecation as described here. Thanks @bmelbourne, I will look at the other PR that changes the default later this week.
/hold cancel
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 6, 2021
@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 6, 2021
Copy link
Member

@geojaz geojaz left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bmelbourne, geojaz, hakman

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 76bd027 into kubernetes:master Jan 6, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.20 milestone Jan 6, 2021
@bmelbourne bmelbourne deleted the add-k8s-docker-deprecation-release-note branch January 6, 2021 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/documentation cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. retest-not-required-docs-only size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants