Merge pull request #277 from seanpang-vmware/fixipblockdel

Fix errors when deleting ip block from ipblock store
vmware-tanzu · Aug 14, 2023 · acf8557 · acf8557
2 parents 70d156a + d3ed048
commit acf8557
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 9 deletions.
diff --git a/pkg/nsx/services/common/types.go b/pkg/nsx/services/common/types.go
@@ -70,6 +70,7 @@ const (
 	VPCFinalizerName               = "vpc.nsx.vmware.com/finalizer"
 
 	IndexKeySubnetID = "IndexKeySubnetID"
+	IndexKeyPathPath = "Path"
 )
 
 var (

diff --git a/pkg/nsx/services/vpc/store.go b/pkg/nsx/services/vpc/store.go
@@ -34,6 +34,18 @@ func indexFunc(obj interface{}) ([]string, error) {
 	}
 }
 
+// for ip block, one vpc may contains multiple ipblock with same vpc cr id
+// add one more indexer using path
+func indexPathFunc(obj interface{}) ([]string, error) {
+	res := make([]string, 0, 5)
+	switch o := obj.(type) {
+	case model.IpAddressBlock:
+		return append(res, *o.Path), nil
+	default:
+		return res, errors.New("indexPathFunc doesn't support unknown type")
+	}
+}
+
 var filterTag = func(v []model.Tag) []string {
 	res := make([]string, 0, 5)
 	for _, tag := range v {
@@ -124,3 +136,14 @@ func (vs *VPCStore) GetByKey(key string) *model.Vpc {
 	}
 	return nil
 }
+
+func (is *IPBlockStore) GetByIndex(index string, value string) *model.IpAddressBlock {
+	indexResults, err := is.ResourceStore.Indexer.ByIndex(index, value)
+	if err != nil || len(indexResults) == 0 {
+		log.Error(err, "failed to get obj by index", "index", value)
+		return nil
+	}
+
+	block := indexResults[0].((model.IpAddressBlock))
+	return &block
+}
diff --git a/pkg/nsx/services/vpc/vpc.go b/pkg/nsx/services/vpc/vpc.go
@@ -107,7 +107,9 @@ func InitializeVPC(service common.Service) (*VPCService, error) {
 	}}
 
 	VPCService.IpblockStore = &IPBlockStore{ResourceStore: common.ResourceStore{
-		Indexer:     cache.NewIndexer(keyFunc, cache.Indexers{common.TagScopeVPCCRUID: indexFunc}),
+		Indexer: cache.NewIndexer(keyFunc, cache.Indexers{
+			common.TagScopeVPCCRUID: indexFunc,
+			common.IndexKeyPathPath: indexPathFunc}),
 		BindingType: model.IpAddressBlockBindingType(),
 	}}
 
@@ -188,14 +190,13 @@ func (service *VPCService) DeleteIPBlock(vpc model.Vpc) error {
 				vpcCRUid = *tag.Tag
 			}
 		}
-		log.Info("search ip block from store using index", "index", common.TagScopeVPCCRUID, "value", vpcCRUid)
-		// TODO: bugfix, seems using vpc cr uid can not get the ip blocks from cache, need further checking
-		ipblocks := service.IpblockStore.GetByIndex(common.TagScopeVPCCRUID, vpcCRUid)
-		if ipblocks != nil && len(ipblocks) != 0 {
-			log.Info("deleting ip blocks", "IPBlock", ipblocks[0])
-			b := ipblocks[0].(model.IpAddressBlock)
-			b.MarkedForDelete = &MarkedForDelete
-			service.IpblockStore.Operate(&ipblocks[0])
+		log.V(2).Info("search ip block from store using index and path", "index", common.TagScopeVPCCRUID, "value", vpcCRUid, "path", block)
+		// using index vpc cr id may get multiple ipblocks, add path to filter the correct one
+		ipblock := service.IpblockStore.GetByIndex(common.IndexKeyPathPath, block)
+		if ipblock != nil {
+			log.Info("deleting ip blocks", "IPBlock", ipblock)
+			ipblock.MarkedForDelete = &MarkedForDelete
+			service.IpblockStore.Operate(ipblock)
 		}
 	}
 	log.Info("successfully deleted all ip blocks")