entrypoint cleanup + non-systemd-host fix #2767
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder 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 |
k8s-ci-robot
added
approved
| @@ -55,43 +55,43 @@ validate_userns() { | |||
| } | |||
|
|
|||
| overlayfs_preferrable() { | |||
| if [[ -z "$userns" ]]; then | |||
There was a problem hiding this comment.
tabs => double space (like the rest of the file)
| @@ -225,9 +227,7 @@ fix_cgroup() { | |||
| # See: https://d2iq.com/blog/running-kind-inside-a-kubernetes-cluster-for-continuous-integration | |||
| # Capture initial state before modifying | |||
| # | |||
| # Basically we're looking for the cgroup-path for the cpu controller for the | |||
There was a problem hiding this comment.
things got re-arranged, and this comment got out of place.
| done | ||
| # workaround for hosts not running systemd | ||
| # we only do this for kubelet.slice because it's not relevant when not using | ||
| # the systemd cgroup driver |
There was a problem hiding this comment.
functional change here
There was a problem hiding this comment.
@AkihiroSuda @kolyshkin can you please chime in here, I'm really in deep waters here and I think that a another pair of eyes will be nice for this change ... the cgroup glue keeps growing ...
There was a problem hiding this comment.
This doesn't work, wishful thinking :-)
I need to get a v1 host going again and take a peek at what exactly systemd is up to with the pseudo-subsystem.
There was a problem hiding this comment.
I setup colima on an old personal machine to debug, this and a real slice works 0ffcf8d.
| @@ -234,7 +234,7 @@ fix_cgroup() { | |||
| # See: https://man7.org/linux/man-pages/man7/cgroups.7.html | |||
| echo 'INFO: fix cgroup mounts for all subsystems' | |||
| local cgroup_subsystems | |||
| cgroup_subsystems=$(findmnt -lun -o source,target -t cgroup | grep "${current_cgroup}" | awk '{print $2}') | |||
| cgroup_subsystems=$(findmnt -lun -o source,target -t cgroup | grep -F "${current_cgroup}" | awk '{print $2}') | |||
There was a problem hiding this comment.
-F / --fixed-strings means match fixed strings, not patterns / regexes.
we're intending to match literal values
BenTheElder
force-pushed
the
systemd-entrypoint
branch
from
949138e to
0ffcf8d
Compare
k8s-ci-robot
added
size/L
|
I've tested it in colima xref #2778, and this works now. |
|
Confirmed WSL 2 #2766 (comment) I'll bump the node image and then we should ship this to v0.14 |
| Before=slices.target | ||
|
|
||
| [Slice] | ||
| MemoryAccounting=true |
There was a problem hiding this comment.
Are these parameters ***Accounting needed twice?
There was a problem hiding this comment.
yes these parameters are for the slice. group vs specific unit.
https://www.freedesktop.org/software/systemd/man/systemd.slice.html
BenTheElder
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
Passed across github actions with the new node image. Tested locally by me in colima and cgroups v2 ~debian (with systemd). Base image already tested in all the pull-kubernetes* jobs across k8s versions in prow. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/retest |
|
/override pull-kind-e2e-kubernetes-1-22 |
|
@BenTheElder: Overrode contexts on behalf of BenTheElder: pull-kind-e2e-kubernetes-1-22 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. |
|
👌👍 |
|
full test runs in #2780, not sure why 1.22 flaked but it only flaked here and not on the 2 subsequent PRs. |
see #2765, #2743 (comment)