-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Decouple nativelink from toolchain containers #1013
Decouple nativelink from toolchain containers #1013
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @kubevalet
Reviewable status: 0 of 2 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Local / ubuntu-22.04, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable (waiting on @adam-singer and @allada)
e2f8907
to
2a2eb30
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 15 of 15 files at r1, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained, and pending CI: Remote / large-ubuntu-22.04 (waiting on @adam-singer)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 LGTMs obtained, and pending CI: Remote / large-ubuntu-22.04, and 1 discussions need to be resolved (waiting on @adam-singer)
-- commits
line 2 at r1:
nit: Not sure if I'd say this is breaking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 15 of 15 files at r1, all commit messages.
Reviewable status: 2 of 2 LGTMs obtained, and pending CI: Remote / large-ubuntu-22.04, and 2 discussions need to be resolved
tools/nativelink-worker-init.nix
line 12 at r1 (raw file):
# becomes hard to independently update toolchains and NativeLink. # # This image is
nit: odd early new line
3e2b289
to
feadd39
Compare
Changes to `nativelink` no longer require rebuilding toolchain containers and vice versa. The `nativelink` executable has been removed from the `createWorker` function. Instead, users should mount the `nativelink` executable into a toolchain container during deployment. This makes deployments more generic and provides out-of-the-box compatibility with any arbitrary toolchain container. Introduce a new `nativelink-worker-init` image, a thin wrapper around the `nativelink` container that copies the bundled `nativelink` executable to specified location. This can be used in worker deployments to populate temporary volumes with the `nativelink` executable. The new setup significantly improves setup times (observerd 90%+) for LRE-style deployments as workflows can fetch the `nativelink-worker-init` container instead of rebuilding the executable from scratch.
feadd39
to
b03eadc
Compare
Changes to
nativelink
no longer require rebuilding toolchain containers and vice versa.The
nativelink
executable has been removed from thecreateWorker
function. Instead, users should mount thenativelink
executable into a toolchain container during deployment. This makes deployments more generic and provides out-of-the-box compatibility with any arbitrary toolchain container.Introduce a new
nativelink-worker-init
image, a thin wrapper around thenativelink
container that copies the bundlednativelink
executable to specified location. This can be used in worker deployments to populate temporary volumes with thenativelink
executable. The new setup significantly improves setup times (observerd 90%+) for LRE-style deployments as workflows can fetch thenativelink-init
container instead of rebuilding the executable from scratch.This change is