fix(ci): privileged Docker for debos OS image build#107
Conversation
debos with --disable-fakemachine requires /dev/loop-control. The default container namespace does not expose it, causing repeated failures to set up loop devices. --privileged matches typical debos CI usage. Co-authored-by: RicciP <rplunger@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe GitHub Actions OS build workflow changes the Docker invocation: it adds Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/os-build.yml (2)
26-36:--cap-add SYS_ADMINis redundant when--privilegedis set.The
--privilegedflag grants all capabilities to the container, making--cap-add SYS_ADMINsuperfluous. Consider removing it to reduce confusion, or keep it as documentation of the minimum capability that would be needed if a less permissive alternative is found later.♻️ Optional: Remove redundant capability
- name: Build image with debos run: | docker run --rm \ --privileged \ -v "${{ github.workspace }}:/build" -w /build \ - --cap-add SYS_ADMIN \ godebos/debos \ --disable-fakemachine -t machine:radxa-rock5c os/image.yaml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/os-build.yml around lines 26 - 36, The docker run step named "Build image with debos" includes a redundant capability flag: remove the explicit --cap-add SYS_ADMIN from the docker run invocation since --privileged already grants that capability (locate the docker run command in the "Build image with debos" step and delete the --cap-add SYS_ADMIN token); if you prefer to keep documentation of required capabilities instead, replace the flag with a brief inline comment mentioning SYS_ADMIN is required when using a non-privileged container.
26-36: Local build script and CI have different privilege handling for--disable-fakemachine.The local build script (
os/build.sh) explicitly avoids sudo escalation when--disable-fakemachineis used (line 16), running debos directly. However, the CI workflow requires--privilegedand--cap-add SYS_ADMINspecifically because--disable-fakemachineneeds access to loop devices (/dev/loop-control). This creates a parity gap: developers running./os/build.sh --disable-fakemachinelocally will encounter/dev/loop-controlpermission errors unless they manually run with sudo or have appropriate permissions.Either document this requirement in the script comments and README, or update
os/build.shto automatically wrap debos in a privileged Docker container when--disable-fakemachineis used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/os-build.yml around lines 26 - 36, The CI workflow uses a privileged Docker run with --privileged and --cap-add SYS_ADMIN to allow debos --disable-fakemachine to access /dev/loop-control, but os/build.sh does not — causing local builds to fail; update os/build.sh so that when the script detects the --disable-fakemachine flag it either 1) documents the requirement clearly in the script/README and exits with a clear message about needing root/privileged Docker, or preferably 2) automatically runs debos inside a privileged container (mirror the CI docker run invocation: mount $PWD, use --privileged and --cap-add SYS_ADMIN, pass --disable-fakemachine through) so local invocation matches .github/workflows/os-build.yml behavior and avoids /dev/loop-control permission errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/os-build.yml:
- Around line 26-36: The docker run step named "Build image with debos" includes
a redundant capability flag: remove the explicit --cap-add SYS_ADMIN from the
docker run invocation since --privileged already grants that capability (locate
the docker run command in the "Build image with debos" step and delete the
--cap-add SYS_ADMIN token); if you prefer to keep documentation of required
capabilities instead, replace the flag with a brief inline comment mentioning
SYS_ADMIN is required when using a non-privileged container.
- Around line 26-36: The CI workflow uses a privileged Docker run with
--privileged and --cap-add SYS_ADMIN to allow debos --disable-fakemachine to
access /dev/loop-control, but os/build.sh does not — causing local builds to
fail; update os/build.sh so that when the script detects the
--disable-fakemachine flag it either 1) documents the requirement clearly in the
script/README and exits with a clear message about needing root/privileged
Docker, or preferably 2) automatically runs debos inside a privileged container
(mirror the CI docker run invocation: mount $PWD, use --privileged and --cap-add
SYS_ADMIN, pass --disable-fakemachine through) so local invocation matches
.github/workflows/os-build.yml behavior and avoids /dev/loop-control permission
errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c15ff789-f72a-44b6-b1ca-89617458bb75
📒 Files selected for processing (1)
.github/workflows/os-build.yml
…-add cargo-deny failed on PR CI: fastrand 2.4.0 was yanked; update to 2.4.1 via cargo update -p fastrand. Remove --cap-add SYS_ADMIN under --privileged (CodeRabbit nit). Co-authored-by: RicciP <rplunger@users.noreply.github.com>
Problem
CI run 24064554874 failed on Build OS image during
deboswith--disable-fakemachine. Logs showed:could not open /dev/loop-control: no such file or directoryWith fakemachine disabled, debos runs on the host namespace inside the container and needs loop devices to build the image. The default
docker rundoes not expose/dev/loop-control.Fix
Add
--privilegedto thedocker runthat invokesgodebos/debos, which is the usual approach for debos/image builds in CI when fakemachine is not available.Verification
Workflow YAML validated; full OS image build is not reproduced locally (Docker-in-Docker differs from GitHub Actions).
Slack Thread
Summary by CodeRabbit