diff --git a/pkg/controllers/subnet/subnet_controller.go b/pkg/controllers/subnet/subnet_controller.go index b3fd62978..8553b8f6c 100644 --- a/pkg/controllers/subnet/subnet_controller.go +++ b/pkg/controllers/subnet/subnet_controller.go @@ -176,6 +176,7 @@ func (r *SubnetReconciler) deleteSubnets(nsxSubnets []*model.VpcSubnet) error { } log.Info("Successfully deleted Subnet", "ID", *nsxSubnet.Id) } + log.Info("Successfully cleaned stale Subnets") return nil } @@ -184,18 +185,6 @@ func (r *SubnetReconciler) deleteSubnetByName(name, ns string) error { return r.deleteSubnets(nsxSubnets) } -func (r *SubnetReconciler) cleanStaleSubnets(name, ns, id string) error { - subnetsToDelete := []*model.VpcSubnet{} - nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetCRNamespacedName, util.CombineNamespaceName(name, ns)) - for i, nsxSubnet := range nsxSubnets { - if *nsxSubnet.Id == id { - continue - } - subnetsToDelete = append(subnetsToDelete, nsxSubnets[i]) - } - return r.deleteSubnets(subnetsToDelete) -} - func (r *SubnetReconciler) updateSubnetStatus(obj *v1alpha1.Subnet) error { nsxSubnet := r.SubnetService.SubnetStore.GetByKey(r.SubnetService.BuildSubnetID(obj)) if nsxSubnet == nil { diff --git a/pkg/controllers/subnet/subnet_controller_test.go b/pkg/controllers/subnet/subnet_controller_test.go index 88b902a13..aadfbc0e5 100644 --- a/pkg/controllers/subnet/subnet_controller_test.go +++ b/pkg/controllers/subnet/subnet_controller_test.go @@ -232,9 +232,6 @@ func TestSubnetReconciler_Reconcile(t *testing.T) { return nil }) defer patches.Reset() - patches.ApplyPrivateMethod(reflect.TypeOf(reconciler), "cleanStaleSubnets", func(_ *SubnetReconciler, name, ns, id string) error { - return nil - }) patches.ApplyMethod(reflect.TypeOf(reconciler.Client), "Delete", func(_ client.Client, _ context.Context, _ client.Object, _ ...client.DeleteOption) error { return nil }) @@ -254,15 +251,11 @@ func TestSubnetReconciler_Reconcile(t *testing.T) { assert.NoError(t, createErr) defer reconciler.Client.Delete(ctx, subnetCR) - patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(reconciler), "cleanStaleSubnets", func(_ *SubnetReconciler, name, ns, id string) error { - return nil - }) - defer patches.Reset() - vpcConfig := &common.VPCNetworkConfigInfo{DefaultSubnetSize: 16} - patches.ApplyPrivateMethod(reflect.TypeOf(reconciler.VPCService), "GetVPCNetworkConfigByNamespace", func(_ *vpc.VPCService, ns string) *common.VPCNetworkConfigInfo { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(reconciler.VPCService), "GetVPCNetworkConfigByNamespace", func(_ *vpc.VPCService, ns string) *common.VPCNetworkConfigInfo { return vpcConfig }) + defer patches.Reset() tags := []model.Tag{{Scope: common.String(common.TagScopeSubnetCRUID), Tag: common.String("fake-tag")}} patches.ApplyMethod(reflect.TypeOf(reconciler.SubnetService), "GenerateSubnetNSTags", func(_ *subnet.SubnetService, obj client.Object) []model.Tag { @@ -294,15 +287,11 @@ func TestSubnetReconciler_Reconcile(t *testing.T) { assert.NoError(t, createErr) defer reconciler.Client.Delete(ctx, subnetCR) - patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(reconciler), "cleanStaleSubnets", func(_ *SubnetReconciler, name, ns, id string) error { - return nil - }) - defer patches.Reset() - vpcConfig := &common.VPCNetworkConfigInfo{DefaultSubnetSize: 16} - patches.ApplyPrivateMethod(reflect.TypeOf(reconciler.VPCService), "GetVPCNetworkConfigByNamespace", func(_ *vpc.VPCService, ns string) *common.VPCNetworkConfigInfo { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(reconciler.VPCService), "GetVPCNetworkConfigByNamespace", func(_ *vpc.VPCService, ns string) *common.VPCNetworkConfigInfo { return vpcConfig }) + defer patches.Reset() tags := []model.Tag{{Scope: common.String(common.TagScopeSubnetCRUID), Tag: common.String("fake-tag")}} patches.ApplyMethod(reflect.TypeOf(reconciler.SubnetService), "GenerateSubnetNSTags", func(_ *subnet.SubnetService, obj client.Object) []model.Tag { @@ -331,62 +320,3 @@ func TestSubnetReconciler_Reconcile(t *testing.T) { assert.Equal(t, ctrl.Result{}, result) }) } - -func TestCleanStaleSubnets(t *testing.T) { - reconciler := createFakeSubnetReconciler() - - name := "test-subnet" - ns := "test-ns" - id := "subnet-1" - - nsxSubnets := []*model.VpcSubnet{ - {Id: &id}, - } - - t.Run("should not delete any subnets if all match the ID", func(t *testing.T) { - patches := gomonkey.ApplyMethod(reflect.TypeOf(reconciler.SubnetService.SubnetStore), "GetByIndex", func(_ *subnet.SubnetStore, key string, value string) []*model.VpcSubnet { - return nsxSubnets - }) - defer patches.Reset() - - err := reconciler.cleanStaleSubnets(name, ns, id) - assert.NoError(t, err) - }) - - t.Run("should delete stale subnets that do not match the ID", func(t *testing.T) { - id1 := "fake-VpcSubnet-id" - nsxSubnets = append(nsxSubnets, &model.VpcSubnet{Id: &id1}) - patches := gomonkey.ApplyMethod(reflect.TypeOf(reconciler.SubnetService.SubnetStore), "GetByIndex", func(_ *subnet.SubnetStore, key string, value string) []*model.VpcSubnet { - return nsxSubnets - }) - patches.ApplyMethod(reflect.TypeOf(reconciler.SubnetPortService), "GetPortsOfSubnet", func(_ *subnetport.SubnetPortService, nsxSubnetID string) (ports []*model.VpcSubnetPort) { - return nil - }) - patches.ApplyMethod(reflect.TypeOf(reconciler.SubnetService), "DeleteSubnet", func(_ *subnet.SubnetService, nsxSubnet model.VpcSubnet) error { - return nil - }) - defer patches.Reset() - - err := reconciler.cleanStaleSubnets(name, ns, id) - assert.NoError(t, err) - }) - - t.Run("should return error if deletion fails", func(t *testing.T) { - id1 := "fake-VpcSubnet-id" - nsxSubnets = append(nsxSubnets, &model.VpcSubnet{Id: &id1}) - patches := gomonkey.ApplyMethod(reflect.TypeOf(reconciler.SubnetService.SubnetStore), "GetByIndex", func(_ *subnet.SubnetStore, key string, value string) []*model.VpcSubnet { - return nsxSubnets - }) - patches.ApplyMethod(reflect.TypeOf(reconciler.SubnetPortService), "GetPortsOfSubnet", func(_ *subnetport.SubnetPortService, nsxSubnetID string) (ports []*model.VpcSubnetPort) { - return nil - }) - patches.ApplyMethod(reflect.TypeOf(reconciler.SubnetService), "DeleteSubnet", func(_ *subnet.SubnetService, nsxSubnet model.VpcSubnet) error { - return errors.New("deletion failed") - }) - defer patches.Reset() - - err := reconciler.cleanStaleSubnets(name, ns, id) - assert.Error(t, err) - assert.Equal(t, "deletion failed", err.Error()) - }) -}