fix(dependencies): address PR review findings

- Read the MetalLB speaker tag check from the managed host with slurp
  instead of a controller-side file lookup, and match the full image
  reference
- Restore the tigera-operator namespace on the Calico operator Deployment
  wait while keeping the managed CRD waits cluster-scoped
- Make Molecule verify inputs durable and scenario-specific via a
  per-scenario verify-vars.yml, driven by explicit verify_cni/verify_lb
  values instead of non-persisted converge facts
- Rename the kube-vip multi-peer BGP env var from bgppeers to bgp_peers
  and vip_cidr to vip_subnet so v1.2.2 actually reads them
- Map the legacy Cilium routed mode to tunnel and stop passing the alias
  directly to the chart
- Use return-code based failed_when on apply and preflight commands so
  non-error failures are no longer treated as success
- Clarify the sequential K3s upgrade path and backups in the README
- Add kube-vip and MetalLB regression tests and a Cilium mode mapping unit
This commit is contained in:
Timothy Stewart 2026-08-01 22:36:14 -05:00
parent 4e71491152
commit 6d49ad6977
19 changed files with 309 additions and 38 deletions

View File

@ -15,6 +15,10 @@ exclude_paths:
- molecule/**/prepare.yml
- molecule/**/reset.yml
# Scenario verify inputs are plain variable files, not playbooks. They are
# loaded as vars, not executed, so ansible-lint must not treat them as plays.
- molecule/**/verify-vars.yml
# The file was generated by galaxy ansible - don't mess with it.
- galaxy.yml

View File

@ -0,0 +1,130 @@
#!/usr/bin/env python3
"""Render the kube-vip DaemonSet template and assert env key correctness.
kube-vip v1.2.2 reads `bgp_peers` and `vip_subnet`; it ignores the older
`bgppeers` and `vip_cidr` names. This test proves the rendered manifest uses
the keys the target image actually parses.
"""
from __future__ import print_function
import os
import subprocess
from jinja2 import Environment, FileSystemLoader, StrictUndefined
def repo_root():
return subprocess.check_output(
["git", "rev-parse", "--show-toplevel"], text=True
).strip()
def fail(message):
raise SystemExit("kube-vip manifest test failed: " + message)
def fake_ipsubnet(value):
# ansible.utils.ipsubnet -> network of the address as x.y.z.0/24
parts = value.split(".")
return ".".join(parts[:3]) + ".0/24"
def fake_ipaddr(_value, expr=None):
# ansible.utils.ipaddr('prefix') -> prefix length
return "24"
def fake_bool(value):
# Minimal stand-in for Ansible's truthiness filter used by the template.
if isinstance(value, bool):
return value
return str(value).lower() in ("1", "true", "yes", "on")
def fake_map(seq, *args, **kwargs):
# Minimal stand-in for Ansible's map() filter in the two forms used by the
# template: map(attribute='x') on a list of dicts, and map('join', sep) on
# a list of sequences.
if "attribute" in kwargs:
return [item[kwargs["attribute"]] for item in seq]
if kwargs:
# e.g. map(default='x') not used here; ignore unknown kwargs.
return list(seq)
if args:
filter_name = args[0]
sep = args[1] if len(args) > 1 else ""
if filter_name == "join":
return [sep.join(str(x) for x in item) for item in seq]
return list(seq)
def fake_zip(*seqs):
return list(zip(*seqs))
def render(env, extra_vars):
base_vars = {
"apiserver_endpoint": "192.168.30.222",
"kube_vip_iface": "",
"kube_vip_arp": True,
"kube_vip_bgp": True,
"kube_vip_bgp_routerid": "127.0.0.1",
"_kube_vip_bgp_peers": [
{"peer_address": "192.168.30.1", "peer_asn": "64512"},
{"peer_address": "192.168.30.2", "peer_asn": "64513"},
],
"kube_vip_tag_version": "v1.2.2",
}
base_vars.update(extra_vars)
template = env.get_template("vip.yaml.j2")
return template.render(**base_vars)
def main():
root = repo_root()
template_dir = os.path.join(root, "roles", "k3s_server", "templates")
env = Environment(
loader=FileSystemLoader(template_dir), undefined=StrictUndefined
)
env.filters["ansible.utils.ipsubnet"] = fake_ipsubnet
env.filters["ansible.utils.ipaddr"] = fake_ipaddr
env.filters["bool"] = fake_bool
env.filters["map"] = fake_map
env.filters["zip"] = fake_zip
# Multi-peer BGP armed: must emit bgp_peers, never bgppeers.
output = render(env, {})
if "name: bgp_peers" not in output:
fail("rendered manifest is missing bgp_peers")
if "name: bgppeers" in output:
fail("rendered manifest still uses the ignored bgppeers key")
if "name: vip_subnet" not in output:
fail("rendered manifest is missing vip_subnet")
if "name: vip_cidr" in output:
fail("rendered manifest still uses the ignored vip_cidr key")
if "192.168.30.1:64512,192.168.30.2:64513" not in output:
fail("bgp_peers value is not comma-separated address:ASN entries")
if "ghcr.io/kube-vip/kube-vip:v1.2.2" not in output:
fail("kube-vip image tag is not v1.2.2")
# BGP enabled with no merged peers: single-peer fallback vars, no bgp_peers.
output = render(
env,
{
"_kube_vip_bgp_peers": [],
"kube_vip_bgp_as": "64513",
"kube_vip_bgp_peeraddress": "192.168.30.1",
"kube_vip_bgp_peeras": "64512",
},
)
if "name: bgp_as" not in output:
fail("single-peer bgp_as was not rendered")
if "name: bgp_peers" in output:
fail("bgp_peers present even though the peer list is empty")
print("kube-vip manifest regression test passed")
if __name__ == "__main__":
main()

29
.github/scripts/test-metallb-remote-read.sh vendored Executable file
View File

@ -0,0 +1,29 @@
#!/usr/bin/env bash
set -Eeuo pipefail
repo_root="$(git rev-parse --show-toplevel)"
metallb_task="$repo_root/roles/k3s_server/tasks/metallb.yml"
# The speaker tag verification must read the rendered manifest on the managed
# host with slurp. A controller-side lookup('ansible.builtin.file', ...) would
# read from the Ansible control node, which does not have the file, and would
# fail on every MetalLB scenario.
grep -Fq -- 'ansible.builtin.slurp' "$metallb_task" || {
printf 'MetalLB speaker tag check does not use slurp on the managed host\n' >&2
exit 1
}
grep -Eq -- 'lookup\(.?ansible\.builtin\.file' "$metallb_task" && {
printf 'MetalLB speaker tag check uses a controller-side file lookup\n' >&2
exit 1
}
# The check must reference the full image reference, not just a bare version
# string that could appear anywhere in the manifest.
grep -Fq -- 'quay.io/metallb/speaker:' "$metallb_task" || {
printf 'MetalLB speaker tag check does not match the full image reference\n' >&2
exit 1
}
printf 'MetalLB remote manifest read regression test passed\n'

View File

@ -70,3 +70,17 @@ repos:
- Jinja2>=3.1
pass_filenames: false
files: ^roles/k3s_server_post/templates/cilium\.crs\.j2$|^\.github/scripts/test-cilium-bgp-manifest\.py$
- id: kube-vip-manifest-test
name: kube-vip manifest test
entry: python3 .github/scripts/test-kube-vip-manifest.py
language: python
additional_dependencies:
- Jinja2>=3.1
pass_filenames: false
files: ^roles/k3s_server/templates/vip\.yaml\.j2$|^\.github/scripts/test-kube-vip-manifest\.py$
- id: metallb-remote-read-test
name: MetalLB remote read test
entry: .github/scripts/test-metallb-remote-read.sh
language: system
pass_filenames: false
files: ^roles/k3s_server/tasks/metallb\.yml$|^\.github/scripts/test-metallb-remote-read\.sh$

View File

