Skip to content

Commit 5f516d6

Browse files
myasnikovdaniilmyasnikovdaniil
committed
fix(charts): honor vipLink override on legacy Talos config path
The .Values.vipLink override added in 5f902a1 was silently ignored when TalosVersion is empty or <1.12, because talos.config.network.legacy never read .Values.vipLink and the legacy schema has no Layer2VIPConfig document. For the generic preset, whose Chart.yaml ships talosVersion: "", the legacy branch is the default code path on a fresh `talm init -p generic` -- so the documented vipLink workflow silently misconfigured the VIP onto the discovered NIC. In the legacy v1alpha1 schema, VIPs live at machine.network.interfaces[].vip. The override is now expressed as a separate top-level interfaces[] entry that carries only the vip block, mirroring multidoc's decoupled-Layer2VIPConfig pattern. The inline discovery-derived vip on the discovered interface is suppressed when vipLink redirects the VIP, to avoid pinning the same VIP on two different links. When vipLink names the same link discovery already picked, no override entry is emitted (Talos legacy rejects duplicate interface names). Adds regression tests for both presets that exercise the legacy branch by rendering with TalosVersion="", including the fresh-node case (no discovery) and the vipLink-matches-discovery no-op case. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
1 parent d689803 commit 5f516d6

3 files changed

Lines changed: 149 additions & 8 deletions

File tree

‎charts/cozystack/templates/_helpers.tpl‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,11 +297,21 @@ link: {{ $vipLinkName }}
297297
{{- (include "talm.discovered.physical_links_info" .) | nindent 4 }}
298298
{{- $existingInterfacesConfiguration := include "talm.discovered.existing_interfaces_configuration" . }}
299299
{{- $defaultLinkName := include "talm.discovered.default_link_name_by_gateway" . }}
300-
{{- if or $existingInterfacesConfiguration $defaultLinkName }}
300+
{{- /* vipLink override on the legacy schema: legacy Talos has no
301+
Layer2VIPConfig document, so the override is expressed as a
302+
top-level interfaces[] entry that carries only the vip block.
303+
When vipLink == $defaultLinkName the inline vip below already
304+
lands on the right link, so no override entry is needed. */}}
305+
{{- $vipOverride := and .Values.floatingIP .Values.vipLink (eq .MachineType "controlplane") (ne .Values.vipLink $defaultLinkName) }}
306+
{{- /* Suppress the inline (discovery-derived) vip when the operator
307+
has redirected it to a different link; otherwise the VIP would
308+
be pinned twice on different interfaces. */}}
309+
{{- $suppressInlineVip := and .Values.vipLink (ne .Values.vipLink $defaultLinkName) }}
310+
{{- if or $existingInterfacesConfiguration $defaultLinkName $vipOverride }}
301311
interfaces:
302312
{{- if $existingInterfacesConfiguration }}
303313
{{- $existingInterfacesConfiguration | nindent 4 }}
304-
{{- else }}
314+
{{- else if $defaultLinkName }}
305315
{{- $isVlan := include "talm.discovered.is_vlan" $defaultLinkName }}
306316
{{- $parentLinkName := "" }}
307317
{{- if $isVlan }}
@@ -323,7 +333,7 @@ link: {{ $vipLinkName }}
323333
routes:
324334
- network: 0.0.0.0/0
325335
gateway: {{ include "talm.discovered.default_gateway" . }}
326-
{{- if and .Values.floatingIP (eq .MachineType "controlplane") }}
336+
{{- if and .Values.floatingIP (eq .MachineType "controlplane") (not $suppressInlineVip) }}
327337
vip:
328338
ip: {{ .Values.floatingIP }}
329339
{{- end }}
@@ -332,12 +342,17 @@ link: {{ $vipLinkName }}
332342
routes:
333343
- network: 0.0.0.0/0
334344
gateway: {{ include "talm.discovered.default_gateway" . }}
335-
{{- if and .Values.floatingIP (eq .MachineType "controlplane") }}
345+
{{- if and .Values.floatingIP (eq .MachineType "controlplane") (not $suppressInlineVip) }}
336346
vip:
337347
ip: {{ .Values.floatingIP }}
338348
{{- end }}
339349
{{- end }}
340350
{{- end }}
351+
{{- if $vipOverride }}
352+
- interface: {{ .Values.vipLink }}
353+
vip:
354+
ip: {{ .Values.floatingIP }}
355+
{{- end }}
341356
{{- end }}
342357
{{- end }}
343358

