[bugfix] Optimize gc method at port group and node (#4722)

* [bugfix] Optimize gc method at port group and node Signed-off-by: cmdy <[email protected]> * skip the centralized subnet policy Signed-off-by: cmdy <[email protected]> * fix gatewayRouterPolicies spelling error Signed-off-by: cmdy <[email protected]> --------- Signed-off-by: cmdy <[email protected]> Signed-off-by: oilbeater <[email protected]>
kubeovn · Nov 14, 2024 · b23c8da · b23c8da
1 parent b929a11
commit b23c8da
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 37 deletions.
diff --git a/pkg/controller/gc.go b/pkg/controller/gc.go
@@ -233,7 +233,7 @@ func (c *Controller) gcNode() error {
 		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 {
+				if err := c.deleteNode(node); err != nil {
 					klog.Errorf("failed to gc node %s: %v", node, err)
 					return err
 				}
@@ -246,11 +246,21 @@ func (c *Controller) gcNode() error {
 		klog.Errorf("failed to list logical router policies on lr %s: %v", c.config.ClusterRouter, err)
 		return err
 	}
+	gatewayRouterPolicies, err := c.OVNNbClient.ListLogicalRouterPolicies(c.config.ClusterRouter, util.GatewayRouterPolicyPriority, map[string]string{"vendor": util.CniTypeName}, false)
+	if err != nil {
+		klog.Errorf("failed to list logical router policies priority %d on lr %s: %v", util.GatewayRouterPolicyPriority, c.config.ClusterRouter, err)
+		return err
+	}
+	policies = append(policies, gatewayRouterPolicies...)
 	for _, policy := range policies {
+		// skip the policy for centralized subnet
+		if _, ok := policy.ExternalIDs["node"]; !ok {
+			continue
+		}
 		if nodeNames.Has(policy.ExternalIDs["node"]) {
 			continue
 		}
-		klog.Infof("gc logical router policy %q on lr %s", policy.Match, c.config.ClusterRouter)
+		klog.Infof("gc logical router policy %q priority %d on lr %s", policy.Match, policy.Priority, 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
@@ -605,6 +615,7 @@ func (c *Controller) gcPortGroup() error {
 	klog.Info("start to gc network policy")
 
 	npNames := strset.New()
+	delPgNames := strset.New()
 
 	if c.config.EnableNP {
 		nps, err := c.npsLister.List(labels.Everything())
@@ -622,53 +633,66 @@ func (c *Controller) gcPortGroup() error {
 
 			npNames.Add(fmt.Sprintf("%s/%s", np.Namespace, npName))
 		}
+	}
 
-		// append node port group to npNames to avoid gc node port group
-		nodes, err := c.nodesLister.List(labels.Everything())
-		if err != nil {
-			klog.Errorf("failed to list nodes, %v", err)
-			return err
-		}
+	// append node port group to npNames to avoid gc node port group
+	nodes, err := c.nodesLister.List(labels.Everything())
+	if err != nil {
+		klog.Errorf("failed to list nodes, %v", err)
+		return err
+	}
 
-		for _, node := range nodes {
-			npNames.Add(fmt.Sprintf("%s/%s", "node", node.Name))
-		}
+	for _, node := range nodes {
+		npNames.Add(fmt.Sprintf("%s/%s", "node", node.Name))
+	}
 
-		// append overlay subnets port group to npNames to avoid gc distributed subnets port group
-		subnets, err := c.subnetsLister.List(labels.Everything())
-		if err != nil {
-			klog.Errorf("failed to list subnets %v", err)
-			return err
+	// append overlay subnets port group to npNames to avoid gc distributed subnets port group
+	subnets, err := c.subnetsLister.List(labels.Everything())
+	if err != nil {
+		klog.Errorf("failed to list subnets %v", err)
+		return err
+	}
+	for _, subnet := range subnets {
+		if subnet.Spec.Vpc != c.config.ClusterRouter || (subnet.Spec.Vlan != "" && !subnet.Spec.LogicalGateway) || subnet.Name == c.config.NodeSwitch || subnet.Spec.GatewayType != kubeovnv1.GWDistributedType {
+			continue
 		}
-		for _, subnet := range subnets {
-			if subnet.Spec.Vpc != c.config.ClusterRouter || (subnet.Spec.Vlan != "" && !subnet.Spec.LogicalGateway) || subnet.Name == c.config.NodeSwitch || subnet.Spec.GatewayType != kubeovnv1.GWDistributedType {
-				continue
-			}
 
-			for _, node := range nodes {
-				npNames.Add(fmt.Sprintf("%s/%s", subnet.Name, node.Name))
-			}
+		for _, node := range nodes {
+			npNames.Add(fmt.Sprintf("%s/%s", subnet.Name, node.Name))
 		}
+	}
 
-		// list all np port groups which externalIDs[np]!=""
-		pgs, err := c.OVNNbClient.ListPortGroups(map[string]string{networkPolicyKey: ""})
-		if err != nil {
-			klog.Errorf("list np port group: %v", err)
-			return err
-		}
+	// list all np port groups which externalIDs[np]!=""
+	pgs, err := c.OVNNbClient.ListPortGroups(map[string]string{networkPolicyKey: ""})
+	if err != nil {
+		klog.Errorf("list np port group: %v", err)
+		return err
+	}
 
-		for _, pg := range pgs {
-			np := strings.Split(pg.ExternalIDs[networkPolicyKey], "/")
-			if len(np) != 2 {
-				// not np port group
-				continue
-			}
-			if !npNames.Has(pg.ExternalIDs[networkPolicyKey]) {
-				klog.Infof("gc port group '%s' network policy '%s'", pg.Name, pg.ExternalIDs[networkPolicyKey])
+	for _, pg := range pgs {
+		np := strings.Split(pg.ExternalIDs[networkPolicyKey], "/")
+		if len(np) != 2 {
+			// not np port group
+			continue
+		}
+		if !npNames.Has(pg.ExternalIDs[networkPolicyKey]) {
+			klog.Infof("gc port group '%s' network policy '%s'", pg.Name, pg.ExternalIDs[networkPolicyKey])
+			delPgNames.Add(pg.Name)
+			if c.config.EnableNP {
 				c.deleteNpQueue.Add(pg.ExternalIDs[networkPolicyKey])
 			}
 		}
 	}
+	// gc port group
+	// the pgName in the network policy is generated differently from the node/subnet pgName
+	// so processes port group gc separately
+	// ensure that the port group can be correctly gc
+	delPgNames.Each(func(item string) bool {
+		if err := c.OVNNbClient.DeletePortGroup(item); err != nil {
+			klog.Errorf("failed to gc port group %s: %v", item, err)
+		}
+		return true
+	})
 
 	return nil
 }

diff --git a/pkg/controller/node.go b/pkg/controller/node.go
@@ -482,7 +482,10 @@ func (c *Controller) handleDeleteNode(key string) error {
 		klog.Warningf("Node %s is adding, skip the node delete handler, but it may leave some gc resources behind", key)
 		return nil
 	}
+	return c.deleteNode(key)
+}
 
+func (c *Controller) deleteNode(key string) error {
 	portName := util.NodeLspName(key)
 	klog.Infof("delete logical switch port %s", portName)
 	if err := c.OVNNbClient.DeleteLogicalSwitchPort(portName); err != nil {