chore(gc): quarantine ip_port_mapping test

Signed-off-by: SkalaNetworks <contact@skala.network>

Author: SkalaNetworks

pkg/controller/gc.go

@@ -51,7 +51,7 @@ func (c *Controller) gc() error {
 		c.gcVip,
 		c.gcLbSvcPods,
 		c.gcVPCDNS,
-		c.gcOvnLb,
+		c.gcIpPortMapping,
 	}
 	for _, gcFunc := range gcFunctions {
 		if err := gcFunc(); err != nil {
@@ -1191,15 +1191,10 @@ func (c *Controller) gcLbSvcPods() error {
 	return nil
 }
 
-// gcOvnLb handles cleaning up loadbalancers created by SwitchLBRules/EndpointSlices
-// For every LB present in OVN, we make sure:
-// - the ip_port_mappings are not stale (they're used by a VIP)
-// - TODO: the VIPs are linked to an EndpointSlice and are not stale
-// - TODO: the healthchecks are linked to an EndpointSlice and are not stale
-// Right now, if the controller is down while EPs are getting deleted, the VIPs will not be cleaned
-// and the healthchecks will not be cleaned. This can lead to dangling resources in OVN.
-func (c *Controller) gcOvnLb() error {
-	klog.Infof("start to gc ovn load balancers")
+// gcIpPortMapping handles cleaning up ip_port_mappings created by SwitchLBRules/EndpointSlices
+// For every LB present in OVN, we make sure the ip_port_mappings are not stale (they're used by a VIP)
+func (c *Controller) gcIpPortMapping() error {
+	klog.Infof("start to gc ovn load balancers ip_port_mappings")
 	lbs, err := c.OVNNbClient.ListLoadBalancers(func(lb *ovnnb.LoadBalancer) bool {
 		return lb.ExternalIDs["vendor"] == util.CniTypeName
 	})

pkg/controller/gc_test.go

@@ -115,9 +115,9 @@ func Test_gcOvnLb(t *testing.T) {
 		fakeCtrl.mockOvnClient.EXPECT().LoadBalancerDeleteIPPortMapping("lb2", "192.168.2.2").Return(nil)
 		fakeCtrl.mockOvnClient.EXPECT().LoadBalancerDeleteIPPortMapping("lb3", "fd00::102").Return(nil)
 
-		err := fakeCtrl.fakeController.gcOvnLb()
+		err := fakeCtrl.fakeController.gcIpPortMapping()
 		if err != nil {
-			t.Errorf("gcOvnLb() error = %v", err)
+			t.Errorf("gcIpPortMapping() error = %v", err)
 		}
 	})
 
@@ -138,9 +138,9 @@ func Test_gcOvnLb(t *testing.T) {
 		fakeCtrl.mockOvnClient.EXPECT().ListLoadBalancers(gomock.Any()).Return([]ovnnb.LoadBalancer{*lb}, nil)
 		// No LoadBalancerDeleteIPPortMapping expected
 
-		err := fakeCtrl.fakeController.gcOvnLb()
+		err := fakeCtrl.fakeController.gcIpPortMapping()
 		if err != nil {
-			t.Errorf("gcOvnLb() error = %v", err)
+			t.Errorf("gcIpPortMapping() error = %v", err)
 		}
 	})
 }

test/e2e/kube-ovn/gc/gc.go

@@ -66,6 +66,7 @@ var _ = framework.Describe("[group:gc]", func() {
 	})
 
 	ginkgo.It("should gc stale ip_port_mappings in OVN LoadBalancer", func() {
+		ginkgo.Skip("GC doesn't yet run periodically and can't be E2E tested")
 		ginkgo.By("1. Creating a SwitchLBRule to populate LoadBalancer")
 		labels := map[string]string{"app": "gc-test"}
 		annotations := map[string]string{util.LogicalSwitchAnnotation: subnetName}
@@ -96,18 +97,21 @@ var _ = framework.Describe("[group:gc]", func() {
 		framework.ExpectNotEmpty(lbName)
 
 		ginkgo.By("Verifying active ip_port_mapping exists for backend " + backendIP)
-		cmd := []string{"ovn-nbctl", "get", "load_balancer", lbName, "ip_port_mappings"}
-		stdout, _, err := framework.NBExec(cmd...)
-		framework.ExpectNil(err)
-		framework.ExpectContainSubstring(string(stdout), backendIP)
+		framework.WaitUntil(5*time.Second, 2*time.Minute, func(_ context.Context) (bool, error) {
+			cmd := []string{"ovn-nbctl", "get", "load_balancer", lbName, "ip_port_mappings"}
+			stdout, _, err := framework.NBExec(cmd...)
+			framework.ExpectNil(err)
+
+			return strings.Contains(string(stdout), backendIP), nil
+		}, "we got correct ip_port_mappings")
 
 		ginkgo.By("3. Manually injecting a stale ip_port_mapping entry")
 		staleIP := "1.2.3.4"
 		staleMapping := "stale-node"
 
 		// Get existing mappings to ensure we don't overwrite them
-		cmd = []string{"ovn-nbctl", "get", "load_balancer", lbName, "ip_port_mappings"}
-		stdout, _, err = framework.NBExec(cmd...)
+		cmd := []string{"ovn-nbctl", "get", "load_balancer", lbName, "ip_port_mappings"}
+		stdout, _, err := framework.NBExec(cmd...)
 		framework.ExpectNil(err)
 		existingMappings := strings.TrimSpace(string(stdout))
 
@@ -129,9 +133,9 @@ var _ = framework.Describe("[group:gc]", func() {
 		}
 
 		ginkgo.By("4. Waiting for GC to clean up the stale entry")
-		// The default GC interval might be long, but in E2E tests we expect the controller to be running.
-		// If GC interval is e.g. 60s, we might need to wait.
-		framework.WaitUntil(5*time.Second, 2*time.Minute, func(_ context.Context) (bool, error) {
+		// Currently, the GC doesn't run periodically for this type of stuff...
+		// This test can be used to test the GC by end, but not yet in E2E testing
+		framework.WaitUntil(5*time.Second, 1*time.Minute, func(_ context.Context) (bool, error) {
 			stdout, _, err = framework.NBExec(cmd...)
 			if err != nil {
 				return false, err