@ -98,9 +98,12 @@ They are not a supported direct in-place upgrade path for an existing cluster.
K3s, Calico, and Cilium each require staged upgrades for long-lived clusters.
- **K3s**: do not jump an embedded-etcd cluster straight to Kubernetes 1.36.
First run a K3s patch that contains etcd 3.5.26 (for example `v1.33.7+k3s3`
or newer in the 1.33 line), then advance one Kubernetes minor version at a
time. Upgrade servers one at a time before agents. See
Upgrade one Kubernetes minor version at a time. From the sample default
(`v1.30.2+k3s2`) the sequence is: the latest supported 1.30 patch, then 1.31,
1.32, a 1.33 patch that contains etcd 3.5.26 (for example `v1.33.7+k3s3`),
then 1.34, 1.35, and finally 1.36. Upgrade servers one at a time before
agents. Take backups and confirm cluster health at each step; this playbook
does not automate the upgrade, so those remain manual operational steps. See
[K3s manual upgrades](https://docs.k3s.io/upgrades/manual) and the
[v1.34 release notes](https://docs.k3s.io/release-notes/v1.34.X).
- **Cilium**: upstream supports only consecutive minor upgrades. Update to the

View File

@ -0,0 +1,6 @@
---
# Durable verify inputs for the calico (Calico CNI + MetalLB) scenario.
verify_cni: calico
verify_lb: metallb
verify_lb_ip_range:
- 192.168.30.100-192.168.30.109

View File

@ -0,0 +1,6 @@
---
# Durable verify inputs for the cilium (Cilium CNI + MetalLB) scenario.
verify_cni: cilium
verify_lb: metallb
verify_lb_ip_range:
- 192.168.30.110-192.168.30.119

View File

@ -0,0 +1,9 @@
---
# Durable verify inputs for the default (flannel + MetalLB) scenario.
# These are plain inventory vars linked into the shared Molecule inventory so
# the verify play can see them even though the converge play's set_fact values
# are not persisted between the two Ansible processes.
verify_cni: flannel
verify_lb: metallb
verify_lb_ip_range:
- 192.168.30.80-192.168.30.90

View File

@ -0,0 +1,7 @@
---
# Durable verify inputs for the ipv6 (flannel CNI + MetalLB) scenario.
verify_cni: flannel
verify_lb: metallb
verify_lb_ip_range:
- fdad:bad:ba55::1b:0/112
- 192.168.123.80-192.168.123.90

View File

@ -0,0 +1,9 @@
---
# Durable verify inputs for the kube-vip (flannel CNI + kube-vip LB) scenario.
verify_cni: flannel
verify_lb: kube-vip
# The kube-vip cloud provider tag is not defined in the linked sample group
# vars (its sample entry is commented out), so it is supplied here.
verify_kube_vip_cloud_provider_tag: v0.0.12
verify_lb_ip_range:
- 192.168.30.110-192.168.30.119

View File

@ -1,5 +1,8 @@
---
- name: Verify
hosts: all
vars_files:
- >-
{{ lookup("ansible.builtin.env", "MOLECULE_SCENARIO_DIRECTORY") }}/verify-vars.yml
roles:
- verify_from_outside

View File

@ -50,20 +50,24 @@
success_msg: "LoadBalancer address {{ ip_value }} is in the configured range"
fail_msg: >-
LoadBalancer address {{ ip_value }} is not in the configured
range {{ lb_range }}
range {{ verify_lb_ip_range }}
vars:
ip_value: >-
{{ nginx_services.resources[0].status.loadBalancer.ingress[0].ip }}
lb_range: >-
{{ kube_vip_lb_ip_range | default(metal_lb_ip_range, true) }}
lb_int: "{{ ip_value | ansible.utils.ipaddr('int') }}"
start_int: "{{ lb_range.split('-')[0] | ansible.utils.ipaddr('int') }}"
end_int: "{{ lb_range.split('-')[1] | ansible.utils.ipaddr('int') }}"
lb_first: "{{ verify_lb_ip_range[0] | default('') }}"
lb_is_range: "{{ '-' in lb_first and '/' not in lb_first }}"
lb_start: "{{ lb_first.split('-')[0] | default(ip_value) }}"
lb_end: "{{ lb_first.split('-')[1] | default(ip_value) }}"
# Strict membership is checked only when the first range entry is a
# start-end pair. CIDR entries (used by the ipv6 scenario) fall back to
# the reachability probe above.
lb_addr_in_range: >-
{{
(lb_range.split('-') | length == 2) and
(lb_int >= start_int) and
(lb_int <= end_int)
(verify_lb_ip_range | length > 0) and
((not lb_is_range) or (
(ip_value | ansible.utils.ipaddr('int') >= lb_start | ansible.utils.ipaddr('int')) and
(ip_value | ansible.utils.ipaddr('int') <= lb_end | ansible.utils.ipaddr('int'))
))
}}
# Deactivated linter rules:
# - jinja[invalid]: As of version 6.6.0, ansible-lint complains that the input to ipwrap

View File

@ -1,7 +1,9 @@
---
# Scenario-aware verification of cluster components and their live image tags.
# Branches on the active CNI (flannel / calico / cilium) and load balancer
# (MetalLB / kube-vip) using the same variable names the install roles use.
# Scenario identity (verify_cni / verify_lb) and expected address range come
# from each scenario's verify-vars.yml, which is plain inventory data available
# to the verify play. Converge-time set_fact values are not persisted between
# the two Ansible processes, so they are never used here.
- name: Verify cluster components report expected versions
block:
- name: Get all nodes with their kubelet versions
@ -23,7 +25,7 @@
label: "{{ item.metadata.name }}"
- name: Verify Flannel is the active CNI
when: cilium_iface is not defined and calico_iface is not defined
when: verify_cni == 'flannel'
block:
- name: Get the Flannel DaemonSet
kubernetes.core.k8s_info:
@ -42,8 +44,24 @@
success_msg: "Flannel DaemonSet is Ready"
fail_msg: "Flannel DaemonSet is not fully Ready"
- name: Verify there is no incompatible CNI with Flannel
when: verify_cni == 'flannel'
block:
- name: Get any Calico namespaces with Flannel enabled
kubernetes.core.k8s_info:
kind: Namespace
name: calico-system
kubeconfig: "{{ kubecfg_path }}"
register: calico_absent
- name: Assert there is no Calico system namespace
ansible.builtin.assert:
that: calico_absent.resources | length == 0
success_msg: "No Calico present with Flannel"
fail_msg: "A Calico namespace exists alongside Flannel"
- name: Verify Calico is the active CNI
when: calico_iface is defined
when: verify_cni == 'calico'
block:
- name: Get the Calico node DaemonSet image
kubernetes.core.k8s_info:
@ -97,6 +115,7 @@
namespace: kube-flannel
kubeconfig: "{{ kubecfg_path }}"
register: no_flannel_ds
- name: Assert there are no Flannel DaemonSets
ansible.builtin.assert:
that: no_flannel_ds.resources | length == 0
@ -104,7 +123,7 @@
fail_msg: "A Flannel DaemonSet exists alongside Calico"
- name: Verify Cilium is the active CNI
when: cilium_iface is defined
when: verify_cni == 'cilium'
block:
- name: Get the Cilium agent and operator images
kubernetes.core.k8s_info:
@ -179,6 +198,7 @@
namespace: kube-flannel
kubeconfig: "{{ kubecfg_path }}"
register: no_flannel_ds_cilium
- name: Assert there are no Flannel DaemonSets
ansible.builtin.assert:
that: no_flannel_ds_cilium.resources | length == 0
@ -186,7 +206,7 @@
fail_msg: "A Flannel DaemonSet exists alongside Cilium"
- name: Verify MetalLB is the active load balancer
when: kube_vip_lb_ip_range is not defined
when: verify_lb == 'metallb'
block:
- name: Get the MetalLB controller and speaker images
kubernetes.core.k8s_info:
@ -231,7 +251,7 @@
| list)[0].spec.template.spec.containers[0].image }}
- name: Verify kube-vip is the active load balancer
when: kube_vip_lb_ip_range is defined
when: verify_lb == 'kube-vip'
block:
- name: Get the kube-vip and cloud provider images
kubernetes.core.k8s_info:
@ -250,10 +270,10 @@
ansible.builtin.assert:
that:
- kubevip_image | regex_search(':' ~ kube_vip_tag_version)
- cloud_provider_image | regex_search(kube_vip_cloud_provider_tag_version)
- cloud_provider_image | regex_search(verify_kube_vip_cloud_provider_tag)
success_msg: >-
kube-vip {{ kube_vip_tag_version }},
cloud provider {{ kube_vip_cloud_provider_tag_version }}
cloud provider {{ verify_kube_vip_cloud_provider_tag }}
fail_msg: >-
kube-vip {{ kubevip_image }},
cloud provider {{ cloud_provider_image }}
@ -281,6 +301,7 @@
name: metallb-system
kubeconfig: "{{ kubecfg_path }}"
register: metallb_absent
- name: Assert the MetalLB namespace does not exist
ansible.builtin.assert:
that: metallb_absent.resources | length == 0

View File

@ -0,0 +1,6 @@
---
# Durable verify inputs for the single_node (flannel + MetalLB) scenario.
verify_cni: flannel
verify_lb: metallb
verify_lb_ip_range:
- 192.168.30.91-192.168.30.99

View File

@ -29,19 +29,26 @@
label: "{{ item.change }} => {{ item.to }}"
when: ansible_hostname == hostvars[groups[group_name_master | default('master')][0]]['ansible_hostname']
- name: Read back MetalLB manifest to verify the speaker image tag
- name: Read back MetalLB manifest from first master
ansible.builtin.slurp:
src: /var/lib/rancher/k3s/server/manifests/metallb-crds.yaml
register: metallb_manifest
when: ansible_hostname == hostvars[groups[group_name_master | default('master')][0]]['ansible_hostname']
- name: Check the MetalLB speaker image reference is present
ansible.builtin.set_fact:
metallb_manifest_has_speaker_tag: >-
{{
metal_lb_speaker_tag_version in
lookup('ansible.builtin.file', '/var/lib/rancher/k3s/server/manifests/metallb-crds.yaml')
('quay.io/metallb/speaker:' ~ metal_lb_speaker_tag_version) in
(metallb_manifest.content | default('') | b64decode)
}}
when: ansible_hostname == hostvars[groups[group_name_master | default('master')][0]]['ansible_hostname']
- name: Fail if MetalLB speaker tag was not applied to the manifest
ansible.builtin.fail:
msg: >-
MetalLB speaker image tag {{ metal_lb_speaker_tag_version }}
MetalLB speaker image reference
quay.io/metallb/speaker:{{ metal_lb_speaker_tag_version }}
was not found in the downloaded manifest. The upstream image
reference may have changed.
when:

View File

@ -36,7 +36,7 @@ spec:
- name: vip_interface
value: {{ kube_vip_iface }}
{% endif %}
- name: vip_cidr
- name: vip_subnet
value: "{{ apiserver_endpoint | ansible.utils.ipsubnet | ansible.utils.ipaddr('prefix') }}"
- name: cp_enable
value: "true"
@ -62,7 +62,7 @@ spec:
value: "{{ kube_vip_bgp_routerid }}"
{% endif %}
{% if _kube_vip_bgp_peers | length > 0 %}
- name: bgppeers
- name: bgp_peers
value: "{{ _kube_vip_bgp_peers | map(attribute='peer_address') | zip(_kube_vip_bgp_peers | map(attribute='peer_asn')) | map('join', ':') | join(',') }}" # yamllint disable-line rule:line-length
{% else %}
{% if kube_vip_bgp_as is defined %}

View File

@ -93,10 +93,14 @@ argument_specs:
default: ~
cilium_mode:
description: Inner-node communication mode
description:
- Inner-node communication mode.
- Cilium accepts `native` or `tunnel`. `routed` is accepted as a
legacy alias and is mapped to `tunnel` at install time.
default: native
choices:
- native
- tunnel
- routed
cilium_tag:

View File

@ -37,7 +37,7 @@
'created' in apply_crds.stdout or
'configured' in apply_crds.stdout or
'applied' in apply_crds.stdout
failed_when: "'Error' in apply_crds.stderr"
failed_when: apply_crds.rc != 0
- name: Apply Tigera Operator manifest idempotently with server-side apply
ansible.builtin.command: >-
@ -49,11 +49,12 @@
'created' in apply_operator.stdout or
'configured' in apply_operator.stdout or
'applied' in apply_operator.stdout
failed_when: "'Error' in apply_operator.stderr"
failed_when: apply_operator.rc != 0
- name: Wait for Tigera Operator and managed CRDs to become available
ansible.builtin.command: >-
{{ k3s_kubectl_binary | default('k3s kubectl') }} wait {{ item.type }}/{{ item.name }}
{% if item.namespace is defined %}--namespace='{{ item.namespace }}'{% endif %}
--for=condition={{ item.condition }}
--timeout=30s
register: tigera_result
@ -62,7 +63,7 @@
retries: 7
delay: 7
with_items:
- { name: tigera-operator, type: deployment, condition: Available=True }
- { name: tigera-operator, type: deployment, namespace: tigera-operator, condition: Available=True }
- { name: installations.operator.tigera.io, type: crd, condition: Established }
- { name: apiservers.operator.tigera.io, type: crd, condition: Established }
loop_control:
@ -85,7 +86,7 @@
'configured' in apply_cr.stdout or
'created' in apply_cr.stdout or
'unchanged' in apply_cr.stdout
failed_when: "'Error' in apply_cr.stderr"
failed_when: apply_cr.rc != 0
- name: Wait for Calico system resources to be available
ansible.builtin.command: >-

