-
Notifications
You must be signed in to change notification settings - Fork 2
ci: improve workflow reliability and add multi-arch containers #8
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,25 +1,55 @@ | ||||||||||||||||||||||||||||||||||||||||||||
| name: container | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||||||||||||||||||||
| # Build validation on PRs (no push) | ||||||||||||||||||||||||||||||||||||||||||||
| pull_request: | ||||||||||||||||||||||||||||||||||||||||||||
| push: | ||||||||||||||||||||||||||||||||||||||||||||
| # Build and push only after CI passes on main/tags | ||||||||||||||||||||||||||||||||||||||||||||
| workflow_run: | ||||||||||||||||||||||||||||||||||||||||||||
| workflows: [CI] | ||||||||||||||||||||||||||||||||||||||||||||
| types: [completed] | ||||||||||||||||||||||||||||||||||||||||||||
| branches: [main] | ||||||||||||||||||||||||||||||||||||||||||||
| # Also trigger on tags (CI doesn't run on tags, so direct trigger) | ||||||||||||||||||||||||||||||||||||||||||||
| push: | ||||||||||||||||||||||||||||||||||||||||||||
| tags: ['v*'] | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+7
to
13
|
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||||||||||||||||||||||||
| contents: read | ||||||||||||||||||||||||||||||||||||||||||||
| packages: write | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| # Cancel in-progress runs for the same branch | ||||||||||||||||||||||||||||||||||||||||||||
| concurrency: | ||||||||||||||||||||||||||||||||||||||||||||
| group: ${{ github.workflow }}-${{ github.ref }} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
| group: ${{ github.workflow }}-${{ github.ref }} | |
| group: ${{ github.workflow }}-${{ github.event_name == 'workflow_run' && github.event.workflow_run.head_branch || github.ref }} |
Copilot
AI
Jan 19, 2026
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.
The runner label ubuntu-24.04-arm is being used for native ARM64 builds. This is a GitHub-hosted runner label that may not be available in all GitHub plans or repositories. You should verify that your repository has access to ARM64 runners before merging this change. If these runners are not available, the workflow will fail with a "No runner matching the specified labels was found" error.
For public repositories or those without access to GitHub-hosted ARM64 runners, consider using QEMU emulation on standard ubuntu-latest runners or self-hosted ARM64 runners instead.
| - runner: ubuntu-24.04-arm | |
| - runner: ubuntu-latest |
Copilot
AI
Jan 19, 2026
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.
When using workflow_run trigger with head_sha, there's a potential issue if the workflow_run is triggered by a pull request. The github.event.workflow_run.head_sha points to the PR commit, but the code doesn't check out the PR merge commit. This could result in building a container image from code that hasn't been tested with the target branch.
For pull_request events that triggered the CI workflow, you might want to use github.event.workflow_run.pull_requests[0].head.sha or ensure the ref matches what CI actually tested.
| # For workflow_run, checkout the commit that triggered CI | |
| ref: ${{ github.event.workflow_run.head_sha || github.ref }} | |
| # For workflow_run, checkout the commit that CI tested | |
| ref: ${{ (github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].head.sha) || github.event.workflow_run.head_sha || github.ref }} |
Copilot
AI
Jan 19, 2026
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.
The metadata extraction in the build job (line 65-69) doesn't include any tag configuration, while the merge job (lines 121-131) has comprehensive tag configuration. This discrepancy means the build job generates labels but no tags, which is fine for the digest-based push approach. However, this could be clarified with a comment or by adding a tags field that explicitly outputs nothing, to make the intentional difference clear for maintainability.
| images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} | |
| images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} | |
| # Intentionally omit tags in the build job; we push by digest only here. | |
| tags: "" |
Copilot
AI
Jan 19, 2026
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.
The Docker build outputs specification on line 78 is very long and complex. The push parameter is conditionally set based on the event type, but it's embedded in a long outputs string. If this evaluates to push=false but push-by-digest=true is still set, the behavior might be unexpected.
Consider breaking this into separate parameters or adding a comment explaining the expected behavior for PR builds vs. production builds with push-by-digest.
| outputs: type=image,name=${{ env.REGISTRY }}/${{ env.IMAGE_NAME }},push-by-digest=true,name-canonical=true,push=${{ github.event_name != 'pull_request' }} | |
| # For pull_request events we only build (no push). For workflow_run and tag | |
| # pushes, we both build and push by digest. The `push` flag below is set | |
| # conditionally based on the event type to reflect this behavior. | |
| outputs: >- | |
| type=image, | |
| name=${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}, | |
| push-by-digest=true, | |
| name-canonical=true, | |
| push=${{ github.event_name != 'pull_request' }} |
Copilot
AI
Jan 19, 2026
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.
The shell command uses printf with a glob pattern * to iterate over digest files. However, if either architecture build fails (despite fail-fast: false), the merge job will still run but may have incomplete digests. This could result in creating a manifest list with only one architecture instead of both, which would be a silent failure.
Consider adding validation to ensure both architecture digests are present before creating the manifest, or handle the case where only partial builds succeeded.
| docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ | |
| $(printf '${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}@sha256:%s ' *) | |
| set -euo pipefail | |
| # Collect digest files and ensure we have one per architecture | |
| shopt -s nullglob | |
| digests=( * ) | |
| if [ "${#digests[@]}" -lt 2 ]; then | |
| echo "Error: expected at least 2 digest files (for multiple architectures), found ${#digests[@]}." >&2 | |
| echo "Digest files present: ${digests[*]:-"<none>"}" >&2 | |
| exit 1 | |
| fi | |
| digest_args=() | |
| for d in "${digests[@]}"; do | |
| digest_args+=( "${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}@sha256:${d}" ) | |
| done | |
| docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ | |
| "${digest_args[@]}" |
Copilot
AI
Jan 19, 2026
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.
The image inspection command uses steps.meta.outputs.version, but this output may not always contain a valid tag depending on the event type. For example, when triggered by workflow_run on the main branch, the metadata action will generate tags like edge (for main branch) or sha-xxx, and version might not be the most appropriate tag to inspect.
Consider using a more robust tag reference like ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:edge for main branch builds, or adjusting the inspection to use one of the generated tags that's guaranteed to exist.
| - name: Inspect image | |
| run: | | |
| - name: Inspect image (workflow_run: edge tag) | |
| if: github.event_name == 'workflow_run' | |
| run: | | |
| docker buildx imagetools inspect ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:edge | |
| - name: Inspect image (push: version tag) | |
| if: github.event_name == 'push' | |
| run: | |
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.
The concurrency control uses
github.refwhich works well for branch pushes but may not be optimal for pull requests. For pull requests,github.refis the merge ref (e.g.,refs/pull/123/merge), which is unique per PR. However, when a PR is updated with new commits, the old CI run might still be valuable to cancel.Consider using
github.head_ref || github.refwhich uses the PR head branch for pull requests, ensuring that new commits to the same PR cancel the old runs, while maintaining proper isolation for different PRs.