Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DefaultPath is not always path used for kubeconfig #228

Closed
kschumy opened this issue Sep 26, 2018 · 3 comments · Fixed by #1763
Closed

DefaultPath is not always path used for kubeconfig #228

kschumy opened this issue Sep 26, 2018 · 3 comments · Fixed by #1763
Labels
help wanted Extra attention is needed

Comments

@kschumy
Copy link

kschumy commented Sep 26, 2018

What happened?
I'm not sure if this is a bug or intentionally designed this way, but I figured I'd bring it up just in case.

doCreateCluster() uses DefaultPath as the kubeconfig path when calling kubeconfig.Write(). DefaultPath is set to $HOME/.kube/config. However, when it goes to actually write the config file with ModifyConfig() in clientcmd/config.go, precidence is given to the KUBECONFIG env variable(s) and $HOME/.kube/config (ie DefaultPath) is used only if there is nothing assigned to KUBECONFIG.

DefaultPath seems to imply that is the path that will be used, but this isn't always the case. While it seems that most people have $HOME/.kube/config assigned to KUBECONFIG and thus this issue doesn't visibly impact them, having it set to this path is not a requirement.

What you expected to happen?
I expected DefaultPath to be the path that will be used for the kubeconfig.

How to reproduce it?
See gist for possibly too many details and printed logger statements to demonstrate this.

Anything else we need to know?
I discovered this while working on a PR to delete the cluster from the kubeconfig file. Since I was unsure of if this was actually a bug, I opted to treat it as intentional and used the same logic as doCreateCluster() to delete the cluster. Since the logic is similar, removing the cluster from the kubeconfig will have the same impact as described above

Versions
NOTE: I was using a branch off my fork of master

~ $ eksctl version
2018-09-26T12:20:59-07:00 [ℹ]  versionInfo = map[string]string{"gitCommit":"e577962c2a3d25af5dcc7995340339c4cdfee804", "gitTag":"0.1.3", "builtAt":"2018-09-19T16:07:46Z"}
~ $ uname -a
Darwin c4b301c23913.ant.amazon.com 16.7.0 Darwin Kernel Version 16.7.0: Thu Jun 21 20:07:39 PDT 2018; root:xnu-3789.73.14~1/RELEASE_X86_64 x86_64
~ $ kubectl version
Client Version: version.Info{Major:"1", Minor:"10", GitVersion:"v1.10.3", GitCommit:"2bba0127d85d5a46ab4b778548be28623b32d0b0", GitTreeState:"clean", BuildDate:"2018-07-26T20:40:11Z", GoVersion:"go1.9.3", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"10+", GitVersion:"v1.10.3-eks", GitCommit:"58c199a59046dbf0a13a387d3491a39213be53df", GitTreeState:"clean", BuildDate:"2018-09-21T21:00:04Z", GoVersion:"go1.9.3", Compiler:"gc", Platform:"linux/amd64"}

Logs
See gist in "How to reproduce it?" section

@richardcase
Copy link
Contributor

Great spot @kschumy.

My take on this is that we should have the same behaviour as kubectl. With kubectl the --kubeconfig flag takes precendence over the KUBECONFIG environment variable. This makes sense to me.

With just the env var:

KUBECONFIG=env.config  kubectl get nodes --v=10
I0927 11:18:16.684990   25631 loader.go:359] Config loaded from file env.config

With the env var and flag:

KUBECONFIG=env.config  kubectl get nodes --kubeconfig='flag.config' --v=10
I0927 11:19:28.889862   26591 loader.go:359] Config loaded from file flag.config

Would you agree @errordeveloper and @christopherhein ?

@richardcase richardcase added the help wanted Extra attention is needed label Sep 27, 2018
@christopherhein
Copy link
Contributor

I would agree, resolving down to the typical kubectl load ordering would cause the least of amount of confusion for customers that are used to the way the standard kubernetes tools work. 💯 @richardcase

@errordeveloper
Copy link
Contributor

@kschumy this is a very nice and detailed bug report, I apologies for a belated response... I'd be keen to see a PR that fixes it, if you need any help - let's chat on Slack.

prometherion added a commit to prometherion/eksctl that referenced this issue Jan 27, 2020
This commit aims to make aware `eksctl` of the `KUBECONFIG` environment
variable, especially during cluster creation and kubeconfig retrieval
via the `eksctl utils write-kubeconfig` command.
To avoid conflicts and keep the actual convention, the variable
`DefaultPath` has been declared using a simple _func_ in order to don't
perform an invasive refactoring requested by a switch to function
declaration.
Actually, using the standard library to retrieve the environment
variable and not using the `viper` helper (although already a dependency)
since the `KUBECONFIG` ome is considered as a global one, so totally
unbounded from `eksctl`.
Code coverage is going to decrease since during the test suite setup,
the environment variable is evaluated before the execution of tests: to
provide further coverage, the conversion from variable to function has
to be performed, although this requires some changes in the following
files and should be considered and approved by maintainers:
- pkg/ctl/cmdutils/cmdutils.go
- pkg/ctl/create/cluster.go
- pkg/ctl/utils/write_kubeconfig.go
- pkg/utils/kubeconfig/kubeconfig.go

Closes eksctl-io#228
prometherion added a commit to prometherion/eksctl that referenced this issue Jan 27, 2020
This commit aims to make aware `eksctl` of the `KUBECONFIG` environment
variable, especially during cluster creation and kubeconfig retrieval
via the `eksctl utils write-kubeconfig` command.
To avoid conflicts and keep the actual convention, the variable
`DefaultPath` has been declared using a simple _func_ in order to don't
perform an invasive refactoring requested by a switch to function
declaration.
Actually, using the standard library to retrieve the environment
variable and not using the `viper` helper (although already a dependency)
since the `KUBECONFIG` one is considered as a global one, so totally
unbounded from `eksctl`.
Code coverage is going to decrease since during the test suite setup,
the environment variable is evaluated before the execution of tests: to
provide further coverage, the conversion from variable to function has
to be performed, although this requires some changes in the following
files and should be considered and approved by maintainers:
- pkg/ctl/cmdutils/cmdutils.go
- pkg/ctl/create/cluster.go
- pkg/ctl/utils/write_kubeconfig.go
- pkg/utils/kubeconfig/kubeconfig.go

Closes eksctl-io#228
prometherion added a commit to prometherion/eksctl that referenced this issue Feb 6, 2020
This commit aims to make aware `eksctl` of the `KUBECONFIG` environment
variable, especially during cluster creation and kubeconfig retrieval
via the `eksctl utils write-kubeconfig` command.
To avoid conflicts and keep the actual convention, the variable
`DefaultPath` has been declared using a simple _func_ in order to don't
perform an invasive refactoring requested by a switch to function
declaration.
Actually, using the standard library to retrieve the environment
variable and not using the `viper` helper (although already a dependency)
since the `KUBECONFIG` one is considered as a global one, so totally
unbounded from `eksctl`.
Code coverage is going to decrease since during the test suite setup,
the environment variable is evaluated before the execution of tests: to
provide further coverage, the conversion from variable to function has
to be performed, although this requires some changes in the following
files and should be considered and approved by maintainers:
- pkg/ctl/cmdutils/cmdutils.go
- pkg/ctl/create/cluster.go
- pkg/ctl/utils/write_kubeconfig.go
- pkg/utils/kubeconfig/kubeconfig.go

Closes eksctl-io#228
prometherion added a commit to prometherion/eksctl that referenced this issue Feb 6, 2020
This commit aims to make aware `eksctl` of the `KUBECONFIG` environment
variable, especially during cluster creation and kubeconfig retrieval
via the `eksctl utils write-kubeconfig` command.
To avoid conflicts and keep the actual convention, the variable
`DefaultPath` has been declared using a simple _func_ in order to don't
perform an invasive refactoring requested by a switch to function
declaration.
Actually, using the standard library to retrieve the environment
variable and not using the `viper` helper (although already a dependency)
since the `KUBECONFIG` one is considered as a global one, so totally
unbounded from `eksctl`.
Code coverage is going to decrease since during the test suite setup,
the environment variable is evaluated before the execution of tests: to
provide further coverage, the conversion from variable to function has
to be performed, although this requires some changes in the following
files and should be considered and approved by maintainers:
- pkg/ctl/cmdutils/cmdutils.go
- pkg/ctl/create/cluster.go
- pkg/ctl/utils/write_kubeconfig.go
- pkg/utils/kubeconfig/kubeconfig.go

Closes eksctl-io#228
prometherion added a commit to prometherion/eksctl that referenced this issue Feb 6, 2020
This commit aims to make aware `eksctl` of the `KUBECONFIG` environment
variable, especially during cluster creation and kubeconfig retrieval
via the `eksctl utils write-kubeconfig` command.
To avoid conflicts and keep the actual convention, the variable
`DefaultPath` has been declared using a simple _func_ in order to don't
perform an invasive refactoring requested by a switch to function
declaration.
Actually, using the standard library to retrieve the environment
variable and not using the `viper` helper (although already a dependency)
since the `KUBECONFIG` one is considered as a global one, so totally
unbounded from `eksctl`.
Code coverage is going to decrease since during the test suite setup,
the environment variable is evaluated before the execution of tests: to
provide further coverage, the conversion from variable to function has
to be performed, although this requires some changes in the following
files and should be considered and approved by maintainers:
- pkg/ctl/cmdutils/cmdutils.go
- pkg/ctl/create/cluster.go
- pkg/ctl/utils/write_kubeconfig.go
- pkg/utils/kubeconfig/kubeconfig.go

Closes eksctl-io#228
prometherion added a commit to prometherion/eksctl that referenced this issue Feb 10, 2020
This commit aims to make aware `eksctl` of the `KUBECONFIG` environment
variable, especially during cluster creation and kubeconfig retrieval
via the `eksctl utils write-kubeconfig` command.
To avoid conflicts and keep the actual convention, the variable
`DefaultPath` has been declared using a simple _func_ in order to don't
perform an invasive refactoring requested by a switch to function
declaration.
Actually, using the standard library to retrieve the environment
variable and not using the `viper` helper (although already a dependency)
since the `KUBECONFIG` one is considered as a global one, so totally
unbounded from `eksctl`.
Code coverage is going to decrease since during the test suite setup,
the environment variable is evaluated before the execution of tests: to
provide further coverage, the conversion from variable to function has
to be performed, although this requires some changes in the following
files and should be considered and approved by maintainers:
- pkg/ctl/cmdutils/cmdutils.go
- pkg/ctl/create/cluster.go
- pkg/ctl/utils/write_kubeconfig.go
- pkg/utils/kubeconfig/kubeconfig.go

Closes eksctl-io#228
prometherion added a commit to prometherion/eksctl that referenced this issue Feb 28, 2020
This commit aims to make aware `eksctl` of the `KUBECONFIG` environment
variable, especially during cluster creation and kubeconfig retrieval
via the `eksctl utils write-kubeconfig` command.
To avoid conflicts and keep the actual convention, the variable
`DefaultPath` has been declared using a simple _func_ in order to don't
perform an invasive refactoring requested by a switch to function
declaration.
Actually, using the standard library to retrieve the environment
variable and not using the `viper` helper (although already a dependency)
since the `KUBECONFIG` one is considered as a global one, so totally
unbounded from `eksctl`.
Code coverage is going to decrease since during the test suite setup,
the environment variable is evaluated before the execution of tests: to
provide further coverage, the conversion from variable to function has
to be performed, although this requires some changes in the following
files and should be considered and approved by maintainers:
- pkg/ctl/cmdutils/cmdutils.go
- pkg/ctl/create/cluster.go
- pkg/ctl/utils/write_kubeconfig.go
- pkg/utils/kubeconfig/kubeconfig.go

Closes eksctl-io#228
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants