build: use nix for building release images#3171
Conversation
|
@claude review |
Pull request overviewReplaces the legacy Changes:
Reviewed changesPer-file summary
FindingsCI is green across Blocking (must fix before merge): None. The flow looks correct and CI confirms it. Non-blocking (nits, follow-ups, suggestions):
✅ Approved |
gilcu3
left a comment
There was a problem hiding this comment.
Overall LGTM
I know this is just a draft but as I am not going to be able to review anything next week left some comments/suggestions, hopefully they are useful somehow.
| export NODE_IMAGE_NAME=mpc-node | ||
| ./deployment/build-images.sh --node --push | ||
| set -euo pipefail | ||
|
|
||
| # Tag mirrors the legacy build script: <branch>-<short-sha>, with | ||
| # `/` sanitized to `-`. | ||
| branch="${GITHUB_REF_NAME//\//-}" | ||
| short_sha=$(git rev-parse --short HEAD) | ||
| tag="${branch}-${short_sha}" | ||
| echo "Pushing as tag: ${tag}" | ||
|
|
||
| archive=$(nix build --no-link --print-out-paths .#node-image) | ||
| digest_file=$(nix build --no-link --print-out-paths .#node-image-manifest-digest) | ||
| echo "Manifest digest: $(cat "$digest_file")" | ||
|
|
||
| skopeo copy --preserve-digests \ | ||
| "docker-archive:${archive}" \ | ||
| "docker://docker.io/nearone/${IMAGE_NAME}:${tag}" |
There was a problem hiding this comment.
would be much nicer to keep the script ./deployment/build-images.sh and just modify it to run the nix steps. In general it is better to keep the ci workflows as lean as possible
| - name: Install skopeo | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y skopeo | ||
| branch="${GITHUB_REF_NAME//\//-}" | ||
| short_sha=$(git rev-parse --short HEAD) | ||
| tag="${branch}-${short_sha}" | ||
| echo "Pushing as tag: ${tag}" | ||
|
|
||
| - name: Build and push node gcp image | ||
| run: | | ||
| export NODE_GCP_IMAGE_NAME=mpc-node-gcp | ||
| ./deployment/build-images.sh --node-gcp --push | ||
| archive=$(nix build --no-link --print-out-paths .#node-gcp-image) | ||
| digest_file=$(nix build --no-link --print-out-paths .#node-gcp-image-manifest-digest) | ||
| echo "Manifest digest: $(cat "$digest_file")" | ||
|
|
||
| skopeo copy --preserve-digests \ | ||
| "docker-archive:${archive}" \ | ||
| "docker://docker.io/nearone/${IMAGE_NAME}:${tag}" |
There was a problem hiding this comment.
same here, we could keep the script and avoid adding logic to the workflows
| commit hash: 828f816be36aed6f0d2438e0131b3e9d7d0931ad | ||
| SOURCE_DATE_EPOCH used: 0 | ||
| node binary hash: 86c8f7d8913d6fe37a6992bba165d15a3a1d88fbf6cdff605e4827d5183721bc | ||
| node manifest digest: sha256:331cfec941671ac343c52847e255eb36a280da65535d2a1e4d002c4c64686e19 | ||
| $ nix build .#node-image-manifest-digest && cat result | ||
| sha256:331cfec941671ac343c52847e255eb36a280da65535d2a1e4d002c4c64686e19 |
There was a problem hiding this comment.
would be nice to keep printing the same things as before, else debugging might get more complicated.
| @@ -0,0 +1,43 @@ | |||
| { | |||
| runCommand, | |||
| skopeo, | |||
There was a problem hiding this comment.
I hoped skopeo would not be needed when moving to full nix. The way we use it is basically a coincidental trick to guarantee the image manifest is compressed deterministically.
|
Closing as stale. |
# Conflicts: # .github/workflows/docker_build_node.yml # .github/workflows/docker_build_node_gcp.yml # .github/workflows/docker_build_rust_launcher.yml # deployment/build-images.sh # docs/reproducible-builds.md # flake.nix # nix/mpc-node.nix # scripts/build-and-verify-rust-launcher-docker-image.sh
The script is kept as the lean entry point for the release workflows per review feedback on #3171. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
build-images.sh now needs nothing but nix and git: skopeo is re-exported from the flake (pinned through flake.lock, the same binary image-dir.nix uses) and invoked with `nix run .#skopeo`. Removes the apt install step from the three docker build workflows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pull request overviewReplaces the legacy Changes:
Reviewed changesPer-file summary
FindingsCI is green; the prior review's blocking-class concerns have all been resolved:
Blocking (must fix before merge): None. Non-blocking (nits, follow-ups, suggestions):
Note: the PR is currently ✅ Approved |
No description provided.