From 652107cabaf95e48044f024b004ffb7b6b93b79a Mon Sep 17 00:00:00 2001 From: Wenying Dong Date: Fri, 13 Dec 2024 07:47:04 -0800 Subject: [PATCH] Fix the issue that NSX VPC not deleted when the Namespace is terminating (#948) * Fix issues with NetworkInfo reconciler that not delete NSX VPC in time This change is to fix an issue with the NetworkInfo deletion logic that NSX VPC is not removed if the CR's Namesapce still exists. The fix is to add a wather on Namespace deletion event and ensure the NSX VPC is deleted when the K8s Namespace is deleted. * Filter out the Namespace which is in terminating state --- .../networkinfo/networkinfo_controller.go | 69 +++---- .../networkinfo_controller_test.go | 174 ++++++++++-------- .../networkinfo/networkinfo_utils.go | 38 ++-- pkg/nsx/services/vpc/vpc.go | 2 +- test/e2e/nsx_networkinfo_test.go | 2 +- 5 files changed, 144 insertions(+), 141 deletions(-) diff --git a/pkg/controllers/networkinfo/networkinfo_controller.go b/pkg/controllers/networkinfo/networkinfo_controller.go index 23a3da1ce..e616ba9a0 100644 --- a/pkg/controllers/networkinfo/networkinfo_controller.go +++ b/pkg/controllers/networkinfo/networkinfo_controller.go @@ -136,7 +136,7 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request) networkInfoCR := &v1alpha1.NetworkInfo{} if err := r.Client.Get(ctx, req.NamespacedName, networkInfoCR); err != nil { if apierrors.IsNotFound(err) { - if err := r.deleteVPCsByName(ctx, req.Namespace); err != nil { + if err := r.deleteVPCsByNamespace(ctx, req.Namespace); err != nil { r.StatusUpdater.DeleteFail(req.NamespacedName, nil, err) return common.ResultRequeue, err } @@ -150,7 +150,7 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request) // Check if the CR is marked for deletion if !networkInfoCR.ObjectMeta.DeletionTimestamp.IsZero() { r.StatusUpdater.IncreaseDeleteTotal() - if err := r.deleteVPCsByID(ctx, networkInfoCR.GetNamespace(), string(networkInfoCR.UID)); err != nil { + if err := r.deleteVPCsByNamespace(ctx, networkInfoCR.GetNamespace()); err != nil { r.StatusUpdater.DeleteFail(req.NamespacedName, nil, err) return common.ResultRequeue, err } @@ -408,8 +408,11 @@ func (r *NetworkInfoReconciler) listNamespaceCRsNameIDSet(ctx context.Context) ( nsSet := sets.Set[string]{} idSet := sets.Set[string]{} for _, ns := range namespaces.Items { - nsSet.Insert(ns.Name) - idSet.Insert(string(ns.UID)) + // Ignore the terminating Namespaces in the list results. + if ns.ObjectMeta.DeletionTimestamp.IsZero() { + nsSet.Insert(ns.Name) + idSet.Insert(string(ns.UID)) + } } return nsSet, idSet, nil } @@ -459,36 +462,18 @@ func (r *NetworkInfoReconciler) CollectGarbage(ctx context.Context) { } } -func (r *NetworkInfoReconciler) fetchStaleVPCsByNamespace(ctx context.Context, ns string) ([]*model.Vpc, error) { - isShared, err := r.Service.IsSharedVPCNamespaceByNS(ctx, ns) - if err != nil { - if apierrors.IsNotFound(err) { - // if the Namespace has been deleted, we never know whether it`s a shared Namespace, The GC will delete the stale VPCs - log.Info("Namespace does not exist while fetching stale VPCs", "Namespace", ns) - return nil, nil - } - return nil, fmt.Errorf("failed to check if Namespace is shared for NS %s: %w", ns, err) - } - if isShared { - log.Info("Shared Namespace, skipping deletion of NSX VPC", "Namespace", ns) - return nil, nil +func (r *NetworkInfoReconciler) deleteVPCsByNamespace(ctx context.Context, ns string) error { + staleVPCs := r.Service.GetVPCsByNamespace(ns) + if len(staleVPCs) == 0 { + return nil } - return r.Service.GetVPCsByNamespace(ns), nil -} - -func (r *NetworkInfoReconciler) deleteVPCsByName(ctx context.Context, ns string) error { _, idSet, err := r.listNamespaceCRsNameIDSet(ctx) if err != nil { log.Error(err, "Failed to list Kubernetes Namespaces") return fmt.Errorf("failed to list Kubernetes Namespaces while deleting VPCs: %v", err) } - staleVPCs, err := r.fetchStaleVPCsByNamespace(ctx, ns) - if err != nil { - return err - } - var vpcToDelete []*model.Vpc for _, nsxVPC := range staleVPCs { namespaceIDofVPC := filterTagFromNSXVPC(nsxVPC, commonservice.TagScopeNamespaceUID) @@ -501,22 +486,6 @@ func (r *NetworkInfoReconciler) deleteVPCsByName(ctx context.Context, ns string) return r.deleteVPCs(ctx, vpcToDelete, ns) } -func (r *NetworkInfoReconciler) deleteVPCsByID(ctx context.Context, ns, id string) error { - staleVPCs, err := r.fetchStaleVPCsByNamespace(ctx, ns) - if err != nil { - return err - } - - var vpcToDelete []*model.Vpc - for _, nsxVPC := range staleVPCs { - namespaceIDofVPC := filterTagFromNSXVPC(nsxVPC, commonservice.TagScopeNamespaceUID) - if namespaceIDofVPC == id { - vpcToDelete = append(vpcToDelete, nsxVPC) - } - } - return r.deleteVPCs(ctx, vpcToDelete, ns) -} - func (r *NetworkInfoReconciler) deleteVPCs(ctx context.Context, staleVPCs []*model.Vpc, ns string) error { if len(staleVPCs) == 0 { log.Info("There is no VPCs found in store, skipping deletion of NSX VPC", "Namespace", ns) @@ -538,14 +507,22 @@ func (r *NetworkInfoReconciler) deleteVPCs(ctx context.Context, staleVPCs []*mod } // Update the VPCNetworkConfiguration Status - ncName, err := r.Service.GetNetworkconfigNameFromNS(ctx, ns) - if err != nil { - return fmt.Errorf("failed to get VPCNetworkConfiguration for Namespace when deleting stale VPCs %s: %w", ns, err) + vpcNetConfig := r.Service.GetVPCNetworkConfigByNamespace(ns) + if vpcNetConfig != nil { + updateVPCNetworkConfigurationStatusWithAliveVPCs(ctx, r.Client, vpcNetConfig.Name, r.listVPCsByNetworkConfigName) } - deleteVPCNetworkConfigurationStatus(ctx, r.Client, ncName, staleVPCs, r.Service.ListVPC()) return nil } +func (r *NetworkInfoReconciler) listVPCsByNetworkConfigName(ncName string) []*model.Vpc { + namespacesUsingNC := r.Service.GetNamespacesByNetworkconfigName(ncName) + aliveVPCs := make([]*model.Vpc, 0) + for _, namespace := range namespacesUsingNC { + aliveVPCs = append(aliveVPCs, r.Service.GetVPCsByNamespace(namespace)...) + } + return aliveVPCs +} + func (r *NetworkInfoReconciler) syncPreCreatedVpcIPs(ctx context.Context) { // Construct a map for the existing NetworkInfo CRs, the key is its Namespace, and the value is // the NetworkInfo CR. diff --git a/pkg/controllers/networkinfo/networkinfo_controller_test.go b/pkg/controllers/networkinfo/networkinfo_controller_test.go index 816b9583c..44ee95ea7 100644 --- a/pkg/controllers/networkinfo/networkinfo_controller_test.go +++ b/pkg/controllers/networkinfo/networkinfo_controller_test.go @@ -21,6 +21,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/sets" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/util/workqueue" controllerruntime "sigs.k8s.io/controller-runtime" @@ -154,20 +155,8 @@ func TestNetworkInfoReconciler_Reconcile(t *testing.T) { { name: "Empty", prepareFunc: func(t *testing.T, r *NetworkInfoReconciler, ctx context.Context) (patches *gomonkey.Patches) { - patches = gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "IsSharedVPCNamespaceByNS", func(_ *vpc.VPCService, ctx context.Context, _ string) (bool, error) { - return false, nil - }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, _ string) []*model.Vpc { - return nil - }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNetworkconfigNameFromNS", func(_ *vpc.VPCService, ctx context.Context, _ string) (string, error) { - return "", nil - }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "ListVPC", func(_ *vpc.VPCService) []model.Vpc { - return nil - }) - patches.ApplyFunc(deleteVPCNetworkConfigurationStatus, func(ctx context.Context, client client.Client, ncName string, staleVPCs []*model.Vpc, aliveVPCs []model.Vpc) { - return + patches = gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, _ string) []*model.Vpc { + return []*model.Vpc{} }) return patches }, @@ -779,22 +768,23 @@ func TestNetworkInfoReconciler_deleteStaleVPCs(t *testing.T) { expectErrStr string }{ { - name: "shared namespace, skip deletion", + name: "no VPC exists for the Namespace,, skip deletion", prepareFuncs: func(r *NetworkInfoReconciler) *gomonkey.Patches { - patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "IsSharedVPCNamespaceByNS", func(_ *vpc.VPCService, ctx context.Context, _ string) (bool, error) { - return true, nil + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, _ string) []*model.Vpc { + return []*model.Vpc{} }) return patches }, - }, - { - name: "non-shared namespace, no VPCs found", + }, { + name: "NSX VPC is used by a valid Namespace,, skip deletion", prepareFuncs: func(r *NetworkInfoReconciler) *gomonkey.Patches { - patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "IsSharedVPCNamespaceByNS", func(_ *vpc.VPCService, ctx context.Context, _ string) (bool, error) { - return false, nil + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, _ string) []*model.Vpc { + return []*model.Vpc{ + {Tags: []model.Tag{ + {Scope: servicecommon.String(servicecommon.TagScopeNamespaceUID), Tag: servicecommon.String("vpc1")}}}} }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, _ string) []*model.Vpc { - return nil + patches.ApplyPrivateMethod(reflect.TypeOf(r), "listNamespaceCRsNameIDSet", func(_ *NetworkInfoReconciler, _ context.Context) (sets.Set[string], sets.Set[string], error) { + return sets.Set[string]{}, sets.New[string]("vpc1"), nil }) return patches }, @@ -802,12 +792,17 @@ func TestNetworkInfoReconciler_deleteStaleVPCs(t *testing.T) { { name: "failed to delete VPC", prepareFuncs: func(r *NetworkInfoReconciler) *gomonkey.Patches { - patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "IsSharedVPCNamespaceByNS", func(_ *vpc.VPCService, ctx context.Context, _ string) (bool, error) { - return false, nil + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, _ string) []*model.Vpc { + return []*model.Vpc{ + { + Tags: []model.Tag{ + {Scope: servicecommon.String(servicecommon.TagScopeNamespaceUID), Tag: servicecommon.String("vpc1")}, + }, + Path: servicecommon.String("/orgs/default/projects/default/vpcs/vpc1"), + }} }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, _ string) []*model.Vpc { - vpcPath := "/vpc/1" - return []*model.Vpc{{Path: &vpcPath}} + patches.ApplyPrivateMethod(reflect.TypeOf(r), "listNamespaceCRsNameIDSet", func(_ *NetworkInfoReconciler, _ context.Context) (sets.Set[string], sets.Set[string], error) { + return sets.Set[string]{}, sets.Set[string]{}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "DeleteVPC", func(_ *vpc.VPCService, _ string) error { return fmt.Errorf("delete failed") @@ -819,26 +814,23 @@ func TestNetworkInfoReconciler_deleteStaleVPCs(t *testing.T) { { name: "successful deletion of VPCs", prepareFuncs: func(r *NetworkInfoReconciler) *gomonkey.Patches { - patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "IsSharedVPCNamespaceByNS", func(_ *vpc.VPCService, ctx context.Context, _ string) (bool, error) { - return false, nil + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, _ string) []*model.Vpc { + return []*model.Vpc{ + { + Tags: []model.Tag{ + {Scope: servicecommon.String(servicecommon.TagScopeNamespaceUID), Tag: servicecommon.String("vpc1")}, + }, + Path: servicecommon.String("/orgs/default/projects/default/vpcs/vpc1"), + }} }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, _ string) []*model.Vpc { - vpcPath1 := "/vpc/1" - vpcPath2 := "/vpc/2" - displayName1 := "fakeDisplayName" - displayName2 := "fakeDisplayName" - return []*model.Vpc{{Path: &vpcPath1, DisplayName: &displayName1}, {Path: &vpcPath2, DisplayName: &displayName2}} + patches.ApplyPrivateMethod(reflect.TypeOf(r), "listNamespaceCRsNameIDSet", func(_ *NetworkInfoReconciler, _ context.Context) (sets.Set[string], sets.Set[string], error) { + return sets.Set[string]{}, sets.Set[string]{}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "DeleteVPC", func(_ *vpc.VPCService, _ string) error { return nil }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "ListVPC", func(_ *vpc.VPCService) []model.Vpc { - vpcPath := "/vpc/1" - displayName := "fakeDisplayName" - return []model.Vpc{{Path: &vpcPath, DisplayName: &displayName}} - }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNetworkconfigNameFromNS", func(_ *vpc.VPCService, ctx context.Context, _ string) (string, error) { - return "", nil + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCNetworkConfigByNamespace", func(_ *vpc.VPCService, _ string) *servicecommon.VPCNetworkConfigInfo { + return nil }) return patches }, @@ -860,26 +852,37 @@ func TestNetworkInfoReconciler_deleteStaleVPCs(t *testing.T) { }, }, prepareFuncs: func(r *NetworkInfoReconciler) *gomonkey.Patches { - patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "IsSharedVPCNamespaceByNS", func(_ *vpc.VPCService, ctx context.Context, _ string) (bool, error) { - return false, nil + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, _ string) []*model.Vpc { + return []*model.Vpc{ + { + Tags: []model.Tag{ + {Scope: servicecommon.String(servicecommon.TagScopeNamespaceUID), Tag: servicecommon.String("vpc1")}, + }, + Path: servicecommon.String("/orgs/default/projects/default/vpcs/vpc1"), + DisplayName: servicecommon.String("fakeDisplayName1"), + }} }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, _ string) []*model.Vpc { - vpcPath1 := "/vpc/1" - vpcPath2 := "/vpc/2" - displayName1 := "fakeDisplayName1" - displayName2 := "fakeDisplayName2" - return []*model.Vpc{{Path: &vpcPath1, DisplayName: &displayName1}, {Path: &vpcPath2, DisplayName: &displayName2}} + patches.ApplyPrivateMethod(reflect.TypeOf(r), "listNamespaceCRsNameIDSet", func(_ *NetworkInfoReconciler, _ context.Context) (sets.Set[string], sets.Set[string], error) { + return sets.Set[string]{}, sets.Set[string]{}, nil }) patches.ApplyMethod(reflect.TypeOf(r.Service), "DeleteVPC", func(_ *vpc.VPCService, _ string) error { return nil }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "ListVPC", func(_ *vpc.VPCService) []model.Vpc { - vpcPath := "/vpc/1" - displayName := "fakeDisplayName1" - return []model.Vpc{{Path: &vpcPath, DisplayName: &displayName}} + patches.ApplyPrivateMethod(reflect.TypeOf(r), "listVPCsByNetworkConfigName", func(_ *NetworkInfoReconciler, _ string) []*model.Vpc { + return []*model.Vpc{ + { + Tags: []model.Tag{ + {Scope: servicecommon.String(servicecommon.TagScopeNamespaceUID), Tag: servicecommon.String("vpc1")}, + }, + Path: servicecommon.String("/orgs/default/projects/default/vpcs/vpc2"), + DisplayName: servicecommon.String("fakeDisplayName2"), + }, + } }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "GetNetworkconfigNameFromNS", func(_ *vpc.VPCService, ctx context.Context, _ string) (string, error) { - return "fakeNetworkconfigName", nil + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCNetworkConfigByNamespace", func(_ *vpc.VPCService, _ string) *servicecommon.VPCNetworkConfigInfo { + return &servicecommon.VPCNetworkConfigInfo{ + Name: "fakeNetworkconfigName", + } }) return patches }, @@ -902,7 +905,7 @@ func TestNetworkInfoReconciler_deleteStaleVPCs(t *testing.T) { defer patches.Reset() } - err := r.deleteVPCsByName(ctx, namespace) + err := r.deleteVPCsByNamespace(ctx, namespace) if tc.expectErrStr != "" { assert.ErrorContains(t, err, tc.expectErrStr) @@ -926,9 +929,15 @@ func TestNetworkInfoReconciler_DeleteNetworkInfo(t *testing.T) { { name: "Delete NetworkInfo and Namespace not existed", existingNamespace: nil, - expectErrStr: "", - expectRes: common.ResultNormal, - req: reconcile.Request{NamespacedName: types.NamespacedName{Namespace: "testNamespace", Name: "testNetworkInfo"}}, + prepareFuncs: func(r *NetworkInfoReconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, _ string) []*model.Vpc { + return []*model.Vpc{} + }) + return patches + }, + expectErrStr: "", + expectRes: common.ResultNormal, + req: reconcile.Request{NamespacedName: types.NamespacedName{Namespace: "testNamespace", Name: "testNetworkInfo"}}, }, { name: "Delete NetworkInfo with delete stale VPC error", @@ -936,18 +945,14 @@ func TestNetworkInfoReconciler_DeleteNetworkInfo(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "testNamespace", UID: "fakeNamespaceUID"}, }, prepareFuncs: func(r *NetworkInfoReconciler) *gomonkey.Patches { - patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service.VpcStore), "GetByIndex", func(_ *vpc.VPCStore, key string, value string) []*model.Vpc { - vpcName := "fakeVPCName" - vpcPath := "fakeVPCPath" + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, _ string) []*model.Vpc { return []*model.Vpc{ { - DisplayName: &vpcName, - Path: &vpcPath, - }, - } - }) - patches.ApplyMethod(reflect.TypeOf(r.Service), "IsSharedVPCNamespaceByNS", func(_ *vpc.VPCService, ctx context.Context, _ string) (bool, error) { - return false, nil + Tags: []model.Tag{ + {Scope: servicecommon.String(servicecommon.TagScopeNamespaceUID), Tag: servicecommon.String("vpc1")}, + }, + Path: servicecommon.String("/orgs/default/projects/default/vpcs/vpc1"), + }} }) patches.ApplyMethod(reflect.TypeOf(r.Service), "DeleteVPC", func(_ *vpc.VPCService, _ string) error { return fmt.Errorf("delete failed") @@ -987,7 +992,7 @@ func TestNetworkInfoReconciler_DeleteNetworkInfo(t *testing.T) { Status: corev1.NamespaceStatus{}, }, prepareFuncs: func(r *NetworkInfoReconciler) *gomonkey.Patches { - return gomonkey.ApplyMethod(reflect.TypeOf(r.Service.VpcStore), "GetByIndex", func(_ *vpc.VPCStore, key string, value string) []*model.Vpc { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service.VpcStore), "GetByIndex", func(_ *vpc.VPCStore, key string, value string) []*model.Vpc { id := "fakeNamespaceUID" scope := servicecommon.TagScopeNamespaceUID tag1 := []model.Tag{ @@ -1004,6 +1009,10 @@ func TestNetworkInfoReconciler_DeleteNetworkInfo(t *testing.T) { }, } }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "deleteVPCs", func(_ *NetworkInfoReconciler, _ context.Context, _ []*model.Vpc, ns string) error { + return fmt.Errorf("failed to get VPCNetworkConfiguration for Namespace when deleting stale VPCs") + }) + return patches }, expectErrStr: "failed to get VPCNetworkConfiguration for Namespace when deleting stale VPCs", existingNetworkInfo: &v1alpha1.NetworkInfo{ @@ -1410,3 +1419,22 @@ func TestRegisterAllNetworkInfo(t *testing.T) { err = r.RegisterAllNetworkInfo(context.Background()) assert.NoError(t, err) } + +func TestListVPCsByNetworkConfigName(t *testing.T) { + r := &NetworkInfoReconciler{ + Service: &vpc.VPCService{}, + } + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Service), "GetNamespacesByNetworkconfigName", func(_ *vpc.VPCService, _c string) []string { + return []string{"ns1", "ns2"} + }) + patches.ApplyMethod(reflect.TypeOf(r.Service), "GetVPCsByNamespace", func(_ *vpc.VPCService, ns string) []*model.Vpc { + if ns == "ns1" { + return []*model.Vpc{{Id: servicecommon.String("vpc1")}} + } + return []*model.Vpc{{Id: servicecommon.String("vpc2")}} + }) + defer patches.Reset() + expVPCs := []*model.Vpc{{Id: servicecommon.String("vpc1")}, {Id: servicecommon.String("vpc2")}} + actVPCs := r.listVPCsByNetworkConfigName("nc1") + assert.ElementsMatch(t, expVPCs, actVPCs) +} diff --git a/pkg/controllers/networkinfo/networkinfo_utils.go b/pkg/controllers/networkinfo/networkinfo_utils.go index cd62ca621..42d6bf00e 100644 --- a/pkg/controllers/networkinfo/networkinfo_utils.go +++ b/pkg/controllers/networkinfo/networkinfo_utils.go @@ -191,15 +191,7 @@ func getGatewayConnectionStatus(ctx context.Context, nc *v1alpha1.VPCNetworkConf return gatewayConnectionReady, reason } -func deleteVPCNetworkConfigurationStatus(ctx context.Context, client client.Client, ncName string, staleVPCs []*model.Vpc, aliveVPCs []model.Vpc) { - aliveVPCNames := sets.New[string]() - for _, vpcModel := range aliveVPCs { - aliveVPCNames.Insert(*vpcModel.DisplayName) - } - staleVPCNames := sets.New[string]() - for _, vpc := range staleVPCs { - staleVPCNames.Insert(*vpc.DisplayName) - } +func updateVPCNetworkConfigurationStatusWithAliveVPCs(ctx context.Context, client client.Client, ncName string, getAliveVPCsFn func(ncName string) []*model.Vpc) { // read v1alpha1.VPCNetworkConfiguration by ncName nc := &v1alpha1.VPCNetworkConfiguration{} err := client.Get(ctx, apitypes.NamespacedName{Name: ncName}, nc) @@ -207,19 +199,25 @@ func deleteVPCNetworkConfigurationStatus(ctx context.Context, client client.Clie log.Error(err, "Failed to get VPCNetworkConfiguration", "Name", ncName) return } - // iterate through VPCNetworkConfiguration.Status.VPCs, if vpcName does not exist in the staleVPCNames, append in new VPCs status - var newVPCInfos []v1alpha1.VPCInfo - for _, vpc := range nc.Status.VPCs { - if !staleVPCNames.Has(vpc.Name) && aliveVPCNames.Has(vpc.Name) { - newVPCInfos = append(newVPCInfos, vpc) + + if getAliveVPCsFn != nil { + aliveVPCs := sets.New[string]() + for _, vpc := range getAliveVPCsFn(ncName) { + aliveVPCs.Insert(*vpc.DisplayName) } + var newVPCInfos []v1alpha1.VPCInfo + for _, vpcInfo := range nc.Status.VPCs { + if aliveVPCs.Has(vpcInfo.Name) { + newVPCInfos = append(newVPCInfos, vpcInfo) + } + } + nc.Status.VPCs = newVPCInfos + if err := client.Status().Update(ctx, nc); err != nil { + log.Error(err, "Failed to update VPCNetworkConfiguration status", "Name", ncName, "nc.Status.VPCs", nc.Status.VPCs) + return + } + log.Info("Updated VPCNetworkConfiguration status", "Name", ncName, "nc.Status.VPCs", nc.Status.VPCs) } - nc.Status.VPCs = newVPCInfos - if err := client.Status().Update(ctx, nc); err != nil { - log.Error(err, "Failed to delete stale VPCNetworkConfiguration status", "Name", ncName, "nc.Status.VPCs", nc.Status.VPCs, "staleVPCs", staleVPCNames) - return - } - log.Info("Deleted stale VPCNetworkConfiguration status", "Name", ncName, "nc.Status.VPCs", nc.Status.VPCs, "staleVPCs", staleVPCNames) } func filterTagFromNSXVPC(nsxVPC *model.Vpc, tagName string) string { diff --git a/pkg/nsx/services/vpc/vpc.go b/pkg/nsx/services/vpc/vpc.go index 0bb75a26c..76874ae4c 100644 --- a/pkg/nsx/services/vpc/vpc.go +++ b/pkg/nsx/services/vpc/vpc.go @@ -559,7 +559,7 @@ func (s *VPCService) resolveSharedVPCNamespace(ctx context.Context, ns string) ( } annos := obj.Annotations - // If no annotaion on ns, then this is not a shared VPC ns + // If no annotation on ns, then this is not a shared VPC ns if len(annos) == 0 { return obj, nil, nil } diff --git a/test/e2e/nsx_networkinfo_test.go b/test/e2e/nsx_networkinfo_test.go index 9ac786d50..cdb225180 100644 --- a/test/e2e/nsx_networkinfo_test.go +++ b/test/e2e/nsx_networkinfo_test.go @@ -289,7 +289,7 @@ func getVPCPathFromVPCNetworkConfiguration(t *testing.T, ncName string) (vpcPath resp, err := testData.crdClientset.CrdV1alpha1().VPCNetworkConfigurations().Get(ctx, ncName, v1.GetOptions{}) if err != nil { log.V(2).Info("Check VPC path of vpcnetworkconfigurations", "resp", resp) - return false, fmt.Errorf("error when waiting for vpcnetworkconfigurations VPC path: %s", ncName) + return false, fmt.Errorf("error when waiting for vpcnetworkconfigurations VPC path: %s: %v", ncName, err) } if len(resp.Status.VPCs) > 0 && resp.Status.VPCs[0].VPCPath != "" { vpcPath = resp.Status.VPCs[0].VPCPath