WIP: Add option to list all profiles #4512

djmgit · 2019-06-17T17:49:39Z

Created a new command minikube profiles to list
all the available profiles

Created a new command `minikube profiles` to list all the available profiles

k8s-ci-robot · 2019-06-17T17:49:48Z

Hi @djmgit. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

minikube-bot · 2019-06-17T17:50:03Z

Can one of the admins verify this patch?

k8s-ci-robot · 2019-06-17T17:50:20Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: djmgit
To complete the pull request process, please assign tstromberg
You can assign the PR to them by writing /assign @tstromberg in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

medyagh · 2019-06-17T20:32:38Z

I am not sure what is better,
to have a separate command to list profiles , minikube profiles
or add it to the existing profile (without s) command ?

something like minikube profile list

I could be convinced either way, what others think ? @afbjorklund

djmgit · 2019-06-18T07:02:17Z

@medyagh I liked the idea of minikube profile list too, however creating a new command like profiles, does not requiring changing the behavior of existing command.
To make minikube profile list work we would have to change the behavior of the profile command. We need to create a new sub command like minikube profile set <profile_name> or something like that (as you have mentioned too in the issue). So basically its like creating 2 new profile sub commands which is very much doable.
However as I mentioned before, creating a new command one level up like minikube profiles will not affect any existing code.
In my pr I have implemented minikube profiles command, however I can update it if required to implement the first way.

medyagh · 2019-06-19T07:53:25Z

pkg/util/kubeconfig.go

+	}
+	profiles := make([]string, 0, 4)
+	for context, _ := range config.Contexts {
+		profiles = append(profiles, context)


This is getting all kubecontexts... Inclduing the ones not created by minikube.

@medyagh the contexts which I get via the above way and the profiles present in ~/.minikube/profiles directory. Although it contains the profile created by kubernetes (minikube), shouldn't it be shown too? Because it is also there under the profiles directory.

that code will get all the kubectl contexts, so for example if I have a GKE cluster or another k8s cluster, they will be in the list of kubectl config get-contexts, and when you create minikube profile it will create another context.

if you get list of all contexts and show it as list of profiles, it will show my gke cluster or my other k8s cluster as a minikube profile , so it is wrong

djmgit · 2019-06-19T16:50:14Z

@medyagh the contexts which I get via the above way and the profiles present in ~/.minikube/profiles directory. Although it contains the profile created by kubernetes (minikube), shouldn't it be shown too? Because it is also there under the profiles directory.

djmgit · 2019-06-20T01:57:26Z

@medyagh got it, so I should instead list the contents of ~/.minikube/profiles . Will that be the right approach?

medyagh · 2019-06-20T02:20:05Z

@medyagh got it, so I should instead list the contents of ~/.minikube/profiles . Will that be the right approach?

Yes there are a few details :

make sure the profiles in ~/.minikube/profiles are valid profile ( not empty left over profile configs)
make sure the machine vm file also exists too

djmgit · 2019-06-20T03:42:20Z

@medyagh Got it.
Just to confirm, the following approach has to be followed :

Iterate over the profiles in ~/.minikube/profiles
Check if they are not empty (contains config.json)
Check if the corresponding machine with the respective profile name is present in ~/.minikube/machines
Create a list of such valid profiles and display

Does the above look fine?

medyagh · 2019-06-20T03:53:52Z

@medyagh Got it.
Just to confirm, the following approach has to be followed :

Iterate over the profiles in ~/.minikube/profiles

Check if they are not empty (contains config.json)

Check if the corresponding machine with the respective profile name is present in ~/.minikube/machines

Create a list of such valid profiles and display

Does the above look fine?

Yes that sounds greats, only think to validate the profile I wouldn't just check if config.json is not empty, I would check if it is loadable( parsable to a real config)

medyagh · 2019-06-25T23:01:02Z

@djmgit this PR needs a rebase, please let me know if you are still working on this PR

djmgit · 2019-06-26T04:54:34Z

@medyagh I am still on it, got caught up with some other stuff, will update and rebase my PR ASAP.

medyagh · 2019-07-01T22:02:12Z

@medyagh I am still on it, got caught up with some other stuff, will update and rebase my PR ASAP.

no worries, let me know if you need any help on this

fenglixa · 2019-07-12T06:59:28Z

cmd/minikube/cmd/config/profiles.go

+	"k8s.io/minikube/pkg/minikube/constants"
+)
+
+// ProfileCmd represents the profile command


