Skip to content

Sampleworks container build fully fixed#281

Merged
denis-zaitsev merged 6 commits into
mainfrom
test-build
Jun 30, 2026
Merged

Sampleworks container build fully fixed#281
denis-zaitsev merged 6 commits into
mainfrom
test-build

Conversation

@denis-zaitsev

@denis-zaitsev denis-zaitsev commented Jun 30, 2026

Copy link
Copy Markdown
Contributor
  • Use the public image digest from the Docker build output instead of rebuilding a sha-* tag manually.
  • Pass that digest-pinned public image into the Astera EXT image build.
  • Add a small validation step so the Astera build fails early if the public image digest is missing or invalid.
  • Login to the public registry before the Astera build, because it needs to pull the public image by digest.
  • Make Docker build cache upload best-effort with ignore-error=true, so cache push failures do not fail the actual image release.
  • Verified that the new Harbor image was pushed and contains ext plus the ext shell hook.

Summary by CodeRabbit

  • Bug Fixes
    • Improved container image handling to use a verified digest for handoff between build stages, reducing the risk of pulling the wrong image version.
    • Added validation to fail builds when the expected image digest is missing or not properly pinned.
    • Made build cache uploads more resilient so cache-related issues won’t fail the workflow.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@denis-zaitsev, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 21 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6df5c4d5-ab7c-4fbc-ab8b-9d78161786c6

📥 Commits

Reviewing files that changed from the base of the PR and between 3312ca0 and 19423b8.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml
📝 Walkthrough

Walkthrough

The Docker workflow's image handoff between the public and astera jobs is upgraded from a tag-based reference to a verified SHA256 digest. The public job now outputs image-digest from the build step. The astera job gains a public registry login, a digest validation step, and injects the digest-pinned image reference as a build argument. Cache uploads in both jobs use ignore-error=true.

Changes

Digest-pinned image handoff in Docker workflow

Layer / File(s) Summary
Public job: digest output and cache update
.github/workflows/docker.yml
Job output renamed from image-ref to image-digest sourced from the public-build step's digest output; cache-to updated with ignore-error=true and provenance explicitly disabled.
Astera job: registry login, digest validation, and build arg
.github/workflows/docker.yml
New step logs in to the public registry; new validation step exits with error if digest is missing or not sha256:-prefixed; PIXI_WITH_CHECKPOINTS_IMAGE build arg updated to PUBLIC_REGISTRY/PUBLIC_IMAGE_NAME@<digest>; Astera cache upload uses ignore-error=true.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • diff-use/sampleworks#244: Introduced the split between public and astera jobs and first wired PIXI_WITH_CHECKPOINTS_IMAGE via a sha-based image-ref, which this PR upgrades to a digest-pinned reference.
  • diff-use/sampleworks#251: Also modifies the same digest/pinned reference handoff between public and astera jobs in the Docker workflow.
  • diff-use/sampleworks#277: Updates the same workflow to pass SHA256 digests between jobs and rewires build-arg/output validation in a closely related way.

Suggested reviewers

  • xraymemory
  • marcuscollins
  • mag-astera
  • Abdelsalam-Abbas

🐇 A digest so strong, no tag can compare,
SHA256 pinned with the greatest of care!
The astera job checks before it will build,
No imposter image — the workflow is filled
with hashes and logins and cache that won't break,
all wrapped in a ribbon for security's sake. 🎀

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is related to the container build fix, but it is too generic and does not clearly state the main workflow change. Rename it to mention the digest-pinned Docker workflow update, e.g. "Use public image digest for Sampleworks container build".
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test-build

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 @.github/workflows/docker.yml:
- Around line 228-231: The digest check in the workflow only verifies the
sha256: prefix and can still allow invalid values like sha256:not-a-digest.
Update the validation in the PUBLIC_IMAGE_DIGEST check to ensure the digest is a
complete, valid SHA256 value, not just prefixed correctly, so the early failure
catches bad inputs before the Astera build uses them.
🪄 Autofix (Beta)

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: 273e2b3f-2860-41ad-bbaa-f05c08955abc

📥 Commits

Reviewing files that changed from the base of the PR and between ae70c3c and 3312ca0.

📒 Files selected for processing (1)
  • .github/workflows/docker.yml

Comment on lines +228 to +231
if [ "${PUBLIC_IMAGE_DIGEST}" = "${PUBLIC_IMAGE_DIGEST#sha256:}" ]; then
echo "public job produced a non-sha256 digest: ${PUBLIC_IMAGE_DIGEST}"
exit 1
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Validate the full SHA256 digest, not just the prefix.

sha256: or sha256:not-a-digest passes this check and then fails later in the Astera build FROM, so the early validation does not catch invalid digests.

Suggested fix
-          if [ "${PUBLIC_IMAGE_DIGEST}" = "${PUBLIC_IMAGE_DIGEST#sha256:}" ]; then
+          if ! printf '%s\n' "${PUBLIC_IMAGE_DIGEST}" | grep -Eq '^sha256:[0-9a-f]{64}$'; then
             echo "public job produced a non-sha256 digest: ${PUBLIC_IMAGE_DIGEST}"
             exit 1
           fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ "${PUBLIC_IMAGE_DIGEST}" = "${PUBLIC_IMAGE_DIGEST#sha256:}" ]; then
echo "public job produced a non-sha256 digest: ${PUBLIC_IMAGE_DIGEST}"
exit 1
fi
if ! printf '%s\n' "${PUBLIC_IMAGE_DIGEST}" | grep -Eq '^sha256:[0-9a-f]{64}$'; then
echo "public job produced a non-sha256 digest: ${PUBLIC_IMAGE_DIGEST}"
exit 1
fi
🤖 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 @.github/workflows/docker.yml around lines 228 - 231, The digest check in the
workflow only verifies the sha256: prefix and can still allow invalid values
like sha256:not-a-digest. Update the validation in the PUBLIC_IMAGE_DIGEST check
to ensure the digest is a complete, valid SHA256 value, not just prefixed
correctly, so the early failure catches bad inputs before the Astera build uses
them.

Comment thread .github/workflows/ci.yml
cancel-in-progress: true

env:
CONDA_OVERRIDE_CUDA: "12"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We added CONDA_OVERRIDE_CUDA=12 because CI runs on CPU-only GitHub runners, but our Pixi environments depend on CUDA 12 packages. Newer Pixi checks this more strictly and fails if CUDA is not detected, even before tests start. This variable tells Pixi to resolve the environment as CUDA 12 without requiring a real GPU on the CI runner.

@denis-zaitsev denis-zaitsev merged commit e7c2830 into main Jun 30, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants