Fix issue 637

[gberche-orange]

Reverts #599 with #638, and harden cgroupv2 mounts detection by selecting the first mount

Hope this helps. Note that I'm unable to do further tests, and rely on suggested fix by @colins in #637

This was root-caused and verified by manually patching the three issues in a running stemcell 1.1234 container, after which the BOSH agent bootstrapped successfully.

---

1e4a114 was filtering on the cgroupv2 device to prevent a cgroup_mount variable with multiline content failing downsteam without clear errors.

cilium-originating mount

cat /proc/self/mounts | grep cgroup2
     # device  mount_point         fs_type dummy
    cgroup2 /sys/fs/cgroup/unified cgroup2 rw,nosuid,nodev,noexec,relatime 0 0
    none    /run/cilium/cgroupv2   cgroup2 rw,relatime 0 0

however, in warden/docker stemcells, the device is cgroup, which introduced regression #637

cat /proc/self/mounts | grep cgroup2
     # device  mount_point         fs_type dummy
     cgroup  /sys/fs/cgroup        cgroup2 rw,...

Applying suggestion by @colins in #637 to instead rely on the chronological ordering of mount points, and select the canonical cgroup2 mount point first added by the kernel during boot process.

https://man7.org/linux/man-pages/man5/proc_pid_mounts.5.html

/proc/self/mounts, lists the mounts of the process's own mount namespace. The format of this file is documented in fstab(5).

https://man7.org/linux/man-pages/man5/fstab.5.html

The order of records in fstab is important because fsck(8), mount(8), and umount(8)
sequentially iterate through fstab doing their thing

https://man7.org/linux/man-pages/man7/cgroups.7.html

Note that on many modern systems, systemd(1) automatically mounts
the cgroup2 filesystem at /sys/fs/cgroup/unified during the boot
process.

[coderabbitai]

Walkthrough

This PR refactors the cgroup v2 detection in the monit access helper script. The change renames the system_using_unified_cgroup_v2 function to monit_using_unified_cgroup_v2 and updates how the unified cgroup mount path is discovered from /proc/self/mounts. The validation logic shifts from counting matching mount entries to checking for empty resolved mount or path values, with a simplified error message on failure.

Suggested reviewers

• mkocher
• rkoster
• lnguyen
• s4heid

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (1 warning, 2 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Fix issue 637' is vague and does not clearly summarize the main change; it references an issue number without describing what the actual fix entails.	Revise the title to be more descriptive, such as 'Harden cgroupv2 mount detection by selecting first mount entry' or 'Fix cgroupv2 mount detection for warden/docker stemcells'.
Description check	❓ Inconclusive	The pull request description is comprehensive and detailed, explaining the motivation, technical context, and the proposed fix; however, it does not follow the repository's required 'Merge Forward' strategy template.	Add information about which stemcell branch this PR targets and confirm the merge-forward strategy (which branch should it be merged into next).

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@stemcell_builder/stages/bosh_monit/assets/monit-access-helper.sh`:
- Line 35: The current error echo "permit_monit_access: unable to resolve cgroup
v2 mount or path" is ambiguous; update the error handling where that string is
emitted to distinguish which check failed and include the actual variable values
(e.g., the cgroup v2 mount and path variables used in the script) in the log so
you can see whether the mount or the path is empty (or both); emit separate
messages like "permit_monit_access: cgroup v2 mount empty: <value>" and/or
"permit_monit_access: cgroup v2 path empty: <value>" or a combined message that
prints both variable values for diagnostics.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c50ba2f7-fbdc-4bca-8e20-dbf11f177b42

📥 Commits

Reviewing files that changed from the base of the PR and between d1790e3 and 701ac72.

📒 Files selected for processing (1)

• stemcell_builder/stages/bosh_monit/assets/monit-access-helper.sh

[coderabbitai]

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider making the error message more diagnostic.

The current message covers both failure cases (empty mount or empty path) but doesn't distinguish between them. For easier troubleshooting, consider separate error messages or include the actual (empty) values.

📋 Optional improvement for diagnostics

-        if [ -z "${cgroup_mount}" ] || [ -z "${current_cgroup}" ]; then
-            echo "permit_monit_access: unable to resolve cgroup v2 mount or path" >&2
+        if [ -z "${cgroup_mount}" ]; then
+            echo "permit_monit_access: no cgroup2 mount found in /proc/self/mounts" >&2
+            return 1
+        fi
+        if [ -z "${current_cgroup}" ]; then
+            echo "permit_monit_access: unable to resolve cgroup v2 path from /proc/self/cgroup" >&2
             return 1
         fi

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@stemcell_builder/stages/bosh_monit/assets/monit-access-helper.sh` at line 35,
The current error echo "permit_monit_access: unable to resolve cgroup v2 mount
or path" is ambiguous; update the error handling where that string is emitted to
distinguish which check failed and include the actual variable values (e.g., the
cgroup v2 mount and path variables used in the script) in the log so you can see
whether the mount or the path is empty (or both); emit separate messages like
"permit_monit_access: cgroup v2 mount empty: <value>" and/or
"permit_monit_access: cgroup v2 path empty: <value>" or a combined message that
prints both variable values for diagnostics.