From b9910f2ebd6addd1bcfb3eb61e594ff5da8c0f75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20GLON?= Date: Tue, 13 Aug 2024 11:12:01 +0200 Subject: [PATCH] fix(natservice): delete resource from status refs --- .../osccluster_natservice_controller.go | 20 +---- ...cluster_natservice_controller_unit_test.go | 75 ------------------- 2 files changed, 4 insertions(+), 91 deletions(-) diff --git a/controllers/osccluster_natservice_controller.go b/controllers/osccluster_natservice_controller.go index 083aeea26..fc4c7b729 100644 --- 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,28 +183,16 @@ 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) @@ -213,6 +200,7 @@ func reconcileDeleteNatService(ctx context.Context, clusterScope *scope.ClusterS 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 index ce3165c8d..36d197347 100644 --- 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) - }) - } -}