Change "ProfileCmd" to "ProfilesCmd"

fenglixa · 2019-07-12T07:05:41Z

pkg/util/kubeconfig.go

+		return nil, errors.Wrap(err, "Error getting kubeconfig status")
+	}
+	profiles := make([]string, 0, 4)
+	for context, _ := range config.Contexts {


I guess you are mean: "for _, context := ..." here, is it right?

Pls run make test before committing the code

tstromberg · 2019-07-17T20:15:41Z

Just checking in - would you like any help with moving this PR forward?

medyagh · 2019-07-19T19:15:19Z

@djmgit Thanks for trying to make this PR work but have to close it PR in favor this one #4811 as this functionality is needed for the other PR depending on it.
. I welcome your future contributions.

Add option to list all profiles

e1d34b4

Created a new command `minikube profiles` to list all the available profiles

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 17, 2019

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 17, 2019

k8s-ci-robot requested review from afbjorklund and medyagh June 17, 2019 17:50

medyagh added the triage/discuss Items for discussion label Jun 17, 2019

medyagh requested changes Jun 19, 2019

View reviewed changes

medyagh added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 19, 2019

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 19, 2019

medyagh changed the title ~~Add option to list all profiles~~ WIP: Add option to list all profiles Jun 20, 2019

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 20, 2019

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 26, 2019

medyagh mentioned this pull request Jun 26, 2019

List and set minikube profiles #1716 #4608

Closed

fenglixa suggested changes Jul 12, 2019

View reviewed changes

medyagh closed this Jul 19, 2019

tstromberg removed the triage/discuss Items for discussion label Jun 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Add option to list all profiles #4512

WIP: Add option to list all profiles #4512

djmgit commented Jun 17, 2019

k8s-ci-robot commented Jun 17, 2019

minikube-bot commented Jun 17, 2019

k8s-ci-robot commented Jun 17, 2019

medyagh commented Jun 17, 2019 •

edited

Loading

djmgit commented Jun 18, 2019 •

edited

Loading

medyagh Jun 19, 2019

medyagh Jun 19, 2019 •

edited

Loading

djmgit commented Jun 19, 2019

djmgit commented Jun 20, 2019

medyagh commented Jun 20, 2019

djmgit commented Jun 20, 2019

medyagh commented Jun 20, 2019 •

edited

Loading

medyagh commented Jun 25, 2019

djmgit commented Jun 26, 2019

medyagh commented Jul 1, 2019

fenglixa Jul 12, 2019

fenglixa Jul 12, 2019

fenglixa Jul 12, 2019

tstromberg commented Jul 17, 2019

medyagh commented Jul 19, 2019

WIP: Add option to list all profiles #4512

WIP: Add option to list all profiles #4512

Conversation

djmgit commented Jun 17, 2019

k8s-ci-robot commented Jun 17, 2019

minikube-bot commented Jun 17, 2019

k8s-ci-robot commented Jun 17, 2019

medyagh commented Jun 17, 2019 • edited Loading

djmgit commented Jun 18, 2019 • edited Loading

medyagh Jun 19, 2019

Choose a reason for hiding this comment

medyagh Jun 19, 2019 • edited Loading

Choose a reason for hiding this comment

djmgit commented Jun 19, 2019

djmgit commented Jun 20, 2019

medyagh commented Jun 20, 2019

djmgit commented Jun 20, 2019

medyagh commented Jun 20, 2019 • edited Loading

medyagh commented Jun 25, 2019

djmgit commented Jun 26, 2019

medyagh commented Jul 1, 2019

fenglixa Jul 12, 2019

Choose a reason for hiding this comment

fenglixa Jul 12, 2019

Choose a reason for hiding this comment

fenglixa Jul 12, 2019

Choose a reason for hiding this comment

tstromberg commented Jul 17, 2019

medyagh commented Jul 19, 2019

medyagh commented Jun 17, 2019 •

edited

Loading

djmgit commented Jun 18, 2019 •

edited

Loading

medyagh Jun 19, 2019 •

edited

Loading

medyagh commented Jun 20, 2019 •

edited

Loading