fix(kubeadm): preserve kubelet.conf server line indent via backrefs - #13429
fix(kubeadm): preserve kubelet.conf server line indent via backrefs#13429mehrdadbn9 wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mehrdadbn9 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @mehrdadbn9. 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 Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
45aefae to
0d0b694
Compare
|
/ok-to-test |
|
I don't think kubeadm output actually changed. v1.35/v1.36 still write kubelet.conf via Against a real kubelet.conf this patch breaks things: the first task replaces the 4-space line with an 8-space one (YAML parse error next to If we want to make this more robust, preserving the existing indent would be better than hardcoding it: regexp: '^(\s*)server: .*$'
line: '\1server: {{ kube_apiserver_endpoint }}'
backrefs: trueWould be good to get an unedited kubelet.conf from the reporter of #13277 first. |
0d0b694 to
34d3fe5
Compare
The lineinfile tasks rewrote the server: line with a hardcoded 4-space indent. The first task's regexp 'server:' could also match the wrong server: key. Use a captured indent group to preserve the real indent. Both tasks need backrefs: true because \1 in line is only expanded when backrefs is enabled; without it the literal string \1server: is written and kubelet.conf becomes invalid YAML (seen as 16 CI job failures across cluster jobs on the previous attempt). Verified with ansible-playbook 2.16 lineinfile on a sample kubelet.conf for 4-space and 8-space indents: indent preserved, endpoint substituted, certificate-authority-data untouched, valid YAML after rewrite. Relates to PR 13429 CI and issue 13277. Signed-off-by: Mehrdad Biukian Naeini <mehrdadbiukian@gmail.com>
34d3fe5 to
ed7bd07
Compare
|
You were right about the previous attempt. Two real bugs, both reproduced locally with ansible-playbook 2.16 lineinfile before this push:
Repro:
The kubeadm-emitted indent question you raised is now moot for correctness: the capture group preserves whatever indent exists (4 or 8 spaces both tested). Happy to test against a real kubelet.conf from the reporter if they can provide one. |
|
LGTM. |
|
/lgtm |
|
@guoard thanks for the lgtm. I'm preparing a Kubernetes GitHub org membership request (kubernetes/org, area/github-membership). Per the requirements I need two sponsors who are reviewers/approvers in an OWNERS file and have had close interactions with my work — your review here (kubespray-reviewers, per OWNERS_ALIASES) fits that. Would you be willing to sponsor? If yes, a brief |
Thanks, but I'll pass. I'm only a reviewer on kubespray and we've just crossed paths on a couple of PRs, not enough for me to vouch for someone. |
|
@mehrdadbn9 I suggest you tone it down with the sponsoring requests. It might not be your intention (written communication is not always as easy as in person), but it comes across as a bit pushy, when you had few interactions with the people before. |
|
@guoard @VannTen Thank you both, genuinely — for the reviews and for telling me straight. @guoard, understood completely, no hard feelings at all. Your lgtm and the backrefs catch already taught me a lot. @VannTen, you're right and I appreciate you saying it. I don't know the community flow well yet and I pushed too hard on the sponsorship asks with too few interactions behind them. I'll stop the asks entirely and just keep doing the code work — starting with fixing the #13429 title/description you flagged. If there's a better way I should be learning the process, I'm glad to hear it. |
Description
The "Update server field in kubelet kubeconfig" tasks rewrote the
server:line with a fixed indent. When the on-disk kubelet.conf uses a different indent than assumed,server:lands outside thecluster:mapping and kubelet fails after upgrade with:and worker nodes crash-loop (NotReady, tainted unreachable).
This switches both variants (default and external-lb) to
backrefs: truewithregexp: '^(\s*)server: .*$'andline: '\1server: {{ kube_apiserver_endpoint }}', so the existing indent is captured and preserved while only the endpoint value is replaced.Refs #13277
Verification
Reproduced with real ansible-playbook 2.16 lineinfile before pushing: without
backrefs: truethe literal text\1server:is written (invalid YAML, parse error); with backrefs, both 4-space and 8-space indents are preserved, the endpoint is substituted, and the CA line is untouched.Type of change