Fix randomly failing build.sh test - #587
Conversation
These fail randomly in GHA with messages like builder-fast-fail.sh: line 3: /cancelled-builds-fifo/fifo: No such file or directory that suggest that the sandbox was not actually set up properly. This could happen if the clone() call in mountAndPidNamespacesSupported() fails under load. So let's ensure an explicit error message in that case.
f555f32 to
e196a67
Compare
📝 WalkthroughWalkthroughThe changes centralize unprivileged user namespace detection and update cancelled-builds functional tests to run only with supported namespaces and without sandbox fallback. ChangesCancelled-builds functional tests
Estimated code review effort: 1 (Trivial) | ~4 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/functional/build.sh (1)
221-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLabel the namespace-limit diagnostics.
catwrites two bare values to stderr. If one/procfile is unavailable, the output does not identify which value belongs tomax_user_namespacesormax_pid_namespaces. Print each limit name with its value.Proposed diagnostic format
- cat /proc/sys/user/max_user_namespaces /proc/sys/user/max_pid_namespaces >&2 || true + for limit in max_user_namespaces max_pid_namespaces; do + printf '%s=' "$limit" >&2 + cat "/proc/sys/user/$limit" >&2 || printf '<unavailable>' >&2 + printf '\n' >&2 + done🤖 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 `@tests/functional/build.sh` at line 221, Update the namespace-limit diagnostic near the existing /proc reads to print each limit’s name together with its value, while retaining the best-effort behavior when either file is unavailable. Ensure max_user_namespaces and max_pid_namespaces remain distinguishable in stderr output.
🤖 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.
Nitpick comments:
In `@tests/functional/build.sh`:
- Line 221: Update the namespace-limit diagnostic near the existing /proc reads
to print each limit’s name together with its value, while retaining the
best-effort behavior when either file is unavailable. Ensure max_user_namespaces
and max_pid_namespaces remain distinguishable in stderr output.
There was a problem hiding this comment.
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 `@src/libutil/linux/linux-namespaces.cc`:
- Around line 76-81: Update the namespace probe error handling around
startProcess(), pid.wait(), and the child status checks to distinguish status 1
mount-namespace failures from status 2 /proc remount PID-namespace failures.
Ensure SysError messages are not always labeled as mount namespace failures;
report the specific failed operation when known, or use a neutral “namespace
probe failed” message.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19977531-95e0-4e7a-9a61-e412179e533d
📒 Files selected for processing (2)
src/libutil/linux/linux-namespaces.cctests/functional/build.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/functional/build.sh
75616d6 to
08b40b5
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Motivation
Fix random failures in
build.shin GHA, e.g. https://github.com/DeterminateSystems/nix-src/actions/runs/30926645783/job/92052907077.Context
Summary by CodeRabbit