feat: Improve subnet and VPC finalizer handling (#5071)

- Add finalizer to subnet and VPC resources if not present
- Remove VPC finalizer when no subnets exist
- Enhance VPC status management by adding subnets to queue when empty
- Use slices.Contains for more idiomatic finalizer checks

Signed-off-by: Mengxin Liu <[email protected]>

pkg/controller/subnet.go

@@ -361,7 +361,7 @@ func (c *Controller) syncSubnetFinalizer(cl client.Client) error {
 }
 
 func (c *Controller) handleSubnetFinalizer(subnet *kubeovnv1.Subnet) (*kubeovnv1.Subnet, bool, error) {
-	if subnet.DeletionTimestamp.IsZero() && len(subnet.GetFinalizers()) == 0 {
+	if subnet.DeletionTimestamp.IsZero() && !slices.Contains(subnet.GetFinalizers(), util.KubeOVNControllerFinalizer) {
 		newSubnet := subnet.DeepCopy()
 		controllerutil.AddFinalizer(newSubnet, util.KubeOVNControllerFinalizer)
 		patch, err := util.GenerateMergePatchPayload(subnet, newSubnet)

pkg/controller/vpc.go

@@ -21,6 +21,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/klog/v2"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
 	"github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb"
@@ -156,6 +157,7 @@ func (c *Controller) handleUpdateVpcStatus(key string) error {
 
 	vpc.Status.DefaultLogicalSwitch = defaultSubnet
 	vpc.Status.Subnets = subnets
+
 	if !vpc.Spec.BFDPort.IsEnabled() && !vpc.Status.BFDPort.IsEmpty() {
 		vpc.Status.BFDPort.Clear()
 	}
@@ -170,6 +172,12 @@ func (c *Controller) handleUpdateVpcStatus(key string) error {
 		klog.Error(err)
 		return err
 	}
+
+	if len(vpc.Status.Subnets) == 0 {
+		klog.Infof("vpc %s has no subnets, add to queue", vpc.Name)
+		c.addOrUpdateVpcQueue.AddAfter(vpc.Name, 5*time.Second)
+	}
+
 	if change {
 		for _, ns := range vpc.Spec.Namespaces {
 			c.addNamespaceQueue.Add(ns)
@@ -262,6 +270,7 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error {
 		klog.Errorf("failed to format vpc %s: %v", key, err)
 		return err
 	}
+
 	if err = c.createVpcRouter(key); err != nil {
 		klog.Errorf("failed to create vpc router for vpc %s: %v", key, err)
 		return err
@@ -1094,6 +1103,16 @@ func (c *Controller) formatVpc(vpc *kubeovnv1.Vpc) (*kubeovnv1.Vpc, error) {
 		}
 	}
 
+	if vpc.DeletionTimestamp.IsZero() && !slices.Contains(vpc.GetFinalizers(), util.KubeOVNControllerFinalizer) {
+		controllerutil.AddFinalizer(vpc, util.KubeOVNControllerFinalizer)
+		changed = true
+	}
+
+	if !vpc.DeletionTimestamp.IsZero() && len(vpc.Status.Subnets) == 0 {
+		controllerutil.RemoveFinalizer(vpc, util.KubeOVNControllerFinalizer)
+		changed = true
+	}
+
 	if changed {
 		newVpc, err := c.config.KubeOvnClient.KubeovnV1().Vpcs().Update(context.Background(), vpc, metav1.UpdateOptions{})
 		if err != nil {

test/e2e/iptables-vpc-nat-gw/e2e_test.go

@@ -421,6 +421,8 @@ var _ = framework.SerialDescribe("[group:iptables-vpc-nat-gw]", func() {
 		cm.Data["image"] = oldImage
 		_, err = f.ClientSet.CoreV1().ConfigMaps(framework.KubeOvnNamespace).Update(context.Background(), cm, metav1.UpdateOptions{})
 		framework.ExpectNoError(err)
+		vpcNatGwClient.DeleteSync(vpcNatGwName)
+		subnetClient.DeleteSync(overlaySubnetName + "image")
 	})
 
 	framework.ConformanceIt("iptables eip fip snat dnat", func() {
@@ -545,16 +547,8 @@ var _ = framework.SerialDescribe("[group:iptables-vpc-nat-gw]", func() {
 		vipClient.DeleteSync(dnatVipName)
 		ginkgo.By("Deleting vip " + sharedVipName)
 		vipClient.DeleteSync(sharedVipName)
-
-		ginkgo.By("Deleting custom vpc " + vpcName)
-		vpcClient.DeleteSync(vpcName)
-
-		ginkgo.By("Deleting custom vpc nat gw")
-		vpcNatGwClient.DeleteSync(vpcNatGwName)
-
 		// the only pod for vpc nat gateway
 		vpcNatGwPodName := util.GenNatGwPodName(vpcNatGwName)
-
 		// delete vpc nat gw statefulset remaining ip for eth0 and net1
 		overlaySubnet := subnetClient.Get(overlaySubnetName)
 		macvlanSubnet := subnetClient.Get(networkAttachDefName)
@@ -568,6 +562,12 @@ var _ = framework.SerialDescribe("[group:iptables-vpc-nat-gw]", func() {
 		ginkgo.By("Deleting overlay subnet " + overlaySubnetName)
 		subnetClient.DeleteSync(overlaySubnetName)
 
+		ginkgo.By("Deleting custom vpc nat gw")
+		vpcNatGwClient.DeleteSync(vpcNatGwName)
+
+		ginkgo.By("Deleting custom vpc " + vpcName)
+		vpcClient.DeleteSync(vpcName)
+
 		// multiple external network case
 		net2OverlaySubnetV4Cidr := "10.0.1.0/24"
 		net2OoverlaySubnetV4Gw := "10.0.1.1"
@@ -590,9 +590,6 @@ var _ = framework.SerialDescribe("[group:iptables-vpc-nat-gw]", func() {
 		ginkgo.By("Deleting iptables eip " + net2EipName)
 		iptablesEIPClient.DeleteSync(net2EipName)
 
-		ginkgo.By("Deleting custom vpc " + net2VpcName)
-		vpcClient.DeleteSync(net2VpcName)
-
 		ginkgo.By("Deleting custom vpc nat gw")
 		vpcNatGwClient.DeleteSync(net2VpcNatGwName)
 
@@ -614,6 +611,9 @@ var _ = framework.SerialDescribe("[group:iptables-vpc-nat-gw]", func() {
 
 		ginkgo.By("Deleting overlay subnet " + net2OverlaySubnetName)
 		subnetClient.DeleteSync(net2OverlaySubnetName)
+
+		ginkgo.By("Deleting custom vpc " + net2VpcName)
+		vpcClient.DeleteSync(net2VpcName)
 	})
 })
 
@@ -1318,12 +1318,6 @@ var _ = framework.Describe("[group:qos-policy]", func() {
 			ginkgo.By("Deleting pod " + vpcQosParams.vpc2PodName)
 			podClient.DeleteSync(vpcQosParams.vpc2PodName)
 
-			ginkgo.By("Deleting custom vpc " + vpcQosParams.vpc1Name)
-			vpcClient.DeleteSync(vpcQosParams.vpc1Name)
-
-			ginkgo.By("Deleting custom vpc " + vpcQosParams.vpc2Name)
-			vpcClient.DeleteSync(vpcQosParams.vpc2Name)
-
 			ginkgo.By("Deleting custom vpc nat gw " + vpcQosParams.vpcNat1GwName)
 			vpcNatGwClient.DeleteSync(vpcQosParams.vpcNat1GwName)
 
@@ -1357,6 +1351,12 @@ var _ = framework.Describe("[group:qos-policy]", func() {
 			ipClient.DeleteSync(net1IpName)
 			ginkgo.By("Deleting overlay subnet " + vpcQosParams.vpc2SubnetName)
 			subnetClient.DeleteSync(vpcQosParams.vpc2SubnetName)
+
+			ginkgo.By("Deleting custom vpc " + vpcQosParams.vpc1Name)
+			vpcClient.DeleteSync(vpcQosParams.vpc1Name)
+
+			ginkgo.By("Deleting custom vpc " + vpcQosParams.vpc2Name)
+			vpcClient.DeleteSync(vpcQosParams.vpc2Name)
 		})
 		framework.ConformanceIt("default nic qos", func() {
 			// case 1: set qos policy for natgw

test/e2e/kube-ovn/pod/vpc_pod_probe.go

@@ -30,7 +30,7 @@ var _ = framework.SerialDescribe("[group:pod]", func() {
 	var eventClient *framework.EventClient
 	var subnetClient *framework.SubnetClient
 	var vpcClient *framework.VpcClient
-	var namespaceName, subnetName, podName, vpcName string
+	var namespaceName, subnetName, podName, vpcName, custVPCSubnetName string
 	var subnet *apiv1.Subnet
 	var cidr string
 
@@ -43,6 +43,7 @@ var _ = framework.SerialDescribe("[group:pod]", func() {
 		subnetName = "subnet-" + framework.RandomSuffix()
 		podName = "pod-" + framework.RandomSuffix()
 		cidr = framework.RandomCIDR(f.ClusterIPFamily)
+		custVPCSubnetName = "subnet-" + framework.RandomSuffix()
 
 		ginkgo.By("Creating subnet " + subnetName)
 		subnet = framework.MakeSubnet(subnetName, "", cidr, "", "", "", nil, nil, []string{namespaceName})
@@ -54,6 +55,7 @@ var _ = framework.SerialDescribe("[group:pod]", func() {
 
 		ginkgo.By("Deleting subnet " + subnetName)
 		subnetClient.DeleteSync(subnetName)
+		subnetClient.DeleteSync(custVPCSubnetName)
 
 		ginkgo.By("Deleting VPC " + vpcName)
 		vpcClient.DeleteSync(vpcName)
@@ -78,12 +80,6 @@ var _ = framework.SerialDescribe("[group:pod]", func() {
 		modifyDs.Spec.Template.Spec.Containers[0].Args = newArgs
 		daemonSetClient.PatchSync(modifyDs)
 
-		custVPCSubnetName := "subnet-" + framework.RandomSuffix()
-		ginkgo.DeferCleanup(func() {
-			ginkgo.By("Deleting subnet " + custVPCSubnetName)
-			subnetClient.DeleteSync(custVPCSubnetName)
-		})
-
 		ginkgo.By("Creating VPC " + vpcName)
 		vpcName = "vpc-" + framework.RandomSuffix()
 		customVPC := framework.MakeVpc(vpcName, "", false, false, nil)