podman-desktop icon indicating copy to clipboard operation
podman-desktop copied to clipboard

feat: allow changing between rootless and rootful connection for an existing Podman machine

Open SoniaSandler opened this issue 6 months ago • 1 comments

What does this PR do?

This PR adds the option to switch between rootless or rootful connections on an existing Podman machine from the UI by exposing the rootful value in the editable properties of Podman machines. (works only on macOS as it is the only OS where the connection is editable)

  • The rootful property was hidden so it would not show up on the provider card as it messes up the layout: Screenshot 2025-06-09 at 9 14 41 PM

A possible follow up could be adding a way to indicate if the machine is rootful or rootless.

Screenshot / video of UI

https://github.com/user-attachments/assets/ad5347e6-89fd-4f62-98bd-73234450fc25

What issues does this PR fix or reference?

Closes #12498

How to test this PR?

  • [X] Tests are covering the bug fix or the new feature

SoniaSandler avatar Jun 10 '25 01:06 SoniaSandler

Codecov Report

:x: Patch coverage is 90.62500% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ensions/podman/packages/extension/src/extension.ts 88.00% 3 Missing :warning:

:loudspeaker: Thoughts on this report? Let us know!

codecov[bot] avatar Jun 10 '25 02:06 codecov[bot]

Question! Isn't it required to restart the machine for rootful status to take effect? Or did that change recently with upstream podman?

cdrage avatar Jul 10 '25 20:07 cdrage

@cdrage you're right, and it's already done in the existing code when editing a podman machine:

if (effective) {
        const state = podmanMachinesStatuses.get(machineInfo.name);
        try {
          if (state === 'started') {
            await lifecycle.stop?.(context, logger);
          }
          await execPodman(args, machineInfo.vmType, {
            logger: new LoggerDelegator(context, logger),
          });
        } finally {
          if (state === 'started') {
            await lifecycle.start?.(context, logger);
          }
        }
      }

SoniaSandler avatar Jul 10 '25 20:07 SoniaSandler

@cdrage you're right, and it's already done in the existing code when editing a podman machine:

if (effective) {
        const state = podmanMachinesStatuses.get(machineInfo.name);
        try {
          if (state === 'started') {
            await lifecycle.stop?.(context, logger);
          }
          await execPodman(args, machineInfo.vmType, {
            logger: new LoggerDelegator(context, logger),
          });
        } finally {
          if (state === 'started') {
            await lifecycle.start?.(context, logger);
          }
        }
      }

Ah thank you! I saw in the initial video on the PR it doesn't show the podman machine stopping / starting again? Unless it did it really fast.

cdrage avatar Jul 10 '25 20:07 cdrage

@cdrage I just moved to the images page right after the edit, but you can see in the statusbar provider tooltip in the video that the machine is starting again

SoniaSandler avatar Jul 10 '25 20:07 SoniaSandler

[!NOTE]

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Adds support for a Podman machine "rootful" configuration and editing flow. A new configuration property podman.machine.rootful (boolean, default true, scoped to ContainerConnection, conditionally visible, hidden) was added to the extension manifest. Extension logic now reads machine inspection Rootful values, updates container connection configuration with machine.rootful, exposes a provider lifecycle edit parameter to pass --rootful=<value> on supported platforms (macOS, Windows), and sets a new context key PODMAN_MACHINE_EDIT_ROOTFUL. Tests were updated to mock and assert Rootful handling. Renderer and several renderer stores were adjusted to respect hidden connection settings and to react to a new provider-container-connection-update-status event. ProviderRegistry now emits that event when connection status changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • podman-desktop/podman-desktop#12939: Modifies Podman extension machine edit capabilities (CPU/memory/disk) with similar lifecycle and manifest changes — closely related code areas.
  • podman-desktop/podman-desktop#13402: Large refactor touching PreferencesResourcesRendering.svelte; overlaps with this PR's conditional rendering change for hidden connection settings.

Suggested labels

qe/testing-required, qe/review

Suggested reviewers

  • cdrage
  • benoitf
  • jeffmaury

Pre-merge checks (4 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly captures the core feature of this pull request—enabling users to switch between rootless and rootful connections on existing Podman machines—and directly reflects the primary change without extraneous details.
Linked Issues Check ✅ Passed The implementation fully addresses issue #12498 by exposing and handling the rootful toggle in configuration, extending the lifecycle edit flow, updating tests, and closing the issue as requested, demonstrating compliance with the linked requirement.
Out of Scope Changes Check ✅ Passed All modifications—including the new configuration property, lifecycle handling, UI rendering adjustments, test updates, and event notifications—are directly tied to enabling and reflecting the rootful toggle feature, and there are no unrelated or extraneous changes.
Description Check ✅ Passed The description clearly explains that the PR exposes the rootful property for editing in the UI, details platform support, references the relevant issue, and includes testing and visual evidence, so it is directly related to and informative about the changeset.

[!TIP]

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing Touches
  • [ ] 📝 Generate Docstrings
🧪 Generate unit tests
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a 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.

coderabbitai[bot] avatar Jul 23 '25 20:07 coderabbitai[bot]