Podman machine resets all providers #22941
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashley-cui The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
Unclear how to test, I don't think we have a way of creating machines with different providers in CI? Going to put no tests needed for now, if anyone disagrees and has an idea for tests, let me know. |
ashley-cui
added
the
No New Tests
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Ephemeral COPR build failed. @containers/packit-build please check. |
ashley-cui
force-pushed
the
machreset
branch
3 times, most recently
from
06f0456
to
a91e0dd
Compare
|
Cockpit tests failed for commit a91e0dd. @martinpitt, @jelly, @mvollmer please check. |
ashley-cui
force-pushed
the
machreset
branch
3 times, most recently
from
c3d3a0b
to
ba80167
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
Ready for review, @containers/podman-maintainers PTAL |
| if !wsl.HasAdminRights() { | ||
| logrus.Warn("managing hyperv machines require admin authority.") | ||
| } else { |
There was a problem hiding this comment.
This will spit out a wring by default for basically every WSL user, I would assume most don't care about hyperV so showing this by default on evey reset seems wrong to me.
There was a problem hiding this comment.
I like the message, though. Drop to a Debug?
There was a problem hiding this comment.
I'm concerned that people would expect reset to hard reset, including removing HyperV machines. This is actually tricky, because doing a reset the way this is implemented right now means that when reset is run without admin rights, HyperV machine configs and files will be deleted but will not be unregistered from the hypervisor, since the machine conf dirs are deleted anyway. I also considered making podman machine reset an admin-only command to get around this problem but that would exclude WSL users who don't have admin access.
I can drop it to a debug, but I guess there's other issues here to think about, since nothing is perfect right now due to HyperV's perm limitations.
There was a problem hiding this comment.
Updated to only warn if not -f, since I think this is important information due to dangling hyperV machines.
ashley-cui
force-pushed
the
machreset
branch
3 times, most recently
from
74739ed
to
2955924
Compare
|
Hmm, needs #23118 to merge, then I'll rebase. |
|
@Luap99 PTAL again :) |
Podman machine reset now removes and resets machines from all providers availabe on the platform. On windows, if the user is does not have admin privs, machine will only reset WSL, but will emit a warning that it is unable to remove hyperV machines without elevated privs. Signed-off-by: Ashley Cui <acui@redhat.com>
|
Tests passed, ready to go. @containers/podman-maintainers PTAL |
|
/lgtm |
Podman machine reset now removes and resets machines from all providers availabe on the platform.
On windows, if the user is does not have admin privs, machine will only reset WSL, but will emit a warning that it is unable to remove hyperV machines without elevated privs.
Does this PR introduce a user-facing change?