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

Docs: Updates for Deprecation of CNI network-plugin Flag #28046

Merged
merged 1 commit into from Oct 10, 2023
Merged

Docs: Updates for Deprecation of CNI network-plugin Flag #28046

merged 1 commit into from Oct 10, 2023

Conversation

danehans
Copy link
Contributor

@danehans danehans commented Sep 8, 2023

  • Updates kubelet command-line parameters in K8s requirements section.
  • Updates install steps for deprecated CLI flag in minikube install section.
  • Since the minikube install docs state minikube ≥ v1.28.0, notes specific to older minikube versions were removed.

Fixes: #26531

@danehans
Docs: Updates for Deprecation of CNI network-plugin Flag
3789a8c 
- Updates minikube install steps for deprecated CLI flag.
- Updates K8s requirements for kubelet command-line parameters.

Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>
@danehans danehans requested review from a team as code owners September 8, 2023 21:55
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Sep 8, 2023
@danehans
Copy link
Contributor Author

/test

learnitall
learnitall approved these changes Sep 13, 2023
View reviewed changes
Copy link
Contributor

@learnitall learnitall left a comment

Choose a reason for hiding this comment

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

This looks great, thank you!

As a follow-up to this, it would be great if we could add information on how to install a specific version of Cilium into minikube using the manifest option for the --cni parameter. I think the workflow would involve rendering a manifest using helm template and then passing the file path of the manifest to --cni. @danehans if you have time and would like to tackle this as part of your PR then please feel free, otherwise I can just make a follow-up issue.

@danehans
Copy link
Contributor Author

if you have time and would like to tackle this as part of your PR then please feel free, otherwise I can just make a follow-up issue.

@learnitall thanks for the review. I believe ^ is a separate issue from the one this PR is fixing. Feel free to create an issue and assign me.

@learnitall
Copy link
Contributor

Sounds good @danehans, will do!

@learnitall learnitall added the release-note/misc This PR makes changes that have no direct user impact. label Sep 13, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Sep 13, 2023
@learnitall learnitall added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. and removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Sep 13, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Sep 13, 2023
@julianwiedmann julianwiedmann added the dont-merge/discussion A discussion is ongoing and should be resolved before merging, regardless of reviews & tests status. label Sep 14, 2023
@ldelossa ldelossa removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Sep 19, 2023
@learnitall
Copy link
Contributor

@joamaki, when you have a moment, do you mind taking another look so we can resolve the feedback you brought up?

@danehans
Copy link
Contributor Author

danehans commented Oct 6, 2023

@learnitall @julianwiedmann is it possible to dismiss the PR review from @joamaki to get this unblocked?

@joamaki joamaki removed the dont-merge/discussion A discussion is ongoing and should be resolved before merging, regardless of reviews & tests status. label Oct 10, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 10, 2023
@joamaki joamaki merged commit 0f947fe into cilium:main Oct 10, 2023
59 checks passed
@danehans danehans deleted the issue_26531 branch March 8, 2024 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@learnitall learnitall learnitall approved these changes

@julianwiedmann julianwiedmann julianwiedmann left review comments

@joamaki joamaki joamaki approved these changes

Assignees
No one assigned
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Docs: Remove Deprecated network-plugin Kubelet Flag
5 participants
@danehans @learnitall @joamaki @julianwiedmann @ldelossa