fix node gc (#3992)

Signed-off-by: zhangzujian <zhangzujian.7@gmail.com>

pkg/controller/gc.go

@@ -217,30 +217,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 !util.IsStringIn(no, nodeNames) {
-			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
 }
 
@@ -326,7 +340,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 {

pkg/controller/init.go

@@ -329,7 +329,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)
@@ -421,7 +421,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,

pkg/controller/node.go

@@ -234,7 +234,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],
@@ -327,7 +327,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{}, "")
@@ -337,7 +337,7 @@ func (c *Controller) handleAddNode(key string) error {
 	}
 
 	if err := c.createOrUpdateCrdIPs("", 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
 	}
 
@@ -472,23 +472,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 {
 			return err
 		}
@@ -501,21 +496,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
@@ -540,6 +525,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
 }
 
@@ -649,7 +638,7 @@ func (c *Controller) createOrUpdateCrdIPs(podName, ip, mac, subnetName, ns, node
 	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
 		ipName = podName
@@ -1082,13 +1071,25 @@ 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
+		}
+		klog.Infof("deleting logical router policy with nexthop %q from %s for node %s", addr.IP, c.config.ClusterRouter, nodeName)
+		if err = c.OVNNbClient.DeleteLogicalRouterPolicyByNexthop(c.config.ClusterRouter, util.NodeRouterPolicyPriority, addr.IP); err != nil {
+			klog.Errorf("failed to delete logical 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

pkg/controller/subnet.go

@@ -2517,7 +2517,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

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

pkg/util/const.go

@@ -129,6 +129,7 @@ const (
 	BindMountPath = "/run/netns"
 
 	NodeNic           = "ovn0"
+	NodeLspPrefix     = "node-"
 	NodeAllowPriority = "3000"
 
 	SecurityGroupHighestPriority = "2300"

pkg/util/ovn.go

@@ -0,0 +1,5 @@
+package util
+
+func NodeLspName(node string) string {
+	return NodeLspPrefix + node
+}