From 36a86ba13f1d6d87781011fc7d78b3119a55209e Mon Sep 17 00:00:00 2001
From: Yurii Popivniak <ypopivniak@cargurus.com>
Date: Mon, 7 Oct 2024 23:53:05 +0300
Subject: [PATCH] fix: e2e test for inconclusive analysisrun

Signed-off-by: Yurii Popivniak <ypopivniak@cargurus.com>
---
 rollout/analysis.go                           |  5 +++-
 test/e2e/analysis_test.go                     |  2 +-
 ...stemplate-web-background-inconclusive.yaml | 25 +++++++++++--------
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/rollout/analysis.go b/rollout/analysis.go
index 5f167c9ca1..2c616beaef 100644
--- a/rollout/analysis.go
+++ b/rollout/analysis.go
@@ -169,7 +169,10 @@ func needsNewAnalysisRun(currentAr *v1alpha1.AnalysisRun, rollout *v1alpha1.Roll
 	// is set and then seeing if the last status was inconclusive.
 	// There is an additional check for the BlueGreen Pause because the prepromotion analysis always has the BlueGreen
 	// Pause and that causes controllerPause to be set. The extra check for the BlueGreen Pause ensures that a new Analysis
-	// Run is created only when the previous AnalysisRun is inconclusive
+	// Run is created only when the previous AnalysisRun is inconclusive.
+	// Additional check for the Canary Pause prevents Canary promotion when AnalysisRun is inconclusive and reached
+	// inconclusiveLimit. Otherwise, another AnalysisRun will be spawned and can cause Success status,
+	// because of termination when the AnalysisRun is still in-flight.
 	if rollout.Status.ControllerPause &&
 		getPauseCondition(rollout, v1alpha1.PauseReasonCanaryPauseStep) == nil &&
 		getPauseCondition(rollout, v1alpha1.PauseReasonBlueGreenPause) == nil {
diff --git a/test/e2e/analysis_test.go b/test/e2e/analysis_test.go
index 56fb82815d..c58ce1c520 100644
--- a/test/e2e/analysis_test.go
+++ b/test/e2e/analysis_test.go
@@ -80,9 +80,9 @@ func (s *AnalysisSuite) TestCanaryInconclusiveBackgroundAnalysis() {
 		When().
 		UpdateSpec().
 		WaitForRolloutStatus("Paused").
+		WaitForBackgroundAnalysisRunPhase("Running").
 		Then().
 		ExpectAnalysisRunCount(1).
-		ExpectBackgroundAnalysisRunPhase("Running").
 		When().
 		WaitForBackgroundAnalysisRunPhase("Inconclusive").
 		WaitForRolloutMessage("InconclusiveAnalysisRun").
diff --git a/test/e2e/functional/analysistemplate-web-background-inconclusive.yaml b/test/e2e/functional/analysistemplate-web-background-inconclusive.yaml
index ee9ec64b68..bc7a16100d 100644
--- a/test/e2e/functional/analysistemplate-web-background-inconclusive.yaml
+++ b/test/e2e/functional/analysistemplate-web-background-inconclusive.yaml
@@ -1,4 +1,4 @@
-# A dummy web metric which uses the kubernetes version endpoint as a metric provider
+# A web metric which uses the httpbin service as a metric provider
 apiVersion: argoproj.io/v1alpha1
 kind: AnalysisTemplate
 metadata:
@@ -6,14 +6,17 @@ metadata:
 spec:
   args:
     - name: url-val
-      value: "https://kubernetes.default.svc/version"
+      value: "https://httpbin.org/anything"
   metrics:
-  - name: web
-    interval: 7s
-    inconclusiveLimit: 3
-    successCondition: result.major == '2000'
-    failureCondition: result.major == '1000'
-    provider:
-      web:
-        url: "{{args.url-val}}"
-        insecure: true
+    - name: web
+      interval: 7s
+      inconclusiveLimit: 3
+      successCondition: result.status == "success"
+      failureCondition: result.status == "failure"
+      provider:
+        web:
+          url: "{{args.url-val}}"
+          method: POST
+          jsonBody:
+            status: "inconclusive"
+          jsonPath: "{$.json}"