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

🌱 Skip kube-vip-prepare for 1.31+ k8s since CAPI won't depend on super-admin.conf #3168

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lubronzhan
Copy link
Contributor

@lubronzhan lubronzhan commented Aug 23, 2024

What this PR does / why we need it:
Skip kube-vip-prepare for 1.31+ k8s since CAPI won't depend on super-admin.conf

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes ##2596

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 23, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign fabriziopandini for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 23, 2024
@lubronzhan
Copy link
Contributor Author

I manually modified the file. Looks like I shouldn't

         localhost.localdomain localhost4 localhost4.localdomain4" >>/etc/hosts
       - mkdir -p /etc/pre-kubeadm-commands
       - for script in $(find /etc/pre-kubeadm-commands/ -name '*.sh' -type f | sort);
-        do echo "Running script $script"; "$script"; done
+        do echo "Running script $script"; "$script"; done
\ No newline at end of file

@lubronzhan
Copy link
Contributor Author

cc @sbueringer @chrischdi

@chrischdi
Copy link
Member

Looks like this does not work, there seems to still be some dependency on the loadbalancer IP in this case:

E0826 09:31:30.638835       1 leaderelection.go:332] error retrieving resource lock kube-system/plndr-cp-lock: leases.coordination.k8s.io "plndr-cp-lock" is forbidden: User "kubernetes-admin" cannot get resource "leases" in API group "coordination.k8s.io" in the namespace "kube-system"

@chrischdi
Copy link
Member

More details:

kube-vip/kube-vip#684 (comment)

@chrischdi
Copy link
Member

cc @sbueringer

@lubronzhan
Copy link
Contributor Author

Dumb, question, how does it work before in 1.28. Kube-vip also use the admin.conf generated by kubeadm, if kubeadm applies the rbac after, then kube-vip can't bootstrap either I assume in old k8s version?

@chrischdi
Copy link
Member

v1.29 introduced the super-admin.conf. In v1.28 and before, the admin.conf immediately had the required permissions.

@lubronzhan
Copy link
Contributor Author

IC. so kubelet is able to talk to api through local ip, but not kubeadm. Kubeadm still talks to api through control plane ip. Is there use case that kubeadm init will be run not on the control plane node? Should it also support this mode to talk to api through localhost?

With ControlPlaneKubeletLocalMode and when referncing admin.conf for kube-vip:

The kubelet got started
The kubelet bootstraps itself using the local control-plane IP (not depending on kube-vip being up)
The admin.conf gets created
The kubelet should be able to start kube-vip now

@lubronzhan
Copy link
Contributor Author

IC. so kubelet is able to talk to api through local ip, but not kubeadm. Kubeadm still talks to api through control plane ip. Is there use case that kubeadm init will be run not on the control plane node? Should it also support this mode to talk to api through localhost?

With ControlPlaneKubeletLocalMode and when referncing admin.conf for kube-vip:

The kubelet got started
The kubelet bootstraps itself using the local control-plane IP (not depending on kube-vip being up)
The admin.conf gets created
The kubelet should be able to start kube-vip now

@neolit123 what do you think

@sbueringer
Copy link
Member

@lubronzhan @neolit123 @chrischdi bump :)

@neolit123
Copy link
Member

neolit123 commented Nov 13, 2024

missed the prior ping.

IC. so kubelet is able to talk to api through local ip, but not kubeadm. Kubeadm still talks to api through control plane ip. Is there use case that kubeadm init will be run not on the control plane node?

not in CAPI (automated) workflows. the user can do it out of band though.
e.g. ssh on a CP machine and call kubeadm init phase ...something as an utility for something.

Should it also support this mode to talk to api through localhost?

the admin.conf and super-admin.conf always talk to the CPE (control plane endpoint), that is because we want the user to be able to reach any API server.

With ControlPlaneKubeletLocalMode and when referncing admin.conf for kube-vip:

The kubelet got started
The kubelet bootstraps itself using the local control-plane IP (not depending on kube-vip being up)
The admin.conf gets created
The kubelet should be able to start kube-vip now

IIUC, at that point admin.conf still doesn't have the permissions.
thus initial kube-vip bootstrap must use super-admin.conf and then move to admin.conf, that is what @chrischdi workaround proposal is doing.

these are points i brought before, but ideally kube-vip should stop using the *admin.conf (because it's not an admin) and stop requiring any RBAC on bootstrap. it could have delayed RBAC requirements IIUC.

@neolit123
Copy link
Member

these are points a brought before but, ideally kube-vip should stop using the *admin.conf (because it's not an admin) and stop requiring any RBAC on bootstrap. it could have delayed RBAC requirements IIUC.

speaking of RBAC and using the incorrect kubeconfigs, does the kube-controller-manager.conf or kube-scheduler.conf clients have the required permissions? i think it was related to leader election.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants