chore: fix linter issues

.golangci.yml

@@ -1,21 +1,33 @@
 run:
   timeout: 5m
   go: '1.19'
+  tests: false
+  skip-dirs:
+    - test
 linters:
   enable:
-    - gofmt         # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification
-    - gci           # Gci controls golang package import order and makes it always deterministic.
-    - errorlint       # errorlint is a linter that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13.
-    - containedctx  # containedctx is a linter that detects struct contained context.Context field
-    - dogsled       # Checks assignments with too many blank identifiers (e.g. x, , , _, := f())
-    - nilnil        # Checks that there is no simultaneous return of nil error and an invalid value.
-    - noctx         # noctx finds sending http request without context.Context
+#    - gofmt         # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification
+#    - gci           # Gci controls golang package import order and makes it always deterministic.
+#    - errorlint       # errorlint is a linter that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13.
+#    - containedctx  # containedctx is a linter that detects struct contained context.Context field
+#    - dogsled       # Checks assignments with too many blank identifiers (e.g. x, , , _, := f())
+#    - nilnil        # Checks that there is no simultaneous return of nil error and an invalid value.
+#    - noctx         # noctx finds sending http request without context.Context
+#    - gocyclo       # measure cyclomatic complexity
+    - gocognit      # measure cognitive complexity
+#    - funlen        # limit function length
 
 issues:
   exclude-rules:
     - linters:
         - gci
       text: '//+kubebuilder'
-    - linters:
-        - containedctx
-      path: _test\.go
+
+linters-settings:
+  gocyclo:
+    min-complexity: 10
+  gocognit:
+    min-complexity: 20
+  funlen:
+    lines: 120
+    statements: 120

operator/controllers/common/evaluationhandler.go

@@ -41,36 +41,35 @@ func (r EvaluationHandler) ReconcileEvaluations(ctx context.Context, phaseCtx co
 		return nil, apicommon.StatusSummary{}, err
 	}
 
-	var evaluations []string
-	var statuses []klcv1alpha1.EvaluationStatus
-
-	switch evaluationCreateAttributes.CheckType {
-	case apicommon.PreDeploymentEvaluationCheckType:
-		evaluations = piWrapper.GetPreDeploymentEvaluations()
-		statuses = piWrapper.GetPreDeploymentEvaluationTaskStatus()
-	case apicommon.PostDeploymentEvaluationCheckType:
-		evaluations = piWrapper.GetPostDeploymentEvaluations()
-		statuses = piWrapper.GetPostDeploymentEvaluationTaskStatus()
-	}
+	evaluations, statuses := r.setupEvaluations(evaluationCreateAttributes, piWrapper)
 
 	var summary apicommon.StatusSummary
 	summary.Total = len(evaluations)
 	// Check current state of the PrePostEvaluationTasks
