Delete context when stopped #9414

prezha · 2020-10-07T21:10:17Z

as stated in the original issue: if keep-context-active is not set to true, the context for the stopped cluster should be deleted (instead of just unset as the current context) to avoid confusion

example:

❯ kubectl config get-contexts
CURRENT   NAME                          CLUSTER                    AUTHINFO                                                       NAMESPACE
          minikube                      minikube                   minikube
*         p2                            p2                         p2
          p3                            p3                         p3

❯ minikube stop -p p2
✋  Stopping node "p2"  ...
🛑  Powering off "p2" via SSH ...
🛑  1 nodes stopped.

current behaviour:

❯ kubectl config get-contexts
CURRENT   NAME                          CLUSTER                    AUTHINFO                                                       NAMESPACE
          minikube                      minikube                   minikube
          p2                            p2                         p2
          p3                            p3                         p3

new behaviour:

❯ kubectl config get-contexts
CURRENT   NAME                          CLUSTER                    AUTHINFO                                                       NAMESPACE
          minikube                      minikube                   minikube
          p3                            p3                         p3

❯ minikube start -p p2
😄  [p2] minikube v1.13.1 on "Opensuse-Tumbleweed"
✨  Using the docker driver based on existing profile
❗  docker is currently using the btrfs storage driver, consider switching to overlay2 for better performance
👍  Starting control plane node p2 in cluster p2
🔄  Restarting existing docker container for "p2" ...
🐳  Preparing Kubernetes v1.19.2 on Docker 19.03.8 ...
🔎  Verifying Kubernetes components...
🌟  Enabled addons: storage-provisioner, default-storageclass
🏄  Done! kubectl is now configured to use "p2" by default

❯ kubectl config get-contexts
CURRENT   NAME                          CLUSTER                    AUTHINFO                                                       NAMESPACE
          minikube                      minikube                   minikube
*         p2                            p2                         p2
          p3                            p3                         p3

sync-2020.10.02

k8s-ci-robot · 2020-10-07T21:10:25Z

Hi @prezha. 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.

k8s-ci-robot · 2020-10-07T21:10:35Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: prezha
To complete the pull request process, please assign priyawadhwa after the PR has been reviewed.
You can assign the PR to them by writing /assign @priyawadhwa 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

minikube-bot · 2020-10-07T21:12:03Z

Can one of the admins verify this patch?

codecov-io · 2020-10-07T21:19:48Z

Codecov Report

Merging #9414 into master will decrease coverage by 0.69%.
The diff coverage is 9.01%.

@@            Coverage Diff             @@
##           master    #9414      +/-   ##
==========================================
- Coverage   29.48%   28.78%   -0.70%     
==========================================
  Files         170      172       +2     
  Lines       10307    10606     +299     
==========================================
+ Hits         3039     3053      +14     
- Misses       6846     7129     +283     
- Partials      422      424       +2

Impacted Files	Coverage Δ
cmd/minikube/cmd/completion.go	`0.00% <ø> (ø)`
cmd/minikube/cmd/config/configure.go	`0.74% <ø> (ø)`
cmd/minikube/cmd/config/profile_list.go	`2.56% <0.00%> (-0.14%)`	⬇️
cmd/minikube/cmd/delete.go	`19.06% <0.00%> (-0.55%)`	⬇️
cmd/minikube/cmd/mount.go	`8.33% <0.00%> (ø)`
cmd/minikube/cmd/stop.go	`4.00% <0.00%> (-1.89%)`	⬇️
cmd/minikube/cmd/update-context.go	`0.00% <0.00%> (ø)`
pkg/drivers/common.go	`8.00% <0.00%> (ø)`
pkg/drivers/kic/oci/cli_runner.go	`0.00% <0.00%> (ø)`
pkg/drivers/kic/oci/errors.go	`0.00% <ø> (ø)`
... and 33 more

medyagh · 2020-10-07T21:27:27Z

