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

k3s: Dynamically generate kubeconfig #212

Merged
merged 2 commits into from
Jul 15, 2021
Merged

Conversation

stv0g
Copy link
Contributor

@stv0g stv0g commented Jul 14, 2021

This PR adjusts the K3S manifests to automatically generate a kubeconfig file which is used by kg based on the token of the kilo ServiceAccount and the master address found in the kubelet's kubeconfig file.

The eliminates the need of manually copying /etc/rancher/k3s/k3s.yaml from the master to all agent nodes.
At the same time, the kg daemon only has the permissions granted to its ServiceAccount instead of a full cluster access.

Closes #49

Copy link
Owner

@squat squat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this, thank you! I have just one suggestion

manifests/kilo-k3s-flannel.yaml Outdated Show resolved Hide resolved
@stv0g
Copy link
Contributor Author

stv0g commented Jul 14, 2021

@squat Sure, I wasnt aware of that. I updated my branch accordingly

@leonnicolas
Copy link
Collaborator

you need to add get to the verbs of cutomresourcedefinitions and we don't need create anymore :)

@squat
Copy link
Owner

squat commented Jul 15, 2021

you need to add get to the verbs of cutomresourcedefinitions and we don't need create anymore :)

I think the branch needs to be rebased but I don't think the commit actually changes the RBAC.

@stv0g mind rebasing and then we can merge?

…r address & cacert from kubelet kubeconfig (closes squat#49)
@stv0g
Copy link
Contributor Author

stv0g commented Jul 15, 2021

@squat I've rebased the branch

@leonnicolas I also dont get your comment regarding the RBAC.

@squat
Copy link
Owner

squat commented Jul 15, 2021

@leonnicolas I also dont get your comment regarding the RBAC.

I think this was about the un-rebased code. When viewing the files from the PR in their entirety, you would see out-of-date RBAC.

@leonnicolas
Copy link
Collaborator

The nkml daemonSet needs the serviceAccountName: kilo in its spec, otherwise it cannot get and label the nodes.

@stv0g
Copy link
Contributor Author

stv0g commented Jul 15, 2021

The nkml daemonSet needs the serviceAccountName: kilo in its spec, otherwise it cannot get and label the nodes.

Fixed

Copy link
Collaborator

@leonnicolas leonnicolas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's such an improvement!

@leonnicolas leonnicolas merged commit daecc2a into squat:main Jul 15, 2021
@unixfox
Copy link

unixfox commented Jul 15, 2021

Why is it through a configmap and not directly using the args parameter in the pod?
It could have been like this:

initContainers:
- name: generate-kubeconfig
  image: squat/kilo
  command:
  - /bin/sh
  args:
  - cat > /etc/kubernetes/kubeconfig <<EOF
        apiVersion: v1
        kind: Config
        name: kilo
        clusters:
        - cluster:
            server: $(sed -n 's/.*server: \(.*\)/\1/p' /var/lib/rancher/k3s/agent/kubelet.kubeconfig)
            certificate-authority: /var/lib/rancher/k3s/agent/server-ca.crt
        users:
        - name: kilo
          user:
            token: $(cat /var/run/secrets/kubernetes.io/serviceaccount/token)
        contexts:
        - name: kilo
          context:
            cluster: kilo
            namespace: ${NAMESPACE}
            user: kilo
        current-context: kilo
    EOF
  imagePullPolicy: Always
  volumeMounts:
  - name: kubeconfig
    mountPath: /etc/kubernetes
    readOnly: true
  - name: k3s-agent
    mountPath: /var/lib/rancher/k3s/agent/
    readOnly: true
  env:
  - name: NAMESPACE
    valueFrom:
      fieldRef:
        fieldPath: metadata.namespace

We save having to store a configmap like this.

@squat
Copy link
Owner

squat commented Jul 15, 2021

Yes totally, I think the one place where this is helpful is in the user-space-heterogeneous manifests where we need to inline this script for both DaemonSets. I'm happy to move this in-line to avoid extra resources. It adds a bit more configuration maintenance complexity on us, however, we may be able to overcome this with some work @leonnicolas is doing to automate manifest generation.

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 this pull request may close these issues.

Why does kilo get cluster config using kubeconfig (or API server URL flag) when it has a service account?
4 participants