ci: wait for control-plane nodes before labeling in chart install#6370
ci: wait for control-plane nodes before labeling in chart install#6370
Conversation
kubectl label with a label selector silently succeeds (exit 0) when no nodes match, which causes the subsequent helm install to fail because no nodes have the kube-ovn/role=master label. This happens intermittently in Talos installation tests when the control-plane node has not yet registered its node-role.kubernetes.io/control-plane label at the time install-chart runs. Add a polling loop (up to 120s) that waits for at least one node with the control-plane label to appear before running kubectl label. Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com>
dosubot
bot
added
size:XS
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness of the CI pipeline by implementing a crucial synchronization step during Kubernetes chart installations. It prevents failures that occurred when node labeling commands were executed prematurely, ensuring that control-plane nodes are fully registered and available before proceeding with the Helm chart deployment. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an intermittent failure in CI by waiting for control-plane nodes to become available before attempting to label them. The approach of using a polling loop with a timeout is sound. My review focuses on improving the maintainability of the Makefile by suggesting the removal of duplicated code. By extracting the new waiting logic into a reusable define block, the code becomes cleaner and easier to manage in the future.
|
|
||
| .PHONY: install-chart | ||
| install-chart: | ||
| @timeout 120 bash -c 'until kubectl get node -l node-role.kubernetes.io/control-plane -o name 2>/dev/null | grep -q .; do echo "Waiting for control-plane nodes to be labeled..."; sleep 2; done' |
There was a problem hiding this comment.
This command is duplicated in the install-chart-v2 target on line 254. To improve maintainability and avoid duplication, consider extracting this logic into a reusable define block, similar to other wait helpers in this Makefile like kubectl_wait_exist.
For example, you could add the following definition alongside the other define blocks:
define wait_for_labeled_nodes
@timeout 120 bash -c 'until kubectl get node -l "$(1)" -o name 2>/dev/null | grep -q .; do echo "Waiting for nodes with label $(1) to appear..."; sleep 2; done'
endefAnd then call it in both install-chart and install-chart-v2 targets. This also makes the waiting message clearer.
$(call wait_for_labeled_nodes,node-role.kubernetes.io/control-plane)
|
|
||
| .PHONY: install-chart-v2 | ||
| install-chart-v2: | ||
| @timeout 120 bash -c 'until kubectl get node -l node-role.kubernetes.io/control-plane -o name 2>/dev/null | grep -q .; do echo "Waiting for control-plane nodes to be labeled..."; sleep 2; done' |
There was a problem hiding this comment.
This command is a duplicate of the one in the install-chart target on line 189. To improve maintainability, please see my suggestion on that line to refactor this into a reusable define block.
$(call wait_for_labeled_nodes,node-role.kubernetes.io/control-plane)
Pull Request Test Coverage Report for Build 22543631985Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
2 participants
Summary
kubectl label node -l node-role.kubernetes.io/control-plane kube-ovn/role=mastersilently succeeds (exit 0) when no nodes match the selector, leaving no nodes withkube-ovn/role=masterhelm installthen fails at template rendering (_helpers.tpl) becauselookupfinds no nodes with that labelkubectl labelthat waits for at least one node withnode-role.kubernetes.io/control-planeto appearTest plan
🤖 Generated with Claude Code