fix(nico-dhcp): pin DHCP server identifier (option 54) to the stable VIP

helm-prereqs/values/nico-core.yaml

@@ -377,7 +377,12 @@ nico-dhcp:
   externalService:
     enabled: true    # must be explicitly enabled - defaults to false in chart
     annotations:
-      metallb.universe.tf/loadBalancerIPs: ""  # REQUIRED: DHCP VIP (internal pool)
+      ## REQUIRED: DHCP VIP (internal pool). Besides pinning the LoadBalancer
+      ## IP, this VIP is also auto-advertised to DHCP clients as the server
+      ## identifier (option 54) so BMC unicast renewals survive nico-dhcp pod
+      ## replacement (issue #3663). Override the advertised identifier
+      ## independently via nico-dhcp.config.kea.serverIdentifier if needed.
+      metallb.universe.tf/loadBalancerIPs: ""
 
   ## DHCP hook libraries' advertised IPs.
   ##

helm/charts/nico-dhcp/templates/_helpers.tpl

@@ -146,6 +146,41 @@ controls the YAML→JSON name mapping.
 {{- if hasKey $k "declineProbationPeriod" -}}
 {{- $declineProbationPeriod = $k.declineProbationPeriod -}}
 {{- end -}}
+{{/*
+  DHCP server identifier (option 54) — pin to the stable DHCP VIP so client
+  unicast renewals survive pod replacement (issue #3663). Without this, kea
+  advertises the ephemeral pod IP: a rescheduled pod leaves every leased BMC
+  unicast-renewing against a dead address until the rebind timer forces a
+  broadcast. Resolution order:
+    1. config.kea.serverIdentifier — explicit override, validated as IPv4
+       (render fails on a malformed explicit value).
+    2. externalService.annotations["metallb.universe.tf/loadBalancerIPs"] —
+       the VIP the chart already advertises; first entry when comma-separated.
+       Used only when it parses as a single IPv4 — a missing/odd annotation
+       silently falls through so upgrades from values that predate this
+       field can never fail at render time.
+    3. Neither — the option is omitted entirely and kea falls back to its
+       default (pod IP), which is byte-identical to the pre-#3663 rendering.
+*/}}
+{{- $ipv4Pattern := "^((25[0-5]|2[0-4][0-9]|1[0-9]{2}|[1-9]?[0-9])\\.){3}(25[0-5]|2[0-4][0-9]|1[0-9]{2}|[1-9]?[0-9])$" -}}
+{{/* printf "%v" coerces non-string overrides (bare numbers, maps) to a string
+     so they reach the friendly IPv4-validation fail below instead of erroring
+     inside sprig's trim with an opaque type message. */}}
+{{- $serverIdentifier := trim (printf "%v" (default "" $k.serverIdentifier)) -}}
+{{- if and $serverIdentifier (not (regexMatch $ipv4Pattern $serverIdentifier)) -}}
+{{- fail (printf "nico-dhcp: config.kea.serverIdentifier must be a single IPv4 address (the stable DHCP VIP); got %q" $serverIdentifier) -}}
+{{- end -}}
+{{- if not $serverIdentifier -}}
+{{- $es := default (dict) .Values.externalService -}}
+{{- if $es.enabled -}}
+{{- $esAnnotations := default (dict) $es.annotations -}}
+{{- $vipAnnotation := index $esAnnotations "metallb.universe.tf/loadBalancerIPs" -}}
+{{- $firstVip := trim (first (splitList "," (printf "%v" (default "" $vipAnnotation)))) -}}
+{{- if regexMatch $ipv4Pattern $firstVip -}}
+{{- $serverIdentifier = $firstVip -}}
+{{- end -}}
+{{- end -}}
+{{- end -}}
 {
   "Dhcp4": {
     "interfaces-config": {
@@ -162,6 +197,14 @@ controls the YAML→JSON name mapping.
     "rebind-timer": {{ default 1800 $k.rebindTimer | toJson }},
     "valid-lifetime": {{ default 3600 $k.validLifetime | toJson }},
     "decline-probation-period": {{ $declineProbationPeriod | toJson }},
+{{- if $serverIdentifier }}
+    "option-data": [
+      {
+        "name": "dhcp-server-identifier",
+        "data": {{ $serverIdentifier | quote }}
+      }
+    ],
+{{- end }}
     {{/*
       Hook parameters — write both nico-* and carbide-* keys with identical
       values so the kea hook library (crates/dhcp/src/kea/loader.cc still

helm/charts/nico-dhcp/templates/configmap.yaml

@@ -16,6 +16,14 @@
 {{- if and (hasKey .Values.config "keaConfigJsonRaw") .Values.config.keaConfigJsonRaw (not (kindIs "string" .Values.config.keaConfigJsonRaw)) }}
 {{- fail (printf "nico-dhcp: `config.keaConfigJsonRaw` must be a string (got %s). Set to a complete kea_config.json blob, or leave empty/omit to use the structured `config.kea` block." (kindOf .Values.config.keaConfigJsonRaw)) }}
 {{- end }}
+{{- /* Conflict guard: the raw escape hatch skips the structured `config.kea`
+       block entirely, including the server-identifier pinning (issue #3663).
+       An explicitly set `config.kea.serverIdentifier` alongside a raw blob
+       would be silently ignored — fail loudly so the operator either drops
+       the structured override or adds the option-data to the raw JSON. */ -}}
+{{- if and (trim (default "" .Values.config.keaConfigJsonRaw)) (trim (printf "%v" (dig "kea" "serverIdentifier" "" .Values.config))) }}
+{{- fail "nico-dhcp: `config.kea.serverIdentifier` is ignored when `config.keaConfigJsonRaw` is set. Remove the structured override, or add the dhcp-server-identifier option-data to the raw kea_config.json blob (see issue #3663)." }}
+{{- end }}
 apiVersion: v1
 kind: ConfigMap
 metadata:

helm/charts/nico-dhcp/tests/server_identifier_test.yaml

@@ -0,0 +1,131 @@
+suite: DHCP server identifier (option 54) pinning — issue #3663
+templates:
+  - configmap.yaml
+release:
+  namespace: nico-system
+tests:
+  - it: omits option-data entirely when no VIP source is configured (pre-#3663 rendering)
+    asserts:
+      - notMatchRegex:
+          path: data["kea_config.json"]
+          pattern: 'dhcp-server-identifier'
+      - notMatchRegex:
+          path: data["kea_config.json"]
+          pattern: '"option-data"'
+
+  - it: derives the server identifier from the externalService loadBalancerIPs annotation
+    set:
+      externalService:
+        enabled: true
+        annotations:
+          metallb.universe.tf/loadBalancerIPs: "10.180.126.161"
+    asserts:
+      - matchRegex:
+          path: data["kea_config.json"]
+          pattern: '"name": "dhcp-server-identifier"'
+      - matchRegex:
+          path: data["kea_config.json"]
+          pattern: '"data": "10.180.126.161"'
+
+  - it: uses the first entry of a comma-separated loadBalancerIPs annotation
+    set:
+      externalService:
+        enabled: true
+        annotations:
+          metallb.universe.tf/loadBalancerIPs: "10.180.126.161, 10.180.126.162"
+    asserts:
+      - matchRegex:
+          path: data["kea_config.json"]
+          pattern: '"data": "10.180.126.161"'
+
+  - it: does not derive from the annotation when externalService is disabled
+    set:
+      externalService:
+        enabled: false
+        annotations:
+          metallb.universe.tf/loadBalancerIPs: "10.180.126.161"
+    asserts:
+      - notMatchRegex:
+          path: data["kea_config.json"]
+          pattern: 'dhcp-server-identifier'
+
+  - it: silently omits the option when the annotation is not a valid IPv4 (upgrade-safe)
+    set:
+      externalService:
+        enabled: true
+        annotations:
+          metallb.universe.tf/loadBalancerIPs: "not-an-ip"
+    asserts:
+      - notMatchRegex:
+          path: data["kea_config.json"]
+          pattern: 'dhcp-server-identifier'
+
+  - it: silently omits the option when the annotation is empty (upgrade-safe)
+    set:
+      externalService:
+        enabled: true
+        annotations:
+          metallb.universe.tf/loadBalancerIPs: ""
+    asserts:
+      - notMatchRegex:
+          path: data["kea_config.json"]
+          pattern: 'dhcp-server-identifier'
+
+  - it: explicit serverIdentifier takes precedence over the annotation
+    set:
+      config.kea.serverIdentifier: "10.180.126.170"
+      externalService:
+        enabled: true
+        annotations:
+          metallb.universe.tf/loadBalancerIPs: "10.180.126.161"
+    asserts:
+      - matchRegex:
+          path: data["kea_config.json"]
+          pattern: '"data": "10.180.126.170"'
+      - notMatchRegex:
+          path: data["kea_config.json"]
+          pattern: '10\.180\.126\.161'
+
+  - it: rejects an explicit serverIdentifier that is not a single IPv4 address
+    set:
+      config.kea.serverIdentifier: "10.180.126.161,10.180.126.162"
+    asserts:
+      - failedTemplate:
+          errorPattern: 'config\.kea\.serverIdentifier must be a single IPv4 address'
+
+  - it: rejects an explicit serverIdentifier with an out-of-range octet
+    set:
+      config.kea.serverIdentifier: "10.180.126.256"
+    asserts:
+      - failedTemplate:
+          errorPattern: 'config\.kea\.serverIdentifier must be a single IPv4 address'
+
+  - it: rejects a non-string serverIdentifier with the friendly IPv4 message
+    set:
+      config.kea.serverIdentifier: 10
+    asserts:
+      - failedTemplate:
+          errorPattern: 'config\.kea\.serverIdentifier must be a single IPv4 address'
+
+  - it: rejects serverIdentifier combined with the raw config escape hatch
+    set:
+      config.kea.serverIdentifier: "10.180.126.170"
+      config.keaConfigJsonRaw: '{"Dhcp4": {}}'
+    asserts:
+      - failedTemplate:
+          errorPattern: 'serverIdentifier` is ignored when `config\.keaConfigJsonRaw` is set'
+
+  - it: raw config escape hatch alone renders without the pinning applied
+    set:
+      config.keaConfigJsonRaw: '{"Dhcp4": {}}'
+      externalService:
+        enabled: true
+        annotations:
+          metallb.universe.tf/loadBalancerIPs: "10.180.126.161"
+    asserts:
+      - notMatchRegex:
+          path: data["kea_config.json"]
+          pattern: 'dhcp-server-identifier'
+      - matchRegex:
+          path: data["kea_config.json"]
+          pattern: 'Dhcp4'

helm/charts/nico-dhcp/values.yaml

@@ -99,6 +99,13 @@ config:
   ## Note: placeholder substitution applies ONLY to this raw string, NOT
   ## to values inside `config.kea` below — set those values to fully
   ## resolved strings (or leave empty to use the helper's derivation).
+  ##
+  ## ⚠ The server-identifier pinning (issue #3663, `config.kea.serverIdentifier`
+  ## below) does NOT apply to a raw blob. Include the equivalent option-data
+  ## in the raw JSON yourself, or unicast renewals will target the ephemeral
+  ## pod IP and orphan clients on pod replacement. Setting
+  ## `config.kea.serverIdentifier` together with a raw blob fails at render
+  ## time to make this conflict loud.
   keaConfigJsonRaw: ""
 
   ## Structured kea-dhcp4 configuration. The configmap template assembles
@@ -129,6 +136,24 @@ config:
     validLifetime: 3600
     declineProbationPeriod: 900
 
+    ## DHCP server identifier (option 54) advertised to clients — issue #3663.
+    ##
+    ## Kea defaults option 54 to the POD IP, which is unreachable from BMCs.
+    ## After a pod replacement, clients unicast-renew against the dead pod IP
+    ## and stay orphaned until the rebind timer (rebindTimer above) forces a
+    ## broadcast. Pinning option 54 to the stable DHCP VIP makes unicast
+    ## renewals survive pod churn.
+    ##
+    ## Leave empty (default) to AUTO-DERIVE from the first entry of the
+    ## externalService `metallb.universe.tf/loadBalancerIPs` annotation —
+    ## sites that already pin the DHCP VIP there get the fix with no values
+    ## change. Set explicitly only when the advertised identifier must differ
+    ## from that annotation (e.g. externalService managed outside this chart).
+    ## An explicit value must be a single IPv4 address (render fails
+    ## otherwise); when neither source yields a valid IPv4 the option is
+    ## omitted and kea falls back to the pod IP (pre-#3663 behavior).
+    serverIdentifier: ""
+
     ## libdhcp hook parameters — these become the JSON object at
     ## Dhcp4.hooks-libraries[0].parameters and are read by the
     ## NICo-specific kea hook (libdhcp.so).