-
Notifications
You must be signed in to change notification settings - Fork 506
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
Do not conflict kubernetes-cni package for RPMs #1367
Conversation
/hold I'm not sure about this solution. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: saschagrunert 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 |
This should unbreak previous releases for debs and rpms which still rely on the kubernetes-cni package. Signed-off-by: Sascha Grunert <sgrunert@suse.com>
Added the review suggestions, please take a look. |
@saschagrunert I just tried this on a personal repo having just
|
Removing
|
I think the simplest path forward would be to remove the cni binaries out of the Kubelet spec and just create the
|
@dkoshkin We had a couple of problems with that approach before, breaking the workflow for many users. IIRC, that was the biggest reason why we didn’t create a new version of that package. |
I think this might be the best option. Release notes and upgrade documentation could include a note about the pre-req. If someone doesn't read that, it appears the package manager tips them off. |
Verified with
|
Okay cool, where can we document that? Is there any official documentation for those yum/deb repos? Otherwise I would point that out in the RPM spec. |
/retest |
The k8s.io docs don't go into using the repos directly, eg: https://kubernetes.io/docs/setup/ There's getting started for local learning (kind, minikube) and for prod various deploy tools. But no recommendation of using the yum and apt repos directly (vs indirectly through kubeadm). |
the existing yum/apt repositories are mentioned here: a note can be added there before the installation steps, but it must be done for all k/website branches where this behavior changed. |
Corresponding docs PR: kubernetes/website#21898 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ping @tpepper and @justaugustus for general consensus.
/hold cancel Seeking for approval. If this one got merged we have to:
|
/hold just until we have a final decision |
Superseded by #1375. |
@justaugustus: Closed this PR. In response to this:
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. |
What type of PR is this?
/kind regression
What this PR does / why we need it:
This should unbreak previous releases for debs and rpms which still rely
on the kubernetes-cni package.
Which issue(s) this PR fixes:
Fixes kubernetes/kubernetes#92242
Special notes for your reviewer:
Hold on, I might be completely wrong with that change. I'm not exactly sure about the implications so please review with care.
Does this PR introduce a user-facing change?