Skip to content

Commit

Permalink
Filter out the Namespace which is in terminating state
Browse files Browse the repository at this point in the history
  • Loading branch information
@wenyingd
wenyingd committed Dec 13, 2024
1 parent 7f57080 commit a04f5c4
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 167 deletions.
58 changes: 20 additions & 38 deletions pkg/controllers/networkinfo/networkinfo_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,6 @@ func (r *NetworkInfoReconciler) setupWithManager(mgr ctrl.Manager) error {
ipBlocksInfoService: r.IPBlocksInfoService,
},
builder.WithPredicates(VPCNetworkConfigurationPredicate)).
Watches(
&corev1.Namespace{},
&NamespaceHandler{},
builder.WithPredicates(NamespacePredicate)).
Complete(r)
}

Expand Down Expand Up @@ -412,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.DeletionTimestamp.IsZero() {
nsSet.Insert(ns.Name)
idSet.Insert(string(ns.UID))
}
}
return nsSet, idSet, nil
}
Expand Down Expand Up @@ -463,24 +462,6 @@ 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
}

return r.Service.GetVPCsByNamespace(ns), nil
}

func (r *NetworkInfoReconciler) deleteVPCsByName(ctx context.Context, ns string) error {
staleVPCs := r.Service.GetVPCsByNamespace(ns)
if len(staleVPCs) == 0 {
Expand All @@ -505,20 +486,12 @@ func (r *NetworkInfoReconciler) deleteVPCsByName(ctx context.Context, ns string)
return r.deleteVPCs(ctx, vpcToDelete, ns)
}

// deleteVPCsByID is called when the NetworkInfo CR still exists in the K8s api-server, and the CR's
// deletionTimestamp is not 0. In this case, it's Namespace must exist, and no other Namespaces configured
// with the same name. So we can directly use the Namespace's name to find the stale VPCs to delete.
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)
staleVPCs := r.Service.GetVPCsByNamespace(ns)
return r.deleteVPCs(ctx, staleVPCs, ns)
}

func (r *NetworkInfoReconciler) deleteVPCs(ctx context.Context, staleVPCs []*model.Vpc, ns string) error {
Expand All @@ -544,11 +517,20 @@ func (r *NetworkInfoReconciler) deleteVPCs(ctx context.Context, staleVPCs []*mod
// Update the VPCNetworkConfiguration Status
vpcNetConfig := r.Service.GetVPCNetworkConfigByNamespace(ns)
if vpcNetConfig != nil {
deleteVPCNetworkConfigurationStatus(ctx, r.Client, vpcNetConfig.Name, staleVPCs, r.Service.ListVPC())
updateVPCNetworkConfigurationStatusWithAliveVPCs(ctx, r.Client, vpcNetConfig.Name, r.listVPCsByNetworkConfigName)
}
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.
Expand Down
172 changes: 100 additions & 72 deletions pkg/controllers/networkinfo/networkinfo_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
},
Expand Down Expand Up @@ -779,35 +768,41 @@ 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
},
},
{
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")
Expand All @@ -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
},
Expand All @@ -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
},
Expand Down Expand Up @@ -926,28 +929,30 @@ 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",
existingNamespace: &corev1.Namespace{
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")
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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)
}
Loading

0 comments on commit a04f5c4

Please sign in to comment.