Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

lokoctl health does not fallback for kubeconfig #608

Closed
surajssd opened this issue Jun 10, 2020 · 14 comments · Fixed by #701
Closed

lokoctl health does not fallback for kubeconfig #608

surajssd opened this issue Jun 10, 2020 · 14 comments · Fixed by #701
Assignees
Labels
bug Something isn't working
Milestone

Comments

@surajssd
Copy link
Member

surajssd commented Jun 10, 2020

I don't have assets dir locally but in S3 and when I run following:

$ lokoctl health
Error: Kubeconfig "assets/cluster-assets/auth/kubeconfig" not found
Usage:
  lokoctl health [flags]

Flags:
  -h, --help   help for health

Global Flags:
      --kubeconfig string     Path to kubeconfig file, taken from the asset dir if not given, and finally falls back to ~/.kube/config
      --lokocfg string        Path to lokocfg directory or file (default "./")
      --lokocfg-vars string   Path to lokocfg.vars file (default "./lokocfg.vars")

It did not fallback to ~/.kube/config as mentioned in the --kubeconfig flag description.

And same for component apply as well:

$ lokoctl component apply
Applying component 'openebs-operator'...
FATA[0000] stat assets/cluster-assets/auth/kubeconfig: no such file or directory  args="[]" command="lokoctl component apply"
@surajssd surajssd added bug Something isn't working proposed/next-sprint Issues proposed for next sprint labels Jun 10, 2020
@invidian
Copy link
Member

This is expected behavior. If you have the valid configuration in working directory, it should be used. This is duplicate of #105, right?

@invidian
Copy link
Member

Turns out we pass kubeconfig path a lot:

$ git grep kubeconfig cli/ pkg/ | grep -v pkg/assets/generated_assets.go | grep kubeconfig | wc -l
55

The idea to solve this would to, to rather than passing kubeconfig path, we could pass the content of this file, which can be taken from anywhere. We could even validate it and pass it as some kind of generic client object, but this might be more difficult to use then.

invidian added a commit that referenced this issue Jun 15, 2020
This commit adds NewClientset exported function, which allows creating
Kubernetes Clientset from content of kubeconfig file, which allows to
create it from sources differant than file on local file system.

This is required for being able to execute 'lokoctl health' without
having kubeconfig file present in local assets directory, as it can be
pulled from other places, e.g. Terraform state.

See #608 for more details.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 15, 2020
This commit adds NewClientset exported function, which allows creating
Kubernetes Clientset from content of kubeconfig file, which allows to
create it from sources differant than file on local file system.

This is required for being able to execute 'lokoctl health' without
having kubeconfig file present in local assets directory, as it can be
pulled from other places, e.g. Terraform state.

See #608 for more details.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 15, 2020
This commit adds NewClientset exported function, which allows creating
Kubernetes Clientset from content of kubeconfig file, which allows to
create it from sources differant than file on local file system.

This is required for being able to execute 'lokoctl health' without
having kubeconfig file present in local assets directory, as it can be
pulled from other places, e.g. Terraform state.

See #608 for more details.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 16, 2020
This commit adds NewClientset exported function, which allows creating
Kubernetes Clientset from content of kubeconfig file, which allows to
create it from sources differant than file on local file system.

This is required for being able to execute 'lokoctl health' without
having kubeconfig file present in local assets directory, as it can be
pulled from other places, e.g. Terraform state.

See #608 for more details.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 16, 2020
Getter struct and NewGetter function is a replacement for
helm.sh/helm/v3/pkg/kube.GetConfig(), which allows to create Helm action
config from the content of kubeconfig file, instead of from the path of
the file, which is part of #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 16, 2020
As part of #608, this commit changes the implementation of
HelmActionConfig function to read the content of given kubeconfig file
and then use the content to construct Helm's actionConfig. This will
make easier changing the function signature to accept content of
kubeconfig file in the future changes.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 16, 2020
Instead of kubeconfig file path. This simplifies the tests and moves us
closer towards resolving #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 16, 2020
So we don't necesserily need a file on file system to perfrom this
action, which is needed to resolve #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 16, 2020
This commit changes InstallComponent and InstallAsRelease methods to
accept kubeconfig content, rather than the file path, to allow loading
it from places other than local filesystem.

Part of #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 16, 2020
The subcommand function will fail anyway while trying to read kubeconfig
file, so there is no need to check it twice.

Also part of #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 16, 2020
Rather than kubeconfig file path. This will allow changing this
function to pull kubeconfig content from Terraform output rather than
from assets directory, which will resolve #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
@iaguis iaguis removed the proposed/next-sprint Issues proposed for next sprint label Jun 18, 2020
invidian added a commit that referenced this issue Jun 23, 2020
This commit adds NewClientset exported function, which allows creating
Kubernetes Clientset from content of kubeconfig file, which allows to
create it from sources differant than file on local file system.

This is required for being able to execute 'lokoctl health' without
having kubeconfig file present in local assets directory, as it can be
pulled from other places, e.g. Terraform state.

