feat: Add --allow-rootless flag for rootless Podman support#68
feat: Add --allow-rootless flag for rootless Podman support#68AlexonOliveiraRH wants to merge 3 commits intominc-org:mainfrom
Conversation
Enable running MicroShift in Container (minc) under rootless Podman without requiring sudo or root privileges. This resolves all blockers identified in issue minc-org#22 by addressing eight distinct problems discovered during rootless bringup: 1. CRI-O cgroup manager fails without D-Bus user session - switched to cgroupfs via a 20-rootless.conf drop-in that overrides 10-microshift.conf. 2. IPv6 localhost resolution causes etcd connection failure - disabled IPv6 inside the container so glibc resolves localhost to 127.0.0.1. 3. oom_score_adj writes fail with EPERM in user namespaces - added a crun-rootless wrapper that strips oomScoreAdj from OCI specs before invoking crun. 4. cpuset cgroup controller not delegated - documented host-side systemd drop-in requirement (same as kind rootless docs). 5. chown projected volumes fails with EINVAL for high UIDs - documented subordinate UID/GID range expansion for OpenShift SCC. 6. crio-subid.service failure inside container - no impact, CRI-O continues to function (subordinate IDs managed by host Podman). 7. /dev/kmsg not accessible in rootless namespaces - mount /dev/null as /dev/kmsg so kubelet OOM watcher proceeds without error. 8. Kubelet needs user namespace awareness - enabled KubeletInUserNamespace feature gate via MicroShift config drop-in. Code changes: - New --allow-rootless persistent flag with viper/config support - Refactored Podman provider from global state to instance fields - Dynamic Podman graph root detection (rootless uses ~/.local/share/) - Auto-generated rootless configs (MicroShift, CRI-O, crun wrapper) written to ~/.config/minc/ and bind-mounted into the container - Rootless marker file (~/.config/minc/.rootless-cluster) persists mode across create/delete lifecycle - Shared MicroShift retry constants (40 retries / ~26 min) to accommodate slower rootless startup Fixes minc-org#22 Signed-off-by: Alexon Oliveira <alolivei@redhat.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughAdds rootless container support across the CLI and providers: new persistent flag and default config key, an on-disk rootless marker, provider-level rootless options and config generation, podman provider refactor to instance mode, and centralized retry constants for MicroShift service polling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "minc CLI"
participant Marker as "Rootless Marker"
participant Registry as "Provider Registry"
participant Provider as "Podman Provider"
participant Config as "Rootless Config Generator"
User->>CLI: minc create --allow-rootless
CLI->>Marker: Set()
CLI->>Registry: Register("podman")
Registry->>Registry: read allow-rootless (viper OR marker)
Registry->>Provider: New(allowRootless=true)
CLI->>Provider: Create(options)
Provider->>Provider: Query Podman store.GraphRoot
Provider->>Config: generate MicroShift YAML / CRI-O drop-in / crun wrapper
Config-->>Provider: artifacts written
Provider-->>CLI: Create complete
User->>CLI: minc delete
CLI->>Provider: Delete()
Provider-->>CLI: Delete complete
CLI->>Marker: Remove()
Marker-->>CLI: Removed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/minc/config.go`:
- Line 27: The "allow-rootless" boolean is being written as a string by `config
set` and thus won't be recognized by `writeConfigWithoutDefaults()` as the
default false; update the code path that handles `config set` (the code that
stores raw args as strings) to detect boolean-like strings ("true"/"false") and
coerce them to actual bool types before persisting, or alternatively add a small
normalization step in `writeConfigWithoutDefaults()` to parse string values for
known boolean keys (e.g., "allow-rootless") back to bool; ensure the change
references the `allow-rootless` key and the `writeConfigWithoutDefaults()`
function so the stored JSON uses a real boolean rather than a string.
In `@cmd/minc/main.go`:
- Around line 114-115: The call to rootlessmarker.Remove() is currently ignoring
errors which can leave stale CLI state while still printing success; update the
delete path in main (where rootlessmarker.Remove() is called) to check the
returned error, and if non-nil print a clear error to stderr (e.g., via
fmt.Fprintf(os.Stderr,...)) and exit non-zero (os.Exit(1)) or return the error
from the command handler so the process reports failure; ensure you reference
and handle the error from rootlessmarker.Remove() and avoid printing "Item
deleted" on failure.
- Around line 60-76: The deferred rootlessmarker.Remove() is never reached
because log.Fatal exits immediately after minc.Create fails; change the error
path so the marker is cleaned up explicitly before exiting: after err :=
minc.Create(cType) if err != nil { if allowRL { _ = rootlessmarker.Remove() }
log.Fatal("error creating cluster", "err", err) } or alternatively replace
log.Fatal with a non-exiting log call and then ensure you call
rootlessmarker.Remove() before os.Exit; reference minc.Create,
rootlessmarker.Remove, allowRL and createFailed when making the change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 74c984c0-c2e4-4c57-9e5c-43793fc2ccaa
📒 Files selected for processing (8)
cmd/minc/config.gocmd/minc/main.gopkg/providers/moby/provider.gopkg/providers/options.gopkg/providers/podman/provider.gopkg/providers/register/register.gopkg/providers/wait.gopkg/rootlessmarker/marker.go
| "http-port": "9080", | ||
| "microshift-config": "", | ||
| "disable-overlay-cache": false, | ||
| "allow-rootless": false, |
There was a problem hiding this comment.
allow-rootless will not round-trip cleanly through config set.
config set stores raw args as strings, so minc config set allow-rootless false will be written as "false" and writeConfigWithoutDefaults() will never treat it as the default false bool. The new key will linger in the JSON with the wrong type.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/minc/config.go` at line 27, The "allow-rootless" boolean is being written
as a string by `config set` and thus won't be recognized by
`writeConfigWithoutDefaults()` as the default false; update the code path that
handles `config set` (the code that stores raw args as strings) to detect
boolean-like strings ("true"/"false") and coerce them to actual bool types
before persisting, or alternatively add a small normalization step in
`writeConfigWithoutDefaults()` to parse string values for known boolean keys
(e.g., "allow-rootless") back to bool; ensure the change references the
`allow-rootless` key and the `writeConfigWithoutDefaults()` function so the
stored JSON uses a real boolean rather than a string.
| _ = rootlessmarker.Remove() | ||
| fmt.Println("Item deleted") |
There was a problem hiding this comment.
Don't silently ignore marker cleanup failures after delete.
If rootlessmarker.Remove() fails here, the command still reports success while leaving stale CLI state behind for the next Podman operation.
Possible fix
- _ = rootlessmarker.Remove()
+ if err := rootlessmarker.Remove(); err != nil {
+ fmt.Fprintf(os.Stderr, "warning: deleted cluster but failed to clear rootless marker: %v\n", err)
+ }
fmt.Println("Item deleted")📝 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.
| _ = rootlessmarker.Remove() | |
| fmt.Println("Item deleted") | |
| if err := rootlessmarker.Remove(); err != nil { | |
| fmt.Fprintf(os.Stderr, "warning: deleted cluster but failed to clear rootless marker: %v\n", err) | |
| } | |
| fmt.Println("Item deleted") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/minc/main.go` around lines 114 - 115, The call to rootlessmarker.Remove()
is currently ignoring errors which can leave stale CLI state while still
printing success; update the delete path in main (where rootlessmarker.Remove()
is called) to check the returned error, and if non-nil print a clear error to
stderr (e.g., via fmt.Fprintf(os.Stderr,...)) and exit non-zero (os.Exit(1)) or
return the error from the command handler so the process reports failure; ensure
you reference and handle the error from rootlessmarker.Remove() and avoid
printing "Item deleted" on failure.
Address two issues found during code review: 1. log.Fatal calls os.Exit(1) which skips deferred functions. The rootless marker was set before minc.Create() but the deferred cleanup never ran on failure, leaving a stale marker that would cause future commands to incorrectly assume rootless mode. Fix: explicitly remove the marker before log.Fatal. 2. Marker removal errors on delete were silently ignored. If Remove() fails (e.g. permission issue), the user now sees a warning instead of a false success message. Signed-off-by: Alexon Oliveira <alolivei@redhat.com>
Hi @praveenkumar, thanks for the initial investigation on this issue. Your findings around The Regarding the CodeRabbit comment about I tested this on Fedora 43 / kernel 6.19 / rootless Podman with the |
|
Tip For best results, initiate chat on the files or code changes.
Noted on the ✏️ Learnings added
|
Add an "Allow rootless Podman" checkbox to the cluster creation form that passes --allow-rootless to the minc CLI. Change the Linux audit from a blocking error to an informational message when running as non-root, since rootless mode is now a supported configuration. Depends on minc-org/minc#68.
| // MicroShiftServiceMaxRetries bounds how long we poll systemctl is-active for the microshift | ||
| // unit inside the container. While the unit is "activating", is-active exits non-zero (often 3), | ||
| // so each poll is a retry. Rootless or slow storage can keep the unit activating for many minutes. | ||
| // With pkg/retry's delay = InitialDelay * attempt, 40 attempts implies ~26 minutes of backoff before failure. |
There was a problem hiding this comment.
how does this implies ~26 mins delay, to me it is 40 attempts with every 2 second delay which should be ~80 sec.
There was a problem hiding this comment.
how does this implies ~26 mins delay, to me it is 40 attempts with every 2 second delay which should be ~80 sec.
@praveenkumar, you're right that at first glance it looks like 80s, but pkg/retry/retry.go uses linear backoff (initialDelay * attempt), not constant delay. So attempt 1 sleeps 2s, attempt 2 sleeps 4s, ..., attempt 39 sleeps 78s. The total is 2 * sum(1..39) = 1560s ≈ 26 min. I'll update the comment to make the math clearer.
|
So I just testing this PR without going through the codebase and during my testing all the pods are not coming up, some issue around mount |
The comment said "~26 minutes" but did not show the calculation, which was confusing because 40 * 2s looks like ~80s at first glance. pkg/retry uses linear backoff (sleep = initialDelay * attempt), so the actual total is 2s * sum(1..39) = 1560s ≈ 26 min. Signed-off-by: Alexon Oliveira <alolivei@redhat.com>
@praveenkumar, this is After that, |
Add an "Allow rootless Podman" checkbox to the cluster creation form that passes --allow-rootless to the minc CLI. Change the Linux audit from a blocking error to an informational message when running as non-root, since rootless mode is now a supported configuration. Depends on minc-org/minc#68. Signed-off-by: Alexon Oliveira <alolivei@redhat.com>
Hey @praveenkumar, just a follow-up on this: the companion PR on the extension side (minc-org/minc-extension#396) has been tested and approved by @danivilla9. During testing, he also initially hit the same port binding error ( The chown error you encountered is the same scenario, problem #5 in the PR description. It requires expanding the subordinate UID/GID ranges on the host (Step 4 in "Host Prerequisites") so that OpenShift SCC UIDs (~1000100000) fall within the mapped range: Could you give it another try with all four host prerequisites applied? Happy to help troubleshoot if anything else comes up. |
* feat: add rootless Podman support for Linux Add an "Allow rootless Podman" checkbox to the cluster creation form that passes --allow-rootless to the minc CLI. Change the Linux audit from a blocking error to an informational message when running as non-root, since rootless mode is now a supported configuration. Depends on minc-org/minc#68. Signed-off-by: Alexon Oliveira <alolivei@redhat.com> * fix: reword audit message to conditional phrasing The audit info message now instructs the user to enable the toggle rather than asserting the cluster will be created in rootless mode, since the audit runs independently of the form parameters. Signed-off-by: Alexon Oliveira <alolivei@redhat.com> --------- Signed-off-by: Alexon Oliveira <alolivei@redhat.com>
Summary
This PR adds rootless Podman support to minc, enabling
minc createwithoutsudoor root privileges via a new--allow-rootlessflag. It resolves all blockers identified in #22.With all changes applied, all MicroShift system pods reach
Runningstatus under rootless Podman on Fedora 43 / kernel 6.19 / crun 1.23.Problems Solved
Eight distinct issues were discovered and resolved during rootless bringup:
Failed to connect to bus: No medium found) - no D-Bus user session in rootless20-rootless.conf) switching tocgroupfs, overriding10-microshift.conf--sysctl net.ipv6.conf.all.disable_ipv6=1at container creationoom_score_adjwrites fail with EPERM in user namespacescrun-rootlesswrapper stripsoomScoreAdjfrom OCI spec viajqcpusetcgroup controller not delegated to user sessionschownfails with EINVAL for high OpenShift SCC UIDscrio-subid.servicefailure inside container/dev/kmsgnot accessible in rootless namespaces/dev/nullas/dev/kmsgKubeletInUserNamespacefeature gate via MicroShift config drop-inCode Changes
cmd/minc/main.go--allow-rootlesspersistent flag, rootless marker lifecyclecmd/minc/config.goallow-rootlessin default configpkg/providers/podman/provider.gopkg/providers/options.gopkg/providers/moby/provider.gopkg/providers/register/register.goallowRootlessto provider constructorpkg/providers/wait.gopkg/rootlessmarker/marker.go~/.config/minc/.rootless-clusterKey design decisions
createanddeleteso subsequent commands (delete, generate-kubeconfig) work without re-specifying--allow-rootless.useSudovariable andsync.Oncewith instance fields on the provider struct, eliminating shared mutable state.Store.GraphRoot) instead of hardcoding/var/lib/containers/storage, which differs in rootless mode.~/.config/minc/and bind-mounted into the container at creation time.20-prefix ensures rootless configs override MicroShift's10-microshift.conf.Host Prerequisites (One-Time Setup)
Four host-side changes are required before using
--allow-rootless. All require root/sudo and persist across reboots.1. Kernel parameters (sysctl):
ip_forward- required for container networking (packet forwarding)ip_unprivileged_port_start = 80- allows rootless Podman to bind HTTP/HTTPS ports (80, 443)max_user_instances = 1024- increases inotify watch limit for MicroShift services2. Load
ip_tableskernel module:3. Delegate cpuset cgroup controller:
4. Expand subordinate UID/GID ranges (for OpenShift SCC high UIDs):
sudo usermod --add-subuids 165536-1265535999 --add-subgids 165536-1265535999 $(whoami) podman system migrateUsage
Test Environment
Test Results
All 9 MicroShift system pods reach
Runningstatus under rootless Podman:Future Work
--rootlessflag fixingoom_score_adj)References
Fixes #22