fix node gc

Signed-off-by: zhangzujian <[email protected]>
kubeovn · May 9, 2024 · 50f7e3d · 50f7e3d
1 parent 0068ce3
commit 50f7e3d
Show file tree

Hide file tree

Showing 9 changed files with 67 additions and 46 deletions.
diff --git a/pkg/controller/gc.go b/pkg/controller/gc.go
@@ -219,30 +219,44 @@ func (c *Controller) gcNode() error {
 		klog.Errorf("failed to list node, %v", err)
 		return err
 	}
-	nodeNames := make([]string, 0, len(nodes))
-	for _, no := range nodes {
-		nodeNames = append(nodeNames, no.Name)
+	nodeNames := strset.NewWithSize(len(nodes))
+	for _, node := range nodes {
+		nodeNames.Add(node.Name)
 	}
 	ips, err := c.ipsLister.List(labels.Everything())
 	if err != nil {
 		klog.Errorf("failed to list ip, %v", err)
 		return err
 	}
-	ipNodeNames := make([]string, 0, len(ips))
+
 	for _, ip := range ips {
-		if !strings.Contains(ip.Name, ".") {
-			ipNodeNames = append(ipNodeNames, strings.TrimPrefix(ip.Name, "node-"))
+		if strings.HasPrefix(ip.Name, util.NodeLspPrefix) && !strings.Contains(ip.Name, ".") {
+			if node := ip.Name[len(util.NodeLspPrefix):]; !nodeNames.Has(node) {
+				klog.Infof("gc node %s", node)
+				if err := c.handleDeleteNode(node); err != nil {
+					klog.Errorf("failed to gc node %s: %v", node, err)
+					return err
+				}
+			}
 		}
 	}
-	for _, no := range ipNodeNames {
-		if !slices.Contains(nodeNames, no) {
-			klog.Infof("gc node %s", no)
-			if err := c.handleDeleteNode(no); err != nil {
-				klog.Errorf("failed to gc node %s, %v", no, err)
-				return err
-			}
+
+	policies, err := c.OVNNbClient.ListLogicalRouterPolicies(c.config.ClusterRouter, util.NodeRouterPolicyPriority, map[string]string{"vendor": util.CniTypeName}, false)
+	if err != nil {
+		klog.Errorf("failed to list logical router policies on lr %s: %v", c.config.ClusterRouter, err)
+		return err
+	}
+	for _, policy := range policies {
+		if nodeNames.Has(policy.ExternalIDs["node"]) {
+			continue
+		}
+		klog.Infof("gc logical router policy %q on lr %s", policy.Match, c.config.ClusterRouter)
+		if err = c.OVNNbClient.DeleteLogicalRouterPolicy(c.config.ClusterRouter, policy.Priority, policy.Match); err != nil {
+			klog.Errorf("failed to delete logical router policy %q on lr %s", policy.Match, c.config.ClusterRouter)
+			return err
 		}
 	}
+
 	return nil
 }
 
@@ -321,7 +335,7 @@ func (c *Controller) markAndCleanLSP() error {
 	}
 	for _, node := range nodes {
 		if node.Annotations[util.AllocatedAnnotation] == "true" {
-			ipMap.Add(fmt.Sprintf("node-%s", node.Name))
+			ipMap.Add(util.NodeLspName(node.Name))
 		}
 
 		if _, err := c.ovnEipsLister.Get(node.Name); err == nil {

diff --git a/pkg/controller/init.go b/pkg/controller/init.go
@@ -339,7 +339,7 @@ func (c *Controller) InitIPAM() error {
 		if ip.Spec.Namespace != "" {
 			ipamKey = fmt.Sprintf("%s/%s", ip.Spec.Namespace, ip.Spec.PodName)
 		} else {
-			ipamKey = fmt.Sprintf("node-%s", ip.Spec.PodName)
+			ipamKey = util.NodeLspName(ip.Spec.PodName)
 		}
 		if _, _, _, err = c.ipam.GetStaticAddress(ipamKey, ip.Name, ip.Spec.IPAddress, &ip.Spec.MacAddress, ip.Spec.Subnet, true); err != nil {
 			klog.Errorf("failed to init IPAM from IP CR %s: %v", ip.Name, err)
@@ -433,7 +433,7 @@ func (c *Controller) InitIPAM() error {
 	}
 	for _, node := range nodes {
 		if node.Annotations[util.AllocatedAnnotation] == "true" {
-			portName := fmt.Sprintf("node-%s", node.Name)
+			portName := util.NodeLspName(node.Name)
 			mac := node.Annotations[util.MacAddressAnnotation]
 			v4IP, v6IP, _, err := c.ipam.GetStaticAddress(portName, portName,
 				node.Annotations[util.IPAddressAnnotation], &mac,
@@ -585,13 +585,14 @@ func (c *Controller) syncIPCR() error {
 	}
 
 	ipMap := strset.New(c.getVMLsps()...)
-	for _, ipCR := range ips {
-		ip := ipCR.DeepCopy()
+	for _, ip := range ips {
 		if !ip.DeletionTimestamp.IsZero() && len(ip.GetFinalizers()) != 0 {
 			klog.Infof("enqueue update for deleting ip %s", ip.Name)
 			c.updateIPQueue.Add(ip.Name)
+			continue
 		}
 		changed := false
+		ip = ip.DeepCopy()
 		if ipMap.Has(ip.Name) && ip.Spec.PodType == "" {
 			ip.Spec.PodType = util.VM
 			changed = true

diff --git a/pkg/controller/ip.go b/pkg/controller/ip.go
@@ -459,7 +459,7 @@ func (c *Controller) createOrUpdateIPCR(ipCRName, podName, ip, mac, subnetName,
 		switch {
 		case subnetName == c.config.NodeSwitch:
 			key = nodeName
-			ipName = fmt.Sprintf("node-%s", nodeName)
+			ipName = util.NodeLspName(nodeName)
 		case strings.HasPrefix(podName, util.U2OInterconnName[0:19]):
 			key = podName // interconn IP name
 			ipName = podName

diff --git a/pkg/controller/node.go b/pkg/controller/node.go
@@ -235,7 +235,7 @@ func (c *Controller) handleAddNode(key string) error {
 	}
 
 	var v4IP, v6IP, mac string
-	portName := fmt.Sprintf("node-%s", key)
+	portName := util.NodeLspName(key)
 	if node.Annotations[util.AllocatedAnnotation] == "true" && node.Annotations[util.IPAddressAnnotation] != "" && node.Annotations[util.MacAddressAnnotation] != "" {
 		macStr := node.Annotations[util.MacAddressAnnotation]
 		v4IP, v6IP, mac, err = c.ipam.GetStaticAddress(portName, portName, node.Annotations[util.IPAddressAnnotation],
@@ -328,7 +328,7 @@ func (c *Controller) handleAddNode(key string) error {
 	node.Annotations[util.GatewayAnnotation] = subnet.Spec.Gateway
 	node.Annotations[util.LogicalSwitchAnnotation] = c.config.NodeSwitch
 	node.Annotations[util.AllocatedAnnotation] = "true"
-	node.Annotations[util.PortNameAnnotation] = fmt.Sprintf("node-%s", key)
+	node.Annotations[util.PortNameAnnotation] = portName
 	raw, _ := json.Marshal(node.Annotations)
 	patchPayload := fmt.Sprintf(patchPayloadTemplate, op, raw)
 	_, err = c.config.KubeClient.CoreV1().Nodes().Patch(context.Background(), key, types.JSONPatchType, []byte(patchPayload), metav1.PatchOptions{}, "")
@@ -338,7 +338,7 @@ func (c *Controller) handleAddNode(key string) error {
 	}
 
 	if err := c.createOrUpdateIPCR("", "", ipStr, mac, c.config.NodeSwitch, "", node.Name, ""); err != nil {
-		klog.Errorf("failed to create or update IPs node-%s: %v", key, err)
+		klog.Errorf("failed to create or update IPs %s: %v", portName, err)
 		return err
 	}
 
@@ -473,23 +473,18 @@ func (c *Controller) handleDeleteNode(key string) error {
 	defer func() { _ = c.nodeKeyMutex.UnlockKey(key) }()
 	klog.Infof("handle delete node %s", key)
 
-	portName := fmt.Sprintf("node-%s", key)
+	portName := util.NodeLspName(key)
 	klog.Infof("delete logical switch port %s", portName)
 	if err := c.OVNNbClient.DeleteLogicalSwitchPort(portName); err != nil {
-		klog.Errorf("failed to delete node switch port node-%s: %v", key, err)
+		klog.Errorf("failed to delete node switch port %s: %v", portName, err)
 		return err
 	}
 	if err := c.OVNSbClient.DeleteChassisByHost(key); err != nil {
 		klog.Errorf("failed to delete chassis for node %s: %v", key, err)
 		return err
 	}
 
-	if err := c.config.KubeOvnClient.KubeovnV1().IPs().Delete(context.Background(), portName, metav1.DeleteOptions{}); err != nil && !k8serrors.IsNotFound(err) {
-		return err
-	}
-
-	afs := []int{4, 6}
-	for _, af := range afs {
+	for _, af := range [...]int{4, 6} {
 		if err := c.deletePolicyRouteForLocalDNSCacheOnNode(key, af); err != nil {
 			klog.Error(err)
 			return err
@@ -503,21 +498,11 @@ func (c *Controller) handleDeleteNode(key string) error {
 		return err
 	}
 
-	if err := c.deletePolicyRouteForNode(key); err != nil {
+	if err := c.deletePolicyRouteForNode(key, portName); err != nil {
 		klog.Errorf("failed to delete policy route for node %s: %v", key, err)
 		return err
 	}
 
-	addresses := c.ipam.GetPodAddress(portName)
-	for _, addr := range addresses {
-		if addr.IP == "" {
-			continue
-		}
-		if err := c.OVNNbClient.DeleteLogicalRouterPolicyByNexthop(c.config.ClusterRouter, util.NodeRouterPolicyPriority, addr.IP); err != nil {
-			klog.Errorf("failed to delete router policy for node %s: %v", key, err)
-			return err
-		}
-	}
 	if err := c.OVNNbClient.DeleteAddressSet(nodeUnderlayAddressSetName(key, 4)); err != nil {
 		klog.Errorf("failed to delete address set for node %s: %v", key, err)
 		return err
@@ -543,6 +528,10 @@ func (c *Controller) handleDeleteNode(key string) error {
 		}
 	}
 
+	if err = c.config.KubeOvnClient.KubeovnV1().IPs().Delete(context.Background(), portName, metav1.DeleteOptions{}); err != nil && !k8serrors.IsNotFound(err) {
+		return err
+	}
+
 	return nil
 }
 
@@ -987,13 +976,24 @@ func (c *Controller) checkPolicyRouteExistForNode(nodeName, cidr, nexthop string
 	return false, nil
 }
 
-func (c *Controller) deletePolicyRouteForNode(nodeName string) error {
+func (c *Controller) deletePolicyRouteForNode(nodeName, portName string) error {
 	subnets, err := c.subnetsLister.List(labels.Everything())
 	if err != nil {
 		klog.Errorf("get subnets: %v", err)
 		return err
 	}
 
+	addresses := c.ipam.GetPodAddress(portName)
+	for _, addr := range addresses {
+		if addr.IP == "" {
+			continue
+		}
+		if err = c.OVNNbClient.DeleteLogicalRouterPolicyByNexthop(c.config.ClusterRouter, util.NodeRouterPolicyPriority, addr.IP); err != nil {
+			klog.Errorf("failed to delete router policy with nexthop %q from %s for node %s: %v", addr.IP, c.config.ClusterRouter, nodeName, err)
+			return err
+		}
+	}
+
 	for _, subnet := range subnets {
 		if (subnet.Spec.Vlan != "" && !subnet.Spec.LogicalGateway) || subnet.Spec.Vpc != c.config.ClusterRouter || subnet.Name == c.config.NodeSwitch {
 			continue

diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go
@@ -2545,7 +2545,7 @@ func (c *Controller) deletePolicyRouteForDistributedSubnet(subnet *kubeovnv1.Sub
 		}
 		pgAs := fmt.Sprintf("%s_%s", pgName, ipSuffix)
 		match := fmt.Sprintf("%s.src == $%s", ipSuffix, pgAs)
-		klog.Infof("delete policy route for router: %s, priority: %d, match %s", c.config.ClusterRouter, util.GatewayRouterPolicyPriority, match)
+		klog.Infof("delete policy route for router: %s, priority: %d, match: %q", c.config.ClusterRouter, util.GatewayRouterPolicyPriority, match)
 		if err := c.deletePolicyRouteFromVpc(c.config.ClusterRouter, util.GatewayRouterPolicyPriority, match); err != nil {
 			klog.Errorf("failed to delete policy route for subnet %s: %v", subnet.Name, err)
 			return err
@@ -3067,7 +3067,7 @@ func (c *Controller) addPolicyRouteForU2ONoLoadBalancer(subnet *kubeovnv1.Subnet
 			klog.Errorf("failed to create u2o port group for subnet %s and node %s: %v", subnet.Name, node.Name, err)
 			return err
 		}
-		key := fmt.Sprintf("node-%s", node.Name)
+		key := util.NodeLspName(node.Name)
 		ip, err := c.ipsLister.Get(key)
 		if err != nil {
 			if k8serrors.IsNotFound(err) {

diff --git a/pkg/daemon/gateway.go b/pkg/daemon/gateway.go
@@ -46,7 +46,7 @@ func (c *Controller) setGatewayBandwidth() error {
 		return err
 	}
 	ingress, egress := node.Annotations[util.IngressRateAnnotation], node.Annotations[util.EgressRateAnnotation]
-	ifaceID := fmt.Sprintf("node-%s", c.config.NodeName)
+	ifaceID := util.NodeLspName(c.config.NodeName)
 	if ingress == "" && egress == "" {
 		if htbQos, _ := ovs.IsHtbQos(ifaceID); !htbQos {
 			return nil

diff --git a/pkg/util/const.go b/pkg/util/const.go
@@ -134,6 +134,7 @@ const (
 	BindMountPath = "/run/netns"
 
 	NodeNic           = "ovn0"
+	NodeLspPrefix     = "node-"
 	NodeAllowPriority = "3000"
 
 	SecurityGroupHighestPriority = "2300"

diff --git a/pkg/util/ovn.go b/pkg/util/ovn.go
@@ -0,0 +1,5 @@
+package util
+
+func NodeLspName(node string) string {
+	return NodeLspPrefix + node
+}
diff --git a/test/e2e/kube-ovn/node/node.go b/test/e2e/kube-ovn/node/node.go
@@ -79,7 +79,7 @@ var _ = framework.OrderedDescribe("[group:node]", func() {
 			framework.ExpectIPInCIDR(node.Annotations[util.IPAddressAnnotation], join.Spec.CIDRBlock)
 			framework.ExpectHaveKeyWithValue(node.Annotations, util.LogicalSwitchAnnotation, join.Name)
 			framework.ExpectMAC(node.Annotations[util.MacAddressAnnotation])
-			framework.ExpectHaveKeyWithValue(node.Annotations, util.PortNameAnnotation, "node-"+node.Name)
+			framework.ExpectHaveKeyWithValue(node.Annotations, util.PortNameAnnotation, util.NodeLspName(node.Name))
 
 			podName = "pod-" + framework.RandomSuffix()
 			ginkgo.By("Creating pod " + podName + " with host network")