fix: use super-admin.conf for kube-vip on first master when it exists #11422
Conversation
k8s-ci-robot
added
kind/bug
k8s-ci-robot
added
the
cncf-cla: no
|
Welcome @Seljuke! |
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @Seljuke. 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-sigs/prow repository. |
k8s-ci-robot
added
the
size/S
k8s-ci-robot
added
cncf-cla: yes
| @@ -6,6 +6,21 @@ | |||
| - kube_proxy_mode == 'ipvs' and not kube_proxy_strict_arp | |||
| - kube_vip_arp_enabled | |||
|
|
|||
| - name: Kube-vip | Check if super-admin.conf exists | |||
| stat: | |||
| path: "{{ kube_config_dir }}/super-admin.conf" | |||
There was a problem hiding this comment.
super-admin.conf will be created in each cp node?
There was a problem hiding this comment.
it does't get created on each cp node and only on first node, after first cp node initialize RBAC fully configured and we can use the admin.conf on other nodes.
There was a problem hiding this comment.
The kube-vip pod run on all the cp node, right? you mean that the kube-vip pod on the first cp node is use the super-admin.conf, and the kube-vip pod on the other cp nodes is use the admin.conf, right?
There was a problem hiding this comment.
@cyclinder Yes.
NB: But this is only necessary on bootstrap, i.e before admin.conf has cluster-admin privileges.
There was a problem hiding this comment.
Got, thank you for your clarification!
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
Co-authored-by: Mathieu Parent <math.parent@gmail.com>
k8s-ci-robot
added
the
lgtm
| @@ -119,6 +119,6 @@ spec: | |||
| hostNetwork: true | |||
| volumes: | |||
| - hostPath: | |||
| path: /etc/kubernetes/admin.conf | |||
| path: /etc/kubernetes/{% if inventory_hostname == (groups['kube_control_plane'] | first) and ((stat_kube_vip_super_admin.stat.exists and stat_kube_vip_super_admin.stat.isreg) or (not kubeadm_already_run.stat.exists )) %}super-{% endif %}admin.conf | |||
There was a problem hiding this comment.
I would rather have a variable elsewhere so it doesn't make the template hard to read.
# default value for all nodes
set_fact:
kube_vip_admin_conf: admin.conf
# special case
set_fact:
kube_vip_admin_conf: super-admin.conf
when:
- inventory_hostname == groups['kube_control_plane'] | first
- (stat_kube_vip_super_admin.stat.exists and stat_kube_vip_super_admin.stat.isreg) or ((not kubeadm_already_run.stat.exists )The template is then:
volumes:
- hostPath:
path: /etc/kubernetes/{{kube_vip_admin_conf}}
There was a problem hiding this comment.
I agree, updated template with your suggestion
k8s-ci-robot
removed
the
lgtm
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ant31, Seljuke 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
approved
|
@Seljuke Have you seen the failing tests? |
|
Do you mean squashing commits? Or resolving conversations? |
|
The merge bot will squash the commit before merge. |
Got. /lgtm |
k8s-ci-robot
added
the
lgtm
|
@Seljuke Please add release notes section 🙏 . |
|
@Seljuke Don't know. @cyclinder ? Trying: /remove do-not-merge/release-note-label-needed |
|
/release-note-none |
k8s-ci-robot
added
release-note-none
|
@cyclinder any idea when this change will be released? |
|
@Mazorius We can add it manually when we release it. |
|
@Seljuke Could you create a backport PR for release-2.25 branch? |
|
@cyclinder can you run |
|
/cherry-pick release-2.25 |
|
@cyclinder: #11422 failed to apply on top of branch "release-2.25": In response to this:
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-sigs/prow repository. |
|
@Seljuke failed to apply this patch to release-2.25, can you create a backport PR manually? |
…kubernetes-sigs#11422) * fix: use super-admin.conf for kube-vip when it exists * Mathieu Parent add as co-author Co-authored-by: Mathieu Parent <math.parent@gmail.com> * template change for readability * fix lint error --------- Co-authored-by: Mathieu Parent <math.parent@gmail.com> (cherry picked from commit e43e08c)
|
@cyclinder @Seljuke I created the backport PR: #11444 |
…#11422) (#11444) * fix: use super-admin.conf for kube-vip when it exists * Mathieu Parent add as co-author Co-authored-by: Mathieu Parent <math.parent@gmail.com> * template change for readability * fix lint error --------- Co-authored-by: Mathieu Parent <math.parent@gmail.com> (cherry picked from commit e43e08c) Co-authored-by: Selçuk Arıbalı <selcukaribali@outlook.com>
What type of PR is this?
/kind bug
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #11229
Special notes for your reviewer:
Tested manually for fresh ha cluster install, cluster upgrade, worker node add/remove, control plane node add/remove, first control plane reordering and remove.
Does this PR introduce a user-facing change?: