diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index 62f0fe1558..848d32c84f 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -237,6 +237,7 @@ ifdef ifeq ifile ifndef +IHandler IManager IMeter Infof diff --git a/lifecycle-operator/controllers/common/phase/fake/handler_mock.go b/lifecycle-operator/controllers/common/phase/fake/handler_mock.go new file mode 100644 index 0000000000..3832e7fc5b --- /dev/null +++ b/lifecycle-operator/controllers/common/phase/fake/handler_mock.go @@ -0,0 +1,105 @@ +// Code generated by moq; DO NOT EDIT. +// github.com/matryer/moq + +package fake + +import ( + "context" + apicommon "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1alpha3/common" + "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/phase" + "go.opentelemetry.io/otel/trace" + "sigs.k8s.io/controller-runtime/pkg/client" + "sync" +) + +// MockHandler is a mock implementation of phase.IHandler. +// +// func TestSomethingThatUsesIHandler(t *testing.T) { +// +// // make and configure a mocked phase.IHandler +// mockedIHandler := &MockHandler{ +// HandlePhaseFunc: func(ctx context.Context, ctxTrace context.Context, tracer trace.Tracer, reconcileObject client.Object, phaseMoqParam apicommon.KeptnPhaseType, reconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error)) (phase.PhaseResult, error) { +// panic("mock out the HandlePhase method") +// }, +// } +// +// // use mockedIHandler in code that requires phase.IHandler +// // and then make assertions. +// +// } +type MockHandler struct { + // HandlePhaseFunc mocks the HandlePhase method. + HandlePhaseFunc func(ctx context.Context, ctxTrace context.Context, tracer trace.Tracer, reconcileObject client.Object, phaseMoqParam apicommon.KeptnPhaseType, reconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error)) (phase.PhaseResult, error) + + // calls tracks calls to the methods. + calls struct { + // HandlePhase holds details about calls to the HandlePhase method. + HandlePhase []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // CtxTrace is the ctxTrace argument value. + CtxTrace context.Context + // Tracer is the tracer argument value. + Tracer trace.Tracer + // ReconcileObject is the reconcileObject argument value. + ReconcileObject client.Object + // PhaseMoqParam is the phaseMoqParam argument value. + PhaseMoqParam apicommon.KeptnPhaseType + // ReconcilePhase is the reconcilePhase argument value. + ReconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error) + } + } + lockHandlePhase sync.RWMutex +} + +// HandlePhase calls HandlePhaseFunc. +func (mock *MockHandler) HandlePhase(ctx context.Context, ctxTrace context.Context, tracer trace.Tracer, reconcileObject client.Object, phaseMoqParam apicommon.KeptnPhaseType, reconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error)) (phase.PhaseResult, error) { + if mock.HandlePhaseFunc == nil { + panic("MockHandler.HandlePhaseFunc: method is nil but IHandler.HandlePhase was just called") + } + callInfo := struct { + Ctx context.Context + CtxTrace context.Context + Tracer trace.Tracer + ReconcileObject client.Object + PhaseMoqParam apicommon.KeptnPhaseType + ReconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error) + }{ + Ctx: ctx, + CtxTrace: ctxTrace, + Tracer: tracer, + ReconcileObject: reconcileObject, + PhaseMoqParam: phaseMoqParam, + ReconcilePhase: reconcilePhase, + } + mock.lockHandlePhase.Lock() + mock.calls.HandlePhase = append(mock.calls.HandlePhase, callInfo) + mock.lockHandlePhase.Unlock() + return mock.HandlePhaseFunc(ctx, ctxTrace, tracer, reconcileObject, phaseMoqParam, reconcilePhase) +} + +// HandlePhaseCalls gets all the calls that were made to HandlePhase. +// Check the length with: +// +// len(mockedIHandler.HandlePhaseCalls()) +func (mock *MockHandler) HandlePhaseCalls() []struct { + Ctx context.Context + CtxTrace context.Context + Tracer trace.Tracer + ReconcileObject client.Object + PhaseMoqParam apicommon.KeptnPhaseType + ReconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error) +} { + var calls []struct { + Ctx context.Context + CtxTrace context.Context + Tracer trace.Tracer + ReconcileObject client.Object + PhaseMoqParam apicommon.KeptnPhaseType + ReconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error) + } + mock.lockHandlePhase.RLock() + calls = mock.calls.HandlePhase + mock.lockHandlePhase.RUnlock() + return calls +} diff --git a/lifecycle-operator/controllers/common/phase/handler.go b/lifecycle-operator/controllers/common/phase/handler.go index 96930f0fc1..1db48af08a 100644 --- a/lifecycle-operator/controllers/common/phase/handler.go +++ b/lifecycle-operator/controllers/common/phase/handler.go @@ -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 @@ -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()) @@ -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) { @@ -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) @@ -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 @@ -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 } diff --git a/lifecycle-operator/controllers/common/phase/handler_test.go b/lifecycle-operator/controllers/common/phase/handler_test.go index c2cea7f6cb..2ae5ce5f56 100644 --- a/lifecycle-operator/controllers/common/phase/handler_test.go +++ b/lifecycle-operator/controllers/common/phase/handler_test.go @@ -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 }{ @@ -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{ @@ -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{ @@ -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{ @@ -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{ @@ -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{ @@ -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{ @@ -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{ @@ -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) +} diff --git a/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller.go b/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller.go index b82ea452c4..aa35cb48cd 100644 --- a/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller.go +++ b/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller.go @@ -59,6 +59,7 @@ type KeptnWorkloadVersionReconciler struct { TracerFactory telemetry.TracerFactory SchedulingGatesHandler schedulinggates.ISchedulingGatesHandler EvaluationHandler evaluation.IEvaluationHandler + PhaseHandler phase.IHandler } // +kubebuilder:rbac:groups=lifecycle.keptn.sh,resources=keptnworkloadversions,verbs=get;list;watch;create;update;patch;delete @@ -105,13 +106,6 @@ func (r *KeptnWorkloadVersionReconciler) Reconcile(ctx context.Context, req ctrl appTraceContextCarrier := propagation.MapCarrier(workloadVersion.Spec.TraceId) ctxAppTrace := otel.GetTextMapPropagator().Extract(context.TODO(), appTraceContextCarrier) - phaseHandler := phase.Handler{ - Client: r.Client, - EventSender: r.EventSender, - Log: r.Log, - SpanHandler: r.SpanHandler, - } - // this will be the parent span for all phases of the WorkloadVersion ctxWorkloadTrace, spanWorkloadTrace, err := r.SpanHandler.GetSpan(ctxAppTrace, r.getTracer(), workloadVersion, "") if err != nil { @@ -123,12 +117,12 @@ func (r *KeptnWorkloadVersionReconciler) Reconcile(ctx context.Context, req ctrl } // Wait for pre-deployment checks of Workload - if result, err := r.doPreDeploymentTaskPhase(ctx, workloadVersion, phaseHandler, ctxWorkloadTrace); !result.Continue { + if result, err := r.doPreDeploymentTaskPhase(ctx, workloadVersion, ctxWorkloadTrace); !result.Continue { return result.Result, err } // Wait for pre-evaluation checks of Workload - if result, err := r.doPreDeploymentEvaluationPhase(ctx, workloadVersion, phaseHandler, ctxWorkloadTrace); !result.Continue { + if result, err := r.doPreDeploymentEvaluationPhase(ctx, workloadVersion, ctxWorkloadTrace); !result.Continue { return result.Result, err } @@ -141,17 +135,17 @@ func (r *KeptnWorkloadVersionReconciler) Reconcile(ctx context.Context, req ctrl } // Wait for deployment of Workload - if result, err := r.doDeploymentPhase(ctx, workloadVersion, phaseHandler, ctxWorkloadTrace); !result.Continue { + if result, err := r.doDeploymentPhase(ctx, workloadVersion, ctxWorkloadTrace); !result.Continue { return result.Result, err } // Wait for post-deployment checks of Workload - if result, err := r.doPostDeploymentTaskPhase(ctx, workloadVersion, phaseHandler, ctxWorkloadTrace); !result.Continue { + if result, err := r.doPostDeploymentTaskPhase(ctx, workloadVersion, ctxWorkloadTrace); !result.Continue { return result.Result, err } // Wait for post-evaluation checks of Workload - if result, err := r.doPostDeploymentEvaluationPhase(ctx, workloadVersion, phaseHandler, ctxWorkloadTrace); !result.Continue { + if result, err := r.doPostDeploymentEvaluationPhase(ctx, workloadVersion, ctxWorkloadTrace); !result.Continue { return result.Result, err } @@ -159,12 +153,12 @@ func (r *KeptnWorkloadVersionReconciler) Reconcile(ctx context.Context, req ctrl return r.finishKeptnWorkloadVersionReconcile(ctx, workloadVersion, spanWorkloadTrace) } -func (r *KeptnWorkloadVersionReconciler) doPreDeploymentTaskPhase(ctx context.Context, workloadVersion *klcv1alpha4.KeptnWorkloadVersion, phaseHandler phase.Handler, ctxWorkloadTrace context.Context) (*phase.PhaseResult, error) { +func (r *KeptnWorkloadVersionReconciler) doPreDeploymentTaskPhase(ctx context.Context, workloadVersion *klcv1alpha4.KeptnWorkloadVersion, ctxWorkloadTrace context.Context) (phase.PhaseResult, error) { if !workloadVersion.IsPreDeploymentSucceeded() { reconcilePre := func(phaseCtx context.Context) (apicommon.KeptnState, error) { return r.reconcilePrePostDeployment(ctx, phaseCtx, workloadVersion, apicommon.PreDeploymentCheckType) } - return phaseHandler.HandlePhase(ctx, + return r.PhaseHandler.HandlePhase(ctx, ctxWorkloadTrace, r.getTracer(), workloadVersion, @@ -172,17 +166,17 @@ func (r *KeptnWorkloadVersionReconciler) doPreDeploymentTaskPhase(ctx context.Co reconcilePre, ) } - return &phase.PhaseResult{ + return phase.PhaseResult{ Continue: true, }, nil } -func (r *KeptnWorkloadVersionReconciler) doPreDeploymentEvaluationPhase(ctx context.Context, workloadVersion *klcv1alpha4.KeptnWorkloadVersion, phaseHandler phase.Handler, ctxWorkloadTrace context.Context) (*phase.PhaseResult, error) { +func (r *KeptnWorkloadVersionReconciler) doPreDeploymentEvaluationPhase(ctx context.Context, workloadVersion *klcv1alpha4.KeptnWorkloadVersion, ctxWorkloadTrace context.Context) (phase.PhaseResult, error) { if !workloadVersion.IsPreDeploymentEvaluationSucceeded() { reconcilePreEval := func(phaseCtx context.Context) (apicommon.KeptnState, error) { return r.reconcilePrePostEvaluation(ctx, phaseCtx, workloadVersion, apicommon.PreDeploymentEvaluationCheckType) } - return phaseHandler.HandlePhase(ctx, + return r.PhaseHandler.HandlePhase(ctx, ctxWorkloadTrace, r.getTracer(), workloadVersion, @@ -190,17 +184,17 @@ func (r *KeptnWorkloadVersionReconciler) doPreDeploymentEvaluationPhase(ctx cont reconcilePreEval, ) } - return &phase.PhaseResult{ + return phase.PhaseResult{ Continue: true, }, nil } -func (r *KeptnWorkloadVersionReconciler) doDeploymentPhase(ctx context.Context, workloadVersion *klcv1alpha4.KeptnWorkloadVersion, phaseHandler phase.Handler, ctxWorkloadTrace context.Context) (*phase.PhaseResult, error) { +func (r *KeptnWorkloadVersionReconciler) doDeploymentPhase(ctx context.Context, workloadVersion *klcv1alpha4.KeptnWorkloadVersion, ctxWorkloadTrace context.Context) (phase.PhaseResult, error) { if !workloadVersion.IsDeploymentSucceeded() { reconcileWorkloadVersion := func(phaseCtx context.Context) (apicommon.KeptnState, error) { return r.reconcileDeployment(ctx, workloadVersion) } - return phaseHandler.HandlePhase(ctx, + return r.PhaseHandler.HandlePhase(ctx, ctxWorkloadTrace, r.getTracer(), workloadVersion, @@ -208,17 +202,17 @@ func (r *KeptnWorkloadVersionReconciler) doDeploymentPhase(ctx context.Context, reconcileWorkloadVersion, ) } - return &phase.PhaseResult{ + return phase.PhaseResult{ Continue: true, }, nil } -func (r *KeptnWorkloadVersionReconciler) doPostDeploymentTaskPhase(ctx context.Context, workloadVersion *klcv1alpha4.KeptnWorkloadVersion, phaseHandler phase.Handler, ctxWorkloadTrace context.Context) (*phase.PhaseResult, error) { +func (r *KeptnWorkloadVersionReconciler) doPostDeploymentTaskPhase(ctx context.Context, workloadVersion *klcv1alpha4.KeptnWorkloadVersion, ctxWorkloadTrace context.Context) (phase.PhaseResult, error) { if !workloadVersion.IsPostDeploymentCompleted() { reconcilePost := func(phaseCtx context.Context) (apicommon.KeptnState, error) { return r.reconcilePrePostDeployment(ctx, phaseCtx, workloadVersion, apicommon.PostDeploymentCheckType) } - return phaseHandler.HandlePhase(ctx, + return r.PhaseHandler.HandlePhase(ctx, ctxWorkloadTrace, r.getTracer(), workloadVersion, @@ -226,17 +220,17 @@ func (r *KeptnWorkloadVersionReconciler) doPostDeploymentTaskPhase(ctx context.C reconcilePost, ) } - return &phase.PhaseResult{ + return phase.PhaseResult{ Continue: true, }, nil } -func (r *KeptnWorkloadVersionReconciler) doPostDeploymentEvaluationPhase(ctx context.Context, workloadVersion *klcv1alpha4.KeptnWorkloadVersion, phaseHandler phase.Handler, ctxWorkloadTrace context.Context) (*phase.PhaseResult, error) { +func (r *KeptnWorkloadVersionReconciler) doPostDeploymentEvaluationPhase(ctx context.Context, workloadVersion *klcv1alpha4.KeptnWorkloadVersion, ctxWorkloadTrace context.Context) (phase.PhaseResult, error) { if !workloadVersion.IsPostDeploymentEvaluationSucceeded() { reconcilePostEval := func(phaseCtx context.Context) (apicommon.KeptnState, error) { return r.reconcilePrePostEvaluation(ctx, phaseCtx, workloadVersion, apicommon.PostDeploymentEvaluationCheckType) } - return phaseHandler.HandlePhase(ctx, + return r.PhaseHandler.HandlePhase(ctx, ctxWorkloadTrace, r.getTracer(), workloadVersion, @@ -244,7 +238,7 @@ func (r *KeptnWorkloadVersionReconciler) doPostDeploymentEvaluationPhase(ctx con reconcilePostEval, ) } - return &phase.PhaseResult{ + return phase.PhaseResult{ Continue: true, }, nil } diff --git a/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller_test.go b/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller_test.go index 6e8572b37d..817207dfed 100644 --- a/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller_test.go +++ b/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller_test.go @@ -14,6 +14,8 @@ import ( "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/evaluation" evaluationfake "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/evaluation/fake" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/eventsender" + "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/phase" + phasefake "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/phase/fake" schedulinggatesfake "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/schedulinggates/fake" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/telemetry" telemetryfake "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/telemetry/fake" @@ -1038,81 +1040,11 @@ func TestKeptnWorkloadVersionReconciler_ReconcileFailed(t *testing.T) { }, ) - r, eventChannel, _ := setupReconciler(app, wi) + r, _, _ := setupReconciler(app, wi) - req := ctrl.Request{ - NamespacedName: types.NamespacedName{ - Namespace: testNamespace, - Name: "some-wi", - }, - } - - result, err := r.Reconcile(context.TODO(), req) - - require.Nil(t, err) - - // do not requeue since we reached completion - require.False(t, result.Requeue) - - // here we do not expect an event about the application preEvaluation being finished since that will have been sent in - // one of the previous reconciliation loops that lead to the first phase being reached - expectedEvents := []string{ - "WorkloadPreDeployTasksFailed", - } - - for _, e := range expectedEvents { - event := <-eventChannel - require.Equal(t, strings.Contains(event, req.Name), true, "wrong workloadVersion") - require.Equal(t, strings.Contains(event, req.Namespace), true, "wrong namespace") - require.Equal(t, strings.Contains(event, e), true, fmt.Sprintf("no %s found in %s", e, event)) - } -} - -func TestKeptnWorkloadVersionReconciler_ReconcileDoNotRetryAfterFailedPhase(t *testing.T) { - - testNamespace := "some-ns" - - wi := &klcv1alpha4.KeptnWorkloadVersion{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: "some-wi", - Namespace: testNamespace, - }, - Spec: klcv1alpha4.KeptnWorkloadVersionSpec{ - KeptnWorkloadSpec: klcv1alpha3.KeptnWorkloadSpec{ - AppName: "some-app", - Version: "1.0.0", - }, - WorkloadName: "some-app-some-workload", - PreviousVersion: "", - TraceId: nil, - }, - Status: klcv1alpha4.KeptnWorkloadVersionStatus{ - CurrentPhase: apicommon.PhaseWorkloadPreDeployment.ShortName, - StartTime: metav1.Time{}, - EndTime: metav1.Time{}, - }, - } - - // simulate a KWI that has been cancelled due to a failed pre deployment check - wi.DeprecateRemainingPhases(apicommon.PhaseWorkloadPreDeployment) - - app := testcommon.ReturnAppVersion( - testNamespace, - "some-app", - "1.0.0", - []klcv1alpha3.KeptnWorkloadRef{ - { - Name: "some-workload", - Version: "1.0.0", - }, - }, - klcv1alpha3.KeptnAppVersionStatus{ - PreDeploymentEvaluationStatus: apicommon.StateSucceeded, - }, - ) - - r, eventChannel, _ := setupReconciler(wi, app) + r.PhaseHandler = &phasefake.MockHandler{HandlePhaseFunc: func(ctx context.Context, ctxTrace context.Context, tracer trace.Tracer, reconcileObject client.Object, phaseMoqParam apicommon.KeptnPhaseType, reconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error)) (phase.PhaseResult, error) { + return phase.PhaseResult{Continue: false, Result: ctrl.Result{Requeue: false}}, nil + }} req := ctrl.Request{ NamespacedName: types.NamespacedName{ @@ -1125,10 +1057,8 @@ func TestKeptnWorkloadVersionReconciler_ReconcileDoNotRetryAfterFailedPhase(t *t require.Nil(t, err) - // do not requeue since we were cancelled earlier + // do not requeue since we reached completion require.False(t, result.Requeue) - - require.Empty(t, len(eventChannel)) } func TestKeptnWorkloadVersionReconciler_ReconcileCouldNotRetrieveObject(t *testing.T) { @@ -1208,6 +1138,14 @@ func TestKeptnWorkloadVersionReconciler_ReconcilePreDeploymentEvaluationUnexpect return nil, apicommon.StatusSummary{}, errors.New("unexpected error") } + mockPhaseHandler := &phasefake.MockHandler{ + HandlePhaseFunc: func(ctx context.Context, ctxTrace context.Context, tracer trace.Tracer, reconcileObject client.Object, phaseMoqParam apicommon.KeptnPhaseType, reconcilePhase func(phaseCtx context.Context) (apicommon.KeptnState, error)) (phase.PhaseResult, error) { + return phase.PhaseResult{Continue: false, Result: ctrl.Result{Requeue: true}}, errors.New("unexpected error") + }, + } + + r.PhaseHandler = mockPhaseHandler + req := ctrl.Request{ NamespacedName: types.NamespacedName{ Namespace: testNamespace, diff --git a/lifecycle-operator/main.go b/lifecycle-operator/main.go index 83e59d979d..7f193aa994 100644 --- a/lifecycle-operator/main.go +++ b/lifecycle-operator/main.go @@ -38,6 +38,7 @@ import ( "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/config" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/evaluation" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/eventsender" + "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/phase" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/schedulinggates" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/telemetry" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/lifecycle/keptnapp" @@ -283,6 +284,12 @@ func main() { mgr.GetScheme(), spanHandler, ) + workloadVersionPhaseHandler := phase.NewHandler( + mgr.GetClient(), + workloadVersionEventSender, + workloadVersionLogger, + spanHandler, + ) workloadVersionReconciler := &keptnworkloadversion.KeptnWorkloadVersionReconciler{ SchedulingGatesHandler: schedulinggates.NewHandler(mgr.GetClient(), workloadVersionLogger, env.SchedulingGatesEnabled), Client: mgr.GetClient(), @@ -293,6 +300,7 @@ func main() { TracerFactory: telemetry.GetOtelInstance(), SpanHandler: spanHandler, EvaluationHandler: workloadVersionEvaluationHandler, + PhaseHandler: workloadVersionPhaseHandler, } if err = (workloadVersionReconciler).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "KeptnWorkloadVersion") diff --git a/lifecycle-operator/test/component/workloadversion/workloadversion_suite_test.go b/lifecycle-operator/test/component/workloadversion/workloadversion_suite_test.go index b68ec2fc93..5e7b5319d6 100644 --- a/lifecycle-operator/test/component/workloadversion/workloadversion_suite_test.go +++ b/lifecycle-operator/test/component/workloadversion/workloadversion_suite_test.go @@ -9,6 +9,7 @@ import ( "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/config" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/evaluation" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/eventsender" + "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/phase" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/schedulinggates" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/telemetry" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/lifecycle/keptnworkloadversion" @@ -19,8 +20,6 @@ import ( sdktest "go.opentelemetry.io/otel/sdk/trace/tracetest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - // nolint:gci - // +kubebuilder:scaffold:imports ) func TestWorkloadVersion(t *testing.T) { @@ -45,27 +44,37 @@ var _ = BeforeSuite(func() { var readyToStart chan struct{} ctx, k8sManager, tracer, spanRecorder, k8sClient, readyToStart = common.InitSuite() + eventSender := eventsender.NewK8sSender(k8sManager.GetEventRecorderFor("test-workloadversion-controller")) + tracerFactory := &common.TracerFactory{Tracer: tracer} - EvaluationHandler := evaluation.NewHandler( + evaluationHandler := evaluation.NewHandler( k8sManager.GetClient(), - eventsender.NewK8sSender(k8sManager.GetEventRecorderFor("test-workloadversion-controller")), + eventSender, GinkgoLogr, tracerFactory.GetTracer(traceComponentName), k8sManager.GetScheme(), &telemetry.Handler{}) + phaseHandler := phase.NewHandler( + k8sManager.GetClient(), + eventSender, + GinkgoLogr, + &telemetry.Handler{}, + ) + // //setup controllers here config.Instance().SetDefaultNamespace(KeptnNamespace) controller := &keptnworkloadversion.KeptnWorkloadVersionReconciler{ SchedulingGatesHandler: schedulinggates.NewHandler(nil, GinkgoLogr, false), Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme(), - EventSender: eventsender.NewK8sSender(k8sManager.GetEventRecorderFor("test-workloadversion-controller")), + EventSender: eventSender, Log: GinkgoLogr, Meters: common.InitKeptnMeters(), SpanHandler: &telemetry.Handler{}, TracerFactory: tracerFactory, - EvaluationHandler: EvaluationHandler, + EvaluationHandler: evaluationHandler, + PhaseHandler: phaseHandler, } Eventually(controller.SetupWithManager(k8sManager)).WithTimeout(30 * time.Second).WithPolling(time.Second).Should(Succeed()) close(readyToStart)