-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[KEP-5573]: follows up from review #5646
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
base: master
Are you sure you want to change the base?
Conversation
/assign @pacoxu @BenTheElder @dchen1107 |
|
||
NA. | ||
In this KEP, we will focus on the kubelet related work to remove cgroup v1. | ||
Projects like minikube, kubeadm, kubespray and others may need work to support this flag going to false |
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.
kubeadm already supports customizing kubelet config so I'm not sure if anything needs to be done here, updates to the preflight checks? docs?
kubeadm is a little bit weird because it's in-tree and used for release-blocking signal (similarly if the node_e2e or kube-up scripts didn't work we'd have a problem), I think calling out out-of-tree projects as out of scope is at least reasonable.
It's not super great to release k/k with the release binaries out of sync.
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.
The rest of the diff LGTM, not sure about this part.
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.
It's not super great to release k/k with the release binaries out of sync.
I don't follow what you mean here.
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.
kubeadm already supports customizing kubelet config so I'm not sure if anything needs to be done here, updates to the preflight checks? docs?
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.
I don't follow what you mean here.
Saying "this kep won't wait for another core release binary to be ready" is a little different from "we're not going to block on out of tree projects updating", kubeadm is part of the core kubernetes release. We shouldn't release a broken state.
I see ... commented
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.
kubeadm is a little bit weird because it's in-tree and used for release-blocking signal (similarly if the node_e2e or kube-up scripts didn't work we'd have a problem), I think calling out out-of-tree projects as out of scope is at least reasonable.
That's true and we also have an issue to move it out which is not updated for a long time. #1424
warns = append(warns, errors.New("cgroups v1 support is in maintenance mode, please migrate to cgroups v2"))
We have a warning message when cgroup v1 support went to maintenance mode.
After this change happened in kubelet, we will update it to an error IIUC. This will be tracked in kubernetes/system-validators#58.
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.
I took a stab at addressing this. I'm not entirely sure what the goal is for system-validators but I call that out as a goal for this KEP.
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.
/lgtm
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kannon92, SergeyKanzhelev 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 |
…folder to reflect true name of KEP
34647d0
to
ebf755a
Compare
/lgtm |
Address followups such as oom.group risk, calling out kubeadm as out of scope and renaming the top level folder to reflect the true name of the KEP.