Skip to content

Commit

Permalink
chore(lifecycle-operator): introduce PhaseHandler interface to be use…
Browse files Browse the repository at this point in the history
…d in KeptnWorkloadVersion reconciler (#2450)

Signed-off-by: Florian Bacher <[email protected]>
  • Loading branch information
bacherfl authored Nov 9, 2023
1 parent 6ac5556 commit 7d4b431
Show file tree
Hide file tree
Showing 8 changed files with 210 additions and 126 deletions.
1 change: 1 addition & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ ifdef
ifeq
ifile
ifndef
IHandler
IManager
IMeter
Infof
Expand Down
105 changes: 105 additions & 0 deletions lifecycle-operator/controllers/common/phase/fake/handler_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 22 additions & 8 deletions lifecycle-operator/controllers/common/phase/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
)

//go:generate moq -pkg fake -skip-ensure -out ./fake/handler_mock.go . IHandler:MockHandler
type IHandler interface {
HandlePhase(ctx context.Context, ctxTrace context.Context, tracer trace.Tracer, reconcileObject client.Object, phase apicommon.KeptnPhaseType, reconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error)) (PhaseResult, error)
}

type Handler struct {
client.Client
EventSender eventsender.IEvent
Expand All @@ -28,17 +33,26 @@ type PhaseResult struct {
ctrl.Result
}

func (r Handler) HandlePhase(ctx context.Context, ctxTrace context.Context, tracer trace.Tracer, reconcileObject client.Object, phase apicommon.KeptnPhaseType, reconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error)) (*PhaseResult, error) {
func NewHandler(client client.Client, eventSender eventsender.IEvent, log logr.Logger, spanHandler telemetry.ISpanHandler) Handler {
return Handler{
Client: client,
EventSender: eventSender,
Log: log,
SpanHandler: spanHandler,
}
}

func (r Handler) HandlePhase(ctx context.Context, ctxTrace context.Context, tracer trace.Tracer, reconcileObject client.Object, phase apicommon.KeptnPhaseType, reconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error)) (PhaseResult, error) {
requeueResult := ctrl.Result{Requeue: true, RequeueAfter: 5 * time.Second}
piWrapper, err := interfaces.NewPhaseItemWrapperFromClientObject(reconcileObject)
if err != nil {
return &PhaseResult{Continue: false, Result: ctrl.Result{Requeue: true}}, err
return PhaseResult{Continue: false, Result: ctrl.Result{Requeue: true}}, err
}
oldStatus := piWrapper.GetState()
oldPhase := piWrapper.GetCurrentPhase()
// do not attempt to execute the current phase if the whole phase item is already in deprecated/failed state
if shouldAbortPhase(oldStatus) {
return &PhaseResult{Continue: false, Result: ctrl.Result{}}, nil
return PhaseResult{Continue: false, Result: ctrl.Result{}}, nil
}
if oldPhase != phase.ShortName {
r.EventSender.Emit(phase, "Normal", reconcileObject, apicommon.PhaseStateStarted, "has started", piWrapper.GetVersion())
Expand All @@ -54,7 +68,7 @@ func (r Handler) HandlePhase(ctx context.Context, ctxTrace context.Context, trac
if err != nil {
spanPhaseTrace.AddEvent(phase.LongName + " could not get reconciled")
r.EventSender.Emit(phase, "Warning", reconcileObject, apicommon.PhaseStateReconcileError, "could not get reconciled", piWrapper.GetVersion())
return &PhaseResult{Continue: false, Result: requeueResult}, err
return PhaseResult{Continue: false, Result: requeueResult}, err
}

defer func(ctx context.Context, oldStatus apicommon.KeptnState, oldPhase string, reconcileObject client.Object) {
Expand All @@ -72,14 +86,14 @@ func (r Handler) HandlePhase(ctx context.Context, ctxTrace context.Context, trac

piWrapper.SetState(apicommon.StateProgressing)

return &PhaseResult{Continue: false, Result: requeueResult}, nil
return PhaseResult{Continue: false, Result: requeueResult}, nil
}

func shouldAbortPhase(oldStatus apicommon.KeptnState) bool {
return oldStatus.IsDeprecated() || oldStatus.IsFailed()
}

func (r Handler) handleCompletedPhase(state apicommon.KeptnState, piWrapper *interfaces.PhaseItemWrapper, phase apicommon.KeptnPhaseType, reconcileObject client.Object, spanPhaseTrace trace.Span) (*PhaseResult, error) {
func (r Handler) handleCompletedPhase(state apicommon.KeptnState, piWrapper *interfaces.PhaseItemWrapper, phase apicommon.KeptnPhaseType, reconcileObject client.Object, spanPhaseTrace trace.Span) (PhaseResult, error) {
if state.IsFailed() {
piWrapper.Complete()
piWrapper.SetState(apicommon.StateFailed)
Expand All @@ -91,7 +105,7 @@ func (r Handler) handleCompletedPhase(state apicommon.KeptnState, piWrapper *int
}
r.EventSender.Emit(phase, "Warning", reconcileObject, apicommon.PhaseStateFailed, "has failed", piWrapper.GetVersion())
piWrapper.DeprecateRemainingPhases(phase)
return &PhaseResult{Continue: false, Result: ctrl.Result{}}, nil
return PhaseResult{Continue: false, Result: ctrl.Result{}}, nil
}

// end the current phase do not set the overall state of the whole object to Succeeded here, as this can cause
Expand All @@ -104,5 +118,5 @@ func (r Handler) handleCompletedPhase(state apicommon.KeptnState, piWrapper *int
}
r.EventSender.Emit(phase, "Normal", reconcileObject, apicommon.PhaseStateFinished, "has finished", piWrapper.GetVersion())

return &PhaseResult{Continue: true, Result: ctrl.Result{Requeue: true, RequeueAfter: 5 * time.Second}}, nil
return PhaseResult{Continue: true, Result: ctrl.Result{Requeue: true, RequeueAfter: 5 * time.Second}}, nil
}
31 changes: 23 additions & 8 deletions lifecycle-operator/controllers/common/phase/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestHandler(t *testing.T) {
phase apicommon.KeptnPhaseType
reconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error)
wantObject *v1alpha3.KeptnAppVersion
want *PhaseResult
want PhaseResult
wantErr error
endTimeSet bool
}{
Expand All @@ -42,7 +42,7 @@ func TestHandler(t *testing.T) {
Status: apicommon.StateDeprecated,
},
},
want: &PhaseResult{Continue: false, Result: ctrl.Result{}},
want: PhaseResult{Continue: false, Result: ctrl.Result{}},
wantErr: nil,
wantObject: &v1alpha3.KeptnAppVersion{
Status: v1alpha3.KeptnAppVersionStatus{
Expand All @@ -68,7 +68,7 @@ func TestHandler(t *testing.T) {
reconcilePhase: func(phaseCtx context.Context) (apicommon.KeptnState, error) {
return "", fmt.Errorf("some err")
},
want: &PhaseResult{Continue: false, Result: requeueResult},
want: PhaseResult{Continue: false, Result: requeueResult},
wantErr: fmt.Errorf("some err"),
wantObject: &v1alpha3.KeptnAppVersion{
Status: v1alpha3.KeptnAppVersionStatus{
Expand All @@ -95,7 +95,7 @@ func TestHandler(t *testing.T) {
reconcilePhase: func(phaseCtx context.Context) (apicommon.KeptnState, error) {
return apicommon.StatePending, nil
},
want: &PhaseResult{Continue: false, Result: requeueResult},
want: PhaseResult{Continue: false, Result: requeueResult},
wantErr: nil,
wantObject: &v1alpha3.KeptnAppVersion{
Status: v1alpha3.KeptnAppVersionStatus{
Expand All @@ -122,7 +122,7 @@ func TestHandler(t *testing.T) {
reconcilePhase: func(phaseCtx context.Context) (apicommon.KeptnState, error) {
return apicommon.StateProgressing, nil
},
want: &PhaseResult{Continue: false, Result: requeueResult},
want: PhaseResult{Continue: false, Result: requeueResult},
wantErr: nil,
wantObject: &v1alpha3.KeptnAppVersion{
Status: v1alpha3.KeptnAppVersionStatus{
Expand All @@ -149,7 +149,7 @@ func TestHandler(t *testing.T) {
reconcilePhase: func(phaseCtx context.Context) (apicommon.KeptnState, error) {
return apicommon.StateSucceeded, nil
},
want: &PhaseResult{Continue: true, Result: requeueResult},
want: PhaseResult{Continue: true, Result: requeueResult},
wantErr: nil,
wantObject: &v1alpha3.KeptnAppVersion{
Status: v1alpha3.KeptnAppVersionStatus{
Expand All @@ -176,7 +176,7 @@ func TestHandler(t *testing.T) {
reconcilePhase: func(phaseCtx context.Context) (apicommon.KeptnState, error) {
return apicommon.StateFailed, nil
},
want: &PhaseResult{Continue: false, Result: ctrl.Result{}},
want: PhaseResult{Continue: false, Result: ctrl.Result{}},
wantErr: nil,
wantObject: &v1alpha3.KeptnAppVersion{
Status: v1alpha3.KeptnAppVersionStatus{
Expand Down Expand Up @@ -204,7 +204,7 @@ func TestHandler(t *testing.T) {
reconcilePhase: func(phaseCtx context.Context) (apicommon.KeptnState, error) {
return apicommon.StateUnknown, nil
},
want: &PhaseResult{Continue: false, Result: requeueResult},
want: PhaseResult{Continue: false, Result: requeueResult},
wantErr: nil,
wantObject: &v1alpha3.KeptnAppVersion{
Status: v1alpha3.KeptnAppVersionStatus{
Expand All @@ -226,3 +226,18 @@ func TestHandler(t *testing.T) {
})
}
}

func TestNewHandler(t *testing.T) {
spanHandler := &telemetry.Handler{}
log := ctrl.Log.WithName("controller")
eventSender := eventsender.NewK8sSender(record.NewFakeRecorder(100))
client := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()

handler := NewHandler(client, eventSender, log, spanHandler)

require.NotNil(t, handler)
require.NotNil(t, handler.Client)
require.NotNil(t, handler.EventSender)
require.NotNil(t, handler.Log)
require.NotNil(t, handler.SpanHandler)
}
Loading

0 comments on commit 7d4b431

Please sign in to comment.