‎charts/generic/templates/_helpers.tpl‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,21 @@ link: {{ $vipLinkName }}
220220
{{- (include "talm.discovered.physical_links_info" .) | nindent 4 }}
221221
{{- $existingInterfacesConfiguration := include "talm.discovered.existing_interfaces_configuration" . }}
222222
{{- $defaultLinkName := include "talm.discovered.default_link_name_by_gateway" . }}
223-
{{- if or $existingInterfacesConfiguration $defaultLinkName }}
223+
{{- /* vipLink override on the legacy schema: legacy Talos has no
224+
Layer2VIPConfig document, so the override is expressed as a
225+
top-level interfaces[] entry that carries only the vip block.
226+
When vipLink == $defaultLinkName the inline vip below already
227+
lands on the right link, so no override entry is needed. */}}
228+
{{- $vipOverride := and .Values.floatingIP .Values.vipLink (eq .MachineType "controlplane") (ne .Values.vipLink $defaultLinkName) }}
229+
{{- /* Suppress the inline (discovery-derived) vip when the operator
230+
has redirected it to a different link; otherwise the VIP would
231+
be pinned twice on different interfaces. */}}
232+
{{- $suppressInlineVip := and .Values.vipLink (ne .Values.vipLink $defaultLinkName) }}
233+
{{- if or $existingInterfacesConfiguration $defaultLinkName $vipOverride }}
224234
interfaces:
225235
{{- if $existingInterfacesConfiguration }}
226236
{{- $existingInterfacesConfiguration | nindent 4 }}
227-
{{- else }}
237+
{{- else if $defaultLinkName }}
228238
{{- $isVlan := include "talm.discovered.is_vlan" $defaultLinkName }}
229239
{{- $parentLinkName := "" }}
230240
{{- if $isVlan }}
@@ -246,7 +256,7 @@ link: {{ $vipLinkName }}
246256
routes:
247257
- network: 0.0.0.0/0
248258
gateway: {{ include "talm.discovered.default_gateway" . }}
249-
{{- if and .Values.floatingIP (eq .MachineType "controlplane") }}
259+
{{- if and .Values.floatingIP (eq .MachineType "controlplane") (not $suppressInlineVip) }}
250260
vip:
251261
ip: {{ .Values.floatingIP }}
252262
{{- end }}
@@ -255,12 +265,17 @@ link: {{ $vipLinkName }}
255265
routes:
256266
- network: 0.0.0.0/0
257267
gateway: {{ include "talm.discovered.default_gateway" . }}
258-
{{- if and .Values.floatingIP (eq .MachineType "controlplane") }}
268+
{{- if and .Values.floatingIP (eq .MachineType "controlplane") (not $suppressInlineVip) }}
259269
vip:
260270
ip: {{ .Values.floatingIP }}
261271
{{- end }}
262272
{{- end }}
263273
{{- end }}
274+
{{- if $vipOverride }}
275+
- interface: {{ .Values.vipLink }}
276+
vip:
277+
ip: {{ .Values.floatingIP }}
278+
{{- end }}
264279
{{- end }}
265280
{{- end }}
266281

‎pkg/engine/render_test.go‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2822,6 +2822,117 @@ func TestMultiDocCozystack_VIPLinkOverrideOnFreshNode(t *testing.T) {
28222822
}
28232823
}
28242824