See #608 for more details.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 23, 2020
Getter struct and NewGetter function is a replacement for
helm.sh/helm/v3/pkg/kube.GetConfig(), which allows to create Helm action
config from the content of kubeconfig file, instead of from the path of
the file, which is part of #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 23, 2020
As part of #608, this commit changes the implementation of
HelmActionConfig function to read the content of given kubeconfig file
and then use the content to construct Helm's actionConfig. This will
make easier changing the function signature to accept content of
kubeconfig file in the future changes.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 23, 2020
Instead of kubeconfig file path. This simplifies the tests and moves us
closer towards resolving #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 23, 2020
So we don't necesserily need a file on file system to perfrom this
action, which is needed to resolve #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 23, 2020
This commit changes InstallComponent and InstallAsRelease methods to
accept kubeconfig content, rather than the file path, to allow loading
it from places other than local filesystem.

Part of #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 23, 2020
The subcommand function will fail anyway while trying to read kubeconfig
file, so there is no need to check it twice.

Also part of #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 23, 2020
Rather than kubeconfig file path. This will allow changing this
function to pull kubeconfig content from Terraform output rather than
from assets directory, which will resolve #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jun 26, 2020
This commit adds NewClientset exported function, which allows creating
Kubernetes Clientset from content of kubeconfig file, which allows to
create it from sources differant than file on local file system.

This is required for being able to execute 'lokoctl health' without
having kubeconfig file present in local assets directory, as it can be
pulled from other places, e.g. Terraform state.

See #608 for more details.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jul 30, 2020
Getter struct and NewGetter function is a replacement for
helm.sh/helm/v3/pkg/kube.GetConfig(), which allows to create Helm action
config from the content of kubeconfig file, instead of from the path of
the file, which is part of #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jul 30, 2020
As part of #608, this commit changes the implementation of
HelmActionConfig function to read the content of given kubeconfig file
and then use the content to construct Helm's actionConfig. This will
make easier changing the function signature to accept content of
kubeconfig file in the future changes.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jul 30, 2020
Instead of kubeconfig file path. This simplifies the tests and moves us
closer towards resolving #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jul 30, 2020
This commit changes InstallComponent and InstallAsRelease methods to
accept kubeconfig content, rather than the file path, to allow loading
it from places other than local filesystem.

Part of #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jul 30, 2020
The subcommand function will fail anyway while trying to read kubeconfig
file, so there is no need to check it twice.

Also part of #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Jul 30, 2020
Rather than kubeconfig file path. This will allow changing this
function to pull kubeconfig content from Terraform output rather than
from assets directory, which will resolve #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
@invidian
Copy link
Member

invidian commented Aug 4, 2020

Oops, that shouldn't be closed. We are still missing #701.

@invidian invidian reopened this Aug 4, 2020
invidian added a commit that referenced this issue Aug 4, 2020
Closes #608

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
@surajssd
Copy link
Member Author

surajssd commented Aug 6, 2020

Not just health but lokoctl compont apply also has fallback issues:

$ lokoctl component apply metrics-server
FATA[0000] Error in finding kubeconfig file: open assets/cluster-assets/auth/kubeconfig: no such file
or directory  args="[metrics-server]" command="lokoctl component apply"

Even though I have the ~/.kube/config file available:

$ kubectl get nodes
NAME                   STATUS   ROLES    AGE   VERSION
foo-controller-0       Ready    <none>   90m   v1.18.6
foo-openebs-worker-0   Ready    <none>   89m   v1.18.6

@surajssd
Copy link
Member Author

surajssd commented Aug 6, 2020

@invidian should we at least get the fallback issues solved? Later we can look into extracting kubeconfig from the state?

@invidian
Copy link
Member

invidian commented Aug 6, 2020

What do you mean by fallback? It shouldn't do any fallback if file is missing, as it may install things on a wrong cluster.

@surajssd
Copy link
Member Author

Then we should fix the kubeconfig flag help.

@invidian
Copy link
Member

Then we should fix the kubeconfig flag help.

To mention if the cluster configuration is present in working directory? That make sense I guess 👍

@invidian
Copy link
Member

It seems like too bit detail to put in the CLI flag description :|

@surajssd
Copy link
Member Author

We should completely remove the fallback to the global kubeconfig automatically and update the flag description.

@invidian
Copy link
Member

We should completely remove the fallback to the global kubeconfig automatically and update the flag description.

If we remove that, we won't be able to install components on clusters managed with kubeconfig contexts, which IMO is a valuable feature. This does not apply to health sub-command of course. Do you suggest to have different behavior for health and component?

@invidian
Copy link
Member

@surajssd BTW current help message looks following:

$ ./lokoctl health --help
Get the health of a cluster

Usage:
  lokoctl health [flags]

Flags:
  -h, --help   help for health

Global Flags:
      --kubeconfig-file string   Path to a kubeconfig file. If empty, the following precedence order is used:
                                   1. Cluster asset dir when a lokocfg file is present in the current directory.
                                   2. KUBECONFIG environment variable.
                                   3. ~/.kube/config file.
      --lokocfg string           Path to lokocfg directory or file (default "./")
      --lokocfg-vars string      Path to lokocfg.vars file (default "./lokocfg.vars")

@invidian
Copy link
Member

Actually, I created #820 to get rid of this flag completely, simplifying the whole thing.

invidian added a commit that referenced this issue Aug 21, 2020
Closes #608

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Aug 21, 2020
Closes #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Aug 21, 2020
Closes #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
@invidian
Copy link
Member

#701 is now ready to review.

invidian added a commit that referenced this issue Aug 27, 2020
Closes #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian added a commit that referenced this issue Sep 2, 2020
Closes #608.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants