Skip to content

RPM post-install should not explicitly install systemd-resolved#138

Merged
ggiguash merged 2 commits intomicroshift-io:mainfrom
ggiguash:systemd-resolved
Nov 25, 2025
Merged

RPM post-install should not explicitly install systemd-resolved#138
ggiguash merged 2 commits intomicroshift-io:mainfrom
ggiguash:systemd-resolved

Conversation

@ggiguash
Copy link
Copy Markdown
Contributor

@ggiguash ggiguash commented Nov 25, 2025

A prerequisite for #137.

The systemd-resolved package is not required in the configurations used for running MicroShift. We should not explicitly install this package as it causes problems when run in privileged Bootc containers.

See https://gitlab.com/fedora/bootc/tracker/-/issues/80 for more information.

Summary by CodeRabbit

  • New Features

    • Automatic firewall configuration now opens Kubernetes API and etcd ports (6443, 2379, 2380) in the public zone during installation.
  • Chores

    • Simplified package installation by removing an unnecessary package and installing only required utilities.
    • Updated CI/build image tag to a specific version.
    • Minor comment/documentation cleanup in installation scripts.

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

@ggiguash ggiguash requested a review from a team as a code owner November 25, 2025 11:20
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 25, 2025

Walkthrough

Removed systemd-resolved from the RPM post-install package list, added firewalld rules to open Kubernetes API (6443/tcp) and etcd (2379/tcp, 2380/tcp) in the public zone, updated an inline comment about systemd-resolved in privileged Bootc containers, and changed bootc-image-tag in the Fedora builders workflow from latest to 42.

Changes

Cohort / File(s) Summary
RPM post-install script updates
src/rpm/postinstall.sh
Removed systemd-resolved from the dnf install line; consolidated installation of firewalld, jq, and bash-completion; added firewall-cmd rules to open 6443/tcp, 2379/tcp, and 2380/tcp in the public zone; adjusted inline comment about systemd-resolved in privileged Bootc containers.
CI workflow change
.github/workflows/builders.yaml
Updated fedora-bootc job: set bootc-image-tag from "latest" to "42".

Sequence Diagram(s)

(omitted — changes are configuration/script edits without new runtime control-flow)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Check firewall commands for correct zone/persistence and idempotence.
  • Verify removing systemd-resolved has no unintended effects in target images/containers.
  • Confirm the bootc-image-tag: "42" change aligns with CI/image availability.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main change: removing systemd-resolved from RPM post-install, which aligns perfectly with the modifications shown in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 c48927d and 81807d8.

📒 Files selected for processing (1)
  • .github/workflows/builders.yaml (1 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:

  • .github/workflows/builders.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: quick-start-and-clean (ubuntu-24.04)
  • 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: centos9-bootc
  • GitHub Check: fedora-bootc (ubuntu-24.04)
  • GitHub Check: centos10-bootc
  • GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
  • GitHub Check: ubuntu-rpm2deb
🔇 Additional comments (1)
.github/workflows/builders.yaml (1)

70-70: Fedora 42 is actively maintained and appropriate for this change.

Fedora 42 (released April 15, 2025) remains supported and is scheduled to be maintained until about May 2026. Pinning bootc-image-tag to 42 improves build reproducibility while maintaining a secure, supported baseline for at least 6+ months.


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
Copy Markdown

@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: 1

📜 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 d43f26f and 108d8bd.

📒 Files selected for processing (1)
  • src/rpm/postinstall.sh (2 hunks)
⏰ 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: quick-start-and-clean (ubuntu-24.04)
  • GitHub Check: quick-start-and-clean (ubuntu-24.04-arm)
  • GitHub Check: ubuntu-rpm2deb
  • GitHub Check: isolated-network (kindnet, ubuntu-24.04, 0)
  • GitHub Check: fedora-bootc (ubuntu-24.04)
  • GitHub Check: fedora-bootc (ubuntu-24.04-arm)
  • GitHub Check: centos10-bootc
  • GitHub Check: centos9-bootc
🔇 Additional comments (2)
src/rpm/postinstall.sh (2)

39-39: LGTM: Removed systemd-resolved from explicit installation.

Aligns with PR objective and avoids issues in privileged Bootc containers. The service can still be disabled if present (line 62).


43-45: Verify firewall rule additions are intentional scope.

The new rules for Kubernetes API (6443) and etcd (2379, 2380) ports are valid for multi-node clusters, but they're not mentioned in the PR objectives. Confirm these changes are intentional as part of the PR #136 prerequisite work.

Comment thread src/rpm/postinstall.sh Outdated
@ggiguash ggiguash merged commit 8bdcfdd into microshift-io:main Nov 25, 2025
12 checks passed
@ggiguash ggiguash deleted the systemd-resolved branch November 25, 2025 15:22
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.

1 participant