feat: add macOS support to quickstart scripts#219
Closed
agullon wants to merge 2 commits into
Closed
Conversation
📝 WalkthroughWalkthroughThis PR adds macOS support to MicroShift's quick-start and quick-clean scripts. The workflow matrix expands to include macOS runners with Podman machine initialization, scripts accept a configurable container name, and Darwin/Linux branching handles LVM disk provisioning and cleanup differently on each platform. ChangesmacOS Support for Quick-Start and Quick-Clean
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
Contributor
|
Does it have to be a separate script? How difficult would it be to integrate it into the existing quickstart.sh? In the worst case, we could keep macos-specific functions in a separate file and include / invoke those conditionally on macos |
agullon
force-pushed
the
macos-quickstart
branch
5 times, most recently
from
1a6148b to
b4b4a3a
Compare
ggiguash
reviewed
May 13, 2026
ggiguash
reviewed
May 13, 2026
agullon
force-pushed
the
macos-quickstart
branch
13 times, most recently
from
1f0fd68 to
d0cacc3
Compare
agullon
changed the title
ggiguash
reviewed
May 13, 2026
ggiguash
reviewed
May 13, 2026
ggiguash
reviewed
May 13, 2026
agullon
force-pushed
the
macos-quickstart
branch
2 times, most recently
from
800aa7c to
d95851c
Compare
agullon
force-pushed
the
macos-quickstart
branch
3 times, most recently
from
355c0f7 to
ed7e192
Compare
ggiguash
reviewed
May 13, 2026
| podman machine set --rootful | ||
| podman machine start | ||
|
|
||
| - name: Run the quick start script |
Contributor
There was a problem hiding this comment.
Can we reuse the existing ./.github/actions/quick-start-clean action?
agullon
force-pushed
the
macos-quickstart
branch
4 times, most recently
from
3dcb65c to
e96ba70
Compare
quickstart.sh now detects macOS and handles podman machine validation (rootful mode required) and LVM setup via `podman machine ssh`. The pre-built image is pulled from GHCR, same as Linux. Also adds macOS to the installers CI workflow matrix. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> pre-commit.check-secrets: ENABLED
Usage: ./src/quickstart.sh [container-name] Defaults to 'microshift-okd' when no argument is provided. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> pre-commit.check-secrets: ENABLED
agullon
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
.github/actions/quick-start-clean/action.yaml (2)
34-34:⚠️ Potential issue | 🟠 MajorHardcoded container name.
The health check assumes
microshift-okd, butquickstart.shnow accepts a custom container name via argument.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/actions/quick-start-clean/action.yaml at line 34, Replace the hardcoded container identifier in the health-check command (the line starting with state=$(sudo podman exec -i microshift-okd ...) ) with the actual container variable used by the quickstart script (e.g., use the same container/name argument or env var that quickstart.sh exposes such as CONTAINER_NAME or $CONTAINER) so the podman exec targets the user-specified instance; ensure the action reads that variable (falling back to the previous default "microshift-okd" if unset) and use it in the systemctl call.
51-52:⚠️ Potential issue | 🟠 MajorHardcoded container name.
Cleanup verification assumes
microshift-okd, which won't work if quickstart was run with a custom container name.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/actions/quick-start-clean/action.yaml around lines 51 - 52, The cleanup check uses a hardcoded container/image name in the podman commands ("sudo podman ps -a | grep microshift-okd && exit 1" and "sudo podman image exists microshift-okd && exit 1"), which breaks when quickstart was run with a custom name; change these to reference a configurable variable (e.g., CONTAINER_NAME or an action input) with a sensible default and use that variable in both the podman ps/grep and podman image exists checks so the cleanup verifies the actual container/image name used at runtime.src/quickclean.sh (1)
15-15:⚠️ Potential issue | 🟠 MajorHardcoded container name.
This script assumes the container is named
microshift-okd, butquickstart.shnow accepts a custom name. Users who run quickstart with a custom name won't be able to clean up.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/quickclean.sh` at line 15, The script hardcodes the container name "microshift-okd" (seen in the image_ref assignment using podman inspect), so change it to use a configurable container name (e.g., read from an environment variable or CLI arg) and fall back to "microshift-okd" if unset; update the image_ref command and any other occurrences of the literal "microshift-okd" in the script to use that variable (e.g., QUICKSTART_CONTAINER_NAME or CONTAINER_NAME) so quickclean works for custom quickstart names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/quickstart.sh`:
- Line 9: The quickstart script defines a configurable CONTAINER_NAME variable
but quickclean.sh still hardcodes "microshift-okd", so custom names won't be
cleaned; fix by making quickclean.sh accept the container name (e.g., read an
argument $1 or environment variable) and update quickstart.sh to either export
CONTAINER_NAME before invoking quickclean.sh or pass CONTAINER_NAME as the
argument when calling quickclean.sh; specifically change quickclean.sh to use
the same symbol (CONTAINER_NAME) instead of the literal and ensure quickstart.sh
uses the same mechanism (export CONTAINER_NAME or pass it) so both scripts
reference the same container identifier.
- Around line 219-222: Make the displayed podman commands platform-aware by
introducing an EXEC_PREFIX determined from uname and uid (e.g., if uname -s ==
"Darwin" and id -u != 0 then EXEC_PREFIX="" else EXEC_PREFIX="sudo "), then use
that EXEC_PREFIX variable when emitting the podman/kubectl examples (replace the
hardcoded "sudo " in the lines that echo the podman exec and kubectl get pods
commands), referencing the existing CONTAINER_NAME variable so outputs become
"${EXEC_PREFIX}podman exec -it ${CONTAINER_NAME} /bin/bash -l" and
"${EXEC_PREFIX}podman exec -it ${CONTAINER_NAME} kubectl get pods -A".
- Line 177: The variable local_rootful is intended to be function-scoped but is
declared without the local keyword; update the assignment to declare it as a
local variable (e.g., use local local_rootful when assigning the result of
podman machine inspect --format '{{.Rootful}}' 2>/dev/null || echo "false") so
it doesn't leak into the global environment; locate the assignment of
local_rootful in src/quickstart.sh and prepend the local keyword to the variable
declaration.
---
Duplicate comments:
In @.github/actions/quick-start-clean/action.yaml:
- Line 34: Replace the hardcoded container identifier in the health-check
command (the line starting with state=$(sudo podman exec -i microshift-okd ...)
) with the actual container variable used by the quickstart script (e.g., use
the same container/name argument or env var that quickstart.sh exposes such as
CONTAINER_NAME or $CONTAINER) so the podman exec targets the user-specified
instance; ensure the action reads that variable (falling back to the previous
default "microshift-okd" if unset) and use it in the systemctl call.
- Around line 51-52: The cleanup check uses a hardcoded container/image name in
the podman commands ("sudo podman ps -a | grep microshift-okd && exit 1" and
"sudo podman image exists microshift-okd && exit 1"), which breaks when
quickstart was run with a custom name; change these to reference a configurable
variable (e.g., CONTAINER_NAME or an action input) with a sensible default and
use that variable in both the podman ps/grep and podman image exists checks so
the cleanup verifies the actual container/image name used at runtime.
In `@src/quickclean.sh`:
- Line 15: The script hardcodes the container name "microshift-okd" (seen in the
image_ref assignment using podman inspect), so change it to use a configurable
container name (e.g., read from an environment variable or CLI arg) and fall
back to "microshift-okd" if unset; update the image_ref command and any other
occurrences of the literal "microshift-okd" in the script to use that variable
(e.g., QUICKSTART_CONTAINER_NAME or CONTAINER_NAME) so quickclean works for
custom quickstart names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 53982915-c345-40f8-aa4b-cd725711b36d
📒 Files selected for processing (4)
.github/actions/quick-start-clean/action.yaml.github/workflows/installers.yamlsrc/quickclean.shsrc/quickstart.sh
| IMAGE=${IMAGE:-"ghcr.io/${OWNER}/${REPO}"} | ||
| TAG=${TAG:-latest} | ||
|
|
||
| CONTAINER_NAME="${1:-microshift-okd}" |
There was a problem hiding this comment.
Container name mismatch with cleanup script.
quickclean.sh hardcodes microshift-okd on line 15, so custom container names passed to this script won't be cleaned up.
🔧 Options to fix
- Pass the container name to
quickclean.shas an argument (requires updating both scripts and the workflow action) - Use an environment variable to share the container name
- Remove the configurable container name feature if it's not needed
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/quickstart.sh` at line 9, The quickstart script defines a configurable
CONTAINER_NAME variable but quickclean.sh still hardcodes "microshift-okd", so
custom names won't be cleaned; fix by making quickclean.sh accept the container
name (e.g., read an argument $1 or environment variable) and update
quickstart.sh to either export CONTAINER_NAME before invoking quickclean.sh or
pass CONTAINER_NAME as the argument when calling quickclean.sh; specifically
change quickclean.sh to use the same symbol (CONTAINER_NAME) instead of the
literal and ensure quickstart.sh uses the same mechanism (export CONTAINER_NAME
or pass it) so both scripts reference the same container identifier.
| fi | ||
|
|
||
| if [ "$(id -u)" -ne 0 ]; then | ||
| local_rootful="$(podman machine inspect --format '{{.Rootful}}' 2>/dev/null || echo "false")" |
There was a problem hiding this comment.
Missing local declaration.
The variable name suggests local scope but it's not declared with the local keyword.
🔧 Proposed fix
- local_rootful="$(podman machine inspect --format '{{.Rootful}}' 2>/dev/null || echo "false")"
+ local local_rootful
+ local_rootful="$(podman machine inspect --format '{{.Rootful}}' 2>/dev/null || echo "false")"📝 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.
Suggested change
| local_rootful="$(podman machine inspect --format '{{.Rootful}}' 2>/dev/null || echo "false")" | |
| local local_rootful | |
| local_rootful="$(podman machine inspect --format '{{.Rootful}}' 2>/dev/null || echo "false")" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/quickstart.sh` at line 177, The variable local_rootful is intended to be
function-scoped but is declared without the local keyword; update the assignment
to declare it as a local variable (e.g., use local local_rootful when assigning
the result of podman machine inspect --format '{{.Rootful}}' 2>/dev/null || echo
"false") so it doesn't leak into the global environment; locate the assignment
of local_rootful in src/quickstart.sh and prepend the local keyword to the
variable declaration.
Comment on lines
+219
to
+222
| echo " - sudo podman exec -it ${CONTAINER_NAME} /bin/bash -l" | ||
| echo | ||
| echo "To verify that MicroShift pods are up and running, run the following command:" | ||
| echo " - sudo podman exec -it microshift-okd kubectl get pods -A" | ||
| echo " - sudo podman exec -it ${CONTAINER_NAME} kubectl get pods -A" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Instructions should be platform-aware.
On macOS, when not running as root, users don't need sudo for podman commands. The Darwin initialization (lines 166-183) allows non-root execution, but these instructions always show sudo.
🔧 Suggested approach
Conditionally emit instructions based on platform and execution context:
if [[ "$(uname -s)" == "Darwin" ]] && [ "$(id -u)" -ne 0 ]; then
EXEC_PREFIX=""
else
EXEC_PREFIX="sudo "
fi
echo "To access the container, run the following command:"
echo " - ${EXEC_PREFIX}podman exec -it ${CONTAINER_NAME} /bin/bash -l"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/quickstart.sh` around lines 219 - 222, Make the displayed podman commands
platform-aware by introducing an EXEC_PREFIX determined from uname and uid
(e.g., if uname -s == "Darwin" and id -u != 0 then EXEC_PREFIX="" else
EXEC_PREFIX="sudo "), then use that EXEC_PREFIX variable when emitting the
podman/kubectl examples (replace the hardcoded "sudo " in the lines that echo
the podman exec and kubectl get pods commands), referencing the existing
CONTAINER_NAME variable so outputs become "${EXEC_PREFIX}podman exec -it
${CONTAINER_NAME} /bin/bash -l" and "${EXEC_PREFIX}podman exec -it
${CONTAINER_NAME} kubectl get pods -A".
Contributor
Author
|
closed in favor of #220 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds macOS support to
quickstart.shandquickclean.sh. The same scripts now work on both Linux and macOS.On macOS, the scripts require a rootful podman machine:
Changes:
quickstart.sh: detect macOS, validate rootful podman machine, run LVM setup inside the VM viapodman machine sshquickclean.sh: detect macOS, clean up LVM inside the VM viapodman machine ssh./src/quickstart.sh [name]🤖 Generated with Claude Code