2825+
// renderLegacyChart renders the controlplane template of the supplied
2826+
// chart against a "legacy" Talos config (TalosVersion=""), routing
2827+
// through talos.config.legacy. Mirrors the multidoc render helpers
2828+
// above but exercises the legacy code path that pre-1.12 Talos still
2829+
// uses by default. Returns the rendered controlplane document.
2830+
func renderLegacyChart(t *testing.T, chartDir, templateName string, lookup func(string, string, string) (map[string]any, error), overrides map[string]any) string {
2831+
t.Helper()
2832+
origLookup := helmEngine.LookupFunc
2833+
t.Cleanup(func() { helmEngine.LookupFunc = origLookup })
2834+
helmEngine.LookupFunc = lookup
2835+
2836+
chrt, err := loader.LoadDir(chartDir)
2837+
if err != nil {
2838+
t.Fatalf("load chart: %v", err)
2839+
}
2840+
values := cloneValues(chrt.Values)
2841+
if v, _ := values["endpoint"].(string); v == "" {
2842+
values["endpoint"] = testEndpoint
2843+
}
2844+
maps.Copy(values, overrides)
2845+
2846+
eng := helmEngine.Engine{}
2847+
out, err := eng.Render(chrt, chartutil.Values{
2848+
"Values": values,
2849+
"TalosVersion": "",
2850+
})
2851+
if err != nil {
2852+
t.Fatalf("render: %v", err)
2853+
}
2854+
return out[templateName]
2855+
}
2856+
2857+
// TestLegacyCozystack_VIPLinkOverride pins the legacy-schema mirror
2858+
// of TestMultiDocCozystack_VIPLinkOverride. The legacy Talos config
2859+
// shape has no Layer2VIPConfig document — VIPs live at
2860+
// machine.network.interfaces[].vip — so the override is expressed as
2861+
// a separate vip-only top-level interfaces[] entry. Without this
2862+
// fix, fresh `talm init -p cozystack` users on the default
2863+
// `talosVersion: ""` chart setting silently lose the override.
2864+
func TestLegacyCozystack_VIPLinkOverride(t *testing.T) {
2865+
result := renderLegacyChart(t, "../../charts/cozystack", "cozystack/templates/controlplane.yaml", simpleNicLookup(), map[string]any{
2866+
"floatingIP": "192.168.201.5",
2867+
"vipLink": "eth0.4000",
2868+
})
2869+
2870+
// Override entry: a top-level interfaces[] entry with the
2871+
// operator's link name and only the vip block.
2872+
assertContains(t, result, "- interface: eth0.4000")
2873+
assertContains(t, result, "ip: 192.168.201.5")
2874+
// Inline (discovery-derived) vip on the bare NIC must be
2875+
// suppressed when vipLink redirects the VIP.
2876+
assertNotContains(t, result, "interface: eth0\n addresses: [\"192.168.201.10/24\"]\n routes:\n - network: 0.0.0.0/0\n gateway: 192.168.201.1\n vip:")
2877+
// Legacy schema has no Layer2VIPConfig kind.
2878+
assertNotContains(t, result, "kind: Layer2VIPConfig")
2879+
}
2880+
2881+
// TestLegacyGeneric_VIPLinkOverride mirrors the cozystack-side legacy
2882+
// override test for the generic preset. The generic chart ships
2883+
// `talosVersion: ""` by default, so the legacy branch is the path a
2884+
// fresh `talm init -p generic` user actually takes.
2885+
func TestLegacyGeneric_VIPLinkOverride(t *testing.T) {
2886+
result := renderLegacyChart(t, "../../charts/generic", "generic/templates/controlplane.yaml", simpleNicLookup(), map[string]any{
2887+
"floatingIP": "192.168.201.5",
2888+
"vipLink": "eth0.4000",
2889+
})
2890+
2891+
assertContains(t, result, "- interface: eth0.4000")
2892+
assertContains(t, result, "ip: 192.168.201.5")
2893+
assertNotContains(t, result, "interface: eth0\n addresses: [\"192.168.201.10/24\"]\n routes:\n - network: 0.0.0.0/0\n gateway: 192.168.201.1\n vip:")
2894+
assertNotContains(t, result, "kind: Layer2VIPConfig")
2895+
}
2896+
2897+
// TestLegacyCozystack_VIPLinkOverrideOnFreshNode pins the
2898+
// chicken-and-egg case for legacy: a node where discovery returns no
2899+
// default-gateway link must still emit the override entry. Without
2900+
// this the override would silently no-op on the exact case it was
2901+
// added for — the operator wants the VIP on a VLAN sub-interface
2902+
// this same template is about to bring up.
2903+
func TestLegacyCozystack_VIPLinkOverrideOnFreshNode(t *testing.T) {
2904+
result := renderLegacyChart(t, "../../charts/cozystack", "cozystack/templates/controlplane.yaml", freshNicLookup(), map[string]any{
2905+
"floatingIP": "192.168.201.5",
2906+
"vipLink": "eth0.4000",
2907+
"advertisedSubnets": []any{"192.168.201.0/24"},
2908+
})
2909+
2910+
assertContains(t, result, "interfaces:")
2911+
assertContains(t, result, "- interface: eth0.4000")
2912+
assertContains(t, result, "ip: 192.168.201.5")
2913+
}
2914+
2915+
// TestLegacyCozystack_VIPLinkMatchesDiscovery pins the no-op case:
2916+
// when vipLink names the same link discovery already picked, the
2917+
// chart must NOT emit a duplicate interfaces[] entry — Talos legacy
2918+
// validation rejects duplicate interface names. The inline vip block
2919+
// on the discovered interface must remain, since it already pins the
2920+
// VIP on the right link.
2921+
func TestLegacyCozystack_VIPLinkMatchesDiscovery(t *testing.T) {
2922+
result := renderLegacyChart(t, "../../charts/cozystack", "cozystack/templates/controlplane.yaml", simpleNicLookup(), map[string]any{
2923+
"floatingIP": "192.168.201.5",
2924+
"vipLink": "eth0",
2925+
})
2926+
2927+
// Exactly one interface entry for eth0 — not a duplicate.
2928+
if c := strings.Count(result, "- interface: eth0"); c != 1 {
2929+
t.Errorf("expected exactly one - interface: eth0 entry, got %d:\n%s", c, result)
2930+
}
2931+
// Inline vip is preserved on the discovered entry.
2932+
assertContains(t, result, "vip:")
2933+
assertContains(t, result, "ip: 192.168.201.5")
2934+
}
2935+
28252936
// TestMergeFileAsPatch_TypedDocPartialEditPreservesIdentityKeys pins
28262937
// the regression that the multi-doc identity prune introduced: a
28272938
// typed multi-doc body where the user changes one field but keeps

0 commit comments

Comments
 (0)