Repository navigation
If namespacing fails, give clearer errors as to why - #2290
Conversation
795cc9b to
d9cea5a
Compare
d9cea5a to
1eb363e
Compare
1eb363e to
a869f44
Compare
a869f44 to
bf735ab
Compare
palfrey
left a comment
There was a problem hiding this comment.
All of the println's in the pid == 0 are now replaced with exit code magic, and the privileged container situation has been documented
@palfrey made 9 comments.
Reviewable status: 0 of 1 LGTMs obtained, and 4 of 6 files reviewed, and 7 discussions need to be resolved.
nativelink-config/src/cas_server.rs line 862 at r1 (raw file):
Previously, chrisstaite-menlo (Chris Staite) wrote…
Might be worth noting that this is only true if they aren't
--priviledged
Added a comment about that
nativelink-worker/src/namespace_utils.rs line 100 at r1 (raw file):
Previously, chrisstaite-menlo (Chris Staite) wrote…
This is not async-signal-safe.
I've replaced all these with specific exit codes and a print outside of the pid == 0 section. I was trying something with https://docs.rs/signal-hook/latest/signal_hook/low_level/channel/struct.Channel.html to send full messages across the barrier, but then realised I could do fun things with exit codes and bitshifting instead
nativelink-worker/src/namespace_utils.rs line 102 at r1 (raw file):
Previously, chrisstaite-menlo (Chris Staite) wrote…
This is not async-signal-safe.
Same as the other ones
nativelink-worker/src/namespace_utils.rs line 107 at r1 (raw file):
Previously, chrisstaite-menlo (Chris Staite) wrote…
This is not async-signal-safe.
Same as the other ones
nativelink-worker/src/namespace_utils.rs line 109 at r1 (raw file):
Previously, chrisstaite-menlo (Chris Staite) wrote…
This is not async-signal-safe.
Same as the other ones
nativelink-worker/src/namespace_utils.rs line 114 at r1 (raw file):
Previously, chrisstaite-menlo (Chris Staite) wrote…
This is not async-signal-safe.
Same as the other ones
nativelink-worker/src/namespace_utils.rs line 115 at r1 (raw file):
Previously, chrisstaite-menlo (Chris Staite) wrote…
This is not async-signal-safe.
Same as the other ones
nativelink-worker/src/namespace_utils.rs line 119 at r1 (raw file):
Previously, chrisstaite-menlo (Chris Staite) wrote…
This is not async-signal-safe.
Same as the other ones
| error!(errno, "Namespaces: Error during unshare"); | ||
| if errno == libc::EPERM { | ||
| error!( | ||
| "If the worker is inside Docker, namespaces don't work unless it's a privileged container" |
There was a problem hiding this comment.
very helpful improvement IMO
chrisstaite-menlo
left a comment
There was a problem hiding this comment.
@chrisstaite-menlo reviewed 5 files and all commit messages, made 1 comment, and resolved 7 discussions.
Reviewable status: 0 of 1 LGTMs obtained, and all files reviewed, and 1 discussions need to be resolved.
web/platform/src/content/docs/docs/config/production-config.mdx line 588 at r2 (raw file):
Note this only applies for non-Dockerised workers, as workers in Docker can't make the new user namespace as they're already in a [chroot](https://en.wikipedia.org/wiki/Chroot). If however you use privileged containers, this works as
This is not the reason why, it's because they don't have the permission to create a nested user namespace. https://man7.org/linux/man-pages/man7/user_namespaces.7.html
bf735ab to
049ded8
Compare
palfrey
left a comment
There was a problem hiding this comment.
chroot -> notes about user namespace permissions
@palfrey made 3 comments.
Reviewable status: 0 of 1 LGTMs obtained, and 5 of 8 files reviewed, and pending CI: Redis store tester, macos-26, ubuntu-24.04, integration-tests, ubuntu-24.04 / stable, windows-2022 / stable, pre-commit-checks, Analyze (javascript-typescript), Analyze (python), Coverage, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Publish nativelink-worker-lre-rs, Web Platform Deployment / macos-26, Web Platform Deployment / ubuntu-24.04, Local / lre-cc / ubuntu-24.04, Local / lre-rs / macos-26, Local / lre-rs / ubuntu-24.04, Bazel Dev / macos-26, Bazel Dev / ubuntu-24.04, Cargo Dev / macos-26, Cargo Dev / ubuntu-24.04, Installation / macos-26, Installation / ubuntu-24.04, buck2, buildstream, mongo, rbe-toolchain, asan / ubuntu-24.04, CodeQL, and 1 discussions need to be resolved.
web/platform/src/content/docs/docs/config/production-config.mdx line 588 at r2 (raw file):
Previously, chrisstaite-menlo (Chris Staite) wrote…
This is not the reason why, it's because they don't have the permission to create a nested user namespace. https://man7.org/linux/man-pages/man7/user_namespaces.7.html
Fixed
049ded8 to
99939f7
Compare
99939f7 to
95d9bde
Compare
Description
Primarily built to aid debug of #2173 (comment) but investigation noted that it just doesn't work inside Docker!
We previously had 3 different locations all saying "Namespaces not supported" with no clarity as to why. This PR clarifies the reason in two cases (not running Linux), and in the 3rd case it adds logging for each of the possible failure paths.
Type of change
Please delete options that aren't relevant.
How Has This Been Tested?
bazel test //...and addinguse_namespacesto the integration testsChecklist
bazel test //...passes locallygit amendsee some docsThis change is