chore: experiment with Remote Build Execution (RBE) @ Namespace#10579
Draft
basvandijk wants to merge 117 commits into
Draft
chore: experiment with Remote Build Execution (RBE) @ Namespace#10579basvandijk wants to merge 117 commits into
basvandijk wants to merge 117 commits into
Conversation
basvandijk
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an experimental GitHub Actions path to run bazel test using Namespace Bazel Remote Execution (BRE), including automation to build/mirror/optimize a worker image and keep the workflow pinned to an immutable digest.
Changes:
- Adds a new experimental workflow to run
bazel teston Namespace runners with remote execution enabled. - Extends the container autobuild workflow with jobs to mirror
ic-buildintonscr.io, optimize it for BRE, and automatically update the pinned worker-image digest used by the new workflow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/container-autobuild.yml | Adds jobs to create an optimized Namespace BRE worker image and auto-update the pinned digest reference in workflows. |
| .github/workflows/bre-namespace-test.yml | New opt-in workflow to run bazel test using Namespace remote execution with a pinned worker image. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add a new 'bre-namespace-test.yml' workflow that runs 'bazel test' on Namespace runners using Bazel Remote Execution (BRE). Actions execute on Namespace workers booted from a custom worker image (a mirror of ic-build). Extend 'container-autobuild.yml' to mirror the freshly built ic-build image into nscr.io, optimize it for BRE, and pin the resulting digest. These jobs are decoupled from the production image-reference update so an early-access BRE failure can never block it.
Addresses Copilot review: 'nsc bazel execution setup' writes short-lived credentials into the bazelrc, so 'cat'-ing it could leak auth material into the Actions logs.
…update job - Use the same fully-qualified nscr.io ref for the upload destination and the digest lookup, so the inspected tag is guaranteed to exist. - Guard bre-worker-image and bre-namespace-test against fork PRs via head.repo.full_name == github.repository (matching ci-kickoff.yml), since both run on privileged Namespace runners with pre-authenticated nsc. - Drop update-image-references from update-worker-reference's needs for true decoupling; the existing 'git pull --rebase' absorbs any concurrent push.
basvandijk
force-pushed
the
basvandijk/namespace-bazel-remote-execution
branch
from
27f3f10 to
95ee442
Compare
…ac895bdd550cac7bacb9dad553bae ic-build: sha256:f4c6c7e0e16da470cba7ebceb0145f588d5fd4859c04acfa607bee475ecfa914 ic-dev: sha256:2f98d344d708a1ae70938d5e777a1f141f7f2a9545687653f407a405eb1a27ea
Contributor
|
Run URL: https://github.com/dfinity/ic/actions/runs/28675533597 New container images with tag: |
update-image-references now depends on bre-worker-image (default needs semantics) and pins the worker digest unconditionally in the same commit, dropping the cancelled()-guard and the separate update-worker-reference job. Waiting on bre-worker-image before committing/pushing avoids the concurrency cancellation seen earlier.
nsc base-image upload prepends $NSC_CONTAINER_REGISTRY (nscr.io/<tenant>) to a relative name. Passing the fully-qualified nscr.io/dfinity ref caused a double-prefixed push (nscr.io/<tenant>/nscr.io/dfinity/...) and a 401 on the digest lookup. Derive the registry from $NSC_CONTAINER_REGISTRY, upload a relative name, pin the resulting full ref, and match any tenant in the pin sed. Also fixes a stale comment referencing the removed update-worker-reference job.
…e-bazel-remote-execution
…rent one On the Namespace RBE cluster the action runs as root inside a nested user namespace but in a network namespace owned by an ancestor userns. Capabilities are relative to the userns owning a resource, so root's CAP_NET_ADMIN does not apply there and every RTNETLINK operation of the local backend fails with 'Operation not permitted' -- even in a privileged action container. Internally this never bites because bazel's linux-sandbox gives each _local test a fresh netns owned by the sandbox's userns. Recreate what linux-sandbox provides: before the parent driver process spawns the tokio runtime and any task subprocess, probe whether the current netns is administrable and, if not, unshare(CLONE_NEWNET) into a fresh netns owned by our userns (where root holds full CAP_NET_ADMIN) and bring lo up. All descendants (tasks, libvirtd, QEMU, dnsmasq) inherit it. The local backend needs no external connectivity, so an isolated netns loses nothing.
Add the ic-test-utilities-privileges crate exposing run_as_nobody_if_root, which runs a test closure in a forked child that drops to the nobody user when the process is root, and in-process otherwise. Root holds CAP_DAC_OVERRIDE and therefore bypasses filesystem permission bits, so tests asserting PermissionDenied instead observe the operation succeeding and fail when run as root (e.g. on Bazel remote-execution workers). The helper forks, redirects TMPDIR/TEST_TMPDIR to a nobody-owned base, drops supplementary groups, gid, and uid to nobody, and runs the whole test body there; the child's outcome (including any panic message) is mirrored to the parent so #[should_panic] and its expected message keep working. Use it in the affected permission tests in rs/sys, the crypto service provider, and rs/state_manager.
nix 0.24.3 configures `setgroups` out on Apple targets, so the `imp` module failed to compile on darwin. The helper's semantics (the Linux `nobody` uid 65534 and CAP_DAC_OVERRIDE) are Linux-specific anyway, so gate `imp` on `target_os = "linux"` and fall through to the in-process no-op on the other platforms.
The tests test_ensure_file_exists_and_is_writeable_fails_if_non_writeable and test_save_proposal_id_to_file_fails_if_write_fails assert PermissionDenied on a read-only file. Root holds CAP_DAC_OVERRIDE and bypasses filesystem permission bits, so under Bazel remote-execution (running as root) the operations succeed and the assertions fail. Wrap both test bodies in ic_test_utilities_privileges::run_as_nobody_if_root so they drop to the nobody user when the process is root, mirroring the treatment applied in 380255a.
* Terminate the forked child with _exit(2) instead of std::process::exit, which would run atexit(3) handlers and stdio flushing inherited from the (potentially multi-threaded) parent test process. * Make the child's temp base nobody-owned with mode 0700 instead of root-owned world-writable 0777, matching the documentation and avoiding a predictable world-writable directory created as root. Create it with create_dir rather than create_dir_all so a pre-existing directory of unknown ownership is rejected. * Make the fork path safe in a multi-threaded test process without forcing --test-threads=1 on whole test suites: the temp base and putenv(3) NAME=VALUE strings are prepared in the parent before forking, and the child no longer calls std::env::set_var (whose global environment lock another parent thread could have held at fork time), using putenv with the pre-allocated strings instead. * Add smoke tests for the helper, including a concurrency regression test that forks from multiple threads; verified as root, where they exercise the fork + privilege-drop path.
Requested in review: each of the 12 wrapped tests now carries a short comment explaining that root bypasses file permission bits (CAP_DAC_OVERRIDE), so the permission denial the test expects only happens when dropping to nobody on root-run workers (e.g. Bazel remote execution).
Requested in review: wrapping whole test bodies in a closure pollutes them with extra code and indentation. Add an attribute that does the wrapping instead. Proc-macros must live in their own crate, so the attribute is defined in the new ic-test-utilities-privileges-macros crate and re-exported from ic-test-utilities-privileges (the serde/serde_derive facade pattern, like canlog re-exports canlog_derive); consumers keep a single dependency. The attribute composes with #[test] and #[should_panic] (the wrapper re-raises the child's panic message) and rejects async fns and non-unit return types at compile time. Convert all 12 call sites to the attribute form and extend the smoke tests with attribute-form cases; verified as root, where the permission-denied tests only pass because the attribute drops privileges to nobody.
…e-bazel-remote-execution
…basvandijk/namespace-bazel-remote-execution
The merge auto-combined HEAD's closure form (run_as_nobody_if_root(|| {...}))
with the incoming branch's #[as_nobody_when_root] attribute, double-wrapping
the permission-denied tests. Take the incoming branch's final attribute-only
form for the affected test files.
The RBE Namespace job crashed linking big system-test binaries
(e.g. //rs/tests/consensus/tecdsa:tecdsa_signature_from_other_subnet_test_bin)
with the zig linker child dying with SIGABRT:
abnormal exit: child_process.ChildProcess.Term{ .Signal = 6 }
On Namespace RBE workers every action runs in its own execroot
(.../execroots/action-<id>) but shares the worker's /tmp/zig-cache, so all
concurrently executing actions fall into the same 'config-catchall' shard of
the zig cache. zig 0.12's cache is racy under concurrent writers
(ziglang/zig#23993,
uber/hermetic_cc_toolchain#224) which is what the
existing cache-dir workaround mitigates for foreign_cc configure builds. The
master merge (rules_rust 0.71.3) invalidated every Rust action at once, so
this run was the first to execute thousands of cold zig links concurrently on
freshly provisioned workers, maximizing the race window.
Extend the zig-wrapper's cache sharding to key the cache on the per-action
execroot segment when the working directory contains an 'execroots' path
segment. Invocations from subdirectories of one action (e.g. foreign_cc
configure scripts) still share that action's cache, and local execution
(no 'execroots' segment) is unaffected. Cold-cache cost is only paid by link
actions (~17s); compile-only zig invocations barely use the cache.
…e-bazel-remote-execution
…e-bazel-remote-execution
…e-bazel-remote-execution
…e-bazel-remote-execution
Replace the large comment block above the bazel invocation with a bazel_args array where each flag (or group of flags) is documented at its definition.
…-remote-execution
…e-bazel-remote-execution
…7dac66460efb58bd61345c7a376cd ic-build: sha256:050045f83ecb0037c4fade65e322aac4d1e22e080f70c18d4c84504bba0acccc ic-dev: sha256:ece8b162493b06a7e59642c24a1a8b27d1a05ad002c1e1241d0d8abc1e8d2ef8 ic-build-worker: nscr.io/c9ptjuknd7oc6/ic-build-worker@sha256:65f912a41be8cf3e9247d8dc34f317ddfc11badf8dc93f137c1702d6a2cd2790
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Experiment with Bazel's Remote Build Execution (RBE) @ Namespace: run
bazel teston Namespace runners, with actions executed on remote Namespace workers booted from a custom worker image (a mirror ofic-build).Changes
Extended:
.github/workflows/container-autobuild.ymlrbe-worker-imagejob (on the Namespace runner wherenscis pre-authenticated): mirrors the freshly builtic-buildimage — by digest — into the Namespace tenant registry ($NSC_CONTAINER_REGISTRY, i.e.nscr.io/<tenant>/ic-build-worker) viansc base-image upload, resolves the pushed digest, and optimizes it for RBE viansc base-image optimize. Same fork guard as above.ic-build-image: if it fails, that's a bug to fix.update-image-referencesjob now also pins the worker image. Itneedsrbe-worker-image(so its commit/push — which re-triggers the workflow and would otherwise cancel the in-flight optimize viacancel-in-progress— only happens once the optimize completes), and rewrites the pinned ref inrbe-namespace-test.ymlin the same commit as theic-build/ic-devref andTAGupdates. The pinsedis tenant-agnostic, so it self-corrects if the Namespace tenant ever changes.New workflow:
.github/workflows/rbe-namespace-test.ymlbazel teston a Namespace runner (namespace-profile-amd64-linux-32x64) using RBE. Targets default to//...and are overridable via aworkflow_dispatchinput.nsc bazel execution setup(writes a bazelrc with the remote executor, cache and credentials — deliberately not printed, since it contains short-lived credentials).--remote_default_exec_properties=container-image=....--noworkspace_rc+ explicit.bazelrc.build+ the Namespace RBE bazelrc), mirroring the existingbazel-test-arm64job.--test_tag_filtersand runs with--keep_going.workflow_dispatch, pushes todev-gh-*, or non-fork PRs labeledCI_RBE. Restricted todfinity/ic; fork PRs are excluded because the job runs on a privileged Namespace runner with pre-authenticatednsc.Notes / follow-ups
ic-buildrebuild (bumpci/container/TAG).rbe-worker-imageis required, a Namespace/RBE outage would block the productionic-build/ic-devreference bump — a conscious trade-off (both must succeed).//...under RBE (ic-os local-strategy targets, privileged/system tests); the broad--test_tag_filtersexclusions and--keep_goingreduce noise while iterating.