Add update-connection to machine start and init
#27405
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: baude 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
openshift-ci
bot
added
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
There are big TOCTOU races here, I think this logic must be moved into the shim Start() call where we hold the startLock to prevent simultaneous starts from overwriting each other.
Because technically what can happen is you start machine 1, then you prompt (can take forever), then the machine 1 gets stopped, machine 2 is started prompts again, answered yes., then go back to the prompt of machine 1 and answer yes there as well.
Sure easy to say user problem and not our thing but I think it costs us nothing doing this under the start lock. Maybe it makes more sense to prompt before we actually start the machine and then we only have to rewrite the connection under the start lock.
cmd/podman/machine/init.go
Outdated
| // Set DefaultProvider | ||
| return checkAndSetDefConnection(cmd, initOpts.Name, initOpts.Rootful, setDefaultSystemConn) |
There was a problem hiding this comment.
aren't you calling this twice on init then? start already called it and now you call it again?
There was a problem hiding this comment.
you are right! how the hell did that get by me.
baude
force-pushed
the
setdefaultconnection
branch
2 times, most recently
from
c0e2585 to
c80e009
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
baude
force-pushed
the
setdefaultconnection
branch
from
c80e009 to
b010c0e
Compare
This allows users to set the associated machine's system connection to the system default when running `podman machine init --now` or `podman machine start`. It also changes the default bbehavior of these commands in that the user will be prompted and asked if they would like to switch the system connection. It also introduces a command line switch called `--update-connection`. If the switch is unset, then the user will be prmpted. If the command value is explicitly set to `false`, the user will not be prompted and the system connection will not be altered. If the value is set to `true`, the system connection will be made the default and the user will not be prompted. Signed-off-by: Brent Baude <[email protected]>
baude
force-pushed
the
setdefaultconnection
branch
from
b010c0e to
2786cab
Compare
|
Changes LGTM |
4 participants
This allows users to set the associated machine's system connection to the system default when running
podman machine init --noworpodman machine start. It also changes the default bbehavior of these commands in that the user will be prompted and asked if they would like to switch the system connection. It also introduces a command line switch called--update-connection. If the switch is unset, then the user will be prmpted. If the command value is explicitly set tofalse, the user will not be prompted and the system connection will not be altered. If the value is set totrue, the system connection will be made the default and the user will not be prompted.Does this PR introduce a user-facing change?