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. 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/suite_test.go b/internal/controller/cisco/nx/suite_test.go index 445463b0f..b56a5b03d 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()) @@ -160,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/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 9175dd60f..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" @@ -77,7 +78,13 @@ 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("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") @@ -124,7 +131,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..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" @@ -46,8 +47,14 @@ 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("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") @@ -108,10 +115,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 +174,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..ff78edebc 100644 --- a/internal/controller/core/bgp_controller_test.go +++ b/internal/controller/core/bgp_controller_test.go @@ -35,37 +35,50 @@ 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()) 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 +134,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 +190,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..6ef7d90d1 100644 --- a/internal/controller/core/bgp_peer_controller_test.go +++ b/internal/controller/core/bgp_peer_controller_test.go @@ -36,37 +36,50 @@ 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()) 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 +159,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 +229,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()) }) @@ -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{ @@ -379,7 +395,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 +446,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..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" @@ -90,7 +91,13 @@ 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("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") @@ -137,7 +144,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.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/configbackup_controller_test.go b/internal/controller/core/configbackup_controller_test.go index 961413bef..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" @@ -53,20 +54,26 @@ 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() { 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() { @@ -157,22 +164,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 +196,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 +213,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 +272,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..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() { @@ -462,10 +473,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 +512,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 +668,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") @@ -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 3b56281c7..1a5943607 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 @@ -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) { @@ -69,13 +81,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..f7e5d15fd 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()) }) @@ -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") @@ -293,8 +296,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 +350,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 +361,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)) @@ -378,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{ @@ -397,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) @@ -477,7 +488,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 +496,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 +660,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 +697,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()) }) }) @@ -697,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{ @@ -728,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()) }) }) @@ -738,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) { @@ -754,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) { @@ -785,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) @@ -809,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) { @@ -832,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) @@ -851,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) { @@ -965,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{} @@ -1037,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 7a91b7ce0..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" @@ -63,7 +64,13 @@ 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("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") @@ -110,9 +117,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..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 := testProvider.GetEthernetSegment(name) + _, 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}, }, }, @@ -146,7 +175,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()) @@ -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 4ad87d3ea..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,16 +45,24 @@ 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) { - 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 +158,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..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,16 +63,24 @@ 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) { - 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 +149,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()) }) @@ -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{ @@ -419,7 +427,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()) }) @@ -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{ @@ -717,7 +728,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 +787,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 +913,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 +1032,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 +1175,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 +1337,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 +1358,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{} @@ -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.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/isis_controller_test.go b/internal/controller/core/isis_controller_test.go index 3129a9874..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" @@ -64,7 +65,13 @@ 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("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") @@ -111,7 +118,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()) }) }) @@ -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 84794703f..c4138129a 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,19 @@ 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 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") @@ -818,7 +830,19 @@ 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 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") @@ -852,8 +876,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 +890,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 +1084,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 +1239,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 +1257,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 +1269,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 +1309,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 +1335,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..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" @@ -61,7 +62,13 @@ 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("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") @@ -108,7 +115,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..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" @@ -64,7 +65,13 @@ 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("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") @@ -111,9 +118,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.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/ospf_controller_test.go b/internal/controller/core/ospf_controller_test.go index e462a6c02..0e6ff9faf 100644 --- a/internal/controller/core/ospf_controller_test.go +++ b/internal/controller/core/ospf_controller_test.go @@ -64,7 +64,13 @@ 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("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") @@ -115,7 +121,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()) }) @@ -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.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 diff --git a/internal/controller/core/pim_controller_test.go b/internal/controller/core/pim_controller_test.go index 8e32dc05d..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" @@ -57,7 +58,13 @@ 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("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") @@ -104,7 +111,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()) }) }) @@ -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 39784857d..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" @@ -68,7 +69,13 @@ 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("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") @@ -115,7 +122,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.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}) } } 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..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,15 +48,27 @@ 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(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 +133,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 +200,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 +345,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..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" @@ -75,7 +76,13 @@ 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("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") @@ -122,9 +129,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..2f847e28d 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" @@ -42,13 +43,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() @@ -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()) @@ -121,7 +125,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(), @@ -355,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() { @@ -424,8 +427,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 +470,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 +492,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 +564,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 +587,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 +651,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 +666,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 +681,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 +904,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 +1028,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 +1180,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..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" @@ -70,7 +71,13 @@ 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("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") @@ -117,7 +124,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..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" @@ -78,7 +79,13 @@ 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("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") @@ -125,7 +132,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..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" @@ -60,7 +61,13 @@ 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("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") @@ -111,7 +118,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..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" ) @@ -79,7 +79,13 @@ 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("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") @@ -122,9 +128,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/controller/evpn/suite_test.go b/internal/controller/evpn/suite_test.go index 7c6e9f1e2..7231ae023 100644 --- a/internal/controller/evpn/suite_test.go +++ b/internal/controller/evpn/suite_test.go @@ -13,9 +13,8 @@ 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" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/events" ctrl "sigs.k8s.io/controller-runtime" @@ -52,7 +51,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 +85,6 @@ var _ = BeforeSuite(func() { k8sManager, err = ctrl.NewManager(cfg, ctrl.Options{ Scheme: scheme.Scheme, - Logger: GinkgoLogr, Metrics: metricsserver.Options{BindAddress: "0"}, HealthProbeBindAddress: "0", }) @@ -145,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 8f7e57ddd..cee3eb238 100644 --- a/internal/controller/pool/suite_test.go +++ b/internal/controller/pool/suite_test.go @@ -12,9 +12,8 @@ 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" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -45,7 +44,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 +78,6 @@ var _ = BeforeSuite(func() { k8sManager, err = ctrl.NewManager(cfg, ctrl.Options{ Scheme: scheme.Scheme, - Logger: GinkgoLogr, }) Expect(err).NotTo(HaveOccurred()) @@ -139,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() { 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 } 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) { 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()) 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" } ]