Skip to content

Isolate per-device provider state in controller tests #558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
felix-kaestner wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Isolate per-device provider state in controller tests #558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f14473
Isolate per-device provider state in controller tests
felix-kaestner Sep 11, 2026
20cba39
Wait for resource deletion before Device in AfterEach
felix-kaestner Sep 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions internal/controller/cisco/nx/bordergateway_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -76,6 +77,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")
Expand Down
7 changes: 7 additions & 0 deletions internal/controller/cisco/nx/system_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -57,6 +58,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")
Expand Down
13 changes: 13 additions & 0 deletions internal/controller/cisco/nx/vpcdomain_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -114,6 +115,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")
Expand Down Expand Up @@ -292,6 +299,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{}
Expand Down
11 changes: 9 additions & 2 deletions internal/controller/core/acl_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -76,7 +77,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")
Expand Down Expand Up @@ -123,7 +130,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())
})
})
Expand Down
27 changes: 17 additions & 10 deletions internal/controller/core/banner_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -45,8 +46,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")
Expand Down Expand Up @@ -107,10 +114,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())
})
Expand Down Expand Up @@ -166,10 +173,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())
})
Expand Down
32 changes: 24 additions & 8 deletions internal/controller/core/bgp_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,37 +34,53 @@ 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("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("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")
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())
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("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("Waiting for RoutingPolicy resources to be fully deleted")
Eventually(func(g Gomega) {
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")
Expand Down Expand Up @@ -120,7 +136,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())
})

Expand Down Expand Up @@ -176,8 +192,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")
Expand Down
34 changes: 25 additions & 9 deletions internal/controller/core/bgp_peer_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,37 +35,53 @@ 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("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("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")
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())
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("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("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: 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")
Expand Down Expand Up @@ -145,7 +161,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())
})

Expand Down Expand Up @@ -215,7 +231,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())
})

Expand Down Expand Up @@ -378,7 +394,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())
})

Expand Down Expand Up @@ -429,7 +445,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())
})

Expand Down
11 changes: 9 additions & 2 deletions internal/controller/core/certificate_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -89,7 +90,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")
Expand Down Expand Up @@ -136,7 +143,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())
})
})
Expand Down
Loading