-
Notifications
You must be signed in to change notification settings - Fork 16
Use microshift healthcheck procedure instead of restarting greenboot service #194
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
This reverts commit 082e76a.
📝 WalkthroughWalkthroughRemoved explicit greenboot-0.15.* installation/pinning from packaging and RPM install scripts; replaced CI restarts of greenboot-healthcheck.service with a podman-run microshift healthcheck; simplified service handling to only enable and restart microshift. (46 words) Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@coderabbitai |
|
✅ Actions performedFull review 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: 2
🤖 Fix all issues with AI agents
In @.github/workflows/builders.yaml:
- Around line 67-69: The workflow uses bootc-image-tag: latest which can cause
non-deterministic test substrates; change the bootc image tag to a fixed tag
(e.g., stream9 or stream10) or implement the same dynamic tag resolution used in
release.yaml; update the bootc-image-tag value for the job that defines
bootc-image-url/bootc-image-tag (keys bootc-image-url and bootc-image-tag) so it
pins to a stable tag or pulls the tag via the release.yaml selection logic to
ensure reproducible runs.
In @.github/workflows/installers.yaml:
- Around line 39-41: The workflow uses bootc-image-tag: latest which is unpinned
and inconsistent with other pinned images; replace this by either referencing
the repository variable ${ { vars.FEDORA_BOOTC_TAG }} (create that variable in
repo settings) or hard-code a specific Fedora bootc tag (matching the
builders.yaml tagging pattern used for other images) so that bootc-image-tag
uses a fixed, reproducible version; update the bootc-image-tag entry and ensure
the variable name FEDORA_BOOTC_TAG or the explicit tag aligns with builders.yaml
patterns.
🧹 Nitpick comments (2)
packaging/rpms-copr.Containerfile (1)
1-1: Pin Fedora base image version for build reproducibility.Both
srpm.Containerfileandrpms-copr.Containerfileuselatest, which drifts over time. Therpm.Containerfilealready pins tostream9, andbootc.Containerfileuses parameterized tags—this Containerfile should follow the same pattern.Proposed change
+ ARG FEDORA_TAG=43 - FROM quay.io/fedora/fedora:latest + FROM quay.io/fedora/fedora:${FEDORA_TAG}src/copr/copr-cli.Containerfile (1)
1-1: Pin the Fedora base image instead oflatest.
latestcurrently maps to Fedora 43 but will change when future releases become stable, making builds non-deterministic. The repo uses ARG patterns for versioning in other Containerfiles (e.g.,bootc.Containerfile); follow that approach here.Suggested approach
+ ARG FEDORA_TAG=43 - FROM quay.io/fedora/fedora:latest + FROM quay.io/fedora/fedora:${FEDORA_TAG}
| bootc-image-url: registry.fedoraproject.org/fedora-bootc | ||
| bootc-image-tag: 42 | ||
| bootc-image-tag: latest | ||
| node-count: 2 |
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.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the builders.yaml file
fd "builders.yaml" --type fRepository: microshift-io/microshift
Length of output: 50
🏁 Script executed:
# Read the builders.yaml file with line numbers, focusing on the area around lines 67-69
cat -n .github/workflows/builders.yaml | head -100Repository: microshift-io/microshift
Length of output: 3675
🏁 Script executed:
# Search for bootc-image references and tag patterns across workflows
rg "bootc-image" --type yaml -A 2 -B 2Repository: microshift-io/microshift
Length of output: 50
🏁 Script executed:
# Check for variable definitions related to BOOTC or Fedora tags
rg "FEDORA_BOOTC|BOOTC_IMAGE" --type yaml -iRepository: microshift-io/microshift
Length of output: 50
🏁 Script executed:
# Check if there are any repo variables or configuration files
fd "(vars|variables|config)" --type f | grep -E "\.(yaml|yml|json|sh)$" | head -20Repository: microshift-io/microshift
Length of output: 205
🏁 Script executed:
# Check other workflow files for tag patterns
fd "\.yaml" .github/workflows --type f -exec basename {} \;Repository: microshift-io/microshift
Length of output: 219
🏁 Script executed:
# Read release.yaml to see if it has any tag configuration patterns
cat -n .github/workflows/release.yaml 2>/dev/null | grep -E "bootc|tag" -i -A 2 -B 2 | head -50Repository: microshift-io/microshift
Length of output: 1595
🏁 Script executed:
# Check for GitHub Variables or organization-level configs
find .github -type f \( -name "*.yml" -o -name "*.yaml" -o -name "*.json" \) -exec grep -l "vars\." {} \; | head -10Repository: microshift-io/microshift
Length of output: 50
🏁 Script executed:
# See all uses of ${{ vars in the codebase
rg '\$\{\{\s*vars\.' --type yamlRepository: microshift-io/microshift
Length of output: 50
🏁 Script executed:
# Look at the build action to understand what tags it expects
cat -n .github/actions/build/action.yaml | head -60Repository: microshift-io/microshift
Length of output: 2326
🏁 Script executed:
# Check the okd-version action to understand version detection
cat -n .github/actions/okd-version/action.yaml 2>/dev/nullRepository: microshift-io/microshift
Length of output: 1610
🏁 Script executed:
# Search for how bootc-image-tag is used across workflows
rg "bootc-image-tag" --type yaml -B 1 -A 1Repository: microshift-io/microshift
Length of output: 50
Pin the Fedora bootc tag to a fixed value. latest can drift test substrate between runs. Other bootc jobs use fixed tags (stream9, stream10); match that pattern or use the dynamic tag selection logic from release.yaml (which selects tag based on OKD version compatibility).
Example: use fixed tag
- bootc-image-tag: latest
+ bootc-image-tag: stream10📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bootc-image-url: registry.fedoraproject.org/fedora-bootc | |
| bootc-image-tag: 42 | |
| bootc-image-tag: latest | |
| node-count: 2 | |
| bootc-image-url: registry.fedoraproject.org/fedora-bootc | |
| bootc-image-tag: stream10 | |
| node-count: 2 |
🤖 Prompt for AI Agents
In @.github/workflows/builders.yaml around lines 67 - 69, The workflow uses
bootc-image-tag: latest which can cause non-deterministic test substrates;
change the bootc image tag to a fixed tag (e.g., stream9 or stream10) or
implement the same dynamic tag resolution used in release.yaml; update the
bootc-image-tag value for the job that defines bootc-image-url/bootc-image-tag
(keys bootc-image-url and bootc-image-tag) so it pins to a stable tag or pulls
the tag via the release.yaml selection logic to ensure reproducible runs.
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.
The stream9 and stream10 are not fixed tags.
They represent the latest from CentOS 9 and 10 streams.
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.
@ggiguash, thank you for the clarification! I misunderstood the tag semantics—you're right that stream9 and stream10 are rolling tags representing the latest from their respective CentOS streams, not pinned versions. Using latest for Fedora bootc is consistent with that pattern.
✏️ Learnings added
Learnt from: ggiguash
Repo: microshift-io/microshift PR: 194
File: .github/workflows/builders.yaml:67-69
Timestamp: 2026-02-03T07:55:10.132Z
Learning: In the microshift-io/microshift repository, bootc-image-tag values like `stream9`, `stream10`, and `latest` are rolling tags that track the latest images from CentOS 9, CentOS 10, and Fedora bootc streams respectively, not fixed version pins.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
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`
Learnt from: pmtk
Repo: microshift-io/microshift PR: 161
File: .github/actions/build/action.yaml:51-53
Timestamp: 2026-01-27T09:05:17.450Z
Learning: In `.github/actions/build/action.yaml`, the `build` input supports flexible workflows: use `build=all` to build both RPMs and bootc-image in one step, or build RPMs externally (e.g., via COPR) and then call the action with `build=bootc-image` to build only the container image. The Makefile validates RPM availability and provides helpful error messages.
Learnt from: CR
Repo: microshift-io/microshift PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T11:22:03.985Z
Learning: Applies to .github/workflows/*.yaml : CI/CD workflows should validate builds with `builders.yaml`, test quickstart scripts with `installers.yaml`, support manual release workflow via `release.yaml`, and run daily OKD ARM builds via `release-okd.yaml`
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/okd/** : OKD version auto-detection should query latest-amd64 or latest-arm64 tags based on detected architecture
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.
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
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`
Closes #139
microshift healthcheckcommandNote: This PR can be merged as soon as the fedora-iot/greenboot-rs#132 fix is released in Fedora 43 and CentOS 9 / 10 upstream.
Summary by CodeRabbit
Chores
Bug Fixes / Maintenance