Fix issue 637 by gberche-orange · Pull Request #640 · cloudfoundry/bosh-linux-stemcell-builder

gberche-orange · 2026-06-12T11:05:04Z

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 · 2026-06-12T11:05:19Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: dc5894b4-be6c-4793-bcff-2d4682afac34

📥 Commits

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

📒 Files selected for processing (1)

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

Walkthrough

This PR adds an exit statement to the awk pipeline in the monit access helper script's cgroup v2 mount path resolution. The change ensures the script terminates after extracting the first matching cgroup2 entry from /proc/self/mounts, preventing potential multiple results when multiple cgroup v2 mounts exist.

Suggested reviewers

colins
rkoster
mkocher

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.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 only references an issue number without describing what was actually fixed.	Use a more descriptive title that explains the actual change, such as 'Fix cgroupv2 mount detection by selecting first mount entry' to better communicate the technical nature of the fix.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description provides detailed context about the issue, the problem, the solution, and relevant documentation references, but does not follow the repository's 'Merge Forward' strategy template regarding branch handling and PR creation process.
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

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

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

beyhan · 2026-06-18T15:08:21Z

FYI: @gberche-orange we look into prs after the AI feedback is resolved.

@colins

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 cloudfoundry#637 ``` cat /proc/self/mounts | grep cgroup2 # device mount_point fs_type dummy cgroup /sys/fs/cgroup cgroup2 rw,... ``` Applying suggestion by @colins in cloudfoundry#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). https://man7.org/linux/man-pages/man5/fstab.5.html > The order of records in fstab is important because [fsck(8)](https://man7.org/linux/man-pages/man8/fsck.8.html), [mount(8)](https://man7.org/linux/man-pages/man8/mount.8.html), and [umount(8)](https://man7.org/linux/man-pages/man8/umount.8.html) > 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.

gberche-orange · 2026-06-23T10:38:53Z

@beyhan thanks for the heads up. I've now rebased the PR to clear unrelated outdated bot reviews, and addressed latest bot review.

coderabbitai Bot requested changes Jun 12, 2026

View reviewed changes

Comment thread stemcell_builder/stages/bosh_monit/assets/monit-access-helper.sh

gberche-orange mentioned this pull request Jun 12, 2026

PR #599 regresses warden/Docker stemcells: broken awk pattern and renamed function breaks restrict-monit-api-access #637

Closed

cf-foundation-community-automation Bot added this to Foundational Infrastructure Working Group Jun 12, 2026

cf-foundation-community-automation Bot moved this to Inbox in Foundational Infrastructure Working Group Jun 12, 2026

github-project-automation Bot moved this from Inbox to Waiting for Changes | Open for Contribution in Foundational Infrastructure Working Group Jun 12, 2026

beyhan requested review from a team, colins and mariash and removed request for a team June 18, 2026 15:06

gberche-orange force-pushed the fix-issue-637 branch from 701ac72 to 3db5ff0 Compare June 23, 2026 10:32

coderabbitai Bot approved these changes Jun 23, 2026

View reviewed changes

github-project-automation Bot moved this from Waiting for Changes | Open for Contribution to Pending Merge | Prioritized in Foundational Infrastructure Working Group Jun 23, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue 637#640

Fix issue 637#640
gberche-orange wants to merge 1 commit into
cloudfoundry:ubuntu-jammyfrom
orange-cloudfoundry:fix-issue-637

gberche-orange commented Jun 12, 2026

Uh oh!

coderabbitai Bot commented Jun 12, 2026 •

edited

Loading

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

beyhan commented Jun 18, 2026

Uh oh!

gberche-orange commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

gberche-orange commented Jun 12, 2026

Uh oh!

coderabbitai Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Suggested reviewers

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

beyhan commented Jun 18, 2026

Uh oh!

gberche-orange commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jun 12, 2026 •

edited

Loading