Merge pull request #163 from cozystack/fix/v1.12-vip-on-vlan-child

fix(charts): pin Layer2VIPConfig to subnet-matching link, not default route

Author: lexfrei

README.md

@@ -61,7 +61,24 @@ talm init -p cozystack -N myawesomecluster --image factory.talos.dev/installer/<
 
 `--image` rewrites the top-level `image:` field in the preset's `values.yaml` before write. The flag is honored on initial `init` only — for an existing project, edit `values.yaml` directly. The `cozystack` preset declares `image:`; the `generic` preset does not, so `--image --preset generic` is rejected up front.
 
-Edit `values.yaml` to set your cluster's control-plane endpoint. This is the URL every node's kubelet and kube-proxy will dial. The chart leaves it empty on purpose so a missed override fails loudly instead of silently embedding a placeholder. For cozystack VIP setups set `endpoint` and `floatingIP` together (same IP, single shared VIP); for single-node clusters use that node's routable IP and leave `floatingIP` blank; for multi-node with an external load balancer use the LB URL and leave `floatingIP` blank. When the VIP must sit on a link that does not yet exist on the live system at first apply (typically a VLAN sub-interface), set `vipLink` to that link name — the chart pins `Layer2VIPConfig.link` to it instead of the default-gateway link that discovery would otherwise pick, and emits the document even on a totally fresh node where no default-gateway link has been discovered yet. The chart does not auto-emit a `LinkConfig` or `VLANConfig` for the override link; the operator is responsible for ensuring the link comes up, typically by adding a `LinkConfig` or `VLANConfig` for that link to the per-node body overlay alongside `vipLink`. Subnet-selector fields (`kubelet.validSubnets`, `etcd.advertisedSubnets`) are derived automatically from the node's default-gateway-bearing link, so no override is needed unless you have a multi-homed node that requires a specific subnet pinned.
+Edit `values.yaml` to set your cluster's control-plane endpoint. This is the URL every node's kubelet and kube-proxy will dial. The chart leaves it empty on purpose so a missed override fails loudly instead of silently embedding a placeholder.
+
+Endpoint / floatingIP combinations:
+
+- **cozystack VIP setup**: set `endpoint` and `floatingIP` together to the same IP — single shared VIP.
+- **single-node cluster**: set `endpoint` to the node's routable IP and leave `floatingIP` blank.
+- **multi-node with external load balancer**: set `endpoint` to the LB URL and leave `floatingIP` blank.
+
+When `vipLink` is left empty the chart picks the link automatically using a two-step rule:
+
+1. **Longest-prefix match across configurable links.** If `floatingIP` falls inside the CIDR of any address on a configurable link (physical NIC, bond, VLAN, bridge), the most specific subnet wins. This handles the Hetzner-style topology where a public NIC carries the default route and a VLAN child carries the private cluster subnet — the VIP lands on the VLAN child.
+2. **Fallback to the IPv4-default-gateway-bearing link.** Used when no configurable link's CIDR contains the `floatingIP` — typical for upstream-routable VIPs that arrive via the default route.
+
+Addresses on links the chart does not emit a per-link document for (Wireguard, kernel-managed loopback, slave NICs of a bond, anything outside the configurable set) are skipped — a VIP pinned there would have no surrounding network document.
+
+Set `vipLink` explicitly when the target link does not yet exist on the live system at first apply (typically a VLAN sub-interface). The chart pins `Layer2VIPConfig.link` to it directly and emits the document even on a fresh node where discovery has not yet populated the addresses table. The chart does not auto-emit a `LinkConfig` or `VLANConfig` for the override link; the operator is responsible for ensuring the link comes up, typically by adding a `LinkConfig` or `VLANConfig` for that link to the per-node body overlay alongside `vipLink`.
+
+Subnet-selector fields (`kubelet.validSubnets`, `etcd.advertisedSubnets`) are derived automatically from the node's default-gateway-bearing link, so no override is needed unless you have a multi-homed node that requires a specific subnet pinned.
 
 Boot Talos Linux node, let's say it has address `192.0.2.4`. Then:
 
@@ -126,7 +143,30 @@ cluster:
         endpoint: https://192.0.2.4:6443
 ```
 
-> **Note:** The output format depends on the Talos version configured in `Chart.yaml` (`templateOptions.talosVersion`) or via the `--talos-version` CLI flag. For Talos < v1.12, the output is a single YAML document with `machine.network` and `machine.registries` sections (as shown above). For Talos >= v1.12, the output uses the multi-document format with separate typed documents instead of the deprecated monolithic fields. `HostnameConfig` and `ResolverConfig` are always emitted; one network interface document is emitted per configurable link on the node (`LinkConfig` for physical NICs, `BondConfig` for bond masters, `VLANConfig` for VLAN sub-interfaces) — multi-NIC nodes therefore produce one document per NIC, not one document total. The link carrying the IPv4 default route gets the gateway entry on its document; every other link is emitted gateway-less. Both IPv4 and IPv6 global-scope addresses on a link are surfaced in its document. Bond slaves are filtered out so they do not collide with the master's `BondConfig`. Bridges are deliberately not auto-emitted as `BridgeConfig` yet — a non-gateway bridge is skipped (declare it via a per-node body overlay if needed); a bridge that carries the default route fails the render with a clear migration hint. The operator-declared `floatingIP` is stripped from per-link addresses so the VIP currently held by a leader does not leak into the static `LinkConfig`. `Layer2VIPConfig` appears on controlplane nodes when `floatingIP` is set; `RegistryMirrorConfig` is emitted only by the cozystack chart.
+> **Note: output format depends on Talos version.**
+>
+> Selected via `Chart.yaml` (`templateOptions.talosVersion`) or `--talos-version`:
+>
+> - **Talos < v1.12** — single YAML document with `machine.network` and `machine.registries` sections (as shown above).
+> - **Talos >= v1.12** — multi-document format with separate typed documents instead of the deprecated monolithic fields.
+>
+> For v1.12+ multi-doc output, one document is emitted per configurable link on the node, plus a fixed pair on every render:
+>
+> - `HostnameConfig` and `ResolverConfig` — always emitted.
+> - `LinkConfig` — physical NICs.
+> - `BondConfig` — bond masters. Bond slaves are filtered out so they do not collide with the master's document.
+> - `VLANConfig` — VLAN sub-interfaces.
+> - `BridgeConfig` — bridges, symmetric to `BondConfig` for bonds. Ports discovered via `spec.slaveKind == "bridge"` + `spec.masterIndex`; STP / VLAN-filtering settings reach the output when the bridge controller reports them on `spec.bridgeMaster`.
+> - `Layer2VIPConfig` — controlplane nodes when `floatingIP` is set.
+> - `RegistryMirrorConfig` — cozystack chart only.
+>
+> Per-link emission rules:
+>
+> - The link carrying the IPv4 default route gets the `routes.gateway` entry on its document; every other link is emitted gateway-less. Applies uniformly to `LinkConfig`, `BondConfig`, `VLANConfig`, `BridgeConfig`.
+> - Both IPv4 and IPv6 global-scope addresses on a link are surfaced.
+> - The operator-declared `floatingIP` is stripped from per-link addresses so the VIP currently held by a leader does not leak into the static document.
+>
+> Multi-NIC nodes therefore produce one document per NIC, not one document total.
 
 > **Version compatibility (`templateOptions.talosVersion` / `--talos-version`).** This setting must match the **Talos version actually running on the target node** — i.e. the maintenance ISO/PXE the node booted from for `apply -i`, or the installed Talos for an authenticated apply. It is **not** the same as `install.image`, which only controls what gets written to disk after a successful apply. When the configured contract is newer than the running binary, machinery injects fields (e.g. `machine.install.grubUseUKICmdline` from v1.12) that the running parser does not know, and the apply fails on the node side with `failed to parse config: unknown keys found during decoding: ...`. `talm apply` runs a best-effort pre-flight check against the running version and prints a `warning: pre-flight: ...` line with a hint when it detects this mismatch; if the warning is missed, the same hint is appended to the apply error. Either reboot the node into a maintenance image that matches the configured contract, or lower `templateOptions.talosVersion` / `--talos-version` to match what is running.
 

charts/cozystack/templates/_helpers.tpl

@@ -199,17 +199,37 @@ nameservers:
 {{- else }}
   []
 {{- end }}
+{{- /* Coerce .Values.floatingIP to its string form once at the
+       top of the multi-doc body and reuse the result everywhere
+       a downstream lookup or formatter needs it. The per-link
+       addresses_by_link strip on every link emission below
+       depends on the same stringified value — a worker render
+       with `floatingIP: 192168` would otherwise feed printf
+       "%s/" an int, producing `%!s(int=192168)/` that never
+       matches a CIDR. The coercion isolates that trap to one
+       place and lets the rest of the template treat the value
+       uniformly.
+
+       "<nil>" is Sprig's serialisation of nil and "" is the
+       unset string; both mean "operator did not supply a
+       value". The shared talm.validate_floatingIP partial below
+       handles the actual fail-fast — invoke it here AND in the
+       legacy define so a malformed value fails at render time
+       regardless of the rendered Talos version. */}}
+{{- $fipStr := .Values.floatingIP | toString }}
+{{- $fipIsSet := and (ne $fipStr "") (ne $fipStr "<nil>") }}
+{{- include "talm.validate_floatingIP" . }}
 {{- /* Operator-declared vipLink override: emit Layer2VIPConfig
        regardless of discovery state. Useful when the target link
        does not yet exist on the live system at first apply (typical
        case: a VLAN sub-interface this template is about to bring up).
        The discovery-derived block below skips its own Layer2VIPConfig
        when this branch fires, so we never emit duplicates. */}}
-{{- if and .Values.floatingIP .Values.vipLink (eq .MachineType "controlplane") }}
+{{- if and $fipIsSet .Values.vipLink (eq .MachineType "controlplane") }}
 ---
 apiVersion: v1alpha1
 kind: Layer2VIPConfig
-name: {{ .Values.floatingIP | quote }}
+name: {{ $fipStr | quote }}
 link: {{ .Values.vipLink }}
 {{- end }}
 {{- $defaultLinkName := include "talm.discovered.default_link_name_by_gateway" . }}
@@ -230,7 +250,13 @@ link: {{ .Values.vipLink }}
        and follower configs out of sync. */}}
 {{- $addresses := list }}
 {{- range $rawAddresses }}
-{{- if not (and $.Values.floatingIP (hasPrefix (printf "%s/" $.Values.floatingIP) .)) }}
+{{- /* Use the hoisted $fipStr/$fipIsSet from the top of the
+       define so the strip honours the same coerced value the
+       validation block above used. Going through `printf "%s/"
+       $.Values.floatingIP` directly would emit
+       `%!s(int=192168)/` for a numeric YAML scalar on a worker
+       render (controlplane was caught by the fail-fast). */ -}}
+{{- if not (and $fipIsSet (hasPrefix (printf "%s/" $fipStr) .)) }}
 {{- $addresses = append $addresses . }}
 {{- end }}
 {{- end }}
@@ -239,17 +265,58 @@ link: {{ .Values.vipLink }}
 {{- $linkGateway = include "talm.discovered.gateway_by_link" $linkName }}
 {{- end }}
 {{- if eq $kind "bridge" }}
-{{- /* BridgeConfig is a separate v1alpha1 typed document the chart
-       does not yet emit. Skipping a non-gateway bridge leaves the
-       rendered config without a bridge document and the operator is
-       responsible for declaring it via a per-node body. A bridge
-       carrying the IPv4 default route, however, cannot be silently
-       skipped: that would drop every network document for the
-       gateway link and the rendered config would describe a node
-       with no working uplink. Surface a fail with the offending
-       link and the migration path. */ -}}
-{{- if $isGatewayLink }}
-{{- fail (printf "talm: discovered bridge %q is the IPv4-default link, but BridgeConfig emission is not yet implemented in the chart. Move the bridge declaration into a per-node body overlay (kind: BridgeConfig), or set Values.vipLink to a different link until bridge support lands." $linkName) }}
+{{- /* BridgeConfig emission. Discovers bridge ports (members) via
+       talm.discovered.bridge_slaves and emits a typed v1.12+
+       BridgeConfig document with the same address / route / mtu
+       shape as the other branches. STP and VLAN filtering are
+       opt-in: they are emitted only when the bridge controller
+       reported a non-nil spec.bridgeMaster.stp / spec.bridgeMaster
+       value, so a default-state bridge stays minimal. */ -}}
+{{- $bridgeMaster := $link.spec.bridgeMaster }}
+{{- $bridgePorts := fromJsonArray (include "talm.discovered.bridge_slaves" $link.spec.index) }}
+---
+apiVersion: v1alpha1
+kind: BridgeConfig
+name: {{ $linkName }}
+{{- if $bridgePorts }}
+links:
+{{- range $bridgePorts }}
+  - {{ . }}
+{{- end }}
+{{- end }}
+{{- if $bridgeMaster }}
+{{- if $bridgeMaster.stp }}
+{{- if hasKey $bridgeMaster.stp "enabled" }}
+stp:
+  enabled: {{ $bridgeMaster.stp.enabled }}
+{{- end }}
+{{- end }}
+{{- /* COSI's BridgeVLANSpec serialises FilteringEnabled as
+       yaml:"filteringEnabled" (verified against
+       siderolabs/talos pkg/machinery/resources/network/link.go).
+       The output-side BridgeConfig schema uses the shorter
+       yaml:"filtering,omitempty" key — so we read the long form
+       from discovery and emit the short form into the rendered
+       document. */ -}}
+{{- if $bridgeMaster.vlan }}
+{{- if hasKey $bridgeMaster.vlan "filteringEnabled" }}
+vlan:
+  filtering: {{ $bridgeMaster.vlan.filteringEnabled }}
+{{- end }}
+{{- end }}
+{{- end }}
+{{- if $addresses }}
+addresses:
+{{- range $addresses }}
+  - address: {{ . }}
+{{- end }}
+{{- end }}
+{{- if $linkGateway }}
+routes:
+  - gateway: {{ $linkGateway }}
+{{- end }}
+{{- if $link.spec.mtu }}
+mtu: {{ $link.spec.mtu }}
 {{- end }}
 {{- else if eq $kind "bond" }}
 {{- $bondMaster := $link.spec.bondMaster }}
@@ -354,18 +421,52 @@ mtu: {{ $link.spec.mtu }}
 {{- /* Discovery-derived Layer2VIPConfig: skipped when the operator
        has set .Values.vipLink, since the override-path block above
        has already emitted the document with the operator's chosen
-       link. */}}
-{{- if and .Values.floatingIP (not .Values.vipLink) (eq .MachineType "controlplane") $defaultLinkName }}
+       link.
+
+       Link selection prefers the link whose discovered addresses
+       contain the floatingIP (talm.discovered.link_name_for_address),
+       so a VIP in a private subnet hosted on a VLAN child lands on
+       that VLAN — not on the IPv4-default-route NIC. The
+       default-gateway link stays as the fallback for topologies
+       where the VIP isn't on any discovered subnet (typical for
+       upstream-routable VIPs that arrive via the default-route
+       link). When neither resolves a link, no Layer2VIPConfig is
+       emitted, matching the prior behaviour. */}}
+{{- if and $fipIsSet (not .Values.vipLink) (eq .MachineType "controlplane") }}
+{{- $vipLink := include "talm.discovered.link_name_for_address" $fipStr }}
+{{- /* Default-gateway fallback must also point at a configurable
+       link — otherwise an unmanaged default-route NIC (Wireguard,
+       a slave NIC of a bond, anything outside the configurable
+       set) would silently win selection and the rendered
+       Layer2VIPConfig would dangle on a link the chart never
+       emits a per-link document for. Mirror the same
+       configurable-link gate link_name_for_address applies inside
+       its own iteration. */ -}}
+{{- if not $vipLink }}
+{{- if has $defaultLinkName $configurableLinks }}
+{{- $vipLink = $defaultLinkName }}
+{{- end }}
+{{- end }}
+{{- if $vipLink }}
 ---
 apiVersion: v1alpha1
 kind: Layer2VIPConfig
-name: {{ .Values.floatingIP | quote }}
-link: {{ $defaultLinkName }}
+name: {{ $fipStr | quote }}
+link: {{ $vipLink }}
+{{- end }}
 {{- end }}
 {{- end }}
 
 {{- /* Shared legacy network section for machine.network */ -}}
 {{- define "talos.config.network.legacy" }}
+{{- /* Coerce floatingIP through toString and call the shared
+       talm.validate_floatingIP partial so legacy renders fail at
+       template time on a malformed value, same as the multi-doc
+       path. $fipStr / $fipIsSet are reused below in place of every
+       direct .Values.floatingIP reference. */ -}}
+{{- $fipStr := .Values.floatingIP | toString }}
+{{- $fipIsSet := and (ne $fipStr "") (ne $fipStr "<nil>") }}
+{{- include "talm.validate_floatingIP" . }}
   network:
     hostname: {{ include "talm.discovered.hostname" . | quote }}
     nameservers: {{ include "talm.discovered.default_resolvers" . }}
@@ -377,7 +478,7 @@ link: {{ $defaultLinkName }}
        top-level interfaces[] entry that carries only the vip block.
        When vipLink == $defaultLinkName the inline vip below already
        lands on the right link, so no override entry is needed. */}}
-    {{- $vipOverride := and .Values.floatingIP .Values.vipLink (eq .MachineType "controlplane") (ne .Values.vipLink $defaultLinkName) }}
+    {{- $vipOverride := and $fipIsSet .Values.vipLink (eq .MachineType "controlplane") (ne .Values.vipLink $defaultLinkName) }}
     {{- /* Suppress the inline (discovery-derived) vip when the operator
        has redirected it to a different link; otherwise the VIP would
        be pinned twice on different interfaces. */}}
@@ -408,25 +509,25 @@ link: {{ $defaultLinkName }}
           routes:
             - network: 0.0.0.0/0
               gateway: {{ include "talm.discovered.default_gateway" . }}
-          {{- if and .Values.floatingIP (eq .MachineType "controlplane") (not $suppressInlineVip) }}
+          {{- if and $fipIsSet (eq .MachineType "controlplane") (not $suppressInlineVip) }}
           vip:
-            ip: {{ .Values.floatingIP }}
+            ip: {{ $fipStr }}
           {{- end }}
       {{- else }}
       addresses: {{ include "talm.discovered.default_addresses_by_gateway" . }}
       routes:
         - network: 0.0.0.0/0
           gateway: {{ include "talm.discovered.default_gateway" . }}
-      {{- if and .Values.floatingIP (eq .MachineType "controlplane") (not $suppressInlineVip) }}
+      {{- if and $fipIsSet (eq .MachineType "controlplane") (not $suppressInlineVip) }}
       vip:
-        ip: {{ .Values.floatingIP }}
+        ip: {{ $fipStr }}
       {{- end }}
       {{- end }}
     {{- end }}
     {{- if $vipOverride }}
     - interface: {{ .Values.vipLink }}
       vip:
-        ip: {{ .Values.floatingIP }}
+        ip: {{ $fipStr }}
     {{- end }}
     {{- end }}
 {{- end }}