-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Added --name flag and MINIKUBE_NAME env var for vm machine name #1320
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1320 +/- ##
==========================================
- Coverage 43.06% 43.03% -0.03%
==========================================
Files 48 48
Lines 2271 2277 +6
==========================================
+ Hits 978 980 +2
- Misses 1133 1137 +4
Partials 160 160
Continue to review full report at Codecov.
|
f74711e
to
ea8a26f
Compare
cmd/minikube/cmd/env.go
Outdated
@@ -160,7 +160,7 @@ func shellCfgSet(api libmachine.API) (*ShellConfig, error) { | |||
} | |||
|
|||
if noProxy { | |||
host, err := api.Load(constants.MachineName) | |||
host, err := api.Load(constants.GetMachineName()) |
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.
Can we move this from constants to config?
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.
done
2e849dd
to
fc4449a
Compare
@minikube-bot retest this please |
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.
Might be worth it to switch the env getting to use viper, since then we also get the config file for free. Either way, LGTM
pkg/minikube/config/config.go
Outdated
|
||
// Minipath is the path to the user's minikube dir | ||
func GetMachineName() string { | ||
if os.Getenv(MinikubeName) == "" { |
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.
Why not just use viper.GetString
here? Viper looks at env vars, so you could call it minikube-name
and that would translate to MINIKUBE_NAME
I think we should also add this to minikube config set/unset
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.
done. Also added "--name" as a flag to all minikube commands
fc4449a
to
18b65c5
Compare
18b65c5
to
1b79cfd
Compare
Fixes #886 |
Although I live the feature had been finally implemented, I have to say that how to set the active instance/VM is not really easy and friendly. Also I would think that using shortcut for options sound be allowed. minikube start --name=test |
@jorgemoralespou |
@aaron-prindle my main concern is about user experience. I find For the -n I would submit a PR but I'm no go developer so better if I don't 😉 I'll open a feature request. |
For anyone else who saw this and expected to be able to use the |
^^ this is due to #1466 "Changed --name to --profile / -p" |
@aaron-prindle @jorgemoralespou Hi, found this PR through Google while looking for this feature, it says, that this change was released in v0.20.0, however |
@fhemberger
This flag can be used in all minikube commands, it is not strictly a |
@aaron-prindle Ah, great. Thanks! 👍 |
So What's the commands for multiple minikubes ? |
Is this feature removed? |
This allows users to specify the vm name for minikube to allow for multiple minikubeVMs to be created. This enables multiple minikubes to be managed on a single machine.
Example use: