Skip to content

Commit

Permalink
Cleanup EPP resource when switching backends (#352)
Browse files Browse the repository at this point in the history
* Cleanup EPP resource when switching backends

* fixed lint issues
  • Loading branch information
mfaizanse authored Dec 20, 2023
1 parent b73d979 commit fdacb2b
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 6 deletions.
12 changes: 10 additions & 2 deletions internal/controller/operator/eventing/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand All @@ -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.
Expand All @@ -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
}

Expand Down
57 changes: 56 additions & 1 deletion internal/controller/operator/eventing/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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)
Expand All @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions internal/controller/operator/eventing/eventmesh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit fdacb2b

Please sign in to comment.