-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Error out if --vm-driver is incompatible with existing VM #5016
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RA489 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 |
/assign @RA489 |
Do you mind sharing the output of the minikube start Before and After this PR in the description ? |
@medyagh sure. |
@medyagh The output of minikube start before was
|
@medyagh The output of minikube start After with none driver.
|
pkg/minikube/cluster/cluster.go
Outdated
@@ -104,8 +104,8 @@ func StartHost(api libmachine.API, config cfg.MachineConfig) (*host.Host, error) | |||
|
|||
if h.Driver.DriverName() != config.VMDriver { | |||
out.T(out.Empty, "\n") | |||
out.WarningT(`Ignoring --vm-driver={{.driver_name}}, as the existing "{{.profile_name}}" VM was created using the {{.driver_name2}} driver.`, | |||
out.V{"driver_name": config.VMDriver, "profile_name": cfg.GetMachineName(), "driver_name2": h.Driver.DriverName()}) | |||
out.WarningT(`The existing "{{.profile_name}}" VM was created using the {{.driver_name}} driver.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the way you worded this.
The description in #4998 was not very clear, but to mirror my intent, could you change this PR to:
- Omit all of these warnings if the driver flag was not set by the user:
if !cmd.Flags().Changed(vmDriver)
- If the driver flag was set, call
exit.WithCode(exit.Config, "<this message>")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tstromberg PTAL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for doing this!
/retest this please |
Anything I could help with to move this PR forwards? |
35da2ee
to
d6e78fb
Compare
Travis tests have failedHey @RA489, 1st Buildmake test
TravisBuddy Request Identifier: 0e9e92c0-d2ce-11e9-a210-7faf5261cf76 |
d6e78fb
to
1972f1c
Compare
Thank you! |
Remove "Ignoring --vm-driver" warning.
fixes #4998