From 21f7317ab822b903d2cead6a4161e08b5fcd430a Mon Sep 17 00:00:00 2001 From: Timothy Stewart Date: Fri, 7 Aug 2026 01:30:53 -0500 Subject: [PATCH] fix(metallb): retry the webhook endpoint check on transient kube API errors - retry the webhook-service endpoint get until it succeeds - extend the regression test to cover the webhook task too --- .github/scripts/test-metallb-namespace.py | 121 ++++++++++++---------- roles/k3s_server_post/tasks/metallb.yml | 6 ++ 2 files changed, 75 insertions(+), 52 deletions(-) diff --git a/.github/scripts/test-metallb-namespace.py b/.github/scripts/test-metallb-namespace.py index 94e8682..77aa7cd 100644 --- a/.github/scripts/test-metallb-namespace.py +++ b/.github/scripts/test-metallb-namespace.py @@ -1,15 +1,20 @@ #!/usr/bin/env python3 -"""Regression test for the MetalLB namespace existence check. +"""Regression test for the MetalLB converge checks. -The "Test metallb-system namespace" task in -roles/k3s_server_post/tasks/metallb.yml must actually verify the namespace -exists. A previous version ran `k3s kubectl -n metallb-system` with no -subcommand, which only printed a usage page and always exited 0, so the task -always succeeded even when the namespace did not exist (issue #350). +The MetalLB tasks in roles/k3s_server_post/tasks/metallb.yml must actually +verify resources through an explicit kubectl get, and must retry on a +transient kube API error while MetalLB converges. -This test loads the real task and asserts the command performs an explicit -`get namespace metallb-system`, which returns non-zero when the namespace is -absent. +The "Test metallb-system namespace" task previously ran `k3s kubectl -n +metallb-system` with no subcommand, which only printed a usage page and always +exited 0, so it always succeeded even when the namespace did not exist (issue +#350). It must instead run an explicit `get namespace metallb-system`, which +returns non-zero when the namespace is absent. + +An explicit get actually contacts the API server, so these tasks need the same +retry wiring as their siblings (register, until rc == 0, retries, delay). A +bare get with no retry would otherwise abort the converge play on a transient +kube API error while MetalLB converges. """ from __future__ import print_function @@ -27,9 +32,53 @@ def repo_root(): def fail(message): - raise SystemExit( - "MetalLB namespace test failed: " + message - ) + raise SystemExit("MetalLB namespace test failed: " + message) + + +def find_task(tasks, name): + for entry in tasks: + if entry.get("name") == name: + return entry + fail("could not find the '{0}' task".format(name)) + return None + + +def command_text(task): + cmd = task.get("ansible.builtin.command") + if not cmd: + cmd = task.get("command") + if not cmd: + fail("task does not use ansible.builtin.command") + return cmd if isinstance(cmd, str) else " ".join(cmd) + + +def check_explicit_get(task, name, needle): + text = command_text(task) + if needle not in text: + fail( + "command does not run '{0}'; the task would only print usage and " + "never verify the resource (got: {1!r})".format(needle, text) + ) + + +def check_retry_wiring(task, name): + # The sibling k3s_server_post metallb tasks retry kubectl because the kube + # API can briefly be unavailable while MetalLB converges. Without the same + # retry, a transient API error aborts the whole converge play. + if not task.get("register"): + fail( + "{0} does not register a result; without retry wiring a transient " + "kube API error aborts the converge play".format(name) + ) + if not isinstance(task.get("until"), str) or "rc == 0" not in task["until"]: + fail( + "{0} does not retry on rc == 0; the kube API can transiently fail " + "while MetalLB converges and abort the play".format(name) + ) + if task.get("retries") is None: + fail("{0} is missing retries".format(name)) + if task.get("delay") is None: + fail("{0} is missing delay".format(name)) def main(): @@ -39,50 +88,18 @@ def main(): with open(task_file, encoding="utf-8") as handle: tasks = yaml.safe_load(handle) - task = None - for entry in tasks: - if entry.get("name") == "Test metallb-system namespace": - task = entry - break - if task is None: - fail("could not find the 'Test metallb-system namespace' task") - - cmd = task.get("ansible.builtin.command") - if not cmd: - cmd = task.get("command") - if not cmd: - fail("task does not use ansible.builtin.command") - - command_text = cmd if isinstance(cmd, str) else " ".join(cmd) - + namespace_task = find_task(tasks, "Test metallb-system namespace") # A bare `-n metallb-system` with no subcommand prints kubectl usage and # always exits 0, so it never proves the namespace exists. The fix must # use an explicit get. - if "get namespace metallb-system" not in command_text: - fail( - "command does not run 'get namespace metallb-system'; " - "the task would only print usage and never verify the namespace " - "(got: {0!r})".format(command_text) - ) + check_explicit_get(namespace_task, "Test metallb-system namespace", + "get namespace metallb-system") + check_retry_wiring(namespace_task, "Test metallb-system namespace") - # The sibling k3s_server_post metallb tasks retry kubectl because the kube - # API can briefly be unavailable while MetalLB converges. Without the same - # retry here, a transient API error aborts the whole converge play. Assert - # the retry wiring is present so it does not regress. - if not task.get("register"): - fail( - "task does not register a result; without retry wiring a transient " - "kube API error aborts the converge play" - ) - if not isinstance(task.get("until"), str) or "rc == 0" not in task["until"]: - fail( - "task does not retry on rc == 0; the kube API can transiently fail " - "while MetalLB converges and abort the play" - ) - if task.get("retries") is None: - fail("task is missing retries") - if task.get("delay") is None: - fail("task is missing delay") + webhook_task = find_task(tasks, "Test metallb-system webhook-service endpoint") + check_explicit_get(webhook_task, "Test metallb-system webhook-service endpoint", + "get endpoints") + check_retry_wiring(webhook_task, "Test metallb-system webhook-service endpoint") print("MetalLB namespace check regression test passed") diff --git a/roles/k3s_server_post/tasks/metallb.yml b/roles/k3s_server_post/tasks/metallb.yml index a4b9308..e06c906 100644 --- a/roles/k3s_server_post/tasks/metallb.yml +++ b/roles/k3s_server_post/tasks/metallb.yml @@ -105,6 +105,12 @@ ansible.builtin.command: >- {{ k3s_kubectl_binary | default('k3s kubectl') }} -n metallb-system get endpoints {{ metallb_webhook_service_name }} changed_when: false + # The kube API can briefly return ServiceUnavailable while MetalLB converges, + # which would otherwise abort the whole converge play on a transient error. + register: metallb_webhook_result + until: metallb_webhook_result.rc == 0 + retries: "{{ download_retries }}" + delay: "{{ download_delay }}" with_items: "{{ groups[group_name_master | default('master')] }}" run_once: true