ISSUE-114: Separate OKD build and push phases #126
Conversation
|
Warning Rate limit exceeded@kasturinarra has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 3 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a cleanup-staging mode and gating to the build-okd action and workflow; refactors OKD image build script into mode dispatch (staging | production | list-packages), derives staging/production registries, adds retag/promotion and package-listing for tolerant staging cleanup, and exposes new input Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as "GitHub Actions"
participant Action as "build-okd Action"
participant Script as "src/okd/build_images.sh"
participant Staging as "Staging Registry"
participant Tests as "MicroShift Build/Test"
participant Prod as "Production Registry"
GH->>Action: run build job (mode=staging)
Action->>Script: mode=staging -> build_okd_images
Script->>Staging: push staging-tagged images (record SHAs)
GH->>Tests: run MicroShift build/tests using staging refs
alt tests pass
GH->>Action: run production flow (mode=production)
Action->>Script: mode=production -> retag_staging_to_production & push_production
Script->>Staging: pull staging tags
Script->>Prod: retag & push production tags
end
GH->>Action: run cleanup job (mode=list-packages, cleanup-staging=true)
Action->>Script: mode=list-packages -> list_packages (emit package names)
Action->>Staging: delete listed staging packages (best-effort)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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
2 times, most recently
from
6a5ab4b to
e6c3708
Compare
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.yamlsrc/okd/build_images.sh
🧰 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 (16)
📓 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 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: 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.
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 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 .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 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.
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
📚 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 .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-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-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 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-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: 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
📚 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
📚 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.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 ansible/** : Ansible roles should support automated builds and deployments for both containerized and host-based MicroShift installations
Applied to files:
.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-04T13:35:05.230Z
Learnt from: pmtk
Repo: microshift-io/microshift PR: 151
File: docs/workflows.md:74-76
Timestamp: 2025-12-04T13:35:05.230Z
Learning: The COPR group microshift-io/microshift exists at https://copr.fedorainfracloud.org/coprs/g/microshift-io/microshift/ and is used for publishing MicroShift RPM packages in the microshift-io/microshift repository.
Applied to files:
.github/actions/build-okd/action.yaml
📚 Learning: 2025-11-26T06:46:33.353Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 137
File: src/quickrpm.sh:83-98
Timestamp: 2025-11-26T06:46:33.353Z
Learning: In the microshift-io/microshift repository, the quickrpm.sh script only supports RHEL 9 or above (and equivalent CentOS/Fedora versions), where util-linux includes the `--nooverlap` flag for losetup.
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: centos9-bootc
- GitHub Check: isolated-network (ovnk, ubuntu-24.04, 1)
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04-arm)
- GitHub Check: centos10-bootc
- 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: quick-rpm-and-clean (ubuntu-24.04)
🔇 Additional comments (10)
src/okd/build_images.sh (6)
7-12: Registry configuration looks good.Staging registry is correctly derived using
dirnameto avoid the/okd/okd-stagingnesting issue from prior reviews.
17-28: Usage documentation updated appropriately.Mode parameter is now mandatory and well-documented with registry destinations shown.
332-347: Build function is well-structured.Cleanly separates build logic, populates
images_shawith tags for downstream use, and correctly skips haproxy-router for non-ARM64.
358-385: Retag function handles staging→production correctly.
- Uses
localdeclarations (per prior review)- String substitution
${production_image/${PRODUCTION_REGISTRY}/${STAGING_REGISTRY}}correctly derives staging reference- Clear error message when staging images are missing
387-406: Mode-specific push functions are clean and well-organized.
push_staginghandles the full staging flow (login, build, push), whilepush_productionhandles retagging and promotion. Good separation of concerns.
471-476: Mode dispatch is straightforward.Cleanly routes to the appropriate function based on validated mode parameter.
.github/actions/build-okd/action.yaml (4)
48-61: Build step correctly uses staging mode.Comments clearly document the staging registry derivation and purpose. Mode mismatch from prior review is resolved.
63-78: RPM build correctly references staging images.Registry derivation logic mirrors the shell script, ensuring consistency. The
OKD_RELEASE_IMAGE_AARCH64override ensures RPMs embed references to the staged (tested) images.
107-120: Production push gated on success.This ensures broken OKD images don't reach production. Aligns with the PR objective of build→test→push sequence.
122-158: Cleanup step has good resilience patterns.
if: always()ensures cleanup runs regardless of prior step outcomescontinue-on-error: trueprevents cleanup failures from failing the workflow- Both user and org API endpoints are tried for flexibility across account types
--paginatehandles large package lists
kasturinarra
force-pushed
the
fix_issue_114
branch
3 times, most recently
from
f1f7d7b to
4dcbfdc
Compare
kasturinarra
force-pushed
the
fix_issue_114
branch
from
3ce55e0 to
2891073
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/okd/build_images.sh (2)
417-439: Consider keeping package list in sync with images array.The hardcoded package list should match the images pushed. Currently
basein the images array maps toscos-${OKD_VERSION}which is covered. However, if new images are added to theimagesarray, this list needs manual update.Consider deriving the package list programmatically from the images array keys to prevent drift.
461-474: Architecture passed as parameter instead of auto-detected.Per coding guidelines, build scripts should detect architecture via
uname -m. CurrentlyTARGET_ARCHis a required parameter. This may be intentional for cross-arch CI builds, but consider adding auto-detection with optional override:# Auto-detect architecture with optional override if [[ $# -eq 3 ]]; then TARGET_ARCH=$(uname -m) [[ "${TARGET_ARCH}" == "aarch64" ]] && TARGET_ARCH="arm64" [[ "${TARGET_ARCH}" == "x86_64" ]] && TARGET_ARCH="amd64" else TARGET_ARCH="$4" fiBased on coding guidelines.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh
🧰 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 (16)
📓 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 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 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* : 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
📚 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
📚 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
📚 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-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* : 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: 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
📚 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
📚 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-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
📚 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 ansible/** : Ansible roles should support automated builds and deployments for both containerized and host-based MicroShift installations
Applied to files:
.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-04T13:35:05.230Z
Learnt from: pmtk
Repo: microshift-io/microshift PR: 151
File: docs/workflows.md:74-76
Timestamp: 2025-12-04T13:35:05.230Z
Learning: The COPR group microshift-io/microshift exists at https://copr.fedorainfracloud.org/coprs/g/microshift-io/microshift/ and is used for publishing MicroShift RPM packages in the microshift-io/microshift repository.
Applied to files:
.github/actions/build-okd/action.yaml
📚 Learning: 2025-10-17T10:53:08.461Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 57
File: .github/workflows/builders.yaml:75-84
Timestamp: 2025-10-17T10:53:08.461Z
Learning: In the microshift repository, the RPM-to-Debian conversion workflow follows this pattern: `convert.sh` receives an RPM directory as input and outputs .deb files to a `deb/` subdirectory within that directory. The `install.sh` script expects to receive the parent RPM directory (not the deb subdirectory) and uses recursive `find` commands to locate the .deb files in subdirectories.
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: quick-start-and-clean (ubuntu-24.04-arm)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04-arm)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04)
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: centos9-bootc
- GitHub Check: isolated-network (ovnk, ubuntu-24.04, 1)
- GitHub Check: centos10-bootc
- GitHub Check: fedora-bootc (ubuntu-24.04)
- GitHub Check: fedora-bootc (ubuntu-24.04-arm)
- GitHub Check: ubuntu-rpm2deb
🔇 Additional comments (10)
.github/actions/build-okd/action.yaml (4)
54-61: LGTM!Clear documentation of staging mode behavior. The mode mismatch from previous review has been addressed.
72-80: LGTM!Proper architecture-aware OKD image override. The x86_64 branch future-proofs the action for potential multi-arch usage.
115-128: LGTM!Production push correctly gated by
success(). This ensures OKD images are only published after validation.
130-163: LGTM!Best-effort cleanup with proper error handling. Package list retrieval from the script avoids duplicating knowledge.
src/okd/build_images.sh (6)
7-12: LGTM!Registry derivation is clean. STAGING_REGISTRY properly computed from PRODUCTION_REGISTRY.
17-37: LGTM!Usage documentation is comprehensive and clearly explains each mode.
341-365: LGTM!Functions are well-factored. The haproxy-router skip logic is consistent and documented.
367-394: LGTM!Retagging logic is correct. Good error message with remediation hint.
396-415: LGTM!Mode functions are symmetric and well-documented. Helpful next-step guidance in push_staging output.
490-526: LGTM!Mode-based registry selection correctly routes staging and production flows to their respective registries.
kasturinarra
force-pushed
the
fix_issue_114
branch
from
2891073 to
a2b6800
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/okd/build_images.sh (1)
417-457: Consider deriving package list from images array to reduce duplication.The
image_keysarray is hardcoded and must be manually kept in sync with theimagesassociative array defined at lines 520-535. If a new image is added toimagesbut not toimage_keys, the cleanup step will miss it.Consider deriving the list dynamically or documenting that both locations must be updated together.
Potential approach
Since
list-packagesmode only needs the OKD_VERSION (not arch-specific), and the package names follow a pattern, you could extract the base image keys into a shared constant array near the top of the script that bothimagesarray population andlist_packagesreference.Alternatively, add a comment cross-referencing the two locations so future maintainers know to update both.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/actions/build-okd/action.yamlsrc/okd/build_images.sh
🧰 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 (16)
📓 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 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* : 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 packaging/Containerfile* : RPM build container requires SRPM image from previous stage and outputs to `RPM_OUTDIR`
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.
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.
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/okd/** : OKD version auto-detection should query latest-amd64 or latest-arm64 tags based on detected architecture
📚 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-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-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 .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-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-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
📚 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 ansible/** : Ansible roles should support automated builds and deployments for both containerized and host-based MicroShift installations
Applied to files:
.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-04T13:35:05.230Z
Learnt from: pmtk
Repo: microshift-io/microshift PR: 151
File: docs/workflows.md:74-76
Timestamp: 2025-12-04T13:35:05.230Z
Learning: The COPR group microshift-io/microshift exists at https://copr.fedorainfracloud.org/coprs/g/microshift-io/microshift/ and is used for publishing MicroShift RPM packages in the microshift-io/microshift repository.
Applied to files:
.github/actions/build-okd/action.yaml
📚 Learning: 2025-10-17T10:53:08.461Z
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 57
File: .github/workflows/builders.yaml:75-84
Timestamp: 2025-10-17T10:53:08.461Z
Learning: In the microshift repository, the RPM-to-Debian conversion workflow follows this pattern: `convert.sh` receives an RPM directory as input and outputs .deb files to a `deb/` subdirectory within that directory. The `install.sh` script expects to receive the parent RPM directory (not the deb subdirectory) and uses recursive `find` commands to locate the .deb files in subdirectories.
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). (5)
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: fedora-bootc (ubuntu-24.04-arm)
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
- GitHub Check: centos10-bootc
- GitHub Check: centos9-bootc
🔇 Additional comments (6)
src/okd/build_images.sh (3)
7-11: LGTM: Registry derivation is correct.STAGING_REGISTRY is automatically derived using
dirnameto replace the final path component, avoiding nested paths like/okd/okd-staging.
341-394: LGTM: Mode-based functions implement the staging workflow correctly.The build, push, and retag functions properly implement the staging-first workflow: build → test → production sequence. The retagging logic at lines 381-392 correctly converts staging image names to production names using parameter substitution.
462-544: LGTM: Main execution flow handles all three modes correctly.Mode validation, argument checking, and conditional execution are all properly implemented. The staging/production dispatch at lines 540-544 matches the PR objective of ensuring OKD images are tested before production push.
.github/actions/build-okd/action.yaml (3)
54-86: LGTM: Staging-based RPM build correctly uses derived staging registry.The workflow properly:
- Builds and pushes images to staging registry (line 58)
- Derives
STAGING_REGISTRYusing the same logic as the script (line 73)- Sets architecture-specific
OKD_OVERRIDEto point RPM builds at staging images (lines 76-85)This ensures MicroShift RPMs are built using staged OKD images for testing before production.
115-128: LGTM: Production push correctly gated by test success.The
if: success()guard ensures images are only pushed to production after successful validation, fulfilling the core PR requirement.
130-163: LGTM: Best-effort cleanup handles staging artifacts appropriately.The cleanup step:
- Runs
always()to clean up regardless of workflow outcome- Uses
continue-on-error: truefor best-effort deletion- Dynamically retrieves package list via
list-packagesmode (line 147)- Handles deletion failures gracefully (lines 155-159)
This prevents accumulation of staging artifacts while not failing the workflow if cleanup encounters issues.
kasturinarra
force-pushed
the
fix_issue_114
branch
from
34ba5a2 to
d096e7a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
.github/workflows/release-okd.yaml (1)
54-77: Consider conditional cleanup based on build job outcome.The cleanup job runs with
if: always()(line 57), meaning it executes even whenbuild-okd-releaseis cancelled or fails early before pushing any staging images. This wastes resources on unnecessary cleanup attempts.Consider changing to
if: success() || failure()to skip cleanup when the workflow is cancelled, or add logic to only clean if staging images were actually pushed.[/scratchpad_end] -->
.github/actions/build-okd/action.yaml (2)
85-89: Architecture detection could be more robust.Lines 85-89 use an if/else that assumes
go_archis either "arm64" or defaults to "amd64". Ifgo_archcontains an unexpected value (empty, malformed, etc.), the else branch silently usesOKD_RELEASE_IMAGE_X86_64.Add explicit validation or use a case statement to handle unexpected architecture values:
🔎 Suggested improvement
- # Set the correct architecture-specific variable for staging override - if [ "${{ steps.detect-cpu-arch.outputs.go_arch }}" = "arm64" ]; then - OKD_OVERRIDE="OKD_RELEASE_IMAGE_AARCH64=${STAGING_REGISTRY}/okd-release-arm64" - else - OKD_OVERRIDE="OKD_RELEASE_IMAGE_X86_64=${STAGING_REGISTRY}/okd-release-amd64" - fi + # Set the correct architecture-specific variable for staging override + GO_ARCH="${{ steps.detect-cpu-arch.outputs.go_arch }}" + case "${GO_ARCH}" in + arm64) + OKD_OVERRIDE="OKD_RELEASE_IMAGE_AARCH64=${STAGING_REGISTRY}/okd-release-arm64" + ;; + amd64) + OKD_OVERRIDE="OKD_RELEASE_IMAGE_X86_64=${STAGING_REGISTRY}/okd-release-amd64" + ;; + *) + echo "ERROR: Unexpected architecture: ${GO_ARCH}" + exit 1 + ;; + esac
166-171: Error suppression may hide authentication or API issues.Line 167 redirects all output to
/dev/nullwith&>/dev/null, which silences both legitimate "not found" cases and actual errors (authentication failures, rate limits, API errors).Consider logging errors to help diagnose cleanup failures:
🔎 Suggested improvement
echo "Deleting package: ${package}" - if gh api --method DELETE "/users/${OWNER}/packages/container/${encoded_package}" \ - -H "Accept: application/vnd.github+json" &>/dev/null; then + if gh api --method DELETE "/users/${OWNER}/packages/container/${encoded_package}" \ + -H "Accept: application/vnd.github+json" 2>&1 | tee /tmp/gh-api-error.log | grep -q "204\|404"; then echo " ✓ Deleted" else - echo " ⚠ Not found or already deleted" + echo " ⚠ Not found or already deleted" + if [ -s /tmp/gh-api-error.log ]; then + echo " API response: $(cat /tmp/gh-api-error.log)" + fi fisrc/okd/build_images.sh (3)
7-11: STAGING_REGISTRY derivation may fail with non-standard registry paths.Line 11 uses
dirnameto derive the staging registry, which assumesPRODUCTION_REGISTRYcontains at least one path separator. IfTARGET_REGISTRYis set to a bare hostname (e.g.,localhost:5000),dirnamereturns., resulting in./okd-staging.Add validation or document the expected format:
🔎 Suggested improvement
# Production registry - must be provided via TARGET_REGISTRY environment variable # or defaults to the upstream registry if not specified PRODUCTION_REGISTRY="${TARGET_REGISTRY:-ghcr.io/microshift-io/okd}" + +# Validate registry format +if [[ "${PRODUCTION_REGISTRY}" != *"/"* ]]; then + echo "ERROR: PRODUCTION_REGISTRY must include a path (e.g., registry.io/org/repo)" + exit 1 +fi + # Automatically derive staging registry by appending '/okd-staging' subpath STAGING_REGISTRY="$(dirname "${PRODUCTION_REGISTRY}")/okd-staging"
384-388: Production push assumes staging images exist in local podman storage.Line 384 checks
podman image existswhich only validates local storage. If the staging images exist in the registry but were built on a different runner or the local cache was cleared, production push fails even though the images are available.Consider pulling from staging registry if not found locally:
🔎 Suggested improvement
if ! podman image exists "${staging_image}" ; then - echo "ERROR: Local staging image ${staging_image} not found." - echo "Run staging build first: $0 staging ${OKD_VERSION} ${OCP_BRANCH} ${TARGET_ARCH}" - exit 1 + echo "Staging image not found locally, attempting to pull from registry..." + if ! podman pull "${staging_image}"; then + echo "ERROR: Staging image ${staging_image} not found locally or in registry." + echo "Run staging build first: $0 staging ${OKD_VERSION} ${OCP_BRANCH} ${TARGET_ARCH}" + exit 1 + fi fi
417-457: Hardcoded image_keys in list_packages risks drift from actual images array.Lines 421-434 hardcode the image keys, duplicating the knowledge from the
imagesarray declared at line 520. If a new image is added toimagesbut not tolist_packages, cleanup will fail to remove those staging packages.Consider deriving the list dynamically from the images array or adding a comment/check to ensure they stay synchronized:
🔎 Suggested improvement
# List packages mode: output staging package names for cleanup list_packages() { - # Define image keys that match the images array defined later in the script - # This ensures the package list stays in sync with actual builds - 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 the images array keys + # This ensures the package list stays in sync with actual builds + for key in "${!images[@]}"; do if [[ "${key}" == "base" ]]; then # base image maps to scos-${OKD_VERSION} packages+=("okd-staging/scos-${OKD_VERSION}")Note: This requires moving the
imagesarray declaration before the function definitions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/actions/build-okd/action.yaml.github/workflows/release-okd.yamlsrc/okd/build_images.sh
🧰 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 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`
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.
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.
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
📚 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.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.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:
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.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:
src/okd/build_images.sh.github/actions/build-okd/action.yaml.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:
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.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.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-arm)
- GitHub Check: fedora-bootc (ubuntu-24.04)
- GitHub Check: isolated-network (ovnk, ubuntu-24.04, 1)
- GitHub Check: ubuntu-rpm2deb
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: centos9-bootc
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04)
- GitHub Check: centos10-bootc
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04-arm)
🔇 Additional comments (8)
.github/actions/build-okd/action.yaml (3)
28-31: LGTM: Cleanup-staging input properly defined.The input uses string type with explicit 'true'/'false' values, which aligns with GitHub Actions best practices and the string comparisons used throughout the action.
126-139: LGTM: Production push correctly gated on test success.The production push step is properly conditional on
success()(line 127), ensuring OKD images are only promoted after successful testing. This implements the desired build→test→push sequence from issue #114.
37-186: LGTM: Consistent conditional gating throughout action.The
cleanup-stagingflag is consistently used to gate build/test/debug steps (skipped when true) and enable cleanup-only steps (active when true). The gating pattern is applied correctly across all relevant steps.src/okd/build_images.sh (5)
17-37: LGTM: Usage documentation clearly describes all modes.The updated usage function comprehensively documents the three modes (staging, production, list-packages) with clear examples and explanations of registry destinations. The distinction between modes that require 4 arguments vs. 2 arguments is well documented.
341-356: LGTM: Build function properly encapsulates image creation.The
build_okd_imagesfunction cleanly wraps the existingcreate_imageslogic and correctly handles the ARM64 haproxy-router exception. Populatingimages_shawith image names (not digests) at this stage is appropriate for the staging workflow.
396-407: LGTM: Staging workflow properly orchestrated.The
push_stagingfunction cleanly sequences the staging build flow: validation → build → push. The helpful output at lines 405-406 guides users on the next step (production push), and the early exit fromcheck_release_image_existsprevents duplicate builds.
409-415: LGTM: Production workflow correctly reuses staging images.The
push_productionfunction properly implements the retag-and-push pattern, converting staging images to production references before pushing. The reuse ofcheck_release_image_existsprevents accidental overwrites of existing production releases.
462-544: LGTM: Mode dispatch logic is clean and well-structured.The main block properly handles the three modes with appropriate validation:
- Lines 469-477: Special case for
list-packages(2 args)- Lines 479-492: Standard validation for
staging/production(4 args)- Lines 508-515: Mode-aware TARGET_REGISTRY selection
- Lines 539-544: Mode-specific execution
The flow is logical and the validation prevents invalid invocations.
kasturinarra
force-pushed
the
fix_issue_114
branch
from
d096e7a to
8c9728d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/okd/build_images.sh (1)
417-459: Synchronization between two lists creates maintenance burden.Lines 423-436 hardcode image keys that must match the
imagesarray at line 523-538. Manual synchronization between two locations is error-prone. Consider deriving the list dynamically from theimagesarray keys instead:list_packages() { local packages=() # Derive from images array for key in "${!images[@]}"; do if [[ "${key}" == "base" ]]; then packages+=("okd-staging/scos-${OKD_VERSION}") else packages+=("okd-staging/${key}") fi done # Add release images packages+=( "okd-staging/okd-release-arm64" "okd-staging/okd-release-amd64" ) printf '%s\n' "${packages[@]}" }This eliminates drift risk and reduces maintenance overhead.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/actions/build-okd/action.yaml.github/workflows/release-okd.yamlsrc/okd/build_images.sh
🧰 Additional context used
📓 Path-based instructions (3)
.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
**/*.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 .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* : 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 .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.yaml.github/workflows/release-okd.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 src/image/prebuild.sh : Replace component images with OKD references during SRPM build stage
Applied to files:
.github/actions/build-okd/action.yaml.github/workflows/release-okd.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* : 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.yaml.github/workflows/release-okd.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 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.yaml.github/workflows/release-okd.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* : 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.yaml.github/workflows/release-okd.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 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). (10)
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04-arm)
- GitHub Check: quick-rpm-and-clean (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: fedora-bootc (ubuntu-24.04)
- GitHub Check: centos10-bootc
- GitHub Check: ubuntu-rpm2deb
- GitHub Check: centos9-bootc
🔇 Additional comments (22)
src/okd/build_images.sh (12)
7-11: Verify default registry is appropriate for all use cases.Line 9 hardcodes
ghcr.io/microshift-io/okdas the default production registry. Per past review comments, this may not work for private forks that can't access the microshift-io registry. Ensure callers always setTARGET_REGISTRYexplicitly when needed, or document this limitation.
17-37: LGTM!Clear documentation of the three modes and their purposes. The usage text helps users understand the staging → test → production workflow.
341-356: LGTM!Function correctly builds images and populates the images_sha array. The haproxy-router skip logic aligns with the TODO at line 108-110.
358-365: LGTM!Function delegates to existing push and release creation logic. Clean separation of concerns.
367-394: LGTM!Retagging logic correctly transforms staging images to production references. Good error handling with clear instructions if staging images are missing.
396-407: LGTM!Staging workflow is well-structured with helpful output guiding users to the next step (production push after testing).
409-415: LGTM!Production workflow correctly retags staging images before pushing, ensuring only tested images reach production.
464-494: LGTM!Argument parsing correctly handles the special case of list-packages (2 args) versus staging/production modes (4 args). Clear validation and error messages.
510-518: LGTM!Mode-based registry selection is correct. TARGET_REGISTRY and OKD_RELEASE_IMAGE are set appropriately for each mode.
520-538: Images array structure is sound.The array declaration is correct. However, see the earlier comment on lines 417-459 regarding the maintenance burden of keeping this synchronized with
list_packages().
542-547: LGTM!Mode dispatch is clean and correct. The if/elif structure clearly routes to the appropriate workflow function.
486-494: Architecture and version parameters comply with workflow-driven design.The script accepts
TARGET_ARCHandOKD_VERSIONas parameters rather than auto-detecting them. This is appropriate for a CI/CD-invoked script where the workflow controls these values. The coding guideline for auto-detection applies to standalone build scripts, not workflow-orchestrated builds..github/workflows/release-okd.yaml (1)
55-78: Cleanup job structure is sound.The job correctly depends on
build-okd-releaseand runs regardless of outcome (success() || failure()). Usingubuntu-latestis appropriate since cleanup is lightweight. The reuse of the same action withcleanup-staging: 'true'is elegant.Note: The cleanup deletes entire staging packages, not just the current version. Per past review comment by ggiguash, this is acceptable because the job runs after the build completes, and future multi-platform builds would call cleanup only once after both platforms finish.
.github/actions/build-okd/action.yaml (9)
28-31: LGTM!New input is well-defined with an appropriate default. Enables the gating pattern used throughout the action.
37-53: LGTM!Consistent gating across preparatory steps. Skips unnecessary work when only cleanup is needed.
55-69: LGTM!Step correctly invokes the script in
stagingmode. The comment clearly explains that staging mode both builds and pushes to the staging registry.
71-95: LGTM!RPM build correctly derives staging registry and handles both architectures symmetrically. The architecture-specific OKD_OVERRIDE ensures RPMs use the correct staging images regardless of build platform.
97-110: LGTM!Bootc image build step is correctly gated. Standard build process.
112-125: LGTM!Test step correctly validates MicroShift functionality before allowing production push. Gating is correct.
126-139: LGTM!Production push correctly gates on test success. The
productionmode invocation completes the validation workflow, ensuring only tested images reach production.
141-174: Cleanup implementation is solid.The step correctly:
- Gates on
cleanup-staging == 'true'- Derives package list from the build script (single source of truth)
- URL-encodes package names properly
- Checks API exit code rather than output (addresses past review concern)
- Uses
continue-on-errorto prevent workflow failure on cleanup issuesThe
/users/endpoint assumption is acceptable since this is specific to the microshift-io organization structure. If future deployments need org-level packages, this can be extended.
182-186: LGTM!Debug collection steps correctly skip when only performing cleanup. Standard workflow hygiene.
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. |
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
Other
✏️ Tip: You can customize this high-level summary in your review settings.