-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[external-lb]: kubelet.conf server address and kube-proxy api-server address fix #10490
Conversation
Signed-off-by: Ugur Ozturk <[email protected]>
Signed-off-by: Furkan Pehlivan <[email protected]>
Hi @ugur99. Thanks for your PR. I'm waiting for a kubernetes-sigs 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. |
/ok-to-test |
Hi @yankay! Is there anything else you would like us to do regarding this PR? I think the failing required pipeline has nothing to do with our commits, right? |
HI @ugur99 The CI had been broken in 1st Oct, and it has been fixed :-) Thanks you :-) |
Hi @yankay, I merged master to our branch and triggered the CI again. It seems like there is no failed pipeline. Please let us know if you need further actions. Thanks 🙂 |
Thanks @pehlicd /lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ugur99 Thank you for the PR 👍
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: floryut, ugur99 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 |
…address fix (kubernetes-sigs#10490) * [external-lb-kubeconfig]: fix server address in worker kubelet.conf Signed-off-by: Ugur Ozturk <[email protected]> * [external-lb-kubeconfig]: fix server address in kube-proxy Signed-off-by: Furkan Pehlivan <[email protected]> --------- Signed-off-by: Ugur Ozturk <[email protected]> Signed-off-by: Furkan Pehlivan <[email protected]> Co-authored-by: Furkan Pehlivan <[email protected]>
…address fix (kubernetes-sigs#10490) * [external-lb-kubeconfig]: fix server address in worker kubelet.conf Signed-off-by: Ugur Ozturk <[email protected]> * [external-lb-kubeconfig]: fix server address in kube-proxy Signed-off-by: Furkan Pehlivan <[email protected]> --------- Signed-off-by: Ugur Ozturk <[email protected]> Signed-off-by: Furkan Pehlivan <[email protected]> Co-authored-by: Furkan Pehlivan <[email protected]>
What type of PR is this?
/kind bug
What this PR does / why we need it:
When trying to configure external lb address for already provisioned clusters; it does not set correct external lb fqdn address for worker nodes, instead of it sets first controlplane's ip address.
The problem is that kubeadm does not update the kubeadm-config configmap in kube-system namespace; it only creates that config in kubeadm init phase, even though it creates new ClusterConfiguration on controlplane node in
/etc/kubernetes/kubeadm-config.yaml
path. Since worker nodes fetch configMap on the cluster when thekubeadm join ...
command is run, it was fetching old clusterConfig in already provisioned clusters.To fix this problem, we added a new task to update kubelet.conf in worker nodes. We also noticed that kube_proxy was not getting the right address for external lb is the case; and we added logic to update kube_proxy config as well.
EDIT: Just a quick edit to fix misunderstanding on my side; it looks like kubeadm fetches the cluster-info configmap in the kube-public namespace to create a new kubelet.conf. But it is the best to handle this on the kubespray side instead of relying on the configmap on the cluster.
Special notes for your reviewer:
For testing purposes, a new cluster was set up and tested.
Does this PR introduce a user-facing change?: