From 0acca61d6939815b73ab916e06573eab938fb107 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Wed, 23 Sep 2026 13:10:17 +0200 Subject: [PATCH 1/9] Correct NX-OS interface deletion fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Felix Kästner --- .../interface_aggregate_vpc_stp_lacp.txtar | 4 ++-- .../interface_physical_switchport_trunk.txtar | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/interface_aggregate_vpc_stp_lacp.txtar b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/interface_aggregate_vpc_stp_lacp.txtar index f76d78c9b..ad34a9037 100644 --- a/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/interface_aggregate_vpc_stp_lacp.txtar +++ b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/interface_aggregate_vpc_stp_lacp.txtar @@ -232,14 +232,14 @@ spec: "If-list": [ { "mode": "default", - "bpdufilter": "enable", + "bpdufilter": "default", "bpduguard": "default", "id": "po10" }, { "mode": "default", "bpdufilter": "default", - "bpduguard": "enable", + "bpduguard": "default", "id": "eth1/2" } ] diff --git a/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/interface_physical_switchport_trunk.txtar b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/interface_physical_switchport_trunk.txtar index e93bbb6ce..0e4b58d3d 100644 --- a/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/interface_physical_switchport_trunk.txtar +++ b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/interface_physical_switchport_trunk.txtar @@ -127,7 +127,7 @@ spec: { "mode": "default", "bpdufilter": "default", - "bpduguard": "enable", + "bpduguard": "default", "id": "eth1/2" } ] From 3ce68255d1585fb9fdd5833485c0db037c4d3290 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Fri, 11 Sep 2026 17:19:12 +0200 Subject: [PATCH 2/9] Isolate per-device provider state in controller tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../controller/core/acl_controller_test.go | 4 +- .../controller/core/banner_controller_test.go | 20 +- .../controller/core/bgp_controller_test.go | 8 +- .../core/bgp_peer_controller_test.go | 10 +- .../core/certificate_controller_test.go | 4 +- .../core/configbackup_controller_test.go | 40 +- .../controller/core/device_controller_test.go | 10 +- .../dhcprelay_controller_deprecated_test.go | 8 +- .../core/dhcprelay_controller_test.go | 39 +- .../controller/core/dns_controller_test.go | 8 +- .../core/ethernetsegment_controller_test.go | 4 +- .../core/evpninstance_controller_test.go | 4 +- .../core/interface_controller_test.go | 26 +- .../controller/core/isis_controller_test.go | 4 +- .../controller/core/lldp_controller_test.go | 70 +-- .../core/managementaccess_controller_test.go | 4 +- .../controller/core/ntp_controller_test.go | 8 +- .../controller/core/nve_controller_test.go | 48 +- .../controller/core/ospf_controller_test.go | 4 +- .../controller/core/pim_controller_test.go | 4 +- .../core/prefixset_controller_test.go | 4 +- .../controller/core/probe_controller_test.go | 6 +- .../core/routingpolicy_controller_test.go | 8 +- .../controller/core/snmp_controller_test.go | 8 +- internal/controller/core/suite_test.go | 573 ++++++++++-------- .../controller/core/syslog_controller_test.go | 4 +- .../controller/core/user_controller_test.go | 4 +- .../controller/core/vlan_controller_test.go | 4 +- .../controller/core/vrf_controller_test.go | 8 +- internal/deviceutil/deviceutil.go | 11 +- 30 files changed, 523 insertions(+), 434 deletions(-) diff --git a/internal/controller/core/acl_controller_test.go b/internal/controller/core/acl_controller_test.go index 9175dd60f..ed0275e2a 100644 --- a/internal/controller/core/acl_controller_test.go +++ b/internal/controller/core/acl_controller_test.go @@ -77,7 +77,7 @@ var _ = Describe("AccessControlList Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.ACLs.Has(name)).To(BeFalse(), "Provider shouldn't have AccessControlList configured anymore") + g.Expect(testDevices.StateFor(name).ACLs.Has(name)).To(BeFalse(), "Provider shouldn't have AccessControlList configured anymore") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -124,7 +124,7 @@ var _ = Describe("AccessControlList Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.ACLs.Has(name)).To(BeTrue(), "Provider should have AccessControlList configured") + g.Expect(testDevices.StateFor(name).ACLs.Has(name)).To(BeTrue(), "Provider should have AccessControlList configured") }).Should(Succeed()) }) }) diff --git a/internal/controller/core/banner_controller_test.go b/internal/controller/core/banner_controller_test.go index 21dcaa565..d10c72207 100644 --- a/internal/controller/core/banner_controller_test.go +++ b/internal/controller/core/banner_controller_test.go @@ -46,8 +46,8 @@ var _ = Describe("Banner Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.PreLoginBanner).To(BeNil(), "Provider PreLogin Banner should be nil") - g.Expect(testProvider.PostLoginBanner).To(BeNil(), "Provider PostLogin Banner should be nil") + g.Expect(testDevices.StateFor(name).PreLoginBanner).To(BeNil(), "Provider PreLogin Banner should be nil") + g.Expect(testDevices.StateFor(name).PostLoginBanner).To(BeNil(), "Provider PostLogin Banner should be nil") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -108,10 +108,10 @@ var _ = Describe("Banner Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.PreLoginBanner).ToNot(BeNil(), "Provider Banner should not be nil") - g.Expect(testProvider.PostLoginBanner).To(BeNil(), "Provider PostLogin Banner should be nil") - if testProvider.PreLoginBanner != nil { - g.Expect(*testProvider.PreLoginBanner).To(Equal("Test Banner")) + g.Expect(testDevices.StateFor(name).PreLoginBanner).ToNot(BeNil(), "Provider Banner should not be nil") + g.Expect(testDevices.StateFor(name).PostLoginBanner).To(BeNil(), "Provider PostLogin Banner should be nil") + if testDevices.StateFor(name).PreLoginBanner != nil { + g.Expect(*testDevices.StateFor(name).PreLoginBanner).To(Equal("Test Banner")) } }).Should(Succeed()) }) @@ -167,10 +167,10 @@ var _ = Describe("Banner Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.PreLoginBanner).To(BeNil(), "Provider PreLogin Banner should be nil") - g.Expect(testProvider.PostLoginBanner).ToNot(BeNil(), "Provider PostLogin Banner should not be nil") - if testProvider.PostLoginBanner != nil { - g.Expect(*testProvider.PostLoginBanner).To(Equal("Test Banner")) + g.Expect(testDevices.StateFor(name).PreLoginBanner).To(BeNil(), "Provider PreLogin Banner should be nil") + g.Expect(testDevices.StateFor(name).PostLoginBanner).ToNot(BeNil(), "Provider PostLogin Banner should not be nil") + if testDevices.StateFor(name).PostLoginBanner != nil { + g.Expect(*testDevices.StateFor(name).PostLoginBanner).To(Equal("Test Banner")) } }).Should(Succeed()) }) diff --git a/internal/controller/core/bgp_controller_test.go b/internal/controller/core/bgp_controller_test.go index 89ce7d1fc..daf722187 100644 --- a/internal/controller/core/bgp_controller_test.go +++ b/internal/controller/core/bgp_controller_test.go @@ -65,7 +65,7 @@ var _ = Describe("BGP Controller", func() { By("Verifying BGP is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.BGP).To(BeNil(), "Provider should not have BGP instance configured") + g.Expect(testDevices.StateFor(device.Name).BGP).To(BeNil(), "Provider should not have BGP instance configured") }).Should(Succeed()) By("Deleting the Device resource") @@ -121,7 +121,7 @@ var _ = Describe("BGP Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.BGP).ToNot(BeNil(), "Provider should have BGP instance configured") + g.Expect(testDevices.StateFor(device.Name).BGP).ToNot(BeNil(), "Provider should have BGP instance configured") }).Should(Succeed()) }) @@ -177,8 +177,8 @@ var _ = Describe("BGP Controller", func() { By("Ensuring the provider receives the VRF") Eventually(func(g Gomega) { - g.Expect(testProvider.BGPVRF).ToNot(BeNil()) - g.Expect(testProvider.BGPVRF.Spec.Name).To(Equal("CC-MGMT")) + g.Expect(testDevices.StateFor(device.Name).BGPVRF).ToNot(BeNil()) + g.Expect(testDevices.StateFor(device.Name).BGPVRF.Spec.Name).To(Equal("CC-MGMT")) }).Should(Succeed()) By("Ensuring ReadyCondition is True") diff --git a/internal/controller/core/bgp_peer_controller_test.go b/internal/controller/core/bgp_peer_controller_test.go index fa89c92b0..f937c7080 100644 --- a/internal/controller/core/bgp_peer_controller_test.go +++ b/internal/controller/core/bgp_peer_controller_test.go @@ -66,7 +66,7 @@ var _ = Describe("BGPPeer Controller", func() { By("Verifying BGP peer is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.BGPPeers.Len()).To(Equal(0), "Provider should not have any BGP peers configured") + g.Expect(testDevices.StateFor(device.Name).BGPPeers.Len()).To(Equal(0), "Provider should not have any BGP peers configured") }).Should(Succeed()) By("Deleting the Device resource") @@ -146,7 +146,7 @@ var _ = Describe("BGPPeer Controller", func() { By("Verifying the BGP peer is configured in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.BGPPeers.Has(host)).To(BeTrue(), "Provider should have BGP peer configured") + g.Expect(testDevices.StateFor(device.Name).BGPPeers.Has(host)).To(BeTrue(), "Provider should have BGP peer configured") }).Should(Succeed()) }) @@ -216,7 +216,7 @@ var _ = Describe("BGPPeer Controller", func() { By("Verifying the BGP peer is configured in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.BGPPeers.Has(host)).To(BeTrue(), "Provider should have BGP peer configured") + g.Expect(testDevices.StateFor(device.Name).BGPPeers.Has(host)).To(BeTrue(), "Provider should have BGP peer configured") }).Should(Succeed()) }) @@ -379,7 +379,7 @@ var _ = Describe("BGPPeer Controller", func() { By("Verifying the BGP peer is NOT configured in the provider") Consistently(func(g Gomega) { - g.Expect(testProvider.BGPPeers.Has(host)).To(BeFalse(), "Provider should not have BGP peer configured") + g.Expect(testDevices.StateFor(device.Name).BGPPeers.Has(host)).To(BeFalse(), "Provider should not have BGP peer configured") }).Should(Succeed()) }) @@ -430,7 +430,7 @@ var _ = Describe("BGPPeer Controller", func() { By("Verifying the BGP peer is NOT configured in the provider") Consistently(func(g Gomega) { - g.Expect(testProvider.BGPPeers.Has("10.0.0.3")).To(BeFalse(), "Provider should not have BGP peer configured") + g.Expect(testDevices.StateFor(device.Name).BGPPeers.Has("10.0.0.3")).To(BeFalse(), "Provider should not have BGP peer configured") }).Should(Succeed()) }) diff --git a/internal/controller/core/certificate_controller_test.go b/internal/controller/core/certificate_controller_test.go index 56c0d6859..87b9e2964 100644 --- a/internal/controller/core/certificate_controller_test.go +++ b/internal/controller/core/certificate_controller_test.go @@ -90,7 +90,7 @@ var _ = Describe("Certificate Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Certs.Has("cert1")).To(BeFalse(), "Certificate should be deleted from the provider") + g.Expect(testDevices.StateFor(name).Certs.Has("cert1")).To(BeFalse(), "Certificate should be deleted from the provider") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -137,7 +137,7 @@ var _ = Describe("Certificate Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Certs.Has("cert1")).To(BeTrue(), "Certificate should be present in the provider") + g.Expect(testDevices.StateFor(name).Certs.Has("cert1")).To(BeTrue(), "Certificate should be present in the provider") }).Should(Succeed()) }) }) diff --git a/internal/controller/core/configbackup_controller_test.go b/internal/controller/core/configbackup_controller_test.go index 961413bef..2e2b8505f 100644 --- a/internal/controller/core/configbackup_controller_test.go +++ b/internal/controller/core/configbackup_controller_test.go @@ -53,10 +53,10 @@ var _ = Describe("ConfigBackup Controller", func() { }).Should(Succeed()) By("Resetting the test provider state") - testProvider.Lock() - testProvider.ConfigBackups = nil - testProvider.StartupConfig = nil - testProvider.Unlock() + testDevices.StateFor(device.Name).Lock() + testDevices.StateFor(device.Name).ConfigBackups = nil + testDevices.StateFor(device.Name).StartupConfig = nil + testDevices.StateFor(device.Name).Unlock() }) AfterEach(func() { @@ -157,22 +157,22 @@ var _ = Describe("ConfigBackup Controller", func() { By("Verifying the provider received the startup backup") Eventually(func(g Gomega) { - testProvider.Lock() - defer testProvider.Unlock() - g.Expect(testProvider.StartupConfig).NotTo(BeNil()) + testDevices.StateFor(device.Name).Lock() + defer testDevices.StateFor(device.Name).Unlock() + g.Expect(testDevices.StateFor(device.Name).StartupConfig).NotTo(BeNil()) }).Should(Succeed()) }) It("Should rotate old backups according to retention policy", func() { By("Pre-seeding the provider with existing backups") - testProvider.Lock() + testDevices.StateFor(device.Name).Lock() size := int64(1024) - testProvider.ConfigBackups = []*provider.ConfigBackupFile{ + testDevices.StateFor(device.Name).ConfigBackups = []*provider.ConfigBackupFile{ {Path: "bootflash:///backups/configbackup-old-1", SizeBytes: &size, CreatedAt: time.Date(2026, time.April, 10, 2, 0, 0, 0, time.UTC)}, {Path: "bootflash:///backups/configbackup-old-2", SizeBytes: &size, CreatedAt: time.Date(2026, time.April, 11, 2, 0, 0, 0, time.UTC)}, {Path: "bootflash:///backups/configbackup-old-3", SizeBytes: &size, CreatedAt: time.Date(2026, time.April, 12, 2, 0, 0, 0, time.UTC)}, } - testProvider.Unlock() + testDevices.StateFor(device.Name).Unlock() By("Creating a ConfigBackup with retention keepLast: 2") backup = &v1alpha1.ConfigBackup{ @@ -189,10 +189,10 @@ var _ = Describe("ConfigBackup Controller", func() { By("Verifying old backups are rotated") Eventually(func(g Gomega) { - testProvider.Lock() - defer testProvider.Unlock() + testDevices.StateFor(device.Name).Lock() + defer testDevices.StateFor(device.Name).Unlock() // 3 pre-seeded + 1 new = 4, keepLast=2 means 2 oldest deleted → 2 remain - g.Expect(testProvider.ConfigBackups).To(HaveLen(2)) + g.Expect(testDevices.StateFor(device.Name).ConfigBackups).To(HaveLen(2)) }).Should(Succeed()) By("Verifying the status reflects the retained count") @@ -206,13 +206,13 @@ var _ = Describe("ConfigBackup Controller", func() { It("Should block backup when storage threshold is exceeded", func() { By("Pre-seeding the provider to simulate full storage") - testProvider.Lock() + testDevices.StateFor(device.Name).Lock() size := int64(95) - testProvider.ConfigBackups = []*provider.ConfigBackupFile{ + testDevices.StateFor(device.Name).ConfigBackups = []*provider.ConfigBackupFile{ {Path: "bootflash:///backups/existing-file", SizeBytes: &size, CreatedAt: time.Now()}, } - testProvider.StorageTotal = 100 - testProvider.Unlock() + testDevices.StateFor(device.Name).StorageTotal = 100 + testDevices.StateFor(device.Name).Unlock() By("Creating a ConfigBackup with a storage threshold") minFreeBytes := int64(10) @@ -265,9 +265,9 @@ var _ = Describe("ConfigBackup Controller", func() { By("Verifying the provider only has one backup (no re-runs)") Consistently(func(g Gomega) { - testProvider.Lock() - defer testProvider.Unlock() - g.Expect(testProvider.ConfigBackups).To(HaveLen(1)) + testDevices.StateFor(device.Name).Lock() + defer testDevices.StateFor(device.Name).Unlock() + g.Expect(testDevices.StateFor(device.Name).ConfigBackups).To(HaveLen(1)) }).Should(Succeed()) }) diff --git a/internal/controller/core/device_controller_test.go b/internal/controller/core/device_controller_test.go index 2febef267..84868a37d 100644 --- a/internal/controller/core/device_controller_test.go +++ b/internal/controller/core/device_controller_test.go @@ -462,10 +462,10 @@ var _ = Describe("Device Controller", func() { It("Should set Reachable=False and Ready=Unknown when the device is unreachable", func() { By("Making the provider return a connect error") - testProvider.SetConnectError(errors.New("connection refused")) + testDevices.StateFor(name).SetConnectFailure(errors.New("connection refused")) DeferCleanup(func() { - testProvider.SetConnectError(nil) + testDevices.StateFor(name).SetConnectFailure(nil) }) By("Creating the custom resource for the Kind Device") @@ -501,7 +501,7 @@ var _ = Describe("Device Controller", func() { }).Should(Succeed()) By("Clearing the connect error to simulate recovery") - testProvider.SetConnectError(nil) + testDevices.StateFor(name).SetConnectFailure(nil) By("Verifying Reachable=True and Ready=True after recovery") Eventually(func(g Gomega) { @@ -657,9 +657,9 @@ var _ = Describe("Device Controller", func() { By("Advancing the reboot time in the provider to simulate a device reboot") newRebootTime := lastRebootTime.Add(time.Hour) - testProvider.SetLastRebootTime(newRebootTime) + testDevices.StateFor(name).SetLastRebootTime(newRebootTime) DeferCleanup(func() { - testProvider.SetLastRebootTime(lastRebootTime) + testDevices.StateFor(name).SetLastRebootTime(lastRebootTime) }) By("Verifying LastRebootTime in status is updated to the new value") diff --git a/internal/controller/core/dhcprelay_controller_deprecated_test.go b/internal/controller/core/dhcprelay_controller_deprecated_test.go index 3b56281c7..9ba690abe 100644 --- a/internal/controller/core/dhcprelay_controller_deprecated_test.go +++ b/internal/controller/core/dhcprelay_controller_deprecated_test.go @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and IronCore contributors // SPDX-License-Identifier: Apache-2.0 package core @@ -69,13 +69,13 @@ var _ = Describe("DHCPRelay Controller with deprecated API fields", func() { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, relay))).To(Succeed()) Eventually(func(g Gomega) { g.Expect(errors.IsNotFound(k8sClient.Get(ctx, relayKey, &v1alpha1.DHCPRelay{}))).To(BeTrue()) - g.Expect(testProvider.DHCPRelay).To(BeNil()) + g.Expect(testDevices.StateFor(device.Name).DHCPRelay).To(BeNil()) }).Should(Succeed()) }) Eventually(func(g Gomega) { - g.Expect(testProvider.DHCPRelay).ToNot(BeNil()) - g.Expect(testProvider.DHCPRelay.GetName()).To(Equal(relay.Name)) + g.Expect(testDevices.StateFor(device.Name).DHCPRelay).ToNot(BeNil()) + g.Expect(testDevices.StateFor(device.Name).DHCPRelay.GetName()).To(Equal(relay.Name)) }).Should(Succeed()) Eventually(func(g Gomega) { diff --git a/internal/controller/core/dhcprelay_controller_test.go b/internal/controller/core/dhcprelay_controller_test.go index 94cc58637..0ebd67518 100644 --- a/internal/controller/core/dhcprelay_controller_test.go +++ b/internal/controller/core/dhcprelay_controller_test.go @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-Fi// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and IronCore contributors // SPDX-License-Identifier: Apache-2.0 package core @@ -138,7 +138,7 @@ var _ = Describe("DHCPRelay Controller", func() { By("Verifying the resource has been deleted") Eventually(func(g Gomega) { - g.Expect(testProvider.DHCPRelay).To(BeNil(), "Provider should have no DHCPRelay configured") + g.Expect(testDevices.StateFor(deviceName).DHCPRelay).To(BeNil(), "Provider should have no DHCPRelay configured") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -198,9 +198,9 @@ var _ = Describe("DHCPRelay Controller", func() { By("Ensuring the DHCPRelay is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.DHCPRelay).ToNot(BeNil(), "Provider DHCPRelay should not be nil") - if testProvider.DHCPRelay != nil { - g.Expect(testProvider.DHCPRelay.GetName()).To(Equal(resourceName), "Provider should have DHCPRelay configured") + g.Expect(testDevices.StateFor(deviceName).DHCPRelay).ToNot(BeNil(), "Provider DHCPRelay should not be nil") + if testDevices.StateFor(deviceName).DHCPRelay != nil { + g.Expect(testDevices.StateFor(deviceName).DHCPRelay.GetName()).To(Equal(resourceName), "Provider should have DHCPRelay configured") } }).Should(Succeed()) }) @@ -239,7 +239,7 @@ var _ = Describe("DHCPRelay Controller", func() { g.Expect(cond).NotTo(BeNil()) g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) g.Expect(cond.Reason).To(Equal(v1alpha1.IncompatibleProviderConfigRef)) - g.Expect(testProvider.DHCPRelay).To(BeNil()) + g.Expect(testDevices.StateFor(deviceName).DHCPRelay).To(BeNil()) }).Should(Succeed()) }) @@ -293,8 +293,8 @@ var _ = Describe("DHCPRelay Controller", func() { By("Ensuring the DHCPRelay is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.DHCPRelay).ToNot(BeNil()) - g.Expect(testProvider.DHCPRelay.GetName()).To(Equal(resourceName)) + g.Expect(testDevices.StateFor(deviceName).DHCPRelay).ToNot(BeNil()) + g.Expect(testDevices.StateFor(deviceName).DHCPRelay.GetName()).To(Equal(resourceName)) }).Should(Succeed()) }) @@ -347,9 +347,10 @@ var _ = Describe("DHCPRelay Controller", func() { }).Should(Succeed()) By("Cleaning up the duplicate DHCPRelay resource") - testProvider.Lock() - deleteCalls := testProvider.DHCPRelayDeleteCalls - testProvider.Unlock() + state := testDevices.StateFor(deviceName) + state.Lock() + deleteCalls := state.DHCPRelayDeleteCalls + state.Unlock() Expect(k8sClient.Delete(ctx, duplicateDHCPRelay)).To(Succeed()) Eventually(func(g Gomega) { err := k8sClient.Get(ctx, duplicateKey, &v1alpha1.DHCPRelay{}) @@ -357,10 +358,10 @@ var _ = Describe("DHCPRelay Controller", func() { }).Should(Succeed()) By("Verifying deleting the duplicate did not remove the active provider configuration") - testProvider.Lock() - actualDeleteCalls := testProvider.DHCPRelayDeleteCalls - providerDHCPRelay := testProvider.DHCPRelay - testProvider.Unlock() + state.Lock() + actualDeleteCalls := state.DHCPRelayDeleteCalls + providerDHCPRelay := state.DHCPRelay + state.Unlock() Expect(actualDeleteCalls).To(Equal(deleteCalls)) Expect(providerDHCPRelay).ToNot(BeNil()) Expect(providerDHCPRelay.Name).To(Equal(resourceName)) @@ -477,7 +478,7 @@ var _ = Describe("DHCPRelay Controller", func() { By("Verifying DHCPRelay is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.DHCPRelay).ToNot(BeNil()) + g.Expect(testDevices.StateFor(deviceName).DHCPRelay).ToNot(BeNil()) }).Should(Succeed()) By("Deleting the DHCPRelay resource") @@ -485,7 +486,7 @@ var _ = Describe("DHCPRelay Controller", func() { By("Verifying the DHCPRelay is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.DHCPRelay).To(BeNil(), "Provider should have no DHCPRelay configured after deletion") + g.Expect(testDevices.StateFor(deviceName).DHCPRelay).To(BeNil(), "Provider should have no DHCPRelay configured after deletion") }).Should(Succeed()) By("Verifying the resource is fully deleted") @@ -649,7 +650,7 @@ var _ = Describe("DHCPRelay Controller", func() { By("Verifying the provider has been cleaned up") Eventually(func(g Gomega) { - g.Expect(testProvider.DHCPRelay).To(BeNil(), "Provider should have no DHCPRelay configured") + g.Expect(testDevices.StateFor(deviceName).DHCPRelay).To(BeNil(), "Provider should have no DHCPRelay configured") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -686,7 +687,7 @@ var _ = Describe("DHCPRelay Controller", func() { By("Ensuring the DHCPRelay is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.DHCPRelay).ToNot(BeNil(), "Provider DHCPRelay should not be nil") + g.Expect(testDevices.StateFor(deviceName).DHCPRelay).ToNot(BeNil(), "Provider DHCPRelay should not be nil") }).Should(Succeed()) }) }) diff --git a/internal/controller/core/dns_controller_test.go b/internal/controller/core/dns_controller_test.go index 7a91b7ce0..9ee858805 100644 --- a/internal/controller/core/dns_controller_test.go +++ b/internal/controller/core/dns_controller_test.go @@ -63,7 +63,7 @@ var _ = Describe("DNS Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.DNS).To(BeNil(), "Provider DNS should be nil") + g.Expect(testDevices.StateFor(name).DNS).To(BeNil(), "Provider DNS should be nil") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -110,9 +110,9 @@ var _ = Describe("DNS Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.DNS).ToNot(BeNil(), "Provider DNS should not be nil") - if testProvider.DNS != nil { - g.Expect(testProvider.DNS.Spec.Domain).To(Equal("example.com")) + g.Expect(testDevices.StateFor(name).DNS).ToNot(BeNil(), "Provider DNS should not be nil") + if testDevices.StateFor(name).DNS != nil { + g.Expect(testDevices.StateFor(name).DNS.Spec.Domain).To(Equal("example.com")) } }).Should(Succeed()) }) diff --git a/internal/controller/core/ethernetsegment_controller_test.go b/internal/controller/core/ethernetsegment_controller_test.go index 3733858f9..0db06ce67 100644 --- a/internal/controller/core/ethernetsegment_controller_test.go +++ b/internal/controller/core/ethernetsegment_controller_test.go @@ -54,7 +54,7 @@ var _ = Describe("EthernetSegment Controller", func() { By("Verifying the EthernetSegment is removed from the provider") Eventually(func(g Gomega) { - _, exists := testProvider.GetEthernetSegment(name) + _, exists := testDevices.StateFor(name).GetEthernetSegment(name) g.Expect(exists).To(BeFalse(), "Provider shouldn't have ESI configured anymore") }).Should(Succeed()) @@ -146,7 +146,7 @@ var _ = Describe("EthernetSegment Controller", func() { By("Verifying the EthernetSegment is configured in the provider") Eventually(func(g Gomega) { - storedESI, exists := testProvider.GetEthernetSegment(name) + storedESI, exists := testDevices.StateFor(name).GetEthernetSegment(name) g.Expect(exists).To(BeTrue(), "Provider should have ESI configured") g.Expect(storedESI).To(Equal(esi)) }).Should(Succeed()) diff --git a/internal/controller/core/evpninstance_controller_test.go b/internal/controller/core/evpninstance_controller_test.go index 4ad87d3ea..679e7a271 100644 --- a/internal/controller/core/evpninstance_controller_test.go +++ b/internal/controller/core/evpninstance_controller_test.go @@ -53,7 +53,7 @@ var _ = Describe("EVPNInstance Controller", func() { By("Verifying the EVPNInstance is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.EVIs.Has(vni)).To(BeFalse(), "Provider shouldn't have VNI configured anymore") + g.Expect(testDevices.StateFor(name).EVIs.Has(vni)).To(BeFalse(), "Provider shouldn't have VNI configured anymore") }).Should(Succeed()) By("Cleaning up the test Device resource") @@ -149,7 +149,7 @@ var _ = Describe("EVPNInstance Controller", func() { By("Verifying the EVPNInstance is configured in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.EVIs.Has(vni)).To(BeTrue(), "Provider should have VNI configured") + g.Expect(testDevices.StateFor(name).EVIs.Has(vni)).To(BeTrue(), "Provider should have VNI configured") }).Should(Succeed()) }) diff --git a/internal/controller/core/interface_controller_test.go b/internal/controller/core/interface_controller_test.go index 2cffc6a56..fb6dd9a98 100644 --- a/internal/controller/core/interface_controller_test.go +++ b/internal/controller/core/interface_controller_test.go @@ -75,7 +75,7 @@ var _ = Describe("Interface Controller", func() { By("Verifying the Interface is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Ports.Has(name)).To(BeFalse(), "Provider shouldn't have Interface configured anymore") + g.Expect(testDevices.StateFor(name).Ports.Has(name)).To(BeFalse(), "Provider shouldn't have Interface configured anymore") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -144,7 +144,7 @@ var _ = Describe("Interface Controller", func() { By("Verifying the Interface is configured in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Ports.Has(name)).To(BeTrue(), "Provider should have Interface configured") + g.Expect(testDevices.StateFor(name).Ports.Has(name)).To(BeTrue(), "Provider should have Interface configured") }).Should(Succeed()) }) @@ -419,7 +419,7 @@ var _ = Describe("Interface Controller", func() { By("Verifying the Aggregate Interface is configured in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Ports.Has(name)).To(BeTrue(), "Provider should have Aggregate Interface configured") + g.Expect(testDevices.StateFor(name).Ports.Has(name)).To(BeTrue(), "Provider should have Aggregate Interface configured") }).Should(Succeed()) }) @@ -717,7 +717,7 @@ var _ = Describe("Interface Controller", func() { By("Verifying the Aggregate Interface is configured in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Ports.Has(name)).To(BeTrue(), "Provider should have L3 Aggregate Interface configured") + g.Expect(testDevices.StateFor(name).Ports.Has(name)).To(BeTrue(), "Provider should have L3 Aggregate Interface configured") }).Should(Succeed()) }) @@ -776,7 +776,7 @@ var _ = Describe("Interface Controller", func() { By("Verifying the member Physical interface is configured in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Ports.Has("eth1-100")).To(BeTrue(), "Provider should have member Physical Interface configured") + g.Expect(testDevices.StateFor(name).Ports.Has("eth1-100")).To(BeTrue(), "Provider should have member Physical Interface configured") }).Should(Succeed()) }) @@ -902,12 +902,12 @@ var _ = Describe("Interface Controller", func() { By("Verifying the Subinterface is configured in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Ports.Has(parentName+".100")).To(BeTrue(), "Provider should have Subinterface configured") + g.Expect(testDevices.StateFor(name).Ports.Has(parentName+".100")).To(BeTrue(), "Provider should have Subinterface configured") }).Should(Succeed()) By("Verifying the parent Physical interface is configured in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Ports.Has(parentName)).To(BeTrue(), "Provider should have parent Physical Interface configured") + g.Expect(testDevices.StateFor(name).Ports.Has(parentName)).To(BeTrue(), "Provider should have parent Physical Interface configured") }).Should(Succeed()) }) @@ -1021,7 +1021,7 @@ var _ = Describe("Interface Controller", func() { By("Verifying the Interface is configured in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Ports.Has(name)).To(BeTrue(), "Provider should have RoutedVLAN Interface configured") + g.Expect(testDevices.StateFor(name).Ports.Has(name)).To(BeTrue(), "Provider should have RoutedVLAN Interface configured") }).Should(Succeed()) }) @@ -1164,7 +1164,7 @@ var _ = Describe("Interface Controller", func() { By("Verifying the Interface is configured in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Ports.Has(name)).To(BeTrue(), "Provider should have Interface with VRF configured") + g.Expect(testDevices.StateFor(name).Ports.Has(name)).To(BeTrue(), "Provider should have Interface with VRF configured") }).Should(Succeed()) }) @@ -1326,7 +1326,7 @@ var _ = Describe("Interface Controller", func() { }).Should(Succeed()) By("Configuring LLDP neighbor on the provider for the local interface") - testProvider.SetLLDPNeighbor("Ethernet1/2", "remote-switch.example.com", "aa:bb:cc:dd:ee:ff", "Ethernet1/1", 120) + testDevices.StateFor(localDevice.Name).SetLLDPNeighbor("Ethernet1/2", "remote-switch.example.com", "aa:bb:cc:dd:ee:ff", "Ethernet1/1", 120) By("Creating a local Physical Interface with neighbor label pointing to the remote interface") localIntf = &v1alpha1.Interface{ @@ -1347,9 +1347,9 @@ var _ = Describe("Interface Controller", func() { AfterEach(func() { By("Cleaning up LLDP neighbor configuration") - testProvider.Lock() - delete(testProvider.LLDPNeighbors, "Ethernet1/2") - testProvider.Unlock() + testDevices.StateFor(localDevice.Name).Lock() + delete(testDevices.StateFor(localDevice.Name).LLDPNeighbors, "Ethernet1/2") + testDevices.StateFor(localDevice.Name).Unlock() By("Cleaning up all Interface resources") intfList := &v1alpha1.InterfaceList{} diff --git a/internal/controller/core/isis_controller_test.go b/internal/controller/core/isis_controller_test.go index 3129a9874..78dbd5406 100644 --- a/internal/controller/core/isis_controller_test.go +++ b/internal/controller/core/isis_controller_test.go @@ -64,7 +64,7 @@ var _ = Describe("ISIS Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.ISIS.Has("UNDERLAY")).To(BeFalse(), "Provider should not have ISIS instance configured") + g.Expect(testDevices.StateFor(name).ISIS.Has("UNDERLAY")).To(BeFalse(), "Provider should not have ISIS instance configured") }).Should(Succeed()) By("Cleanup the Device resource") @@ -111,7 +111,7 @@ var _ = Describe("ISIS Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.ISIS.Has("UNDERLAY")).To(BeTrue(), "Provider should have ISIS instance configured") + g.Expect(testDevices.StateFor(name).ISIS.Has("UNDERLAY")).To(BeTrue(), "Provider should have ISIS instance configured") }).Should(Succeed()) }) }) diff --git a/internal/controller/core/lldp_controller_test.go b/internal/controller/core/lldp_controller_test.go index 84794703f..1774566ce 100644 --- a/internal/controller/core/lldp_controller_test.go +++ b/internal/controller/core/lldp_controller_test.go @@ -59,7 +59,7 @@ var _ = Describe("LLDP Controller", func() { By("Verifying the resource has been deleted") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).To(BeNil(), "Provider should have no LLDP configured") + g.Expect(testDevices.StateFor(deviceName).LLDP).To(BeNil(), "Provider should have no LLDP configured") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -125,9 +125,9 @@ var _ = Describe("LLDP Controller", func() { By("Ensuring the LLDP is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).ToNot(BeNil(), "Provider LLDP should not be nil") - if testProvider.LLDP != nil { - g.Expect(testProvider.LLDP.GetName()).To(Equal(deviceName+"-lldp"), "Provider should have LLDP configured") + g.Expect(testDevices.StateFor(deviceName).LLDP).ToNot(BeNil(), "Provider LLDP should not be nil") + if testDevices.StateFor(deviceName).LLDP != nil { + g.Expect(testDevices.StateFor(deviceName).LLDP.GetName()).To(Equal(deviceName+"-lldp"), "Provider should have LLDP configured") } }).Should(Succeed()) }) @@ -164,9 +164,9 @@ var _ = Describe("LLDP Controller", func() { By("Ensuring the LLDP is created in the provider with AdminState Down") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).ToNot(BeNil()) - if testProvider.LLDP != nil { - g.Expect(testProvider.LLDP.Spec.AdminState).To(Equal(v1alpha1.AdminStateDown)) + g.Expect(testDevices.StateFor(deviceName).LLDP).ToNot(BeNil()) + if testDevices.StateFor(deviceName).LLDP != nil { + g.Expect(testDevices.StateFor(deviceName).LLDP.Spec.AdminState).To(Equal(v1alpha1.AdminStateDown)) } }).Should(Succeed()) }) @@ -243,7 +243,7 @@ var _ = Describe("LLDP Controller", func() { By("Verifying LLDP is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).ToNot(BeNil()) + g.Expect(testDevices.StateFor(deviceName).LLDP).ToNot(BeNil()) }).Should(Succeed()) By("Deleting the LLDP resource") @@ -251,7 +251,7 @@ var _ = Describe("LLDP Controller", func() { By("Verifying the LLDP is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).To(BeNil(), "Provider should have no LLDP configured after deletion") + g.Expect(testDevices.StateFor(deviceName).LLDP).To(BeNil(), "Provider should have no LLDP configured after deletion") }).Should(Succeed()) By("Verifying the resource is fully deleted") @@ -337,7 +337,7 @@ var _ = Describe("LLDP Controller", func() { By("Verifying the provider has been cleaned up") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).To(BeNil(), "Provider should have no LLDP configured") + g.Expect(testDevices.StateFor(deviceName).LLDP).To(BeNil(), "Provider should have no LLDP configured") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -396,8 +396,8 @@ var _ = Describe("LLDP Controller", func() { By("Verifying the provider still has AdminState Up (reconciliation was skipped)") Consistently(func(g Gomega) { - g.Expect(testProvider.LLDP).ToNot(BeNil()) - g.Expect(testProvider.LLDP.Spec.AdminState).To(Equal(v1alpha1.AdminStateUp)) + g.Expect(testDevices.StateFor(deviceName).LLDP).ToNot(BeNil()) + g.Expect(testDevices.StateFor(deviceName).LLDP.Spec.AdminState).To(Equal(v1alpha1.AdminStateUp)) }).Should(Succeed()) By("Unpausing the Device") @@ -410,8 +410,8 @@ var _ = Describe("LLDP Controller", func() { By("Verifying the provider now has AdminState Down (reconciliation resumed)") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).ToNot(BeNil()) - g.Expect(testProvider.LLDP.Spec.AdminState).To(Equal(v1alpha1.AdminStateDown)) + g.Expect(testDevices.StateFor(deviceName).LLDP).ToNot(BeNil()) + g.Expect(testDevices.StateFor(deviceName).LLDP.Spec.AdminState).To(Equal(v1alpha1.AdminStateDown)) }).Should(Succeed()) }) }) @@ -458,7 +458,7 @@ var _ = Describe("LLDP Controller", func() { By("Verifying the resource has been deleted") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).To(BeNil(), "Provider should have no LLDP configured") + g.Expect(testDevices.StateFor(deviceName).LLDP).To(BeNil(), "Provider should have no LLDP configured") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -598,7 +598,7 @@ var _ = Describe("LLDP Controller", func() { By("Verifying the resource has been deleted") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).To(BeNil(), "Provider should have no LLDP configured") + g.Expect(testDevices.StateFor(deviceName).LLDP).To(BeNil(), "Provider should have no LLDP configured") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -818,7 +818,7 @@ var _ = Describe("LLDP Controller", func() { By("Verifying the resource has been deleted") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).To(BeNil(), "Provider should have no LLDP configured") + g.Expect(testDevices.StateFor(deviceName).LLDP).To(BeNil(), "Provider should have no LLDP configured") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -852,8 +852,8 @@ var _ = Describe("LLDP Controller", func() { By("Verifying provider has AdminState Up") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).ToNot(BeNil()) - g.Expect(testProvider.LLDP.Spec.AdminState).To(Equal(v1alpha1.AdminStateUp)) + g.Expect(testDevices.StateFor(deviceName).LLDP).ToNot(BeNil()) + g.Expect(testDevices.StateFor(deviceName).LLDP.Spec.AdminState).To(Equal(v1alpha1.AdminStateUp)) }).Should(Succeed()) By("Updating AdminState to Down") @@ -866,8 +866,8 @@ var _ = Describe("LLDP Controller", func() { By("Verifying provider has AdminState Down") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).ToNot(BeNil()) - g.Expect(testProvider.LLDP.Spec.AdminState).To(Equal(v1alpha1.AdminStateDown)) + g.Expect(testDevices.StateFor(deviceName).LLDP).ToNot(BeNil()) + g.Expect(testDevices.StateFor(deviceName).LLDP.Spec.AdminState).To(Equal(v1alpha1.AdminStateDown)) }).Should(Succeed()) }) @@ -1060,7 +1060,7 @@ var _ = Describe("LLDP Controller", func() { By("Verifying the provider has been cleaned up") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).To(BeNil(), "Provider should have no LLDP configured") + g.Expect(testDevices.StateFor(deviceName).LLDP).To(BeNil(), "Provider should have no LLDP configured") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -1215,9 +1215,9 @@ var _ = Describe("LLDP Controller", func() { AfterEach(func() { By("Resetting provider LLDP operational status to true") - testProvider.Lock() - testProvider.LLDPOperStatus = true - testProvider.Unlock() + testDevices.StateFor(deviceName).Lock() + testDevices.StateFor(deviceName).LLDPOperStatus = true + testDevices.StateFor(deviceName).Unlock() By("Cleaning up the LLDP resource") lldp = &v1alpha1.LLDP{} @@ -1233,7 +1233,7 @@ var _ = Describe("LLDP Controller", func() { By("Verifying the provider has been cleaned up") Eventually(func(g Gomega) { - g.Expect(testProvider.LLDP).To(BeNil(), "Provider should have no LLDP configured") + g.Expect(testDevices.StateFor(deviceName).LLDP).To(BeNil(), "Provider should have no LLDP configured") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -1245,9 +1245,9 @@ var _ = Describe("LLDP Controller", func() { It("Should set OperationalCondition to False when LLDP is operationally down", func() { By("Setting provider to return operational status down") - testProvider.Lock() - testProvider.LLDPOperStatus = false - testProvider.Unlock() + testDevices.StateFor(deviceName).Lock() + testDevices.StateFor(deviceName).LLDPOperStatus = false + testDevices.StateFor(deviceName).Unlock() By("Creating LLDP resource") lldp = &v1alpha1.LLDP{ @@ -1285,9 +1285,9 @@ var _ = Describe("LLDP Controller", func() { It("Should recover when LLDP becomes operationally up", func() { By("Setting provider to return operational status down") - testProvider.Lock() - testProvider.LLDPOperStatus = false - testProvider.Unlock() + testDevices.StateFor(deviceName).Lock() + testDevices.StateFor(deviceName).LLDPOperStatus = false + testDevices.StateFor(deviceName).Unlock() By("Creating LLDP resource") lldp = &v1alpha1.LLDP{ @@ -1311,9 +1311,9 @@ var _ = Describe("LLDP Controller", func() { }).Should(Succeed()) By("Setting provider to return operational status up") - testProvider.Lock() - testProvider.LLDPOperStatus = true - testProvider.Unlock() + testDevices.StateFor(deviceName).Lock() + testDevices.StateFor(deviceName).LLDPOperStatus = true + testDevices.StateFor(deviceName).Unlock() By("Verifying OperationalCondition becomes True after requeue") Eventually(func(g Gomega) { diff --git a/internal/controller/core/managementaccess_controller_test.go b/internal/controller/core/managementaccess_controller_test.go index 04847bf1b..679662818 100644 --- a/internal/controller/core/managementaccess_controller_test.go +++ b/internal/controller/core/managementaccess_controller_test.go @@ -61,7 +61,7 @@ var _ = Describe("ManagementAccess Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Access).To(BeNil(), "Provider should not have ManagementAccess configured") + g.Expect(testDevices.StateFor(name).Access).To(BeNil(), "Provider should not have ManagementAccess configured") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -108,7 +108,7 @@ var _ = Describe("ManagementAccess Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Access).ToNot(BeNil(), "Provider should have ManagementAccess configured") + g.Expect(testDevices.StateFor(name).Access).ToNot(BeNil(), "Provider should have ManagementAccess configured") }).Should(Succeed()) }) }) diff --git a/internal/controller/core/ntp_controller_test.go b/internal/controller/core/ntp_controller_test.go index 4c7fa81e1..b43f52772 100644 --- a/internal/controller/core/ntp_controller_test.go +++ b/internal/controller/core/ntp_controller_test.go @@ -64,7 +64,7 @@ var _ = Describe("NTP Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.NTP).To(BeNil(), "Provider NTP should be nil") + g.Expect(testDevices.StateFor(name).NTP).To(BeNil(), "Provider NTP should be nil") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -111,9 +111,9 @@ var _ = Describe("NTP Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.NTP).ToNot(BeNil(), "Provider NTP should not be nil") - if testProvider.NTP != nil { - g.Expect(testProvider.NTP.Spec.SourceInterfaceName).To(Equal("mgmt0")) + g.Expect(testDevices.StateFor(name).NTP).ToNot(BeNil(), "Provider NTP should not be nil") + if testDevices.StateFor(name).NTP != nil { + g.Expect(testDevices.StateFor(name).NTP.Spec.SourceInterfaceName).To(Equal("mgmt0")) } }).Should(Succeed()) }) diff --git a/internal/controller/core/nve_controller_test.go b/internal/controller/core/nve_controller_test.go index bafef299c..241632f26 100644 --- a/internal/controller/core/nve_controller_test.go +++ b/internal/controller/core/nve_controller_test.go @@ -107,7 +107,7 @@ var _ = Describe("NVE Controller", func() { }).Should(BeTrue()) Eventually(func(g Gomega) { - g.Expect(testProvider.NVE).To(BeNil(), "Provider NVE should be empty") + g.Expect(testDevices.StateFor(name).NVE).To(BeNil(), "Provider NVE should be empty") }).Should(Succeed()) }) @@ -148,13 +148,13 @@ var _ = Describe("NVE Controller", func() { By("Ensuring the NVE is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.NVE).ToNot(BeNil(), "Provider NVE should not be nil") - g.Expect(testProvider.NVE.Spec.AdminState).To(BeEquivalentTo(v1alpha1.AdminStateUp)) - g.Expect(testProvider.NVE.Spec.SuppressARP).To(BeTrue()) - g.Expect(testProvider.NVE.Spec.HostReachability).To(BeEquivalentTo("BGP")) - g.Expect(testProvider.NVE.Spec.SourceInterfaceRef.Name).To(Equal(name + "-lo0")) - g.Expect(testProvider.NVE.Spec.MulticastGroups).ToNot(BeNil()) - g.Expect(testProvider.NVE.Spec.MulticastGroups.L2).To(HaveValue(Equal(v1alpha1.MustParsePrefix("234.0.0.0/8")))) + g.Expect(testDevices.StateFor(name).NVE).ToNot(BeNil(), "Provider NVE should not be nil") + g.Expect(testDevices.StateFor(name).NVE.Spec.AdminState).To(BeEquivalentTo(v1alpha1.AdminStateUp)) + g.Expect(testDevices.StateFor(name).NVE.Spec.SuppressARP).To(BeTrue()) + g.Expect(testDevices.StateFor(name).NVE.Spec.HostReachability).To(BeEquivalentTo("BGP")) + g.Expect(testDevices.StateFor(name).NVE.Spec.SourceInterfaceRef.Name).To(Equal(name + "-lo0")) + g.Expect(testDevices.StateFor(name).NVE.Spec.MulticastGroups).ToNot(BeNil()) + g.Expect(testDevices.StateFor(name).NVE.Spec.MulticastGroups.L2).To(HaveValue(Equal(v1alpha1.MustParsePrefix("234.0.0.0/8")))) }).Should(Succeed()) By("Verifying referenced interfaces exist and are loopbacks") @@ -275,7 +275,7 @@ var _ = Describe("NVE Controller", func() { }).Should(BeTrue()) Eventually(func(g Gomega) { - g.Expect(testProvider.NVE).To(BeNil(), "Provider NVE should be empty") + g.Expect(testDevices.StateFor(name).NVE).To(BeNil(), "Provider NVE should be empty") }).Should(Succeed()) }) @@ -286,9 +286,9 @@ var _ = Describe("NVE Controller", func() { By("Verifying reconciliation modifies provider and status") Eventually(func(g Gomega) { - g.Expect(testProvider.NVE).ToNot(BeNil()) - g.Expect(testProvider.NVE.Spec.SourceInterfaceRef.Name).To(Equal(name + "-lo1")) - g.Expect(testProvider.NVE.Status.SourceInterfaceName).To(Equal(name + "-lo1")) + g.Expect(testDevices.StateFor(name).NVE).ToNot(BeNil()) + g.Expect(testDevices.StateFor(name).NVE.Spec.SourceInterfaceRef.Name).To(Equal(name + "-lo1")) + g.Expect(testDevices.StateFor(name).NVE.Status.SourceInterfaceName).To(Equal(name + "-lo1")) }).Should(Succeed()) }) @@ -299,10 +299,10 @@ var _ = Describe("NVE Controller", func() { By("Verifying reconciliation modifies provider and status") Eventually(func(g Gomega) { - if testProvider.NVE != nil { - g.Expect(testProvider.NVE).ToNot(BeNil()) - g.Expect(testProvider.NVE.Spec.AnycastSourceInterfaceRef.Name).To(Equal(name + "-lo2")) - g.Expect(testProvider.NVE.Status.AnycastSourceInterfaceName).To(Equal(name + "-lo2")) + if testDevices.StateFor(name).NVE != nil { + g.Expect(testDevices.StateFor(name).NVE).ToNot(BeNil()) + g.Expect(testDevices.StateFor(name).NVE.Spec.AnycastSourceInterfaceRef.Name).To(Equal(name + "-lo2")) + g.Expect(testDevices.StateFor(name).NVE.Status.AnycastSourceInterfaceName).To(Equal(name + "-lo2")) } }, 5*time.Second, 100*time.Millisecond).Should(Succeed()) }) @@ -361,7 +361,7 @@ var _ = Describe("NVE Controller", func() { }).Should(BeTrue()) Eventually(func(g Gomega) { - g.Expect(testProvider.NVE).To(BeNil(), "Provider NVE should be empty") + g.Expect(testDevices.StateFor(name).NVE).To(BeNil(), "Provider NVE should be empty") }).Should(Succeed()) }) @@ -454,14 +454,14 @@ var _ = Describe("NVE Controller", func() { }).Should(BeTrue()) Eventually(func(g Gomega) { - g.Expect(testProvider.NVE).To(BeNil(), "Provider NVE should be empty") + g.Expect(testDevices.StateFor(name).NVE).To(BeNil(), "Provider NVE should be empty") }).Should(Succeed()) }) It("Should reconcile with nil anycast and empty status AnycastSourceInterfaceName", func() { Eventually(func(g Gomega) { - g.Expect(testProvider.NVE).NotTo(BeNil()) - g.Expect(testProvider.NVE.Spec.AnycastSourceInterfaceRef).To(BeNil()) + g.Expect(testDevices.StateFor(name).NVE).NotTo(BeNil()) + g.Expect(testDevices.StateFor(name).NVE.Spec.AnycastSourceInterfaceRef).To(BeNil()) }).Should(Succeed()) Eventually(func(g Gomega) { @@ -567,7 +567,7 @@ var _ = Describe("NVE Controller", func() { }).Should(BeTrue()) Eventually(func(g Gomega) { - g.Expect(testProvider.NVE).To(BeNil(), "Provider NVE should be empty") + g.Expect(testDevices.StateFor(name).NVE).To(BeNil(), "Provider NVE should be empty") }).Should(Succeed()) }) @@ -666,7 +666,7 @@ var _ = Describe("NVE Controller", func() { }).Should(BeTrue()) Eventually(func(g Gomega) { - g.Expect(testProvider.NVE).To(BeNil(), "Provider NVE should be empty") + g.Expect(testDevices.StateFor(name).NVE).To(BeNil(), "Provider NVE should be empty") }).Should(Succeed()) }) @@ -784,7 +784,7 @@ var _ = Describe("NVE Controller", func() { }).Should(BeTrue()) Eventually(func(g Gomega) { - g.Expect(testProvider.NVE).To(BeNil(), "Provider NVE should be empty") + g.Expect(testDevices.StateFor(name).NVE).To(BeNil(), "Provider NVE should be empty") }).Should(Succeed()) }) @@ -887,7 +887,7 @@ var _ = Describe("NVE Controller", func() { }).Should(BeTrue()) Eventually(func(g Gomega) { - g.Expect(testProvider.NVE).To(BeNil(), "Provider NVE should be empty") + g.Expect(testDevices.StateFor(name).NVE).To(BeNil(), "Provider NVE should be empty") }).Should(Succeed()) }) diff --git a/internal/controller/core/ospf_controller_test.go b/internal/controller/core/ospf_controller_test.go index e462a6c02..862cb5fe3 100644 --- a/internal/controller/core/ospf_controller_test.go +++ b/internal/controller/core/ospf_controller_test.go @@ -64,7 +64,7 @@ var _ = Describe("OSPF Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.OSPF.Has("UNDERLAY")).ToNot(BeTrue(), "Provider should not have OSPF instance configured") + g.Expect(testDevices.StateFor(name).OSPF.Has("UNDERLAY")).ToNot(BeTrue(), "Provider should not have OSPF instance configured") }).Should(Succeed()) By("Cleanup the Device resource") @@ -115,7 +115,7 @@ var _ = Describe("OSPF Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.OSPF.Has("UNDERLAY")).To(BeTrue(), "Provider should have OSPF instance configured") + g.Expect(testDevices.StateFor(name).OSPF.Has("UNDERLAY")).To(BeTrue(), "Provider should have OSPF instance configured") }).Should(Succeed()) }) diff --git a/internal/controller/core/pim_controller_test.go b/internal/controller/core/pim_controller_test.go index 8e32dc05d..6120a6736 100644 --- a/internal/controller/core/pim_controller_test.go +++ b/internal/controller/core/pim_controller_test.go @@ -57,7 +57,7 @@ var _ = Describe("PIM Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.PIM).To(BeNil(), "Provider should not have PIM instance configured") + g.Expect(testDevices.StateFor(name).PIM).To(BeNil(), "Provider should not have PIM instance configured") }).Should(Succeed()) By("Cleanup the Device resource") @@ -104,7 +104,7 @@ var _ = Describe("PIM Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.PIM).ToNot(BeNil(), "Provider should have PIM instance configured") + g.Expect(testDevices.StateFor(name).PIM).ToNot(BeNil(), "Provider should have PIM instance configured") }).Should(Succeed()) }) }) diff --git a/internal/controller/core/prefixset_controller_test.go b/internal/controller/core/prefixset_controller_test.go index 39784857d..1d2155f9a 100644 --- a/internal/controller/core/prefixset_controller_test.go +++ b/internal/controller/core/prefixset_controller_test.go @@ -68,7 +68,7 @@ var _ = Describe("PrefixSet Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.PrefixSets.Has(set)).To(BeFalse(), "Provider should not have PrefixSet configured") + g.Expect(testDevices.StateFor(name).PrefixSets.Has(set)).To(BeFalse(), "Provider should not have PrefixSet configured") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -115,7 +115,7 @@ var _ = Describe("PrefixSet Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.PrefixSets.Has(set)).To(BeTrue(), "Provider should have PrefixSet configured") + g.Expect(testDevices.StateFor(name).PrefixSets.Has(set)).To(BeTrue(), "Provider should have PrefixSet configured") }).Should(Succeed()) }) }) diff --git a/internal/controller/core/probe_controller_test.go b/internal/controller/core/probe_controller_test.go index 97a671fa1..2a35835e4 100644 --- a/internal/controller/core/probe_controller_test.go +++ b/internal/controller/core/probe_controller_test.go @@ -63,8 +63,8 @@ var _ = Describe("Probe Controller", func() { }) It("Should wait for its Device to become reachable", func() { - testProvider.SetConnectError(errors.New("device unreachable")) - DeferCleanup(func() { testProvider.SetConnectError(nil) }) + testDevices.StateFor(name).SetConnectFailure(errors.New("device unreachable")) + DeferCleanup(func() { testDevices.StateFor(name).SetConnectFailure(nil) }) Eventually(func(g Gomega) { device := &v1alpha1.Device{} @@ -99,7 +99,7 @@ var _ = Describe("Probe Controller", func() { ))) }).Should(Succeed()) - testProvider.SetConnectError(nil) + testDevices.StateFor(name).SetConnectFailure(nil) Eventually(func(g Gomega) { resource := &v1alpha1.Probe{} g.Expect(k8sClient.Get(ctx, key, resource)).To(Succeed()) diff --git a/internal/controller/core/routingpolicy_controller_test.go b/internal/controller/core/routingpolicy_controller_test.go index 4c0df2d63..c9ae23eb4 100644 --- a/internal/controller/core/routingpolicy_controller_test.go +++ b/internal/controller/core/routingpolicy_controller_test.go @@ -55,7 +55,7 @@ var _ = Describe("RoutingPolicy Controller", func() { By("Verifying the RoutingPolicy is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.RoutingPolicies.Has(name)).To(BeFalse(), "Provider shouldn't have RoutingPolicy configured anymore") + g.Expect(testDevices.StateFor(name).RoutingPolicies.Has(name)).To(BeFalse(), "Provider shouldn't have RoutingPolicy configured anymore") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -120,7 +120,7 @@ var _ = Describe("RoutingPolicy Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.RoutingPolicies.Has(name)).To(BeTrue(), "Provider should have RoutingPolicy configured") + g.Expect(testDevices.StateFor(name).RoutingPolicies.Has(name)).To(BeTrue(), "Provider should have RoutingPolicy configured") }).Should(Succeed()) }) @@ -187,7 +187,7 @@ var _ = Describe("RoutingPolicy Controller", func() { By("Verifying the RoutingPolicy is configured in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.RoutingPolicies.Has(name)).To(BeTrue(), "Provider should have RoutingPolicy configured") + g.Expect(testDevices.StateFor(name).RoutingPolicies.Has(name)).To(BeTrue(), "Provider should have RoutingPolicy configured") }).Should(Succeed()) }) @@ -332,7 +332,7 @@ var _ = Describe("RoutingPolicy Controller", func() { By("Verifying the RoutingPolicy is configured in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.RoutingPolicies.Has(name)).To(BeTrue(), "Provider should have RoutingPolicy configured") + g.Expect(testDevices.StateFor(name).RoutingPolicies.Has(name)).To(BeTrue(), "Provider should have RoutingPolicy configured") }).Should(Succeed()) }) diff --git a/internal/controller/core/snmp_controller_test.go b/internal/controller/core/snmp_controller_test.go index 52c4598e7..2105778c7 100644 --- a/internal/controller/core/snmp_controller_test.go +++ b/internal/controller/core/snmp_controller_test.go @@ -75,7 +75,7 @@ var _ = Describe("SNMP Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.SNMP).To(BeNil(), "Provider should not have SNMP configured") + g.Expect(testDevices.StateFor(name).SNMP).To(BeNil(), "Provider should not have SNMP configured") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -122,9 +122,9 @@ var _ = Describe("SNMP Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.SNMP).ToNot(BeNil(), "Provider should have SNMP configured") - if testProvider.SNMP != nil { - g.Expect(testProvider.SNMP.Spec.Contact).To(Equal("123")) + g.Expect(testDevices.StateFor(name).SNMP).ToNot(BeNil(), "Provider should have SNMP configured") + if testDevices.StateFor(name).SNMP != nil { + g.Expect(testDevices.StateFor(name).SNMP.Spec.Contact).To(Equal("123")) } }).Should(Succeed()) }) diff --git a/internal/controller/core/suite_test.go b/internal/controller/core/suite_test.go index 10679d5fe..dd3bc4728 100644 --- a/internal/controller/core/suite_test.go +++ b/internal/controller/core/suite_test.go @@ -42,13 +42,13 @@ import ( // http://onsi.github.io/ginkgo/ to learn more about Ginkgo. var ( - ctx context.Context - cancel context.CancelFunc - testEnv *envtest.Environment - k8sClient client.Client - k8sManager ctrl.Manager - testProvider = NewProvider() - testLocker *resourcelock.ResourceLocker + ctx context.Context + cancel context.CancelFunc + testEnv *envtest.Environment + k8sClient client.Client + k8sManager ctrl.Manager + testDevices = NewDeviceStore() + testLocker *resourcelock.ResourceLocker // testEvents is a slice that stores events recorded during the tests. It is used to verify that the expected events are generated by the controllers. testEvents []string testS3Store = NewMockObjectStorage() @@ -121,7 +121,7 @@ var _ = BeforeSuite(func() { _, err = k8sManager.GetCache().GetInformer(ctx, &coordinationv1.Lease{}) Expect(err).NotTo(HaveOccurred()) - provider.Register("test-provider", func() provider.Provider { return testProvider }) + provider.Register("test-provider", func() provider.Provider { return &Provider{devices: testDevices} }) err = (&DeviceReconciler{ Client: k8sManager.GetClient(), @@ -424,8 +424,9 @@ var ( _ provider.ProbeProvider = (*Provider)(nil) ) -// Provider is a simple in-memory provider for testing purposes only. -type Provider struct { +// DeviceState holds per-device mutable state for testing. Each device created in +// a test gets its own DeviceState, preventing state bleed between test suites. +type DeviceState struct { sync.Mutex ConnectError error // if non-nil, Connect returns this error @@ -466,8 +467,8 @@ type Provider struct { StorageTotal int64 } -func NewProvider() *Provider { - return &Provider{ +func NewDeviceState() *DeviceState { + return &DeviceState{ LastRebootTime: lastRebootTime, Ports: sets.New[string](), User: sets.New[string](), @@ -488,26 +489,63 @@ func NewProvider() *Provider { } } -// SetConnectError sets the error that Connect will return on subsequent calls. -// Pass nil to clear the error and allow connections to succeed. -func (p *Provider) SetConnectError(err error) { - p.Lock() - defer p.Unlock() - p.ConnectError = err +// DeviceStore holds per-device state for all test devices, keyed by device name. +type DeviceStore struct { + sync.Mutex + states map[string]*DeviceState } -// SetLastRebootTime sets the time returned by GetLastRebootTime on subsequent calls. -func (p *Provider) SetLastRebootTime(t time.Time) { - p.Lock() - defer p.Unlock() - p.LastRebootTime = t +func NewDeviceStore() *DeviceStore { + return &DeviceStore{states: make(map[string]*DeviceState)} } -func (p *Provider) Connect(_ context.Context, _ *deviceutil.Connection) error { - p.Lock() - defer p.Unlock() - return p.ConnectError +// StateFor returns the DeviceState for the given device name, creating one if it doesn't exist. +func (s *DeviceStore) StateFor(name string) *DeviceState { + s.Lock() + defer s.Unlock() + ds, ok := s.states[name] + if !ok { + ds = NewDeviceState() + s.states[name] = ds + } + return ds } + +// Provider is an in-memory provider for testing. The ProviderFunc factory creates +// a new Provider per reconcile call, sharing the DeviceStore. Connect captures the +// device name from the Connection so subsequent method calls dispatch to the correct +// DeviceState. +type Provider struct { + devices *DeviceStore + deviceName string // set during Connect +} + +// SetConnectFailure sets the error that Connect will return for this device. +func (s *DeviceState) SetConnectFailure(err error) { + s.Lock() + defer s.Unlock() + s.ConnectError = err +} + +// SetLastRebootTime sets the time returned by GetLastRebootTime for this device. +func (s *DeviceState) SetLastRebootTime(t time.Time) { + s.Lock() + defer s.Unlock() + s.LastRebootTime = t +} + +func (p *Provider) Connect(_ context.Context, conn *deviceutil.Connection) error { + s := p.devices.StateFor(conn.DeviceName) + s.Lock() + err := s.ConnectError + s.Unlock() + if err != nil { + return err + } + p.deviceName = conn.DeviceName + return nil +} + func (p *Provider) Disconnect(context.Context, *deviceutil.Connection) error { return nil } func (p *Provider) ListPorts(context.Context) (ports []provider.DevicePort, err error) { @@ -523,9 +561,10 @@ func (p *Provider) ListPorts(context.Context) (ports []provider.DevicePort, err } func (p *Provider) GetLastRebootTime(_ context.Context) (time.Time, error) { - p.Lock() - defer p.Unlock() - return p.LastRebootTime, nil + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + return s.LastRebootTime, nil } func (p *Provider) GetDeviceInfo(context.Context) (*provider.DeviceInfo, error) { @@ -545,58 +584,58 @@ func (p *Provider) VerifyProvisioned(context.Context, *deviceutil.Connection, *v return true } -func (p *Provider) Reboot(ctx context.Context, conn *deviceutil.Connection) error { +func (p *Provider) Reboot(context.Context, *deviceutil.Connection) error { return nil } -func (p *Provider) FactoryReset(ctx context.Context, conn *deviceutil.Connection) error { +func (p *Provider) FactoryReset(context.Context, *deviceutil.Connection) error { return nil } func (p *Provider) UpgradeFirmware(ctx context.Context, conn *deviceutil.Connection, target provider.TargetFirmware) error { - p.Lock() - defer p.Unlock() - return p.UpgradeError + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + return s.UpgradeError } // SetUpgradeError sets the error that UpgradeFirmware returns on subsequent // calls. Pass nil to clear it. func (p *Provider) SetUpgradeError(err error) { - p.Lock() - defer p.Unlock() - p.UpgradeError = err + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.UpgradeError = err } -func (p *Provider) Reprovision(ctx context.Context, conn *deviceutil.Connection) (reterr error) { +func (p *Provider) Reprovision(context.Context, *deviceutil.Connection) error { return nil } -func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInterfaceRequest) error { - p.Lock() - defer p.Unlock() - p.Ports.Insert(req.Interface.Spec.Name) +func (p *Provider) EnsureInterface(_ context.Context, req *provider.EnsureInterfaceRequest) error { + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.Ports.Insert(req.Interface.Spec.Name) return nil } func (p *Provider) DeleteInterface(_ context.Context, req *provider.DeleteInterfaceRequest) error { - p.Lock() - defer p.Unlock() - p.Ports.Delete(req.Interface.Spec.Name) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.Ports.Delete(req.Interface.Spec.Name) return nil } func (p *Provider) GetInterfaceStatus(_ context.Context, req *provider.InterfaceRequest) (provider.InterfaceStatus, error) { - p.Lock() - defer p.Unlock() - - status := provider.InterfaceStatus{ - OperStatus: true, - } - - if neighbor, ok := p.LLDPNeighbors[req.Interface.Spec.Name]; ok { + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + status := provider.InterfaceStatus{OperStatus: true} + if neighbor, ok := s.LLDPNeighbors[req.Interface.Spec.Name]; ok { status.LLDPAdjacencies = []provider.LLDPAdjacency{*neighbor} } - return status, nil } @@ -609,13 +648,14 @@ func (p *Provider) LoopbackInterfaceName(id int) (string, error) { } func (p *Provider) EnsureBanner(_ context.Context, req *provider.EnsureBannerRequest) error { - p.Lock() - defer p.Unlock() + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() switch req.Type { case v1alpha1.BannerTypePreLogin: - p.PreLoginBanner = &req.Message + s.PreLoginBanner = &req.Message case v1alpha1.BannerTypePostLogin: - p.PostLoginBanner = &req.Message + s.PostLoginBanner = &req.Message default: return errors.New("unknown banner type") } @@ -623,13 +663,14 @@ func (p *Provider) EnsureBanner(_ context.Context, req *provider.EnsureBannerReq } func (p *Provider) DeleteBanner(_ context.Context, req *provider.DeleteBannerRequest) error { - p.Lock() - defer p.Unlock() + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() switch req.Type { case v1alpha1.BannerTypePreLogin: - p.PreLoginBanner = nil + s.PreLoginBanner = nil case v1alpha1.BannerTypePostLogin: - p.PostLoginBanner = nil + s.PostLoginBanner = nil default: return errors.New("unknown banner type") } @@ -637,186 +678,212 @@ func (p *Provider) DeleteBanner(_ context.Context, req *provider.DeleteBannerReq } func (p *Provider) EnsureUser(_ context.Context, req *provider.EnsureUserRequest) error { - p.Lock() - defer p.Unlock() - p.User.Insert(req.Username) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.User.Insert(req.Username) return nil } func (p *Provider) DeleteUser(_ context.Context, req *provider.DeleteUserRequest) error { - p.Lock() - defer p.Unlock() - p.User.Delete(req.Username) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.User.Delete(req.Username) return nil } func (p *Provider) EnsureDNS(_ context.Context, req *provider.EnsureDNSRequest) error { - p.Lock() - defer p.Unlock() - p.DNS = req.DNS + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.DNS = req.DNS return nil } func (p *Provider) DeleteDNS(_ context.Context) error { - p.Lock() - defer p.Unlock() - p.DNS = nil + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.DNS = nil return nil } func (p *Provider) EnsureNTP(_ context.Context, req *provider.EnsureNTPRequest) error { - p.Lock() - defer p.Unlock() - p.NTP = req.NTP + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.NTP = req.NTP return nil } func (p *Provider) DeleteNTP(context.Context) error { - p.Lock() - defer p.Unlock() - p.NTP = nil + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.NTP = nil return nil } func (p *Provider) EnsureACL(_ context.Context, req *provider.ACLRequest) error { - p.Lock() - defer p.Unlock() - p.ACLs.Insert(req.ACL.Spec.Name) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.ACLs.Insert(req.ACL.Spec.Name) return nil } func (p *Provider) DeleteACL(_ context.Context, req *provider.DeleteACLRequest) error { - p.Lock() - defer p.Unlock() - p.ACLs.Delete(req.ACL.Spec.Name) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.ACLs.Delete(req.ACL.Spec.Name) return nil } func (p *Provider) EnsureCertificate(_ context.Context, req *provider.EnsureCertificateRequest) error { - p.Lock() - defer p.Unlock() - p.Certs.Insert(req.ID) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.Certs.Insert(req.ID) return nil } func (p *Provider) DeleteCertificate(_ context.Context, req *provider.DeleteCertificateRequest) error { - p.Lock() - defer p.Unlock() - p.Certs.Delete(req.ID) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.Certs.Delete(req.ID) return nil } func (p *Provider) EnsureSNMP(_ context.Context, req *provider.EnsureSNMPRequest) error { - p.Lock() - defer p.Unlock() - p.SNMP = req.SNMP + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.SNMP = req.SNMP return nil } func (p *Provider) DeleteSNMP(context.Context) error { - p.Lock() - defer p.Unlock() - p.SNMP = nil + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.SNMP = nil return nil } func (p *Provider) EnsureSyslog(_ context.Context, req *provider.EnsureSyslogRequest) error { - p.Lock() - defer p.Unlock() - p.Syslog = req.Syslog + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.Syslog = req.Syslog return nil } func (p *Provider) DeleteSyslog(_ context.Context) error { - p.Lock() - defer p.Unlock() - p.Syslog = nil + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.Syslog = nil return nil } func (p *Provider) EnsureManagementAccess(_ context.Context, req *provider.EnsureManagementAccessRequest) error { - p.Lock() - defer p.Unlock() - p.Access = req.ManagementAccess + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.Access = req.ManagementAccess return nil } func (p *Provider) DeleteManagementAccess(context.Context, *provider.DeleteManagementAccessRequest) error { - p.Lock() - defer p.Unlock() - p.Access = nil + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.Access = nil return nil } func (p *Provider) EnsureISIS(_ context.Context, req *provider.EnsureISISRequest) error { - p.Lock() - defer p.Unlock() - p.ISIS.Insert(req.ISIS.Spec.Instance) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.ISIS.Insert(req.ISIS.Spec.Instance) return nil } func (p *Provider) DeleteISIS(_ context.Context, req *provider.DeleteISISRequest) error { - p.Lock() - defer p.Unlock() - p.ISIS.Delete(req.ISIS.Spec.Instance) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.ISIS.Delete(req.ISIS.Spec.Instance) return nil } func (p *Provider) EnsureVRF(_ context.Context, req *provider.VRFRequest) error { - p.Lock() - defer p.Unlock() - p.VRF.Insert(req.VRF.Spec.Name) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.VRF.Insert(req.VRF.Spec.Name) return nil } func (p *Provider) DeleteVRF(_ context.Context, req *provider.DeleteVRFRequest) error { - p.Lock() - defer p.Unlock() - p.VRF.Delete(req.VRF.Spec.Name) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.VRF.Delete(req.VRF.Spec.Name) return nil } func (p *Provider) EnsurePIM(_ context.Context, req *provider.EnsurePIMRequest) error { - p.Lock() - defer p.Unlock() - p.PIM = req.PIM + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.PIM = req.PIM return nil } func (p *Provider) DeletePIM(context.Context) error { - p.Lock() - defer p.Unlock() - p.PIM = nil + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.PIM = nil return nil } func (p *Provider) EnsureBGP(_ context.Context, req *provider.EnsureBGPRequest) error { - p.Lock() - defer p.Unlock() - p.BGP = req.BGP - p.BGPVRF = req.VRF + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.BGP = req.BGP + s.BGPVRF = req.VRF return nil } func (p *Provider) DeleteBGP(context.Context, *provider.DeleteBGPRequest) error { - p.Lock() - defer p.Unlock() - p.BGP = nil - p.BGPVRF = nil + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.BGP = nil + s.BGPVRF = nil return nil } func (p *Provider) EnsureBGPPeer(_ context.Context, req *provider.EnsureBGPPeerRequest) error { - p.Lock() - defer p.Unlock() - p.BGPPeers.Insert(req.BGPPeer.Spec.Address) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.BGPPeers.Insert(req.BGPPeer.Spec.Address) return nil } func (p *Provider) DeleteBGPPeer(_ context.Context, req *provider.DeleteBGPPeerRequest) error { - p.Lock() - defer p.Unlock() - p.BGPPeers.Delete(req.BGPPeer.Spec.Address) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.BGPPeers.Delete(req.BGPPeer.Spec.Address) return nil } @@ -834,112 +901,120 @@ func (p *Provider) GetPeerStatus(context.Context, *provider.BGPPeerStatusRequest } func (p *Provider) EnsureOSPF(_ context.Context, req *provider.EnsureOSPFRequest) error { - p.Lock() - defer p.Unlock() - p.OSPF.Insert(req.OSPF.Spec.Instance) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.OSPF.Insert(req.OSPF.Spec.Instance) return nil } func (p *Provider) DeleteOSPF(_ context.Context, req *provider.DeleteOSPFRequest) error { - p.Lock() - defer p.Unlock() - p.OSPF.Delete(req.OSPF.Spec.Instance) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.OSPF.Delete(req.OSPF.Spec.Instance) return nil } func (p *Provider) GetOSPFStatus(context.Context, *provider.OSPFStatusRequest) (provider.OSPFStatus, error) { - return provider.OSPFStatus{ - OperStatus: true, - }, nil + return provider.OSPFStatus{OperStatus: true}, nil } func (p *Provider) EnsureVLAN(_ context.Context, req *provider.VLANRequest) error { - p.Lock() - defer p.Unlock() - p.VLANs.Insert(req.VLAN.Spec.ID) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.VLANs.Insert(req.VLAN.Spec.ID) return nil } func (p *Provider) DeleteVLAN(_ context.Context, req *provider.DeleteVLANRequest) error { - p.Lock() - defer p.Unlock() - p.VLANs.Delete(req.VLAN.Spec.ID) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.VLANs.Delete(req.VLAN.Spec.ID) return nil } func (p *Provider) GetVLANStatus(context.Context, *provider.VLANRequest) (provider.VLANStatus, error) { - return provider.VLANStatus{ - OperStatus: true, - }, nil + return provider.VLANStatus{OperStatus: true}, nil } func (p *Provider) EnsureEVPNInstance(_ context.Context, req *provider.EVPNInstanceRequest) error { - p.Lock() - defer p.Unlock() - p.EVIs.Insert(req.EVPNInstance.Spec.VNI) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.EVIs.Insert(req.EVPNInstance.Spec.VNI) return nil } func (p *Provider) DeleteEVPNInstance(_ context.Context, req *provider.DeleteEVPNInstanceRequest) error { - p.Lock() - defer p.Unlock() - p.EVIs.Delete(req.EVPNInstance.Spec.VNI) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.EVIs.Delete(req.EVPNInstance.Spec.VNI) return nil } -// EnsurePrefixSet implements provider.PrefixSetProvider. func (p *Provider) EnsurePrefixSet(_ context.Context, req *provider.PrefixSetRequest) error { - p.Lock() - defer p.Unlock() - p.PrefixSets.Insert(req.PrefixSet.Spec.Name) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.PrefixSets.Insert(req.PrefixSet.Spec.Name) return nil } func (p *Provider) DeletePrefixSet(_ context.Context, req *provider.DeletePrefixSetRequest) error { - p.Lock() - defer p.Unlock() - p.PrefixSets.Delete(req.PrefixSet.Spec.Name) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.PrefixSets.Delete(req.PrefixSet.Spec.Name) return nil } func (p *Provider) EnsureRoutingPolicy(_ context.Context, req *provider.EnsureRoutingPolicyRequest) error { - p.Lock() - defer p.Unlock() - p.RoutingPolicies.Insert(req.Name) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.RoutingPolicies.Insert(req.Name) return nil } func (p *Provider) DeleteRoutingPolicy(_ context.Context, req *provider.DeleteRoutingPolicyRequest) error { - p.Lock() - defer p.Unlock() - p.RoutingPolicies.Delete(req.Name) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.RoutingPolicies.Delete(req.Name) return nil } func (p *Provider) EnsureNVE(_ context.Context, req *provider.NVERequest) error { - p.Lock() - defer p.Unlock() - p.NVE = req.NVE + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.NVE = req.NVE return nil } func (p *Provider) DeleteNVE(context.Context) error { - p.Lock() - defer p.Unlock() - p.NVE = nil + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.NVE = nil return nil } func (p *Provider) GetNVEStatus(_ context.Context, _ *provider.NVERequest) (provider.NVEStatus, error) { - status := provider.NVEStatus{ - OperStatus: true, - } - if p.NVE != nil { - if p.NVE.Spec.SourceInterfaceRef.Name != "" { - status.SourceInterfaceName = p.NVE.Spec.SourceInterfaceRef.Name + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + status := provider.NVEStatus{OperStatus: true} + if s.NVE != nil { + if s.NVE.Spec.SourceInterfaceRef.Name != "" { + status.SourceInterfaceName = s.NVE.Spec.SourceInterfaceRef.Name } - if p.NVE.Spec.AnycastSourceInterfaceRef != nil { - status.AnycastSourceInterfaceName = p.NVE.Spec.AnycastSourceInterfaceRef.Name + if s.NVE.Spec.AnycastSourceInterfaceRef != nil { + status.AnycastSourceInterfaceName = s.NVE.Spec.AnycastSourceInterfaceRef.Name } } return status, nil @@ -950,122 +1025,132 @@ func (p *Provider) RunningConfig(context.Context) ([]byte, error) { } func (p *Provider) CreateConfigBackup(_ context.Context, req *provider.ConfigBackupRequest) (*provider.ConfigBackupFile, error) { - p.Lock() - defer p.Unlock() - + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() if req.ConfigBackup.Spec.Type == v1alpha1.ConfigBackupTypeStartup { - p.StartupConfig = req.ConfigBackup + s.StartupConfig = req.ConfigBackup return nil, nil //nolint:nilnil } - file := &provider.ConfigBackupFile{ Path: path.Join(req.ConfigBackup.Spec.Path, fmt.Sprintf("configbackup-%s-", req.ConfigBackup.UID)) + time.Now().Format("20060102T150405Z"), SizeBytes: new(int64(1024)), CreatedAt: time.Now(), } - p.ConfigBackups = append(p.ConfigBackups, file) + s.ConfigBackups = append(s.ConfigBackups, file) return file, nil } func (p *Provider) ListConfigBackups(_ context.Context, _ *provider.ConfigBackupRequest) (*provider.ConfigBackupInventory, error) { - p.Lock() - defer p.Unlock() + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() var used int64 - for _, f := range p.ConfigBackups { + for _, f := range s.ConfigBackups { if f.SizeBytes != nil { used += *f.SizeBytes } } return &provider.ConfigBackupInventory{ - Backups: p.ConfigBackups, - TotalBytes: &p.StorageTotal, + Backups: s.ConfigBackups, + TotalBytes: &s.StorageTotal, UsedBytes: &used, - FreeBytes: new(p.StorageTotal - used), + FreeBytes: new(s.StorageTotal - used), }, nil } func (p *Provider) DeleteConfigBackups(_ context.Context, files ...*provider.ConfigBackupFile) error { - p.Lock() - defer p.Unlock() + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() remove := make(map[string]struct{}, len(files)) for _, f := range files { remove[f.Path] = struct{}{} } - filtered := p.ConfigBackups[:0] - for _, b := range p.ConfigBackups { + filtered := s.ConfigBackups[:0] + for _, b := range s.ConfigBackups { if _, ok := remove[b.Path]; !ok { filtered = append(filtered, b) } } - p.ConfigBackups = filtered + s.ConfigBackups = filtered return nil } func (p *Provider) EnsureLLDP(_ context.Context, req *provider.LLDPRequest) error { - p.Lock() - defer p.Unlock() - p.LLDP = req.LLDP + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.LLDP = req.LLDP return nil } func (p *Provider) DeleteLLDP(context.Context) error { - p.Lock() - defer p.Unlock() - p.LLDP = nil + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.LLDP = nil return nil } func (p *Provider) GetLLDPStatus(_ context.Context, _ *provider.LLDPRequest) (provider.LLDPStatus, error) { - p.Lock() - defer p.Unlock() - return provider.LLDPStatus{OperStatus: p.LLDPOperStatus}, nil + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + return provider.LLDPStatus{OperStatus: s.LLDPOperStatus}, nil } func (p *Provider) EnsureDHCPRelay(_ context.Context, req *provider.DHCPRelayRequest) error { - p.Lock() - defer p.Unlock() - p.DHCPRelay = req.DHCPRelay + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.DHCPRelay = req.DHCPRelay return nil } func (p *Provider) DeleteDHCPRelay(context.Context, *provider.DeleteDHCPRelayRequest) error { - p.Lock() - defer p.Unlock() - p.DHCPRelayDeleteCalls++ - p.DHCPRelay = nil + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + s.DHCPRelay = nil + s.DHCPRelayDeleteCalls++ return nil } func (p *Provider) EnsureEthernetSegment(_ context.Context, req *provider.EnsureEthernetSegmentRequest) error { - p.Lock() - defer p.Unlock() + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() esi := req.EthernetSegment.Spec.ESI if esi == "" { // Simulate auto-generated ESI (Type 3 MAC-based) esi = "03:aa:bb:cc:dd:ee:ff:00:00:01" } - p.EthernetSegments[req.EthernetSegment.Name] = esi + s.EthernetSegments[req.EthernetSegment.Name] = esi return nil } func (p *Provider) DeleteEthernetSegment(_ context.Context, req *provider.DeleteEthernetSegmentRequest) error { - p.Lock() - defer p.Unlock() - delete(p.EthernetSegments, req.EthernetSegment.Name) + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + delete(s.EthernetSegments, req.EthernetSegment.Name) return nil } func (p *Provider) GetEthernetSegmentStatus(_ context.Context, req *provider.EthernetSegmentStatusRequest) (provider.EthernetSegmentStatus, error) { - p.Lock() - defer p.Unlock() - esi := p.EthernetSegments[req.EthernetSegment.Name] + s := p.devices.StateFor(p.deviceName) + s.Lock() + defer s.Unlock() + esi := s.EthernetSegments[req.EthernetSegment.Name] return provider.EthernetSegmentStatus{ESI: esi, OperStatus: esi != ""}, nil } -func (p *Provider) GetEthernetSegment(name string) (string, bool) { - p.Lock() - defer p.Unlock() - esi, ok := p.EthernetSegments[name] +// GetEthernetSegment is a test helper to retrieve the ESI for an ethernet segment by name. +func (s *DeviceState) GetEthernetSegment(name string) (string, bool) { + s.Lock() + defer s.Unlock() + esi, ok := s.EthernetSegments[name] return esi, ok } @@ -1092,10 +1177,10 @@ func (p *Provider) GetVTEPPeers(context.Context, *provider.VTEPPeersRequest) ([] } // SetLLDPNeighbor is a test helper to configure LLDP neighbor information for an interface. -func (p *Provider) SetLLDPNeighbor(interfaceName, sysName, chassisID, portID string, ttl uint32) { - p.Lock() - defer p.Unlock() - p.LLDPNeighbors[interfaceName] = &provider.LLDPAdjacency{ +func (s *DeviceState) SetLLDPNeighbor(interfaceName, sysName, chassisID, portID string, ttl uint32) { + s.Lock() + defer s.Unlock() + s.LLDPNeighbors[interfaceName] = &provider.LLDPAdjacency{ SysName: sysName, ChassisID: chassisID, ChassisIDType: 4, // MACAddress diff --git a/internal/controller/core/syslog_controller_test.go b/internal/controller/core/syslog_controller_test.go index 25fcb7e2b..ea89df9f7 100644 --- a/internal/controller/core/syslog_controller_test.go +++ b/internal/controller/core/syslog_controller_test.go @@ -70,7 +70,7 @@ var _ = Describe("Syslog Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Syslog).To(BeNil(), "Provider should not have Syslog configured") + g.Expect(testDevices.StateFor(name).Syslog).To(BeNil(), "Provider should not have Syslog configured") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -117,7 +117,7 @@ var _ = Describe("Syslog Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.Syslog).NotTo(BeNil(), "Provider should have Syslog configured") + g.Expect(testDevices.StateFor(name).Syslog).NotTo(BeNil(), "Provider should have Syslog configured") }).Should(Succeed()) }) }) diff --git a/internal/controller/core/user_controller_test.go b/internal/controller/core/user_controller_test.go index 8b816c012..b67617476 100644 --- a/internal/controller/core/user_controller_test.go +++ b/internal/controller/core/user_controller_test.go @@ -78,7 +78,7 @@ var _ = Describe("User Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.User.Has(username)).To(BeFalse(), "User should not exist") + g.Expect(testDevices.StateFor(name).User.Has(username)).To(BeFalse(), "User should not exist") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -125,7 +125,7 @@ var _ = Describe("User Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.User.Has(username)).To(BeTrue(), "User should exist") + g.Expect(testDevices.StateFor(name).User.Has(username)).To(BeTrue(), "User should exist") }).Should(Succeed()) }) }) diff --git a/internal/controller/core/vlan_controller_test.go b/internal/controller/core/vlan_controller_test.go index acfe2e2ba..0fe72d350 100644 --- a/internal/controller/core/vlan_controller_test.go +++ b/internal/controller/core/vlan_controller_test.go @@ -60,7 +60,7 @@ var _ = Describe("VLAN Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.VLANs.Has(id)).To(BeFalse(), "Provider VLAN should not exist") + g.Expect(testDevices.StateFor(name).VLANs.Has(id)).To(BeFalse(), "Provider VLAN should not exist") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -111,7 +111,7 @@ var _ = Describe("VLAN Controller", func() { By("Ensuring the resource is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.VLANs.Has(id)).To(BeTrue(), "Provider VLAN should exist") + g.Expect(testDevices.StateFor(name).VLANs.Has(id)).To(BeTrue(), "Provider VLAN should exist") }).Should(Succeed()) }) }) diff --git a/internal/controller/core/vrf_controller_test.go b/internal/controller/core/vrf_controller_test.go index 5e5c30ab6..c002bd121 100644 --- a/internal/controller/core/vrf_controller_test.go +++ b/internal/controller/core/vrf_controller_test.go @@ -79,7 +79,7 @@ var _ = Describe("VRF Controller", func() { By("Verifying the resource is removed from the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.VRF.Has("CC-ADMIN-TEST")).To(BeFalse(), "Provider should not have VRF configured anymore") + g.Expect(testDevices.StateFor(name).VRF.Has("CC-ADMIN-TEST")).To(BeFalse(), "Provider should not have VRF configured anymore") }).Should(Succeed()) By("Cleaning up the Device resource") @@ -122,9 +122,9 @@ var _ = Describe("VRF Controller", func() { By("Ensuring the VRF is created in the provider") Eventually(func(g Gomega) { - g.Expect(testProvider.VRF).ToNot(BeNil(), "Provider VRF should not be nil") - if testProvider.VRF != nil { - g.Expect(testProvider.VRF.Has("CC-ADMIN-TEST")).To(BeTrue(), "Provider should have VRF configured") + g.Expect(testDevices.StateFor(name).VRF).ToNot(BeNil(), "Provider VRF should not be nil") + if testDevices.StateFor(name).VRF != nil { + g.Expect(testDevices.StateFor(name).VRF.Has("CC-ADMIN-TEST")).To(BeTrue(), "Provider should have VRF configured") } }).Should(Succeed()) }) diff --git a/internal/deviceutil/deviceutil.go b/internal/deviceutil/deviceutil.go index 44bbb36dd..dac8f3e89 100644 --- a/internal/deviceutil/deviceutil.go +++ b/internal/deviceutil/deviceutil.go @@ -107,6 +107,8 @@ func GetDeviceBySerial(ctx context.Context, r client.Reader, serial string) (*v1 // Connection holds the necessary information to connect to a device's API. type Connection struct { + // DeviceName is the name of the Device object this connection belongs to. + DeviceName string // Address is the API address of the device, in the format "host:port". Address string // Username for basic authentication. Might be empty if the device does not require authentication. @@ -156,9 +158,10 @@ func GetDeviceConnection(ctx context.Context, r client.Reader, obj *v1alpha1.Dev } return &Connection{ - Address: obj.Spec.Endpoint.Address, - Username: string(user), - Password: string(pass), - TLS: conf, + DeviceName: obj.Name, + Address: obj.Spec.Endpoint.Address, + Username: string(user), + Password: string(pass), + TLS: conf, }, nil } From 1ea8f9aa9a6baf5862f50d29248cda3a89e0a5af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Mon, 14 Sep 2026 15:19:22 +0200 Subject: [PATCH 3/9] Wait for resource deletion before Device in AfterEach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../cisco/nx/bordergateway_controller_test.go | 7 + .../cisco/nx/system_controller_test.go | 7 + .../cisco/nx/vpcdomain_controller_test.go | 25 +++ .../controller/core/acl_controller_test.go | 7 + .../controller/core/banner_controller_test.go | 7 + .../controller/core/bgp_controller_test.go | 27 +++- .../core/bgp_peer_controller_test.go | 30 +++- .../core/certificate_controller_test.go | 7 + .../core/configbackup_controller_test.go | 11 +- .../controller/core/device_controller_test.go | 21 ++- .../dhcprelay_controller_deprecated_test.go | 12 ++ .../core/dhcprelay_controller_test.go | 150 ++++++++++++++---- .../controller/core/dns_controller_test.go | 7 + .../core/ethernetsegment_controller_test.go | 54 +++++-- .../core/evpninstance_controller_test.go | 9 ++ .../core/interface_controller_test.go | 37 +++-- .../controller/core/isis_controller_test.go | 13 ++ .../controller/core/lldp_controller_test.go | 24 +++ .../core/managementaccess_controller_test.go | 7 + .../controller/core/ntp_controller_test.go | 7 + .../controller/core/ospf_controller_test.go | 22 ++- .../controller/core/pim_controller_test.go | 11 ++ .../core/prefixset_controller_test.go | 7 + .../core/routingpolicy_controller_test.go | 13 ++ .../controller/core/snmp_controller_test.go | 7 + .../controller/core/syslog_controller_test.go | 7 + .../controller/core/user_controller_test.go | 7 + .../controller/core/vlan_controller_test.go | 7 + .../controller/core/vrf_controller_test.go | 10 +- 29 files changed, 481 insertions(+), 79 deletions(-) diff --git a/internal/controller/cisco/nx/bordergateway_controller_test.go b/internal/controller/cisco/nx/bordergateway_controller_test.go index b2df80e5f..1f2dbb29b 100644 --- a/internal/controller/cisco/nx/bordergateway_controller_test.go +++ b/internal/controller/cisco/nx/bordergateway_controller_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -77,6 +78,12 @@ var _ = Describe("BorderGateway Controller", func() { bg.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, bg))).To(Succeed()) + By("Waiting for BorderGateway to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &nxv1alpha1.BorderGateway{}) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Ensuring the resource is deleted from the provider") Eventually(func(g Gomega) { g.Expect(testProvider.BorderGateway).To(BeNil(), "Provider BorderGateway settings should be reset after deletion") diff --git a/internal/controller/cisco/nx/system_controller_test.go b/internal/controller/cisco/nx/system_controller_test.go index ddb5dc1e6..70d6dacc8 100644 --- a/internal/controller/cisco/nx/system_controller_test.go +++ b/internal/controller/cisco/nx/system_controller_test.go @@ -6,6 +6,7 @@ package nx import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -58,6 +59,12 @@ var _ = Describe("System Controller", func() { system.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, system))).To(Succeed()) + By("Waiting for System to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &nxv1alpha1.System{}) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Ensuring the resource is deleted from the provider") Eventually(func(g Gomega) { g.Expect(testProvider.Settings).To(BeNil(), "Provider System settings should be reset after deletion") diff --git a/internal/controller/cisco/nx/vpcdomain_controller_test.go b/internal/controller/cisco/nx/vpcdomain_controller_test.go index 682ea1a38..0ba4844e2 100644 --- a/internal/controller/cisco/nx/vpcdomain_controller_test.go +++ b/internal/controller/cisco/nx/vpcdomain_controller_test.go @@ -6,6 +6,7 @@ package nx import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -115,6 +116,12 @@ var _ = Describe("VPCDomain Controller", func() { By("Cleanup the specific resource instance VPCDomain") Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + By("Waiting for VPCDomain to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, vpcdomainKey, &nxv1.VPCDomain{}) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Ensuring the resource is deleted from the provider") Eventually(func(g Gomega) { g.Expect(testProvider.VPCDomain).To(BeNil(), "Provider VPCDomain should be nil") @@ -293,6 +300,12 @@ var _ = Describe("VPCDomain Controller", func() { By("Cleanup the VPCDomain") Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + By("Waiting for VPCDomain to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, vpcdomainKey, &nxv1.VPCDomain{}) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleanup Interface and VRF resources") for _, ifName := range []string{name + "-phys", name + "-po", name + "-phys-b", name + "-po-b", name + "-lo0"} { intf := &corev1.Interface{} @@ -300,12 +313,24 @@ var _ = Describe("VPCDomain Controller", func() { intf.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, intf))).To(Succeed()) } + Eventually(func(g Gomega) { + for _, ifName := range []string{name + "-phys", name + "-po", name + "-phys-b", name + "-po-b", name + "-lo0"} { + err := k8sClient.Get(ctx, client.ObjectKey{Name: ifName, Namespace: metav1.NamespaceDefault}, &corev1.Interface{}) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + } + }).Should(Succeed()) for _, vrfName := range []string{name + "-vrf-a", name + "-vrf-b"} { vrf := &corev1.VRF{} vrf.Name = vrfName vrf.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, vrf))).To(Succeed()) } + Eventually(func(g Gomega) { + for _, vrfName := range []string{name + "-vrf-a", name + "-vrf-b"} { + err := k8sClient.Get(ctx, client.ObjectKey{Name: vrfName, Namespace: metav1.NamespaceDefault}, &corev1.VRF{}) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + } + }).Should(Succeed()) By("Ensuring the resource is deleted from the provider") Eventually(func(g Gomega) { diff --git a/internal/controller/core/acl_controller_test.go b/internal/controller/core/acl_controller_test.go index ed0275e2a..2ac68c5e2 100644 --- a/internal/controller/core/acl_controller_test.go +++ b/internal/controller/core/acl_controller_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -80,6 +81,12 @@ var _ = Describe("AccessControlList Controller", func() { g.Expect(testDevices.StateFor(name).ACLs.Has(name)).To(BeFalse(), "Provider shouldn't have AccessControlList configured anymore") }).Should(Succeed()) + By("Waiting for the AccessControlList to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.AccessControlList{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device := &v1alpha1.Device{} device.Name = name diff --git a/internal/controller/core/banner_controller_test.go b/internal/controller/core/banner_controller_test.go index d10c72207..e01c6d640 100644 --- a/internal/controller/core/banner_controller_test.go +++ b/internal/controller/core/banner_controller_test.go @@ -6,6 +6,7 @@ package core import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -50,6 +51,12 @@ var _ = Describe("Banner Controller", func() { g.Expect(testDevices.StateFor(name).PostLoginBanner).To(BeNil(), "Provider PostLogin Banner should be nil") }).Should(Succeed()) + By("Waiting for the Banner to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.Banner{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device := &v1alpha1.Device{} device.Name = name diff --git a/internal/controller/core/bgp_controller_test.go b/internal/controller/core/bgp_controller_test.go index daf722187..ff78edebc 100644 --- a/internal/controller/core/bgp_controller_test.go +++ b/internal/controller/core/bgp_controller_test.go @@ -35,31 +35,44 @@ var _ = Describe("BGP Controller", func() { }) AfterEach(func() { + // Use the manager client for MatchingFields queries — the direct k8sClient + // does not have the custom field indexes registered on the API server. By("Cleaning up BGP resources for this device") bgpList := &v1alpha1.BGPList{} - Expect(k8sClient.List(ctx, bgpList, client.InNamespace(metav1.NamespaceDefault), client.MatchingLabels{v1alpha1.DeviceLabel: device.Name})).To(Succeed()) + Expect(k8sManager.GetClient().List(ctx, bgpList, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: device.Name})).To(Succeed()) for i := range bgpList.Items { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &bgpList.Items[i]))).To(Succeed()) } + By("Waiting for BGP resources to be fully deleted") + Eventually(func(g Gomega) { + list := &v1alpha1.BGPList{} + g.Expect(k8sManager.GetClient().List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: device.Name})).To(Succeed()) + g.Expect(list.Items).To(BeEmpty()) + }).Should(Succeed()) By("Cleaning up VRF resources for this device") vrfList := &v1alpha1.VRFList{} - Expect(k8sClient.List(ctx, vrfList, client.InNamespace(metav1.NamespaceDefault), client.MatchingLabels{v1alpha1.DeviceLabel: device.Name})).To(Succeed()) + Expect(k8sManager.GetClient().List(ctx, vrfList, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: device.Name})).To(Succeed()) for i := range vrfList.Items { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &vrfList.Items[i]))).To(Succeed()) } + By("Waiting for VRF resources to be fully deleted") + Eventually(func(g Gomega) { + list := &v1alpha1.VRFList{} + g.Expect(k8sManager.GetClient().List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: device.Name})).To(Succeed()) + g.Expect(list.Items).To(BeEmpty()) + }).Should(Succeed()) By("Cleaning up RoutingPolicy resources for this device") rpList := &v1alpha1.RoutingPolicyList{} - Expect(k8sClient.List(ctx, rpList, client.InNamespace(metav1.NamespaceDefault), client.MatchingLabels{v1alpha1.DeviceLabel: device.Name})).To(Succeed()) + Expect(k8sManager.GetClient().List(ctx, rpList, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: device.Name})).To(Succeed()) for i := range rpList.Items { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &rpList.Items[i]))).To(Succeed()) } - - By("Waiting for BGP resources to be fully deleted") + By("Waiting for RoutingPolicy resources to be fully deleted") Eventually(func(g Gomega) { - list := &v1alpha1.BGPList{} - g.Expect(k8sClient.List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingLabels{v1alpha1.DeviceLabel: device.Name})).To(Succeed()) + list := &v1alpha1.RoutingPolicyList{} + g.Expect(k8sManager.GetClient().List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: device.Name})).To(Succeed()) g.Expect(list.Items).To(BeEmpty()) }).Should(Succeed()) diff --git a/internal/controller/core/bgp_peer_controller_test.go b/internal/controller/core/bgp_peer_controller_test.go index f937c7080..6ef7d90d1 100644 --- a/internal/controller/core/bgp_peer_controller_test.go +++ b/internal/controller/core/bgp_peer_controller_test.go @@ -36,31 +36,44 @@ var _ = Describe("BGPPeer Controller", func() { }) AfterEach(func() { + // Use the manager client for MatchingFields queries — the direct k8sClient + // does not have the custom field indexes registered on the API server. By("Cleaning up BGPPeer resources for this device") peerList := &v1alpha1.BGPPeerList{} - Expect(k8sClient.List(ctx, peerList, client.InNamespace(metav1.NamespaceDefault), client.MatchingLabels{v1alpha1.DeviceLabel: device.Name})).To(Succeed()) + Expect(k8sManager.GetClient().List(ctx, peerList, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: device.Name})).To(Succeed()) for i := range peerList.Items { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &peerList.Items[i]))).To(Succeed()) } + By("Waiting for BGPPeer resources to be fully deleted") + Eventually(func(g Gomega) { + list := &v1alpha1.BGPPeerList{} + g.Expect(k8sManager.GetClient().List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: device.Name})).To(Succeed()) + g.Expect(list.Items).To(BeEmpty()) + }).Should(Succeed()) By("Cleaning up BGP resources for this device") bgpList := &v1alpha1.BGPList{} - Expect(k8sClient.List(ctx, bgpList, client.InNamespace(metav1.NamespaceDefault), client.MatchingLabels{v1alpha1.DeviceLabel: device.Name})).To(Succeed()) + Expect(k8sManager.GetClient().List(ctx, bgpList, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: device.Name})).To(Succeed()) for i := range bgpList.Items { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &bgpList.Items[i]))).To(Succeed()) } + By("Waiting for BGP resources to be fully deleted") + Eventually(func(g Gomega) { + list := &v1alpha1.BGPList{} + g.Expect(k8sManager.GetClient().List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: device.Name})).To(Succeed()) + g.Expect(list.Items).To(BeEmpty()) + }).Should(Succeed()) By("Cleaning up Interface resources for this device") intfList := &v1alpha1.InterfaceList{} - Expect(k8sClient.List(ctx, intfList, client.InNamespace(metav1.NamespaceDefault), client.MatchingLabels{v1alpha1.DeviceLabel: device.Name})).To(Succeed()) + Expect(k8sManager.GetClient().List(ctx, intfList, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: device.Name})).To(Succeed()) for i := range intfList.Items { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &intfList.Items[i]))).To(Succeed()) } - - By("Waiting for BGPPeer resources to be fully deleted") + By("Waiting for Interface resources to be fully deleted") Eventually(func(g Gomega) { - list := &v1alpha1.BGPPeerList{} - g.Expect(k8sClient.List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingLabels{v1alpha1.DeviceLabel: device.Name})).To(Succeed()) + list := &v1alpha1.InterfaceList{} + g.Expect(k8sManager.GetClient().List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: device.Name})).To(Succeed()) g.Expect(list.Items).To(BeEmpty()) }).Should(Succeed()) @@ -313,6 +326,9 @@ var _ = Describe("BGPPeer Controller", func() { }, } Expect(k8sClient.Create(ctx, intf)).To(Succeed()) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, intf))).To(Succeed()) + }) By("Creating a BGPPeer resource with LocalAddress pointing to the cross-device Interface") bgppeer := &v1alpha1.BGPPeer{ diff --git a/internal/controller/core/certificate_controller_test.go b/internal/controller/core/certificate_controller_test.go index 87b9e2964..d0e06a86d 100644 --- a/internal/controller/core/certificate_controller_test.go +++ b/internal/controller/core/certificate_controller_test.go @@ -17,6 +17,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -93,6 +94,12 @@ var _ = Describe("Certificate Controller", func() { g.Expect(testDevices.StateFor(name).Certs.Has("cert1")).To(BeFalse(), "Certificate should be deleted from the provider") }).Should(Succeed()) + By("Waiting for the Certificate to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.Certificate{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device := &v1alpha1.Device{} device.Name = name diff --git a/internal/controller/core/configbackup_controller_test.go b/internal/controller/core/configbackup_controller_test.go index 2e2b8505f..0c8721979 100644 --- a/internal/controller/core/configbackup_controller_test.go +++ b/internal/controller/core/configbackup_controller_test.go @@ -14,6 +14,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -62,11 +63,17 @@ var _ = Describe("ConfigBackup Controller", func() { AfterEach(func() { if backup != nil { By("Deleting the ConfigBackup resource") - Expect(k8sClient.Delete(ctx, backup)).To(Succeed()) + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, backup))).To(Succeed()) + + By("Waiting for ConfigBackup to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(backup), &v1alpha1.ConfigBackup{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) backup = nil } By("Deleting the Device resource") - Expect(k8sClient.Delete(ctx, device)).To(Succeed()) + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, device))).To(Succeed()) }) It("Should successfully reconcile a one-shot local backup", func() { diff --git a/internal/controller/core/device_controller_test.go b/internal/controller/core/device_controller_test.go index 84868a37d..c1fe0ae7d 100644 --- a/internal/controller/core/device_controller_test.go +++ b/internal/controller/core/device_controller_test.go @@ -10,6 +10,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -47,6 +48,12 @@ var _ = Describe("Device Controller", func() { device.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, device))).To(Succeed()) + By("Waiting for Device to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKey{Name: name, Namespace: metav1.NamespaceDefault}, &v1alpha1.Device{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleanup the specific resource instance Secret") secret := &corev1.Secret{} secret.Name = name @@ -140,6 +147,10 @@ var _ = Describe("Device Controller", func() { err := k8sClient.Get(ctx, key, intf) Expect(err).NotTo(HaveOccurred()) Expect(k8sClient.Delete(ctx, intf)).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.Interface{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) }) It("Should transition from Pending to Provisioning when provisioning is configured", func() { @@ -752,8 +763,8 @@ var _ = Describe("Device Controller", func() { It("Should skip provisioning and transition from Pending to Running when skip-provisioning annotation is set", func() { By("Creating a Device with provisioning configured and skip-provisioning annotation") device := &v1alpha1.Device{ - GenerateName: name, - Namespace: metav1.NamespaceDefault, + Name: name, + Namespace: metav1.NamespaceDefault, Annotations: map[string]string{ v1alpha1.DeviceMaintenanceAnnotation: v1alpha1.DeviceMaintenanceSkipProvisioning, }, @@ -778,7 +789,6 @@ var _ = Describe("Device Controller", func() { }, } Expect(k8sClient.Create(ctx, device)).To(Succeed()) - key := client.ObjectKeyFromObject(device) By("Verifying the device reaches Running phase with annotation removed and SkipProvisioning event emitted") Eventually(func(g Gomega) { @@ -795,8 +805,8 @@ var _ = Describe("Device Controller", func() { It("Should close active provisioning entry and transition to Running when skip-provisioning annotation is set during Provisioning phase", func() { By("Creating a Device with provisioning configured") device := &v1alpha1.Device{ - GenerateName: name, - Namespace: metav1.NamespaceDefault, + Name: name, + Namespace: metav1.NamespaceDefault, Spec: v1alpha1.DeviceSpec{ Provider: "test-provider", Endpoint: v1alpha1.Endpoint{ @@ -818,7 +828,6 @@ var _ = Describe("Device Controller", func() { }, } Expect(k8sClient.Create(ctx, device)).To(Succeed()) - key := client.ObjectKeyFromObject(device) By("Waiting for the device to enter Provisioning phase") Eventually(func(g Gomega) { diff --git a/internal/controller/core/dhcprelay_controller_deprecated_test.go b/internal/controller/core/dhcprelay_controller_deprecated_test.go index 9ba690abe..1a5943607 100644 --- a/internal/controller/core/dhcprelay_controller_deprecated_test.go +++ b/internal/controller/core/dhcprelay_controller_deprecated_test.go @@ -20,8 +20,12 @@ var _ = Describe("DHCPRelay Controller with deprecated API fields", func() { It("Should reconcile interfaceRefs", func() { device := &v1alpha1.Device{GenerateName: "test-dhcprelay-deprecated-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.DeviceSpec{Endpoint: v1alpha1.Endpoint{Address: "192.168.20.50:9339"}, Provider: "test-provider"}} Expect(k8sClient.Create(ctx, device)).To(Succeed()) + deviceKey := client.ObjectKeyFromObject(device) DeferCleanup(func() { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, device))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, deviceKey, &v1alpha1.Device{}))).To(BeTrue()) + }).Should(Succeed()) }) Eventually(func(g Gomega) { g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(device), device)).To(Succeed()) @@ -30,8 +34,12 @@ var _ = Describe("DHCPRelay Controller with deprecated API fields", func() { vlan := &v1alpha1.VLAN{GenerateName: "test-dhcprelay-deprecated-vlan-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.VLANSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: device.Name}, ID: 70, Name: "vlan70"}} Expect(k8sClient.Create(ctx, vlan)).To(Succeed()) + vlanKey := client.ObjectKeyFromObject(vlan) DeferCleanup(func() { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, vlan))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, vlanKey, &v1alpha1.VLAN{}))).To(BeTrue()) + }).Should(Succeed()) }) intf := &v1alpha1.Interface{ @@ -42,8 +50,12 @@ var _ = Describe("DHCPRelay Controller with deprecated API fields", func() { }, } Expect(k8sClient.Create(ctx, intf)).To(Succeed()) + intfKey := client.ObjectKeyFromObject(intf) DeferCleanup(func() { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, intf))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, intfKey, &v1alpha1.Interface{}))).To(BeTrue()) + }).Should(Succeed()) }) Eventually(func(g Gomega) { diff --git a/internal/controller/core/dhcprelay_controller_test.go b/internal/controller/core/dhcprelay_controller_test.go index 0ebd67518..f7e5d15fd 100644 --- a/internal/controller/core/dhcprelay_controller_test.go +++ b/internal/controller/core/dhcprelay_controller_test.go @@ -257,6 +257,9 @@ var _ = Describe("DHCPRelay Controller", func() { vrfKey := client.ObjectKey{Name: vrf.Name, Namespace: metav1.NamespaceDefault} defer func() { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, vrf))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, vrfKey, &v1alpha1.VRF{}))).To(BeTrue()) + }).Should(Succeed()) }() By("Waiting for VRF to be ready") @@ -379,8 +382,12 @@ var _ = Describe("DHCPRelay Controller", func() { }, } Expect(k8sClient.Create(ctx, otherVLAN)).To(Succeed()) + otherVLANKey := client.ObjectKeyFromObject(otherVLAN) defer func() { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherVLAN))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, otherVLANKey, &v1alpha1.VLAN{}))).To(BeTrue()) + }).Should(Succeed()) }() otherInterface := &v1alpha1.Interface{ @@ -398,12 +405,15 @@ var _ = Describe("DHCPRelay Controller", func() { }, } Expect(k8sClient.Create(ctx, otherInterface)).To(Succeed()) + otherInterfaceKey := client.ObjectKeyFromObject(otherInterface) defer func() { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherInterface))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, otherInterfaceKey, &v1alpha1.Interface{}))).To(BeTrue()) + }).Should(Succeed()) }() By("Waiting for the second Interface to be configured") - otherInterfaceKey := client.ObjectKeyFromObject(otherInterface) Eventually(func(g Gomega) { g.Expect(k8sClient.Get(ctx, otherInterfaceKey, otherInterface)).To(Succeed()) cond := meta.FindStatusCondition(otherInterface.Status.Conditions, v1alpha1.ConfiguredCondition) @@ -698,24 +708,6 @@ var _ = Describe("DHCPRelay Controller", func() { deviceKey client.ObjectKey ) - cleanupObject := func(object client.Object) { - DeferCleanup(func() { - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, object))).To(Succeed()) - }) - } - - cleanupDHCPRelay := func(dhcprelay *v1alpha1.DHCPRelay) { - resourceKey := client.ObjectKeyFromObject(dhcprelay) - DeferCleanup(func() { - By("Cleaning up the DHCPRelay resource") - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, dhcprelay))).To(Succeed()) - Eventually(func(g Gomega) { - err := k8sClient.Get(ctx, resourceKey, &v1alpha1.DHCPRelay{}) - g.Expect(errors.IsNotFound(err)).To(BeTrue()) - }).Should(Succeed()) - }) - } - BeforeEach(func() { By("Creating the Device resource") device := &v1alpha1.Device{ @@ -729,6 +721,9 @@ var _ = Describe("DHCPRelay Controller", func() { By("Cleaning up the Device resource") device := &v1alpha1.Device{Name: deviceKey.Name, Namespace: deviceKey.Namespace} Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, device))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, deviceKey, &v1alpha1.Device{}))).To(BeTrue()) + }).Should(Succeed()) }) }) @@ -739,7 +734,14 @@ var _ = Describe("DHCPRelay Controller", func() { Spec: v1alpha1.DHCPRelaySpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, InterfaceRef: &v1alpha1.LocalObjectReference{Name: "non-existent-interface"}, Servers: []string{"192.168.1.1"}}, } Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) - cleanupDHCPRelay(dhcprelay) + resourceKey := client.ObjectKeyFromObject(dhcprelay) + DeferCleanup(func() { + By("Cleaning up the DHCPRelay resource") + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, dhcprelay))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, resourceKey, &v1alpha1.DHCPRelay{}))).To(BeTrue()) + }).Should(Succeed()) + }) By("Verifying the controller sets ConfiguredCondition to False with WaitingForDependenciesReason") Eventually(func(g Gomega) { @@ -755,22 +757,47 @@ var _ = Describe("DHCPRelay Controller", func() { By("Creating another Device resource") otherDevice := &v1alpha1.Device{GenerateName: "test-dhcprelay-crossdev-other-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.DeviceSpec{Endpoint: v1alpha1.Endpoint{Address: "192.168.10.53:9339"}, Provider: "test-provider"}} Expect(k8sClient.Create(ctx, otherDevice)).To(Succeed()) - cleanupObject(otherDevice) + otherDeviceKey := client.ObjectKeyFromObject(otherDevice) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherDevice))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, otherDeviceKey, &v1alpha1.Device{}))).To(BeTrue()) + }).Should(Succeed()) + }) By("Creating a VLAN on the other Device") otherVLAN := &v1alpha1.VLAN{GenerateName: "test-dhcprelay-crossdev-vlan-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.VLANSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: otherDevice.Name}, ID: 20, Name: "vlan20"}} Expect(k8sClient.Create(ctx, otherVLAN)).To(Succeed()) - cleanupObject(otherVLAN) + otherVLANKey := client.ObjectKeyFromObject(otherVLAN) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherVLAN))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, otherVLANKey, &v1alpha1.VLAN{}))).To(BeTrue()) + }).Should(Succeed()) + }) By("Creating an Interface on the other Device") otherInterface := &v1alpha1.Interface{GenerateName: "test-dhcprelay-crossdev-intf-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.InterfaceSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: otherDevice.Name}, Name: "vlan20", Type: v1alpha1.InterfaceTypeRoutedVLAN, VlanRef: &v1alpha1.LocalObjectReference{Name: otherVLAN.Name}, AdminState: v1alpha1.AdminStateUp, IPv4: &v1alpha1.InterfaceIPv4{Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.1.1/24")}}}}} Expect(k8sClient.Create(ctx, otherInterface)).To(Succeed()) - cleanupObject(otherInterface) + otherInterfaceKey := client.ObjectKeyFromObject(otherInterface) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherInterface))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, otherInterfaceKey, &v1alpha1.Interface{}))).To(BeTrue()) + }).Should(Succeed()) + }) By("Creating DHCPRelay referencing an Interface from a different device") dhcprelay := &v1alpha1.DHCPRelay{GenerateName: "test-dhcprelay-crossdev-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.DHCPRelaySpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, InterfaceRef: &v1alpha1.LocalObjectReference{Name: otherInterface.Name}, Servers: []string{"192.168.1.1"}}} Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) - cleanupDHCPRelay(dhcprelay) + resourceKey := client.ObjectKeyFromObject(dhcprelay) + DeferCleanup(func() { + By("Cleaning up the DHCPRelay resource") + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, dhcprelay))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, resourceKey, &v1alpha1.DHCPRelay{}))).To(BeTrue()) + }).Should(Succeed()) + }) By("Verifying the controller sets ConfiguredCondition to False with CrossDeviceReferenceReason") Eventually(func(g Gomega) { @@ -786,17 +813,35 @@ var _ = Describe("DHCPRelay Controller", func() { By("Creating another Device resource") otherDevice := &v1alpha1.Device{GenerateName: "test-dhcprelay-vrfcross-other-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.DeviceSpec{Endpoint: v1alpha1.Endpoint{Address: "192.168.10.58:9339"}, Provider: "test-provider"}} Expect(k8sClient.Create(ctx, otherDevice)).To(Succeed()) - cleanupObject(otherDevice) + otherDeviceKey := client.ObjectKeyFromObject(otherDevice) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherDevice))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, otherDeviceKey, &v1alpha1.Device{}))).To(BeTrue()) + }).Should(Succeed()) + }) By("Creating a VLAN on the main Device") vlan := &v1alpha1.VLAN{GenerateName: "test-dhcprelay-vrfcross-vlan-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.VLANSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, ID: 60, Name: "vlan60"}} Expect(k8sClient.Create(ctx, vlan)).To(Succeed()) - cleanupObject(vlan) + vlanKey := client.ObjectKeyFromObject(vlan) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, vlan))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, vlanKey, &v1alpha1.VLAN{}))).To(BeTrue()) + }).Should(Succeed()) + }) By("Creating an Interface on the main Device") intf := &v1alpha1.Interface{GenerateName: "test-dhcprelay-vrfcross-intf-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.InterfaceSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, Name: "vlan60", Type: v1alpha1.InterfaceTypeRoutedVLAN, VlanRef: &v1alpha1.LocalObjectReference{Name: vlan.Name}, AdminState: v1alpha1.AdminStateUp, IPv4: &v1alpha1.InterfaceIPv4{Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.6.1/24")}}}}} Expect(k8sClient.Create(ctx, intf)).To(Succeed()) - cleanupObject(intf) + intfKey := client.ObjectKeyFromObject(intf) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, intf))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, intfKey, &v1alpha1.Interface{}))).To(BeTrue()) + }).Should(Succeed()) + }) By("Waiting for Interface to be configured") interfaceKey := client.ObjectKeyFromObject(intf) @@ -810,12 +855,25 @@ var _ = Describe("DHCPRelay Controller", func() { By("Creating a VRF on the other Device") otherVRF := &v1alpha1.VRF{GenerateName: "test-dhcprelay-vrfcross-vrf-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.VRFSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: otherDevice.Name}, Name: "VRF-OTHER"}} Expect(k8sClient.Create(ctx, otherVRF)).To(Succeed()) - cleanupObject(otherVRF) + otherVRFKey := client.ObjectKeyFromObject(otherVRF) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, otherVRF))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, otherVRFKey, &v1alpha1.VRF{}))).To(BeTrue()) + }).Should(Succeed()) + }) By("Creating DHCPRelay with a VRF from a different device") dhcprelay := &v1alpha1.DHCPRelay{GenerateName: "test-dhcprelay-vrfcross-new-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.DHCPRelaySpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, InterfaceRef: &v1alpha1.LocalObjectReference{Name: intf.Name}, VrfRef: &v1alpha1.LocalObjectReference{Name: otherVRF.Name}, Servers: []string{"192.168.1.1"}}} Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) - cleanupDHCPRelay(dhcprelay) + resourceKey := client.ObjectKeyFromObject(dhcprelay) + DeferCleanup(func() { + By("Cleaning up the DHCPRelay resource") + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, dhcprelay))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, resourceKey, &v1alpha1.DHCPRelay{}))).To(BeTrue()) + }).Should(Succeed()) + }) By("Verifying the controller sets ConfiguredCondition to False with CrossDeviceReferenceReason") Eventually(func(g Gomega) { @@ -833,12 +891,24 @@ var _ = Describe("DHCPRelay Controller", func() { By("Creating the VLAN resource") vlan := &v1alpha1.VLAN{GenerateName: "test-dhcprelay-intfnr-vlan-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.VLANSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, ID: 40, Name: "vlan40", AdminState: v1alpha1.AdminStateUp}} Expect(k8sClient.Create(ctx, vlan)).To(Succeed()) - cleanupObject(vlan) + vlanKey := client.ObjectKeyFromObject(vlan) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, vlan))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, vlanKey, &v1alpha1.VLAN{}))).To(BeTrue()) + }).Should(Succeed()) + }) By("Creating an Interface resource with a VRF reference to a non-existent VRF") intf := &v1alpha1.Interface{GenerateName: "test-dhcprelay-intfnr-intf-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.InterfaceSpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, Name: "vlan40", AdminState: v1alpha1.AdminStateUp, Type: v1alpha1.InterfaceTypeRoutedVLAN, VlanRef: &v1alpha1.LocalObjectReference{Name: vlan.Name}, VrfRef: &v1alpha1.LocalObjectReference{Name: nonExistentVrfName}, IPv4: &v1alpha1.InterfaceIPv4{Addresses: []v1alpha1.IPPrefix{{Prefix: netip.MustParsePrefix("10.0.4.1/24")}}}}} Expect(k8sClient.Create(ctx, intf)).To(Succeed()) - cleanupObject(intf) + intfKey := client.ObjectKeyFromObject(intf) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, intf))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, intfKey, &v1alpha1.Interface{}))).To(BeTrue()) + }).Should(Succeed()) + }) By("Verifying the Interface is NOT Ready") interfaceKey := client.ObjectKeyFromObject(intf) @@ -852,7 +922,14 @@ var _ = Describe("DHCPRelay Controller", func() { By("Creating DHCPRelay referencing a non-configured Interface") dhcprelay := &v1alpha1.DHCPRelay{GenerateName: "test-dhcprelay-intfnr-", Namespace: metav1.NamespaceDefault, Spec: v1alpha1.DHCPRelaySpec{DeviceRef: v1alpha1.LocalObjectReference{Name: deviceName}, InterfaceRef: &v1alpha1.LocalObjectReference{Name: intf.Name}, Servers: []string{"192.168.1.1"}}} Expect(k8sClient.Create(ctx, dhcprelay)).To(Succeed()) - cleanupDHCPRelay(dhcprelay) + resourceKey := client.ObjectKeyFromObject(dhcprelay) + DeferCleanup(func() { + By("Cleaning up the DHCPRelay resource") + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, dhcprelay))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, resourceKey, &v1alpha1.DHCPRelay{}))).To(BeTrue()) + }).Should(Succeed()) + }) By("Verifying the controller sets ConfiguredCondition to False with WaitingForDependenciesReason") Eventually(func(g Gomega) { @@ -966,12 +1043,18 @@ var _ = Describe("DHCPRelay Controller", func() { i.Name = interfaceKey.Name i.Namespace = interfaceKey.Namespace Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, i))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, interfaceKey, &v1alpha1.Interface{}))).To(BeTrue()) + }).Should(Succeed()) By("Cleaning up the VLAN resource") vlan := &v1alpha1.VLAN{} vlan.Name = vlanKey.Name vlan.Namespace = vlanKey.Namespace Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, vlan))).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, vlanKey, &v1alpha1.VLAN{}))).To(BeTrue()) + }).Should(Succeed()) By("Cleaning up the Device resource") device := &v1alpha1.Device{} @@ -1038,6 +1121,9 @@ var _ = Describe("DHCPRelay Controller", func() { By("Cleaning up the VRF resource") Expect(k8sClient.Delete(ctx, vrf)).To(Succeed()) + Eventually(func(g Gomega) { + g.Expect(errors.IsNotFound(k8sClient.Get(ctx, client.ObjectKeyFromObject(vrf), &v1alpha1.VRF{}))).To(BeTrue()) + }).Should(Succeed()) }) }) }) diff --git a/internal/controller/core/dns_controller_test.go b/internal/controller/core/dns_controller_test.go index 9ee858805..86870ae1f 100644 --- a/internal/controller/core/dns_controller_test.go +++ b/internal/controller/core/dns_controller_test.go @@ -6,6 +6,7 @@ package core import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -66,6 +67,12 @@ var _ = Describe("DNS Controller", func() { g.Expect(testDevices.StateFor(name).DNS).To(BeNil(), "Provider DNS should be nil") }).Should(Succeed()) + By("Waiting for the DNS to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.DNS{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device := &v1alpha1.Device{} device.Name = name diff --git a/internal/controller/core/ethernetsegment_controller_test.go b/internal/controller/core/ethernetsegment_controller_test.go index 0db06ce67..e1f837da0 100644 --- a/internal/controller/core/ethernetsegment_controller_test.go +++ b/internal/controller/core/ethernetsegment_controller_test.go @@ -6,6 +6,7 @@ package core import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -17,8 +18,9 @@ var _ = Describe("EthernetSegment Controller", func() { Context("When reconciling a resource", func() { const esi = "00:11:22:33:44:55:66:77:88:01" var ( - name string - key client.ObjectKey + name string + key client.ObjectKey + memberIntf *v1alpha1.Interface ) BeforeEach(func() { @@ -43,6 +45,19 @@ var _ = Describe("EthernetSegment Controller", func() { g.Expect(k8sClient.Get(ctx, key, d)).To(Succeed()) g.Expect(d.Status.Phase).To(Equal(v1alpha1.DevicePhaseRunning)) }).Should(Succeed()) + + By("Creating a Physical member interface for Aggregate references") + memberIntf = &v1alpha1.Interface{ + GenerateName: "test-es-member-", + Namespace: metav1.NamespaceDefault, + Spec: v1alpha1.InterfaceSpec{ + DeviceRef: v1alpha1.LocalObjectReference{Name: name}, + Name: "Ethernet1/1", + AdminState: v1alpha1.AdminStateUp, + Type: v1alpha1.InterfaceTypePhysical, + }, + } + Expect(k8sClient.Create(ctx, memberIntf)).To(Succeed()) }) AfterEach(func() { @@ -52,17 +67,31 @@ var _ = Describe("EthernetSegment Controller", func() { es.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, es))).To(Succeed()) + By("Waiting for EthernetSegment resource to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKey{Name: name, Namespace: metav1.NamespaceDefault}, &v1alpha1.EthernetSegment{}) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Verifying the EthernetSegment is removed from the provider") Eventually(func(g Gomega) { _, exists := testDevices.StateFor(name).GetEthernetSegment(name) g.Expect(exists).To(BeFalse(), "Provider shouldn't have ESI configured anymore") }).Should(Succeed()) - By("Cleaning up test Interface resource") - intf := &v1alpha1.Interface{} - intf.Name = name - intf.Namespace = metav1.NamespaceDefault - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, intf))).To(Succeed()) + By("Cleaning up Interface resources for this device") + list := &v1alpha1.InterfaceList{} + Expect(k8sManager.GetClient().List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: name})).To(Succeed()) + for i := range list.Items { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &list.Items[i]))).To(Succeed()) + } + + By("Waiting for Interface resources to be fully deleted") + Eventually(func(g Gomega) { + list := &v1alpha1.InterfaceList{} + g.Expect(k8sManager.GetClient().List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: name})).To(Succeed()) + g.Expect(list.Items).To(BeEmpty()) + }).Should(Succeed()) By("Cleaning up the test Device resource") device := &v1alpha1.Device{} @@ -85,7 +114,7 @@ var _ = Describe("EthernetSegment Controller", func() { Mode: v1alpha1.SwitchportModeTrunk, }, Aggregation: &v1alpha1.Aggregation{ - MemberInterfaceRefs: []v1alpha1.LocalObjectReference{{Name: "eth1"}}, + MemberInterfaceRefs: []v1alpha1.LocalObjectReference{{Name: memberIntf.Name}}, ControlProtocol: v1alpha1.ControlProtocol{Mode: v1alpha1.LACPModeActive}, }, }, @@ -205,12 +234,15 @@ var _ = Describe("EthernetSegment Controller", func() { Mode: v1alpha1.SwitchportModeTrunk, }, Aggregation: &v1alpha1.Aggregation{ - MemberInterfaceRefs: []v1alpha1.LocalObjectReference{{Name: "eth1"}}, + MemberInterfaceRefs: []v1alpha1.LocalObjectReference{{Name: memberIntf.Name}}, ControlProtocol: v1alpha1.ControlProtocol{Mode: v1alpha1.LACPModeActive}, }, }, } Expect(k8sClient.Create(ctx, intf)).To(Succeed()) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, intf))).To(Succeed()) + }) By("Creating an EthernetSegment referencing the cross-device Interface") es := &v1alpha1.EthernetSegment{ @@ -300,7 +332,7 @@ var _ = Describe("EthernetSegment Controller", func() { Type: v1alpha1.InterfaceTypeAggregate, AdminState: v1alpha1.AdminStateUp, Aggregation: &v1alpha1.Aggregation{ - MemberInterfaceRefs: []v1alpha1.LocalObjectReference{{Name: "eth1"}}, + MemberInterfaceRefs: []v1alpha1.LocalObjectReference{{Name: memberIntf.Name}}, ControlProtocol: v1alpha1.ControlProtocol{Mode: v1alpha1.LACPModeActive}, }, }, @@ -350,7 +382,7 @@ var _ = Describe("EthernetSegment Controller", func() { Mode: v1alpha1.SwitchportModeTrunk, }, Aggregation: &v1alpha1.Aggregation{ - MemberInterfaceRefs: []v1alpha1.LocalObjectReference{{Name: "eth1"}}, + MemberInterfaceRefs: []v1alpha1.LocalObjectReference{{Name: memberIntf.Name}}, ControlProtocol: v1alpha1.ControlProtocol{Mode: v1alpha1.LACPModeActive}, }, }, diff --git a/internal/controller/core/evpninstance_controller_test.go b/internal/controller/core/evpninstance_controller_test.go index 679e7a271..4a9abc18f 100644 --- a/internal/controller/core/evpninstance_controller_test.go +++ b/internal/controller/core/evpninstance_controller_test.go @@ -6,6 +6,7 @@ package core import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -44,12 +45,20 @@ var _ = Describe("EVPNInstance Controller", func() { evi.Name = name evi.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, evi))).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.EVPNInstance{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) By("Cleaning up test VLAN resource") vlan := &v1alpha1.VLAN{} vlan.Name = name vlan.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, vlan))).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.VLAN{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) By("Verifying the EVPNInstance is removed from the provider") Eventually(func(g Gomega) { diff --git a/internal/controller/core/interface_controller_test.go b/internal/controller/core/interface_controller_test.go index fb6dd9a98..bf2aa968f 100644 --- a/internal/controller/core/interface_controller_test.go +++ b/internal/controller/core/interface_controller_test.go @@ -46,18 +46,15 @@ var _ = Describe("Interface Controller", func() { AfterEach(func() { By("Cleaning up Interface resources for this device") interfaces := &v1alpha1.InterfaceList{} - Expect(k8sClient.List(ctx, interfaces, client.InNamespace(metav1.NamespaceDefault))).To(Succeed()) + Expect(k8sManager.GetClient().List(ctx, interfaces, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: name})).To(Succeed()) for i := range interfaces.Items { - deviceName := interfaces.Items[i].Spec.DeviceRef.Name - if deviceName == name || deviceName == "different-device" || deviceName == "non-existing-device" { - Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &interfaces.Items[i]))).To(Succeed()) - } + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &interfaces.Items[i]))).To(Succeed()) } By("Waiting for Interfaces to be fully deleted") Eventually(func(g Gomega) { list := &v1alpha1.InterfaceList{} - g.Expect(k8sClient.List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingLabels{v1alpha1.DeviceLabel: name})).To(Succeed()) + g.Expect(k8sManager.GetClient().List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: name})).To(Succeed()) g.Expect(list.Items).To(BeEmpty()) }).Should(Succeed()) @@ -66,12 +63,20 @@ var _ = Describe("Interface Controller", func() { vlan.Name = name vlan.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, vlan))).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.VLAN{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) By("Cleaning up test VRF resource") vrf := &v1alpha1.VRF{} vrf.Name = name vrf.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, vrf))).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.VRF{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) By("Verifying the Interface is removed from the provider") Eventually(func(g Gomega) { @@ -180,7 +185,7 @@ var _ = Describe("Interface Controller", func() { resource := new(v1alpha1.Interface) g.Expect(k8sClient.Get(ctx, key, resource)).To(Succeed()) g.Expect(controllerutil.ContainsFinalizer(resource, v1alpha1.FinalizerName)).To(BeTrue()) - g.Expect(testProvider.Ports.Has(name)).To(BeTrue()) + g.Expect(testDevices.StateFor(name).Ports.Has(name)).To(BeTrue()) }).Should(Succeed()) By("Deleting the provider config before the Interface") @@ -195,7 +200,7 @@ var _ = Describe("Interface Controller", func() { Eventually(func(g Gomega) { err := k8sClient.Get(ctx, key, new(v1alpha1.Interface)) g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) - g.Expect(testProvider.Ports.Has(name)).To(BeFalse()) + g.Expect(testDevices.StateFor(name).Ports.Has(name)).To(BeFalse()) }).Should(Succeed()) }) @@ -263,6 +268,9 @@ var _ = Describe("Interface Controller", func() { }, } Expect(k8sClient.Create(ctx, lb)).To(Succeed()) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, lb))).To(Succeed()) + }) By("Creating a Physical Interface with unnumbered reference to the cross-device Interface") eth := &v1alpha1.Interface{ @@ -475,6 +483,9 @@ var _ = Describe("Interface Controller", func() { }, } Expect(k8sClient.Create(ctx, member)).To(Succeed()) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, member))).To(Succeed()) + }) By("Creating an Aggregate Interface referencing the cross-device member") aggregate := &v1alpha1.Interface{ @@ -1370,15 +1381,19 @@ var _ = Describe("Interface Controller", func() { By("Cleaning up DNS resource") if dns != nil { - Expect(k8sClient.Delete(ctx, dns)).To(Succeed()) + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, dns))).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dns), &v1alpha1.DNS{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) } By("Cleaning up Device resources") if localDevice != nil { - Expect(k8sClient.Delete(ctx, localDevice)).To(Succeed()) + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, localDevice))).To(Succeed()) } if remoteDevice != nil { - Expect(k8sClient.Delete(ctx, remoteDevice)).To(Succeed()) + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, remoteDevice))).To(Succeed()) } }) diff --git a/internal/controller/core/isis_controller_test.go b/internal/controller/core/isis_controller_test.go index 78dbd5406..f843f9d48 100644 --- a/internal/controller/core/isis_controller_test.go +++ b/internal/controller/core/isis_controller_test.go @@ -5,6 +5,7 @@ package core import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -67,6 +68,12 @@ var _ = Describe("ISIS Controller", func() { g.Expect(testDevices.StateFor(name).ISIS.Has("UNDERLAY")).To(BeFalse(), "Provider should not have ISIS instance configured") }).Should(Succeed()) + By("Waiting for the ISIS to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.ISIS{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleanup the Device resource") device := &v1alpha1.Device{} device.Name = name @@ -146,6 +153,12 @@ var _ = Describe("ISIS Controller", func() { isis.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, isis))).To(Succeed()) + By("Waiting for the ISIS to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.ISIS{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleanup the Device resource") device := &v1alpha1.Device{} device.Name = name diff --git a/internal/controller/core/lldp_controller_test.go b/internal/controller/core/lldp_controller_test.go index 1774566ce..c4138129a 100644 --- a/internal/controller/core/lldp_controller_test.go +++ b/internal/controller/core/lldp_controller_test.go @@ -601,6 +601,18 @@ var _ = Describe("LLDP Controller", func() { g.Expect(testDevices.StateFor(deviceName).LLDP).To(BeNil(), "Provider should have no LLDP configured") }).Should(Succeed()) + By("Cleaning up Interface resources for this device") + intfList := &v1alpha1.InterfaceList{} + Expect(k8sManager.GetClient().List(ctx, intfList, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: deviceName})).To(Succeed()) + for i := range intfList.Items { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &intfList.Items[i]))).To(Succeed()) + } + Eventually(func(g Gomega) { + list := &v1alpha1.InterfaceList{} + g.Expect(k8sManager.GetClient().List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: deviceName})).To(Succeed()) + g.Expect(list.Items).To(BeEmpty()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device = &v1alpha1.Device{} device.Name = deviceKey.Name @@ -821,6 +833,18 @@ var _ = Describe("LLDP Controller", func() { g.Expect(testDevices.StateFor(deviceName).LLDP).To(BeNil(), "Provider should have no LLDP configured") }).Should(Succeed()) + By("Cleaning up Interface resources for this device") + intfList := &v1alpha1.InterfaceList{} + Expect(k8sManager.GetClient().List(ctx, intfList, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: deviceName})).To(Succeed()) + for i := range intfList.Items { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &intfList.Items[i]))).To(Succeed()) + } + Eventually(func(g Gomega) { + list := &v1alpha1.InterfaceList{} + g.Expect(k8sManager.GetClient().List(ctx, list, client.InNamespace(metav1.NamespaceDefault), client.MatchingFields{v1alpha1.DeviceRefIndexKey: deviceName})).To(Succeed()) + g.Expect(list.Items).To(BeEmpty()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device = &v1alpha1.Device{} device.Name = deviceKey.Name diff --git a/internal/controller/core/managementaccess_controller_test.go b/internal/controller/core/managementaccess_controller_test.go index 679662818..d4f8a2e9a 100644 --- a/internal/controller/core/managementaccess_controller_test.go +++ b/internal/controller/core/managementaccess_controller_test.go @@ -6,6 +6,7 @@ package core import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -64,6 +65,12 @@ var _ = Describe("ManagementAccess Controller", func() { g.Expect(testDevices.StateFor(name).Access).To(BeNil(), "Provider should not have ManagementAccess configured") }).Should(Succeed()) + By("Waiting for the ManagementAccess to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.ManagementAccess{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device := &v1alpha1.Device{} device.Name = name diff --git a/internal/controller/core/ntp_controller_test.go b/internal/controller/core/ntp_controller_test.go index b43f52772..38c18dc10 100644 --- a/internal/controller/core/ntp_controller_test.go +++ b/internal/controller/core/ntp_controller_test.go @@ -6,6 +6,7 @@ package core import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -67,6 +68,12 @@ var _ = Describe("NTP Controller", func() { g.Expect(testDevices.StateFor(name).NTP).To(BeNil(), "Provider NTP should be nil") }).Should(Succeed()) + By("Waiting for the NTP to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.NTP{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device := &v1alpha1.Device{} device.Name = name diff --git a/internal/controller/core/ospf_controller_test.go b/internal/controller/core/ospf_controller_test.go index 862cb5fe3..0e6ff9faf 100644 --- a/internal/controller/core/ospf_controller_test.go +++ b/internal/controller/core/ospf_controller_test.go @@ -67,6 +67,12 @@ var _ = Describe("OSPF Controller", func() { g.Expect(testDevices.StateFor(name).OSPF.Has("UNDERLAY")).ToNot(BeTrue(), "Provider should not have OSPF instance configured") }).Should(Succeed()) + By("Waiting for the OSPF to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.OSPF{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleanup the Device resource") device := &v1alpha1.Device{} device.Name = name @@ -153,7 +159,7 @@ var _ = Describe("OSPF Controller", func() { Eventually(func(g Gomega) { err := k8sClient.Get(ctx, key, new(v1alpha1.OSPF)) g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) - g.Expect(testProvider.OSPF.Has("UNDERLAY")).To(BeFalse()) + g.Expect(testDevices.StateFor(name).OSPF.Has("UNDERLAY")).To(BeFalse()) }).Should(Succeed()) }) }) @@ -206,7 +212,15 @@ var _ = Describe("OSPF Controller", func() { AfterEach(func() { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &v1alpha1.OSPF{Name: name, Namespace: metav1.NamespaceDefault}))).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.OSPF{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &v1alpha1.Interface{Name: name, Namespace: metav1.NamespaceDefault}))).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.Interface{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &v1alpha1.Device{Name: name, Namespace: metav1.NamespaceDefault}))).To(Succeed()) }) @@ -269,6 +283,12 @@ var _ = Describe("OSPF Controller", func() { ospf.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, ospf))).To(Succeed()) + By("Waiting for the OSPF to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.OSPF{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleanup the Device resource") device := &v1alpha1.Device{} device.Name = name diff --git a/internal/controller/core/pim_controller_test.go b/internal/controller/core/pim_controller_test.go index 6120a6736..db4b062b2 100644 --- a/internal/controller/core/pim_controller_test.go +++ b/internal/controller/core/pim_controller_test.go @@ -6,6 +6,7 @@ package core import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -60,6 +61,12 @@ var _ = Describe("PIM Controller", func() { g.Expect(testDevices.StateFor(name).PIM).To(BeNil(), "Provider should not have PIM instance configured") }).Should(Succeed()) + By("Waiting for the PIM to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.PIM{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleanup the Device resource") device := &v1alpha1.Device{} device.Name = name @@ -138,6 +145,10 @@ var _ = Describe("PIM Controller", func() { pim.Name = name pim.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, pim))).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.PIM{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) By("Cleanup the Device resource") device := &v1alpha1.Device{} diff --git a/internal/controller/core/prefixset_controller_test.go b/internal/controller/core/prefixset_controller_test.go index 1d2155f9a..a80450161 100644 --- a/internal/controller/core/prefixset_controller_test.go +++ b/internal/controller/core/prefixset_controller_test.go @@ -6,6 +6,7 @@ package core import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -71,6 +72,12 @@ var _ = Describe("PrefixSet Controller", func() { g.Expect(testDevices.StateFor(name).PrefixSets.Has(set)).To(BeFalse(), "Provider should not have PrefixSet configured") }).Should(Succeed()) + By("Waiting for the PrefixSet to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.PrefixSet{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device := &v1alpha1.Device{} device.Name = name diff --git a/internal/controller/core/routingpolicy_controller_test.go b/internal/controller/core/routingpolicy_controller_test.go index c9ae23eb4..1814acf64 100644 --- a/internal/controller/core/routingpolicy_controller_test.go +++ b/internal/controller/core/routingpolicy_controller_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -47,12 +48,24 @@ var _ = Describe("RoutingPolicy Controller", func() { rp.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, rp))).To(Succeed()) + By("Waiting for RoutingPolicy resource to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKey{Name: name, Namespace: metav1.NamespaceDefault}, &v1alpha1.RoutingPolicy{}) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleaning up the PrefixSet resource") ps := &v1alpha1.PrefixSet{} ps.Name = name ps.Namespace = metav1.NamespaceDefault Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, ps))).To(Succeed()) + By("Waiting for PrefixSet resource to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKey{Name: name, Namespace: metav1.NamespaceDefault}, &v1alpha1.PrefixSet{}) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Verifying the RoutingPolicy is removed from the provider") Eventually(func(g Gomega) { g.Expect(testDevices.StateFor(name).RoutingPolicies.Has(name)).To(BeFalse(), "Provider shouldn't have RoutingPolicy configured anymore") diff --git a/internal/controller/core/snmp_controller_test.go b/internal/controller/core/snmp_controller_test.go index 2105778c7..cd2649e44 100644 --- a/internal/controller/core/snmp_controller_test.go +++ b/internal/controller/core/snmp_controller_test.go @@ -6,6 +6,7 @@ package core import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -78,6 +79,12 @@ var _ = Describe("SNMP Controller", func() { g.Expect(testDevices.StateFor(name).SNMP).To(BeNil(), "Provider should not have SNMP configured") }).Should(Succeed()) + By("Waiting for the SNMP to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.SNMP{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device := &v1alpha1.Device{} device.Name = name diff --git a/internal/controller/core/syslog_controller_test.go b/internal/controller/core/syslog_controller_test.go index ea89df9f7..2fbc1da09 100644 --- a/internal/controller/core/syslog_controller_test.go +++ b/internal/controller/core/syslog_controller_test.go @@ -6,6 +6,7 @@ package core import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -73,6 +74,12 @@ var _ = Describe("Syslog Controller", func() { g.Expect(testDevices.StateFor(name).Syslog).To(BeNil(), "Provider should not have Syslog configured") }).Should(Succeed()) + By("Waiting for the Syslog to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.Syslog{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device := &v1alpha1.Device{} device.Name = name diff --git a/internal/controller/core/user_controller_test.go b/internal/controller/core/user_controller_test.go index b67617476..332906561 100644 --- a/internal/controller/core/user_controller_test.go +++ b/internal/controller/core/user_controller_test.go @@ -7,6 +7,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -81,6 +82,12 @@ var _ = Describe("User Controller", func() { g.Expect(testDevices.StateFor(name).User.Has(username)).To(BeFalse(), "User should not exist") }).Should(Succeed()) + By("Waiting for the User to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.User{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device := &v1alpha1.Device{} device.Name = name diff --git a/internal/controller/core/vlan_controller_test.go b/internal/controller/core/vlan_controller_test.go index 0fe72d350..c63089538 100644 --- a/internal/controller/core/vlan_controller_test.go +++ b/internal/controller/core/vlan_controller_test.go @@ -6,6 +6,7 @@ package core import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -63,6 +64,12 @@ var _ = Describe("VLAN Controller", func() { g.Expect(testDevices.StateFor(name).VLANs.Has(id)).To(BeFalse(), "Provider VLAN should not exist") }).Should(Succeed()) + By("Waiting for the VLAN to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.VLAN{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device := &v1alpha1.Device{} device.Name = name diff --git a/internal/controller/core/vrf_controller_test.go b/internal/controller/core/vrf_controller_test.go index c002bd121..473fd0a5f 100644 --- a/internal/controller/core/vrf_controller_test.go +++ b/internal/controller/core/vrf_controller_test.go @@ -6,11 +6,11 @@ package core import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/ironcore-dev/network-operator/api/core/v1alpha1" ) @@ -82,6 +82,12 @@ var _ = Describe("VRF Controller", func() { g.Expect(testDevices.StateFor(name).VRF.Has("CC-ADMIN-TEST")).To(BeFalse(), "Provider should not have VRF configured anymore") }).Should(Succeed()) + By("Waiting for the VRF to be fully deleted") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, key, &v1alpha1.VRF{}) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }).Should(Succeed()) + By("Cleaning up the Device resource") device := &v1alpha1.Device{} device.Name = name From 7a47ceeb9717ed6b4e3132401c9f15d6e3c09afa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Mon, 21 Sep 2026 15:02:03 +0200 Subject: [PATCH 4/9] Persist one-shot status before metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../controller/core/configbackup_controller.go | 14 +++++++------- internal/controller/core/probe_controller.go | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/internal/controller/core/configbackup_controller.go b/internal/controller/core/configbackup_controller.go index 97037f7b4..b7900179c 100644 --- a/internal/controller/core/configbackup_controller.go +++ b/internal/controller/core/configbackup_controller.go @@ -189,16 +189,16 @@ func (r *ConfigBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request // Always attempt to update the metadata/status after reconciliation defer func() { - if !equality.Semantic.DeepEqual(orig.ObjectMeta, obj.ObjectMeta) { - // Pass obj.DeepCopy() to avoid Patch() modifying obj and interfering with status update below - if err := r.Patch(ctx, obj.DeepCopy(), client.MergeFrom(orig)); err != nil { - log.Error(err, "Failed to update resource metadata") + if !equality.Semantic.DeepEqual(orig.Status, obj.Status) { + // Pass obj.DeepCopy() to avoid Patch() modifying obj and interfering with metadata update below + if err := r.Status().Patch(ctx, obj.DeepCopy(), client.MergeFrom(orig)); err != nil { + log.Error(err, "Failed to update status") reterr = kerrors.NewAggregate([]error{reterr, err}) } } - if !equality.Semantic.DeepEqual(orig.Status, obj.Status) { - if err := r.Status().Patch(ctx, obj, client.MergeFrom(orig)); err != nil { - log.Error(err, "Failed to update status") + if !equality.Semantic.DeepEqual(orig.ObjectMeta, obj.ObjectMeta) { + if err := r.Patch(ctx, obj, client.MergeFrom(orig)); err != nil { + log.Error(err, "Failed to update resource metadata") reterr = kerrors.NewAggregate([]error{reterr, err}) } } diff --git a/internal/controller/core/probe_controller.go b/internal/controller/core/probe_controller.go index 844f0c405..80deded81 100644 --- a/internal/controller/core/probe_controller.go +++ b/internal/controller/core/probe_controller.go @@ -171,16 +171,16 @@ func (r *ProbeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ct // Always attempt to update the metadata/status after reconciliation defer func() { - if !equality.Semantic.DeepEqual(orig.ObjectMeta, obj.ObjectMeta) { - // Pass obj.DeepCopy() to avoid Patch() modifying obj and interfering with status update below - if err := r.Patch(ctx, obj.DeepCopy(), client.MergeFrom(orig)); err != nil { - log.Error(err, "Failed to update resource metadata") + if !equality.Semantic.DeepEqual(orig.Status, obj.Status) { + // Pass obj.DeepCopy() to avoid Patch() modifying obj and interfering with metadata update below + if err := r.Status().Patch(ctx, obj.DeepCopy(), client.MergeFrom(orig)); err != nil { + log.Error(err, "Failed to update status") reterr = kerrors.NewAggregate([]error{reterr, err}) } } - if !equality.Semantic.DeepEqual(orig.Status, obj.Status) { - if err := r.Status().Patch(ctx, obj, client.MergeFrom(orig)); err != nil { - log.Error(err, "Failed to update status") + if !equality.Semantic.DeepEqual(orig.ObjectMeta, obj.ObjectMeta) { + if err := r.Patch(ctx, obj, client.MergeFrom(orig)); err != nil { + log.Error(err, "Failed to update resource metadata") reterr = kerrors.NewAggregate([]error{reterr, err}) } } From c5771229933620fdb50daaca864ba351175ea77f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Wed, 23 Sep 2026 13:18:56 +0200 Subject: [PATCH 5/9] Fix gnmi test suite exclusion from test Makefile target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gNMI Integration Suite has a dedicated Makefile target in 'test-gnmi'. Therefore, it shall be excluded on the usual 'test' target runs. The exclusion had a bug in matching the package, which this change fixes. Signed-off-by: Felix Kästner --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index a853b10ed..758647e15 100644 --- a/Makefile +++ b/Makefile @@ -66,7 +66,7 @@ vet: ## Run go vet against code. .PHONY: test test: manifests generate setup-envtest ## Run tests. - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e | grep -v /lab | grep -v /gnmi/) -coverprofile cover.out + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e | grep -v /lab | grep -v /gnmi) -coverprofile cover.out .PHONY: coverage coverage: test ## Run tests and generate coverage report. From 34ce2d4b8e846a3bb68481628e07515bdb5db1b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Wed, 23 Sep 2026 14:46:52 +0200 Subject: [PATCH 6/9] Enable verbose logging in test suites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Configure each zap-backed test suite to emit logr messages through V(3). This makes detailed reconciliation and lock diagnostics available through GinkgoWriter during test runs. Let controller managers inherit the configured global logger instead of overriding it with GinkgoLogr, whose default verbosity filters these messages. Signed-off-by: Felix Kästner --- internal/controller/cisco/nx/suite_test.go | 8 ++++++-- internal/controller/core/suite_test.go | 8 ++++++-- internal/controller/evpn/suite_test.go | 8 ++++++-- internal/controller/pool/suite_test.go | 8 ++++++-- internal/webhook/cisco/nx/v1alpha1/webhook_suite_test.go | 7 ++++++- internal/webhook/core/v1alpha1/webhook_suite_test.go | 7 ++++++- internal/webhook/pool/v1alpha1/webhook_suite_test.go | 7 ++++++- test/gnmi/gnmi_suite_test.go | 8 ++++++-- 8 files changed, 48 insertions(+), 13 deletions(-) diff --git a/internal/controller/cisco/nx/suite_test.go b/internal/controller/cisco/nx/suite_test.go index 445463b0f..d0b811007 100644 --- a/internal/controller/cisco/nx/suite_test.go +++ b/internal/controller/cisco/nx/suite_test.go @@ -13,6 +13,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "go.uber.org/zap/zapcore" coordinationv1 "k8s.io/api/coordination/v1" corev1 "k8s.io/api/core/v1" @@ -55,7 +56,11 @@ func TestControllers(t *testing.T) { } var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + logf.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.UseDevMode(true), + zap.Level(zapcore.Level(-3)), + )) SetDefaultEventuallyTimeout(time.Minute) SetDefaultEventuallyPollingInterval(200 * time.Millisecond) @@ -90,7 +95,6 @@ var _ = BeforeSuite(func() { k8sManager, err = ctrl.NewManager(cfg, ctrl.Options{ Scheme: scheme.Scheme, - Logger: GinkgoLogr, Metrics: metricsserver.Options{BindAddress: "0"}, }) Expect(err).ToNot(HaveOccurred()) diff --git a/internal/controller/core/suite_test.go b/internal/controller/core/suite_test.go index dd3bc4728..96ed313ea 100644 --- a/internal/controller/core/suite_test.go +++ b/internal/controller/core/suite_test.go @@ -17,6 +17,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "go.uber.org/zap/zapcore" coordinationv1 "k8s.io/api/coordination/v1" corev1 "k8s.io/api/core/v1" @@ -62,7 +63,11 @@ func TestControllers(t *testing.T) { } var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + logf.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.UseDevMode(true), + zap.Level(zapcore.Level(-3)), + )) SetDefaultEventuallyTimeout(90 * time.Second) SetDefaultEventuallyPollingInterval(200 * time.Millisecond) @@ -94,7 +99,6 @@ var _ = BeforeSuite(func() { k8sManager, err = ctrl.NewManager(cfg, ctrl.Options{ Scheme: scheme.Scheme, - Logger: GinkgoLogr, Metrics: metricsserver.Options{BindAddress: "0"}, }) Expect(err).ToNot(HaveOccurred()) diff --git a/internal/controller/evpn/suite_test.go b/internal/controller/evpn/suite_test.go index 7c6e9f1e2..4d1aca9c4 100644 --- a/internal/controller/evpn/suite_test.go +++ b/internal/controller/evpn/suite_test.go @@ -13,6 +13,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "go.uber.org/zap/zapcore" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -52,7 +53,11 @@ func TestControllers(t *testing.T) { } var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + logf.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.UseDevMode(true), + zap.Level(zapcore.Level(-3)), + )) SetDefaultEventuallyTimeout(time.Minute) SetDefaultEventuallyPollingInterval(200 * time.Millisecond) @@ -82,7 +87,6 @@ var _ = BeforeSuite(func() { k8sManager, err = ctrl.NewManager(cfg, ctrl.Options{ Scheme: scheme.Scheme, - Logger: GinkgoLogr, Metrics: metricsserver.Options{BindAddress: "0"}, HealthProbeBindAddress: "0", }) diff --git a/internal/controller/pool/suite_test.go b/internal/controller/pool/suite_test.go index 8f7e57ddd..b1d12613c 100644 --- a/internal/controller/pool/suite_test.go +++ b/internal/controller/pool/suite_test.go @@ -12,6 +12,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "go.uber.org/zap/zapcore" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -45,7 +46,11 @@ func TestControllers(t *testing.T) { } var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + logf.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.UseDevMode(true), + zap.Level(zapcore.Level(-3)), + )) SetDefaultEventuallyTimeout(time.Minute) SetDefaultEventuallyPollingInterval(200 * time.Millisecond) @@ -75,7 +80,6 @@ var _ = BeforeSuite(func() { k8sManager, err = ctrl.NewManager(cfg, ctrl.Options{ Scheme: scheme.Scheme, - Logger: GinkgoLogr, }) Expect(err).NotTo(HaveOccurred()) diff --git a/internal/webhook/cisco/nx/v1alpha1/webhook_suite_test.go b/internal/webhook/cisco/nx/v1alpha1/webhook_suite_test.go index 26deb0422..bfa6877d3 100644 --- a/internal/webhook/cisco/nx/v1alpha1/webhook_suite_test.go +++ b/internal/webhook/cisco/nx/v1alpha1/webhook_suite_test.go @@ -15,6 +15,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "go.uber.org/zap/zapcore" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -48,7 +49,11 @@ func TestAPIs(t *testing.T) { } var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + logf.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.UseDevMode(true), + zap.Level(zapcore.Level(-3)), + )) ctx, cancel = context.WithCancel(context.TODO()) diff --git a/internal/webhook/core/v1alpha1/webhook_suite_test.go b/internal/webhook/core/v1alpha1/webhook_suite_test.go index 3f48e6f9e..e7c01eedf 100644 --- a/internal/webhook/core/v1alpha1/webhook_suite_test.go +++ b/internal/webhook/core/v1alpha1/webhook_suite_test.go @@ -15,6 +15,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "go.uber.org/zap/zapcore" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -48,7 +49,11 @@ func TestAPIs(t *testing.T) { } var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + logf.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.UseDevMode(true), + zap.Level(zapcore.Level(-3)), + )) ctx, cancel = context.WithCancel(context.TODO()) diff --git a/internal/webhook/pool/v1alpha1/webhook_suite_test.go b/internal/webhook/pool/v1alpha1/webhook_suite_test.go index a717247d4..d94a2c7ab 100644 --- a/internal/webhook/pool/v1alpha1/webhook_suite_test.go +++ b/internal/webhook/pool/v1alpha1/webhook_suite_test.go @@ -15,6 +15,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "go.uber.org/zap/zapcore" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -48,7 +49,11 @@ func TestAPIs(t *testing.T) { } var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + logf.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.UseDevMode(true), + zap.Level(zapcore.Level(-3)), + )) ctx, cancel = context.WithCancel(context.TODO()) diff --git a/test/gnmi/gnmi_suite_test.go b/test/gnmi/gnmi_suite_test.go index cb7606bdc..f9e0e781d 100644 --- a/test/gnmi/gnmi_suite_test.go +++ b/test/gnmi/gnmi_suite_test.go @@ -13,6 +13,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/onsi/gomega/format" + "go.uber.org/zap/zapcore" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -64,7 +65,11 @@ func TestGNMI(t *testing.T) { // BeforeSuite initializes the test environment. // It starts the gNMI test server, sets up the Kubernetes client, and starts the controller manager. var _ = BeforeSuite(func(ctx SpecContext) { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + logf.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.UseDevMode(true), + zap.Level(zapcore.Level(-3)), + )) format.MaxLength = 0 SetDefaultEventuallyTimeout(60 * time.Second) SetDefaultEventuallyPollingInterval(time.Second) @@ -107,7 +112,6 @@ var _ = BeforeSuite(func(ctx SpecContext) { By("starting controller manager") mgr, err := ctrl.NewManager(restConfig, ctrl.Options{ Scheme: k8sClient.Scheme(), - Logger: GinkgoLogr, Metrics: metricsserver.Options{BindAddress: "0"}, // Disable metrics server }) Expect(err).ToNot(HaveOccurred()) From e02ad9a38c84fd87762655981a0cd14c8d133d25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Wed, 23 Sep 2026 16:23:45 +0200 Subject: [PATCH 7/9] Align protocol watches with Interface readiness checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use conditions.IsConfigured in the OSPF, ISIS, and PIM Interface watch predicates to match the dependency check used during reconciliation. This ensures a protocol is requeued whenever an Interface transitions between configured and not configured, including when ObservedGeneration catches up with an updated spec. Signed-off-by: Felix Kästner --- internal/controller/core/isis_controller.go | 4 +--- internal/controller/core/ospf_controller.go | 5 +---- internal/controller/core/pim_controller.go | 4 +--- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/internal/controller/core/isis_controller.go b/internal/controller/core/isis_controller.go index 82fd51b4b..be0d92ba4 100644 --- a/internal/controller/core/isis_controller.go +++ b/internal/controller/core/isis_controller.go @@ -259,9 +259,7 @@ func (r *ISISReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) UpdateFunc: func(e event.UpdateEvent) bool { oldInterface := e.ObjectOld.(*v1alpha1.Interface) newInterface := e.ObjectNew.(*v1alpha1.Interface) - oldConfigured := conditions.Get(oldInterface, v1alpha1.ConfiguredCondition) - newConfigured := conditions.Get(newInterface, v1alpha1.ConfiguredCondition) - return ((oldConfigured == nil) != (newConfigured == nil)) || (newConfigured != nil && oldConfigured.Status != newConfigured.Status) + return conditions.IsConfigured(oldInterface) != conditions.IsConfigured(newInterface) }, GenericFunc: func(e event.GenericEvent) bool { return false diff --git a/internal/controller/core/ospf_controller.go b/internal/controller/core/ospf_controller.go index 5f40f3db9..7e6578834 100644 --- a/internal/controller/core/ospf_controller.go +++ b/internal/controller/core/ospf_controller.go @@ -269,11 +269,8 @@ func (r *OSPFReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) UpdateFunc: func(e event.UpdateEvent) bool { oldInterface := e.ObjectOld.(*v1alpha1.Interface) newInterface := e.ObjectNew.(*v1alpha1.Interface) - oldConfigured := conditions.Get(oldInterface, v1alpha1.ConfiguredCondition) - newConfigured := conditions.Get(newInterface, v1alpha1.ConfiguredCondition) return oldInterface.HasIPv4() != newInterface.HasIPv4() || - ((oldConfigured == nil) != (newConfigured == nil)) || - (newConfigured != nil && oldConfigured.Status != newConfigured.Status) + conditions.IsConfigured(oldInterface) != conditions.IsConfigured(newInterface) }, GenericFunc: func(e event.GenericEvent) bool { return false diff --git a/internal/controller/core/pim_controller.go b/internal/controller/core/pim_controller.go index db8d64020..d881e55f6 100644 --- a/internal/controller/core/pim_controller.go +++ b/internal/controller/core/pim_controller.go @@ -259,9 +259,7 @@ func (r *PIMReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) UpdateFunc: func(e event.UpdateEvent) bool { oldInterface := e.ObjectOld.(*v1alpha1.Interface) newInterface := e.ObjectNew.(*v1alpha1.Interface) - oldConfigured := conditions.Get(oldInterface, v1alpha1.ConfiguredCondition) - newConfigured := conditions.Get(newInterface, v1alpha1.ConfiguredCondition) - return ((oldConfigured == nil) != (newConfigured == nil)) || (newConfigured != nil && oldConfigured.Status != newConfigured.Status) + return conditions.IsConfigured(oldInterface) != conditions.IsConfigured(newInterface) }, GenericFunc: func(e event.GenericEvent) bool { return false From 86b13b8aefbac85c28bf259c8e4b6a411719044b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Wed, 23 Sep 2026 16:31:14 +0200 Subject: [PATCH 8/9] Wait for controller test caches to sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use each manager cache's WaitForCacheSync before specs create resources. This replaces the default namespace API probe, which only confirms API server availability and does not ensure controller watches are ready. Signed-off-by: Felix Kästner --- internal/controller/cisco/nx/suite_test.go | 7 +++---- internal/controller/core/suite_test.go | 7 +++---- internal/controller/evpn/suite_test.go | 9 +++------ internal/controller/pool/suite_test.go | 9 +++------ 4 files changed, 12 insertions(+), 20 deletions(-) diff --git a/internal/controller/cisco/nx/suite_test.go b/internal/controller/cisco/nx/suite_test.go index d0b811007..b56a5b03d 100644 --- a/internal/controller/cisco/nx/suite_test.go +++ b/internal/controller/cisco/nx/suite_test.go @@ -164,10 +164,9 @@ var _ = BeforeSuite(func() { Expect(err).ToNot(HaveOccurred(), "failed to run manager") }() - Eventually(func() error { - var namespace corev1.Namespace - return k8sClient.Get(ctx, client.ObjectKey{Name: metav1.NamespaceDefault}, &namespace) - }).Should(Succeed()) + Eventually(func() bool { + return k8sManager.GetCache().WaitForCacheSync(ctx) + }).Should(BeTrue()) }) var _ = AfterSuite(func() { diff --git a/internal/controller/core/suite_test.go b/internal/controller/core/suite_test.go index 96ed313ea..2f847e28d 100644 --- a/internal/controller/core/suite_test.go +++ b/internal/controller/core/suite_test.go @@ -359,10 +359,9 @@ var _ = BeforeSuite(func() { Expect(err).ToNot(HaveOccurred(), "failed to run manager") }() - Eventually(func() error { - var namespace corev1.Namespace - return k8sClient.Get(ctx, client.ObjectKey{Name: metav1.NamespaceDefault}, &namespace) - }).Should(Succeed()) + Eventually(func() bool { + return k8sManager.GetCache().WaitForCacheSync(ctx) + }).Should(BeTrue()) }) var _ = AfterSuite(func() { diff --git a/internal/controller/evpn/suite_test.go b/internal/controller/evpn/suite_test.go index 4d1aca9c4..7231ae023 100644 --- a/internal/controller/evpn/suite_test.go +++ b/internal/controller/evpn/suite_test.go @@ -15,8 +15,6 @@ import ( . "github.com/onsi/gomega" "go.uber.org/zap/zapcore" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/events" ctrl "sigs.k8s.io/controller-runtime" @@ -149,10 +147,9 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred(), "failed to run manager") }() - Eventually(func() error { - var namespace corev1.Namespace - return k8sClient.Get(context.Background(), client.ObjectKey{Name: metav1.NamespaceDefault}, &namespace) - }).Should(Succeed()) + Eventually(func() bool { + return k8sManager.GetCache().WaitForCacheSync(ctx) + }).Should(BeTrue()) }) var _ = AfterSuite(func() { diff --git a/internal/controller/pool/suite_test.go b/internal/controller/pool/suite_test.go index b1d12613c..cee3eb238 100644 --- a/internal/controller/pool/suite_test.go +++ b/internal/controller/pool/suite_test.go @@ -14,8 +14,6 @@ import ( . "github.com/onsi/gomega" "go.uber.org/zap/zapcore" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -143,10 +141,9 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred(), "failed to run manager") }() - Eventually(func() error { - var namespace corev1.Namespace - return k8sClient.Get(context.Background(), client.ObjectKey{Name: metav1.NamespaceDefault}, &namespace) - }).Should(Succeed()) + Eventually(func() bool { + return k8sManager.GetCache().WaitForCacheSync(ctx) + }).Should(BeTrue()) }) var _ = AfterSuite(func() { From cb97e453c80e28c99b874df81970b0f060967ca1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Wed, 23 Sep 2026 16:36:40 +0200 Subject: [PATCH 9/9] Stop lease renewal before releasing locks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cancel the renewal goroutine before reading the Lease during release. A cached client can temporarily return NotFound immediately after creating a Lease; cancellation must still happen so the unseen Lease is not renewed indefinitely and does not starve other reconcilers. Add a regression test covering the cached-NotFound release path. Signed-off-by: Felix Kästner --- internal/resourcelock/resourcelock.go | 10 +++++----- internal/resourcelock/resourcelock_test.go | 9 ++++++++- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/internal/resourcelock/resourcelock.go b/internal/resourcelock/resourcelock.go index 9bad53c93..ab56a1113 100644 --- a/internal/resourcelock/resourcelock.go +++ b/internal/resourcelock/resourcelock.go @@ -139,6 +139,11 @@ func (rl *ResourceLocker) AcquireLock(ctx context.Context, name, lockerID string func (rl *ResourceLocker) ReleaseLock(ctx context.Context, name, lockerID string) error { log := ctrl.LoggerFrom(ctx).WithValues("namespace", rl.namespace, "lease", name, "locker", lockerID) + if cancel, ok := rl.cancelFuncs.LoadAndDelete(name); ok { + cancel.(context.CancelFunc)() + log.V(3).Info("Stopped renewal goroutine") + } + lease := &coordinationv1.Lease{} if err := rl.client.Get(ctx, client.ObjectKey{Namespace: rl.namespace, Name: name}, lease); err != nil { if apierrors.IsNotFound(err) { @@ -161,11 +166,6 @@ func (rl *ResourceLocker) ReleaseLock(ctx context.Context, name, lockerID string return fmt.Errorf("resourcelock: failed to delete lease: %w", err) } - if cancel, ok := rl.cancelFuncs.LoadAndDelete(name); ok { - cancel.(context.CancelFunc)() - log.V(3).Info("Stopped renewal goroutine") - } - log.V(2).Info("Lock released") return nil } diff --git a/internal/resourcelock/resourcelock_test.go b/internal/resourcelock/resourcelock_test.go index e6b1c66d6..0cb1a0ab1 100644 --- a/internal/resourcelock/resourcelock_test.go +++ b/internal/resourcelock/resourcelock_test.go @@ -4,6 +4,7 @@ package resourcelock import ( + "context" "errors" "testing" "time" @@ -323,10 +324,16 @@ func TestReleaseLock_NotFound(t *testing.T) { t.Fatalf("NewResourceLocker() error = %v", err) } - ctx := t.Context() + ctx, cancel := context.WithCancel(t.Context()) + rl.cancelFuncs.Store("non-existent-lease", cancel) if err := rl.ReleaseLock(ctx, "non-existent-lease", "locker-1"); err != nil { t.Errorf("ReleaseLock() error = %v, expected success (noop) when lease not found", err) } + select { + case <-ctx.Done(): + default: + t.Error("ReleaseLock() did not cancel renewal when lease was not found") + } } func TestRenewLease(t *testing.T) {