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

Update documentation to reflect removal of --network-plugin command line parameter in Kubelet 1.24 #39625

Closed
wants to merge 2 commits into from

Conversation

nitishfy
Copy link
Member

Fixes #34756
Closed PR: #34764

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. language/en Issues or PRs related to English language labels Feb 22, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign divya-mohan0209 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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 sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Feb 22, 2023
@netlify
Copy link

netlify bot commented Feb 22, 2023

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit d75820a
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/6419b4c52460df0008bc54fd
😎 Deploy Preview https://deploy-preview-39625--kubernetes-io-main-staging.netlify.app
📱 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.

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Maybe we should also update https://kubernetes.io/docs/tasks/administer-cluster/migrating-from-dockershim/troubleshooting-cni-plugin-related-errors/ to mention that the kubelet no longer needs a --network-plugin command line argument.

@kbhawkey
Copy link
Contributor

@nitishfy nitishfy force-pushed the Nitish/dockershim branch from 5ade2ea to d75820a Compare March 21, 2023 13:44
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 21, 2023
@@ -314,6 +314,15 @@ This is an incomplete list of things that could go wrong, and how to adjust your
- Mitigates: Node shutdown
- Mitigates: Kubelet software fault

{{<note>}}
From Kubernetes 1.24,the kubelet always uses CNI mode for networking. The legacy `--network-plugin`
Copy link
Contributor

@Ritikaa96 Ritikaa96 May 25, 2023

Choose a reason for hiding this comment

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

Suggested change
From Kubernetes 1.24,the kubelet always uses CNI mode for networking. The legacy `--network-plugin`
From Kubernetes 1.24, the kubelet is going to use CNI mode for networking. The legacy `--network-plugin`

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Thanks.

Here's some feedback. I'm sorry it took a while to get round to it - as a project, we're short on maintainer capacity.

Comment on lines +16 to +21
{{<note>}}

From Kubernetes 1.24,the kubelet always uses CNI mode for networking since the legacy `--network-plugin`
command line parameter has been removed from the binary. For more information, refer to [Troubleshooting Clusters](https://kubernetes.io/docs/tasks/debug/debug-cluster/) page.

{{</note>}}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is too early in the page. Make it normal text (not a note callout) and give it a heading.

@@ -314,6 +314,15 @@ This is an incomplete list of things that could go wrong, and how to adjust your
- Mitigates: Node shutdown
- Mitigates: Kubelet software fault

{{<note>}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be a callout?

@a-mccarthy
Copy link
Contributor

Hi @nitishfy, a gentle ping to see if you still have time to address the feedback on this PR? Thanks!

@nitishfy
Copy link
Member Author

Hi @nitishfy, a gentle ping to see if you still have time to address the feedback on this PR? Thanks!

Hi, I'll mark the changes soon. Thank you!

@kbhawkey
Copy link
Contributor

Hi @nitishfy .
I plan to close this pull request. When you have time to address the suggestions, feel free to reopen the pull request.
Thanks
/close

@k8s-ci-robot
Copy link
Contributor

@kbhawkey: Closed this PR.

In response to this:

Hi @nitishfy .
I plan to close this pull request. When you have time to address the suggestions, feel free to reopen the pull request.
Thanks
/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Signpost readers migrating from dockershim to migration guide (troubleshooting page)
6 participants