Improve Subnet/SubnetSet GC

Signed-off-by: Wenqi Qiu <[email protected]>
vmware-tanzu · Oct 11, 2024 · 8c01f6e · 8c01f6e
1 parent 762863a
commit 8c01f6e
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 49 deletions.
diff --git a/pkg/controllers/subnet/subnet_controller.go b/pkg/controllers/subnet/subnet_controller.go
@@ -379,31 +379,22 @@ func (r *SubnetReconciler) collectGarbage(ctx context.Context) {
 		log.Error(err, "Failed to list Subnet CRs")
 		return
 	}
-
-	var nsxSubnetList []*model.VpcSubnet
-	for _, crdSubnetID := range crdSubnetIDs {
-		nsxSubnetList = append(nsxSubnetList, r.SubnetService.ListSubnetCreatedBySubnet(crdSubnetID)...)
-	}
-	if len(nsxSubnetList) == 0 {
-		log.Info("No Subnets found in NSX, garbage collection complete")
-		return
-	}
-
 	crdSubnetIDsSet := sets.NewString(crdSubnetIDs...)
-	for _, nsxSubnet := range nsxSubnetList {
-		uid := nsxutil.FindTag(nsxSubnet.Tags, servicecommon.TagScopeSubnetCRUID)
+
+	subnetUIDs := r.SubnetService.SubnetStore.ListIndexFuncValues(servicecommon.TagScopeSubnetCRUID)
+	for uid := range subnetUIDs {
 		if crdSubnetIDsSet.Has(uid) {
 			continue
 		}
-
-		log.Info("GC collected Subnet CR", "UID", uid)
+		nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetCRUID, string(uid))
 		metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteTotal, common.MetricResTypeSubnet)
 
-		if err := r.SubnetService.DeleteSubnet(*nsxSubnet); err != nil {
-			log.Error(err, "Failed to delete NSX subnet", "NSX Subnet UID", uid)
+		log.Info("Subnet garbage collection, cleaning stale Subnets", "Count", len(nsxSubnets))
+		if err := r.deleteSubnets(nsxSubnets); err != nil {
+			log.Error(err, "Subnet garbage collection, failed to delete NSX subnet", "NSXSubnetUID", uid)
 			metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteFailTotal, common.MetricResTypeSubnet)
 		} else {
-			log.Info("Successfully deleted NSX subnet", "NSX Subnet UID", uid)
+			log.Info("Subnet garbage collection, successfully deleted NSX subnet", "NSXSubnetUID", uid)
 			metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteSuccessTotal, common.MetricResTypeSubnet)
 		}
 	}

diff --git a/pkg/controllers/subnetset/subnetset_controller.go b/pkg/controllers/subnetset/subnetset_controller.go
@@ -261,39 +261,30 @@ func (r *SubnetSetReconciler) CollectGarbage(ctx context.Context) {
 	defer func() {
 		log.Info("SubnetSet garbage collection completed", "duration(ms)", time.Since(startTime).Milliseconds())
 	}()
-	subnetSetList := &v1alpha1.SubnetSetList{}
-	err := r.Client.List(ctx, subnetSetList)
+
+	crdSubnetSetIDs, err := r.listSubnetSetIDsFromCRs(ctx)
 	if err != nil {
-		log.Error(err, "Failed to list SubnetSet CR")
-		return
-	}
-	var nsxSubnetList []*model.VpcSubnet
-	for _, subnetSet := range subnetSetList.Items {
-		nsxSubnetList = append(nsxSubnetList, r.SubnetService.ListSubnetCreatedBySubnetSet(string(subnetSet.UID))...)
-	}
-	if len(nsxSubnetList) == 0 {
+		log.Error(err, "Failed to list Subnet CRs")
 		return
 	}
+	crdSubnetSetIDsSet := sets.NewString(crdSubnetSetIDs...)
 
-	subnetSetIDs := sets.New[string]()
-	for _, subnetSet := range subnetSetList.Items {
-		if err := r.deleteSubnetForSubnetSet(subnetSet, true); err != nil {
-			metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteFailTotal, MetricResTypeSubnetSet)
-		} else {
-			metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteSuccessTotal, MetricResTypeSubnetSet)
-		}
-		subnetSetIDs.Insert(string(subnetSet.UID))
-	}
-	for _, subnet := range nsxSubnetList {
-		if !r.SubnetService.IsOrphanSubnet(*subnet, subnetSetIDs) {
+	subnetSetUIDs := r.SubnetService.SubnetStore.ListIndexFuncValues(servicecommon.TagScopeSubnetSetCRUID)
+	for uid := range subnetSetUIDs {
+		if crdSubnetSetIDsSet.Has(uid) {
 			continue
 		}
-		if err := r.SubnetService.DeleteSubnet(*subnet); err != nil {
+		nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetSetCRUID, string(uid))
+		log.Info("SubnetSet garbage collection, cleaning stale Subnets for SubnetSet", "Count", len(nsxSubnets))
+		if err := r.deleteSubnets(nsxSubnets); err != nil {
+			log.Error(err, "SubnetSet garbage collection, failed to delete NSX subnet", "SubnetSetUID", uid)
 			metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteFailTotal, MetricResTypeSubnetSet)
 		} else {
+			log.Info("SubnetSet garbage collection, successfully deleted NSX subnet", "SubnetSetUID", uid)
 			metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteSuccessTotal, MetricResTypeSubnetSet)
 		}
 	}
+
 }
 
 func (r *SubnetSetReconciler) deleteSubnetBySubnetSetName(ctx context.Context, subnetSetName, ns string) error {

diff --git a/pkg/nsx/services/subnet/subnet.go b/pkg/nsx/services/subnet/subnet.go
@@ -225,16 +225,6 @@ func (service *SubnetService) ListSubnetBySubnetSetName(ns, subnetSetName string
 	return res
 }
 
-// check if subnet belongs to a subnetset, if yes, check if that subnetset still exists
-func (service *SubnetService) IsOrphanSubnet(subnet model.VpcSubnet, subnetsetIDs sets.Set[string]) bool {
-	for _, tag := range subnet.Tags {
-		if *tag.Scope == common.TagScopeSubnetSetCRUID && subnetsetIDs.Has(*tag.Tag) {
-			return false
-		}
-	}
-	return true
-}
-
 func (service *SubnetService) DeleteIPAllocation(orgID, projectID, vpcID, subnetID string) error {
 	ipAllocations, err := service.NSXClient.IPAllocationClient.List(orgID, projectID, vpcID, subnetID, ipPoolID,
 		nil, nil, nil, nil, nil, nil)