fix(e2e): eliminate race between VIP finalizer addition and WaitToBeReady

Root cause: handleAddVirtualIP called createOrUpdateVipCR to set
Status.V4ip (triggering WaitToBeReady), but the finalizer was only
added later in handleUpdateVirtualIP (triggered by a subsequent
update event). This created a race window where CreateSync could
return a VIP object with V4ip set but without the finalizer.

Fix 1 (controller): In createOrUpdateVipCR's else branch, add the
finalizer atomically in the same Update() call that sets spec/status,
so the VIP is fully initialized in one API operation.

Fix 2 (test framework): Update WaitToBeReady to require both an IP
address AND the controller finalizer before declaring a VIP ready,
ensuring tests only proceed with a fully-initialized VIP.

Fix 3 (test): Add ginkgo.DeferCleanup for testVipName inside the
It block so the VIP is deleted even on test failure, preventing
the AfterEach subnetClient.DeleteSync from timing out.

Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com>

Author: oilbeater

pkg/controller/vip.go

@@ -424,8 +424,14 @@ func (c *Controller) createOrUpdateVipCR(key, ns, subnet, v4ip, v6ip, mac string
 			vip.Status.V6ip = v6ip
 			vip.Status.Mac = mac
 			vip.Status.Type = vip.Spec.Type
-			// TODO:// Ready = true as subnet.Status.Ready
-			// Update with labels, spec and status in one call
+
+			// Ensure finalizer is added atomically with status initialization,
+			// preventing a race where WaitToBeReady returns (V4ip is set) before
+			// handleUpdateVirtualIP has a chance to add the finalizer.
+			controllerutil.RemoveFinalizer(vip, util.DeprecatedFinalizerName)
+			controllerutil.AddFinalizer(vip, util.KubeOVNControllerFinalizer)
+
+			// Update with labels, spec, status, and finalizer in one call
 			if _, err := c.config.KubeOvnClient.KubeovnV1().Vips().Update(context.Background(), vip, metav1.UpdateOptions{}); err != nil {
 				err := fmt.Errorf("failed to update vip '%s', %w", key, err)
 				klog.Error(err)

test/e2e/framework/vip.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"slices"
 	"time"
 
 	"github.com/onsi/ginkgo/v2"
@@ -58,10 +59,14 @@ func (c *VipClient) CreateSync(vip *apiv1.Vip) *apiv1.Vip {
 }
 
 // WaitToBeReady returns whether the ovn vip is ready within timeout.
+// A VIP is considered ready when it has an IP assigned AND has the controller finalizer,
+// ensuring the controller has fully processed the VIP before tests proceed.
 func (c *VipClient) WaitToBeReady(name string, timeout time.Duration) bool {
 	Logf("Waiting up to %v for ovn vip %s to be ready", timeout, name)
 	for start := time.Now(); time.Since(start) < timeout; time.Sleep(poll) {
-		if c.Get(name).Status.V4ip != "" || c.Get(name).Status.V6ip != "" {
+		vip := c.Get(name)
+		if (vip.Status.V4ip != "" || vip.Status.V6ip != "") &&
+			slices.Contains(vip.GetFinalizers(), util.KubeOVNControllerFinalizer) {
 			Logf("ovn vip %s is ready", name)
 			return true
 		}

test/e2e/vip/e2e_test.go

@@ -219,8 +219,13 @@ var _ = framework.Describe("[group:vip]", func() {
 		testVipName := "test-vip-finalizer-" + framework.RandomSuffix()
 		testVip := makeOvnVip(namespaceName, testVipName, subnetName, "", "", "")
 		testVip = vipClient.CreateSync(testVip)
+		// Ensure the VIP is cleaned up even if the test fails early,
+		// otherwise the subnet deletion in AfterEach will time out.
+		ginkgo.DeferCleanup(func() {
+			vipClient.DeleteSync(testVipName)
+		})
 
-		// Verify VIP has finalizer
+		// Verify VIP has finalizer (CreateSync now waits for both IP and finalizer)
 		framework.ExpectContainElement(testVip.Finalizers, util.KubeOVNControllerFinalizer)
 
 		ginkgo.By("3. Wait for subnet status to be updated after VIP creation")