Minikube: UI: Advice when user tries to set profile name to "delete" or "start"

[medyagh]

to delete a minikube profile you need to do

minikube delete -p prof_name

but if you do

minikube profile delete

it will actually create a profile named delete. because that command sets profile name.

we need to at lest add an advice to the user, that says

Did you mean to delete a profile ? and here is the command to delete profile !

I recommend the same approach for other key words such as, start, stop, status, ... to make it less confusing for new users.

we could add this to the Unit tests of profile, so if someone tries to set the profile name to a list of reserved words, it should return an error.

[bhanu-lab]

i would like to pick this up. /assign

[bhanu-lab]

/assign

[medyagh]

its yours ! @bhanu011 let me know if you need help

@bhanu011 I just updated the description, we need to have a unit test for profile that checks for a list of reservered words and returns error if you try to set that name

[bhanu-lab]

@medyagh when user tries to create profiles with reserved keywords(delete, start, stop, status, etc ...) system should restrict the user from creating and throw an error?
If that is the scenario i will use exit.WithError() for throwing errors

[medyagh]

I would say exit with an Advice, and yes it should not allow you create a profile in the reserved names.

the reserved names could be the basic commands,
start,stop, status, delete, config

[medyagh]

@bhanu011 we had a discussion on this in minikube office hours.

Please sync with @josedonizetti to handle this issue

[bhanu-lab]

@medyagh ok @josedonizetti any changes to the design discussed in above comments?

[medyagh]

@bhanu011 yes we had a change in design, you could still work on it if you like, please sync with @josedonizetti for any help you need with this PR !

[josedonizetti]

@bhanu011 Currently if you do minikube profiler <NAME> this will create a new profile with the given name, and switch you to it. After discussing this today we thought that profile creation should only happen by the start command, and the profile should be only used to switch among existing profiles.

Basically, we need to change profile to validate if a profile exists before switching and if not warn the user.

Let me know if you still want to work on this! And if you have any questions. :)

[bhanu-lab]

@josedonizetti i am done with code according to previous design i was just writing unit test cases. I would like to continue working on this. Here are few questions I have

1. where can i get all existing profiles? is it from this folder~/.minikube/profiles/ or is there any better way?
2. What should happen when user trying to create profiles with kind of reserved words like delete, stop etc? Do we need to allow the user to create profile with that name?
3. If we are allowing we should only show some advice like if you are looking for this try with this command ?

[josedonizetti]

@bhanu011

1. https://github.com/kubernetes/minikube/blob/master/pkg/minikube/config/profile.go#L40
2. Yup, no validation here, if the user intentionally do minikube start -p delete it's okay
3. Didn't understand the question.

[bhanu-lab]

1. If user tries to create profile with minikube profile delete do we need to show any info message like use minikube delete -p prof_name for deleting

[josedonizetti]

@bhanu011 That's the thing, minikube profile won't create profiles anymore. It will be only to switch among existing profiles, creating will only be done by minikube start -p <PROFILE_NAME>.

[bhanu-lab]

@josedonizetti ok got it, i will implement changes

[bhanu-lab]

@medyagh @josedonizetti sorry for posting here, but i am getting below error when doing go build

# k8s.io/minikube/pkg/minikube/translate
../../pkg/minikube/translate/translate.go:77:12: undefined: Asset

Could you please help on this I am not able to find root cause/fix

[josedonizetti]

@bhanu011 if you rebase from the latest master you shouldn't get this problem when building make, but you also should be able to do to make pkg/minikube/assets/assets.go to regenerate the asset.go

[rzr]

Relate-to: https://github.com/kubernetes/minikube/issues/4909

[bhanu-lab]

@bhanu011

1. https://github.com/kubernetes/minikube/blob/master/pkg/minikube/config/profile.go#L40

@josedonizetti looks like valid profiles doesnt return default profile "minikube" which is a valid profile when profile name is given as "default" right?

[josedonizetti]

@bhanu011 sorry for the delay, was away last week. I have just tested ListProfiles locally and it did list all my 3 profiles as valid, including the default one. Did you rebase your code from master?
Maybe delete/recreate the main profile, might be from an older version?

[woodcockjosh]

@bhanu011 @tstromberg when I run minikube profile options it sets the profile to options instead of listing the command options. Will this PR fix this issue?

[nanikjava]

@josedonizetti @medyagh If the original assignee is not working on this I'm happy to take over this issue ?. Let me know. Thanks

[nanikjava]

Looks like this issue need to be closed as it's been merged to master.

[josedonizetti]

@nanikjava Do you want to work on https://github.com/kubernetes/minikube/issues/4913? I haven't had much time to look into it. Let me know and I'll assign it to you.

[nanikjava]

@josedonizetti yes please assign it to me. Thanks

[woodcockjosh]

@nanikjava you can assign to yourself by adding a comment with /assign on its own line

[nanikjava]

/assign

[nanikjava]

@josedonizetti PR is ready for review https://github.com/kubernetes/minikube/pull/5624

Thanks