Skip to content

fix: ensure kubelet and containerd in kube.slice via systemd drop-ins#8958

Open
titilambert wants to merge 19 commits into
mainfrom
fix-test-ensure-kubelet-and
Open

fix: ensure kubelet and containerd in kube.slice via systemd drop-ins#8958
titilambert wants to merge 19 commits into
mainfrom
fix-test-ensure-kubelet-and

Conversation

@titilambert

@titilambert titilambert commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

When node hardening is enabled, ensureKubeletCgroupHierarchy() creates a dedicated kubelet.slice for kube-reserved cgroup enforcement. However the existing drop-in for kubelet.service only declared Wants= and After= ordering — it did not set Slice=kubelet.slice, so kubelet remained in system.slice. Additionally, containerd.service had no drop-in at all and was never placed in the slice.

This PR fixes both issues:

  1. Adds Slice=kubelet.slice to the kubelet drop-in so kubelet actually runs inside the dedicated slice.
  2. Creates an equivalent drop-in for containerd.service (10-kubelet-slice.conf) so containerd is co-located in the same slice.

Without this fix, cgroup resource accounting under kubelet.slice is incomplete — node hardening's kube-reserved budget doesn't capture the real resource usage of kubelet and containerd, which blocks nodes to be ready since they are affected to system.slice which has a low max memory limit

Testing:

  • Updated ShellSpec tests to validate both drop-ins are written with the
    correct [Service] Slice=kubelet.slice directive.
  •  $ systemctl status containerd | grep "CGroup"
     CGroup: /kubelet.slice/containerd.service
     $ systemctl status kubelet | grep "CGroup"
     CGroup: /kubelet.slice/kubelet.service
    

Which issue(s) this PR fixes:

Fixes #

@titilambert

Copy link
Copy Markdown
Contributor Author

This change is part of the following stack:

Change managed by git-spice.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ensureKubeletCgroupHierarchy to set Slice=kubelet.slice for kubelet.service and add a matching drop-in for containerd.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.

Comment thread parts/linux/cloud-init/artifacts/cse_helpers.sh Outdated
Comment thread spec/parts/linux/cloud-init/artifacts/cse_helpers_spec.sh
Copilot AI review requested due to automatic review settings July 15, 2026 19:42
@titilambert
titilambert force-pushed the fix-test-ensure-kubelet-and branch from 39476df to 4045739 Compare July 15, 2026 19:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread parts/linux/cloud-init/artifacts/cse_helpers.sh Outdated
Comment thread spec/parts/linux/cloud-init/artifacts/cse_helpers_spec.sh
@titilambert titilambert changed the title fix(test): ensure kubelet and containerd are in kubelet.slice when node hardening is on fix: place kubelet and containerd in kubelet.slice via systemd drop-ins Jul 15, 2026

@SriHarsha001 SriHarsha001 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to look into the AI's comments, apart from that changes look good to me.

titilambert and others added 4 commits July 20, 2026 14:21
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>
Copilot AI review requested due to automatic review settings July 20, 2026 18:21
@titilambert
titilambert force-pushed the fix-test-ensure-kubelet-and branch from 9b3ab19 to 04ff5c2 Compare July 20, 2026 18:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copilot AI review requested due to automatic review settings July 21, 2026 17:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.

Files not reviewed (1)
  • aks-node-controller/pkg/gen/aksnodeconfig/v1/kubelet_config.pb.go: Generated file

Comment thread e2e/scenario_test.go
Comment thread pkg/agent/utils.go
Comment thread e2e/validators.go
Copilot AI review requested due to automatic review settings July 21, 2026 17:50
@titilambert
titilambert force-pushed the fix-test-ensure-kubelet-and branch from 9f31c19 to cd4fd98 Compare July 21, 2026 17:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 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 (2)

parts/linux/cloud-init/artifacts/cse_config.sh:394

  • This logs_to_events key is under ensureContainerd, but it uses the AKS.CSE.ensureKubelet... prefix. That makes event/telemetry attribution misleading when debugging containerd vs kubelet provisioning failures. Consider using an ensureContainerd-scoped key here while keeping the existing ensureKubelet key in ensureKubelet().
  # Node Memory Hardening: create kube-reserved.slice and drop-ins BEFORE starting
  # containerd/kubelet so both services start in the correct slice from the
  # beginning — avoids needing a disruptive restart after the fact.
  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

parts/linux/cloud-init/artifacts/cse_helpers.sh:1495

  • The PR title/description still describe creating/using kubelet.slice and 10-kubelet-slice.conf, but the implementation has moved to kube-reserved.slice and 10-kube-reserved-slice.conf. Please update the PR metadata to match the actual slice/drop-in names so consumers and future triage don’t search for the wrong units/files.
# 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.
#
# The function:
#   - Asserts cgroupv2 unified hierarchy (cgroupv1 is not supported by this feature
#     because mixed/legacy hierarchies cannot reliably enforce per-slice MemoryMax).
#   - Drops a /etc/systemd/system/kube-reserved.slice unit (system.slice ships with systemd).
#   - Triggers `systemctl daemon-reload` and `systemctl start kube-reserved.slice` so the
#     cgroup is materialised at /sys/fs/cgroup/kube-reserved.slice prior to kubelet boot.

Comment thread parts/linux/cloud-init/artifacts/cse_config.sh
Comment thread parts/linux/cloud-init/artifacts/cse_helpers.sh Outdated
Comment thread pkg/agent/utils.go

@SriHarsha001 SriHarsha001 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the code changes a few times and looks good to me.

@mxj220 mxj220 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Confirmed the cgroup/slice assignment will replace the aks-rp code exactly

Copilot AI review requested due to automatic review settings July 21, 2026 21:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 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:1551

  • ensureKubeletCgroupHierarchy now rejects any KUBE_RESERVED_CGROUP other than /kubereserved.slice. Given the repo still references /kubelet.slice as a legacy value (e.g., e2e/scenario_test.go and pkg/agent/utils_test.go simulate the RP sending it), hard-failing here risks breaking older/custom data payloads that still use /kubelet.slice (6‑month VHD support window). Consider accepting /kubelet.slice as a backward-compatible input (or translating it earlier) rather than returning failure.
    case "${KUBE_RESERVED_CGROUP:-}" in
        ""|"/kubereserved.slice"|"kubereserved.slice") ;;
        *)
            echo "ensureKubeletCgroupHierarchy: unsupported KUBE_RESERVED_CGROUP=${KUBE_RESERVED_CGROUP}; only /kubereserved.slice is supported"
            return 1

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 21, 2026 22:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 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

Comment thread parts/linux/cloud-init/artifacts/cse_helpers.sh Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 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 (2)

parts/linux/cloud-init/artifacts/cse_config.sh:392

  • In ensureContainerd(), the logs_to_events event name uses the "AKS.CSE.ensureKubelet." prefix even though this call is happening from the ensureContainerd path (other events in this function use "AKS.CSE.ensureContainerd."). This makes telemetry harder to attribute and can break event-based dashboards/alerts.
      if ! logs_to_events "AKS.CSE.ensureKubelet.ensureKubeletCgroupHierarchy" ensureKubeletCgroupHierarchy; then

parts/linux/cloud-init/artifacts/cse_helpers.sh:1585

  • The PR description/title refer to creating/using "kubelet.slice" and a "10-kubelet-slice.conf" drop-in, but the implementation in this PR uses "kubereserved.slice" and "10-kubereserved-slice.conf". This mismatch makes it harder for reviewers/operators to correlate the behavior described in the PR with what actually ships.
        # Drop-in on kubelet.service so systemd starts kubereserved.slice first
        # on every boot and places kubelet inside the slice.
        mkdir -p "${kubelet_dropin_dir}"
        tee "${kubelet_dropin_dir}/10-kubereserved-slice.conf" > /dev/null <<'EOF'
[Unit]

Comment on lines +1543 to +1545
# Validate supported values: /kubereserved.slice (or bare kubereserved.slice) is
# the only value accepted for KUBE_RESERVED_CGROUP. Only /system.slice is supported
# for SYSTEM_RESERVED_CGROUP (a built-in systemd slice). Reject any other value
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants