[bugfix] Unable to correctly gc port group (#4694)

Signed-off-by: cmdy <[email protected]> Co-authored-by: zhanglin02 <[email protected]>
kubeovn · Nov 11, 2024 · c328da4 · c328da4
1 parent ee560e8
commit c328da4
Showing 1 changed file with 49 additions and 35 deletions.
diff --git a/pkg/controller/gc.go b/pkg/controller/gc.go
@@ -605,6 +605,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 +623,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
 }