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.

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

Update kubeadm CRI detection docs in light of dockershim deprecation #32761

Merged
merged 1 commit into from
Apr 12, 2022
Merged

Update kubeadm CRI detection docs in light of dockershim deprecation #32761

merged 1 commit into from
Apr 12, 2022

Conversation

sftim
Copy link
Contributor

@sftim sftim commented Apr 5, 2022 •
edited
Loading

Fixes issue #26780

Preview: https://deploy-preview-32761--kubernetes-io-main-staging.netlify.app/docs/setup/production-environment/tools/kubeadm/install-kubeadm/#installing-runtime

This change is for v1.23; I expect that v1.24 changes will overwrite this.

/sig cluster-lifecycle node
/language en
/priority important-longterm

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/node Categorizes an issue or PR as relevant to SIG Node. language/en Issues or PRs related to English language cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 5, 2022
@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Apr 5, 2022
@sftim
Copy link
Contributor Author

sftim commented Apr 5, 2022

/sig cluster-lifecycle
/sig node

@netlify
Copy link

netlify bot commented Apr 5, 2022 •
edited
Loading

✅ Deploy Preview for kubernetes-io-main-staging ready!

Name Link
🔨 Latest commit 9c8227a
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/624e02de9746dd0009c18566
😎 Deploy Preview https://deploy-preview-32761--kubernetes-io-main-staging.netlify.app/docs/setup/production-environment/tools/kubeadm/install-kubeadm
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sftim sftim marked this pull request as ready for review April 5, 2022 15:36
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 5, 2022
@dims
Copy link
Member

dims commented Apr 5, 2022

/assign @SergeyKanzhelev

reylejano
reylejano reviewed Apr 5, 2022
View reviewed changes
content/en/docs/setup/production-environment/tools/kubeadm/install-kubeadm.md Outdated
{{< /table >}}

<br />
If both Docker and containerd are detected, Docker takes precedence. This is
If both Docker Engine and containerd are detected, the kubelet gives precedence to Docker Engine. This is
Copy link
Member

Choose a reason for hiding this comment

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

@SergeyKanzhelev , is this still true in v1.24?
Seeing if we need to make a follow-up PR to the dev-1.24 branch

Copy link
Member

@neolit123 neolit123 Apr 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

the 1.24 PR merged here, the behavior is different in 1.24:
https://github.com/kubernetes/website/pull/31309/files#diff-548585d5ff3dc7ff8a61b658052d84f223adca9361c9a4df3a8ce1105d9413bbR94-R95

If both Docker Engine and containerd are detected, the kubelet gives precedence to Docker Engine. This is

kubelet has no precedence or detection. it just defaults flag / config to docker in <1.24.

the "Docker Engine" rename seems fine.

Copy link
Member

@neolit123 neolit123 Apr 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

wouldn't this PR conflict with #31309 when k/website branches are merged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Part of the release docs process is to merge main into the release docs branch. For these changes, I'd expect that that merge should make sure that the v1.24 changes win (and I hope that's obvious enough). This comment should serve as an extra hint.

Copy link
Contributor Author

@sftim sftim Apr 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I'm happy if none of the changes from this PR survive into v1.24 docs.

@reylejano
Copy link
Member

LGTM from docs perspective
@kubernetes/sig-node-pr-reviews ,
It would be great to get a tech review from SIG Node

@sftim sftim force-pushed the 20220405_update_kubelet_container_runtime_detection_dockershim branch from 9de70a1 to bbe8e54 Compare April 6, 2022 12:45
@sftim @neolit123
Update kubeadm CRI detection docs
9c8227a 
Update kubeadm CRI detection docs in light of dockershim deprecation.

Co-authored-by: Lubomir I. Ivanov <[email protected]>
@sftim sftim force-pushed the 20220405_update_kubelet_container_runtime_detection_dockershim branch from b247c1a to 9c8227a Compare April 6, 2022 21:15
neolit123
neolit123 reviewed Apr 6, 2022
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.

/lgtm

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

LGTM label has been added.

Git tree hash: b73b167b6b2c8f5d314a2bd3ed2b8832c73954c9

@sftim
Copy link
Contributor Author

sftim commented Apr 12, 2022

/assign @reylejano

@reylejano
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: reylejano

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 12, 2022
@k8s-ci-robot k8s-ci-robot merged commit 383fd26 into kubernetes:main Apr 12, 2022
@sftim sftim deleted the 20220405_update_kubelet_container_runtime_detection_dockershim branch April 13, 2022 08:07
@chrisnegus chrisnegus mentioned this pull request Apr 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reylejano reylejano reylejano left review comments

@neolit123 neolit123 neolit123 left review comments

@natalisucks natalisucks Awaiting requested review from natalisucks

@tengqm tengqm Awaiting requested review from tengqm

@onlydole onlydole Awaiting requested review from onlydole

@shannonxtreme shannonxtreme Awaiting requested review from shannonxtreme

Assignees

@neolit123 neolit123

@SergeyKanzhelev SergeyKanzhelev

@reylejano reylejano

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/node Categorizes an issue or PR as relevant to SIG Node. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sftim @dims @reylejano @k8s-ci-robot @neolit123 @SergeyKanzhelev