Fix: enable DHCP feature using SetBuilder by adamtrizuljak-sap · Pull Request #646 · ironcore-dev/network-operator

adamtrizuljak-sap · 2026-10-16T10:30:11Z

What

Enable DHCP feature using SetBuilder, ensuring it is always enabled before relayServerIdOverrideDisable is enabled
Move API definition System.RelayServerIDOverrideDisable into DHCP sub-struct
Move provider field DHCPRelayServerIDOverrideDisable definition from DHCPRelay into System

Reason

EnsureSystemSettings enabled the dhcp feature with a merge-mode
client.Patch and wrote the server-id-override-disable value in a
separate Patch. On a device where the dhcp feature was off, the
override write was rejected with "DHCP feature is not enabled".

Build the settings into a single SetBuilder and apply them through
Provider.Do, matching EnsureDHCPRelay. Do splits the feature
activation into its own Set RPC executed before the dependent write,
so a device with the feature disabled succeeds on the first
reconcile. Idempotency is preserved: subsequent reconciles produce
no Set.

Testing

I reproduced the state where the error occurred using the Cisco N9K-C9300v image in Containerlab - DHCP feature is enabled, relayInformationOptionServerIdOverrideDisable is not enabled

gnmic -a 127.0.0.1 --port 9339 -u *** -p *** --skip-verify get -e json --values-only --path 'System/fm-items/dhcp-items' --path 'System/dhcp-items/inst-items/relayInformationOptionServerIdOverrideDisableEnabled'
[
  {
    "adminSt": "enabled",
    "maxInstance": 1,
    "operSt": "enabled"
  },
  0
]

Deploying the System sample with spec.dhcp.relayServerIdOverrideDisable: true now reconciles without problems. DHCP feature stays enabled and relayInformationOptionServerIdOverrideDisable is correctly set to 1.

It also works if DHCP feature and spec.dhcp.relayServerIdOverrideDisable are both disabled:

# Deploy System sample with ``spec.dhcp.relayServerIdOverrideDisable: false` and manually disable DHCP feature
% gnmic -a 127.0.0.1 --port 9339 -u *** -p *** --skip-verify set --update-path "System/fm-items/dhcp-items"  --update-value '{"adminSt":"disabled"}'
# ...

% gnmic -a 127.0.0.1 --port 9339 -u *** -p *** --skip-verify get -e json --values-only --path 'System/fm-items/dhcp-items' --path 'System/dhcp-items/inst-items/relayInformationOptionServerIdOverrideDisableEnabled'
[
  {
    "adminSt": "disabled",
    "maxInstance": 1,
    "operSt": "disabled"
  },
  null
]

# Deploy System sample with `spec.dhcp.relayServerIdOverrideDisable: true` -> DHCP gets enabled
% gnmic -a 127.0.0.1 --port 9339 -u *** -p *** --skip-verify get -e json --values-only --path 'System/fm-items/dhcp-items' --path 'System/dhcp-items/inst-items/relayInformationOptionServerIdOverrideDisableEnabled'
[
  {
    "adminSt": "enabled",
    "maxInstance": 1,
    "operSt": "enabled"
  },
  1
]

Signed-off-by: Adam Trizuljak <adam.trizuljak@sap.com>

EnsureSystemSettings enabled the dhcp feature with a merge-mode client.Patch and wrote the server-id-override-disable value in a separate Patch. On a device where the dhcp feature was off, the override write was rejected with "DHCP feature is not enabled". Build the settings into a single SetBuilder and apply them through Provider.Do, matching EnsureDHCPRelay. Do splits the feature activation into its own Set RPC executed before the dependent write, so a device with the feature disabled succeeds on the first reconcile. Idempotency is preserved: subsequent reconciles produce no Set. Signed-off-by: Adam Trizuljak <adam.trizuljak@sap.com>

This will support an extend - migrate - contract migration strategy We perform logical OR between the old and new field to preserve intent from any old version of the System resource Fix GNMI test files Signed-off-by: Adam Trizuljak <adam.trizuljak@sap.com>

