fix(metrics-operator): introduce .status.state in Analysis (#2061)

Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>

.github/scripts/.helm-tests/default/result.yaml

@@ -162,6 +162,9 @@ spec:
     - jsonPath: .spec.analysisDefinition.name
       name: AnalysisDefinition
       type: string
+    - jsonPath: .status.state
+      name: State
+      type: string
     - jsonPath: .status.warning
       name: Warning
       type: string
@@ -241,6 +244,9 @@ spec:
               raw:
                 description: Raw contains the raw result of the SLO computation
                 type: string
+              state:
+                description: State describes the current state of the Analysis (Pending/Progressing/Completed)
+                type: string
               storedValues:
                 additionalProperties:
                   description: ProviderResult stores reference of already collected
@@ -273,6 +279,8 @@ spec:
               warning:
                 description: Warning returns whether the analysis returned a warning
                 type: boolean
+            required:
+            - state
             type: object
         type: object
     served: true

docs/content/en/docs/crd-ref/metrics/v1alpha3/_index.md

@@ -125,6 +125,17 @@ _Appears in:_
 | `analysisDefinition` _[ObjectReference](#objectreference)_ | AnalysisDefinition refers to the AnalysisDefinition, a CRD that stores the AnalysisValuesTemplates |
 
 
+#### AnalysisState
+
+_Underlying type:_ `string`
+
+AnalysisState represents the state of the analysis
+
+_Appears in:_
+- [AnalysisStatus](#analysisstatus)
+
+
+
 #### AnalysisStatus
 
 
@@ -139,6 +150,7 @@ _Appears in:_
 | `raw` _string_ | Raw contains the raw result of the SLO computation |
 | `pass` _boolean_ | Pass returns whether the SLO is satisfied |
 | `warning` _boolean_ | Warning returns whether the analysis returned a warning |
+| `state` _[AnalysisState](#analysisstate)_ | State describes the current state of the Analysis (Pending/Progressing/Completed) |
 | `storedValues` _object (keys:string, values:[ProviderResult](#providerresult))_ | StoredValues contains all analysis values that have already been retrieved successfully |
 
 

helm/chart/templates/analysis-crd.yaml

@@ -23,6 +23,9 @@ spec:
     - jsonPath: .spec.analysisDefinition.name
       name: AnalysisDefinition
       type: string
+    - jsonPath: .status.state
+      name: State
+      type: string
     - jsonPath: .status.warning
       name: Warning
       type: string
@@ -102,6 +105,9 @@ spec:
               raw:
                 description: Raw contains the raw result of the SLO computation
                 type: string
+              state:
+                description: State describes the current state of the Analysis (Pending/Progressing/Completed)
+                type: string
               storedValues:
                 additionalProperties:
                   description: ProviderResult stores reference of already collected
@@ -134,6 +140,8 @@ spec:
               warning:
                 description: Warning returns whether the analysis returned a warning
                 type: boolean
+            required:
+            - state
             type: object
         type: object
     served: true

metrics-operator/api/v1alpha3/analysis_types.go

@@ -48,13 +48,16 @@ type AnalysisStatus struct {
 	Pass bool `json:"pass,omitempty"`
 	// Warning returns whether the analysis returned a warning
 	Warning bool `json:"warning,omitempty"`
+	// State describes the current state of the Analysis (Pending/Progressing/Completed)
+	State AnalysisState `json:"state"`
 	// StoredValues contains all analysis values that have already been retrieved successfully
 	StoredValues map[string]ProviderResult `json:"storedValues,omitempty"`
 }
 
 //+kubebuilder:object:root=true
 //+kubebuilder:subresource:status
 //+kubebuilder:printcolumn:name="AnalysisDefinition",type=string,JSONPath=.spec.analysisDefinition.name
+//+kubebuilder:printcolumn:name="State",type=string,JSONPath=`.status.state`
 //+kubebuilder:printcolumn:name="Warning",type=string,JSONPath=`.status.warning`
 //+kubebuilder:printcolumn:name="Pass",type=string,JSONPath=`.status.pass`
 

metrics-operator/api/v1alpha3/common.go

@@ -18,3 +18,20 @@ func (o *ObjectReference) GetNamespace(defaultNamespace string) string {
 
 	return defaultNamespace
 }
+
+// AnalysisState represents the state of the analysis
+type AnalysisState string
+
+const (
+	StatePending     AnalysisState = "Pending"
+	StateProgressing AnalysisState = "Progressing"
+	StateCompleted   AnalysisState = "Completed"
+)
+
+func (s AnalysisState) IsPending() bool {
+	return s == StatePending || s == ""
+}
+
+func (s AnalysisState) IsCompleted() bool {
+	return s == StateCompleted
+}

metrics-operator/api/v1alpha3/common_test.go

@@ -25,3 +25,22 @@ func TestObjectReference_GetNamespace(t *testing.T) {
 
 	require.Equal(t, "ns", o.GetNamespace("default"))
 }
+
+func TestAnalysisState_IsPending(t *testing.T) {
+	a := StatePending
+	require.True(t, a.IsPending())
+
+	a = ""
+	require.True(t, a.IsPending())
+
+	a = StateCompleted
+	require.False(t, a.IsPending())
+}
+
+func TestAnalysisState_IsCompleted(t *testing.T) {
+	a := StateCompleted
+	require.True(t, a.IsCompleted())
+
+	a = StateProgressing
+	require.False(t, a.IsCompleted())
+}

metrics-operator/config/crd/bases/metrics.keptn.sh_analyses.yaml

@@ -18,6 +18,9 @@ spec:
     - jsonPath: .spec.analysisDefinition.name
       name: AnalysisDefinition
       type: string
+    - jsonPath: .status.state
+      name: State
+      type: string
     - jsonPath: .status.warning
       name: Warning
       type: string
@@ -97,6 +100,9 @@ spec:
               raw:
                 description: Raw contains the raw result of the SLO computation
                 type: string
+              state:
+                description: State describes the current state of the Analysis (Pending/Progressing/Completed)
+                type: string
               storedValues:
                 additionalProperties:
                   description: ProviderResult stores reference of already collected
@@ -129,6 +135,8 @@ spec:
               warning:
                 description: Warning returns whether the analysis returned a warning
                 type: boolean
+            required:
+            - state
             type: object
         type: object
     served: true

metrics-operator/controllers/analysis/controller.go

@@ -23,6 +23,7 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
+	"github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1alpha3"
 	metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1alpha3"
 	common "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common/analysis"
 	"golang.org/x/exp/maps"
@@ -71,75 +72,60 @@ func (a *AnalysisReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		return ctrl.Result{}, err
 	}
 
-	//find AnalysisDefinition to have the collection of Objectives
-	analysisDefNamespace := analysis.Spec.AnalysisDefinition.GetNamespace(analysis.Namespace)
-	analysisDef := &metricsapi.AnalysisDefinition{}
-	err := a.Client.Get(ctx,
-		types.NamespacedName{
-			Name:      analysis.Spec.AnalysisDefinition.Name,
-			Namespace: analysisDefNamespace},
-		analysisDef,
-	)
+	if analysis.Status.State.IsCompleted() {
+		return ctrl.Result{}, nil
+	}
 
+	//find AnalysisDefinition to have the collection of Objectives
+	analysisDef, err := a.retrieveAnalysisDefinition(ctx, analysis)
 	if err != nil {
-		if errors.IsNotFound(err) {
-			a.Log.Info(
-				fmt.Sprintf("AnalysisDefinition '%s' in namespace '%s' not found, requeue",
-					analysis.Spec.AnalysisDefinition.Name,
-					analysisDefNamespace),
-			)
-			return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, nil
-		}
-		a.Log.Error(err, "Failed to retrieve the AnalysisDefinition")
-		return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
+		// do not return error, as here we should always try to fetch the definition again
+		// in the next reconcile loop
+		return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, nil
+	}
+
+	if analysis.Status.State.IsPending() {
+		analysis.Status.State = v1alpha3.StateProgressing
 	}
 
 	var done map[string]metricsapi.ProviderResult
 	todo := analysisDef.Spec.Objectives
 	if analysis.Status.StoredValues != nil {
 		todo, done = extractMissingObjectives(analysisDef.Spec.Objectives, analysis.Status.StoredValues)
-		if len(todo) == 0 {
-			return ctrl.Result{}, nil
-		}
 	}
 
 	//create multiple workers handling the Objectives
-	childCtx, wp := a.NewWorkersPoolFactory(ctx, analysis, todo, a.MaxWorkers, a.Client, a.Log, analysisDefNamespace)
+	childCtx, wp := a.NewWorkersPoolFactory(ctx, analysis, todo, a.MaxWorkers, a.Client, a.Log, analysisDef.Namespace)
 
 	res, err := wp.DispatchAndCollect(childCtx)
 	if err != nil {
 		a.Log.Error(err, "Failed to collect all values required for the Analysis, caching collected values")
 		analysis.Status.StoredValues = res
-		err = a.updateStatus(ctx, analysis)
-		return ctrl.Result{RequeueAfter: 10 * time.Second}, err
+		return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, a.updateStatus(ctx, analysis)
 	}
 
 	maps.Copy(res, done)
-
-	err = a.evaluateObjectives(ctx, res, analysisDef, analysis)
-
-	// if evaluation was successful remove the stored values
-	if err == nil {
-		analysis.Status.StoredValues = nil
-		err = a.updateStatus(ctx, analysis)
+	a.evaluateObjectives(ctx, res, analysisDef, analysis)
+	if err := a.updateStatus(ctx, analysis); err != nil {
+		return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, err
 	}
 
-	return ctrl.Result{}, err
+	return ctrl.Result{}, nil
 }
 
-func (a *AnalysisReconciler) evaluateObjectives(ctx context.Context, res map[string]metricsapi.ProviderResult, analysisDef *metricsapi.AnalysisDefinition, analysis *metricsapi.Analysis) error {
+func (a *AnalysisReconciler) evaluateObjectives(ctx context.Context, res map[string]metricsapi.ProviderResult, analysisDef *metricsapi.AnalysisDefinition, analysis *metricsapi.Analysis) {
 	eval := a.Evaluate(res, analysisDef)
 	analysisResultJSON, err := json.Marshal(eval)
 	if err != nil {
 		a.Log.Error(err, "Could not marshal status")
 	} else {
 		analysis.Status.Raw = string(analysisResultJSON)
 	}
-	if eval.Warning {
-		analysis.Status.Warning = true
-	}
+	analysis.Status.Warning = eval.Warning
 	analysis.Status.Pass = eval.Pass
-	return a.updateStatus(ctx, analysis)
+	analysis.Status.State = metricsapi.StateCompleted
+	// if evaluation was successful remove the stored values
+	analysis.Status.StoredValues = nil
 }
 
 func (a *AnalysisReconciler) updateStatus(ctx context.Context, analysis *metricsapi.Analysis) error {
@@ -157,6 +143,32 @@ func (a *AnalysisReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(a)
 }
 
+func (a *AnalysisReconciler) retrieveAnalysisDefinition(ctx context.Context, analysis *metricsapi.Analysis) (*metricsapi.AnalysisDefinition, error) {
+	analysisDefNamespace := analysis.Spec.AnalysisDefinition.GetNamespace(analysis.Namespace)
+	analysisDef := &metricsapi.AnalysisDefinition{}
+	err := a.Client.Get(ctx,
+		types.NamespacedName{
+			Name:      analysis.Spec.AnalysisDefinition.Name,
+			Namespace: analysisDefNamespace},
+		analysisDef,
+	)
+
+	if err != nil {
+		if errors.IsNotFound(err) {
+			a.Log.Info(
+				fmt.Sprintf("AnalysisDefinition '%s' in namespace '%s' not found, requeueing",
+					analysis.Spec.AnalysisDefinition.Name,
+					analysis.Spec.AnalysisDefinition.Name),
+			)
+			return nil, err
+		}
+		a.Log.Error(err, "Failed to retrieve the AnalysisDefinition")
+		return nil, err
+	}
+
+	return analysisDef, nil
+}
+
 func extractMissingObjectives(objectives []metricsapi.Objective, status map[string]metricsapi.ProviderResult) ([]metricsapi.Objective, map[string]metricsapi.ProviderResult) {
 	var todo []metricsapi.Objective
 	done := make(map[string]metricsapi.ProviderResult, len(status))