fix(natservice): delete resource from status refs

outscale · Aug 21, 2024 · b9910f2 · b9910f2
1 parent 0403afb
commit b9910f2
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 91 deletions.
diff --git a/controllers/osccluster_natservice_controller.go b/controllers/osccluster_natservice_controller.go
@@ -25,7 +25,6 @@ import (
 	"github.com/outscale-dev/cluster-api-provider-outscale.git/cloud/services/net"
 	tag "github.com/outscale-dev/cluster-api-provider-outscale.git/cloud/tag"
 	osc "github.com/outscale/osc-sdk-go/v2"
-	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
@@ -184,35 +183,24 @@ func reconcileNatService(ctx context.Context, clusterScope *scope.ClusterScope,
 
 // reconcileDeleteNatService reconcile the destruction of the NatService of the cluster.
 func reconcileDeleteNatService(ctx context.Context, clusterScope *scope.ClusterScope, natServiceSvc net.OscNatServiceInterface) (reconcile.Result, error) {
-	osccluster := clusterScope.OscCluster
-	var natServicesSpec []*infrastructurev1beta1.OscNatService
-	networkSpec := clusterScope.GetNetwork()
-	if networkSpec.NatServices == nil {
-		// Add backwards compatibility with NatService parameter that used single NatService
-		natServiceSpec := clusterScope.GetNatService()
-		natServiceSpec.SetDefaultValue()
-		natServicesSpec = append(natServicesSpec, natServiceSpec)
-	} else {
-		natServicesSpec = clusterScope.GetNatServices()
-	}
+	natServiceRef := clusterScope.GetNatServiceRef()
 
-	for _, natServiceSpec := range natServicesSpec {
-		natServiceName := natServiceSpec.Name + "-" + clusterScope.GetUID()
-		natServiceId := natServiceSpec.ResourceId
+	for natServiceName, natServiceId := range natServiceRef.ResourceMap {
 		natService, err := natServiceSvc.GetNatService(natServiceId)
 		if err != nil {
 			return reconcile.Result{}, err
 		}
 		if natService == nil {
 			clusterScope.V(2).Info("The desired natService does not exist anymore", "natServiceName", natServiceName)
-			controllerutil.RemoveFinalizer(osccluster, "oscclusters.infrastructure.cluster.x-k8s.io")
+			delete(natServiceRef.ResourceMap, natServiceName)
 			return reconcile.Result{}, nil
 		}
 		clusterScope.V(2).Info("Delete the desired natService", "natServiceName", natServiceName)
 		err = natServiceSvc.DeleteNatService(natServiceId)
 		if err != nil {
 			return reconcile.Result{}, fmt.Errorf("%w Can not delete natService for Osccluster %s/%s", err, clusterScope.GetNamespace(), clusterScope.GetName())
 		}
+		delete(natServiceRef.ResourceMap, natServiceName)
 	}
 	return reconcile.Result{}, nil
 }
diff --git a/controllers/osccluster_natservice_controller_unit_test.go b/controllers/osccluster_natservice_controller_unit_test.go
@@ -784,78 +784,3 @@ func TestReconcileDeleteNatServiceDelete(t *testing.T) {
 		})
 	}
 }
-
-// TestReconcileDeleteNatServiceGet has several tests to cover the code of the function reconcileDeleteNatService
-func TestReconcileDeleteNatServiceGet(t *testing.T) {
-	natServiceTestCases := []struct {
-		name                            string
-		spec                            infrastructurev1beta1.OscClusterSpec
-		expSubnetFound                  bool
-		expPublicIpFound                bool
-		expNatServiceFound              bool
-		expGetNatServiceErr             error
-		expReconcileDeleteNatServiceErr error
-	}{
-		{
-			name:                            "failed to get natService",
-			spec:                            defaultNatServiceReconcile,
-			expSubnetFound:                  true,
-			expPublicIpFound:                true,
-			expNatServiceFound:              false,
-			expGetNatServiceErr:             fmt.Errorf("GetNatService generic error"),
-			expReconcileDeleteNatServiceErr: fmt.Errorf("GetNatService generic error"),
-		},
-		{
-			name:                            "Remove finalizer (user delete natService without cluster-api)",
-			spec:                            defaultNatServiceReconcile,
-			expSubnetFound:                  true,
-			expPublicIpFound:                true,
-			expNatServiceFound:              false,
-			expGetNatServiceErr:             nil,
-			expReconcileDeleteNatServiceErr: nil,
-		},
-	}
-	for _, nstc := range natServiceTestCases {
-		t.Run(nstc.name, func(t *testing.T) {
-			clusterScope, ctx, mockOscNatServiceInterface, _ := SetupWithNatServiceMock(t, nstc.name, nstc.spec)
-
-			natServiceName := nstc.spec.Network.NatService.Name + "-uid"
-			natServiceId := "nat-" + natServiceName
-			natServiceRef := clusterScope.GetNatServiceRef()
-			natServiceRef.ResourceMap = make(map[string]string)
-			if nstc.expNatServiceFound {
-				natServiceRef.ResourceMap[natServiceName] = natServiceId
-			}
-			natService := osc.CreateNatServiceResponse{
-				NatService: &osc.NatService{
-					NatServiceId: &natServiceId,
-				},
-			}
-			readNatServices := osc.ReadNatServicesResponse{
-				NatServices: &[]osc.NatService{
-					*natService.NatService,
-				},
-			}
-			readNatService := *readNatServices.NatServices
-			if nstc.expNatServiceFound {
-				mockOscNatServiceInterface.
-					EXPECT().
-					GetNatService(gomock.Eq(natServiceId)).
-					Return(&readNatService[0], nstc.expGetNatServiceErr)
-			} else {
-				mockOscNatServiceInterface.
-					EXPECT().
-					GetNatService(gomock.Eq(natServiceId)).
-					Return(nil, nstc.expGetNatServiceErr)
-			}
-			reconcileDeleteNatService, err := reconcileDeleteNatService(ctx, clusterScope, mockOscNatServiceInterface)
-			if err != nil {
-				assert.Equal(t, nstc.expReconcileDeleteNatServiceErr, err, "reconcileDeleteNatService() should return the same error")
-			} else {
-				assert.Nil(t, nstc.expReconcileDeleteNatServiceErr)
-
-			}
-			t.Logf("Find reconcileDeleteNatService %v\n", reconcileDeleteNatService)
-		})
-	}
-}