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

Explore consistent kubeconfig settings between Tekton and K8s ext #312

Closed
siamaksade opened this issue May 28, 2020 · 15 comments · Fixed by #513
Closed

Explore consistent kubeconfig settings between Tekton and K8s ext #312

siamaksade opened this issue May 28, 2020 · 15 comments · Fixed by #513
Assignees
Milestone

Comments

@siamaksade
Copy link

The Kubernetes ext allows defining a custom kubeconfig. When user does that, the Tekton ext and Kubernetes ext would display different clusters which creates problems when user uses Kubernetes actions to create a pipeline via YAML and that ends up on the wrong cluster/namespace.

@siamaksade siamaksade changed the title Explore consistent kubeconfig settings between Tekto nand K8s ext Explore consistent kubeconfig settings between Tekton nand K8s ext May 28, 2020
@siamaksade siamaksade changed the title Explore consistent kubeconfig settings between Tekton nand K8s ext Explore consistent kubeconfig settings between Tekton and K8s ext May 28, 2020
@siamaksade siamaksade added this to the Sprint #185 milestone May 28, 2020
@evidolob evidolob self-assigned this Jun 4, 2020
@evidolob
Copy link
Collaborator

evidolob commented Jun 9, 2020

I have done an investigation and found next:
vscode-kubernetos extension provide current k8s configuration, but not provide event when it changed. But vscode-kubernetos store path to current configuration in vscode preferences, we in tekton extension can read and receive event when it change. So we able to track when user start using another configuration file.
To finish this issue I have two options:

  1. Integrate deeply with vscode-kubernetos extension, use their api to invoke kubectl, in this case we automatically pick up changes, and need to rewrite the way how we use tkn, by adding path to current configuration file.

  2. Do not integrate vscode-kubernetos, just check if it installed and their configuration, and pass that configuration to all invocations of kubectl and tkn cli.

Basically both of this options requires almost same amount of work, so design should be more strategic/architectural.

@siamaksade @jeffmaury WDYT?

@jeffmaury
Copy link
Member

Is an extension allowed to read preferences declared by other extensions ? I thought it was not possible

@evidolob
Copy link
Collaborator

evidolob commented Jun 9, 2020

Hm, I try and it work, at least in dev mode, you just need to know configuration keys

@gorkem
Copy link
Collaborator

gorkem commented Jun 9, 2020

Can we have a 3rd alternate where vscode-kubernetes-tools set the environment variable like KUBECONFIG=$KUBECONFIG:/path/to/new/kubeconfig? I think all of our tools should be able to pick up this change, (if not they should). Also nowadays it should be able to set env variable to the integrated terminal too as an added bonus.

@evidolob
Copy link
Collaborator

evidolob commented Jun 9, 2020

@gorkem Yes, I think it is an option, but now they not do it, they just write path to preferences from user input: in https://github.com/Azure/vscode-kubernetes-tools/blob/e692354159d038d308546a295703984e7be5b1fe/src/extension.ts#L1925
setActiveKubeconfig just write new value in properties.
I can make a PR to fix that.
And we need an event when user change the config file, to be able refresh tekton tree

@lstocchi
Copy link
Contributor

lstocchi commented Jun 9, 2020

@evidolob look at https://github.com/Azure/vscode-kubernetes-tools/blob/e692354159d038d308546a295703984e7be5b1fe/src/components/kubectl/kubeconfig.ts#L39
It first checks if the property is set, if nothing is found, it checks other things. If you only listens to that property you won't have a valid result. I would vote for making the k8s tools send info to external extensions when the context changes.

@evidolob
Copy link
Collaborator

evidolob commented Jun 9, 2020

@lstocchi I agree with you, I never say that it is the best solution, I want to use that method to get current config, and use

 vscode.workspace.onDidChangeConfiguration(e => {
    if (e.affectsConfiguration('vs-kubernetes',)) {
       //refresh tekton tree there
    }
  });

I cannot find better way to get notification that something changed in k8s extension, without k8s extension modification.
And it is definitely better to improve k8s extension, instead of making a hack in tekton extension

@siamaksade
Copy link
Author

Can we have a 3rd alternate where
Is it relevant to send a PR to k8s ext switch to using KUBECONFIG env var? Otherwise I prefer (2)

@evidolob
Copy link
Collaborator

evidolob commented Jun 9, 2020

Is it relevant to send a PR to k8s ext switch to using KUBECONFIG env var? Otherwise I prefer (2)

@siamaksade Yes, to have KUBECONFIG env var set up, we need to send PR for k8s extension

@gorkem
Copy link
Collaborator

gorkem commented Jun 9, 2020

Just that you are aware this is not a problem unique to tekton. vscode-knative and vscode-openshift-tools also does similar things. The root cause of the problem is vscode-kubernetes-tools has a feature that does something unexpected with kubecontext so the solution should start from vscode-kubernetes-tools and we adjust the other extensions.
cc @dgolovin @joshuawilson

@evidolob
Copy link
Collaborator

evidolob commented Jun 9, 2020

OK, so what I should do with this? I may(and want) to make PR for vscode-kubernetes-tools with:

  • expose to KUBECONFIG env variable path to current config file
  • set that variable to integrated terminal
  • add some sort of notification for vscode-kubernetes-tools API to inform other extensions that path to config file is changed.

And after that returns to this issue for vscode-tekton. I ideal case it will require just handling that event and refresh tekton tree.

Of course if no one objects, or maybe have a better plan.

@gorkem
Copy link
Collaborator

gorkem commented Jun 9, 2020

I would start with

* add some sort of notification for  `vscode-kubernetes-tools` API to inform other extensions that path to config file is changed.

The other 2 are nice to haves that does not solve the original problem

@joshuawilson
Copy link
Member

vscode-knative watches the files listed in KUBECONFIG ENV and the .kube/ so that if there is a change it refreshes to pick up the difference.

But I agree, having vscode-kubernetes-tools own the notification out when it changes makes sense.

@evidolob
Copy link
Collaborator

evidolob commented Jun 10, 2020

@gorkem @lstocchi
I have PR with events: vscode-kubernetes-tools/vscode-kubernetes-tools#766 , can you look on it?

@evidolob evidolob modified the milestones: Sprint #185, Sprint #186 Jun 24, 2020
@evidolob evidolob modified the milestones: Sprint #186, Spring #187 Jul 15, 2020
@evidolob evidolob modified the milestones: Sprint #187, Sprint #188 Aug 5, 2020
@evidolob evidolob modified the milestones: Sprint #188, Sprint #189 Sep 16, 2020
@evidolob evidolob added this to the Sprint #190 milestone Sep 16, 2020
@evidolob evidolob removed the blocked label Jan 28, 2021
@evidolob evidolob modified the milestones: Sprint #190, 0.11.0 Jan 28, 2021
@evidolob
Copy link
Collaborator

vscode-kubernetes-tools/vscode-kubernetes-tools#766 is finally merged(thx @lstocchi for assist), we can continue with this after vscode-kubernetes-tools next release.

@siamaksade siamaksade modified the milestones: 0.11.0, 0.12.0 Feb 4, 2021
@siamaksade siamaksade modified the milestones: 0.12.0, 0.13.0 Feb 25, 2021
evidolob added a commit that referenced this issue Mar 1, 2021
evidolob added a commit that referenced this issue Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants