feat(analysis): Add ConsecutiveSuccessLimit feature to Analysis (argo…

…proj#3970)

Signed-off-by: Youssef Rabie <youssef.rabie@procore.com>

analysis/analysis.go

@@ -387,17 +387,21 @@ func (c *Controller) runMeasurements(run *v1alpha1.AnalysisRun, tasks []metricTa
 					metricResult.Successful++
 					metricResult.Count++
 					metricResult.ConsecutiveError = 0
+					metricResult.ConsecutiveSuccess++
 				case v1alpha1.AnalysisPhaseFailed:
 					metricResult.Failed++
 					metricResult.Count++
 					metricResult.ConsecutiveError = 0
+					metricResult.ConsecutiveSuccess = 0
 				case v1alpha1.AnalysisPhaseInconclusive:
 					metricResult.Inconclusive++
 					metricResult.Count++
 					metricResult.ConsecutiveError = 0
+					metricResult.ConsecutiveSuccess = 0
 				case v1alpha1.AnalysisPhaseError:
 					metricResult.Error++
 					metricResult.ConsecutiveError++
+					metricResult.ConsecutiveSuccess = 0
 					logger.Warnf("Measurement had error: %s", newMeasurement.Message)
 				}
 			}
@@ -599,11 +603,47 @@ func assessMetricStatus(metric v1alpha1.Metric, result v1alpha1.MetricResult, te
 		return phaseFailureInconclusiveOrError
 	}
 
+	// Check if consecutiveSuccessLimit is applicable and was reached.
+	if metric.ConsecutiveSuccessLimit != nil && metric.ConsecutiveSuccessLimit.IntValue() > 0 && result.ConsecutiveSuccess >= int32(metric.ConsecutiveSuccessLimit.IntValue()) {
+		logger.Infof("Metric Assessment Result - %s: ConsecutiveSuccessLimit (%s) Reached", v1alpha1.AnalysisPhaseSuccessful, metric.ConsecutiveSuccessLimit.String())
+		return v1alpha1.AnalysisPhaseSuccessful
+	}
+
 	// If a count was specified, and we reached that count, then metric is considered Successful.
 	// The Error, Failed, Inconclusive counters are ignored because those checks have already been
 	// taken into consideration above, and we do not want to fail if failures < failureLimit.
 	effectiveCount := metric.EffectiveCount()
 	if effectiveCount != nil && result.Count >= int32(effectiveCount.IntValue()) {
+
+		failureApplicable := (metric.FailureLimit != nil && metric.FailureLimit.IntValue() >= 0) || metric.FailureLimit == nil
+		successApplicable := metric.ConsecutiveSuccessLimit != nil && metric.ConsecutiveSuccessLimit.IntValue() > 0
+
+		if failureApplicable && successApplicable {
+
+			// failureLimit was checked above and not reached.
+			// consecutiveSuccessLimit was checked above and not reached.
+
+			failureLimit := "0"
+			if metric.FailureLimit != nil {
+				failureLimit = metric.FailureLimit.String()
+			}
+
+			logger.Infof("Metric Assessment Result - %s: ConsecutiveSuccessLimit (%s) Not Reached and FailureLimit (%s) Not Violated", v1alpha1.AnalysisPhaseInconclusive, metric.ConsecutiveSuccessLimit.String(), failureLimit)
+			return v1alpha1.AnalysisPhaseInconclusive
+
+		} else if successApplicable {
+
+			logger.Infof("Metric Assessment Result - %s: ConsecutiveSuccessLimit (%s) Not Reached", v1alpha1.AnalysisPhaseFailed, metric.ConsecutiveSuccessLimit.String())
+			return v1alpha1.AnalysisPhaseFailed
+
+		} else if failureApplicable {
+			// failureLimit was not reached in assessMetricFailureInconclusiveOrError above.
+			// AnalysisPhaseSuccessful below.
+		} else {
+			// This cannot happen, since one of failureLimit or consecutiveSuccessLimit will be applicable
+			// We validate that failureLimit >= 0 when consecutiveSuccessLimit == 0
+		}
+
 		logger.Infof("Metric Assessment Result - %s: Count (%s) Reached", v1alpha1.AnalysisPhaseSuccessful, effectiveCount.String())
 		return v1alpha1.AnalysisPhaseSuccessful
 	}
@@ -623,7 +663,8 @@ func assessMetricFailureInconclusiveOrError(metric v1alpha1.Metric, result v1alp
 	if metric.FailureLimit != nil {
 		failureLimit = int32(metric.FailureLimit.IntValue())
 	}
-	if result.Failed > failureLimit {
+	// If failureLimit is negative, that means it isn't applicable.
+	if failureLimit >= 0 && result.Failed > failureLimit {
 		phase = v1alpha1.AnalysisPhaseFailed
 		message = fmt.Sprintf("failed (%d) > failureLimit (%d)", result.Failed, failureLimit)
 	}

analysis/analysis_test.go

@@ -594,6 +594,208 @@ func TestAssessMetricStatusFailureLimit(t *testing.T) { // max failures
 	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, true))
 }
 