EnsureSystemSettings patched relayInformationOptionServerIdOverride DisableEnabled on every reconcile, even as 0 when the dhcp feature was off. That leaf lives under the dhcp feature tree, so writing it on a feature-disabled device is rejected with "DHCP feature is not enabled". The fake gNMI test server does not enforce the feature gate, so system.txtar passed while asserting that bad write. Read the current feature state and realize the leaf per case: - override requested: set 1 and enable the feature if needed - override off but feature enabled (e.g. by DHCPRelay, or a true->false flip): patch 0, leaving the feature untouched - override off and feature disabled: write nothing This also fixes the true->false flip, which previously left the leaf stuck at 1. Update system.txtar to expect no dhcp-items in the base case. Signed-off-by: Adam Trizuljak <adam.trizuljak@sap.com>

adamtrizuljak-sap · 2026-10-19T13:48:58Z

Updated behavior according to @felix-kaestner 's review comments.

Clean state: Deployed the System sample with spec.dhcp.relayServerIdOverrideDisable commented out. The resource gets created with the default value false. The DHCP feature stays disabled, both values are missing from the YANG tree

% kubectl get system leaf1 -oyaml
apiVersion: nx.cisco.networking.metal.ironcore.dev/v1alpha1
kind: System
# ...
spec:
  deviceRef:
    name: leaf1
  dhcp:
    relayServerIdOverrideDisable: false
  # ...

% gnmic -a 127.0.0.1 --port 9339 -u *** -p *** --skip-verify get -e json --values-only --path 'System/fm-items/dhcp-items' --path 'System/dhcp-items/inst-items/relayInformationOptionServerIdOverrideDisableEnabled'
[
  null,
  null
]

Deploying the System sample with spec.dhcp.relayServerIdOverrideDisable: true enables the DHCP feature and relayInformationOptionServerIdOverrideDisableEnabled becomes 1

% gnmic -a 127.0.0.1 --port 9339 -u *** -p *** --skip-verify get -e json --values-only --path 'System/fm-items/dhcp-items' --path 'System/dhcp-items/inst-items/relayInformationOptionServerIdOverrideDisableEnabled'
[
  {
    "adminSt": "enabled",
    "maxInstance": 1,
    "operSt": "enabled"
  },
  1
]

Deploying the System sample with spec.dhcp.relayServerIdOverrideDisable: false sets relayInformationOptionServerIdOverrideDisableEnabled to 0, DHCP stays enabled

% gnmic -a 127.0.0.1 --port 9339 -u *** -p *** --skip-verify get -e json --values-only --path 'System/fm-items/dhcp-items' --path 'System/dhcp-items/inst-items/relayInformationOptionServerIdOverrideDisableEnabled'
[
  {
    "adminSt": "enabled",
    "maxInstance": 1,
    "operSt": "enabled"
  },
  0
]

After manually disabling the DHCP feature, the System sample can be deployed with spec.dhcp.relayServerIdOverrideDisable: true and the DHCP feature gets enabled.

% gnmic -a 127.0.0.1 --port 9339 -u *** -p *** --skip-verify set --update-path "System/fm-items/dhcp-items"  --update-value '{"adminSt":"disabled"}'
# ...

% gnmic -a 127.0.0.1 --port 9339 -u *** -p *** --skip-verify get -e json --values-only --path 'System/fm-items/dhcp-items' --path 'System/dhcp-items/inst-items/relayInformationOptionServerIdOverrideDisableEnabled'
[
  {
    "adminSt": "disabled",
    "maxInstance": 1,
    "operSt": "disabled"
  },
  null
]

# After deploying the `System` sample with `spec.dhcp.relayServerIdOverrideDisable: true`:
% gnmic -a 127.0.0.1 --port 9339 -u *** -p *** --skip-verify get -e json --values-only --path 'System/fm-items/dhcp-items' --path 'System/dhcp-items/inst-items/relayInformationOptionServerIdOverrideDisableEnabled'
[
  {
    "adminSt": "enabled",
    "maxInstance": 1,
    "operSt": "enabled"
  },
  1
]

