subnet: forbid subnet vpc change

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

Author: oilbeater

pkg/controller/controller.go

@@ -52,7 +52,6 @@ const (
 	portGroupKey                  = "pg"
 	networkPolicyKey              = "np"
 	sgKey                         = "sg"
-	associatedSgKeyPrefix         = "associated_sg_"
 	sgsKey                        = "security_groups"
 	u2oKey                        = "u2o"
 	adminNetworkPolicyKey         = "anp"
@@ -125,7 +124,6 @@ type Controller struct {
 	subnetsLister           kubeovnlister.SubnetLister
 	subnetSynced            cache.InformerSynced
 	addOrUpdateSubnetQueue  workqueue.TypedRateLimitingInterface[string]
-	subnetLastVpcNameMap    *xsync.Map[string, string]
 	deleteSubnetQueue       workqueue.TypedRateLimitingInterface[*kubeovnv1.Subnet]
 	updateSubnetStatusQueue workqueue.TypedRateLimitingInterface[string]
 	syncVirtualPortsQueue   workqueue.TypedRateLimitingInterface[string]
@@ -431,7 +429,6 @@ func Run(ctx context.Context, config *Configuration) {
 		subnetsLister:           subnetInformer.Lister(),
 		subnetSynced:            subnetInformer.Informer().HasSynced,
 		addOrUpdateSubnetQueue:  newTypedRateLimitingQueue[string]("AddSubnet", nil),
-		subnetLastVpcNameMap:    xsync.NewMap[string, string](),
 		deleteSubnetQueue:       newTypedRateLimitingQueue[*kubeovnv1.Subnet]("DeleteSubnet", nil),
 		updateSubnetStatusQueue: newTypedRateLimitingQueue[string]("UpdateSubnetStatus", nil),
 		syncVirtualPortsQueue:   newTypedRateLimitingQueue[string]("SyncVirtualPort", nil),

pkg/controller/subnet.go

@@ -77,18 +77,6 @@ func (c *Controller) enqueueUpdateSubnet(oldObj, newObj any) {
 		return
 	}
 
-	if oldSubnet.Spec.Vpc != newSubnet.Spec.Vpc &&
-		((oldSubnet.Spec.Vpc != "" || newSubnet.Spec.Vpc != c.config.ClusterRouter) && (oldSubnet.Spec.Vpc != c.config.ClusterRouter || newSubnet.Spec.Vpc != "")) {
-		// recode last vpc name for subnet
-		if oldSubnet.Spec.Vpc == "" {
-			c.subnetLastVpcNameMap.Store(newSubnet.Name, c.config.ClusterRouter)
-		} else {
-			c.subnetLastVpcNameMap.Store(newSubnet.Name, oldSubnet.Spec.Vpc)
-		}
-
-		c.updateVpcStatusQueue.Add(oldSubnet.Spec.Vpc)
-	}
-
 	if oldSubnet.Spec.U2OInterconnection != newSubnet.Spec.U2OInterconnection {
 		klog.Infof("enqueue update vpc %s triggered by u2o interconnection change of subnet %s", newSubnet.Spec.Vpc, key)
 		c.addOrUpdateVpcQueue.Add(newSubnet.Spec.Vpc)
@@ -482,9 +470,9 @@ func (c *Controller) validateVpcBySubnet(subnet *kubeovnv1.Subnet) (*kubeovnv1.V
 			klog.Errorf("failed to list vpc, %v", err)
 			return vpc, err
 		}
-		lastVpcName, _ := c.subnetLastVpcNameMap.Load(subnet.Name)
+
 		for _, vpc := range vpcs {
-			if (lastVpcName == "" && subnet.Spec.Vpc != vpc.Name || lastVpcName != "" && lastVpcName != vpc.Name) &&
+			if subnet.Spec.Vpc != vpc.Name &&
 				!vpc.Status.Default && util.IsStringsOverlap(vpc.Spec.Namespaces, subnet.Spec.Namespaces) {
 				err = fmt.Errorf("namespaces %v are overlap with vpc '%s'", subnet.Spec.Namespaces, vpc.Name)
 				klog.Error(err)
@@ -1035,9 +1023,6 @@ func (c *Controller) handleDeleteSubnet(subnet *kubeovnv1.Subnet) error {
 		}
 	}
 
-	// clean up subnet last vpc name cached
-	c.subnetLastVpcNameMap.Delete(subnet.Name)
-
 	return nil
 }
 

pkg/util/const.go

@@ -122,9 +122,6 @@ const (
 	VpcEgressGatewayLabel  = "ovn.kubernetes.io/vpc-egress-gateway"
 	GenerateHashAnnotation = "ovn.kubernetes.io/generate-hash"
 
-	VpcLastName     = "ovn.kubernetes.io/last_vpc_name"
-	VpcLastPolicies = "ovn.kubernetes.io/last_policies"
-
 	ServiceExternalIPFromSubnetAnnotation = "ovn.kubernetes.io/service_external_ip_from_subnet"
 	ServiceHealthCheck                    = "ovn.kubernetes.io/service_health_check"
 

pkg/webhook/subnet.go

@@ -69,6 +69,12 @@ func (v *ValidatingHook) SubnetUpdateHook(ctx context.Context, req admission.Req
 		return ctrlwebhook.Denied("can't update gateway of cidr when any IPs in Using")
 	}
 
+	if o.Spec.Vpc != oldSubnet.Spec.Vpc {
+		if oldSubnet.Spec.Vpc != "" || o.Spec.Vpc != util.DefaultVpc {
+			return ctrlwebhook.Denied("vpc can only be changed from empty to ovn-cluster")
+		}
+	}
+
 	if err := util.ValidateSubnet(o); err != nil {
 		return ctrlwebhook.Denied(err.Error())
 	}

test/e2e/webhook/subnet/subnet.go

@@ -90,4 +90,42 @@ var _ = framework.Describe("[group:webhook-subnet]", func() {
 		_, err = subnetClient.SubnetInterface.Create(context.TODO(), subnet, metav1.CreateOptions{})
 		framework.ExpectError(err, "subnet %s cidr %s is invalid", subnet.Name, subnet.Spec.CIDRBlock)
 	})
+
+	framework.ConformanceIt("check subnet vpc update validation", func() {
+		ginkgo.By("Creating subnet " + subnetName)
+		subnet := framework.MakeSubnet(subnetName, "", cidr, "", "", "", nil, nil, nil)
+		subnet = subnetClient.CreateSync(subnet)
+
+		ginkgo.By("Validating vpc can be changed from empty to ovn-cluster")
+		modifiedSubnet := subnet.DeepCopy()
+		modifiedSubnet.Spec.Vpc = util.DefaultVpc
+		_, err := subnetClient.SubnetInterface.Update(context.TODO(), modifiedSubnet, metav1.UpdateOptions{})
+		framework.ExpectNoError(err)
+
+		ginkgo.By("Validating vpc cannot be changed from ovn-cluster to another value")
+		subnet = subnetClient.Get(subnetName)
+		modifiedSubnet = subnet.DeepCopy()
+		modifiedSubnet.Spec.Vpc = "test-vpc"
+		_, err = subnetClient.SubnetInterface.Update(context.TODO(), modifiedSubnet, metav1.UpdateOptions{})
+		framework.ExpectError(err, "vpc can only be changed from empty to ovn-cluster")
+
+		ginkgo.By("Validating vpc cannot be changed from ovn-cluster to empty")
+		subnet = subnetClient.Get(subnetName)
+		modifiedSubnet = subnet.DeepCopy()
+		modifiedSubnet.Spec.Vpc = ""
+		_, err = subnetClient.SubnetInterface.Update(context.TODO(), modifiedSubnet, metav1.UpdateOptions{})
+		framework.ExpectError(err, "vpc can only be changed from empty to ovn-cluster")
+	})
+
+	framework.ConformanceIt("check subnet vpc cannot be set to non-ovn-cluster on update", func() {
+		ginkgo.By("Creating subnet " + subnetName + " with empty vpc")
+		subnet := framework.MakeSubnet(subnetName, "", cidr, "", "", "", nil, nil, nil)
+		subnet = subnetClient.CreateSync(subnet)
+
+		ginkgo.By("Validating vpc cannot be changed from empty to non-ovn-cluster value")
+		modifiedSubnet := subnet.DeepCopy()
+		modifiedSubnet.Spec.Vpc = "test-vpc"
+		_, err := subnetClient.SubnetInterface.Update(context.TODO(), modifiedSubnet, metav1.UpdateOptions{})
+		framework.ExpectError(err, "vpc can only be changed from empty to ovn-cluster")
+	})
 })