-
Notifications
You must be signed in to change notification settings - Fork 13
ISSUE-114: Separate OKD build and push phases #126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a staging→production CI flow: the Action builds OKD images to a staging registry for testing, then on success runs a production push. The build script now accepts a mode argument, derives production and staging registries, and implements mode-specific build, retag, and push logic with architecture-aware handling. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Script as build_images.sh
participant Staging as Staging Registry
participant Tests as Test Jobs
participant Prod as Production Registry
GH->>Script: run(mode=staging, version, branch, arch)
Script->>Staging: build & push OKD images & manifests
GH->>Tests: run MicroShift builds/tests referencing staging images
alt tests pass
GH->>Script: run(mode=production, version, branch, arch)
Script->>Staging: pull/retag staging images -> production names
Script->>Prod: push retagged images & manifests to production registry
else tests fail
GH->>GH: run failure-cleanup step (log non-promotion)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
61e7990 to
846b22d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
846b22d to
7030690
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
7030690 to
c32164b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
c32164b to
c592ac4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
7ed71e3 to
9ff24fe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
|
be3c978 to
e676b2e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/actions/build-okd/action.yaml (1)
120-127: Cleanup step is informational only.The cleanup step logs messaging but does not actually remove staging images from the registry. If cleanup is intended as part of the staging→production flow, consider implementing actual deletion. For now, this is acceptable since staging images will be overwritten on the next build attempt.
📜 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 (6)
📓 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:
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:
.github/actions/build-okd/action.yaml
📚 Learning: 2025-12-04T13:35:05.210Z
Learnt from: pmtk
Repo: microshift-io/microshift PR: 151
File: docs/workflows.md:74-76
Timestamp: 2025-12-04T13:35:05.210Z
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
📚 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). (2)
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04-arm)
🔇 Additional comments (5)
src/okd/build_images.sh (4)
7-27: Registry initialization and mode documentation look solid.The production/staging registry derivation is correct:
STAGING_REGISTRYusesdirnameto extract the parent path and appends/okd-staging, avoiding double nesting. Mode naming is clear and consistent throughout.
332-403: Functions are well-structured and modularized.The build/push separation into distinct functions (
build_okd_images,push_okd_images,retag_staging_to_production,push_staging,push_production) is clean and maintainable. Error handling inretag_staging_to_productionprovides clear feedback if staging images are missing before attempting production push.
437-472: Mode-based registry selection and dispatch logic is correct.The script correctly sets
TARGET_REGISTRYbased on mode before derivingOKD_RELEASE_IMAGE(line 444), ensuring the image path uses the correct registry. The dispatch topush_stagingorpush_production(lines 469–472) properly routes execution.
1-2: Dependency: mode names in action.yaml must be fixed for this script to work.Once the action calls the script with correct modes ("staging" and "production"), this script will execute the intended staging→production workflow. Ensure the three mode references in action.yaml (build → staging, push → production) are corrected.
.github/actions/build-okd/action.yaml (1)
63-76: Verify path construction consistency with staging registry derivation.The concern about
OKD_RELEASE_IMAGEpath construction in line 75 requires verification. Confirm whetherinputs.target-registryis passed as a parent path (e.g.,ghcr.io/microshift-io) or includes the/okdsuffix. If it includes/okd, appending/okd-staging/would create incorrect nesting inconsistent with howbuild_images.shderives the staging registry via$(dirname "${PRODUCTION_REGISTRY}")/okd-staging.
af7e20a to
e9c0eea
Compare
|
/retest |
e9c0eea to
70bbf45
Compare
|
@ggiguash all tests passed after your fix, can you help review when you get sometime ? thanks !! |
|
@ggiguash could you please give another round of review when you get some time ? thanks !! |
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
✏️ Tip: You can customize this high-level summary in your review settings.