Fix order of parameters to CurrentContext funcs #5439
Conversation
Apparently it is easy to get name and path swapped around.
k8s-ci-robot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: afbjorklund The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
| @@ -77,7 +77,7 @@ func runStop(cmd *cobra.Command, args []string) { | |||
| } | |||
|
|
|||
| machineName := pkg_config.GetMachineName() | |||
| err = kubeconfig.UnsetCurrentContext(constants.KubeconfigPath, machineName) | |||
| err = kubeconfig.UnsetCurrentContext(machineName, constants.KubeconfigPath) | |||
There was a problem hiding this comment.
This PR caused me to LOL because of how easy it was, but sad because of how difficult to detect it was.
There is a second hidden bug here: constants.KubeconfigPath does not respect the KUBECONFIG environment variable. If the filename isn't provided, we do the right thing. Here is my suggested change to reduce the number of paths to sadness in our code base:
- Change
*SetCurrentContextto either take a second argument, or not take a second argument. Having to support two behaviors in one function is a recipe for future mistakes. - If you prefer the 1-argument version, update
context_test.goto set the temp path via an alternative mechanism, such asKUBECONFIG - If you prefer the 2-argument version, update these the
kubeconfig.*Context()calls to passkubeconfig.PathFromEnv().
|
Never got around to updating this PR, but you can of course still improve |
Apparently it is easy to get name and path swapped around.
Fixes #5436
Also fixes the issue where "stop" didn't unset the current-context (due to profile name not matching).