+	newStatus, evaluationStatuses, statusSummary, err2 := r.handlePrePostEvaluations(ctx, phaseCtx, reconcileObject, evaluationCreateAttributes, evaluations, statuses, apicommon.PhaseReconcileEvaluation, piWrapper, summary)
+	if err2 != nil {
+		return evaluationStatuses, statusSummary, err2
+	}
+
+	for _, ns := range newStatus {
+		summary = apicommon.UpdateStatusSummary(ns.Status, summary)
+	}
+	if apicommon.GetOverallState(summary) != apicommon.StateSucceeded {
+		RecordEvent(r.Recorder, apicommon.PhaseReconcileEvaluation, "Warning", reconcileObject, "NotFinished", "has not finished", piWrapper.GetVersion())
+	}
+	return newStatus, summary, nil
+}
+
+func (r EvaluationHandler) handlePrePostEvaluations(ctx context.Context, phaseCtx context.Context, reconcileObject client.Object, evaluationCreateAttributes EvaluationCreateAttributes, evaluations []string, statuses []klcv1alpha1.EvaluationStatus, phase apicommon.KeptnPhaseType, piWrapper *interfaces.PhaseItemWrapper, summary apicommon.StatusSummary) ([]klcv1alpha1.EvaluationStatus, []klcv1alpha1.EvaluationStatus, apicommon.StatusSummary, error) {
 	var newStatus []klcv1alpha1.EvaluationStatus
 	for _, evaluationName := range evaluations {
-		var oldstatus apicommon.KeptnState
-		for _, ts := range statuses {
-			if ts.EvaluationDefinitionName == evaluationName {
-				oldstatus = ts.Status
-			}
-		}
+		oldStatus := r.findOldEvaluationStatus(statuses, evaluationName)
 
 		evaluationStatus := GetEvaluationStatus(evaluationName, statuses)
 		evaluation := &klcv1alpha1.KeptnEvaluation{}
-		evaluationExists := false
 
-		if oldstatus != evaluationStatus.Status {
-			RecordEvent(r.Recorder, apicommon.PhaseReconcileEvaluation, "Normal", reconcileObject, "EvaluationStatusChanged", fmt.Sprintf("evaluation status changed from %s to %s", oldstatus, evaluationStatus.Status), piWrapper.GetVersion())
+		if oldStatus != evaluationStatus.Status {
+			RecordEvent(r.Recorder, phase, "Normal", reconcileObject, "EvaluationStatusChanged", fmt.Sprintf("evaluation status changed from %s to %s", oldStatus, evaluationStatus.Status), piWrapper.GetVersion())
 		}
 
 		// Check if evaluation has already succeeded or failed
@@ -80,28 +79,16 @@ func (r EvaluationHandler) ReconcileEvaluations(ctx context.Context, phaseCtx co
 		}
 
 		// Check if Evaluation is already created
-		if evaluationStatus.EvaluationName != "" {
-			err := r.Client.Get(ctx, types.NamespacedName{Name: evaluationStatus.EvaluationName, Namespace: piWrapper.GetNamespace()}, evaluation)
-			if err != nil && errors.IsNotFound(err) {
-				evaluationStatus.EvaluationName = ""
-			} else if err != nil {
-				return nil, summary, err
-			}
-			evaluationExists = true
+		evaluationExists, evaluationStatuses, i, err := r.checkAlreadyCreated(ctx, evaluationStatus, piWrapper, evaluation, &summary)
+		if err != nil {
+			return evaluationStatuses, i, summary, err
 		}
 
 		// Create new Evaluation if it does not exist
 		if !evaluationExists {
-			evaluationCreateAttributes.EvaluationDefinition = evaluationName
-			evaluationName, err := r.CreateKeptnEvaluation(ctx, piWrapper.GetNamespace(), reconcileObject, evaluationCreateAttributes)
-			if err != nil {
-				return nil, summary, err
-			}
-			evaluationStatus.EvaluationName = evaluationName
-			evaluationStatus.SetStartTime()
-			_, _, err = r.SpanHandler.GetSpan(phaseCtx, r.Tracer, evaluation, "")
+			statusSummary, err := r.createEvaluation(ctx, phaseCtx, reconcileObject, evaluationCreateAttributes, evaluationName, piWrapper, summary, evaluationStatus, evaluation)
 			if err != nil {
-				r.Log.Error(err, "could not get span")
+				return nil, nil, statusSummary, err
 			}
 		} else {
 			_, spanEvaluationTrace, err := r.SpanHandler.GetSpan(phaseCtx, r.Tracer, evaluation, "")
@@ -110,34 +97,83 @@ func (r EvaluationHandler) ReconcileEvaluations(ctx context.Context, phaseCtx co
 			}
 			// Update state of Evaluation if it is already created
 			evaluationStatus.Status = evaluation.Status.OverallStatus
-			if evaluationStatus.Status.IsCompleted() {
-				if evaluationStatus.Status.IsSucceeded() {
-					spanEvaluationTrace.AddEvent(evaluation.Name + " has finished")
-					spanEvaluationTrace.SetStatus(codes.Ok, "Finished")
-					RecordEvent(r.Recorder, apicommon.PhaseReconcileEvaluation, "Normal", evaluation, "Succeeded", "evaluation succeeded", piWrapper.GetVersion())
-				} else {
-					spanEvaluationTrace.AddEvent(evaluation.Name + " has failed")
-					r.emitEvaluationFailureEvents(evaluation, spanEvaluationTrace, piWrapper)
-					spanEvaluationTrace.SetStatus(codes.Error, "Failed")
-				}
-				spanEvaluationTrace.End()
-				if err := r.SpanHandler.UnbindSpan(evaluation, ""); err != nil {
-					r.Log.Error(err, controllererrors.ErrCouldNotUnbindSpan, evaluation.Name)
-				}
-				evaluationStatus.SetEndTime()
-			}
+			r.updateEvaluation(evaluationStatus, spanEvaluationTrace, evaluation)
 		}
 		// Update state of the Check
 		newStatus = append(newStatus, evaluationStatus)
 	}
+	return newStatus, nil, apicommon.StatusSummary{}, nil
+}
 
-	for _, ns := range newStatus {
-		summary = apicommon.UpdateStatusSummary(ns.Status, summary)
+func (r EvaluationHandler) updateEvaluation(evaluationStatus klcv1alpha1.EvaluationStatus, spanEvaluationTrace trace.Span, evaluation *klcv1alpha1.KeptnEvaluation) {
+	if evaluationStatus.Status.IsCompleted() {
+		if evaluationStatus.Status.IsSucceeded() {
+			spanEvaluationTrace.AddEvent(evaluation.Name + " has finished")
+			spanEvaluationTrace.SetStatus(codes.Ok, "Finished")
+		} else {
+			spanEvaluationTrace.AddEvent(evaluation.Name + " has failed")
+			spanEvaluationTrace.SetStatus(codes.Error, "Failed")
+		}
+		spanEvaluationTrace.End()
+		if err := r.SpanHandler.UnbindSpan(evaluation, ""); err != nil {
+			r.Log.Error(err, controllererrors.ErrCouldNotUnbindSpan, evaluation.Name)
+		}
+		evaluationStatus.SetEndTime()
 	}
-	if apicommon.GetOverallState(summary) != apicommon.StateSucceeded {
-		RecordEvent(r.Recorder, apicommon.PhaseReconcileEvaluation, "Warning", reconcileObject, "NotFinished", "has not finished", piWrapper.GetVersion())
+}
+
+func (r EvaluationHandler) createEvaluation(ctx context.Context, phaseCtx context.Context, reconcileObject client.Object, evaluationCreateAttributes EvaluationCreateAttributes, evaluationName string, piWrapper *interfaces.PhaseItemWrapper, summary apicommon.StatusSummary, evaluationStatus klcv1alpha1.EvaluationStatus, evaluation *klcv1alpha1.KeptnEvaluation) (apicommon.StatusSummary, error) {
+	evaluationCreateAttributes.EvaluationDefinition = evaluationName
+	evaluationName, err := r.CreateKeptnEvaluation(ctx, piWrapper.GetNamespace(), reconcileObject, evaluationCreateAttributes)
+	if err != nil {
+		return summary, err
 	}
-	return newStatus, summary, nil
+	evaluationStatus.EvaluationName = evaluationName
+	evaluationStatus.SetStartTime()
+	_, _, err = r.SpanHandler.GetSpan(phaseCtx, r.Tracer, evaluation, "")
+	if err != nil {
+		r.Log.Error(err, "could not get span")
+	}
+	return apicommon.StatusSummary{}, nil
+}
+
+func (r EvaluationHandler) checkAlreadyCreated(ctx context.Context, evaluationStatus klcv1alpha1.EvaluationStatus, piWrapper *interfaces.PhaseItemWrapper, evaluation *klcv1alpha1.KeptnEvaluation, summary *apicommon.StatusSummary) (bool, []klcv1alpha1.EvaluationStatus, []klcv1alpha1.EvaluationStatus, error) {
+	evaluationExists := false
+	if evaluationStatus.EvaluationName != "" {
+		err := r.Client.Get(ctx, types.NamespacedName{Name: evaluationStatus.EvaluationName, Namespace: piWrapper.GetNamespace()}, evaluation)
+		if err != nil && errors.IsNotFound(err) {
+			evaluationStatus.EvaluationName = ""
+		} else if err != nil {
+			return false, nil, nil, err
+		}
+		evaluationExists = true
+	}
+	return evaluationExists, nil, nil, nil
+}
+
+func (r EvaluationHandler) findOldEvaluationStatus(statuses []klcv1alpha1.EvaluationStatus, evaluationName string) apicommon.KeptnState {
+	var oldstatus apicommon.KeptnState
+	for _, ts := range statuses {
+		if ts.EvaluationDefinitionName == evaluationName {
+			oldstatus = ts.Status
+		}
+	}
+	return oldstatus
+}
+
+func (r EvaluationHandler) setupEvaluations(evaluationCreateAttributes EvaluationCreateAttributes, piWrapper *interfaces.PhaseItemWrapper) ([]string, []klcv1alpha1.EvaluationStatus) {
+	var evaluations []string
+	var statuses []klcv1alpha1.EvaluationStatus
+
+	switch evaluationCreateAttributes.CheckType {
+	case apicommon.PreDeploymentEvaluationCheckType:
+		evaluations = piWrapper.GetPreDeploymentEvaluations()
+		statuses = piWrapper.GetPreDeploymentEvaluationTaskStatus()
+	case apicommon.PostDeploymentEvaluationCheckType:
+		evaluations = piWrapper.GetPostDeploymentEvaluations()
+		statuses = piWrapper.GetPostDeploymentEvaluationTaskStatus()
+	}
+	return evaluations, statuses
 }
 
 func (r EvaluationHandler) CreateKeptnEvaluation(ctx context.Context, namespace string, reconcileObject client.Object, evaluationCreateAttributes EvaluationCreateAttributes) (string, error) {

operator/controllers/common/phasehandler.go

@@ -73,34 +73,38 @@ func (r PhaseHandler) HandlePhase(ctx context.Context, ctxTrace context.Context,
 	}(ctx, oldStatus, oldPhase, reconcileObject)
 
 	if state.IsCompleted() {
-		if state.IsFailed() {
-			piWrapper.Complete()
-			piWrapper.SetState(apicommon.StateFailed)
-			spanPhaseTrace.AddEvent(phase.LongName + " has failed")
-			spanPhaseTrace.SetStatus(codes.Error, "Failed")
-			spanPhaseTrace.End()
-			if err := r.SpanHandler.UnbindSpan(reconcileObject, phase.ShortName); err != nil {
-				r.Log.Error(err, controllererrors.ErrCouldNotUnbindSpan, reconcileObject.GetName())
-			}
-			RecordEvent(r.Recorder, phase, "Warning", reconcileObject, "Failed", "has failed", piWrapper.GetVersion())
-			piWrapper.CancelRemainingPhases(phase)
-			return &PhaseResult{Continue: false, Result: ctrl.Result{}}, nil
-		}
+		return r.handleCompletedState(state, piWrapper, spanPhaseTrace, phase, reconcileObject, requeueResult)
+	}
+
+	piWrapper.SetState(apicommon.StateProgressing)
+	RecordEvent(r.Recorder, phase, "Warning", reconcileObject, "NotFinished", "has not finished", piWrapper.GetVersion())
 
-		piWrapper.SetState(apicommon.StateSucceeded)
-		spanPhaseTrace.AddEvent(phase.LongName + " has succeeded")
-		spanPhaseTrace.SetStatus(codes.Ok, "Succeeded")
+	return &PhaseResult{Continue: false, Result: requeueResult}, nil
+}
+
+func (r PhaseHandler) handleCompletedState(state apicommon.KeptnState, piWrapper *interfaces.PhaseItemWrapper, spanPhaseTrace trace.Span, phase apicommon.KeptnPhaseType, reconcileObject client.Object, requeueResult ctrl.Result) (*PhaseResult, error) {
+	if state.IsFailed() {
+		piWrapper.Complete()
+		piWrapper.SetState(apicommon.StateFailed)
+		spanPhaseTrace.AddEvent(phase.LongName + " has failed")
+		spanPhaseTrace.SetStatus(codes.Error, "Failed")
 		spanPhaseTrace.End()
 		if err := r.SpanHandler.UnbindSpan(reconcileObject, phase.ShortName); err != nil {
 			r.Log.Error(err, controllererrors.ErrCouldNotUnbindSpan, reconcileObject.GetName())
 		}
-		RecordEvent(r.Recorder, phase, "Normal", reconcileObject, "Succeeded", "has succeeded", piWrapper.GetVersion())
-
-		return &PhaseResult{Continue: true, Result: requeueResult}, nil
+		RecordEvent(r.Recorder, phase, "Warning", reconcileObject, "Failed", "has failed", piWrapper.GetVersion())
+		piWrapper.CancelRemainingPhases(phase)
+		return &PhaseResult{Continue: false, Result: ctrl.Result{}}, nil
 	}
 
-	piWrapper.SetState(apicommon.StateProgressing)
-	RecordEvent(r.Recorder, phase, "Warning", reconcileObject, "NotFinished", "has not finished", piWrapper.GetVersion())
+	piWrapper.SetState(apicommon.StateSucceeded)
+	spanPhaseTrace.AddEvent(phase.LongName + " has succeeded")
+	spanPhaseTrace.SetStatus(codes.Ok, "Succeeded")
+	spanPhaseTrace.End()
+	if err := r.SpanHandler.UnbindSpan(reconcileObject, phase.ShortName); err != nil {
+		r.Log.Error(err, controllererrors.ErrCouldNotUnbindSpan, reconcileObject.GetName())
+	}
+	RecordEvent(r.Recorder, phase, "Normal", reconcileObject, "Succeeded", "has succeeded", piWrapper.GetVersion())
 
-	return &PhaseResult{Continue: false, Result: requeueResult}, nil
+	return &PhaseResult{Continue: true, Result: requeueResult}, nil
 }