+func TestAssessMetricStatusConsecutiveSuccessLimit(t *testing.T) {
+	failureLimit := intstr.FromInt(-1)
+	consecutiveSuccessLimit := intstr.FromInt(3)
+	metric := v1alpha1.Metric{
+		Name:                    "success-rate",
+		ConsecutiveSuccessLimit: &consecutiveSuccessLimit,
+		FailureLimit:            &failureLimit,
+		Interval:                "60s",
+	}
+	result := v1alpha1.MetricResult{
+		Failed:             3,
+		Successful:         4,
+		ConsecutiveSuccess: 3,
+		Count:              7,
+		Measurements: []v1alpha1.Measurement{{
+			Value:      "99",
+			Phase:      v1alpha1.AnalysisPhaseFailed,
+			StartedAt:  timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))),
+			FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))),
+		}},
+	}
+
+	/////////////////////////////////////////////////////////////////////////
+	// For indefinite analysis (count is not set)
+
+	// When ConsecutiveSuccess == ConsecutiveSuccessLimit
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, true))
+
+	// When ConsecutiveSuccess < ConsecutiveSuccessLimit
+	consecutiveSuccessLimit = intstr.FromInt(5)
+	metric.ConsecutiveSuccessLimit = &consecutiveSuccessLimit
+	assert.Equal(t, v1alpha1.AnalysisPhaseRunning, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, true))
+
+	/////////////////////////////////////////////////////////////////////////
+	// For limited analysis (count is >= 1)
+
+	/// When metric.Count is reached
+	metricCount := intstr.FromInt(7)
+	metric.Count = &metricCount
+
+	//// ConsecutiveSuccess=3 < ConsecutiveSuccessLimit=5
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, true))
+
+	//// ConsecutiveSuccess = ConsecutiveSuccessLimit = 3
+	consecutiveSuccessLimit = intstr.FromInt(3)
+	metric.ConsecutiveSuccessLimit = &consecutiveSuccessLimit
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, true))
+
+	/// When metric.Count is not reached
+	metricCount = intstr.FromInt(9)
+	metric.Count = &metricCount
+
+	//// ConsecutiveSuccess = ConsecutiveSuccessLimit = 3
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, true))
+
+	//// ConsecutiveSuccess=3 < ConsecutiveSuccessLimit=5
+	consecutiveSuccessLimit = intstr.FromInt(5)
+	metric.ConsecutiveSuccessLimit = &consecutiveSuccessLimit
+	assert.Equal(t, v1alpha1.AnalysisPhaseRunning, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, true))
+}
+
+func TestAssessMetricStatusFailureLimitAndConsecutiveSuccessLimit(t *testing.T) {
+	failureLimit := intstr.FromInt(4)
+	consecutiveSuccessLimit := intstr.FromInt(4)
+	metric := v1alpha1.Metric{
+		Name:                    "success-rate",
+		ConsecutiveSuccessLimit: &consecutiveSuccessLimit,
+		FailureLimit:            &failureLimit,
+		Interval:                "60s",
+	}
+	result := v1alpha1.MetricResult{
+		Failed:             3,
+		Successful:         4,
+		ConsecutiveSuccess: 3,
+		Count:              7,
+		Measurements: []v1alpha1.Measurement{{
+			Value:      "99",
+			Phase:      v1alpha1.AnalysisPhaseFailed,
+			StartedAt:  timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))),
+			FinishedAt: timePtr(metav1.NewTime(time.Now().Add(-60 * time.Second))),
+		}},
+	}
+
+	/////////////////////////////////////////////////////////////////////////
+	// For indefinite analysis (count is not set)
+
+	// FailureLimit is not violated and consecutiveSuccessLimit not yet satisfied.
+	assert.Equal(t, v1alpha1.AnalysisPhaseRunning, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, true))
+
+	// FailureLimit is violated and consecutiveSuccessLimit is not yet satisfied.
+	result.Failed = 5
+	result.Successful = 9
+	result.Count = 9
+	result.ConsecutiveSuccess = 0
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, true))
+
+	// FailureLimit is not violated and consecutiveSuccessLimit is satisfied.
+	result.Failed = 3
+	result.Successful = 5
+	result.Count = 8
+	result.ConsecutiveSuccess = 4
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, true))
+
+	// FailureLimit is violated and consecutiveSuccessLimit is satisfied.
+	result.Failed = 5
+	result.Successful = 5
+	result.Count = 10
+	result.ConsecutiveSuccess = 4
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, true))
+
+	/////////////////////////////////////////////////////////////////////////
+	// For limited analysis (count is >= 1)
+	metricCount := intstr.FromInt(10)
+	metric.Count = &metricCount
+
+	/// When metric.Count is reached
+
+	//// FailureLimit is not violated and consecutiveSuccessLimit not yet satisfied.
+	result.Failed = 4
+	result.Successful = 6
+	result.Count = 10
+	result.ConsecutiveSuccess = 3
+
+	assert.Equal(t, v1alpha1.AnalysisPhaseInconclusive, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseInconclusive, assessMetricStatus(metric, result, true))
+
+	//// FailureLimit is violated and consecutiveSuccessLimit is not yet satisfied.
+	result.Failed = 5
+	result.Successful = 5
+	result.Count = 10
+	result.ConsecutiveSuccess = 3
+
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, true))
+
+	//// FailureLimit is not violated and consecutiveSuccessLimit is satisfied.
+	result.Failed = 4
+	result.Successful = 6
+	result.Count = 10
+	result.ConsecutiveSuccess = 4
+
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, true))
+
+	//// FailureLimit is violated and consecutiveSuccessLimit is satisfied.
+	result.Failed = 5
+	result.Successful = 5
+	result.Count = 10
+	result.ConsecutiveSuccess = 4
+
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, true))
+
+	/// When metric.Count is not yet reached
+
+	//// FailureLimit is not violated and consecutiveSuccessLimit not yet satisfied.
+	result.Failed = 3
+	result.Successful = 5
+	result.Count = 8
+	result.ConsecutiveSuccess = 3
+
+	assert.Equal(t, v1alpha1.AnalysisPhaseRunning, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, true))
+
+	//// FailureLimit is violated and consecutiveSuccessLimit is not yet satisfied.
+	result.Failed = 5
+	result.Successful = 3
+	result.Count = 8
+	result.ConsecutiveSuccess = 3
+
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, true))
+
+	//// FailureLimit is not violated and consecutiveSuccessLimit is satisfied.
+	result.Failed = 3
+	result.Successful = 5
+	result.Count = 8
+	result.ConsecutiveSuccess = 4
+
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, assessMetricStatus(metric, result, true))
+
+	//// FailureLimit is violated and consecutiveSuccessLimit is satisfied.
+	result.Failed = 5
+	result.Successful = 4
+	result.Count = 9
+	result.ConsecutiveSuccess = 4
+
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, false))
+	assert.Equal(t, v1alpha1.AnalysisPhaseFailed, assessMetricStatus(metric, result, true))
+}
+
 func TestAssessMetricStatusInconclusiveLimit(t *testing.T) {
 	inconclusiveLimit := intstr.FromInt(2)
 	metric := v1alpha1.Metric{

docs/features/kustomize/rollout_cr_schema.json

@@ -105,6 +105,17 @@
                                         ],
                                         "x-kubernetes-int-or-string": true
                                     },
