Skip to content

Isolate per-device provider state in controller tests - #558

Open
felix-kaestner wants to merge 3 commits into
mainfrom
test-suites
Open

Isolate per-device provider state in controller tests#558
felix-kaestner wants to merge 3 commits into
mainfrom
test-suites

Conversation

@felix-kaestner

@felix-kaestner felix-kaestner commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

The test provider was a singleton shared by all controllers. Every test creates a distinct Device via GenerateName, but the provider stored all state (DNS, BGP, NTP, LLDP, etc.) in flat fields on a single struct. This caused state bleed between test suites and intermittent failures.

Refactor the test provider into three parts:

  • DeviceState: per-device mutable state (all resource fields, ConnectFailure, LastRebootTime), each with its own mutex
  • DeviceStore: shared map of device name to DeviceState, with a StateFor(name) accessor that creates entries on demand
  • Provider: lightweight per-reconcile instance created by the ProviderFunc factory, capturing the device name during Connect via the new Connection.DeviceName field

Add DeviceName to deviceutil.Connection and set it in GetDeviceConnection so the mock provider can identify the device without changing any controller code.

@github-actions github-actions Bot added size/XXL and removed size/XL labels Sep 11, 2026
@github-actions github-actions Bot added size/XL and removed size/XXL labels Sep 11, 2026
@felix-kaestner
felix-kaestner marked this pull request as ready for review September 11, 2026 16:43
@hardikdr hardikdr added the area/switch-automation Automation processes for network switch management and operations. label Sep 12, 2026
@hardikdr hardikdr added this to Roadmap Sep 12, 2026
@github-actions github-actions Bot added size/XXL and removed size/XL labels Sep 15, 2026
@felix-kaestner
felix-kaestner force-pushed the test-suites branch 5 times, most recently from 1534e47 to 20cba39 Compare September 16, 2026 16:53
@adamtrizuljak-sap

adamtrizuljak-sap commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

The idea seems sound. I tried to run the tests from this branch about 50x and all controllers seem to be working fine, except for the ConfigBackup controller which had 3 failures. You may want to look at it, but it may be out of scope of this PR to fix it.

configbackup_fail.txt

@felix-kaestner

felix-kaestner commented Sep 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Nice catch @adamtrizuljak-sap. I added another commit in c3777cc to address this.

@felix-kaestner
felix-kaestner force-pushed the test-suites branch 3 times, most recently from 607cf86 to 417d576 Compare September 22, 2026 12:12
@felix-kaestner
Isolate per-device provider state in controller tests
9c8d999 
The test provider was a singleton shared by all controllers.
Every test creates a distinct Device via GenerateName, but the
provider stored all state (DNS, BGP, NTP, LLDP, etc.) in flat
fields on a single struct. This caused state bleed between test
suites and intermittent failures.

Refactor the test provider into three parts:

- DeviceState: per-device mutable state (all resource fields,
  ConnectFailure, LastRebootTime), each with its own mutex
- DeviceStore: shared map of device name to DeviceState, with
  a StateFor(name) accessor that creates entries on demand
- Provider: lightweight per-reconcile instance created by the
  ProviderFunc factory, capturing the device name during
  Connect via the new Connection.DeviceName field

Add DeviceName to deviceutil.Connection and set it in
GetDeviceConnection so the mock provider can identify the
device without changing any controller code.

Signed-off-by: Felix Kästner <felix.kaestner@sap.com>
@felix-kaestner
Wait for resource deletion before Device in AfterEach
5a88529 
Every controller calls GetDeviceByName before the
DeletionTimestamp check. If the Device is deleted before a
dependent resource completes its finalizer, the resource
enters an infinite error-retry loop that creates API server
contention and causes flaky test timeouts.

Add Eventually waits for dependent resources to be fully
deleted before deleting the Device in all AfterEach hooks.

Signed-off-by: Felix Kästner <felix.kaestner@sap.com>
@felix-kaestner
Persist one-shot status before metadata
fa3a752 
Patch ConfigBackup and Probe status before metadata so completion markers
are visible before metadata updates enqueue another reconciliation. Patch
a deep copy for status to preserve pending metadata on the original object.

This prevents one-shot backups and probes from running twice when the
controller cache observes the metadata update before the status update.

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

github-actions Bot commented Sep 22, 2026

Copy link
Copy Markdown

Merging this branch will decrease overall coverage

Impacted Packages Coverage Δ 🤖
github.com/ironcore-dev/network-operator/internal/controller/cisco/nx 68.52% (-0.30%) 👎
github.com/ironcore-dev/network-operator/internal/controller/core 62.42% (-0.32%) 👎
github.com/ironcore-dev/network-operator/internal/deviceutil 47.76% (ø)
Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/ironcore-dev/network-operator/internal/controller/core/configbackup_controller.go 59.84% (-0.54%) 371 222 (-2) 149 (+2) 👎
github.com/ironcore-dev/network-operator/internal/controller/core/probe_controller.go 53.56% (ø) 407 218 189
github.com/ironcore-dev/network-operator/internal/deviceutil/deviceutil.go 47.76% (ø) 67 32 35

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/bordergateway_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/cisco/nx/system_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/cisco/nx/vpcdomain_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/acl_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/banner_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/bgp_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/bgp_peer_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/certificate_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/configbackup_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/device_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/dhcprelay_controller_deprecated_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/dhcprelay_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/dns_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/ethernetsegment_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/evpninstance_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/interface_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/isis_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/lldp_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/managementaccess_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/ntp_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/nve_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/pim_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/prefixset_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/probe_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/routingpolicy_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/snmp_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/suite_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/syslog_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/user_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/vlan_controller_test.go
  • github.com/ironcore-dev/network-operator/internal/controller/core/vrf_controller_test.go

This branch has not been deployed

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

Reviewers

No reviews

Assignees

No one assigned

Labels

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

Projects

Roadmap
Status: No status

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@felix-kaestner @adamtrizuljak-sap @hardikdr