Feat/kubeconfig multi cluster #512
Conversation
Signed-off-by: Marc Khouzam <marc.khouzam@ville.montreal.qc.ca>
kubeadm does not currently allow to configure a user reference id, and instead always uses kubernetes-admin. This causes a problem when we create multiple clusters with Kind and want to use each corresponding kubeconfig file at the same time in KUBECONFIG. Until kubeadm supports configuring a user reference id, the only option to fix this for Kind is to modify the kubeconfig file that kubeadm provides. As Kind already did this with the server name, it made sense to take the logic further and also make the user reference id unique to a cluster. Three approaches were considered: 1- continue with the current approach of parsing each line of the admin.conf kubeconfig file, and make the modifications necessary. After implementing this approach, the solution seemed quite brittle as it uses regex but no yaml structure. 2- use the go package yaml.v2 to -fully- parse the yaml of the admin.conf kubeconfig file, make the modifications, and then output the new yaml kubeconfig file. This solution requires to define a detailed struct of every field contained in the original yaml file. Having to map every field in advance is brittle as any modification that kubeadm may make to the file in the future would require adaptation in Kind. 3- use the go package yaml.v2 to -generically- parse the yaml of the admin.conf kubeconfig file, make the modifications, and then output the new yaml kubeconfig file. This solution only accesses the yaml fields that are required to be modified. Although any future changes from kubeadm to those fields would require modifications in Kind, modifications to all other fields would not. This commit implements solution #3 which was felt to be the most future-proof and least brittle of the three. Signed-off-by: Marc Khouzam <marc.khouzam@ville.montreal.qc.ca>
|
Welcome @marckhouzam! |
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: marckhouzam If they are not already assigned, you can assign the PR to them by writing 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
size/M
|
Hi @marckhouzam. Thanks for your PR. I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
needs-ok-to-test
| for k := range user { | ||
| if k == "name" { | ||
| user[k] = newUserName | ||
| } else if k == "user" { |
There was a problem hiding this comment.
Is this for backward compatibility? The current config looks like this:
(MoeLove) ➜ kind git:(using-network) ✗ kubectl config view
apiVersion: v1
clusters:
- cluster:
certificate-authority-data: DATA+OMITTED
server: https://localhost:45965
name: moe
contexts:
- context:
cluster: moe
user: kubernetes-admin
name: kubernetes-admin@moe
current-context: kubernetes-admin@moe
kind: Config
preferences: {}
users:
- name: kubernetes-admin
user:
client-certificate-data: REDACTED
client-key-data: REDACTED
There was a problem hiding this comment.
Which part?
As I understand it, the field contexts[0].context.user refers to a user defined in this file in the users[] section. So, this PR makes the user unique and changes it in the two sections: contexts[0].context.user and users[].name.
Finally, the username within the cluster is taken by default to be users[].name but since we changed it, we need to tell what the real username is. That is why we must insert the field username.
There is not backwards-compatibility issue to worry about, as far as I can tell, although I am no expert in kubeconfig definitions.
However, the real username in the cluster is still kubernetes-admin, which used to be the
|
/cc @neolit123 |
|
Thanks for the PR! /ok-to-test I wonder if the KUBECONFIG format is fully stable by now, this change seems a bit brittle still. |
k8s-ci-robot
added
ok-to-test
|
@BenTheElder Your comment made me realize I may be able to make these kubeconfig modifications by calling I'll try it out and report back. |
yes the format is stable. https://github.com/kubernetes/client-go/blob/master/tools/clientcmd/api/types.go the alternative from kubectl is:
kubeadm has the following command: if you call this on a control-plane node it will output to STDOUT a kubeconfig for a separate user for the current cluster.
kubeadm indeed does not allow configuring the default user name, but by configuring separate users for multiple clusters you can technically merge the resulted kubeconfig files.
see my comments above about using client-go. the library can also be used to merge kubeconfig files. alternatively see how it can be done using /hold |
k8s-ci-robot
added
the
do-not-merge/hold
|
@neolit123 Thanks for the information. I will look into the different options and re-work this PR. |
|
np, i guess part of my point was that this should be already doable without changes in kind. |
|
I did a new PR #520 to use client-go to modify the server in the kubeconfig file, as suggested by @neolit123 I've also written the modification needed to make the user unique using client-go, but I haven't posted it. I'll try @neolit123 suggestion about using kubeadm directly for this and see if I can get that to work nicely. Ultimately, I think it would be nice if kubeadm would automatically generate the userid with the cluster name suffix, as this problem is not limited to Kind but to any time multiple clusters are created using kubeadm. |
k8s-ci-robot
added
the
needs-rebase
|
@marckhouzam: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@marckhouzam: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
This is done now as part of #850. Thanks again for the PR 🙃 |
Modify kubeconfig for multiple clusters ( #112 )
kubeadm does not currently allow to configure a user reference id,
and instead always uses kubernetes-admin. This causes a problem
when we create multiple clusters with Kind and want to use
each corresponding kubeconfig file at the same time in KUBECONFIG.
Until kubeadm supports configuring a user reference id, the only
option to fix this for Kind is to modify the kubeconfig file that
kubeadm provides. As Kind already did this with the server name,
it made sense to take the logic further and also make the user
reference id unique to a cluster.
Three approaches were considered:
1- continue with the current approach of parsing each line of
the admin.conf kubeconfig file, and make the modifications
necessary. After implementing this approach, the solution
seemed quite brittle as it uses regex but no yaml structure.
2- use the go package yaml.v2 to -fully- parse the yaml of the
admin.conf kubeconfig file, make the modifications, and then
output the new yaml kubeconfig file. This solution requires
to define a detailed struct of every field contained in the
original yaml file. Having to map every field in advance is
brittle as any modification that kubeadm may make to the file
in the future would require adaptation in Kind.
3- use the go package yaml.v2 to -generically- parse the yaml of
the admin.conf kubeconfig file, make the modifications, and then
output the new yaml kubeconfig file. This solution only
accesses the yaml fields that are required to be modified.
Although any future changes from kubeadm to those fields would
require modifications in Kind, modifications to all other fields
would not.
This PR implements solution #3 which was felt to be the most
future-proof and least brittle of the three.