Skip to content

Conversation

@ggiguash
Copy link
Contributor

@ggiguash ggiguash commented Jan 3, 2026

Resolves #187

Summary by CodeRabbit

  • Chores
    • During container image build, added a step to detect and disable an optional system service so it won’t be enabled in the produced images. This prevents unwanted service activation at runtime and keeps images clean.

✏️ Tip: You can customize this high-level summary in your review settings.

@ggiguash ggiguash requested a review from a team as a code owner January 3, 2026 10:19
@coderabbitai
Copy link

coderabbitai bot commented Jan 3, 2026

📝 Walkthrough

Walkthrough

Adds a RUN step to packaging/bootc.Containerfile that probes for and, if present, disables the bootc-publish-rhsm-facts.service systemd unit via systemctl disable. The instruction is placed after the VOLUME directive and includes a hadolint suppression.

Changes

Cohort / File(s) Summary
Service disabling
packaging/bootc.Containerfile
Added a RUN step (after VOLUME) that checks for bootc-publish-rhsm-facts.service in systemd unit-files and conditionally runs systemctl disable if found; includes hadolint DL4006 suppression.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: disabling a specific service in the Containerfile, matching the core objective.
Linked Issues check ✅ Passed The changes directly address issue #187 by disabling bootc-publish-rhsm-facts.service in the bootc image when it exists.
Out of Scope Changes check ✅ Passed The changes are limited to a single Containerfile with a targeted RUN step disabling the service, fully aligned with the linked issue scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0aa0a86 and 763654d.

📒 Files selected for processing (1)
  • packaging/bootc.Containerfile
🧰 Additional context used
🧠 Learnings (4)
📓 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* : Bootc image build requires RPM image from previous stage, is configurable via `WITH_KINDNET`, `WITH_TOPOLVM`, `WITH_OLM`, and `EMBED_CONTAINER_IMAGES`, and is based on `BOOTC_IMAGE_URL:BOOTC_IMAGE_TAG`
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to 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:

  • packaging/bootc.Containerfile
📚 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:

  • packaging/bootc.Containerfile
📚 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:

  • packaging/bootc.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). (7)
  • GitHub Check: centos10-bootc
  • GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
  • GitHub Check: isolated-network (ovnk, ubuntu-24.04, 1)
  • GitHub Check: centos9-bootc
  • GitHub Check: ubuntu-rpm2deb
  • GitHub Check: fedora-bootc (ubuntu-24.04)
  • GitHub Check: fedora-bootc (ubuntu-24.04-arm)
🔇 Additional comments (2)
packaging/bootc.Containerfile (2)

70-73: AI summary mentions non-existent hadolint suppression.

The summary claims this includes a hadolint suppression, but no such directive is present in the code.


71-73: The condition correctly uses systemctl list-unit-files to probe for the unit—it returns non-zero when the unit doesn't exist, so the logic works as intended. No changes needed.

Likely an incorrect or invalid review comment.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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)
packaging/bootc.Containerfile (1)

70-73: Consider a more direct existence check.

The list-unit-files | grep pattern works but is indirect. A simpler approach:

-RUN if systemctl list-unit-files --type=service | grep -Fwq 'bootc-publish-rhsm-facts.service' ; then \
-        systemctl disable bootc-publish-rhsm-facts.service ; \
-    fi
+RUN systemctl disable bootc-publish-rhsm-facts.service 2>/dev/null || true

Or if you prefer explicit checking:

-RUN if systemctl list-unit-files --type=service | grep -Fwq 'bootc-publish-rhsm-facts.service' ; then \
+RUN if systemctl list-unit-files bootc-publish-rhsm-facts.service >/dev/null 2>&1 ; then \
         systemctl disable bootc-publish-rhsm-facts.service ; \
     fi
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between de83e40 and 135e4f4.

📒 Files selected for processing (1)
  • packaging/bootc.Containerfile
🧰 Additional context used
🧠 Learnings (4)
📓 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* : Bootc image build requires RPM image from previous stage, is configurable via `WITH_KINDNET`, `WITH_TOPOLVM`, `WITH_OLM`, and `EMBED_CONTAINER_IMAGES`, and is based on `BOOTC_IMAGE_URL:BOOTC_IMAGE_TAG`
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to 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:

  • packaging/bootc.Containerfile
📚 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:

  • packaging/bootc.Containerfile
📚 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:

  • packaging/bootc.Containerfile

@ggiguash ggiguash force-pushed the disable-bootc-publish-rhsm-facts.service branch from 135e4f4 to 0aa0a86 Compare January 3, 2026 11:18
Copy link

@coderabbitai coderabbitai bot left a 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)
packaging/bootc.Containerfile (1)

70-74: Logic is correct and resolves the issue.

The conditional check and disable approach properly addresses the PR objective. The grep pattern and flags are appropriate.

Optional: Simplify to avoid hadolint suppression

You could eliminate the pipe and suppression by checking directly:

 # Disable the bootc-publish-rhsm-facts.service if it exists
-# hadolint ignore=DL4006
-RUN if systemctl list-unit-files --type=service | grep -Fwq 'bootc-publish-rhsm-facts.service' ; then \
+RUN if systemctl list-unit-files bootc-publish-rhsm-facts.service &>/dev/null ; then \
         systemctl disable bootc-publish-rhsm-facts.service ; \
     fi

Alternatively, consider systemctl mask instead of disable for stronger prevention (creates symlink to /dev/null, prevents any enabling):

-RUN if systemctl list-unit-files bootc-publish-rhsm-facts.service &>/dev/null ; then \
-        systemctl disable bootc-publish-rhsm-facts.service ; \
-    fi
+RUN systemctl mask bootc-publish-rhsm-facts.service 2>/dev/null || true
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 135e4f4 and 0aa0a86.

📒 Files selected for processing (1)
  • packaging/bootc.Containerfile
🧰 Additional context used
🧠 Learnings (4)
📓 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* : Bootc image build requires RPM image from previous stage, is configurable via `WITH_KINDNET`, `WITH_TOPOLVM`, `WITH_OLM`, and `EMBED_CONTAINER_IMAGES`, and is based on `BOOTC_IMAGE_URL:BOOTC_IMAGE_TAG`
📚 Learning: 2025-12-13T11:22:03.985Z
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to 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:

  • packaging/bootc.Containerfile
📚 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* : Containerfiles should pass linting with hadolint as part of the `make check` validation step

Applied to files:

  • packaging/bootc.Containerfile
📚 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:

  • packaging/bootc.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: 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: quick-start-and-clean (ubuntu-24.04-arm)
  • GitHub Check: isolated-network (ovnk, ubuntu-24.04, 1)
  • GitHub Check: fedora-bootc (ubuntu-24.04-arm)
  • GitHub Check: centos10-bootc
  • GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
  • GitHub Check: ubuntu-rpm2deb
  • GitHub Check: fedora-bootc (ubuntu-24.04)
  • GitHub Check: centos9-bootc

@ggiguash ggiguash force-pushed the disable-bootc-publish-rhsm-facts.service branch from 0aa0a86 to 1e216fc Compare January 3, 2026 11:30
@ggiguash ggiguash force-pushed the disable-bootc-publish-rhsm-facts.service branch from 1e216fc to 763654d Compare January 3, 2026 11:32
Copy link
Contributor

@agullon agullon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ggiguash ggiguash merged commit af1f773 into microshift-io:main Jan 5, 2026
14 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: bootc-publish-rhsm-facts.service enabled in bootc image

2 participants