View File

@ -143,6 +143,13 @@
Target Cilium version: {{ cilium_tag }},
Update needed: {{ cilium_needs_update }}
- name: Map the legacy routed mode to Cilium tunnel mode
ansible.builtin.set_fact:
# Cilium 1.20 accepts `native` or `tunnel`. `routed` was the name used
# by older releases and is kept only as a backward-compatible alias.
cilium_routing_mode: >-
{{ 'tunnel' if cilium_mode == 'routed' else cilium_mode }}
- name: Install Cilium
ansible.builtin.command: >-
{% if cilium_installed.rc != 0 %}
@ -154,13 +161,13 @@
--helm-set operator.replicas="1"
{{ '--helm-set devices=' + cilium_iface if cilium_iface != 'auto' else '' }}
--helm-set ipam.operator.clusterPoolIPv4PodCIDRList={{ cluster_cidr }}
{% if cilium_mode == "native" or (cilium_bgp and cilium_exportPodCIDR != 'false') %}
{% if cilium_routing_mode == "native" or (cilium_bgp and cilium_exportPodCIDR != 'false') %}
--helm-set ipv4NativeRoutingCIDR={{ cluster_cidr }}
{% endif %}
--helm-set k8sServiceHost="127.0.0.1"
--helm-set k8sServicePort="6444"
--helm-set routingMode={{ cilium_mode }}
--helm-set autoDirectNodeRoutes={{ "true" if cilium_mode == "native" else "false" }}
--helm-set routingMode={{ cilium_routing_mode }}
--helm-set autoDirectNodeRoutes={{ "true" if cilium_routing_mode == "native" else "false" }}
--helm-set kubeProxyReplacement={{ kube_proxy_replacement }}
--helm-set bpf.masquerade={{ enable_bpf_masquerade }}
--helm-set bgpControlPlane.enabled={{ cilium_bgp | default("false") }}
@ -236,7 +243,7 @@
apply --dry-run=server -f /tmp/k3s/cilium-bgp.yaml
register: preflight_cr
changed_when: false
failed_when: "'error' in preflight_cr.stderr | lower"
failed_when: preflight_cr.rc != 0
- name: Apply BGP manifests
ansible.builtin.command: >-
@ -244,7 +251,7 @@
apply -f /tmp/k3s/cilium-bgp.yaml
register: apply_cr
changed_when: "'configured' in apply_cr.stdout or 'created' in apply_cr.stdout"
failed_when: "'is invalid' in apply_cr.stderr"
failed_when: apply_cr.rc != 0
- name: Remove deprecated CiliumBGPPeeringPolicy after v2 resources are accepted
ansible.builtin.command: >-
@ -252,8 +259,9 @@
delete CiliumBGPPeeringPolicy.cilium.io 01-bgp-peering-policy
register: delete_old_policy
changed_when: "'deleted' in delete_old_policy.stdout"
# The policy (and possibly its CRD) may already be absent; this is
# intentionally tolerated, not a command whose failure must be hidden.
failed_when: false
ignore_errors: true
- name: Test for BGP config resources
ansible.builtin.command: "{{ item }}"