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