-
Notifications
You must be signed in to change notification settings - Fork 12
Implement MicroShift ISO creation and deployment procedures #181
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
📝 WalkthroughWalkthroughAdds ISO creation and deployment support: new Makefile Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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 |
845abf3 to
6c2a6e2
Compare
6c2a6e2 to
0f0ed11
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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 (6)
src/iso/makeiso.sh (2)
35-47: Consider removing the redundant image pull.Line 36 explicitly pulls the image, but line 41's
--pull=newerflag will also pull if the image is missing or outdated, making the explicit pull redundant.🔎 Proposed simplification
-# Pull the bootc image builder image -sudo podman pull "${BIB_IMAGE}" - # Create the ISO image sudo podman run --rm -i \ --privileged \ --pull=newer \
49-56: Consider defensive handling of multiple install.iso files.The
findcommand could theoretically return multiple files if duplicates exist in subdirectories. Using-print -quitwould ensure only the first match is used.🔎 Proposed fix
-ISO_FILE="$(find "${ISO_OUT}" -name "install.iso")" +ISO_FILE="$(find "${ISO_OUT}" -name "install.iso" -print -quit)"docs/run-iso.md (3)
127-127: Hardcoded image tag will become stale.This example URL references a specific commit hash and version. Consider using a placeholder like
<IMAGE_TAG>or documenting how users should find the latest available tag.🔎 Suggested fix
-export BOOTC_IMAGE_URL="ghcr.io/microshift-io/microshift:4.21.0_g29f429c21_4.21.0_okd_scos.ec.15" +# Replace <IMAGE_TAG> with the latest tag from https://github.com/microshift-io/microshift/pkgs/container/microshift +export BOOTC_IMAGE_URL="ghcr.io/microshift-io/microshift:<IMAGE_TAG>"
176-181: Cleanup missing disk image removal.The cleanup commands undefine the VM but don't delete the disk image at
/var/lib/libvirt/images/${VMNAME}.qcow2. Users may accumulate orphaned disk files.🔎 Suggested fix
sudo virsh destroy "${VMNAME}" sudo virsh undefine "${VMNAME}" +sudo rm -f "/var/lib/libvirt/images/${VMNAME}.qcow2"
22-23: Minor style nit.Per static analysis: "hostname" is now typically written as one word.
src/iso/kickstart.ks.template (1)
46-48: Hardcoded password hash in template.The SHA-256 password hash corresponds to the
microshiftpassword documented indocs/run-iso.md. This is fine for a quickstart template, but consider adding a comment noting this is an example credential that users should change for production deployments.🔎 Suggested fix
-# Create a default microshift user, allowing it to run sudo commands without password +# Create a default microshift user with password 'microshift', allowing sudo without password. +# NOTE: Change the password hash for production deployments. useradd -m -p \$5\$XCVQ6AZVF6D5YW6V\$7wV451u/3oxvWA8vlqX.bWiBAYO/M8VRpZidA0vmzj5 microshift
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/release.mdMakefileREADME.mddocs/build.mddocs/run-iso.mdsrc/iso/kickstart.ks.templatesrc/iso/makeiso.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/iso/makeiso.sh
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 (14)
📓 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 ansible/** : Ansible roles should support automated builds and deployments for both containerized and host-based MicroShift installations
📚 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/workflows/release.mddocs/run-iso.mddocs/build.mdsrc/iso/kickstart.ks.templateMakefile
📚 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/workflows/release.mddocs/run-iso.mdMakefileREADME.md
📚 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/deb/** : DEB package conversion should use RPM_OUTDIR as input and support standard Debian package naming and installation paths
Applied to files:
docs/build.md
📚 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: Build artifacts are temporary by default; users must specify output directories (SRPM_WORKDIR, RPM_OUTDIR) to preserve build outputs
Applied to files:
docs/build.md
📚 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:
docs/build.md
📚 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:
docs/build.mdMakefile
📚 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:
docs/build.mdMakefile
📚 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:
docs/build.mdsrc/iso/makeiso.shsrc/iso/kickstart.ks.templateMakefile
📚 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/iso/kickstart.ks.template
📚 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:
src/iso/kickstart.ks.templateMakefile
📚 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/rpm/** : RPM installation scripts should manage repository creation/deletion and execute post-install configuration before service start
Applied to files:
src/iso/kickstart.ks.template
📚 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:
src/iso/kickstart.ks.template
📚 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:
Makefile
🪛 LanguageTool
docs/run-iso.md
[style] ~23-~23: It’s more common nowadays to write this noun as one word.
Context: ... user-specific settings like host name, user names and passwords, SSH keys, disk partition...
(RECOMMENDED_COMPOUNDS)
🔇 Additional comments (10)
.github/workflows/release.md (1)
34-37: LGTM! Documentation structure is consistent.The ISO deployment section follows the same pattern as the Bootc deployment block and properly references the new documentation.
src/iso/makeiso.sh (3)
1-9: LGTM! Proper error handling and clear usage.The script uses
set -euo pipefailfor robust error handling and provides clear usage instructions.
15-30: LGTM! Thorough input validation.The validation checks ensure the bootc image exists and the output directory is valid before proceeding.
1-56: Shellcheck verification inconclusive.Could not complete shellcheck verification due to execution timeout. Manual inspection of the script shows proper bash syntax and variable quoting, but automated validation is required per coding guidelines. Please run
shellcheck src/iso/makeiso.shlocally to confirm compliance.Note: This script does not include architecture detection (uname -m), which may not be applicable since it's an ISO creation utility rather than a build system script.
README.md (1)
67-67: LGTM! Documentation link addition is consistent.The new ISO deployment documentation link follows the established pattern and properly references the new documentation file.
Makefile (2)
56-56: LGTM! Help text properly updated.The ISO target is now documented in the help output alongside existing targets.
137-146: LGTM! ISO target properly implements sequential build dependency.The target correctly validates that the bootc image exists before attempting ISO creation, following the established build pattern (SRPM → RPM → Bootc → ISO). The use of
BOOTC_IMAGEwith a sensible default allows flexibility while maintaining the standard workflow.docs/build.md (2)
64-66: LGTM! Table formatting improvement.The reformatted table headers improve consistency with other sections.
147-172: LGTM! ISO creation documentation is clear and comprehensive.The new section properly documents the ISO creation workflow, including configuration options, usage examples, and expected output. The reference to Bootc Image Builder provides helpful context.
src/iso/kickstart.ks.template (1)
1-79: Well-structured Kickstart template.The template properly handles:
- Hardware-aware partitioning with
reqpart --add-boot- LVM setup leaving space for CSI driver
- Both local (
oci) and remote (registry) bootc transports- Persistent journal configuration with size limits
- Serial console access for VM debugging
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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 (5)
src/iso/makeiso.sh (2)
4-4: Consider pinning the bootc-image-builder version.Using
:latesttag reduces build reproducibility. Consider pinning to a specific version or making it configurable.🔎 Proposed fix
-BIB_IMAGE="quay.io/centos-bootc/bootc-image-builder:latest" +BIB_IMAGE="${BIB_IMAGE:-quay.io/centos-bootc/bootc-image-builder:latest}"
45-49: Consider limitingfinddepth.The
findcould be more targeted with-maxdepthto avoid unexpected matches in nested directories.🔎 Proposed fix
-ISO_FILE="$(find "${ISO_OUT}" -name "install.iso" -print -quit)" +ISO_FILE="$(find "${ISO_OUT}" -maxdepth 2 -name "install.iso" -print -quit)"docs/build.md (1)
168-171: Add language specifier to fenced code block.Static analysis flagged this. Use
textorconsolefor command output.🔎 Proposed fix
-``` +```text ... ... ISO image created at '/tmp/microshift-iso-niQwAN/bootiso/install.iso' -``` +```docs/run-iso.md (2)
22-26: Minor style nit: "user names" → "usernames".Common convention is one word.
🔎 Proposed fix
-hostname, user names and passwords, SSH keys, disk partitioning, etc. +hostname, usernames and passwords, SSH keys, disk partitioning, etc.
125-131: Placeholder requires user action.Consider adding a note clarifying that
<IMAGE_TAG>must be replaced with an actual tag (e.g., from GitHub releases).🔎 Proposed fix
# Loading the bootc image from a container registry export BOOTC_IMAGE_TRANSPORT=registry +# Replace <IMAGE_TAG> with the desired release tag export BOOTC_IMAGE_URL="ghcr.io/microshift-io/microshift:<IMAGE_TAG>"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Makefiledocs/build.mddocs/run-iso.mdsrc/iso/kickstart.ks.templatesrc/iso/makeiso.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- src/iso/kickstart.ks.template
- Makefile
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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/iso/makeiso.sh
🧠 Learnings (10)
📓 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 ansible/** : Ansible roles should support automated builds and deployments for both containerized and host-based MicroShift installations
📚 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/iso/makeiso.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:
src/iso/makeiso.shdocs/build.md
📚 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/deb/** : DEB package conversion should use RPM_OUTDIR as input and support standard Debian package naming and installation paths
Applied to files:
docs/build.md
📚 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: Build artifacts are temporary by default; users must specify output directories (SRPM_WORKDIR, RPM_OUTDIR) to preserve build outputs
Applied to files:
docs/build.md
📚 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:
docs/build.md
📚 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:
docs/build.md
📚 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:
docs/build.mddocs/run-iso.md
📚 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:
docs/build.md
📚 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:
docs/run-iso.md
🪛 LanguageTool
docs/run-iso.md
[style] ~23-~23: It’s more common nowadays to write this noun as one word.
Context: ...h user-specific settings like hostname, user names and passwords, SSH keys, disk partition...
(RECOMMENDED_COMPOUNDS)
🪛 markdownlint-cli2 (0.18.1)
docs/build.md
168-168: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (5)
src/iso/makeiso.sh (2)
35-43: Podman invocation looks correct.Privileged mode, storage volume mount, and output binding are appropriately configured for bootc-image-builder.
1-52: No architecture detection needed—bootc-image-builder is a multi-arch container image.Unlike scripts that download architecture-specific artifacts or query architecture-specific repositories (e.g.,
latest-amd64vslatest-arm64tags), this script uses a multi-arch container image that automatically selects the correct architecture variant at runtime. Similar to how sonobuoy container images handle this automatically, bootc-image-builder requires no script-level detection.Likely an incorrect or invalid review comment.
docs/build.md (2)
64-66: LGTM!Table formatting is consistent with the rest of the document.
147-167: Clear and well-structured documentation.The new "Create ISO" section follows established patterns and provides useful guidance.
docs/run-iso.md (1)
1-182: Well-structured deployment guide.Documentation covers prerequisites, both installation types, login verification, and cleanup comprehensively. The vendor-neutral approach and clear step-by-step instructions are helpful.
Resolves #178
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.