+                                    "consecutiveSuccessLimit": {
+                                        "anyOf": [
+                                            {
+                                                "type": "integer"
+                                            },
+                                            {
+                                                "type": "string"
+                                            }
+                                        ],
+                                        "x-kubernetes-int-or-string": true
+                                    },
                                     "count": {
                                         "anyOf": [
                                             {
@@ -4971,6 +4982,17 @@
                                         ],
                                         "x-kubernetes-int-or-string": true
                                     },
+                                    "consecutiveSuccessLimit": {
+                                        "anyOf": [
+                                            {
+                                                "type": "integer"
+                                            },
+                                            {
+                                                "type": "string"
+                                            }
+                                        ],
+                                        "x-kubernetes-int-or-string": true
+                                    },
                                     "count": {
                                         "anyOf": [
                                             {
@@ -9850,6 +9872,17 @@
                                         ],
                                         "x-kubernetes-int-or-string": true
                                     },
+                                    "consecutiveSuccessLimit": {
+                                        "anyOf": [
+                                            {
+                                                "type": "integer"
+                                            },
+                                            {
+                                                "type": "string"
+                                            }
+                                        ],
+                                        "x-kubernetes-int-or-string": true
+                                    },
                                     "count": {
                                         "anyOf": [
                                             {

manifests/crds/analysis-run-crd.yaml

@@ -98,6 +98,11 @@ spec:
                       - type: integer
                       - type: string
                       x-kubernetes-int-or-string: true
+                    consecutiveSuccessLimit:
+                      anyOf:
+                      - type: integer
+                      - type: string
+                      x-kubernetes-int-or-string: true
                     count:
                       anyOf:
                       - type: integer
@@ -3219,6 +3224,9 @@ spec:
                     consecutiveError:
                       format: int32
                       type: integer
+                    consecutiveSuccess:
+                      format: int32
+                      type: integer
                     count:
                       format: int32
                       type: integer

manifests/crds/analysis-template-crd.yaml

@@ -94,6 +94,11 @@ spec:
                       - type: integer
                       - type: string
                       x-kubernetes-int-or-string: true
+                    consecutiveSuccessLimit:
+                      anyOf:
+                      - type: integer
+                      - type: string
+                      x-kubernetes-int-or-string: true
                     count:
                       anyOf:
                       - type: integer

manifests/crds/cluster-analysis-template-crd.yaml

@@ -94,6 +94,11 @@ spec:
                       - type: integer
                       - type: string
                       x-kubernetes-int-or-string: true
+                    consecutiveSuccessLimit:
+                      anyOf:
+                      - type: integer
+                      - type: string
+                      x-kubernetes-int-or-string: true
                     count:
                       anyOf:
                       - type: integer