Skip to content

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

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

felix-kaestner wants to merge 2 commits into
mainfrom
test-suites

Conversation

@felix-kaestner

Copy link
Copy Markdown
Contributor

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
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>
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>
@github-actions

Copy link
Copy Markdown

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

Impacted Packages Coverage Δ 🤖
github.com/ironcore-dev/network-operator/internal/controller/cisco/nx 69.56% (+4.50%) 👍
github.com/ironcore-dev/network-operator/internal/controller/core 63.42% (+1.81%) 👍
github.com/ironcore-dev/network-operator/internal/deviceutil 47.76% (-2.24%) 👎

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/ironcore-dev/network-operator/internal/deviceutil/deviceutil.go 47.76% (-2.24%) 67 (+9) 32 (+3) 35 (+6) 👎

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_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

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

Labels

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

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants