From fdacb2bde1edd835fdb9535a4086637243dfd2fc Mon Sep 17 00:00:00 2001 From: Muhammad Faizan Date: Wed, 20 Dec 2023 15:22:09 +0100 Subject: [PATCH] Cleanup EPP resource when switching backends (#352) * Cleanup EPP resource when switching backends * fixed lint issues --- .../operator/eventing/controller.go | 12 +++- .../operator/eventing/controller_test.go | 57 ++++++++++++++++++- .../operator/eventing/eventmesh_test.go | 7 ++- 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/internal/controller/operator/eventing/controller.go b/internal/controller/operator/eventing/controller.go index e07f3923..87773fdb 100644 --- a/internal/controller/operator/eventing/controller.go +++ b/internal/controller/operator/eventing/controller.go @@ -470,7 +470,7 @@ func (r *Reconciler) handleEventingReconcile(ctx context.Context, // handle switching of backend. if eventing.Status.ActiveBackend != "" { - if err := r.handleBackendSwitching(eventing, log); err != nil { + if err := r.handleBackendSwitching(ctx, eventing, log); err != nil { return kctrl.Result{}, r.syncStatusWithSubscriptionManagerErr(ctx, eventing, err, log) } } @@ -497,7 +497,7 @@ func (r *Reconciler) handleEventingReconcile(ctx context.Context, } } -func (r *Reconciler) handleBackendSwitching( +func (r *Reconciler) handleBackendSwitching(ctx context.Context, eventing *operatorv1alpha1.Eventing, log *zap.SugaredLogger, ) error { // check if the backend was changed. @@ -521,6 +521,14 @@ func (r *Reconciler) handleBackendSwitching( // update the Eventing CR status. eventing.Status.SetStateProcessing() eventing.Status.ClearConditions() + + // delete publisher proxy resources. + log.Infof("deleting publisher proxy resources") + err := r.eventingManager.DeletePublisherProxyResources(ctx, eventing) + if err != nil { + return err + } + return nil } diff --git a/internal/controller/operator/eventing/controller_test.go b/internal/controller/operator/eventing/controller_test.go index f85dba1c..e70dcf38 100644 --- a/internal/controller/operator/eventing/controller_test.go +++ b/internal/controller/operator/eventing/controller_test.go @@ -12,6 +12,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/event" operatorv1alpha1 "github.com/kyma-project/eventing-manager/api/operator/v1alpha1" + eventingmocks "github.com/kyma-project/eventing-manager/pkg/eventing/mocks" submgrmanagermocks "github.com/kyma-project/eventing-manager/pkg/subscriptionmanager/manager/mocks" "github.com/kyma-project/eventing-manager/pkg/watcher" watchermocks "github.com/kyma-project/eventing-manager/pkg/watcher/mocks" @@ -118,6 +119,7 @@ func Test_handleBackendSwitching(t *testing.T) { givenEventing *operatorv1alpha1.Eventing givenNATSSubManagerMock func() *submgrmanagermocks.Manager givenEventMeshSubManagerMock func() *submgrmanagermocks.Manager + givenEventingManagerMock func() *eventingmocks.Manager wantNATSStopped bool wantEventMeshStopped bool wantEventingState string @@ -138,6 +140,9 @@ func Test_handleBackendSwitching(t *testing.T) { givenEventMeshSubManagerMock: func() *submgrmanagermocks.Manager { return new(submgrmanagermocks.Manager) }, + givenEventingManagerMock: func() *eventingmocks.Manager { + return new(eventingmocks.Manager) + }, wantError: nil, wantEventingState: operatorv1alpha1.StateReady, wantEventingConditionsLen: 1, @@ -160,6 +165,12 @@ func Test_handleBackendSwitching(t *testing.T) { givenEventMeshSubManagerMock: func() *submgrmanagermocks.Manager { return new(submgrmanagermocks.Manager) }, + givenEventingManagerMock: func() *eventingmocks.Manager { + emMock := new(eventingmocks.Manager) + emMock.On("DeletePublisherProxyResources", mock.Anything, + mock.Anything).Return(nil).Once() + return emMock + }, wantError: nil, wantEventingState: operatorv1alpha1.StateProcessing, wantEventingConditionsLen: 0, @@ -182,6 +193,9 @@ func Test_handleBackendSwitching(t *testing.T) { givenEventMeshSubManagerMock: func() *submgrmanagermocks.Manager { return new(submgrmanagermocks.Manager) }, + givenEventingManagerMock: func() *eventingmocks.Manager { + return new(eventingmocks.Manager) + }, wantError: ErrFailedToStop, wantEventingState: operatorv1alpha1.StateReady, wantEventingConditionsLen: 1, @@ -204,6 +218,12 @@ func Test_handleBackendSwitching(t *testing.T) { managerMock.On("Stop", true).Return(nil).Once() return managerMock }, + givenEventingManagerMock: func() *eventingmocks.Manager { + emMock := new(eventingmocks.Manager) + emMock.On("DeletePublisherProxyResources", mock.Anything, + mock.Anything).Return(nil).Once() + return emMock + }, wantError: nil, wantEventingState: operatorv1alpha1.StateProcessing, wantEventingConditionsLen: 0, @@ -226,12 +246,43 @@ func Test_handleBackendSwitching(t *testing.T) { managerMock.On("Stop", true).Return(ErrFailedToStop).Once() return managerMock }, + givenEventingManagerMock: func() *eventingmocks.Manager { + return new(eventingmocks.Manager) + }, wantError: ErrFailedToStop, wantEventingState: operatorv1alpha1.StateReady, wantEventingConditionsLen: 1, wantNATSStopped: false, wantEventMeshStopped: false, }, + { + name: "it should return error because it failed to remove EPP resources", + givenEventing: testutils.NewEventingCR( + testutils.WithEventMeshBackend("test"), + testutils.WithStatusActiveBackend(operatorv1alpha1.NatsBackendType), + testutils.WithStatusState(operatorv1alpha1.StateReady), + testutils.WithStatusConditions([]kmetav1.Condition{{Type: "Available"}}), + ), + givenNATSSubManagerMock: func() *submgrmanagermocks.Manager { + managerMock := new(submgrmanagermocks.Manager) + managerMock.On("Stop", true).Return(nil).Once() + return managerMock + }, + givenEventMeshSubManagerMock: func() *submgrmanagermocks.Manager { + return new(submgrmanagermocks.Manager) + }, + givenEventingManagerMock: func() *eventingmocks.Manager { + emMock := new(eventingmocks.Manager) + emMock.On("DeletePublisherProxyResources", mock.Anything, + mock.Anything).Return(ErrFailedToRemove).Once() + return emMock + }, + wantError: ErrFailedToRemove, + wantEventingState: operatorv1alpha1.StateProcessing, + wantEventingConditionsLen: 0, + wantNATSStopped: true, + wantEventMeshStopped: false, + }, } // run test cases @@ -255,13 +306,15 @@ func Test_handleBackendSwitching(t *testing.T) { // get mocks from test-case. givenNATSSubManagerMock := tc.givenNATSSubManagerMock() givenEventMeshSubManagerMock := tc.givenEventMeshSubManagerMock() + givenEventingManagerMock := tc.givenEventingManagerMock() // connect mocks with reconciler. testEnv.Reconciler.natsSubManager = givenNATSSubManagerMock testEnv.Reconciler.eventMeshSubManager = givenEventMeshSubManagerMock + testEnv.Reconciler.eventingManager = givenEventingManagerMock // when - err := testEnv.Reconciler.handleBackendSwitching(tc.givenEventing, logger) + err := testEnv.Reconciler.handleBackendSwitching(context.TODO(), tc.givenEventing, logger) // then if tc.wantError != nil { @@ -283,6 +336,7 @@ func Test_handleBackendSwitching(t *testing.T) { if tc.wantNATSStopped { require.Nil(t, testEnv.Reconciler.natsSubManager) require.False(t, testEnv.Reconciler.isNATSSubManagerStarted) + givenEventingManagerMock.AssertExpectations(t) } else { require.NotNil(t, testEnv.Reconciler.natsSubManager) require.True(t, testEnv.Reconciler.isNATSSubManagerStarted) @@ -292,6 +346,7 @@ func Test_handleBackendSwitching(t *testing.T) { if tc.wantEventMeshStopped { require.Nil(t, testEnv.Reconciler.eventMeshSubManager) require.False(t, testEnv.Reconciler.isEventMeshSubManagerStarted) + givenEventingManagerMock.AssertExpectations(t) } else { require.NotNil(t, testEnv.Reconciler.eventMeshSubManager) require.True(t, testEnv.Reconciler.isEventMeshSubManagerStarted) diff --git a/internal/controller/operator/eventing/eventmesh_test.go b/internal/controller/operator/eventing/eventmesh_test.go index 3f33ea56..a80d33d2 100644 --- a/internal/controller/operator/eventing/eventmesh_test.go +++ b/internal/controller/operator/eventing/eventmesh_test.go @@ -31,9 +31,10 @@ const ( ) var ( - ErrFailedToStart = errors.New("failed to start") - ErrFailedToStop = errors.New("failed to stop") - errNotFound = errors.New("secret not found") + ErrFailedToStart = errors.New("failed to start") + ErrFailedToStop = errors.New("failed to stop") + ErrFailedToRemove = errors.New("failed to remove") + errNotFound = errors.New("secret not found") ) //nolint:goerr113 // all tests here need to be fixed, as they use require.ErrorAs and use it wrongly