ISSUE-114: Separate OKD build and push phases #126
Conversation
📝 WalkthroughWalkthroughAdds staging/production modes to OKD image build/push with registry derivation and promotion; refactors build_images.sh to support staging/production/list-packages flows and arch-specific release-image overrides; exports OKD version as a job output; adds cleanup-staging composite action; updates Makefile to require arch-specific OKD_RELEASE_IMAGE_* variables. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as "GitHub Actions"
participant BuildAction as "build-okd Action"
participant BuildScript as "build_images.sh"
participant Staging as "Staging Registry"
participant Tests as "MicroShift Build/Test"
participant Prod as "Production Registry"
GH->>BuildAction: start build-okd (mode=staging, arch)
BuildAction->>BuildScript: build_okd_images (mode=staging)
BuildScript->>Staging: push staging-tagged images (record SHAs)
GH->>Tests: run MicroShift build/tests referencing staging images
alt tests pass
GH->>BuildAction: trigger production flow
BuildAction->>BuildScript: retag_staging_to_production
BuildScript->>Staging: pull staging tags
BuildScript->>Prod: retag & push production tags
end
GH->>BuildAction: run cleanup-staging (uses okd-version-tag)
BuildAction->>BuildScript: list-packages -> output staging package names
BuildAction->>Staging: delete listed staging packages (best-effort)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
kasturinarra
changed the title
kasturinarra
force-pushed
the
fix_issue_114
branch
from
61e7990 to
846b22d
Compare
kasturinarra
changed the title
kasturinarra
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/actions/build-okd/action.yaml(2 hunks)src/okd/build_images.sh(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-17T07:44:32.742Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 92
File: .github/workflows/release.yaml:44-50
Timestamp: 2025-10-17T07:44:32.742Z
Learning: When Podman builds an image without a registry prefix (e.g., `podman build -t microshift-okd`), it automatically adds the `localhost/` prefix and `:latest` tag, resulting in `localhost/microshift-okd:latest`. This means the Makefile in microshift-io/microshift building with `-t microshift-okd` produces `localhost/microshift-okd:latest` without explicit retagging.
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: ubuntu-rpm2deb
- GitHub Check: fedora-bootc (ubuntu-24.04-arm)
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: fedora-bootc (ubuntu-24.04)
- GitHub Check: isolated-network (ovnk, ubuntu-24.04, 1)
- GitHub Check: centos10-bootc
- GitHub Check: centos9-bootc
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
🔇 Additional comments (5)
src/okd/build_images.sh (1)
14-19: Usage message now displays registry values after variable definitions.Once STAGING_REGISTRY and PRODUCTION_REGISTRY are defined (per the first comment), the usage message will correctly display them. No further action needed here after the variable definitions are added.
.github/actions/build-okd/action.yaml (4)
48-59: Mode parameter is correctly positioned as first argument.The build phase correctly invokes
build_images.sh buildwith mode as the mandatory first parameter, addressing prior review feedback.
61-73: Staging registry reference is consistent with build_images.sh.Line 72 hardcodes
ghcr.io/microshift-io/okd-staging, which aligns with theSTAGING_REGISTRYvalue that should be defined in build_images.sh (per the critical fix in the first file).
102-115: Conditional push to production mirrors test success.The
if: success()guard correctly ensures push only occurs after all prior steps (build, RPM build, bootc build, and tests) complete successfully, enforcing the build→test→push sequencing per PR objectives.
117-125: Cleanup step is informational only.The "Cleanup staging registry on failure" step logs advisory messages but performs no actual cleanup (no skopeo delete, podman rmi, etc.). This is acceptable for now since staging images will be overwritten on the next build, though true cleanup could be added later if needed.
kasturinarra
force-pushed
the
fix_issue_114
branch
from
846b22d to
7030690
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/actions/build-okd/action.yaml (1)
117-124: Cleanup step does not actually remove staging artifacts.This step only logs messages; it does not delete or clean up images from the staging registry. If staging images should be retained temporarily (to allow re-runs or inspection) or cleaned up atomically by the next build, consider clarifying the intent in the comment or implementing actual cleanup logic (e.g., using
podman rmior a registry deletion API call).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/actions/build-okd/action.yaml(2 hunks)src/okd/build_images.sh(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/okd/build_images.sh
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-17T07:44:32.742Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 92
File: .github/workflows/release.yaml:44-50
Timestamp: 2025-10-17T07:44:32.742Z
Learning: When Podman builds an image without a registry prefix (e.g., `podman build -t microshift-okd`), it automatically adds the `localhost/` prefix and `:latest` tag, resulting in `localhost/microshift-okd:latest`. This means the Makefile in microshift-io/microshift building with `-t microshift-okd` produces `localhost/microshift-okd:latest` without explicit retagging.
Applied to files:
.github/actions/build-okd/action.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: isolated-network (ovnk, ubuntu-24.04, 1)
- GitHub Check: fedora-bootc (ubuntu-24.04-arm)
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
- GitHub Check: fedora-bootc (ubuntu-24.04)
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
- GitHub Check: centos10-bootc
- GitHub Check: centos9-bootc
- GitHub Check: ubuntu-rpm2deb
🔇 Additional comments (4)
.github/actions/build-okd/action.yaml (4)
61-73: Reference to staging registry in OKD_RELEASE_IMAGE is clear.The RPM build step correctly pulls from the staging registry path, which aligns with the two-stage workflow. The hardcoded staging registry path
ghcr.io/microshift-io/okd-stagingis appropriate for this intermediate step.
102-115: Push step withif: success()is correct.The conditional execution ensures OKD images are only promoted to production after all prior steps (build, RPM build, bootc build, and tests) succeed. The mode=
pushinvocation aligns with the expected re-tagging and production registry promotion.
22-24: Unable to verify due to repository access issues.The repository could not be cloned, preventing verification of whether the
target-registryinput is still used in workflows or the build step. Without access to the codebase, I cannot confirm if this input is referenced by downstream workflows, consumed in build steps, or truly obsolete.
48-59: Unfortunately, I'm unable to access the microshift-io/microshift repository to examine thebuild_images.shscript directly—the repository clone is failing consistently. Additionally, general web searches don't reveal the specific implementation details of this script's "build" mode.Given this constraint, I cannot definitively verify whether the "build" mode in
build_images.shactually performs a staging registry push or if the step name is misleading.Step name does not match invoked mode: "Build OKD images and push to staging" executes
build_images.sh build, notbuild_images.sh push.The step name suggests images will be pushed to staging, but the script is invoked with
buildmode only. Without access to the script, I cannot confirm whether:
- The
buildmode internally handles the staging push, making the step name accurate but obscure, or- The
buildmode only builds locally, making the step name incorrect.Recommendation: Verify the
build_images.shimplementation directly. Ifbuildmode includes staging push, clarify the step name (e.g., "Build OKD images and stage to registry"). Ifbuildmode does not push, either rename the step or restructure to explicitly invoke staging push logic.
kasturinarra
force-pushed
the
fix_issue_114
branch
from
7030690 to
c32164b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/actions/build-okd/action.yaml (1)
120-127: Cleanup step provides visibility but lacks action.The informational logging documents that staging images may be overwritten on the next build. While helpful for troubleshooting, consider whether explicit cleanup (registry deletion) is needed if images linger unexpectedly. Current approach is reasonable for a staging registry, but evaluate based on retention policies.
Is intentional overwriting of staging images on next build acceptable, or should orphaned staging images be explicitly purged? This may depend on your container registry's retention and quota policies.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/actions/build-okd/action.yaml(2 hunks)src/okd/build_images.sh(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 92
File: .github/workflows/release.yaml:44-50
Timestamp: 2025-10-17T07:44:32.742Z
Learning: When Podman builds an image without a registry prefix (e.g., `podman build -t microshift-okd`), it automatically adds the `localhost/` prefix and `:latest` tag, resulting in `localhost/microshift-okd:latest`. This means the Makefile in microshift-io/microshift building with `-t microshift-okd` produces `localhost/microshift-okd:latest` without explicit retagging.
📚 Learning: 2025-10-17T07:44:32.742Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 92
File: .github/workflows/release.yaml:44-50
Timestamp: 2025-10-17T07:44:32.742Z
Learning: When Podman builds an image without a registry prefix (e.g., `podman build -t microshift-okd`), it automatically adds the `localhost/` prefix and `:latest` tag, resulting in `localhost/microshift-okd:latest`. This means the Makefile in microshift-io/microshift building with `-t microshift-okd` produces `localhost/microshift-okd:latest` without explicit retagging.
Applied to files:
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh
📚 Learning: 2025-10-17T10:31:57.408Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 57
File: src/debian/install.sh:12-0
Timestamp: 2025-10-17T10:31:57.408Z
Learning: In the MicroShift project, the Ubuntu version in src/debian/install.sh is intentionally hardcoded to "xUbuntu_20.04" because CRI-O builds are not available for all the latest Ubuntu versions. This should not be changed to dynamic detection.
Applied to files:
.github/actions/build-okd/action.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: ubuntu-rpm2deb
- GitHub Check: centos10-bootc
- GitHub Check: fedora-bootc (ubuntu-24.04-arm)
- GitHub Check: fedora-bootc (ubuntu-24.04)
- GitHub Check: centos9-bootc
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: isolated-network (ovnk, ubuntu-24.04, 1)
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
🔇 Additional comments (10)
src/okd/build_images.sh (6)
7-11: Registry definitions now properly initialized.The PRODUCTION_REGISTRY and STAGING_REGISTRY are correctly defined with appropriate defaults and derivation. This resolves the previous undefined-variable issue.
335-348: Parameter parsing and mode validation are correct.The mandatory 4-parameter contract is enforced, and mode validation properly gates build vs. push execution. Implementation aligns with prior review feedback.
364-374: TARGET_REGISTRY derivation by MODE is sound.The conditional logic correctly routes to staging for build mode and production for push mode, enabling the staged promotion workflow.
399-413: build_okd_images correctly builds locally without pushing.The function builds all images and populates images_sha with local image references only, deferring push to push_okd_images. The haproxy-router skip (lines 404–406) correctly handles the arm64-only constraint with clear comments.
416-422: push_okd_images correctly orchestrates push and release creation.Delegates to push_image_manifests and create_new_okd_release, providing clear output messaging about the published image location.
425-433: build_images correctly chains build→push for staging workflow.Calls build_okd_images then push_okd_images in sequence, and outputs helpful instructions for the subsequent production push. Workflow intent is clear.
.github/actions/build-okd/action.yaml (4)
48-61: Build step correctly invokes build mode.The step runs the script with "build" mode and all required arguments. Comments clearly explain that build mode produces staging-tagged images and pushes to the staging registry.
63-76: RPM build correctly references staging OKD release image.Line 75 derives
OKD_RELEASE_IMAGEfrom the staging registry path (target-registry/okd-staging), ensuring MicroShift RPM build uses the locally-tested staging images. Comments document this intent clearly.
78-102: Tests execute against staging images, enabling validation before production push.The bootc build and test execution flow now runs against staging artifacts, satisfying the PR objective to validate OKD images before publishing to production.
105-118: Production push step correctly gates on success and invokes push mode.The step runs only when prior steps succeed (
if: success()), invokes the script in "push" mode with correct arguments, and includes instructive comments. This ensures images are promoted to production only after validation.
kasturinarra
force-pushed
the
fix_issue_114
branch
from
c32164b to
c592ac4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/okd/build_images.sh (1)
436-461: Critical bug in staging image path construction at line 446.The staging image derivation uses
${images[$key]#*/}which removes only the first path component. For a production image likeghcr.io/microshift-io/okd/cli:4.16-arm64, this yieldsmicroshift-io/okd/cli:4.16-arm64, resulting in a doubled path when prepended:ghcr.io/microshift-io/okd-staging/microshift-io/okd/cli:4.16-arm64. Thepodman image existscheck at line 449 will fail because the actual locally-built image is tagged asghcr.io/microshift-io/okd-staging/cli:4.16-arm64.Extract only the short name (image:tag) using
##*/(longest match) instead of#*/(shortest match).Apply this diff:
- staging_image="${STAGING_REGISTRY}/${images[$key]#*/}" + staging_image="${STAGING_REGISTRY}/${images[$key]##*/}"Alternatively, directly substitute the registry prefix:
- staging_image="${STAGING_REGISTRY}/${images[$key]#*/}" + staging_image="${images[$key]/${PRODUCTION_REGISTRY}/${STAGING_REGISTRY}}"
🧹 Nitpick comments (1)
.github/actions/build-okd/action.yaml (1)
120-127: Cleanup step is informational only; doesn't remove staging images.The step logs a message but doesn't actually delete or clean up staging images on failure. This means staging images remain in the registry if a build fails, and will be overwritten on the next successful build. Consider whether this aligns with your cleanup strategy, or if you need to add cleanup logic (e.g.,
podman rmior registry API calls) to remove orphaned staging images.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/actions/build-okd/action.yaml(2 hunks)src/okd/build_images.sh(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-17T07:44:32.742Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 92
File: .github/workflows/release.yaml:44-50
Timestamp: 2025-10-17T07:44:32.742Z
Learning: When Podman builds an image without a registry prefix (e.g., `podman build -t microshift-okd`), it automatically adds the `localhost/` prefix and `:latest` tag, resulting in `localhost/microshift-okd:latest`. This means the Makefile in microshift-io/microshift building with `-t microshift-okd` produces `localhost/microshift-okd:latest` without explicit retagging.
Applied to files:
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh
📚 Learning: 2025-10-17T10:31:57.408Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 57
File: src/debian/install.sh:12-0
Timestamp: 2025-10-17T10:31:57.408Z
Learning: In the MicroShift project, the Ubuntu version in src/debian/install.sh is intentionally hardcoded to "xUbuntu_20.04" because CRI-O builds are not available for all the latest Ubuntu versions. This should not be changed to dynamic detection.
Applied to files:
.github/actions/build-okd/action.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: fedora-bootc (ubuntu-24.04)
- GitHub Check: isolated-network (ovnk, ubuntu-24.04, 1)
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: fedora-bootc (ubuntu-24.04-arm)
- GitHub Check: ubuntu-rpm2deb
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
- GitHub Check: centos9-bootc
- GitHub Check: centos10-bootc
🔇 Additional comments (7)
src/okd/build_images.sh (4)
7-11: Registry configuration is well-structured. ✓The separation of
PRODUCTION_REGISTRY(from input) and derivedSTAGING_REGISTRYprovides flexibility and clarity.
335-348: Mode validation looks good. ✓Proper argument count check and mode validation logic.
364-371: Conditional TARGET_REGISTRY assignment is correct. ✓Cleanly branches registry selection based on mode.
399-413: Build function correctly populates images_sha with local image names.Skips haproxy-router for non-ARM64 as intended.
.github/actions/build-okd/action.yaml (3)
48-61: Build step correctly invokes build mode. ✓Passes the
buildmode argument and targets the staging registry via the script's derivedSTAGING_REGISTRYpath.
63-76: RPM build correctly references staging OKD release image.Line 75 hardcodes the staging path derivation (
/okd-staging/okd-release-...), which aligns with howbuild_images.shderivesSTAGING_REGISTRY = PRODUCTION_REGISTRY/okd-staging. This ensures RPM builds consume the locally-staged OKD images.
105-118: Push step conditionally executes only on test success. ✓The
if: success()gate ensures images are promoted to production only after passing the test suite. Correctly invokesbuild_images.shinpushmode with matching arguments.
kasturinarra
changed the title
kasturinarra
force-pushed
the
fix_issue_114
branch
2 times, most recently
from
7ed71e3 to
9ff24fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/okd/build_images.sh (2)
48-48: Fix step name to clarify sequence.Current step name "Build OKD images and push to staging" may mislead; the full workflow (build → test → push to production) is orchestrated by the action, not this step alone. Consider naming it "Build OKD images and push to staging registry" to match the action.yaml context, or accept as-is if clarity is not a concern.
404-405: Comment line reference is off by one.The comment "Skip haproxy-router for non-ARM64 architectures (see TODO at line 93)" refers to line 99 in the router_image() function where haproxy-router handling occurs. Update the reference for accuracy.
.github/actions/build-okd/action.yaml (1)
75-75: Include version tag in OKD_RELEASE_IMAGE for clarity and safety.The OKD_RELEASE_IMAGE variable is missing the version tag. While the script and build system may handle this (given OKD_VERSION_TAG is passed separately), it's safer and more explicit to include the full image reference with tag to avoid ambiguity.
Apply this diff:
make rpm \ USHIFT_GITREF="${{ inputs.ushift-gitref }}" \ OKD_VERSION_TAG="${{ inputs.okd-version-tag }}" \ - OKD_RELEASE_IMAGE="${{ inputs.target-registry }}/okd-staging/okd-release-${{ steps.detect-cpu-arch.outputs.go_arch }}" \ + OKD_RELEASE_IMAGE="${{ inputs.target-registry }}/okd-staging/okd-release-${{ steps.detect-cpu-arch.outputs.go_arch }}:${{ inputs.okd-version-tag }}" \ RPM_OUTDIR=/mnt/rpms
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/actions/build-okd/action.yaml(2 hunks)src/okd/build_images.sh(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 92
File: .github/workflows/release.yaml:44-50
Timestamp: 2025-10-17T07:44:32.742Z
Learning: When Podman builds an image without a registry prefix (e.g., `podman build -t microshift-okd`), it automatically adds the `localhost/` prefix and `:latest` tag, resulting in `localhost/microshift-okd:latest`. This means the Makefile in microshift-io/microshift building with `-t microshift-okd` produces `localhost/microshift-okd:latest` without explicit retagging.
📚 Learning: 2025-10-17T07:44:32.742Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 92
File: .github/workflows/release.yaml:44-50
Timestamp: 2025-10-17T07:44:32.742Z
Learning: When Podman builds an image without a registry prefix (e.g., `podman build -t microshift-okd`), it automatically adds the `localhost/` prefix and `:latest` tag, resulting in `localhost/microshift-okd:latest`. This means the Makefile in microshift-io/microshift building with `-t microshift-okd` produces `localhost/microshift-okd:latest` without explicit retagging.
Applied to files:
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh
📚 Learning: 2025-10-17T10:31:57.408Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 57
File: src/debian/install.sh:12-0
Timestamp: 2025-10-17T10:31:57.408Z
Learning: In the MicroShift project, the Ubuntu version in src/debian/install.sh is intentionally hardcoded to "xUbuntu_20.04" because CRI-O builds are not available for all the latest Ubuntu versions. This should not be changed to dynamic detection.
Applied to files:
.github/actions/build-okd/action.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: ubuntu-rpm2deb
- GitHub Check: fedora-bootc (ubuntu-24.04)
- GitHub Check: centos10-bootc
- GitHub Check: centos9-bootc
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
🔇 Additional comments (1)
.github/actions/build-okd/action.yaml (1)
105-118: Verify integration of staging-to-production promotion after tests.The push step correctly gates on
if: success()to ensure tests pass before pushing to production. The re-tagging flow in build_images.sh (push_images function) reconstructs staging image references and promotes them to production. Ensure that podman image storage persists across the build and push steps within the same job (it should, given single container).Can you confirm that podman image storage is shared across steps in this GitHub Actions job? If steps run in separate containers, the staging images built in the build step will not be available for re-tagging in the push step, and the logic will fail.
kasturinarra
force-pushed
the
fix_issue_114
branch
2 times, most recently
from
81253c9 to
89f64e2
Compare
|
Tip For best results, initiate chat on the files or code changes. @kasturinarra, hello! How can I assist you with this PR? I can:
What would you like me to do? |
Review the changes |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
kasturinarra
force-pushed
the
fix_issue_114
branch
from
28aef82 to
6884436
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/release-okd.yaml (1)
54-77: Use consistent gitref for cleanup job.Line 72 hardcodes
ushift-gitref: mainwhile the build job respectsenv.USHIFT_GITREF. For consistency and to ensure the cleanup script matches the build script version when users override the gitref viaworkflow_dispatch, consider using the same environment variable.🔎 Proposed fix
- name: Run cleanup of staging OKD images uses: ./.github/actions/build-okd with: - ushift-gitref: main + ushift-gitref: ${{ env.USHIFT_GITREF }} okd-version-tag: ${{ env.OKD_VERSION_TAG != 'latest' && env.OKD_VERSION_TAG || steps.detect-okd-version.outputs.okd-version-tag }} target-arch: arm64 target-registry: ${{ env.OKD_TARGET_REGISTRY }} token: ${{ secrets.GITHUB_TOKEN }} cleanup-staging: 'true'
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/actions/build-okd/action.yaml.github/workflows/release-okd.yamlMakefilesrc/okd/build_images.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- src/okd/build_images.sh
🧰 Additional context used
📓 Path-based instructions (2)
Makefile
📄 CodeRabbit inference engine (CLAUDE.md)
Build system follows three sequential stages: SRPM Build (
make srpm), RPM Build (make rpm), and Bootc Image (make image). Each stage must be completed before the next can be executed.
Files:
Makefile
.github/workflows/*.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
CI/CD workflows should validate builds with
builders.yaml, test quickstart scripts withinstallers.yaml, support manual release workflow viarelease.yaml, and run daily OKD ARM builds viarelease-okd.yaml
Files:
.github/workflows/release-okd.yaml
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : SRPM build container must clone MicroShift from upstream repository at `USHIFT_GITREF` and replace component images with OKD references via `src/image/prebuild.sh`
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to src/image/prebuild.sh : Replace component images with OKD references during SRPM build stage
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to .github/workflows/*.yaml : CI/CD workflows should validate builds with `builders.yaml`, test quickstart scripts with `installers.yaml`, support manual release workflow via `release.yaml`, and run daily OKD ARM builds via `release-okd.yaml`
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : RPM build container requires SRPM image from previous stage and outputs to `RPM_OUTDIR`
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : Bootc image build requires RPM image from previous stage, is configurable via `WITH_KINDNET`, `WITH_TOPOLVM`, `WITH_OLM`, and `EMBED_CONTAINER_IMAGES`, and is based on `BOOTC_IMAGE_URL:BOOTC_IMAGE_TAG`
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to src/okd/** : OKD version auto-detection should query latest-amd64 or latest-arm64 tags based on detected architecture
Applied to files:
Makefile.github/workflows/release-okd.yaml.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-22T06:35:50.684Z
Learnt from: kasturinarra
Repo: microshift-io/microshift PR: 162
File: src/cncf/run_sonobuoy_tests.sh:43-45
Timestamp: 2025-12-22T06:35:50.684Z
Learning: In src/cncf/run_sonobuoy_tests.sh, architecture detection via `uname -m` is not needed because all container images (docker.io/sonobuoy/sonobuoy, docker.io/sonobuoy/systemd-logs, registry.k8s.io/conformance) are multi-arch manifests that automatically pull the correct architecture variant for the host platform.
Applied to files:
Makefile.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to src/image/prebuild.sh : Replace component images with OKD references during SRPM build stage
Applied to files:
Makefile.github/workflows/release-okd.yaml.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to Makefile : Build system follows three sequential stages: SRPM Build (`make srpm`), RPM Build (`make rpm`), and Bootc Image (`make image`). Each stage must be completed before the next can be executed.
Applied to files:
Makefile.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : SRPM build container must clone MicroShift from upstream repository at `USHIFT_GITREF` and replace component images with OKD references via `src/image/prebuild.sh`
Applied to files:
Makefile.github/workflows/release-okd.yaml.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : Bootc image build requires RPM image from previous stage, is configurable via `WITH_KINDNET`, `WITH_TOPOLVM`, `WITH_OLM`, and `EMBED_CONTAINER_IMAGES`, and is based on `BOOTC_IMAGE_URL:BOOTC_IMAGE_TAG`
Applied to files:
Makefile.github/actions/build-okd/action.yaml
📚 Learning: 2025-10-17T07:44:32.742Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 92
File: .github/workflows/release.yaml:44-50
Timestamp: 2025-10-17T07:44:32.742Z
Learning: When Podman builds an image without a registry prefix (e.g., `podman build -t microshift-okd`), it automatically adds the `localhost/` prefix and `:latest` tag, resulting in `localhost/microshift-okd:latest`. This means the Makefile in microshift-io/microshift building with `-t microshift-okd` produces `localhost/microshift-okd:latest` without explicit retagging.
Applied to files:
Makefile.github/workflows/release-okd.yaml.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : RPM build container requires SRPM image from previous stage and outputs to `RPM_OUTDIR`
Applied to files:
Makefile.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to .github/workflows/*.yaml : CI/CD workflows should validate builds with `builders.yaml`, test quickstart scripts with `installers.yaml`, support manual release workflow via `release.yaml`, and run daily OKD ARM builds via `release-okd.yaml`
Applied to files:
.github/workflows/release-okd.yaml.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Always build SRPM before RPM, and RPM before bootc image to maintain dependency chain
Applied to files:
.github/actions/build-okd/action.yaml
📚 Learning: 2025-10-17T10:31:57.408Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 57
File: src/debian/install.sh:12-0
Timestamp: 2025-10-17T10:31:57.408Z
Learning: In the MicroShift project, the Ubuntu version in src/debian/install.sh is intentionally hardcoded to "xUbuntu_20.04" because CRI-O builds are not available for all the latest Ubuntu versions. This should not be changed to dynamic detection.
Applied to files:
.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to **/*.sh : Build scripts must detect architecture automatically via `uname -m` to determine between x86_64 or aarch64
Applied to files:
.github/actions/build-okd/action.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: fedora-bootc (ubuntu-24.04)
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: centos10-bootc
- GitHub Check: isolated-network (ovnk, ubuntu-24.04, 1)
- GitHub Check: centos9-bootc
- GitHub Check: fedora-bootc (ubuntu-24.04-arm)
- GitHub Check: ubuntu-rpm2deb
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04-arm)
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04)
🔇 Additional comments (6)
Makefile (1)
32-34: LGTM! Architecture-specific release image variables.The refactoring from a single
OKD_RELEASE_IMAGEto architecture-specific variables (OKD_RELEASE_IMAGE_X86_64,OKD_RELEASE_IMAGE_AARCH64) correctly supports the per-mode staging/production workflow and aligns with the override logic in the build action..github/actions/build-okd/action.yaml (5)
28-31: LGTM! Cleanup-staging input enables workflow gating.The new input correctly gates cleanup vs build/test/push paths throughout the action.
62-69: LGTM! Staging mode builds and pushes to staging registry.The
stagingmode correctly builds images locally and pushes them to the automatically-derived staging registry for testing before production promotion.
71-95: LGTM! Staging registry derivation and architecture-specific overrides.The staging registry derivation using
dirnameand the architecture-conditional override logic correctly ensure RPMs are built using staged OKD images. The variable expansion in themake rpminvocation is valid.
126-139: LGTM! Production push gated on test success.The conditional
success() && inputs.cleanup-staging != 'true'correctly ensures OKD images are pushed to production only after successful validation, implementing the PR's core objective.
141-174: LGTM! Cleanup logic with tolerant error handling.The cleanup step correctly retrieves the package list from the build script (avoiding knowledge duplication), uses exit-code-based deletion checks, and applies tolerant error handling with
continue-on-error: true.
kasturinarra
force-pushed
the
fix_issue_114
branch
from
6884436 to
ffcb1e9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/okd/build_images.sh (1)
417-459: Duplicate image key list creates sync risk.
list_packages()defines its ownimage_keysarray that must stay synchronized with theimagesassociative array at line 520. The warning comments help, but a single source of truth would be safer.Consider extracting keys from a shared definition or generating list_packages output dynamically (e.g., from a config file both locations source).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/actions/build-okd/action.yaml.github/workflows/release-okd.yamlMakefilesrc/okd/build_images.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/release-okd.yaml
- Makefile
🧰 Additional context used
📓 Path-based instructions (2)
**/*.sh
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.sh: Shell scripts should pass linting with shellcheck as part of themake checkvalidation step
Build scripts must detect architecture automatically viauname -mto determine between x86_64 or aarch64
Files:
src/okd/build_images.sh
src/okd/**
📄 CodeRabbit inference engine (CLAUDE.md)
OKD version auto-detection should query latest-amd64 or latest-arm64 tags based on detected architecture
Files:
src/okd/build_images.sh
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : SRPM build container must clone MicroShift from upstream repository at `USHIFT_GITREF` and replace component images with OKD references via `src/image/prebuild.sh`
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to src/image/prebuild.sh : Replace component images with OKD references during SRPM build stage
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to .github/workflows/*.yaml : CI/CD workflows should validate builds with `builders.yaml`, test quickstart scripts with `installers.yaml`, support manual release workflow via `release.yaml`, and run daily OKD ARM builds via `release-okd.yaml`
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : Bootc image build requires RPM image from previous stage, is configurable via `WITH_KINDNET`, `WITH_TOPOLVM`, `WITH_OLM`, and `EMBED_CONTAINER_IMAGES`, and is based on `BOOTC_IMAGE_URL:BOOTC_IMAGE_TAG`
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to src/image/prebuild.sh : Replace component images with OKD references during SRPM build stage
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : SRPM build container must clone MicroShift from upstream repository at `USHIFT_GITREF` and replace component images with OKD references via `src/image/prebuild.sh`
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : Bootc image build requires RPM image from previous stage, is configurable via `WITH_KINDNET`, `WITH_TOPOLVM`, `WITH_OLM`, and `EMBED_CONTAINER_IMAGES`, and is based on `BOOTC_IMAGE_URL:BOOTC_IMAGE_TAG`
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml
📚 Learning: 2025-10-17T07:44:32.742Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 92
File: .github/workflows/release.yaml:44-50
Timestamp: 2025-10-17T07:44:32.742Z
Learning: When Podman builds an image without a registry prefix (e.g., `podman build -t microshift-okd`), it automatically adds the `localhost/` prefix and `:latest` tag, resulting in `localhost/microshift-okd:latest`. This means the Makefile in microshift-io/microshift building with `-t microshift-okd` produces `localhost/microshift-okd:latest` without explicit retagging.
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to src/okd/** : OKD version auto-detection should query latest-amd64 or latest-arm64 tags based on detected architecture
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : RPM build container requires SRPM image from previous stage and outputs to `RPM_OUTDIR`
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to **/*.sh : Build scripts must detect architecture automatically via `uname -m` to determine between x86_64 or aarch64
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-22T06:35:50.684Z
Learnt from: kasturinarra
Repo: microshift-io/microshift PR: 162
File: src/cncf/run_sonobuoy_tests.sh:43-45
Timestamp: 2025-12-22T06:35:50.684Z
Learning: In src/cncf/run_sonobuoy_tests.sh, architecture detection via `uname -m` is not needed because all container images (docker.io/sonobuoy/sonobuoy, docker.io/sonobuoy/systemd-logs, registry.k8s.io/conformance) are multi-arch manifests that automatically pull the correct architecture variant for the host platform.
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to .github/workflows/*.yaml : CI/CD workflows should validate builds with `builders.yaml`, test quickstart scripts with `installers.yaml`, support manual release workflow via `release.yaml`, and run daily OKD ARM builds via `release-okd.yaml`
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml
📚 Learning: 2025-10-17T10:31:57.408Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 57
File: src/debian/install.sh:12-0
Timestamp: 2025-10-17T10:31:57.408Z
Learning: In the MicroShift project, the Ubuntu version in src/debian/install.sh is intentionally hardcoded to "xUbuntu_20.04" because CRI-O builds are not available for all the latest Ubuntu versions. This should not be changed to dynamic detection.
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Always build SRPM before RPM, and RPM before bootc image to maintain dependency chain
Applied to files:
.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to Makefile : Build system follows three sequential stages: SRPM Build (`make srpm`), RPM Build (`make rpm`), and Bootc Image (`make image`). Each stage must be completed before the next can be executed.
Applied to files:
.github/actions/build-okd/action.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: fedora-bootc (ubuntu-24.04)
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: isolated-network (ovnk, ubuntu-24.04, 1)
- GitHub Check: ubuntu-rpm2deb
- GitHub Check: centos9-bootc
- GitHub Check: fedora-bootc (ubuntu-24.04-arm)
- GitHub Check: centos10-bootc
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04-arm)
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
🔇 Additional comments (9)
.github/actions/build-okd/action.yaml (4)
28-31: Clean input definition for cleanup-staging mode.Well-structured input with clear description and sensible default.
55-69: Staging build step correctly configured.The mode is now
stagingand comments clearly explain the registry derivation logic.
71-95: RPM build step properly derives staging registry and sets architecture-specific override.The logic correctly handles both arm64 and amd64 cases by setting the appropriate
OKD_RELEASE_IMAGE_*variable.
126-139: Production push gated on success.The
success()condition ensures OKD images only reach production after tests pass—this implements the core objective of Issue #114.src/okd/build_images.sh (5)
7-12: Registry derivation looks correct.STAGING_REGISTRY properly derived from PRODUCTION_REGISTRY using
dirname, addressing earlier review feedback.
341-365: Well-factored build and push functions.Clean separation between building images locally (
build_okd_images) and pushing to registry (push_okd_images).
367-394: Retagging logic correctly maps staging → production.The string substitution
${production_image/${PRODUCTION_REGISTRY}/${STAGING_REGISTRY}}properly constructs staging references for retagging.
486-488: Architecture passed as parameter rather than auto-detected.Per coding guidelines, build scripts should detect architecture via
uname -m. This script requiresTARGET_ARCHas a parameter. In a CI context with explicit workflow control, this is likely intentional.Confirm this design choice is intentional given the workflow explicitly sets
target-arch: arm64. If local developer usage is expected, consider addinguname -mfallback:-TARGET_ARCH="$4" +TARGET_ARCH="${4:-$(uname -m | sed 's/x86_64/amd64/' | sed 's/aarch64/arm64/')}"Based on coding guidelines, build scripts must detect architecture automatically via
uname -m.
540-547: Mode dispatch is clean and readable.Final execution block properly routes to mode-specific handlers after validation.
kasturinarra
force-pushed
the
fix_issue_114
branch
from
ffcb1e9 to
f077749
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/release-okd.yaml (1)
55-78: Consider reusing OKD version from build job.The cleanup job re-runs version detection (lines 64-68) instead of consuming the version from
build-okd-release. If the build job fails before version detection completes, the cleanup job may detect a different version.🔎 Optional refactor to share version
Add an output to the build job:
build-okd-release: name: Build OKD release images for ARM if: github.event_name != 'schedule' || github.repository == 'microshift-io/microshift' runs-on: ubuntu-24.04-arm + outputs: + okd-version-tag: ${{ steps.detect-okd-version.outputs.okd-version-tag }} steps:Then reference it in cleanup:
- name: Run cleanup of staging OKD images uses: ./.github/actions/build-okd with: ushift-gitref: ${{ env.USHIFT_GITREF }} - okd-version-tag: ${{ env.OKD_VERSION_TAG != 'latest' && env.OKD_VERSION_TAG || steps.detect-okd-version.outputs.okd-version-tag }} + okd-version-tag: ${{ needs.build-okd-release.outputs.okd-version-tag }} target-arch: arm64src/okd/build_images.sh (1)
417-459: Manual sync required between image_keys and images array.Lines 419-422 warn that
image_keysmust be kept synchronized with theimagesarray (line ~520). This manual maintenance is error-prone.💡 Optional: Dynamic package listing
Consider deriving package names directly from the
imagesarray to eliminate manual sync:list_packages() { local packages=() # Derive from images array keys for key in "${!images[@]}"; do # Extract package name from image reference local image="${images[$key]}" local package="${image#${STAGING_REGISTRY}/}" package="${package%%:*}" # Remove tag packages+=("okd-staging/${package}") done # Add release images packages+=( "okd-staging/okd-release-arm64" "okd-staging/okd-release-amd64" ) # Deduplicate and output printf '%s\n' "${packages[@]}" | sort -u }This would require declaring the
imagesarray before the function, or refactoring argument handling.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/actions/build-okd/action.yaml.github/workflows/release-okd.yamlMakefilesrc/okd/build_images.sh
🧰 Additional context used
📓 Path-based instructions (4)
**/*.sh
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.sh: Shell scripts should pass linting with shellcheck as part of themake checkvalidation step
Build scripts must detect architecture automatically viauname -mto determine between x86_64 or aarch64
Files:
src/okd/build_images.sh
src/okd/**
📄 CodeRabbit inference engine (CLAUDE.md)
OKD version auto-detection should query latest-amd64 or latest-arm64 tags based on detected architecture
Files:
src/okd/build_images.sh
.github/workflows/*.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
CI/CD workflows should validate builds with
builders.yaml, test quickstart scripts withinstallers.yaml, support manual release workflow viarelease.yaml, and run daily OKD ARM builds viarelease-okd.yaml
Files:
.github/workflows/release-okd.yaml
Makefile
📄 CodeRabbit inference engine (CLAUDE.md)
Build system follows three sequential stages: SRPM Build (
make srpm), RPM Build (make rpm), and Bootc Image (make image). Each stage must be completed before the next can be executed.
Files:
Makefile
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : SRPM build container must clone MicroShift from upstream repository at `USHIFT_GITREF` and replace component images with OKD references via `src/image/prebuild.sh`
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to src/image/prebuild.sh : Replace component images with OKD references during SRPM build stage
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : Bootc image build requires RPM image from previous stage, is configurable via `WITH_KINDNET`, `WITH_TOPOLVM`, `WITH_OLM`, and `EMBED_CONTAINER_IMAGES`, and is based on `BOOTC_IMAGE_URL:BOOTC_IMAGE_TAG`
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : RPM build container requires SRPM image from previous stage and outputs to `RPM_OUTDIR`
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to src/image/prebuild.sh : Replace component images with OKD references during SRPM build stage
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml.github/workflows/release-okd.yamlMakefile
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : SRPM build container must clone MicroShift from upstream repository at `USHIFT_GITREF` and replace component images with OKD references via `src/image/prebuild.sh`
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml.github/workflows/release-okd.yamlMakefile
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : Bootc image build requires RPM image from previous stage, is configurable via `WITH_KINDNET`, `WITH_TOPOLVM`, `WITH_OLM`, and `EMBED_CONTAINER_IMAGES`, and is based on `BOOTC_IMAGE_URL:BOOTC_IMAGE_TAG`
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yamlMakefile
📚 Learning: 2025-10-17T07:44:32.742Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 92
File: .github/workflows/release.yaml:44-50
Timestamp: 2025-10-17T07:44:32.742Z
Learning: When Podman builds an image without a registry prefix (e.g., `podman build -t microshift-okd`), it automatically adds the `localhost/` prefix and `:latest` tag, resulting in `localhost/microshift-okd:latest`. This means the Makefile in microshift-io/microshift building with `-t microshift-okd` produces `localhost/microshift-okd:latest` without explicit retagging.
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml.github/workflows/release-okd.yamlMakefile
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to src/okd/** : OKD version auto-detection should query latest-amd64 or latest-arm64 tags based on detected architecture
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml.github/workflows/release-okd.yamlMakefile
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : RPM build container requires SRPM image from previous stage and outputs to `RPM_OUTDIR`
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yamlMakefile
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to **/*.sh : Build scripts must detect architecture automatically via `uname -m` to determine between x86_64 or aarch64
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-22T06:35:50.684Z
Learnt from: kasturinarra
Repo: microshift-io/microshift PR: 162
File: src/cncf/run_sonobuoy_tests.sh:43-45
Timestamp: 2025-12-22T06:35:50.684Z
Learning: In src/cncf/run_sonobuoy_tests.sh, architecture detection via `uname -m` is not needed because all container images (docker.io/sonobuoy/sonobuoy, docker.io/sonobuoy/systemd-logs, registry.k8s.io/conformance) are multi-arch manifests that automatically pull the correct architecture variant for the host platform.
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yamlMakefile
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to .github/workflows/*.yaml : CI/CD workflows should validate builds with `builders.yaml`, test quickstart scripts with `installers.yaml`, support manual release workflow via `release.yaml`, and run daily OKD ARM builds via `release-okd.yaml`
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml.github/workflows/release-okd.yaml
📚 Learning: 2025-10-17T10:31:57.408Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 57
File: src/debian/install.sh:12-0
Timestamp: 2025-10-17T10:31:57.408Z
Learning: In the MicroShift project, the Ubuntu version in src/debian/install.sh is intentionally hardcoded to "xUbuntu_20.04" because CRI-O builds are not available for all the latest Ubuntu versions. This should not be changed to dynamic detection.
Applied to files:
src/okd/build_images.sh.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Always build SRPM before RPM, and RPM before bootc image to maintain dependency chain
Applied to files:
.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to Makefile : Build system follows three sequential stages: SRPM Build (`make srpm`), RPM Build (`make rpm`), and Bootc Image (`make image`). Each stage must be completed before the next can be executed.
Applied to files:
.github/actions/build-okd/action.yamlMakefile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04-arm)
- GitHub Check: centos9-bootc
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04)
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: centos10-bootc
- GitHub Check: fedora-bootc (ubuntu-24.04-arm)
- GitHub Check: ubuntu-rpm2deb
🔇 Additional comments (13)
Makefile (2)
32-34: LGTM: Architecture-specific release image variables.The split into
OKD_RELEASE_IMAGE_X86_64andOKD_RELEASE_IMAGE_AARCH64enables per-architecture registry overrides (staging vs production) and aligns with the PR's goal of separating build and push phases.
89-90: Architecture-specific OKD release variables correctly passed and consumed.Both
OKD_RELEASE_IMAGE_X86_64andOKD_RELEASE_IMAGE_AARCH64are declared inpackaging/srpm.Containerfileand used in architecture-specificRUNcommands with the prebuild script..github/actions/build-okd/action.yaml (5)
28-31: LGTM: Cleanup-staging input definition.The new input enables conditional cleanup mode while preserving normal build/test/push flow by default.
55-69: LGTM: Build step invokes staging mode.The step correctly invokes
stagingmode which builds locally and pushes to the derived staging registry. Gating prevents execution during cleanup.
71-95: LGTM: RPM build uses staging images.The step correctly derives the staging registry and overrides the appropriate architecture-specific variable to point RPM builds at staged OKD images for validation.
126-139: LGTM: Production push gated on success.The production push only executes after all tests pass and is correctly skipped during cleanup mode. This implements the desired build → test → push flow.
141-184: Verify list-packages mode implementation.The cleanup step invokes
list-packagesmode on line 167. Ensure the script implements this mode and outputs one package name per line.src/okd/build_images.sh (6)
7-11: LGTM: Automatic staging registry derivation.The staging registry is automatically derived from the production registry, ensuring consistency and eliminating hardcoded paths. The derivation logic matches the action.yaml implementation.
17-37: LGTM: Clear mode-based usage.The usage documentation clearly explains the three modes and their argument requirements, making the script's new invocation pattern evident.
341-365: LGTM: Focused build and push functions.The extracted functions separate concerns clearly:
build_okd_imageshandles local image creation, whilepush_okd_imageshandles registry operations. The haproxy-router skip logic is documented.
367-394: LGTM: Retag function correctly maps staging to production.The function correctly substitutes registry paths to locate staging images locally, then retags them for production push. Error handling guides users to run staging mode first.
396-415: LGTM: Mode-specific workflows.The
push_stagingandpush_productionfunctions implement the distinct workflows for each mode, with appropriate checks and helpful output guiding the next steps.
464-547: Mode dispatch logic is correct.The main execution correctly handles the three modes, validates arguments, sets the appropriate target registry, and dispatches to the corresponding workflow function.
kasturinarra
force-pushed
the
fix_issue_114
branch
from
4c6f780 to
8aa61a4
Compare
please review again. |
kasturinarra
force-pushed
the
fix_issue_114
branch
from
ab36d22 to
6445306
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/okd/build_images.sh (1)
417-459: Consider deriving image_keys from images array.The manual synchronization between
image_keys(line 423) and theimagesarray (line 523) creates maintenance risk. If a new image is added to one location but not the other, cleanup will silently fail for that package.🔎 Dynamic derivation approach
Extract keys dynamically in
list_packages:list_packages() { - # WARNING: Keep this list synchronized with the 'images' array at line ~520 - # When adding a new image to the build, add its key to BOTH locations: - # 1. Here (image_keys array) - # 2. Line ~520 (images associative array) - local image_keys=( - "base" - "cli" - "haproxy-router-base" - "haproxy-router" - "kube-proxy" - "coredns" - "csi-snapshot-controller" - "kube-rbac-proxy" - "pod" - "service-ca-operator" - "operator-lifecycle-manager" - "operator-registry" - ) - local packages=() - # Derive package names from image keys - for key in "${image_keys[@]}"; do + # Derive package names from images array keys + for key in "${!images[@]}"; do if [[ "${key}" == "base" ]]; then # base image maps to scos-${OKD_VERSION} packages+=("okd-staging/scos-${OKD_VERSION}")This eliminates the synchronization requirement entirely.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/actions/build-okd/action.yaml.github/workflows/release-okd.yamlMakefilesrc/okd/build_images.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
🧰 Additional context used
📓 Path-based instructions (3)
**/*.sh
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.sh: Shell scripts should pass linting with shellcheck as part of themake checkvalidation step
Build scripts must detect architecture automatically viauname -mto determine between x86_64 or aarch64
Files:
src/okd/build_images.sh
src/okd/**
📄 CodeRabbit inference engine (CLAUDE.md)
OKD version auto-detection should query latest-amd64 or latest-arm64 tags based on detected architecture
Files:
src/okd/build_images.sh
.github/workflows/*.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
CI/CD workflows should validate builds with
builders.yaml, test quickstart scripts withinstallers.yaml, support manual release workflow viarelease.yaml, and run daily OKD ARM builds viarelease-okd.yaml
Files:
.github/workflows/release-okd.yaml
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : SRPM build container must clone MicroShift from upstream repository at `USHIFT_GITREF` and replace component images with OKD references via `src/image/prebuild.sh`
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to .github/workflows/*.yaml : CI/CD workflows should validate builds with `builders.yaml`, test quickstart scripts with `installers.yaml`, support manual release workflow via `release.yaml`, and run daily OKD ARM builds via `release-okd.yaml`
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to src/image/prebuild.sh : Replace component images with OKD references during SRPM build stage
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : Bootc image build requires RPM image from previous stage, is configurable via `WITH_KINDNET`, `WITH_TOPOLVM`, `WITH_OLM`, and `EMBED_CONTAINER_IMAGES`, and is based on `BOOTC_IMAGE_URL:BOOTC_IMAGE_TAG`
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : RPM build container requires SRPM image from previous stage and outputs to `RPM_OUTDIR`
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to .github/workflows/*.yaml : CI/CD workflows should validate builds with `builders.yaml`, test quickstart scripts with `installers.yaml`, support manual release workflow via `release.yaml`, and run daily OKD ARM builds via `release-okd.yaml`
Applied to files:
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh.github/workflows/release-okd.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to src/image/prebuild.sh : Replace component images with OKD references during SRPM build stage
Applied to files:
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh.github/workflows/release-okd.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : RPM build container requires SRPM image from previous stage and outputs to `RPM_OUTDIR`
Applied to files:
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : SRPM build container must clone MicroShift from upstream repository at `USHIFT_GITREF` and replace component images with OKD references via `src/image/prebuild.sh`
Applied to files:
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh.github/workflows/release-okd.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to src/okd/** : OKD version auto-detection should query latest-amd64 or latest-arm64 tags based on detected architecture
Applied to files:
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh.github/workflows/release-okd.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to packaging/Containerfile* : Bootc image build requires RPM image from previous stage, is configurable via `WITH_KINDNET`, `WITH_TOPOLVM`, `WITH_OLM`, and `EMBED_CONTAINER_IMAGES`, and is based on `BOOTC_IMAGE_URL:BOOTC_IMAGE_TAG`
Applied to files:
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Always build SRPM before RPM, and RPM before bootc image to maintain dependency chain
Applied to files:
.github/actions/build-okd/action.yaml
📚 Learning: 2025-10-17T07:44:32.742Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 92
File: .github/workflows/release.yaml:44-50
Timestamp: 2025-10-17T07:44:32.742Z
Learning: When Podman builds an image without a registry prefix (e.g., `podman build -t microshift-okd`), it automatically adds the `localhost/` prefix and `:latest` tag, resulting in `localhost/microshift-okd:latest`. This means the Makefile in microshift-io/microshift building with `-t microshift-okd` produces `localhost/microshift-okd:latest` without explicit retagging.
Applied to files:
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh.github/workflows/release-okd.yaml
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to Makefile : Build system follows three sequential stages: SRPM Build (`make srpm`), RPM Build (`make rpm`), and Bootc Image (`make image`). Each stage must be completed before the next can be executed.
Applied to files:
.github/actions/build-okd/action.yaml
📚 Learning: 2025-10-17T10:31:57.408Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 57
File: src/debian/install.sh:12-0
Timestamp: 2025-10-17T10:31:57.408Z
Learning: In the MicroShift project, the Ubuntu version in src/debian/install.sh is intentionally hardcoded to "xUbuntu_20.04" because CRI-O builds are not available for all the latest Ubuntu versions. This should not be changed to dynamic detection.
Applied to files:
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh
📚 Learning: 2025-12-22T06:35:50.684Z
Learnt from: kasturinarra
Repo: microshift-io/microshift PR: 162
File: src/cncf/run_sonobuoy_tests.sh:43-45
Timestamp: 2025-12-22T06:35:50.684Z
Learning: In src/cncf/run_sonobuoy_tests.sh, architecture detection via `uname -m` is not needed because all container images (docker.io/sonobuoy/sonobuoy, docker.io/sonobuoy/systemd-logs, registry.k8s.io/conformance) are multi-arch manifests that automatically pull the correct architecture variant for the host platform.
Applied to files:
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to **/*.sh : Build scripts must detect architecture automatically via `uname -m` to determine between x86_64 or aarch64
Applied to files:
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: fedora-bootc (ubuntu-24.04-arm)
- GitHub Check: centos9-bootc
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: ubuntu-rpm2deb
- GitHub Check: fedora-bootc (ubuntu-24.04)
- GitHub Check: centos10-bootc
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04)
- GitHub Check: isolated-network (ovnk, ubuntu-24.04, 1)
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04-arm)
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
🔇 Additional comments (10)
.github/actions/build-okd/action.yaml (4)
28-31: LGTM: Clean separation of concerns.The
cleanup-staginginput elegantly isolates cleanup operations from the main build/test/push flow.
71-95: Correct registry derivation and architecture handling.The staging registry computation and arch-specific overrides properly support both ARM64 and X86_64, maintaining action reusability despite the current ARM-only workflow.
126-139: Proper production gate.The
success()conditional correctly prevents publishing untested OKD images, fulfilling the PR objective.
141-184: Robust cleanup with proper endpoint detection.The org/user detection via API probe and endpoint-appropriate deletion handle both account types correctly. Tolerant error handling prevents cleanup failures from blocking the workflow.
.github/workflows/release-okd.yaml (2)
33-59: Excellent version consistency solution.Exporting the resolved OKD version as a job output prevents build/cleanup mismatches and addresses the concern from past reviews. The comments clearly document the intent.
70-87: Correct cleanup job orchestration.The
success() || failure()conditional ensures staging images are always cleaned up, and the version reuse guarantees cleanup targets the correct staging packages.src/okd/build_images.sh (4)
7-12: Clean registry derivation.The staging registry computation correctly replaces the final path component, allowing flexible TARGET_REGISTRY overrides while maintaining staging/production separation.
17-37: Clear mode documentation.The updated usage text effectively communicates the three-mode workflow and staging→production progression.
341-415: Well-structured mode functions.The extracted functions improve readability and maintain clear separation between build, retag, and push operations. The staging image existence check at lines 384-388 provides helpful error messages.
464-547: Clean mode dispatch and validation.The argument parsing and mode-specific execution flow are well-structured. The explicit TARGET_ARCH parameter is appropriate for CI-driven workflows where architecture is predetermined.
kasturinarra
force-pushed
the
fix_issue_114
branch
from
6445306 to
a069e60
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
kasturinarra
force-pushed
the
fix_issue_114
branch
from
4f4d509 to
10c9a68
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/actions/cleanup-okd/action.yaml:
- Around line 38-40: The step invoking the build script uses an unquoted
${GITHUB_WORKSPACE}, which will break on paths with spaces; update the command
that changes directory and the mapfile invocation to reference
"${GITHUB_WORKSPACE}" (and similarly quote any other variable expansions like
"${{ inputs.okd-version-tag }}" if needed) so cd and the process substitution
work correctly; locate the lines using cd ${GITHUB_WORKSPACE}/ and mapfile -t
packages < <(./src/okd/build_images.sh list-packages "${{ inputs.okd-version-tag
}}") and add quotes around GITHUB_WORKSPACE (and other path variables) to ensure
safe handling of spaces.
In @.github/workflows/release-okd.yaml:
- Around line 71-84: The cleanup-staging job may run with an empty
okd-version-tag when build-okd-release is skipped; update the workflow to skip
the cleanup when needs.build-okd-release.outputs.okd-version-tag is empty by
adding a guard (e.g., change the job or the cleanup step's if to also require
needs.build-okd-release.outputs.okd-version-tag != ''), or add a preliminary
step in the cleanup-staging job (calling the action
./.github/actions/cleanup-okd) that checks okd-version-tag and exits/returns
early if empty so the action does not invoke build_images.sh list-packages "".
🧹 Nitpick comments (3)
src/okd/build_images.sh (2)
421-445:list_packagesoutputs hardcoded release images that may not reflect actual builds.The function adds both
okd-release-arm64andokd-release-amd64(lines 438-441) regardless of what was actually built. If only arm64 was built, attempting to delete a non-existent amd64 package wastes API calls (though harmless due to error handling).Also,
haproxy-router-basewill be listed but isn't a separate package in staging - it's an intermediate build artifact that may not be pushed.♻️ Consider filtering based on TARGET_ARCH
# Add release images for both architectures - packages+=( - "okd-staging/okd-release-arm64" - "okd-staging/okd-release-amd64" - ) + # Only add the release image for the target architecture + packages+=("okd-staging/okd-release-${TARGET_ARCH}")
456-465:list-packagesmode hardcodesarm64architecture.Line 464 hardcodes
TARGET_ARCH="arm64"for list-packages mode. If amd64 builds are added later, this won't list the correct packages. Consider making it a parameter or documenting this limitation..github/actions/cleanup-okd/action.yaml (1)
15-21:continue-on-errorcombined withset -euo pipefailmay cause partial cleanup.If a command fails mid-script (e.g., network issue),
pipefailexits immediately, skipping remaining packages. The step'scontinue-on-error: trueprevents workflow failure but doesn't retry.Consider removing
set -eor handling errors explicitly per-command to ensure all packages are attempted.♻️ Suggested: remove -e for resilient cleanup
run: | - set -euo pipefail + set -uo pipefail
kasturinarra
force-pushed
the
fix_issue_114
branch
from
10c9a68 to
7c7b84e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/okd/build_images.sh (1)
450-534: Add auto-detection fallback for arch and OKD version.
Line 469 enforces exact args; this script should default touname -mand to the latest OKD tag when callers omit these values.Proposed fix
- if [[ $# -ne 4 ]]; then + if [[ $# -lt 3 || $# -gt 4 ]]; then usage fi - OKD_VERSION="$2" - OCP_BRANCH="$3" - TARGET_ARCH="$4" + OKD_VERSION="${2:-latest}" + OCP_BRANCH="$3" + TARGET_ARCH="${4:-}" + if [[ -z "${TARGET_ARCH}" ]]; then + case "$(uname -m)" in + x86_64) TARGET_ARCH="amd64" ;; + aarch64) TARGET_ARCH="arm64" ;; + *) echo "ERROR: Unsupported host arch: $(uname -m)"; exit 1 ;; + esac + fi + if [[ "${OKD_VERSION}" == "latest" ]]; then + OKD_VERSION="$(./src/okd/get_version.sh latest-${TARGET_ARCH})" + fiAs per coding guidelines: Build scripts must detect architecture automatically via
uname -mto determine between x86_64 or aarch64; OKD version auto-detection should query latest-amd64 or latest-arm64 tags based on detected architecture.
🤖 Fix all issues with AI agents
In @.github/actions/build-okd/action.yaml:
- Around line 69-80: The staging-override condition currently checks the runner
CPU arch via steps.detect-cpu-arch.outputs.go_arch which can cause cross-arch
mismatches; change the conditional to inspect the action input
inputs.target-arch instead (i.e. test "${{ inputs.target-arch }}" against
"arm64"), and set OKD_OVERRIDE accordingly so STAGING_REGISTRY and OKD_OVERRIDE
use the staging paths as before (keep OKD_RELEASE_IMAGE_AARCH64 for arm64 and
OKD_RELEASE_IMAGE_X86_64 for other cases).
b10adbf to
d105186
Compare
2 participants
Resolves #114
Rearrange the workflow so OKD images are pushed after testing process.
Build OKD images, push to OKD staging registry
Build MicroShift RPMs (using images from staging)
Build MicroShift bootc image (using local images)
Test MicroShift (validates the OKD images work)
ONLY IF TESTS PASS → Push OKD images to ghcr.io prod repository.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.