kinvolk / lokomotive

🪦 DISCONTINUED Further Lokomotive development has been discontinued. Lokomotive is a 100% open-source, easy to use and secure Kubernetes distribution from the volks at Kinvolk
https://kinvolk.io/lokomotive-kubernetes/
Apache License 2.0
320 stars 49 forks source link

`lokoctl health` does not fallback for kubeconfig #608

Closed surajssd closed 4 years ago

surajssd commented 4 years ago

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"
invidian commented 4 years ago

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

invidian commented 4 years ago

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 commented 4 years ago

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

surajssd commented 4 years ago

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 commented 4 years ago

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

invidian commented 4 years ago

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 commented 4 years ago

Then we should fix the kubeconfig flag help.

invidian commented 4 years ago

Then we should fix the kubeconfig flag help.

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

invidian commented 4 years ago

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

surajssd commented 4 years ago

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

invidian commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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

invidian commented 4 years ago

701 is now ready to review.