fix(metallb): deploy MetalLB with a non-BGP Cilium CNI (#692)

- Correct the Deploy metallb manifest/pool when condition so MetalLB is
  installed whenever kube-vip does not own the VIP range and Cilium BGP
  is disabled
- The previous guard (cilium_bgp is not defined or cilium_iface is not
  defined) skipped MetalLB whenever cilium_iface was set, breaking the
  cilium + MetalLB scenario
- Use cilium_bgp | default(false) | bool to stay safe when Cilium vars are
  not in scope (#644) while still deploying MetalLB for non-BGP Cilium
- Retry the converge-side MetalLB resource wait so a transient kube API
  ServiceUnavailable does not abort the converge play
- Add a regression test that evaluates both when conditions across flannel,
  calico, non-BGP cilium, BGP cilium, and kube-vip scenarios

ci: skip CI for Dependabot pull requests

- Add an actor guard to the CI workflow jobs so automatic Dependabot PRs
  do not consume the shared self-hosted runner
- Dependabot CI runs need maintainer approval instead of auto-running

.github/scripts/test-metallb-deploy-condition.py

@@ -0,0 +1,170 @@
+#!/usr/bin/env python3
+"""Regression test for the MetalLB deploy conditions.
+
+The MetalLB manifest (roles/k3s_server/tasks/main.yml) and the MetalLB pool
+(roles/k3s_server_post/tasks/main.yml) are included under a `when` condition
+that decides whether MetalLB provides load balancing. A previous change (#683)
+guarded `cilium_bgp` but accidentally skipped MetalLB whenever a non-BGP
+Cilium CNI was in use (`cilium_iface` defined), breaking the cilium + MetalLB
+scenario.
+
+This test loads the real `when` expressions from both task files and evaluates
+them against representative variable sets, asserting MetalLB is deployed in
+every topology except when kube-vip owns the VIP range or Cilium BGP is enabled.
+"""
+
+from __future__ import print_function
+
+import os
+import re
+import subprocess
+
+import yaml
+from jinja2 import Environment
+
+METALLB_WHEN = (
+    "kube_vip_lb_ip_range is not defined and "
+    "not (cilium_bgp | default(false) | bool)"
+)
+
+
+def repo_root():
+    return subprocess.check_output(
+        ["git", "rev-parse", "--show-toplevel"], text=True
+    ).strip()
+
+
+def fail(message):
+    raise SystemExit("MetalLB deploy condition test failed: " + message)
+
+
+def extract_when(path, task_name):
+    """Return the `when:` expression string for the named task."""
+    with open(path, encoding="utf-8") as handle:
+        doc = yaml.safe_load(handle)
+    for task in doc:
+        if task.get("name") == task_name:
+            when = task.get("when")
+            return (when or "").strip()
+    return None
+
+
+def evaluate(when, variables):
+    """Evaluate a `when` expression against variables using Jinja2."""
+    env = Environment()
+
+    def fake_bool(value):
+        # Minimal stand-in for Ansible's truthiness filter used by `| bool`.
+        if isinstance(value, bool):
+            return value
+        if value is None:
+            return False
+        return str(value).lower() in ("1", "true", "yes", "on")
+
+    env.filters["bool"] = fake_bool
+    template = env.from_string("{{ " + when + " }}")
+    rendered = template.render(**variables)
+    # The expression renders to the literal strings "True"/"False".
+    if rendered == "True":
+        return True
+    if rendered == "False":
+        return False
+    fail("condition did not render to a boolean: {0!r}".format(rendered))
+
+
+def assert_deployment(when, variables, expected, label):
+    result = evaluate(when, variables)
+    verdict = "deploy" if result else "skip"
+    expected_verdict = "deploy" if expected else "skip"
+    if result != expected:
+        fail(
+            "{0}: expected to {1} MetalLB but the condition chose to {2} "
+            "(vars: {3})".format(label, expected_verdict, verdict, variables)
+        )
+
+
+def scenarios():
+    """Yield (variables, expected_deploy, label) pairs."""
+    yield (
+        # Default Flannel inventory (all.yml sets cilium_bgp: false).
+        {
+            "cilium_bgp": False,
+            "cilium_iface": None,
+        },
+        True,
+        "flannel default (cilium_bgp: false)",
+    )
+    yield (
+        # Calico CNI with no Cilium variable in scope (issue #644): cilium_bgp
+        # is genuinely undefined, so `default(false)` must keep MetalLB on.
+        {
+            "calico_iface": "eth1",
+        },
+        True,
+        "calico, cilium_bgp undefined (#644)",
+    )
+    yield (
+        # Cilium CNI with BGP disabled: MetalLB must still be deployed.
+        {
+            "cilium_bgp": False,
+            "cilium_iface": "eth1",
+        },
+        True,
+        "cilium non-BGP (regression catch)",
+    )
+    yield (
+        # Cilium CNI with BGP enabled: Cilium provides the LB, skip MetalLB.
+        {
+            "cilium_bgp": True,
+            "cilium_iface": "eth1",
+        },
+        False,
+        "cilium BGP enabled",
+    )
+    yield (
+        # kube-vip is the load balancer provider: skip MetalLB.
+        {
+            "kube_vip_lb_ip_range": "192.168.30.80-192.168.30.90",
+            "cilium_bgp": False,
+        },
+        False,
+        "kube-vip owns the VIP range",
+    )
+
+
+def main():
+    root = repo_root()
+    server_tasks = os.path.join(root, "roles", "k3s_server", "tasks", "main.yml")
+    server_post_tasks = os.path.join(
+        root, "roles", "k3s_server_post", "tasks", "main.yml"
+    )
+
+    server_when = extract_when(server_tasks, "Deploy metallb manifest")
+    server_post_when = extract_when(server_post_tasks, "Deploy metallb pool")
+
+    if server_when is None:
+        fail("could not find 'Deploy metallb manifest' when condition")
+    if server_post_when is None:
+        fail("could not find 'Deploy metallb pool' when condition")
+
+    for when, source in (
+        (server_when, "k3s_server/tasks/main.yml"),
+        (server_post_when, "k3s_server_post/tasks/main.yml"),
+    ):
+        if when != METALLB_WHEN:
+            fail(
+                "{0} when condition changed unexpectedly:\n"
+                "  expected: {1}\n  got:      {2}".format(source, METALLB_WHEN, when)
+            )
+
+    for variables, expected, label in scenarios():
+        assert_deployment(server_when, variables, expected, "server " + label)
+        assert_deployment(
+            server_post_when, variables, expected, "server_post " + label
+        )
+
+    print("MetalLB deploy condition regression test passed for all scenarios")
+
+
+if __name__ == "__main__":
+    main()

.github/workflows/ci.yml

@@ -32,10 +32,13 @@ concurrency:
 
 jobs:
   pre:
+    if: github.actor != 'dependabot[bot]'
     uses: ./.github/workflows/cache.yml
     needs: [lint]
   lint:
+    if: github.actor != 'dependabot[bot]'
     uses: ./.github/workflows/lint.yml
   test:
+    if: github.actor != 'dependabot[bot]'
     uses: ./.github/workflows/test.yml
     needs: [pre, lint]

.pre-commit-config.yaml

@@ -84,6 +84,15 @@ repos:
         language: system
         pass_filenames: false
         files: ^roles/k3s_server/tasks/metallb\.yml$|^\.github/scripts/test-metallb-remote-read\.sh$
+      - id: metallb-deploy-condition-test
+        name: MetalLB deploy condition test
+        entry: python3 .github/scripts/test-metallb-deploy-condition.py
+        language: python
+        additional_dependencies:
+          - Jinja2>=3.1
+          - PyYAML
+        pass_filenames: false
+        files: ^roles/k3s_server/tasks/main\.yml$|^roles/k3s_server_post/tasks/main\.yml$|^\.github/scripts/test-metallb-deploy-condition\.py$ # noqa yaml[line-length]
       - id: default-interface-test
         name: default interface test
         entry: python3 .github/scripts/test-default-interface.py

roles/k3s_server/tasks/main.yml

@@ -27,7 +27,11 @@
 - name: Deploy metallb manifest
   ansible.builtin.include_tasks: metallb.yml
   tags: metallb
-  when: kube_vip_lb_ip_range is not defined and (cilium_bgp is not defined or cilium_iface is not defined)
+  # Deploy MetalLB unless kube-vip owns the load balancer IP range, or Cilium
+  # BGP is enabled (Cilium then provides its own load balancing). The cilium_bgp
+  # default keeps this safe when Cilium variables are not in scope at all (#644)
+  # while still deploying MetalLB when a non-BGP Cilium CNI is in use.
+  when: kube_vip_lb_ip_range is not defined and not (cilium_bgp | default(false) | bool)
 
 - name: Deploy kube-vip manifest
   ansible.builtin.include_tasks: kube-vip.yml

roles/k3s_server_post/defaults/main.yml

@@ -23,6 +23,13 @@ cilium_mode: native
 cilium_tag: v1.20.0
 cilium_cli_tag: v0.19.7
 
+# Shared retry/delay for remote manifest, asset downloads, and waiting on
+# Kubernetes resources. The CI runner's resolver intermittently times out on
+# GitHub-hosted domains and the kube API can transiently return
+# ServiceUnavailable, so retry transient DNS/network/API failures.
+download_retries: 5
+download_delay: 10
+
 cluster_cidr: 10.52.0.0/16
 enable_bpf_masquerade: true
 kube_proxy_replacement: true

roles/k3s_server_post/tasks/main.yml

@@ -12,7 +12,11 @@
 - name: Deploy metallb pool
   ansible.builtin.include_tasks: metallb.yml
   tags: metallb
-  when: kube_vip_lb_ip_range is not defined and (cilium_bgp is not defined or cilium_iface is not defined)
+  # Deploy MetalLB unless kube-vip owns the load balancer IP range, or Cilium
+  # BGP is enabled (Cilium then provides its own load balancing). The cilium_bgp
+  # default keeps this safe when Cilium variables are not in scope at all (#644)
+  # while still deploying MetalLB when a non-BGP Cilium CNI is in use.
+  when: kube_vip_lb_ip_range is not defined and not (cilium_bgp | default(false) | bool)
 
 - name: Remove tmp directory used for manifests
   ansible.builtin.file:

roles/k3s_server_post/tasks/metallb.yml

@@ -54,6 +54,12 @@
     {% if item.condition | default(False) -%}{{ item.condition }}{%- endif %}
     --timeout='{{ metal_lb_available_timeout }}'
   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_wait_result
+  until: metallb_wait_result.rc == 0
+  retries: "{{ download_retries }}"
+  delay: "{{ download_delay }}"
   run_once: true
   with_items:
     - description: controller