Skip to content

Allow finalization without provider config - #584

Merged
nikatza merged 2 commits into
mainfrom
fix-missing-providerconfig
Sep 22, 2026
Merged

Allow finalization without provider config#584
nikatza merged 2 commits into
mainfrom
fix-missing-providerconfig

Conversation

@nikatza

@nikatza nikatza commented Sep 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

More info in commit message.

@nikatza
nikatza force-pushed the fix-missing-providerconfig branch 2 times, most recently from 96183ff to d23f20d Compare September 18, 2026 15:42
@hardikdr hardikdr added the area/switch-automation Automation processes for network switch management and operations. label Sep 19, 2026
@hardikdr hardikdr added this to Roadmap Sep 19, 2026
@nikatza
nikatza marked this pull request as ready for review September 21, 2026 06:58
@nikatza
nikatza force-pushed the fix-missing-providerconfig branch from d23f20d to 9e0689c Compare September 21, 2026 09:13
@felix-kaestner

felix-kaestner commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Provider-specific configuration affecting device model paths outside the main resource may remain on the device because that configuration is unavailable during cleanup.

Ideally, the clean up of configuration on the device, done in the finalize of the reconcilers, will make sure to clean all paths eventually written, regardless of previous desired config. So as long as the ProviderConfig doesn't include any identifying criteria of a YANG path, I think we should be good to just always delete/cleanup the yang tree and as such don't even need to pass the ProviderConfig at all to the Delete... methods of the provider implementation.

@nikatza
nikatza force-pushed the fix-missing-providerconfig branch from 9e0689c to 42910a2 Compare September 21, 2026 12:58
@nikatza
nikatza marked this pull request as draft September 21, 2026 13:02
@nikatza
nikatza force-pushed the fix-missing-providerconfig branch 2 times, most recently from a5fa644 to e094166 Compare September 21, 2026 14:39
@nikatza

nikatza commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Provider-specific configuration affecting device model paths outside the main resource may remain on the device because that configuration is unavailable during cleanup.

Ideally, the clean up of configuration on the device, done in the finalize of the reconcilers, will make sure to clean all paths eventually written, regardless of previous desired config. So as long as the ProviderConfig doesn't include any identifying criteria of a YANG path, I think we should be good to just always delete/cleanup the yang tree and as such don't even need to pass the ProviderConfig at all to the Delete... methods of the provider implementation.

I seems this is actually possible with the models we have. I have updated this PR. Thanks for looking at this.

@nikatza
nikatza force-pushed the fix-missing-providerconfig branch from e094166 to 5216052 Compare September 21, 2026 15:43
@nikatza
nikatza marked this pull request as ready for review September 21, 2026 15:43
felix-kaestner
felix-kaestner reviewed Sep 22, 2026
View reviewed changes
Comment thread internal/provider/cisco/nxos/intf_test.go Outdated
Comment thread internal/provider/cisco/nxos/lldp.go Outdated
Comment thread internal/provider/cisco/nxos/lldp_test.go Outdated
Comment thread internal/provider/cisco/nxos/term_test.go
@nikatza
nikatza force-pushed the fix-missing-providerconfig branch 2 times, most recently from 79c8ce5 to 5628284 Compare September 22, 2026 14:20
@nikatza
Decouple finalization from provider configuration
571e844 
Finalize resources before resolving provider-specific configuration so
deletion can complete when the referenced configuration no longer
exists.

Remove provider configuration from delete requests and pass only the
resource identity and context required for cleanup. Providers now remove
all owned device paths independently of the previous desired state.

Cover paths outside primary resource trees, including spanning-tree
settings for aggregate interfaces, ManagementAccess VTY access classes,
and LLDP configuration before disabling the feature.

Add regression tests for missing provider configuration and the affected
NX-OS cleanup paths.

Signed-off-by: Pujol <enric.pujol@sap.com>
@nikatza
Update gNMI tests
139faba 
Signed-off-by: Pujol <enric.pujol@sap.com>
@nikatza
nikatza force-pushed the fix-missing-providerconfig branch from 5628284 to 139faba Compare September 22, 2026 14:24
@github-actions

github-actions Bot commented Sep 22, 2026

Copy link
Copy Markdown

Merging this branch changes the coverage (1 decrease, 3 increase)

Impacted Packages Coverage Δ 🤖
github.com/ironcore-dev/network-operator/hack/provider 0.00% (ø)
github.com/ironcore-dev/network-operator/internal/controller/cisco/nx 68.52% (-0.30%) 👎
github.com/ironcore-dev/network-operator/internal/controller/core 62.92% (+0.18%) 👍
github.com/ironcore-dev/network-operator/internal/controller/evpn 85.68% (+0.95%) 👍
github.com/ironcore-dev/network-operator/internal/provider 0.00% (ø)
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr 37.06% (ø)
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos 11.17% (+1.19%) 👍
github.com/ironcore-dev/network-operator/internal/provider/openconfig 8.61% (ø)
Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/ironcore-dev/network-operator/hack/provider/main.go 0.00% (ø) 710 (-11) 0 710 (-11)
github.com/ironcore-dev/network-operator/internal/controller/core/aaa_controller.go 0.00% (ø) 167 (-1) 0 167 (-1)
github.com/ironcore-dev/network-operator/internal/controller/core/acl_controller.go 60.56% (-0.22%) 180 (-1) 109 (-1) 71 👎
github.com/ironcore-dev/network-operator/internal/controller/core/banner_controller.go 61.21% (-0.18%) 214 (-1) 131 (-1) 83 👎
github.com/ironcore-dev/network-operator/internal/controller/core/bgp_controller.go 71.23% (+0.60%) 285 (-1) 203 (+1) 82 (-2) 👍
github.com/ironcore-dev/network-operator/internal/controller/core/bgp_peer_controller.go 60.11% (-0.11%) 376 (-1) 226 (-1) 150 👎
github.com/ironcore-dev/network-operator/internal/controller/core/certificate_controller.go 62.94% (-0.19%) 197 (-1) 124 (-1) 73 👎
github.com/ironcore-dev/network-operator/internal/controller/core/configbackup_controller.go 60.27% (-0.11%) 370 (-1) 223 (-1) 147 👎
github.com/ironcore-dev/network-operator/internal/controller/core/dhcprelay_controller.go 71.73% (+0.61%) 329 236 (+2) 93 (-2) 👍
github.com/ironcore-dev/network-operator/internal/controller/core/dns_controller.go 63.28% (-0.21%) 177 (-1) 112 (-1) 65 👎
github.com/ironcore-dev/network-operator/internal/controller/core/ethernetsegment_controller.go 67.97% (-0.12%) 256 (-1) 174 (-1) 82 👎
github.com/ironcore-dev/network-operator/internal/controller/core/evpninstance_controller.go 63.55% (-0.12%) 310 (-1) 197 (-1) 113 👎
github.com/ironcore-dev/network-operator/internal/controller/core/interface_controller.go 73.93% (+0.99%) 679 (-1) 502 (+6) 177 (-7) 👍
github.com/ironcore-dev/network-operator/internal/controller/core/isis_controller.go 62.33% (-0.17%) 215 (-1) 134 (-1) 81 👎
github.com/ironcore-dev/network-operator/internal/controller/core/lldp_controller.go 69.11% (ø) 259 179 80
github.com/ironcore-dev/network-operator/internal/controller/core/managementaccess_controller.go 60.45% (-0.22%) 177 (-1) 107 (-1) 70 👎
github.com/ironcore-dev/network-operator/internal/controller/core/ntp_controller.go 60.45% (-0.22%) 177 (-1) 107 (-1) 70 👎
github.com/ironcore-dev/network-operator/internal/controller/core/nve_controller.go 68.46% (-0.12%) 260 (-1) 178 (-1) 82 👎
github.com/ironcore-dev/network-operator/internal/controller/core/ospf_controller.go 66.27% (+1.43%) 255 (-1) 169 (+3) 86 (-4) 👍
github.com/ironcore-dev/network-operator/internal/controller/core/pim_controller.go 61.97% (-0.18%) 213 (-1) 132 (-1) 81 👎
github.com/ironcore-dev/network-operator/internal/controller/core/prefixset_controller.go 62.78% (-0.21%) 180 (-1) 113 (-1) 67 👎
github.com/ironcore-dev/network-operator/internal/controller/core/probe_controller.go 53.45% (-0.11%) 406 (-1) 217 (-1) 189 👎
github.com/ironcore-dev/network-operator/internal/controller/core/routingpolicy_controller.go 72.27% (+0.72%) 238 (-1) 172 (+1) 66 (-2) 👍
github.com/ironcore-dev/network-operator/internal/controller/core/snmp_controller.go 60.45% (-0.22%) 177 (-1) 107 (-1) 70 👎
github.com/ironcore-dev/network-operator/internal/controller/core/syslog_controller.go 60.56% (-0.22%) 180 (-1) 109 (-1) 71 👎
github.com/ironcore-dev/network-operator/internal/controller/core/user_controller.go 59.72% (-0.19%) 211 (-1) 126 (-1) 85 👎
github.com/ironcore-dev/network-operator/internal/controller/core/vlan_controller.go 64.36% (+0.87%) 188 (-1) 121 (+1) 67 (-2) 👍
github.com/ironcore-dev/network-operator/internal/controller/core/vrf_controller.go 63.84% (-0.20%) 177 (-1) 113 (-1) 64 👎
github.com/ironcore-dev/network-operator/internal/provider/cisco/iosxr/provider.go 22.26% (ø) 337 75 262
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/lldp.go 66.67% (+16.67%) 6 (+2) 4 (+2) 2 🎉
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/provider.go 2.15% (+1.29%) 3349 (-6) 72 (+43) 3277 (-49) 👍
github.com/ironcore-dev/network-operator/internal/provider/openconfig/acl.go 62.50% (ø) 40 25 15
github.com/ironcore-dev/network-operator/internal/provider/openconfig/interface.go 9.60% (ø) 177 17 160
github.com/ironcore-dev/network-operator/internal/provider/openconfig/lldp.go 0.00% (ø) 24 0 24
github.com/ironcore-dev/network-operator/internal/provider/openconfig/prefixset.go 0.00% (ø) 18 0 18
github.com/ironcore-dev/network-operator/internal/provider/openconfig/vrf.go 0.00% (ø) 19 0 19
github.com/ironcore-dev/network-operator/internal/provider/provider.go 0.00% (ø) 54 0 54

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/controller/cisco/nx/suite_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/interface_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/ospf_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/suite_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/evpn/suite_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/term_test.go

@nikatza
nikatza merged commit 4923cdd into main Sep 22, 2026
26 checks passed
@nikatza
nikatza deleted the fix-missing-providerconfig branch September 22, 2026 14:40
@github-project-automation github-project-automation Bot moved this to Done in Roadmap Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felix-kaestner felix-kaestner felix-kaestner approved these changes

Assignees

No one assigned

Labels

area/switch-automation Automation processes for network switch management and operations. size/XL

Projects

Roadmap
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nikatza @felix-kaestner @hardikdr