-
Notifications
You must be signed in to change notification settings - Fork 26
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
kernel version should be 4.19+; recommend version 5.8 for cgroup v2 #37
kernel version should be 4.19+; recommend version 5.8 for cgroup v2 #37
Conversation
02aa92c
to
d773f70
Compare
/assign @neolit123 @SataQiu |
a4259a9
to
8aaafa2
Compare
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.
we could show an error for any version that is not in that list of Versions and we don't need a RecommendedVersions or warnings IMO.
the error message could be x is not in the list of recommended versions. %s, where %s is a VersionsNote from the KernelSpec API.
validators/types_unix.go
Outdated
// cgroup v2: Kubernetes recommended kernel version | ||
// https://kubernetes.io/docs/concepts/architecture/cgroups/ | ||
RecommendedVersions: []string{`^5\.[8-9].*$`, `^5\.[1-9][0-9].*$`, `^([6-9]|[1-9][0-9]+)\.([0-9]+)\.([0-9]+).*$`}, // Requires 5.8+, or newer for cgroup v2 | ||
RecommendedNote: "kernel version should >= '4.15', and 5.8+ is recommended.", |
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.
can be called VersionsNote
from a discussion on slack we had:
the checks are regexp based so could be 4.4 == ok, 4.19 == ok, any other >= 4.19 version == ok, any other version == err.
so in this note we can say that all those 4.x LTS versions are supported but the recommendation is to use 4.15 or 5.8+.
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.
VersionsNote: "kernel version should be in maintenance, at least 4.4 or 4.19+, and to use cgroup v2, 5.8+ is recommended.",
update it as above..
8aaafa2
to
f1c81c0
Compare
a313836
to
42443a8
Compare
c80cf2a
to
a9871d7
Compare
a9871d7
to
9778272
Compare
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.
just one final nit
https://github.com/kubernetes/system-validators/pull/37/files#r1691979088
and LGTM
9778272
to
c1779ba
Compare
Updated. |
c1779ba
to
8a3acbe
Compare
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
/approve
should we create a release after this PR merges?
/hold |
Yes.
We may wait for another week or two in case SIG-Node owners have some inputs. |
you should probably give them a ping on slack, or they will miss this. |
validators/types_unix.go
Outdated
Versions: []string{`^3\.[1-9][0-9].*$`, `^([4-9]|[1-9][0-9]+)\.([0-9]+)\.([0-9]+).*$`}, // Requires 3.10+, or newer | ||
// 4.4 & 4.19 are selected as kernel Super Long Term Support (SLTS), and the Civil Infrastructure Platform will provide support until at least 2026. | ||
Versions: []string{`^4\.4.*$`, `^4\.19.*$`, `^4\.[2-9][0-9].*$`, `^([5-9]|[1-9][0-9]+)\.([0-9]+)\.([0-9]+).*$`}, | ||
VersionsNote: "Recommended LTS versions from the 4.x series are 4.4 and 4.19. Any 4.x version newer than 4.19 is also supported. For cgroups v2 support, the minimal version is 4.15 and the recommended version is 5.8+", |
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.
for this note, do we have a check on cgroupv2 that the minimal version is satisfied?
cc: @harche
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 current statement is from
- https://github.com/opencontainers/runc/blob/ad5b481dace5cda8ca7c659b7717a15517333198/docs/cgroup-v2.md?plain=1#L23-L24
Recommended version: 5.2 or later
&Minimum version: 4.15
- https://kubernetes.io/docs/concepts/architecture/cgroups/#requirements:
Linux Kernel version is 5.8 or later
If the runc one is not accurate, we may change to the minimal version is 5.8
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'm fine with the message. Just was wondering if we need to add a validaiton in kubelet on start when cgroupv2 are used.
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.
there are many kernel version validations in kube. Mainly related to some FG or sysctl.
We may add a warning first for kubelet using cgoup v2.
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.
[1.32 placeholder for the cgroup v2 kernel version check in kubelet]kubernetes/kubernetes#126595
8a3acbe
to
b7357e1
Compare
According to some discussions in endoflife-date/endoflife.date#5608 (comment), the CIP seems to be not in https://www.kernel.org/category/releases.html and maintained in a different place(gitlab) than I prefer to remove the 4.4.* here. |
+1 to keep only official lts |
Updated in commit: update according to kernel official active kernel releases. |
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.
please squash the commits to 1 as well.
Signed-off-by: Paco Xu <[email protected]>
020ec91
to
e6e7785
Compare
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 APPROVED This pull-request has been approved by: neolit123, pacoxu, SataQiu 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 |
/unhold |
For kernel long term support, see https://wiki.linuxfoundation.org/civilinfrastructureplatform/start and https://endoflife.date/linux
Other comments that may be related: