Delete context when stopped #9414
Conversation
|
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 Once the patch is verified, the new status will be reflected by the 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: prezha The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Can one of the admins verify this patch? |
Codecov Report
@@ 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
|
cmd/minikube/cmd/stop.go
Outdated
| 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) |
There was a problem hiding this comment.
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}
There was a problem hiding this comment.
@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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
yes, that sounds just about right :)
the update is on its way...
thanks @medyagh !
fixes: #9410
as stated in the original issue: if
keep-context-activeis not set totrue, the context for the stopped cluster should be deleted (instead of just unset as the current context) to avoid confusionexample:
current behaviour:
new behaviour: