From 7f15baf85bdc7f30537ad9ce5a0c582e65ffb16f Mon Sep 17 00:00:00 2001 From: odubajDT <93584209+odubajDT@users.noreply.github.com> Date: Fri, 18 Nov 2022 10:26:08 +0100 Subject: [PATCH] fix(operator): cancel pending phases when evaluation fails (#408) Fixes https://github.com/keptn/lifecycle-toolkit/issues/381 --- operator/api/v1alpha1/common/common.go | 14 +- operator/api/v1alpha1/common/common_test.go | 304 ++++++++++++++++++ operator/api/v1alpha1/common/phases.go | 1 + .../api/v1alpha1/keptnappversion_types.go | 16 + .../v1alpha1/keptnworkloadinstance_types.go | 17 + .../v1alpha1/tests/keptnappversion_test.go | 70 ++++ .../tests/keptnworkloadinstance_test.go | 70 ++++ .../controllers/common/fake/phaseitem_mock.go | 43 +++ operator/controllers/common/interfaces.go | 5 + .../controllers/common/interfaces_test.go | 6 +- operator/controllers/common/phasehandler.go | 24 +- .../controllers/common/phasehandler_test.go | 227 +++++++++++++ .../workloadinstancecontroller_test.go | 99 ++++++ 13 files changed, 882 insertions(+), 14 deletions(-) create mode 100644 operator/api/v1alpha1/common/common_test.go create mode 100644 operator/controllers/common/phasehandler_test.go diff --git a/operator/api/v1alpha1/common/common.go b/operator/api/v1alpha1/common/common.go index ff05e89a19..182975445b 100644 --- a/operator/api/v1alpha1/common/common.go +++ b/operator/api/v1alpha1/common/common.go @@ -39,10 +39,11 @@ const ( StateFailed KeptnState = "Failed" StateUnknown KeptnState = "Unknown" StatePending KeptnState = "Pending" + StateCancelled KeptnState = "Cancelled" ) func (k KeptnState) IsCompleted() bool { - return k == StateSucceeded || k == StateFailed + return k == StateSucceeded || k == StateFailed || k == StateCancelled } func (k KeptnState) IsSucceeded() bool { @@ -53,6 +54,10 @@ func (k KeptnState) IsFailed() bool { return k == StateFailed } +func (k KeptnState) IsCancelled() bool { + return k == StateCancelled +} + func (k KeptnState) IsPending() bool { return k == StatePending } @@ -64,12 +69,15 @@ type StatusSummary struct { succeeded int pending int unknown int + cancelled int } func UpdateStatusSummary(status KeptnState, summary StatusSummary) StatusSummary { switch status { case StateFailed: summary.failed++ + case StateCancelled: + summary.cancelled++ case StateSucceeded: summary.succeeded++ case StateProgressing: @@ -83,11 +91,11 @@ func UpdateStatusSummary(status KeptnState, summary StatusSummary) StatusSummary } func (s StatusSummary) GetTotalCount() int { - return s.failed + s.succeeded + s.progressing + s.pending + s.unknown + return s.failed + s.succeeded + s.progressing + s.pending + s.unknown + s.cancelled } func GetOverallState(s StatusSummary) KeptnState { - if s.failed > 0 { + if s.failed > 0 || s.cancelled > 0 { return StateFailed } if s.progressing > 0 { diff --git a/operator/api/v1alpha1/common/common_test.go b/operator/api/v1alpha1/common/common_test.go new file mode 100644 index 0000000000..7b20c9826d --- /dev/null +++ b/operator/api/v1alpha1/common/common_test.go @@ -0,0 +1,304 @@ +package common + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestKeptnKeptnState_IsCompleted(t *testing.T) { + tests := []struct { + State KeptnState + Want bool + }{ + { + State: StateProgressing, + Want: false, + }, + { + State: StateFailed, + Want: true, + }, + { + State: StateSucceeded, + Want: true, + }, + { + State: StateCancelled, + Want: true, + }, + } + for _, tt := range tests { + t.Run("", func(t *testing.T) { + require.Equal(t, tt.State.IsCompleted(), tt.Want) + }) + } +} + +func TestKeptnKeptnState_IsSucceeded(t *testing.T) { + tests := []struct { + State KeptnState + Want bool + }{ + { + State: StateProgressing, + Want: false, + }, + { + State: StateSucceeded, + Want: true, + }, + } + for _, tt := range tests { + t.Run("", func(t *testing.T) { + require.Equal(t, tt.State.IsSucceeded(), tt.Want) + }) + } +} + +func TestKeptnKeptnState_IsFailed(t *testing.T) { + tests := []struct { + State KeptnState + Want bool + }{ + { + State: StateSucceeded, + Want: false, + }, + { + State: StateFailed, + Want: true, + }, + } + for _, tt := range tests { + t.Run("", func(t *testing.T) { + require.Equal(t, tt.State.IsFailed(), tt.Want) + }) + } +} + +func TestKeptnKeptnState_IsCancelled(t *testing.T) { + tests := []struct { + State KeptnState + Want bool + }{ + { + State: StateSucceeded, + Want: false, + }, + { + State: StateCancelled, + Want: true, + }, + } + for _, tt := range tests { + t.Run("", func(t *testing.T) { + require.Equal(t, tt.State.IsCancelled(), tt.Want) + }) + } +} + +func TestKeptnKeptnState_IsPending(t *testing.T) { + tests := []struct { + State KeptnState + Want bool + }{ + { + State: StateSucceeded, + Want: false, + }, + { + State: StatePending, + Want: true, + }, + } + for _, tt := range tests { + t.Run("", func(t *testing.T) { + require.Equal(t, tt.State.IsPending(), tt.Want) + }) + } +} + +func Test_UpdateStatusSummary(t *testing.T) { + emmptySummary := StatusSummary{0, 0, 0, 0, 0, 0, 0} + tests := []struct { + State KeptnState + Want StatusSummary + }{ + { + State: StateProgressing, + Want: StatusSummary{0, 1, 0, 0, 0, 0, 0}, + }, + { + State: StateFailed, + Want: StatusSummary{0, 0, 1, 0, 0, 0, 0}, + }, + { + State: StateSucceeded, + Want: StatusSummary{0, 0, 0, 1, 0, 0, 0}, + }, + { + State: StatePending, + Want: StatusSummary{0, 0, 0, 0, 1, 0, 0}, + }, + { + State: "", + Want: StatusSummary{0, 0, 0, 0, 1, 0, 0}, + }, + { + State: StateUnknown, + Want: StatusSummary{0, 0, 0, 0, 0, 1, 0}, + }, + { + State: StateCancelled, + Want: StatusSummary{0, 0, 0, 0, 0, 0, 1}, + }, + } + for _, tt := range tests { + t.Run("", func(t *testing.T) { + require.Equal(t, UpdateStatusSummary(tt.State, emmptySummary), tt.Want) + }) + } +} + +func Test_GetTotalCount(t *testing.T) { + summary := StatusSummary{2, 0, 2, 1, 0, 3, 5} + require.Equal(t, summary.GetTotalCount(), 11) +} + +func Test_GeOverallState(t *testing.T) { + tests := []struct { + Name string + Summary StatusSummary + Want KeptnState + }{ + { + Name: "failed", + Summary: StatusSummary{0, 0, 1, 0, 0, 0, 0}, + Want: StateFailed, + }, + { + Name: "cancelled", + Summary: StatusSummary{0, 0, 0, 0, 0, 0, 1}, + Want: StateFailed, + }, + { + Name: "progressing", + Summary: StatusSummary{0, 1, 0, 0, 0, 0, 0}, + Want: StateProgressing, + }, + { + Name: "pending", + Summary: StatusSummary{0, 0, 0, 0, 1, 0, 0}, + Want: StatePending, + }, + { + Name: "unknown", + Summary: StatusSummary{0, 0, 0, 0, 0, 1, 0}, + Want: StateUnknown, + }, + { + Name: "unknown totalcount", + Summary: StatusSummary{5, 0, 0, 0, 0, 1, 0}, + Want: StateUnknown, + }, + { + Name: "succeeded", + Summary: StatusSummary{1, 0, 0, 1, 0, 0, 0}, + Want: StateSucceeded, + }, + } + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + require.Equal(t, GetOverallState(tt.Summary), tt.Want) + }) + } +} + +func Test_TruncateString(t *testing.T) { + tests := []struct { + Input string + Max int + Want string + }{ + { + Input: "some_string", + Max: 20, + Want: "some_string", + }, + { + Input: "some_string", + Max: 5, + Want: "some_", + }, + { + Input: "", + Max: 5, + Want: "", + }, + } + for _, tt := range tests { + t.Run("", func(t *testing.T) { + require.Equal(t, TruncateString(tt.Input, tt.Max), tt.Want) + }) + } +} + +func Test_GenerateTaskName(t *testing.T) { + tests := []struct { + Check CheckType + Name string + Want string + }{ + { + Check: PreDeploymentCheckType, + Name: "short-name", + Want: "pre-short-name-", + }, + { + Check: PreDeploymentCheckType, + Name: "", + Want: "pre--", + }, + { + Check: PreDeploymentCheckType, + Name: "loooooooooooooooooooooooooooooooooooooong_name", + Want: "pre-looooooooooooooooooooooooooooooo-", + }, + } + for _, tt := range tests { + t.Run("", func(t *testing.T) { + require.True(t, strings.HasPrefix(GenerateTaskName(tt.Check, tt.Name), tt.Want)) + }) + } +} + +func Test_GenerateEvaluationName(t *testing.T) { + tests := []struct { + Check CheckType + Name string + Want string + }{ + { + Check: PreDeploymentEvaluationCheckType, + Name: "short-name", + Want: "pre-eval-short-name-", + }, + { + Check: PreDeploymentEvaluationCheckType, + Name: "", + Want: "pre-eval--", + }, + { + Check: PreDeploymentEvaluationCheckType, + Name: "loooooooooooooooooooooooooooooooooooooong_name", + Want: "pre-eval-loooooooooooooooooooooooooo-", + }, + } + for _, tt := range tests { + t.Run("", func(t *testing.T) { + require.True(t, strings.HasPrefix(GenerateEvaluationName(tt.Check, tt.Name), tt.Want)) + }) + } +} diff --git a/operator/api/v1alpha1/common/phases.go b/operator/api/v1alpha1/common/phases.go index f3f43bde5a..b453a93318 100644 --- a/operator/api/v1alpha1/common/phases.go +++ b/operator/api/v1alpha1/common/phases.go @@ -19,4 +19,5 @@ var ( PhaseAppPostEvaluation = KeptnPhaseType{LongName: "App Post-Deployment Evaluations", ShortName: "AppPostDeployEvaluations"} PhaseAppDeployment = KeptnPhaseType{LongName: "App Deployment", ShortName: "AppDeploy"} PhaseCompleted = KeptnPhaseType{LongName: "Completed", ShortName: "Completed"} + PhaseCancelled = KeptnPhaseType{LongName: "Cancelled", ShortName: "Cancelled"} ) diff --git a/operator/api/v1alpha1/keptnappversion_types.go b/operator/api/v1alpha1/keptnappversion_types.go index b7257ce0a2..16da556e68 100644 --- a/operator/api/v1alpha1/keptnappversion_types.go +++ b/operator/api/v1alpha1/keptnappversion_types.go @@ -359,3 +359,19 @@ func (a KeptnAppVersion) GetSpanKey(phase string) string { func (v KeptnAppVersion) GetWorkloadNameOfApp(workloadName string) string { return fmt.Sprintf("%s-%s", v.Spec.AppName, workloadName) } + +func (a *KeptnAppVersion) CancelRemainingPhases(phase common.KeptnPhaseType) { + // no need to cancel anything when post-eval tasks fail + if phase == common.PhaseAppPostEvaluation { + return + } + // cancel workload deployment and post-deployment tasks if app pre-eval failed + if phase == common.PhaseAppPreEvaluation { + a.Status.WorkloadOverallStatus = common.StateCancelled + } + // cancel post-deployment tasks if workload deployment failed + a.Status.PostDeploymentStatus = common.StateCancelled + a.Status.PostDeploymentEvaluationStatus = common.StateCancelled + a.Status.Status = common.StateFailed + +} diff --git a/operator/api/v1alpha1/keptnworkloadinstance_types.go b/operator/api/v1alpha1/keptnworkloadinstance_types.go index 2eaad509d7..0400e2bb8a 100644 --- a/operator/api/v1alpha1/keptnworkloadinstance_types.go +++ b/operator/api/v1alpha1/keptnworkloadinstance_types.go @@ -394,3 +394,20 @@ func (w KeptnWorkloadInstance) GetSpanName(phase string) string { func (w KeptnWorkloadInstance) SetSpanAttributes(span trace.Span) { span.SetAttributes(w.GetSpanAttributes()...) } + +func (w *KeptnWorkloadInstance) CancelRemainingPhases(phase common.KeptnPhaseType) { + // no need to cancel anything when post-eval tasks fail + if phase == common.PhaseWorkloadPostEvaluation { + return + } + //cancel everything if app pre-eval tasks have failed + if phase == common.PhaseAppPreEvaluation { + w.Status.PreDeploymentStatus = common.StateCancelled + w.Status.PreDeploymentEvaluationStatus = common.StateCancelled + } + //cancel deployment and post-deployment tasks if workload pre-eval tasks have failed + w.Status.DeploymentStatus = common.StateCancelled + w.Status.PostDeploymentStatus = common.StateCancelled + w.Status.PostDeploymentEvaluationStatus = common.StateCancelled + w.Status.Status = common.StateFailed +} diff --git a/operator/api/v1alpha1/tests/keptnappversion_test.go b/operator/api/v1alpha1/tests/keptnappversion_test.go index 687341e7b2..60a2384ba3 100644 --- a/operator/api/v1alpha1/tests/keptnappversion_test.go +++ b/operator/api/v1alpha1/tests/keptnappversion_test.go @@ -243,3 +243,73 @@ func TestKeptnAppVersion_GetWorkloadNameOfApp(t *testing.T) { }) } } + +func TestKeptnAppVersion_CancelRemainingPhases(t *testing.T) { + app := v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + PreDeploymentStatus: common.StatePending, + PreDeploymentEvaluationStatus: common.StatePending, + PostDeploymentStatus: common.StatePending, + PostDeploymentEvaluationStatus: common.StatePending, + WorkloadOverallStatus: common.StatePending, + Status: common.StatePending, + }, + } + + tests := []struct { + app v1alpha1.KeptnAppVersion + phase common.KeptnPhaseType + want v1alpha1.KeptnAppVersion + }{ + { + app: app, + phase: common.PhaseAppPostEvaluation, + want: v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + PreDeploymentStatus: common.StatePending, + PreDeploymentEvaluationStatus: common.StatePending, + PostDeploymentStatus: common.StatePending, + PostDeploymentEvaluationStatus: common.StatePending, + WorkloadOverallStatus: common.StatePending, + Status: common.StatePending, + }, + }, + }, + { + app: app, + phase: common.PhaseAppPreEvaluation, + want: v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + PreDeploymentStatus: common.StatePending, + PreDeploymentEvaluationStatus: common.StatePending, + PostDeploymentStatus: common.StateCancelled, + PostDeploymentEvaluationStatus: common.StateCancelled, + WorkloadOverallStatus: common.StateCancelled, + Status: common.StateFailed, + }, + }, + }, + { + app: app, + phase: common.PhaseWorkloadDeployment, + want: v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + PreDeploymentStatus: common.StatePending, + PreDeploymentEvaluationStatus: common.StatePending, + PostDeploymentStatus: common.StateCancelled, + PostDeploymentEvaluationStatus: common.StateCancelled, + WorkloadOverallStatus: common.StatePending, + Status: common.StateFailed, + }, + }, + }, + } + + for _, tt := range tests { + t.Run("", func(t *testing.T) { + tt.app.CancelRemainingPhases(tt.phase) + require.Equal(t, tt.want, tt.app) + }) + } + +} diff --git a/operator/api/v1alpha1/tests/keptnworkloadinstance_test.go b/operator/api/v1alpha1/tests/keptnworkloadinstance_test.go index 5152e3b6dd..8b0f36d425 100644 --- a/operator/api/v1alpha1/tests/keptnworkloadinstance_test.go +++ b/operator/api/v1alpha1/tests/keptnworkloadinstance_test.go @@ -214,3 +214,73 @@ func TestKeptnWorkloadInstance(t *testing.T) { common.WorkloadNamespace.String("namespace"), }, workload.GetSpanAttributes()) } + +func TestKeptnWorkloadInstance_CancelRemainingPhases(t *testing.T) { + workloadInstance := v1alpha1.KeptnWorkloadInstance{ + Status: v1alpha1.KeptnWorkloadInstanceStatus{ + PreDeploymentStatus: common.StatePending, + PreDeploymentEvaluationStatus: common.StatePending, + PostDeploymentStatus: common.StatePending, + PostDeploymentEvaluationStatus: common.StatePending, + DeploymentStatus: common.StatePending, + Status: common.StatePending, + }, + } + + tests := []struct { + workloadInstance v1alpha1.KeptnWorkloadInstance + phase common.KeptnPhaseType + want v1alpha1.KeptnWorkloadInstance + }{ + { + workloadInstance: workloadInstance, + phase: common.PhaseWorkloadPostEvaluation, + want: v1alpha1.KeptnWorkloadInstance{ + Status: v1alpha1.KeptnWorkloadInstanceStatus{ + PreDeploymentStatus: common.StatePending, + PreDeploymentEvaluationStatus: common.StatePending, + PostDeploymentStatus: common.StatePending, + PostDeploymentEvaluationStatus: common.StatePending, + DeploymentStatus: common.StatePending, + Status: common.StatePending, + }, + }, + }, + { + workloadInstance: workloadInstance, + phase: common.PhaseAppPreEvaluation, + want: v1alpha1.KeptnWorkloadInstance{ + Status: v1alpha1.KeptnWorkloadInstanceStatus{ + PreDeploymentStatus: common.StateCancelled, + PreDeploymentEvaluationStatus: common.StateCancelled, + PostDeploymentStatus: common.StateCancelled, + PostDeploymentEvaluationStatus: common.StateCancelled, + DeploymentStatus: common.StateCancelled, + Status: common.StateFailed, + }, + }, + }, + { + workloadInstance: workloadInstance, + phase: common.PhaseWorkloadPreEvaluation, + want: v1alpha1.KeptnWorkloadInstance{ + Status: v1alpha1.KeptnWorkloadInstanceStatus{ + PreDeploymentStatus: common.StatePending, + PreDeploymentEvaluationStatus: common.StatePending, + PostDeploymentStatus: common.StateCancelled, + PostDeploymentEvaluationStatus: common.StateCancelled, + DeploymentStatus: common.StateCancelled, + Status: common.StateFailed, + }, + }, + }, + } + + for _, tt := range tests { + t.Run("", func(t *testing.T) { + tt.workloadInstance.CancelRemainingPhases(tt.phase) + require.Equal(t, tt.want, tt.workloadInstance) + }) + } + +} diff --git a/operator/controllers/common/fake/phaseitem_mock.go b/operator/controllers/common/fake/phaseitem_mock.go index eb12bb52a5..fb1a4242ad 100644 --- a/operator/controllers/common/fake/phaseitem_mock.go +++ b/operator/controllers/common/fake/phaseitem_mock.go @@ -19,6 +19,9 @@ import ( // // // make and configure a mocked common.PhaseItem // mockedPhaseItem := &PhaseItemMock{ +// CancelRemainingPhasesFunc: func(phase apicommon.KeptnPhaseType) { +// panic("mock out the CancelRemainingPhases method") +// }, // CompleteFunc: func() { // panic("mock out the Complete method") // }, @@ -116,6 +119,9 @@ import ( // // } type PhaseItemMock struct { + // CancelRemainingPhasesFunc mocks the CancelRemainingPhases method. + CancelRemainingPhasesFunc func(phase apicommon.KeptnPhaseType) + // CompleteFunc mocks the Complete method. CompleteFunc func() @@ -208,6 +214,11 @@ type PhaseItemMock struct { // calls tracks calls to the methods. calls struct { + // CancelRemainingPhases holds details about calls to the CancelRemainingPhases method. + CancelRemainingPhases []struct { + // Phase is the phase argument value. + Phase apicommon.KeptnPhaseType + } // Complete holds details about calls to the Complete method. Complete []struct { } @@ -321,6 +332,7 @@ type PhaseItemMock struct { KeptnState apicommon.KeptnState } } + lockCancelRemainingPhases sync.RWMutex lockComplete sync.RWMutex lockGenerateEvaluation sync.RWMutex lockGenerateTask sync.RWMutex @@ -353,6 +365,37 @@ type PhaseItemMock struct { lockSetState sync.RWMutex } +// CancelRemainingPhases calls CancelRemainingPhasesFunc. +func (mock *PhaseItemMock) CancelRemainingPhases(phase apicommon.KeptnPhaseType) { + if mock.CancelRemainingPhasesFunc == nil { + panic("PhaseItemMock.CancelRemainingPhasesFunc: method is nil but PhaseItem.CancelRemainingPhases was just called") + } + callInfo := struct { + Phase apicommon.KeptnPhaseType + }{ + Phase: phase, + } + mock.lockCancelRemainingPhases.Lock() + mock.calls.CancelRemainingPhases = append(mock.calls.CancelRemainingPhases, callInfo) + mock.lockCancelRemainingPhases.Unlock() + mock.CancelRemainingPhasesFunc(phase) +} + +// CancelRemainingPhasesCalls gets all the calls that were made to CancelRemainingPhases. +// Check the length with: +// len(mockedPhaseItem.CancelRemainingPhasesCalls()) +func (mock *PhaseItemMock) CancelRemainingPhasesCalls() []struct { + Phase apicommon.KeptnPhaseType +} { + var calls []struct { + Phase apicommon.KeptnPhaseType + } + mock.lockCancelRemainingPhases.RLock() + calls = mock.calls.CancelRemainingPhases + mock.lockCancelRemainingPhases.RUnlock() + return calls +} + // Complete calls CompleteFunc. func (mock *PhaseItemMock) Complete() { if mock.CompleteFunc == nil { diff --git a/operator/controllers/common/interfaces.go b/operator/controllers/common/interfaces.go index 6527b0eff1..50b3ae903d 100644 --- a/operator/controllers/common/interfaces.go +++ b/operator/controllers/common/interfaces.go @@ -44,6 +44,7 @@ type PhaseItem interface { GetSpanKey(phase string) string GetSpanName(phase string) string SetSpanAttributes(span trace.Span) + CancelRemainingPhases(phase common.KeptnPhaseType) } type PhaseItemWrapper struct { @@ -178,6 +179,10 @@ func (pw PhaseItemWrapper) GetSpanName(phase string) string { return pw.Obj.GetSpanName(phase) } +func (pw PhaseItemWrapper) CancelRemainingPhases(phase common.KeptnPhaseType) { + pw.Obj.CancelRemainingPhases(phase) +} + func NewListItemWrapperFromClientObjectList(object client.ObjectList) (*ListItemWrapper, error) { pi, ok := object.(ListItem) if !ok { diff --git a/operator/controllers/common/interfaces_test.go b/operator/controllers/common/interfaces_test.go index ec9a6b0841..ba40518938 100644 --- a/operator/controllers/common/interfaces_test.go +++ b/operator/controllers/common/interfaces_test.go @@ -145,7 +145,8 @@ func TestPhaseItem(t *testing.T) { GenerateEvaluationFunc: func(traceContextCarrier propagation.MapCarrier, evaluationDefinition string, checkType common.CheckType) v1alpha1.KeptnEvaluation { return v1alpha1.KeptnEvaluation{} }, - SetSpanAttributesFunc: func(span trace.Span) {}, + SetSpanAttributesFunc: func(span trace.Span) {}, + CancelRemainingPhasesFunc: func(phase common.KeptnPhaseType) {}, } wrapper := PhaseItemWrapper{Obj: &phaseItemMock} @@ -240,4 +241,7 @@ func TestPhaseItem(t *testing.T) { wrapper.SetSpanAttributes(nil) require.Len(t, phaseItemMock.SetSpanAttributesCalls(), 1) + wrapper.CancelRemainingPhases(common.PhaseAppDeployment) + require.Len(t, phaseItemMock.CancelRemainingPhasesCalls(), 1) + } diff --git a/operator/controllers/common/phasehandler.go b/operator/controllers/common/phasehandler.go index aaf451e1b8..b3eb0c7423 100644 --- a/operator/controllers/common/phasehandler.go +++ b/operator/controllers/common/phasehandler.go @@ -30,7 +30,7 @@ func RecordEvent(recorder record.EventRecorder, phase common.KeptnPhaseType, eve recorder.Event(reconcileObject, eventType, fmt.Sprintf("%s%s", phase.ShortName, shortReason), fmt.Sprintf("%s %s / Namespace: %s, Name: %s, Version: %s ", phase.LongName, longReason, reconcileObject.GetNamespace(), reconcileObject.GetName(), version)) } -func (r PhaseHandler) HandlePhase(ctx context.Context, ctxAppTrace context.Context, tracer trace.Tracer, reconcileObject client.Object, phase common.KeptnPhaseType, span trace.Span, reconcilePhase func() (common.KeptnState, error)) (*PhaseResult, error) { +func (r PhaseHandler) HandlePhase(ctx context.Context, ctxTrace context.Context, tracer trace.Tracer, reconcileObject client.Object, phase common.KeptnPhaseType, span trace.Span, reconcilePhase func() (common.KeptnState, error)) (*PhaseResult, error) { requeueResult := ctrl.Result{Requeue: true, RequeueAfter: 5 * time.Second} piWrapper, err := NewPhaseItemWrapperFromClientObject(reconcileObject) if err != nil { @@ -38,17 +38,20 @@ func (r PhaseHandler) HandlePhase(ctx context.Context, ctxAppTrace context.Conte } oldStatus := piWrapper.GetState() oldPhase := piWrapper.GetCurrentPhase() + if oldStatus.IsCancelled() { + return &PhaseResult{Continue: false, Result: ctrl.Result{}}, nil + } piWrapper.SetCurrentPhase(phase.ShortName) r.Log.Info(phase.LongName + " not finished") - ctxAppTrace, spanAppTrace, err := r.SpanHandler.GetSpan(ctxAppTrace, tracer, reconcileObject, phase.ShortName) + ctxTrace, spanTrace, err := r.SpanHandler.GetSpan(ctxTrace, tracer, reconcileObject, phase.ShortName) if err != nil { r.Log.Error(err, "could not get span") } state, err := reconcilePhase() if err != nil { - spanAppTrace.AddEvent(phase.LongName + " could not get reconciled") + spanTrace.AddEvent(phase.LongName + " could not get reconciled") RecordEvent(r.Recorder, phase, "Warning", reconcileObject, "ReconcileErrored", "could not get reconciled", piWrapper.GetVersion()) span.SetStatus(codes.Error, err.Error()) return &PhaseResult{Continue: false, Result: requeueResult}, err @@ -61,7 +64,7 @@ func (r PhaseHandler) HandlePhase(ctx context.Context, ctxAppTrace context.Conte defer func(oldStatus common.KeptnState, oldPhase string, reconcileObject client.Object) { piWrapper, _ := NewPhaseItemWrapperFromClientObject(reconcileObject) if oldStatus != piWrapper.GetState() || oldPhase != piWrapper.GetCurrentPhase() { - ctx, spanAppTrace, err = r.SpanHandler.GetSpan(ctxAppTrace, tracer, reconcileObject, piWrapper.GetCurrentPhase()) + ctx, spanTrace, err = r.SpanHandler.GetSpan(ctxTrace, tracer, reconcileObject, piWrapper.GetCurrentPhase()) if err != nil { r.Log.Error(err, "could not get span") } @@ -75,20 +78,21 @@ func (r PhaseHandler) HandlePhase(ctx context.Context, ctxAppTrace context.Conte if state.IsFailed() { piWrapper.Complete() piWrapper.SetState(common.StateFailed) - spanAppTrace.AddEvent(phase.LongName + " has failed") - spanAppTrace.SetStatus(codes.Error, "Failed") - spanAppTrace.End() + spanTrace.AddEvent(phase.LongName + " has failed") + spanTrace.SetStatus(codes.Error, "Failed") + spanTrace.End() if err := r.SpanHandler.UnbindSpan(reconcileObject, phase.ShortName); err != nil { r.Log.Error(err, "cannot unbind span") } RecordEvent(r.Recorder, phase, "Warning", reconcileObject, "Failed", "has failed", piWrapper.GetVersion()) + piWrapper.CancelRemainingPhases(phase) return &PhaseResult{Continue: false, Result: ctrl.Result{}}, nil } piWrapper.SetState(common.StateSucceeded) - spanAppTrace.AddEvent(phase.LongName + " has succeeded") - spanAppTrace.SetStatus(codes.Ok, "Succeeded") - spanAppTrace.End() + spanTrace.AddEvent(phase.LongName + " has succeeded") + spanTrace.SetStatus(codes.Ok, "Succeeded") + spanTrace.End() if err := r.SpanHandler.UnbindSpan(reconcileObject, phase.ShortName); err != nil { r.Log.Error(err, "cannot unbind span") } diff --git a/operator/controllers/common/phasehandler_test.go b/operator/controllers/common/phasehandler_test.go new file mode 100644 index 0000000000..3c627e7af7 --- /dev/null +++ b/operator/controllers/common/phasehandler_test.go @@ -0,0 +1,227 @@ +package common + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1" + "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/common" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestPhaseHandler(t *testing.T) { + //phase, state, endtimeset + requeueResult := ctrl.Result{Requeue: true, RequeueAfter: 5 * time.Second} + tests := []struct { + name string + handler PhaseHandler + object *v1alpha1.KeptnAppVersion + phase common.KeptnPhaseType + reconcilePhase func() (common.KeptnState, error) + wantObject *v1alpha1.KeptnAppVersion + want *PhaseResult + wantErr error + endTimeSet bool + }{ + { + name: "cancelled", + handler: PhaseHandler{ + SpanHandler: &SpanHandler{}, + }, + object: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StateCancelled, + }, + }, + want: &PhaseResult{Continue: false, Result: ctrl.Result{}}, + wantErr: nil, + wantObject: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StateCancelled, + }, + }, + }, + { + name: "reconcilePhase error", + handler: PhaseHandler{ + SpanHandler: &SpanHandler{}, + Log: ctrl.Log.WithName("controller"), + Recorder: record.NewFakeRecorder(100), + Client: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(), + }, + object: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StatePending, + CurrentPhase: common.PhaseAppDeployment.LongName, + }, + }, + phase: common.PhaseAppDeployment, + reconcilePhase: func() (common.KeptnState, error) { + return "", fmt.Errorf("some err") + }, + want: &PhaseResult{Continue: false, Result: requeueResult}, + wantErr: fmt.Errorf("some err"), + wantObject: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StatePending, + CurrentPhase: common.PhaseAppDeployment.ShortName, + }, + }, + }, + { + name: "reconcilePhase pending state", + handler: PhaseHandler{ + SpanHandler: &SpanHandler{}, + Log: ctrl.Log.WithName("controller"), + Recorder: record.NewFakeRecorder(100), + Client: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(), + }, + object: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StatePending, + CurrentPhase: common.PhaseAppDeployment.LongName, + }, + }, + phase: common.PhaseAppDeployment, + reconcilePhase: func() (common.KeptnState, error) { + return common.StatePending, nil + }, + want: &PhaseResult{Continue: false, Result: requeueResult}, + wantErr: nil, + wantObject: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StateProgressing, + CurrentPhase: common.PhaseAppDeployment.ShortName, + }, + }, + }, + { + name: "reconcilePhase progressing state", + handler: PhaseHandler{ + SpanHandler: &SpanHandler{}, + Log: ctrl.Log.WithName("controller"), + Recorder: record.NewFakeRecorder(100), + Client: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(), + }, + object: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StatePending, + CurrentPhase: common.PhaseAppDeployment.LongName, + }, + }, + phase: common.PhaseAppDeployment, + reconcilePhase: func() (common.KeptnState, error) { + return common.StateProgressing, nil + }, + want: &PhaseResult{Continue: false, Result: requeueResult}, + wantErr: nil, + wantObject: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StateProgressing, + CurrentPhase: common.PhaseAppDeployment.ShortName, + }, + }, + }, + { + name: "reconcilePhase succeeded state", + handler: PhaseHandler{ + SpanHandler: &SpanHandler{}, + Log: ctrl.Log.WithName("controller"), + Recorder: record.NewFakeRecorder(100), + Client: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(), + }, + object: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StatePending, + CurrentPhase: common.PhaseAppDeployment.LongName, + }, + }, + phase: common.PhaseAppDeployment, + reconcilePhase: func() (common.KeptnState, error) { + return common.StateSucceeded, nil + }, + want: &PhaseResult{Continue: true, Result: requeueResult}, + wantErr: nil, + wantObject: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StateSucceeded, + CurrentPhase: common.PhaseAppDeployment.ShortName, + }, + }, + }, + { + name: "reconcilePhase failed state", + handler: PhaseHandler{ + SpanHandler: &SpanHandler{}, + Log: ctrl.Log.WithName("controller"), + Recorder: record.NewFakeRecorder(100), + Client: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(), + }, + object: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StateProgressing, + CurrentPhase: common.PhaseAppPreEvaluation.LongName, + }, + }, + phase: common.PhaseAppPreEvaluation, + reconcilePhase: func() (common.KeptnState, error) { + return common.StateFailed, nil + }, + want: &PhaseResult{Continue: false, Result: ctrl.Result{}}, + wantErr: nil, + wantObject: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StateFailed, + CurrentPhase: common.PhaseAppPreEvaluation.ShortName, + EndTime: v1.Time{Time: time.Now().UTC()}, + }, + }, + }, + { + name: "reconcilePhase unknown state", + handler: PhaseHandler{ + SpanHandler: &SpanHandler{}, + Log: ctrl.Log.WithName("controller"), + Recorder: record.NewFakeRecorder(100), + Client: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(), + }, + object: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StateProgressing, + CurrentPhase: common.PhaseAppPreEvaluation.LongName, + }, + }, + phase: common.PhaseAppPreEvaluation, + reconcilePhase: func() (common.KeptnState, error) { + return common.StateUnknown, nil + }, + want: &PhaseResult{Continue: false, Result: requeueResult}, + wantErr: nil, + wantObject: &v1alpha1.KeptnAppVersion{ + Status: v1alpha1.KeptnAppVersionStatus{ + Status: common.StateProgressing, + CurrentPhase: common.PhaseAppPreEvaluation.ShortName, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := tt.handler.HandlePhase(context.TODO(), context.TODO(), trace.NewNoopTracerProvider().Tracer("tracer"), tt.object, tt.phase, trace.SpanFromContext(context.TODO()), tt.reconcilePhase) + require.Equal(t, tt.want, result) + require.Equal(t, tt.wantErr, err) + require.Equal(t, tt.wantObject.Status.Status, tt.object.Status.Status) + require.Equal(t, tt.wantObject.Status.CurrentPhase, tt.object.Status.CurrentPhase) + require.Equal(t, tt.wantObject.Status.Status.IsFailed(), tt.object.IsEndTimeSet()) + }) + } +} diff --git a/operator/test/component/workloadinstancecontroller_test.go b/operator/test/component/workloadinstancecontroller_test.go index f64e400bef..df54836e63 100644 --- a/operator/test/component/workloadinstancecontroller_test.go +++ b/operator/test/component/workloadinstancecontroller_test.go @@ -2,6 +2,8 @@ package component import ( "context" + "time" + klcv1alpha1 "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1" "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/common" keptncontroller "github.com/keptn/lifecycle-toolkit/operator/controllers/common" @@ -290,6 +292,101 @@ var _ = Describe("KeptnWorkloadInstanceController", Ordered, func() { g.Expect(wi.Status.DeploymentStatus).To(Equal(common.StateSucceeded)) }, "20s").Should(Succeed()) }) + It("should be cancelled when pre-eval checks failed", func() { + evaluation := &klcv1alpha1.KeptnEvaluation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pre-eval-eval-def", + Namespace: namespace, + }, + Spec: klcv1alpha1.KeptnEvaluationSpec{ + EvaluationDefinition: "eval-def", + Workload: "test-app-wname", + WorkloadVersion: "2.0", + Type: common.PreDeploymentEvaluationCheckType, + Retries: 10, + }, + } + + defer func() { + _ = k8sClient.Delete(ctx, evaluation) + }() + + By("Creating Evaluation") + err := k8sClient.Create(context.TODO(), evaluation) + Expect(err).To(BeNil()) + + evaluation.Status = klcv1alpha1.KeptnEvaluationStatus{ + OverallStatus: common.StateFailed, + RetryCount: 10, + EvaluationStatus: map[string]klcv1alpha1.EvaluationStatusItem{ + "something": { + Status: common.StateFailed, + Value: "10", + }, + }, + StartTime: metav1.Time{Time: time.Now().UTC()}, + EndTime: metav1.Time{Time: time.Now().UTC().Add(5 * time.Second)}, + } + + err = k8sClient.Status().Update(ctx, evaluation) + Expect(err).To(BeNil()) + + wi = &klcv1alpha1.KeptnWorkloadInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-app-wname-2.0", + Namespace: namespace, + }, + Spec: klcv1alpha1.KeptnWorkloadInstanceSpec{ + KeptnWorkloadSpec: klcv1alpha1.KeptnWorkloadSpec{ + Version: "2.0", + AppName: appVersion.GetAppName(), + PreDeploymentEvaluations: []string{"eval-def"}, + }, + WorkloadName: "test-app-wname", + }, + } + By("Creating WorkloadInstance") + err = k8sClient.Create(context.TODO(), wi) + Expect(err).To(BeNil()) + + wi.Status = klcv1alpha1.KeptnWorkloadInstanceStatus{ + PreDeploymentStatus: common.StateSucceeded, + PreDeploymentEvaluationStatus: common.StateProgressing, + DeploymentStatus: common.StatePending, + PostDeploymentStatus: common.StatePending, + PostDeploymentEvaluationStatus: common.StatePending, + CurrentPhase: common.PhaseWorkloadPreEvaluation.ShortName, + Status: common.StateProgressing, + PreDeploymentEvaluationTaskStatus: []klcv1alpha1.EvaluationStatus{ + { + EvaluationName: "pre-eval-eval-def", + Status: common.StateProgressing, + EvaluationDefinitionName: "eval-def", + }, + }, + } + + err = k8sClient.Status().Update(ctx, wi) + Expect(err).To(BeNil()) + + By("Ensuring all phases after pre-eval checks are cancelled") + wiNameObj := types.NamespacedName{ + Namespace: wi.Namespace, + Name: wi.Name, + } + Eventually(func(g Gomega) { + wi := &klcv1alpha1.KeptnWorkloadInstance{} + err := k8sClient.Get(ctx, wiNameObj, wi) + g.Expect(err).To(BeNil()) + g.Expect(wi).To(Not(BeNil())) + g.Expect(wi.Status.PreDeploymentStatus).To(BeEquivalentTo(common.StateSucceeded)) + g.Expect(wi.Status.PreDeploymentEvaluationStatus).To(BeEquivalentTo(common.StateFailed)) + g.Expect(wi.Status.DeploymentStatus).To(BeEquivalentTo(common.StateCancelled)) + g.Expect(wi.Status.PostDeploymentStatus).To(BeEquivalentTo(common.StateCancelled)) + g.Expect(wi.Status.PostDeploymentEvaluationStatus).To(BeEquivalentTo(common.StateCancelled)) + g.Expect(wi.Status.Status).To(BeEquivalentTo(common.StateFailed)) + }, "30s").Should(Succeed()) + }) AfterEach(func() { // Remember to clean up the cluster after each test k8sClient.Delete(ctx, appVersion) @@ -325,5 +422,7 @@ func createAppVersionInCluster(name string, namespace string, version string) *k By("Invoking Reconciling for Create") Expect(ignoreAlreadyExists(k8sClient.Create(ctx, instance))).Should(Succeed()) + instance.Status.PreDeploymentEvaluationStatus = common.StateSucceeded + _ = k8sClient.Status().Update(ctx, instance) return instance }