feat: multi-arch operator, bundle, and catalog image builds#3055
Open
polasudo wants to merge 9 commits into
Open
feat: multi-arch operator, bundle, and catalog image builds#3055polasudo wants to merge 9 commits into
polasudo wants to merge 9 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Member
|
/agentic_review |
Code Review by Qodo
1. Branch detection uses HEAD
|
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
Comment on lines
+185
to
+242
| run: | | ||
| SHORT_SHA=$(git rev-parse --short HEAD) | ||
| echo "SHORT_SHA=$SHORT_SHA" >> $GITHUB_ENV | ||
| BASE_VERSION=$(grep -E "^VERSION \?=" Makefile | sed -r -e "s/.+= //") | ||
| echo "BASE_VERSION=$BASE_VERSION" >> $GITHUB_ENV | ||
|
|
||
| latestNext="next" | ||
| if [[ $(git rev-parse --abbrev-ref HEAD) != "main" ]]; then | ||
| latestNext="latest" | ||
| fi | ||
| echo "LATEST_NEXT=$latestNext" >> $GITHUB_ENV | ||
|
|
||
| # Check which architectures actually built | ||
| echo "Build markers found:" | ||
| ls -la /tmp/build-markers/ || echo "No markers found" | ||
|
|
||
| - name: Login to registry (${{ env.REGISTRY }}) | ||
| uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4 | ||
| with: | ||
| registry: ${{ env.REGISTRY }} | ||
| username: ${{ vars.QUAY_USERNAME }} | ||
| password: ${{ secrets.QUAY_TOKEN }} | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3.12.0 | ||
|
|
||
| - name: Create manifest lists and push | ||
| run: | | ||
| export REGISTRY_WITH_ORG=${{ env.REGISTRY }}/${{ vars.REGISTRY_ORG }} | ||
| export OPERATOR_IMAGE_NAME=${{ vars.OPERATOR_IMAGE_NAME }} | ||
| OPERATOR_IMAGE_NAME=${OPERATOR_IMAGE_NAME:-operator} | ||
|
|
||
| set -ex | ||
|
|
||
| for image in ${OPERATOR_IMAGE_NAME} ${OPERATOR_IMAGE_NAME}-bundle ${OPERATOR_IMAGE_NAME}-catalog; do | ||
| echo "=== Creating manifest list for ${image} ===" | ||
|
|
||
| # Create manifest list for version tag | ||
| docker buildx imagetools create \ | ||
| -t ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION} \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-amd64 \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-arm64 | ||
|
|
||
| # Create manifest list for version-sha tag | ||
| docker buildx imagetools create \ | ||
| -t ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-${SHORT_SHA} \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-amd64 \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-arm64 | ||
|
|
||
| # Create manifest list for latestNext tag (next or latest) | ||
| docker buildx imagetools create \ | ||
| -t ${REGISTRY_WITH_ORG}/${image}:${LATEST_NEXT} \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${LATEST_NEXT}-amd64 \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${LATEST_NEXT}-arm64 | ||
|
|
||
| echo "=== Inspecting manifest for ${image}:${LATEST_NEXT} ===" | ||
| docker buildx imagetools inspect ${REGISTRY_WITH_ORG}/${image}:${LATEST_NEXT} | ||
| done |
There was a problem hiding this comment.
1. Missing set -u strictness 📘 Rule violation ☼ Reliability
New GitHub Actions run scripts use set -ex (or no strict mode) but omit set -u and do not
validate required environment variables, which can mask unset-variable failures. The scripts also
use unquoted variable expansions (e.g., >> $GITHUB_ENV, ${REGISTRY_WITH_ORG}/${image}...),
increasing brittleness and ShellCheck-style compliance risk.
Agent Prompt
## Issue description
The workflow `run` scripts are not hardened: they omit `set -euo pipefail` (notably `-u`), do not validate required env vars, and contain unquoted variable expansions.
## Issue Context
PR Compliance ID 5 requires strict modes, quoting, and env validation for shell scripts embedded in workflows.
## Fix Focus Areas
- .github/workflows/next-container-build.yaml[71-99]
- .github/workflows/next-container-build.yaml[114-140]
- .github/workflows/next-container-build.yaml[184-242]
- .github/workflows/next-container-build.yaml[247-264]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+228
to
+232
| # Create manifest list for version-sha tag | ||
| docker buildx imagetools create \ | ||
| -t ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-${SHORT_SHA} \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-amd64 \ | ||
| ${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-arm64 |
There was a problem hiding this comment.
2. Wrong sha manifest sources 🐞 Bug ≡ Correctness
The merge job creates the ${BASE_VERSION}-${SHORT_SHA} manifest list from
${BASE_VERSION}-amd64/arm64 instead of the SHA-scoped per-arch tags, so the commit-specific tag
points at the wrong image content. This also makes the pushed per-arch SHA tags unused (and cleanup
deletes tags that were never referenced).
Agent Prompt
## Issue description
The merge job tags `${BASE_VERSION}-${SHORT_SHA}` but incorrectly uses `${BASE_VERSION}-{amd64,arm64}` as sources. This makes the SHA tag not represent the commit build.
## Issue Context
The build job already pushes per-arch SHA tags `${BASE_VERSION}-${SHORT_SHA}-${arch}`.
## Fix Focus Areas
- .github/workflows/next-container-build.yaml[228-233]
## Implementation notes
Update the `docker buildx imagetools create` invocation for the `${BASE_VERSION}-${SHORT_SHA}` tag to reference:
- `${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-${SHORT_SHA}-amd64`
- `${REGISTRY_WITH_ORG}/${image}:${BASE_VERSION}-${SHORT_SHA}-arm64`
(instead of `${BASE_VERSION}-amd64/arm64`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+161
to
+183
| merge: | ||
| name: Create multi-arch manifests | ||
| runs-on: ubuntu-latest | ||
| needs: | ||
| - build | ||
| permissions: | ||
| contents: read | ||
| packages: write | ||
| env: | ||
| HAS_QUAY_OAUTH: ${{ secrets.QUAY_OAUTH_TOKEN != '' }} | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Download build markers | ||
| uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 | ||
| with: | ||
| path: /tmp/build-markers | ||
| pattern: build-marker-* | ||
| merge-multiple: true | ||
|
|
There was a problem hiding this comment.
3. Merge runs without builds 🐞 Bug ☼ Reliability
All build/push and marker upload steps are gated by any_changed == 'true', but the merge job runs unconditionally and will attempt to download markers and create manifests even when nothing was built. This can fail the workflow (no artifacts/tags) on pushes that only change ignored files (e.g., docs/tests).
Agent Prompt
## Issue description
`merge` always runs after the matrix job, but the matrix build/push steps are skipped when `changed-files` reports `any_changed != 'true'`. In that case, `merge` still tries to download build markers and create manifests from tags that were never pushed.
## Issue Context
The build job already computes `steps.changed-files.outputs.any_changed`, but that value is not available to `merge` and the build marker artifacts are only uploaded when changes exist.
## Fix Focus Areas
- .github/workflows/next-container-build.yaml[39-62]
- .github/workflows/next-container-build.yaml[146-160]
- .github/workflows/next-container-build.yaml[161-183]
## Implementation notes
Recommended approach:
1. Add a new `changes` job that runs `tj-actions/changed-files` once and exposes `outputs.any_changed`.
2. Add job-level `if: needs.changes.outputs.any_changed == 'true'` to both `build` and `merge`.
3. Update `merge.needs` to include `changes`.
Alternative (less clean): re-run `changed-files` in `merge` and gate the manifest/cleanup steps on its result.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+93
to
+98
| latestNext="next" | ||
| # for main branch, use next tags; for 1.y branches, use :latest tags | ||
| if [[ $(git rev-parse --abbrev-ref HEAD) != "main" ]]; then | ||
| latestNext="latest" | ||
| fi | ||
| echo "LATEST_NEXT=$latestNext" >> $GITHUB_ENV |
There was a problem hiding this comment.
4. Branch detection uses head 🐞 Bug ≡ Correctness
LATEST_NEXT is computed using git rev-parse --abbrev-ref HEAD, but the checkout step does not set ref: and commonly results in a detached HEAD; this can resolve to HEAD and incorrectly pick latest even on main. That mis-tags published images/manifests (next vs latest) for the main branch.
Agent Prompt
## Issue description
Branch/tag selection uses `git rev-parse --abbrev-ref HEAD`, which is unreliable under `actions/checkout` because the repo may be in detached HEAD state. This can miscompute `LATEST_NEXT`.
## Issue Context
Other workflows in this repo already use `${{ github.ref_name }}` to determine the branch.
## Fix Focus Areas
- .github/workflows/next-container-build.yaml[34-38]
- .github/workflows/next-container-build.yaml[93-98]
- .github/workflows/next-container-build.yaml[191-195]
## Implementation notes
Replace the `git rev-parse --abbrev-ref HEAD` checks with `${GITHUB_REF_NAME}` (or `${{ github.ref_name }}`) and compare against `main`. Apply the same fix in both the build and merge "Prepare" steps.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Replace the single-runner amd64-only build with a matrix strategy that builds on both ubuntu-24.04 (amd64) and ubuntu-24.04-arm (arm64), then merges per-arch images into multi-arch manifest lists. This follows the same proven pattern used by the hub repo's next-build-image.yaml workflow. The operator Dockerfile already supports TARGETOS/TARGETARCH and bundle images use FROM scratch, so no other changes are needed. All three images (operator, operator-bundle, operator-catalog) are now built and published as multi-arch manifest lists. Assisted-by: Claude Code
GitHub Actions doesn't allow direct secrets references in step `if` conditions. Use an env var intermediary instead. Assisted-by: Claude Code
The --annotation flag is not supported by docker buildx imagetools create on the ubuntu-latest runner. Expiry labels are already set during the per-arch image build via the Makefile LABEL variable. Assisted-by: Claude Code
- Use ${{ github.ref_name }} instead of detached-HEAD-unsafe git command
for branch detection; avoids mistagging images (next vs latest)
- Add strict shell mode (set -euo pipefail) and env var validation
across all run: blocks; quote all variable expansions
- Fix SHA-scoped manifest list to use ${BASE_VERSION}-${SHORT_SHA}-{amd64,arm64}
instead of ${BASE_VERSION}-{amd64,arm64} (was using wrong image content)
- Add separate `changes` job to gate both `build` and `merge` on
any_changed output; prevents merge from running when no files changed
- Remove unquoted redirections and improve shell robustness
All 4 Qodo findings resolved.
Assisted-by: Claude Code
polasudo
force-pushed
the
feat/multi-arch-operator-build
branch
from
367d356 to
38ac280
Compare
Add if: needs.changes.outputs.any_changed == 'true' to individual steps in build job (Prepare, Setup Go, Login, Build, Upload marker) to explicitly skip when no relevant changes detected. This makes the gating more granular and avoids unnecessary compute. Add clarifying comment above Prepare step. Assisted-by: Claude Code
GitHub Actions doesn't support bash-style default syntax (:-). Use || 'operator' instead for fallback values. Assisted-by: Claude Code
- Move QUAY_OAUTH_TOKEN from inline ${{ secrets }} expansion in the
curl command to the step's env: block, resolving SonarCloud warning
about secrets expanded directly in run steps
- Add always() && needs.build.result == 'success' to merge job's if
condition so it won't attempt manifest creation when build failed
Assisted-by: Claude Code
polasudo
force-pushed
the
feat/multi-arch-operator-build
branch
from
09fe0c7 to
256a876
Compare
The login step in the build job still referenced steps.changed-files.outputs.any_changed from the old single-job layout. Since the changed-files step now runs in a separate 'changes' job, this condition would always evaluate to false, skipping the registry login and causing all image pushes to fail with auth errors. Assisted-by: Claude Code
|
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
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
3 participants
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.
Summary
ubuntu-24.04(amd64) andubuntu-24.04-arm(arm64)mergejob that creates multi-arch manifest lists usingdocker buildx imagetools createWhat changes
File:
.github/workflows/next-container-build.yamlThe single
next-buildjob is replaced with two jobs:build— matrix over[ubuntu-24.04, ubuntu-24.04-arm], each runner builds all 3 images for its native architecture using existing Makefile targets (image-build,bundle-build,catalog-buildwithPLATFORMoverride), then pushes with per-arch tag suffixes (e.g.,operator:next-amd64)merge— downloads build markers, creates multi-arch manifest lists viadocker buildx imagetools create, tags final images (e.g.,operator:next), and optionally cleans up per-arch tags via Quay APIWhy
The community hub image (
quay.io/rhdh-community/rhdh) already ships as a multi-arch manifest list (amd64 + arm64). The operator stack is the remaining gap blocking ARM64 RHDH deployments via the operator.No Makefile or Dockerfile changes are needed — the operator Dockerfile already uses
TARGETOS/TARGETARCHbuild args and bundle images useFROM scratch.Pattern
Follows the same proven multi-arch build pattern from the hub repo's
next-build-image.yamlworkflow (matrix runners → per-arch push → manifest merge).Test plan
docker buildx imagetools inspect quay.io/rhdh-community/operator:nextshows both architecturespodman pull --platform linux/arm64 quay.io/rhdh-community/operator:nextsucceeds🤖 Generated with Claude Code