github-actions · 2026-10-19T13:55:01Z

Merging this branch changes the coverage (4 decrease, 2 increase)

Impacted Packages	Coverage Δ	🤖
github.com/ironcore-dev/network-operator/api/cisco/nx/v1alpha1	0.00% (ø)
github.com/ironcore-dev/network-operator/internal/controller/cisco/nx	64.62% (-0.53%)	👎
github.com/ironcore-dev/network-operator/internal/controller/core	60.55% (-0.06%)	👎
github.com/ironcore-dev/network-operator/internal/controller/evpn	80.15% (-0.45%)	👎
github.com/ironcore-dev/network-operator/internal/controller/overlay	66.36% (+6.36%)	👍
github.com/ironcore-dev/network-operator/internal/controller/pool	75.53% (+1.48%)	👍
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos	17.45% (-0.03%)	👎

Coverage details

_{Compared to commit 3ec995a (run #39222396545)}

Changed File	Coverage Δ	Total	Covered	Missed	🤖
github.com/ironcore-dev/network-operator/api/cisco/nx/v1alpha1/system_types.go	0.00% (ø)	5	0	5
github.com/ironcore-dev/network-operator/api/cisco/nx/v1alpha1/zz_generated.deepcopy.go	0.00% (ø)	691 (+7)	0	691 (+7)
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/dhcprelay.go	0.00% (-16.67%)	4 (-2)	0 (-1)	4 (-1)	💀
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/provider.go	4.34% (-0.01%)	2511 (+8)	109	2402 (+8)	👎
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/system.go	6.25% (+3.02%)	32 (+1)	2 (+1)	30	👍

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files:

github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/dhcprelay_test.go
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/system_test.go

github-actions Bot added the size/L label Oct 16, 2026

felix-kaestner reviewed Oct 16, 2026

View reviewed changes

Comment thread api/cisco/nx/v1alpha1/system_types.go Resolved

Comment thread internal/provider/cisco/nxos/system.go Resolved

Comment thread internal/provider/cisco/nxos/provider.go Resolved

hardikdr added the area/switch-automation Automation processes for network switch management and operations. label Oct 17, 2026

hardikdr added this to Roadmap Oct 17, 2026

adamtrizuljak-sap added 5 commits October 19, 2026 12:57

Move DHCPRelayServerIDOverrideDisable definition into System

f43192c

Signed-off-by: Adam Trizuljak <adam.trizuljak@sap.com>

Move RelayServerIDOverrideDisable into DHCP sub-struct

64cd243

Signed-off-by: Adam Trizuljak <adam.trizuljak@sap.com>

adamtrizuljak-sap force-pushed the fix/dhcp-feature-setbuilder branch from 631b4cf to f2bdab6 Compare October 19, 2026 13:48

felix-kaestner approved these changes Oct 19, 2026

View reviewed changes

adamtrizuljak-sap merged commit a56059b into main Oct 19, 2026
26 checks passed

adamtrizuljak-sap deleted the fix/dhcp-feature-setbuilder branch October 19, 2026 14:26

github-project-automation Bot moved this to Done in Roadmap Oct 19, 2026

Navigation Menu

Repository navigation

Fix: enable DHCP feature using SetBuilder - #646

Fix: enable DHCP feature using SetBuilder#646
adamtrizuljak-sap merged 5 commits into
mainfrom
fix/dhcp-feature-setbuilder

adamtrizuljak-sap commented Oct 16, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adamtrizuljak-sap commented Oct 19, 2026

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

adamtrizuljak-sap commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Reason

Testing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adamtrizuljak-sap commented Oct 19, 2026

Uh oh!

github-actions Bot commented Oct 19, 2026

Merging this branch changes the coverage (4 decrease, 2 increase)

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adamtrizuljak-sap commented Oct 16, 2026 •

edited

Loading