fix: ensure kubelet and containerd in kube.slice via systemd drop-ins#8958
fix: ensure kubelet and containerd in kube.slice via systemd drop-ins#8958titilambert wants to merge 15 commits into
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
There was a problem hiding this comment.
Pull request overview
This PR updates the Linux CSE helper responsible for Node Memory Hardening cgroup hierarchy setup so that both kubelet and containerd are configured to run under kubelet.slice, and adjusts ShellSpec coverage accordingly.
Changes:
- Extend
ensureKubeletCgroupHierarchyto setSlice=kubelet.sliceforkubelet.serviceand add a matching drop-in forcontainerd.service. - Update ShellSpec assertions to validate the additional systemd drop-in content for containerd.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
parts/linux/cloud-init/artifacts/cse_helpers.sh |
Adds systemd drop-ins to place kubelet and containerd into kubelet.slice when hardening cgroups are enabled. |
spec/parts/linux/cloud-init/artifacts/cse_helpers_spec.sh |
Updates tests to assert Slice= and validates the containerd drop-in is created. |
39476df to
4045739
Compare
SriHarsha001
left a comment
There was a problem hiding this comment.
We need to look into the AI's comments, apart from that changes look good to me.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- aks-node-controller/pkg/gen/aksnodeconfig/v1/kubelet_config.pb.go: Generated file
Comments suppressed due to low confidence (2)
parts/linux/cloud-init/artifacts/cse_helpers.sh:1551
- ensureKubeletCgroupHierarchy now hard-rejects any kube-reserved cgroup value other than /kube-reserved.slice (and will return failure). This is a breaking behavior change for any existing hardening configurations that still pass the legacy /kubelet.slice value (as described in the PR context), and would cause CSE to exit early (via ERR_KUBELET_START_FAIL) before containerd/kubelet start.
Recommendation: keep backward compatibility by accepting both /kubelet.slice and /kube-reserved.slice, and create/refresh the unit + drop-ins for whichever slice name is configured (or implement a safe migration strategy that also updates the kubelet config input).
# Validate supported values: only /kube-reserved.slice (or bare kube-reserved.slice) is
# supported for KUBE_RESERVED_CGROUP, and only /system.slice for
# SYSTEM_RESERVED_CGROUP (a built-in systemd slice). Reject any other value
# explicitly so kubelet doesn't fail later with an opaque enforcement error.
case "${KUBE_RESERVED_CGROUP:-}" in
""|"/kube-reserved.slice"|"kube-reserved.slice") ;;
*)
echo "ensureKubeletCgroupHierarchy: unsupported KUBE_RESERVED_CGROUP=${KUBE_RESERVED_CGROUP}; only /kube-reserved.slice is supported"
return 1
parts/linux/cloud-init/artifacts/cse_config.sh:885
- This refresh updates kubelet’s --runtime-cgroups value based on the desired slice, but if containerd is already running (e.g., PIS real nodes booting from an older cached VHD), the new containerd.service Slice= drop-in won’t take effect until containerd is restarted. That can leave kubelet pointing at /kube-reserved.slice/containerd.service while containerd is still in system.slice for the current boot.
Consider detecting a slice mismatch for an active containerd.service and restarting it before kubelet starts, so the runtime-cgroups value matches reality and the slice accounting fix actually takes effect on upgrade scenarios.
local containerd_runtime_cgroups="/system.slice/containerd.service"
if [ "${KUBE_RESERVED_CGROUP:-}" = "/kube-reserved.slice" ] || [ "${KUBE_RESERVED_CGROUP:-}" = "kube-reserved.slice" ]; then
containerd_runtime_cgroups="/kube-reserved.slice/containerd.service"
fi
tee "/etc/systemd/system/kubelet.service.d/10-containerd-base-flag.conf" > /dev/null <<EOF
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- aks-node-controller/pkg/gen/aksnodeconfig/v1/kubelet_config.pb.go: Generated file
Comments suppressed due to low confidence (1)
parts/linux/cloud-init/artifacts/cse_helpers.sh:1566
- The function claims to accept legacy KUBE_RESERVED_CGROUP values ("/kubelet.slice" / "kubelet.slice") for backward compatibility, but the implementation below always creates/starts kube-reserved.slice and always places kubelet/containerd into kube-reserved.slice. If an older RP still configures kubelet with --kube-reserved-cgroup=/kubelet.slice, kubelet will require /kubelet.slice to exist before startup and may fail NodeAllocatable enforcement (or at minimum enforce against a different cgroup than configured).
Either (a) fully support the legacy value by creating/enabling/starting kubelet.slice and wiring drop-ins/runtime-cgroups to kubelet.slice when that value is used, or (b) stop accepting /kubelet.slice and fail fast so the behavior is consistent and discoverable.
# /system.slice is a built-in systemd slice; we only need to create kube-reserved.slice.
# Accept the legacy /kubelet.slice value for backward compatibility with older RPs.
if [ "${KUBE_RESERVED_CGROUP:-}" = "/kube-reserved.slice" ] || [ "${KUBE_RESERVED_CGROUP:-}" = "kube-reserved.slice" ] \
|| [ "${KUBE_RESERVED_CGROUP:-}" = "/kubelet.slice" ] || [ "${KUBE_RESERVED_CGROUP:-}" = "kubelet.slice" ]; then
# Write all unit/drop-in files unconditionally (idempotent). This ensures
39ffe64 to
9b3ab19
Compare
| # Validate supported values: /kube-reserved.slice (or bare kube-reserved.slice) and | ||
| # the legacy /kubelet.slice are accepted for KUBE_RESERVED_CGROUP. Only /system.slice | ||
| # is supported for SYSTEM_RESERVED_CGROUP (a built-in systemd slice). Reject any other | ||
| # value explicitly so kubelet doesn't fail later with an opaque enforcement error. | ||
| case "${KUBE_RESERVED_CGROUP:-}" in | ||
| ""|"/kubelet.slice"|"kubelet.slice") ;; | ||
| ""|"/kube-reserved.slice"|"kube-reserved.slice"|"/kubelet.slice"|"kubelet.slice") ;; | ||
| *) | ||
| echo "ensureKubeletCgroupHierarchy: unsupported KUBE_RESERVED_CGROUP=${KUBE_RESERVED_CGROUP}; only /kubelet.slice is supported" | ||
| echo "ensureKubeletCgroupHierarchy: unsupported KUBE_RESERVED_CGROUP=${KUBE_RESERVED_CGROUP}; only /kube-reserved.slice (or legacy /kubelet.slice) is supported" | ||
| return 1 |
| # Refresh --runtime-cgroups for PIS nodes where basePrep may have baked an older | ||
| # 10-containerd-base-flag.conf pointing at /system.slice/containerd.service. | ||
| local containerd_runtime_cgroups="/system.slice/containerd.service" | ||
| if [ "${KUBE_RESERVED_CGROUP:-}" = "/kube-reserved.slice" ] || [ "${KUBE_RESERVED_CGROUP:-}" = "kube-reserved.slice" ] \ | ||
| || [ "${KUBE_RESERVED_CGROUP:-}" = "/kubelet.slice" ] || [ "${KUBE_RESERVED_CGROUP:-}" = "kubelet.slice" ]; then | ||
| containerd_runtime_cgroups="/kube-reserved.slice/containerd.service" | ||
| fi |
…de hardening is on
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
9b3ab19 to
04ff5c2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- aks-node-controller/pkg/gen/aksnodeconfig/v1/kubelet_config.pb.go: Generated file
Comments suppressed due to low confidence (2)
parts/linux/cloud-init/artifacts/cse_helpers.sh:1489
- The PR title/description refers to creating/using kubelet.slice (or "kube.slice"), but the implementation and tests now use kube-reserved.slice throughout. This mismatch can confuse verification/debugging; update the PR metadata to reflect kube-reserved.slice naming (or clarify/adjust the intended slice name).
# ensureKubeletCgroupHierarchy creates the systemd slices used by kubelet for the
# kube-reserved and system-reserved enforcement tiers (Node Memory Hardening F2/F5).
# It MUST be called before kubelet starts so that /kube-reserved.slice and /system.slice
# exist and are managed by systemd before the first kubelet enforcement pass.
#
parts/linux/cloud-init/artifacts/cse_config.sh:394
- In ensureContainerd(), the logs_to_events key uses the "AKS.CSE.ensureKubelet.*" namespace even though the call occurs during containerd setup. This will misattribute telemetry/diagnostics for failures in this path; use an ensureContainerd-scoped event name instead.
resolveKubeletReservedCgroups
if [ -n "${KUBE_RESERVED_CGROUP}" ] || [ -n "${SYSTEM_RESERVED_CGROUP}" ]; then
if ! logs_to_events "AKS.CSE.ensureKubelet.ensureKubeletCgroupHierarchy" ensureKubeletCgroupHierarchy; then
exit $ERR_KUBELET_START_FAIL
fi
What this PR does / why we need it:
When node hardening is enabled,
ensureKubeletCgroupHierarchy()creates a dedicatedkubelet.slicefor kube-reserved cgroup enforcement. However the existing drop-in forkubelet.serviceonly declaredWants=andAfter=ordering — it did not setSlice=kubelet.slice, so kubelet remained insystem.slice. Additionally,containerd.servicehad no drop-in at all and was never placed in the slice.This PR fixes both issues:
Slice=kubelet.sliceto the kubelet drop-in so kubelet actually runs inside the dedicated slice.containerd.service(10-kubelet-slice.conf) so containerd is co-located in the same slice.Without this fix, cgroup resource accounting under
kubelet.sliceis incomplete — node hardening'skube-reservedbudget doesn't capture the real resource usage ofkubeletandcontainerd, which blocks nodes to be ready since they are affected tosystem.slicewhich has a low max memory limitTesting:
correct
[Service] Slice=kubelet.slicedirective.Which issue(s) this PR fixes:
Fixes #