From 1652a3348c2fd7f6224bd7fa8401205d9d04041b Mon Sep 17 00:00:00 2001 From: Wenqi Qiu Date: Fri, 20 Sep 2024 18:35:25 +0800 Subject: [PATCH] Add Subnet NamespacedName index Signed-off-by: Wenqi Qiu --- pkg/controllers/common/utils.go | 2 +- pkg/controllers/subnet/subnet_controller.go | 53 ++++++++++++++----- .../subnetset/subnetset_controller.go | 2 +- pkg/nsx/services/common/services.go | 2 +- pkg/nsx/services/common/types.go | 1 + pkg/nsx/services/subnet/store.go | 9 ++++ pkg/nsx/services/subnet/store_test.go | 13 +++-- pkg/nsx/services/subnet/subnet.go | 12 +++-- 8 files changed, 73 insertions(+), 21 deletions(-) diff --git a/pkg/controllers/common/utils.go b/pkg/controllers/common/utils.go index 018e5e30e..ce812491c 100644 --- a/pkg/controllers/common/utils.go +++ b/pkg/controllers/common/utils.go @@ -40,7 +40,7 @@ func AllocateSubnetFromSubnetSet(subnetSet *v1alpha1.SubnetSet, vpcService servi return *nsxSubnet.Path, nil } } - tags := subnetService.GenerateSubnetNSTags(subnetSet, subnetSet.Namespace) + tags := subnetService.GenerateSubnetNSTags(subnetSet, subnetSet.Name, subnetSet.Namespace) if tags == nil { return "", errors.New("failed to generate subnet tags") } diff --git a/pkg/controllers/subnet/subnet_controller.go b/pkg/controllers/subnet/subnet_controller.go index 6bbcec9e6..00e749c48 100644 --- a/pkg/controllers/subnet/subnet_controller.go +++ b/pkg/controllers/subnet/subnet_controller.go @@ -56,12 +56,18 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr subnetCR := &v1alpha1.Subnet{} if err := r.Client.Get(ctx, req.NamespacedName, subnetCR); err != nil { + if err := r.DeleteSubnetByName(req.Name, req.Namespace); err != nil { + return ResultRequeue, err + } log.Error(err, "unable to fetch Subnet CR", "req", req.NamespacedName) return ResultNormal, client.IgnoreNotFound(err) } + if err := r.cleanStaleSubnets(subnetCR.Name, subnetCR.Namespace, string(subnetCR.UID)); err != nil { + return ResultRequeue, err + } if !subnetCR.DeletionTimestamp.IsZero() { metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteTotal, MetricResTypeSubnet) - if err := r.DeleteSubnet(*subnetCR); err != nil { + if err := r.DeleteSubnetByID(*subnetCR); err != nil { log.Error(err, "deletion failed, would retry exponentially", "Subnet", req.NamespacedName) deleteFail(r, ctx, subnetCR, err.Error()) return ResultRequeue, err @@ -104,7 +110,7 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } } - tags := r.SubnetService.GenerateSubnetNSTags(subnetCR, subnetCR.Namespace) + tags := r.SubnetService.GenerateSubnetNSTags(subnetCR, subnetCR.Name, subnetCR.Namespace) if tags == nil { return ResultRequeue, errors.New("failed to generate Subnet tags") } @@ -131,19 +137,41 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, nil } -func (r *SubnetReconciler) DeleteSubnet(obj v1alpha1.Subnet) error { +func (r *SubnetReconciler) DeleteSubnetByID(obj v1alpha1.Subnet) error { nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetCRUID, string(obj.GetUID())) - if len(nsxSubnets) == 0 { - log.Info("no Subnet found for Subnet CR", "uid", string(obj.GetUID())) - return nil + return r.deleteSubnets(nsxSubnets) +} + +func (r *SubnetReconciler) deleteSubnets(nsxSubnets []*model.VpcSubnet) error { + for _, nsxSubnet := range nsxSubnets { + portNums := len(r.SubnetPortService.GetPortsOfSubnet(*nsxSubnet.Id)) + if portNums > 0 { + err := errors.New("subnet still attached by port") + log.Error(err, "delete Subnet from NSX failed", "ID", *nsxSubnet.Id) + return err + } + if err := r.SubnetService.DeleteSubnet(*nsxSubnet); err != nil { + return err + } } - portNums := len(r.SubnetPortService.GetPortsOfSubnet(*nsxSubnets[0].Id)) - if portNums > 0 { - err := errors.New("subnet still attached by port") - log.Error(err, "", "ID", *nsxSubnets[0].Id) - return err + return nil +} + +func (r *SubnetReconciler) DeleteSubnetByName(name, namespace string) error { + nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetCRNamespacedName, subnet.SubnetNamespacedName(name, namespace)) + return r.deleteSubnets(nsxSubnets) +} + +func (r *SubnetReconciler) cleanStaleSubnets(name, namespace, id string) error { + subnetsToDelete := []*model.VpcSubnet{} + nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetCRNamespacedName, subnet.SubnetNamespacedName(name, namespace)) + for i, nsxSubnet := range nsxSubnets { + if *nsxSubnet.Id == id { + continue + } + subnetsToDelete = append(subnetsToDelete, nsxSubnets[i]) } - return r.SubnetService.DeleteSubnet(*nsxSubnets[0]) + return r.deleteSubnets(subnetsToDelete) } func (r *SubnetReconciler) updateSubnetStatus(obj *v1alpha1.Subnet) error { @@ -267,6 +295,7 @@ func deleteSuccess(r *SubnetReconciler, _ context.Context, o *v1alpha1.Subnet) { func (r *SubnetReconciler) setupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.Subnet{}). + // WithEventFilter(). Watches( &v1.Namespace{}, &EnqueueRequestForNamespace{Client: mgr.GetClient()}, diff --git a/pkg/controllers/subnetset/subnetset_controller.go b/pkg/controllers/subnetset/subnetset_controller.go index a188db93a..965404310 100644 --- a/pkg/controllers/subnetset/subnetset_controller.go +++ b/pkg/controllers/subnetset/subnetset_controller.go @@ -94,7 +94,7 @@ func (r *SubnetSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // update SubnetSet tags if labels of namespace changed nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetSetCRUID, string(obj.UID)) if len(nsxSubnets) > 0 { - tags := r.SubnetService.GenerateSubnetNSTags(obj, obj.Namespace) + tags := r.SubnetService.GenerateSubnetNSTags(obj, obj.Name, obj.Namespace) if tags == nil { return ResultRequeue, errors.New("failed to generate subnet tags") } diff --git a/pkg/nsx/services/common/services.go b/pkg/nsx/services/common/services.go index f5e027835..7690e3809 100644 --- a/pkg/nsx/services/common/services.go +++ b/pkg/nsx/services/common/services.go @@ -29,7 +29,7 @@ type SubnetServiceProvider interface { GetSubnetByPath(path string) (*model.VpcSubnet, error) GetSubnetsByIndex(key, value string) []*model.VpcSubnet CreateOrUpdateSubnet(obj client.Object, vpcInfo VPCResourceInfo, tags []model.Tag) (string, error) - GenerateSubnetNSTags(obj client.Object, nsUID string) []model.Tag + GenerateSubnetNSTags(obj client.Object, name, nsUID string) []model.Tag } type SubnetPortServiceProvider interface { diff --git a/pkg/nsx/services/common/types.go b/pkg/nsx/services/common/types.go index f4b77e2a3..0336c57d9 100644 --- a/pkg/nsx/services/common/types.go +++ b/pkg/nsx/services/common/types.go @@ -72,6 +72,7 @@ const ( SystemVPCNetworkConfigurationName string = "system" TagScopeSubnetCRUID string = "nsx-op/subnet_uid" TagScopeSubnetCRName string = "nsx-op/subnet_name" + TagScopeSubnetCRNamespacedName string = "nsx-op/subnet_namespaced_name" TagScopeSubnetSetCRName string = "nsx-op/subnetset_name" TagScopeSubnetSetCRUID string = "nsx-op/subnetset_uid" TagValueGroupScope string = "scope" diff --git a/pkg/nsx/services/subnet/store.go b/pkg/nsx/services/subnet/store.go index b85f7010b..1acd1fda7 100644 --- a/pkg/nsx/services/subnet/store.go +++ b/pkg/nsx/services/subnet/store.go @@ -48,6 +48,15 @@ func subnetSetIndexFunc(obj interface{}) ([]string, error) { } } +func subnetNamespacedNameIndexFunc(obj interface{}) ([]string, error) { + switch o := obj.(type) { + case *model.VpcSubnet: + return filterTag(o.Tags, common.TagScopeSubnetCRNamespacedName), nil + default: + return nil, errors.New("subnetSetIndexFunc doesn't support unknown type") + } +} + // SubnetStore is a store for subnet. type SubnetStore struct { common.ResourceStore diff --git a/pkg/nsx/services/subnet/store_test.go b/pkg/nsx/services/subnet/store_test.go index 4db3c39c1..6e72323cd 100644 --- a/pkg/nsx/services/subnet/store_test.go +++ b/pkg/nsx/services/subnet/store_test.go @@ -43,13 +43,14 @@ func Test_IndexFunc(t *testing.T) { if !reflect.DeepEqual(got, []string{"cr_uid"}) { t.Errorf("subnetCRUIDScopeIndexFunc() = %v, want %v", got, model.Tag{Tag: &tag, Scope: &scope}) } + // }) } func Test_KeyFunc(t *testing.T) { id := "test_id" subnet := model.VpcSubnet{Id: &id} - t.Run("1", func(t *testing.T) { + t.Run("subnetKeyFunc", func(t *testing.T) { got, _ := keyFunc(&subnet) if got != "test_id" { t.Errorf("keyFunc() = %v, want %v", got, "test_id") @@ -62,7 +63,10 @@ func Test_InitializeSubnetStore(t *testing.T) { cluster, _ := nsx.NewCluster(config2) rc, _ := cluster.NewRestConnector() - subnetCacheIndexer := cache.NewIndexer(keyFunc, cache.Indexers{common.TagScopeSubnetCRUID: subnetIndexFunc}) + subnetCacheIndexer := cache.NewIndexer(keyFunc, cache.Indexers{ + common.TagScopeSubnetCRUID: subnetIndexFunc, + common.TagScopeSubnetCRNamespacedName: subnetNamespacedNameIndexFunc, + }) service := SubnetService{ Service: common.Service{ NSXClient: &nsx.Client{ @@ -107,7 +111,10 @@ func Test_InitializeSubnetStore(t *testing.T) { } func TestSubnetStore_Apply(t *testing.T) { - subnetCacheIndexer := cache.NewIndexer(keyFunc, cache.Indexers{common.TagScopeSubnetCRUID: subnetIndexFunc}) + subnetCacheIndexer := cache.NewIndexer(keyFunc, cache.Indexers{ + common.TagScopeSubnetCRUID: subnetIndexFunc, + common.TagScopeSubnetCRNamespacedName: subnetNamespacedNameIndexFunc, + }) resourceStore := common.ResourceStore{ Indexer: subnetCacheIndexer, BindingType: model.SecurityPolicyBindingType(), diff --git a/pkg/nsx/services/subnet/subnet.go b/pkg/nsx/services/subnet/subnet.go index 28d3fd29c..8b6095971 100644 --- a/pkg/nsx/services/subnet/subnet.go +++ b/pkg/nsx/services/subnet/subnet.go @@ -56,8 +56,9 @@ func InitializeSubnetService(service common.Service) (*SubnetService, error) { SubnetStore: &SubnetStore{ ResourceStore: common.ResourceStore{ Indexer: cache.NewIndexer(keyFunc, cache.Indexers{ - common.TagScopeSubnetCRUID: subnetIndexFunc, - common.TagScopeSubnetSetCRUID: subnetSetIndexFunc, + common.TagScopeSubnetCRUID: subnetIndexFunc, + common.TagScopeSubnetSetCRUID: subnetSetIndexFunc, + common.TagScopeSubnetCRNamespacedName: subnetNamespacedNameIndexFunc, }), BindingType: model.VpcSubnetBindingType(), }, @@ -339,7 +340,11 @@ func (service *SubnetService) GetSubnetsByIndex(key, value string) []*model.VpcS return service.SubnetStore.GetByIndex(key, value) } -func (service *SubnetService) GenerateSubnetNSTags(obj client.Object, ns string) []model.Tag { +func SubnetNamespacedName(name, namespace string) string { + return fmt.Sprintf("%s/%s", namespace, name) +} + +func (service *SubnetService) GenerateSubnetNSTags(obj client.Object, name, ns string) []model.Tag { namespace := &v1.Namespace{} namespacedName := types.NamespacedName{ Name: ns, @@ -353,6 +358,7 @@ func (service *SubnetService) GenerateSubnetNSTags(obj client.Object, ns string) case *v1alpha1.Subnet: tags = append(tags, model.Tag{Scope: String(common.TagScopeVMNamespaceUID), Tag: String(nsUID)}, + model.Tag{Scope: String(common.TagScopeSubnetCRNamespacedName), Tag: String(SubnetNamespacedName(name, ns))}, model.Tag{Scope: String(common.TagScopeVMNamespace), Tag: String(obj.GetNamespace())}) case *v1alpha1.SubnetSet: findLabelDefaultPodSubnetSet := false