Skip to content

Diff list entries individually on Patch to remove stale config - #565

Open
felix-kaestner wants to merge 3 commits into
mainfrom
patch-lists
Open

felix-kaestner wants to merge 3 commits into
mainfrom
patch-lists

Conversation

@felix-kaestner

Copy link
Copy Markdown
Contributor

When the provider uses Patch (gNMI update) on a struct containing list fields, the device merges entries into existing config but never removes entries that are no longer desired. Stale list entries survive across reconciliations.

Fix this by removing list fields from parent structs and managing each list entry as a standalone DataElement. The provider now fetches current entries from the device, patches each desired entry individually, and deletes any stale entries not present in the desired set.

When the provider uses Patch (gNMI update) on a struct containing list
fields, the device merges entries into existing config but never removes
entries that are no longer desired. Stale list entries survive across
reconciliations.

Fix this by removing list fields from parent structs and managing each
list entry as a standalone DataElement. The provider now fetches current
entries from the device, patches each desired entry individually, and
deletes any stale entries not present in the desired set.

Signed-off-by: Felix Kästner <felix.kaestner@sap.com>
Signed-off-by: Felix Kästner <felix.kaestner@sap.com>
Some NX-OS platforms return keepalive source and destination addresses
with a /32 prefix (e.g. "10.1.1.1/32") while others return them as
plain addresses (e.g. "10.1.1.1"). This inconsistency causes a false
diff on every reconcile, triggering unnecessary gNMI Set requests.

Introduce a Prefix type that normalizes both formats: UnmarshalText
infers /32 for IPv4 (or /128 for IPv6) when the mask is absent, and
MarshalText always includes it. Use this type for the VPC keepalive
DestIP and SrcIP fields so the provider is idempotent regardless of
platform behavior.

Signed-off-by: Felix Kästner <felix.kaestner@sap.com>
@github-actions

Copy link
Copy Markdown

Merging this branch will decrease overall coverage

Impacted Packages Coverage Δ 🤖
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos 7.02% (-0.12%) 👎

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/bgp.go 23.88% (-0.73%) 67 (+2) 16 51 (+2) 👎
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/intf.go 30.52% (-1.89%) 154 (+9) 47 107 (+9) 👎
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/lldp.go 50.00% (ø) 6 (+2) 3 (+1) 3 (+1)
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/nve.go 69.23% (-3.50%) 13 (+2) 9 (+1) 4 (+1) 👎
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/provider.go 0.24% (-0.00%) 3365 (+59) 8 3357 (+59) 👎
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/user.go 33.33% (-3.03%) 45 (+1) 15 (-1) 30 (+2) 👎
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/vpc.go 8.70% (-3.30%) 46 (+21) 4 (+1) 42 (+20) 👎

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/bgp_test.go
  • github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/intf_test.go
  • github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/lldp_test.go
  • github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/nve_test.go
  • github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/user_test.go
  • github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/vpc_test.go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant