From c328da4b67a258ea846c3ee6ad22a64d9a1ef749 Mon Sep 17 00:00:00 2001
From: cmdy <zhang_lin66@foxmail.com>
Date: Mon, 11 Nov 2024 14:48:55 +0800
Subject: [PATCH] [bugfix] Unable to correctly gc port group (#4694)

Signed-off-by: cmdy <zhang_lin66@foxmail.com>
Co-authored-by: zhanglin02 <zhanglin02@kanzhun.com>
---
 pkg/controller/gc.go | 84 ++++++++++++++++++++++++++------------------
 1 file changed, 49 insertions(+), 35 deletions(-)

diff --git a/pkg/controller/gc.go b/pkg/controller/gc.go
index b196634dada..9406e955019 100644
--- 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
 }