-
Notifications
You must be signed in to change notification settings - Fork 12
Local build from srpm #164
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
Conversation
WalkthroughSwitches to an SRPM-first build pipeline: adds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer (make)
participant Host as Host FS
participant SRPM_IMG as SRPM Image (SRPM_IMAGE)
participant RPM_IMG as RPM Image (RPM_IMAGE)
participant CI as CI runner
Dev->>Host: run `make srpm` (produce SRPMs in SRPM_WORKDIR)
CI->>SRPM_IMG: build/tag SRPM image including SRPM artifacts
Dev->>Host: run `make rpm` (consumes SRPM_IMAGE)
CI->>RPM_IMG: build `packaging/rpm.Containerfile` (extract SRPMs, run rpmbuild)
RPM_IMG->>Host: export RPMs / repo metadata to RPM_OUTDIR
Dev->>CI: run `make image` (build `packaging/bootc.Containerfile` FROM `RPM_IMAGE`)
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)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-10-17T10:53:08.461ZApplied to files:
📚 Learning: 2025-12-04T13:35:05.230ZApplied to files:
📚 Learning: 2025-11-26T06:46:33.353ZApplied to files:
📚 Learning: 2025-10-17T07:44:32.742ZApplied to files:
📚 Learning: 2025-10-17T10:31:57.408ZApplied to files:
⏰ 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)
🔇 Additional comments (4)
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 |
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 (2)
Makefile (1)
15-16: RPM/SRPM image wiring and workdirs look coherent; consider tweaking the log message
SRPM_WORKDIR(Line 15) and its use in thesrpmtarget (Line 93) correctly default to a mktemp path under/tmp/..., matching the new SRPM docs.RPM_IMAGE := microshift-okd-rpmandSRPM_IMAGE := microshift-okd-srpm(Lines 41–43) align withFROM localhost/microshift-okd-{rpm,srpm}:latestin the Containerfiles and with Podman’s implicitlocalhost/prefix. Based on learnings.rpm: srpm(Line 74), the rpm build usingpackaging/rpm.Containerfileand tagging${RPM_IMAGE}(Lines 77–79), and theimageprecheckpodman image exists "${RPM_IMAGE}"pluspackaging/bootc.Containerfile(Lines 117–120, 134) together enforce the intended SRPM→RPM→bootc flow.clean-allnow also removes${RPM_IMAGE}and${SRPM_IMAGE}(Lines 191–192), which matches the new image names.Minor nit: the echo in Line 75 still says “Building the MicroShift builder image”; renaming it to “RPM image” would better reflect the new flow:
- @echo "Building the MicroShift builder image" + @echo "Building the MicroShift RPM image"Also applies to: 41-44, 74-85, 117-120, 191-192
docs/build.md (1)
19-54: Docs align with the new SRPM→RPM→bootc flow; add a language to the sample output blockThe new “Create SRPM Package”, “Create RPM Packages”, and “Create Bootc Image” sections correctly match the updated Makefile and Containerfiles:
SRPM_WORKDIRandRPM_OUTDIRdefaults,microshift-okd-{srpm,rpm}image names, CentOS Stream 9 as the RPM build base, and the fact thatmake imagenow consumes artifacts frommicroshift-okd-rpmare all consistent.To satisfy markdownlint MD040 and improve readability, add a language to the SRPM sample output block around Lines 42–46, e.g.:
-``` +```text ... ... SRPMs are available in '/tmp/microshift-srpms-1tzW3h'Also applies to: 55-76, 72-76, 133-136 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 6dda473a869ec528e50ab78a1b11292f3b540be0 and fdf866ddedd6735b1c4a575decf69f62ee94a22e. </details> <details> <summary>📒 Files selected for processing (5)</summary> * `Makefile` (6 hunks) * `docs/build.md` (2 hunks) * `packaging/bootc.Containerfile` (1 hunks) * `packaging/microshift-builder.Containerfile` (0 hunks) * `packaging/rpm.Containerfile` (1 hunks) </details> <details> <summary>💤 Files with no reviewable changes (1)</summary> * packaging/microshift-builder.Containerfile </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (5)</summary> <details> <summary>📓 Common learnings</summary>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.shreceives an RPM directory as input and outputs .deb files to adeb/subdirectory within that directory. Theinstall.shscript expects to receive the parent RPM directory (not the deb subdirectory) and uses recursivefindcommands to locate the .deb files in subdirectories.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.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--nooverlapflag for losetup.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 thelocalhost/prefix and:latesttag, resulting inlocalhost/microshift-okd:latest. This means the Makefile in microshift-io/microshift building with-t microshift-okdproduceslocalhost/microshift-okd:latestwithout explicit retagging.</details> <details> <summary>📚 Learning: 2025-10-17T07:44:32.742Z</summary>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 thelocalhost/prefix and:latesttag, resulting inlocalhost/microshift-okd:latest. This means the Makefile in microshift-io/microshift building with-t microshift-okdproduceslocalhost/microshift-okd:latestwithout explicit retagging.**Applied to files:** - `Makefile` - `packaging/rpm.Containerfile` - `packaging/bootc.Containerfile` - `docs/build.md` </details> <details> <summary>📚 Learning: 2025-10-17T10:53:08.461Z</summary>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.shreceives an RPM directory as input and outputs .deb files to adeb/subdirectory within that directory. Theinstall.shscript expects to receive the parent RPM directory (not the deb subdirectory) and uses recursivefindcommands to locate the .deb files in subdirectories.**Applied to files:** - `Makefile` - `packaging/rpm.Containerfile` - `docs/build.md` </details> <details> <summary>📚 Learning: 2025-12-04T13:35:05.230Z</summary>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:** - `Makefile` - `packaging/rpm.Containerfile` - `docs/build.md` </details> <details> <summary>📚 Learning: 2025-11-26T06:46:33.353Z</summary>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--nooverlapflag for losetup.**Applied to files:** - `Makefile` - `packaging/rpm.Containerfile` - `docs/build.md` </details> </details><details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>docs/build.md</summary> 42-42: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>⏰ 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)</summary> * GitHub Check: fedora-bootc (ubuntu-24.04-arm) * GitHub Check: centos10-bootc * GitHub Check: fedora-bootc (ubuntu-24.04) * GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0) * GitHub Check: ubuntu-rpm2deb * GitHub Check: isolated-network (ovnk, ubuntu-24.04, 1) * 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: quick-start-and-clean (ubuntu-24.04) * GitHub Check: quick-rpm-and-clean (ubuntu-24.04-arm) </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>packaging/bootc.Containerfile (1)</summary><blockquote> `5-5`: **Builder base image switch is consistent with new RPM flow** Using `localhost/microshift-okd-rpm:latest` as the builder (Line 5) matches the `RPM_IMAGE := microshift-okd-rpm` tag in the Makefile and the RPM repo layout produced by `packaging/rpm.Containerfile` under `/home/microshift/microshift/_output/rpmbuild/RPMS`, so the subsequent `COPY --from=builder` instructions remain valid. Based on learnings, this also aligns with Podman’s implicit `localhost/` handling. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
fdf866d to
b2cf453
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)
packaging/rpm.Containerfile (1)
11-11: Add trailing slash to COPY source path.Without the trailing slash, this creates
/tmp/SRPMS/, but line 22 expects the SRPM in/tmp/.
🧹 Nitpick comments (2)
packaging/rpm.Containerfile (1)
21-21: Clarify the comment.The phrase "Bootc builder is reusing file" is unclear. Consider explaining which file and why it matters.
docs/build.md (1)
42-46: Add language specifier to fenced code block.The fenced code block should specify a language (e.g.,
text) to fix the markdownlint warning.-``` +```text ... ... SRPMs are available in '/tmp/microshift-srpms-1tzW3h'</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between fdf866ddedd6735b1c4a575decf69f62ee94a22e and b2cf453a76b150348ce02b78fbdc970d9f95c25b. </details> <details> <summary>📒 Files selected for processing (5)</summary> * `Makefile` (6 hunks) * `docs/build.md` (2 hunks) * `packaging/bootc.Containerfile` (1 hunks) * `packaging/microshift-builder.Containerfile` (0 hunks) * `packaging/rpm.Containerfile` (1 hunks) </details> <details> <summary>💤 Files with no reviewable changes (1)</summary> * packaging/microshift-builder.Containerfile </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (2)</summary> * packaging/bootc.Containerfile * Makefile </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (5)</summary> <details> <summary>📓 Common learnings</summary>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.shreceives an RPM directory as input and outputs .deb files to adeb/subdirectory within that directory. Theinstall.shscript expects to receive the parent RPM directory (not the deb subdirectory) and uses recursivefindcommands to locate the .deb files in subdirectories.</details> <details> <summary>📚 Learning: 2025-10-17T10:53:08.461Z</summary>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.shreceives an RPM directory as input and outputs .deb files to adeb/subdirectory within that directory. Theinstall.shscript expects to receive the parent RPM directory (not the deb subdirectory) and uses recursivefindcommands to locate the .deb files in subdirectories.**Applied to files:** - `docs/build.md` - `packaging/rpm.Containerfile` </details> <details> <summary>📚 Learning: 2025-12-04T13:35:05.230Z</summary>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:** - `docs/build.md` - `packaging/rpm.Containerfile` </details> <details> <summary>📚 Learning: 2025-10-17T07:44:32.742Z</summary>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 thelocalhost/prefix and:latesttag, resulting inlocalhost/microshift-okd:latest. This means the Makefile in microshift-io/microshift building with-t microshift-okdproduceslocalhost/microshift-okd:latestwithout explicit retagging.**Applied to files:** - `docs/build.md` - `packaging/rpm.Containerfile` </details> <details> <summary>📚 Learning: 2025-11-26T06:46:33.353Z</summary>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--nooverlapflag for losetup.**Applied to files:** - `docs/build.md` - `packaging/rpm.Containerfile` </details> </details><details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>docs/build.md</summary> 42-42: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>⏰ 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)</summary> * 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: centos10-bootc * GitHub Check: centos9-bootc * GitHub Check: ubuntu-rpm2deb * GitHub Check: quick-start-and-clean (ubuntu-24.04-arm) * 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) </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>packaging/rpm.Containerfile (1)</summary><blockquote> `22-27`: The glob patterns `microshift-*.src.rpm` and `microshift-*.tar.gz` are designed for single-file matches based on the SRPM building process, which produces one package. While defensive coding could explicitly handle multiple file scenarios, the current approach is safe due to command-level validation (rpm2cpio, tar, and rpmbuild reject multiple files) and `set -e` error handling. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
0c2b2d8 to
7fb7fbe
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/build.md (1)
55-76: Minor grammar tweak in RPM build description.Tiny wording issue: “RPMs are build from the SRPM…” → “RPMs are built…”. Suggested edit:
-The `make rpm` command builds MicroShift RPMs for CentOS Stream 9 operating -system. RPMs are build from the SRPM built by running `make srpm`. +The `make rpm` command builds MicroShift RPMs for the CentOS Stream 9 operating +system. RPMs are built from the SRPM produced by running `make srpm`.The rest of the RPM section (including
RPM_OUTDIRexplanation and success message) looks consistent with the new flow.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Makefile(6 hunks)docs/build.md(2 hunks)packaging/bootc.Containerfile(1 hunks)packaging/microshift-builder.Containerfile(0 hunks)packaging/rpm.Containerfile(1 hunks)
💤 Files with no reviewable changes (1)
- packaging/microshift-builder.Containerfile
🚧 Files skipped from review as they are similar to previous changes (2)
- packaging/rpm.Containerfile
- Makefile
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
📚 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:
docs/build.md
📚 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:
docs/build.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.mdpackaging/bootc.Containerfile
📚 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:
docs/build.md
🪛 markdownlint-cli2 (0.18.1)
docs/build.md
42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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: ubuntu-rpm2deb
- GitHub Check: fedora-bootc (ubuntu-24.04)
- 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)
- GitHub Check: centos9-bootc
- GitHub Check: centos10-bootc
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
🔇 Additional comments (2)
packaging/bootc.Containerfile (1)
5-25: Switch tomicroshift-okd-rpmbuilder image looks correct; verify path layout matches.The new
FROM localhost/microshift-okd-rpm:latest AS builderaligns with the SRPM→RPM-based flow. Just ensure themicroshift-okd-rpmimage still exposes:
${BUILDER_RPM_REPO_PATH}=/home/microshift/microshift/_output/rpmbuild/RPMS${BUILDER_RSHARED_SERVICE}=/home/microshift/microshift/packaging/imagemode/systemd/microshift-make-rshared.serviceso the
COPY --from=buildersteps continue to work without breakage.docs/build.md (1)
133-136: Bootc image description correctly points tomicroshift-okd-rpm.The text now stating that
make imageuses artifacts from themicroshift-okd-rpmimage created bymake rpmmatches the updated Containerfile and SRPM→RPM workflow; no further changes needed here.
- microshift-runner -> bootc - microshift-builder -> rpm
7fb7fbe to
0f7d202
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
73-94: Ensurepodman image umountalways runs (avoid mount leaks on failure).Right now, if the
cpat Line 89 fails, Line 90 won’t execute, leaving the image mounted. Suggest atrapto guarantee unmount.rpm: @if ! sudo podman image exists "${SRPM_IMAGE}" ; then \ echo "ERROR: Run 'make srpm' to build the MicroShift SRPMs" ; \ exit 1 ; \ fi @echo "Building the MicroShift RPMs image" sudo podman build \ -t "${RPM_IMAGE}" \ --ulimit nofile=524288:524288 \ -f packaging/rpm.Containerfile . @echo "Extracting the MicroShift RPMs" - outdir="$${RPM_OUTDIR:-$$(mktemp -d /tmp/microshift-rpms-XXXXXX)}" && \ - mntdir="$$(sudo podman image mount "${RPM_IMAGE}")" && \ - sudo cp -r "$${mntdir}/home/microshift/microshift/_output/rpmbuild/RPMS/." "$${outdir}" && \ - sudo podman image umount "${RPM_IMAGE}" && \ + set -euo pipefail ; \ + outdir="$${RPM_OUTDIR:-$$(mktemp -d /tmp/microshift-rpms-XXXXXX)}" ; \ + mntdir="$$(sudo podman image mount "${RPM_IMAGE}")" ; \ + trap 'sudo podman image umount "${RPM_IMAGE}" >/dev/null 2>&1 || true' EXIT ; \ + sudo cp -r "$${mntdir}/home/microshift/microshift/_output/rpmbuild/RPMS/." "$${outdir}" ; \ echo "" && \ echo "Build completed successfully" && \ echo "RPMs are available in '$${outdir}'"
♻️ Duplicate comments (1)
docs/build.md (1)
19-54: Add a language to the sample output fenced block (MD040).The fenced block at Line 42 should be annotated (e.g., ```text) to satisfy markdownlint MD040, as already noted in prior review comments.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Makefile(6 hunks)docs/build.md(2 hunks)packaging/bootc.Containerfile(1 hunks)packaging/microshift-builder.Containerfile(0 hunks)packaging/rpm.Containerfile(1 hunks)
💤 Files with no reviewable changes (1)
- packaging/microshift-builder.Containerfile
🚧 Files skipped from review as they are similar to previous changes (2)
- packaging/rpm.Containerfile
- packaging/bootc.Containerfile
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
📚 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:
Makefiledocs/build.md
📚 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:
Makefiledocs/build.md
📚 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:
Makefiledocs/build.md
📚 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:
Makefiledocs/build.md
🪛 markdownlint-cli2 (0.18.1)
docs/build.md
42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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: isolated-network (ovnk, ubuntu-24.04, 1)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04)
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04-arm)
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: fedora-bootc (ubuntu-24.04)
- GitHub Check: ubuntu-rpm2deb
- GitHub Check: centos10-bootc
- GitHub Check: centos9-bootc
🔇 Additional comments (5)
docs/build.md (2)
55-61: SRPM→RPM flow description matches the new pipeline.The “RPMs are built from the SRPM built by
make srpm” and themicroshift-okd-rpmimage references are consistent with the new build flow.Also applies to: 72-75
133-136: Bootc section correctly points at RPM-derived image.Referencing artifacts from
microshift-okd-rpmcreated bymake rpmaligns with the SRPM-first pipeline.Makefile (3)
95-108: SRPM output via host-mounted/outputlooks good.Using
SRPM_WORKDIR(or mktemp fallback) and a volume mount is a clean way to persist SRPM artifacts outside the image.
192-197: Cleanup updates are consistent with new images.Removing
${RPM_IMAGE}and${SRPM_IMAGE}inclean-allmatches the SRPM-first pipeline.
122-140: No action needed—podman build --envis supported. The feature was added in Podman v4.1.1 (June 2022), and the project's CI/CD environment (Ubuntu 24.04) ships Podman 4.9.3. The code at lines 135–138 is compatible with all current build environments.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/build/action.yaml (1)
45-59: Ensuremake rpmis actually consuming the SRPM produced in the previous step (and pass the same build inputs).
Right nowmake srpmis parameterized (USHIFT_GITREF,OKD_VERSION_TAG) butmake rpmisn’t; if the Makefile expects SRPM artifacts (e.g., viaSRPM_WORKDIR) or needs the same vars for consistent output, this can silently diverge or fail.Concrete tweak (if Makefile supports it):
make srpm \ USHIFT_GITREF="${{ inputs.ushift-gitref }}" \ OKD_VERSION_TAG="${{ inputs.okd-version-tag }}" make rpm \ + USHIFT_GITREF="${{ inputs.ushift-gitref }}" \ + OKD_VERSION_TAG="${{ inputs.okd-version-tag }}" \ RPM_OUTDIR=/mnt/rpms
🧹 Nitpick comments (1)
.github/actions/build/action.yaml (1)
45-60: Gate the RPM build step oninputs.build(and considerset -euo pipefail).
The “Build MicroShift RPMs” step runs unconditionally even wheninputs.buildisbootc-image, which can add avoidable work/failure modes.Example:
- name: Build MicroShift RPMs + if: contains(fromJSON('["all", "rpms"]'), inputs.build) shell: bash run: | + set -euo pipefail cd ${GITHUB_WORKSPACE}/
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/actions/build/action.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
📚 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/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/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/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/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: isolated-network (ovnk, ubuntu-24.04, 1)
- GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
- GitHub Check: centos10-bootc
- GitHub Check: fedora-bootc (ubuntu-24.04)
- GitHub Check: fedora-bootc (ubuntu-24.04-arm)
- GitHub Check: ubuntu-rpm2deb
- GitHub Check: centos9-bootc
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04)
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04-arm)
|
Failures in the presubmits are due to following:
openshift/microshift#5892 should allow us to drop the |
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 (3)
packaging/rpm.Containerfile (3)
5-9: Consider installing build dependencies from the SRPM instead of hand-picking packages.
Right now the container may break whenBuildRequireschange (or differ by arch). Usingdnf builddepmakes the rebuild track the specfile more reliably.RUN dnf install -y \ --setopt=install_weak_deps=False \ - rpm-build which git cpio createrepo \ + rpm-build which git cpio createrepo \ gcc gettext golang jq make policycoreutils selinux-policy selinux-policy-devel systemd && \ dnf clean all + +# (later, before rpmbuild) +# dnf -y install 'dnf-command(builddep)' && dnf -y builddep ./microshift-*.src.rpm
18-23: Harden therpm2cpio | cpiopipeline withpipefail.
This avoids edge cases where the pipeline status doesn’t reflect the failing command.FROM quay.io/centos/centos:stream9 + +SHELL ["/bin/bash", "-o", "pipefail", "-c"]
25-26: Make the variant override configurable (defaulting tocommunity).
This fixes the current mismatch, but hardcoding makes it harder to reuse the Containerfile for other variants.ARG BUILDER_RPM_REPO_PATH=/home/microshift/microshift/_output/rpmbuild/ +ARG MICROSHIFT_VARIANT=community @@ - rpmbuild --quiet --define 'microshift_variant community' --rebuild ./microshift-*.src.rpm && \ + rpmbuild --quiet --define "microshift_variant ${MICROSHIFT_VARIANT}" --rebuild ./microshift-*.src.rpm && \
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packaging/rpm.Containerfile(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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.
📚 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:
packaging/rpm.Containerfile
📚 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:
packaging/rpm.Containerfile
📚 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:
packaging/rpm.Containerfile
📚 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:
packaging/rpm.Containerfile
📚 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:
packaging/rpm.Containerfile
⏰ 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: isolated-network (ovnk, ubuntu-24.04, 1)
- GitHub Check: ubuntu-rpm2deb
- GitHub Check: fedora-bootc (ubuntu-24.04)
- GitHub Check: fedora-bootc (ubuntu-24.04-arm)
- 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: centos9-bootc
- GitHub Check: quick-rpm-and-clean (ubuntu-24.04)
- GitHub Check: quick-start-and-clean (ubuntu-24.04)
- GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
🔇 Additional comments (1)
packaging/rpm.Containerfile (1)
1-3: SRPM→RPM stage split looks right for the SRPM-first pipeline.
No concerns with the overall multi-stage layout.
Because downstream make-rpm.sh does not persist it in the specfile, it gets lost when building RPMs from SRPM.
cb6be4d to
8c1aca2
Compare
Closes #163
Summary by CodeRabbit
Build System
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.