cmd/minikube/cmd/stop.go

-		if err := kubeconfig.UnsetCurrentContext(profile, kubeconfig.PathFromEnv()); err != nil {
-			exit.Error(reason.HostKubeconfigUnset, "update config", err)
+		if err := kubeconfig.DeleteContext(profile, kubeconfig.PathFromEnv()); err != nil {
+			exit.Error(reason.HostKubeconfigUpdate, "update config", err)


thank you for this quick PR, lets create a new Reason Exit here
https://github.com/medyagh/minikube/blob/82a5189b934ea02d127be85dbdba50dfa4282984/pkg/minikube/reason/reason.go#L207

HostKubeconfigDelete = Kind{ID: "HOST_KUBECONFIG_DELETE", ExitCode: ExHostConfig}

@medyagh you are most welcome!
that did cross my mind, but HostKubeconfigDelete sounded a bit wrong (we are not going to delete kubeconfig), while HostKubeconfigUpdate not so bad as we are effectively just updating the kubeconfig
on the other hand, maybe:

HostKubeconfigContextDelete = Kind{ID: "HOST_KUBECONFIG_CONTEXT_DELETE", ExitCode: ExHostConfig}

would be more appropriate?
what do you think?

that is actually good thinking ! I like that

how about something shorter but unique

"HostKubeconfigDeleteCtx"

the goal is if users collect these exit codes as metrics, they should find out what was the cause of the problem in a unique word

yes, that sounds just about right :)
the update is on its way...
thanks @medyagh !

medyagh

thanks

prezha added 2 commits October 2, 2020 22:42

Merge pull request #1 from kubernetes/master

be196bf

sync-2020.10.02

delete context when minikube is stopped

65a2618

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 7, 2020

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 7, 2020

k8s-ci-robot requested review from blueelvis and sharifelgamal October 7, 2020 21:10

medyagh requested changes Oct 7, 2020

View reviewed changes

add HostKubeconfigDeleteCtx Reason Exit

c63de60

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 7, 2020

prezha requested a review from medyagh October 7, 2020 21:59

medyagh approved these changes Oct 7, 2020

View reviewed changes

medyagh merged commit c459fea into kubernetes:master Oct 8, 2020

fmotrifork mentioned this pull request Oct 15, 2020

minikube 1.14.0 fishworks/fish-food#1016

Merged

prezha mentioned this pull request Feb 19, 2021

REQUEST: New membership for prezha kubernetes/org#2514

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete context when stopped #9414

Delete context when stopped #9414

prezha commented Oct 7, 2020 •

edited

Loading

k8s-ci-robot commented Oct 7, 2020

k8s-ci-robot commented Oct 7, 2020

minikube-bot commented Oct 7, 2020

codecov-io commented Oct 7, 2020 •

edited

Loading

medyagh Oct 7, 2020 •

edited

Loading

prezha Oct 7, 2020

medyagh Oct 7, 2020

prezha Oct 7, 2020

medyagh left a comment

Delete context when stopped #9414

Delete context when stopped #9414

Conversation

prezha commented Oct 7, 2020 • edited Loading

k8s-ci-robot commented Oct 7, 2020

k8s-ci-robot commented Oct 7, 2020

minikube-bot commented Oct 7, 2020

codecov-io commented Oct 7, 2020 • edited Loading

Codecov Report

medyagh Oct 7, 2020 • edited Loading

Choose a reason for hiding this comment

prezha Oct 7, 2020

Choose a reason for hiding this comment

medyagh Oct 7, 2020

Choose a reason for hiding this comment

prezha Oct 7, 2020

Choose a reason for hiding this comment

medyagh left a comment

Choose a reason for hiding this comment

prezha commented Oct 7, 2020 •

edited

Loading

codecov-io commented Oct 7, 2020 •

edited

Loading

medyagh Oct 7, 2020 •

edited

Loading