Honor empty condition when evaluating prom results (argoproj#265)

chris-altamimi · Nov 6, 2019 · 08112e1 · 08112e1
1 parent 353c60e
commit 08112e1
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 7 deletions.
diff --git a/metricproviders/prometheus/prometheus.go b/metricproviders/prometheus/prometheus.go
@@ -85,16 +85,36 @@ func (p *Provider) GarbageCollect(run *v1alpha1.AnalysisRun, metric v1alpha1.Met
 }
 
 func (p *Provider) evaluateResult(result interface{}, metric v1alpha1.Metric) v1alpha1.AnalysisStatus {
-	successCondition, err := evaluate.EvalCondition(result, metric.SuccessCondition)
-	if err != nil {
-		p.logCtx.Warning(err.Error())
-		return v1alpha1.AnalysisStatusError
+	successCondition := false
+	failCondition := false
+	var err error
+
+	if metric.SuccessCondition != "" {
+		successCondition, err = evaluate.EvalCondition(result, metric.SuccessCondition)
+		if err != nil {
+			p.logCtx.Warning(err.Error())
+			return v1alpha1.AnalysisStatusError
+		}
+	}
+	if metric.FailureCondition != "" {
+		failCondition, err = evaluate.EvalCondition(result, metric.FailureCondition)
+		if err != nil {
+			return v1alpha1.AnalysisStatusError
+		}
 	}
 
-	failCondition, err := evaluate.EvalCondition(result, metric.FailureCondition)
-	if err != nil {
-		return v1alpha1.AnalysisStatusError
+	switch {
+	case metric.SuccessCondition == "" && metric.FailureCondition == "":
+		//Always return success unless there is an error
+		return v1alpha1.AnalysisStatusSuccessful
+	case metric.SuccessCondition != "" && metric.FailureCondition == "":
+		// Without a failure condition, a measurement is considered a failure if the measurement's success condition is not true
+		failCondition = !successCondition
+	case metric.SuccessCondition == "" && metric.FailureCondition != "":
+		// Without a success condition, a measurement is considered a successful if the measurement's failure condition is not true
+		successCondition = !failCondition
 	}
+
 	if failCondition {
 		return v1alpha1.AnalysisStatusFailed
 	}

diff --git a/metricproviders/prometheus/prometheus_test.go b/metricproviders/prometheus/prometheus_test.go
@@ -197,6 +197,36 @@ func TestEvaluateResultInconclusive(t *testing.T) {
 	assert.Equal(t, v1alpha1.AnalysisStatusInconclusive, status)
 }
 
+func TestEvaluateResultNoSuccessConditionAndNotFailing(t *testing.T) {
+	p := Provider{}
+	metric := v1alpha1.Metric{
+		SuccessCondition: "",
+		FailureCondition: "false",
+	}
+	status := p.evaluateResult(true, metric)
+	assert.Equal(t, v1alpha1.AnalysisStatusSuccessful, status)
+}
+
+func TestEvaluateResultNoFailureConditionAndNotSuccessful(t *testing.T) {
+	p := Provider{}
+	metric := v1alpha1.Metric{
+		SuccessCondition: "false",
+		FailureCondition: "",
+	}
+	status := p.evaluateResult(true, metric)
+	assert.Equal(t, v1alpha1.AnalysisStatusFailed, status)
+}
+
+func TestEvaluateResultNoFailureConditionAndNoSuccessCondition(t *testing.T) {
+	p := Provider{}
+	metric := v1alpha1.Metric{
+		SuccessCondition: "",
+		FailureCondition: "",
+	}
+	status := p.evaluateResult(true, metric)
+	assert.Equal(t, v1alpha1.AnalysisStatusSuccessful, status)
+}
+
 func TestEvaluateResultWithErrorOnSuccessCondition(t *testing.T) {
 	logCtx := log.WithField("test", "test")
 	p := Provider{