Repository: argoproj/argo-rollouts

Issue:
Inconclusive state is not respected for background analysis
<!-- If you are trying to resolve an environment-specific issue or have a one-off question about the edge case that does not require a feature then please consider asking a question in argo rollouts slack [channel](https://argoproj.github.io/community/join-slack). -->

Checklist:

* [x] I've included steps to reproduce the bug.
* [x] I've included the version of argo rollouts.

**Describe the bug**

When I create a Rollout that uses background analysis + a canary pause step, I am unable to get the Rollout to stay in the paused state when the canary analysis is inconclusive. Instead, I'm seeing the AnalysisRun getting into the Inconclusive state but and restarted/recreated over and over until it finally promotes the Rollout as healthy (at the end of the pause duration).

I've inspected the Rollout and AnalysisRuns during the canary analysis and noticed that the `status.pauseConditions` never includes Inconclusive as a reason which may be contributing to this issue. In spot checking the logs for Argo Rollouts after rerunning my test a couple of times, I do see logs that indicate that the inconclusive pause condition was added but it seems to be inconsistent, in most cases the Rollout never reflects that the background AnalysisRuns were inconclusive.

**To Reproduce**

Below is the exact AnalysisTemplate and Rollout that I've used to reproduce this.

```yaml
apiVersion: argoproj.io/v1alpha1
kind: AnalysisTemplate
metadata:
  name: misconfigured-analysis
spec:
  metrics:
  - name: faked-success
    successCondition: result.status < 300
    failureCondition: result.status > 400
    count: 5
    interval: 10s
    inconclusiveLimit: 0
    provider:
      web:
        url: https://httpbin.org/anything
        method: POST
        jsonBody:
          status: 302
        jsonPath: "{$.json}"
---
apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
  name: inconclusive-analysis-test-rollout
spec:
  replicas: 2
  strategy:
    canary:
      canaryService: inconclusive-analysis-rollout-canary
      stableService: inconclusive-analysis-rollout-stable
      trafficRouting:
        nginx:
          stableIngress: inconclusive-analysis-rollout-stable
      steps:
      - setWeight: 33
      - pause: {duration: 5m}
      analysis:
        templates:
        - templateName: inconclusive-analysis-misconfigured-analysis
        startingStep: 1 
  revisionHistoryLimit: 2
  selector:
    matchLabels:
      app: inconclusive-analysis-rollout
  template:
    metadata:
      labels:
        app: inconclusive-analysis-rollout
    spec:
      containers:
        - name: rollouts-demo
          image: argoproj/rollouts-demo:green
          ports:
            - name: http
              containerPort: 8080
              protocol: TCP
          resources:
            requests:
              memory: 32Mi
              cpu: 5m
```

After applying those alongside the references service and ingress, you should notice that the AnalysisRuns are created and enter an Inconclusive state but after the pause duration, the Rollout promotes the canary revision to stable.

**Expected behavior**

The Rollout reflects that it's encountered Inconclusive analysis and pauses as described in the docs.

**Screenshots**

![Screenshot 2024-09-24 at 3 28 40 PM](https://github.com/user-attachments/assets/a733bad0-4f13-4113-840a-f025d4d5a2b4)


**Version**

`v1.7.2`

**Logs**

There's a ton of logs generated for this Rollout so I've omitted them but happy to pull them if needed.

---
<!-- Issue Author: Don't delete this message to encourage other users to support your issue! -->
**Message from the maintainers**:

Impacted by this bug? Give it a 👍. We prioritize the issues with the most 👍.


Discussion:

Solution patch:
diff --git a/rollout/analysis.go b/rollout/analysis.go
index f23f680c..2c616bea 100644
--- a/rollout/analysis.go
+++ b/rollout/analysis.go
@@ -169,8 +169,14 @@ 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
-	if rollout.Status.ControllerPause && getPauseCondition(rollout, v1alpha1.PauseReasonBlueGreenPause) == nil {
+	// 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 {
+
 		return currentAr.Status.Phase == v1alpha1.AnalysisPhaseInconclusive
 	}
 	return rollout.Status.AbortedAt != nil



Test patch:
diff --git a/test/e2e/analysis_test.go b/test/e2e/analysis_test.go
index 13b6d19b..c58ce1c5 100644
--- a/test/e2e/analysis_test.go
+++ b/test/e2e/analysis_test.go
@@ -27,6 +27,7 @@ func (s *AnalysisSuite) SetupSuite() {
 	s.E2ESuite.SetupSuite()
 	// shared analysis templates for suite
 	s.ApplyManifests("@functional/analysistemplate-web-background.yaml")
+	s.ApplyManifests("@functional/analysistemplate-web-background-inconclusive.yaml")
 	s.ApplyManifests("@functional/analysistemplate-sleep-job.yaml")
 	s.ApplyManifests("@functional/analysistemplate-multiple-job.yaml")
 	s.ApplyManifests("@functional/analysistemplate-fail-multiple-job.yaml")
@@ -68,6 +69,28 @@ func (s *AnalysisSuite) TestCanaryBackgroundAnalysis() {
 		WaitForBackgroundAnalysisRunPhase("Successful")
 }
 
+func (s *AnalysisSuite) TestCanaryInconclusiveBackgroundAnalysis() {
+	s.Given().
+		RolloutObjects("@functional/rollout-background-analysis-inconclusive.yaml").
+		When().
+		ApplyManifests().
+		WaitForRolloutStatus("Healthy").
+		Then().
+		ExpectAnalysisRunCount(0).
+		When().
+		UpdateSpec().
+		WaitForRolloutStatus("Paused").
+		WaitForBackgroundAnalysisRunPhase("Running").
+		Then().
+		ExpectAnalysisRunCount(1).
+		When().
+		WaitForBackgroundAnalysisRunPhase("Inconclusive").
+		WaitForRolloutMessage("InconclusiveAnalysisRun").
+		Then().
+		ExpectRolloutStatus("Paused").
+		ExpectRolloutMessage("InconclusiveAnalysisRun")
+}
+
 func (s *AnalysisSuite) TestCanaryInlineAnalysis() {
 	s.Given().
 		RolloutObjects("@functional/rollout-inline-analysis.yaml").
diff --git a/test/e2e/functional/analysistemplate-web-background-inconclusive.yaml b/test/e2e/functional/analysistemplate-web-background-inconclusive.yaml
new file mode 100644
index 00000000..bc7a1610
--- /dev/null
+++ b/test/e2e/functional/analysistemplate-web-background-inconclusive.yaml
@@ -0,0 +1,22 @@
+# A web metric which uses the httpbin service as a metric provider
+apiVersion: argoproj.io/v1alpha1
+kind: AnalysisTemplate
+metadata:
+  name: web-background-inconclusive
+spec:
+  args:
+    - name: url-val
+      value: "https://httpbin.org/anything"
+  metrics:
+    - 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}"
diff --git a/test/e2e/functional/rollout-background-analysis-inconclusive.yaml b/test/e2e/functional/rollout-background-analysis-inconclusive.yaml
new file mode 100644
index 00000000..932655fb
--- /dev/null
+++ b/test/e2e/functional/rollout-background-analysis-inconclusive.yaml
@@ -0,0 +1,30 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Rollout
+metadata:
+  name: rollout-background-analysis-inconclusive
+spec:
+  replicas: 2
+  strategy:
+    canary:
+      steps:
+      - setWeight: 10
+      - pause: { duration: 30s }
+      analysis:
+        templates:
+        - templateName: web-background-inconclusive
+        startingStep: 1
+  selector:
+    matchLabels:
+      app: rollout-background-analysis-inconclusive
+  template:
+    metadata:
+      labels:
+        app: rollout-background-analysis-inconclusive
+    spec:
+      containers:
+      - name: rollouts-demo
+        image: nginx:1.19-alpine
+        resources:
+          requests:
+            memory: 16Mi
+            cpu: 5m
diff --git a/test/fixtures/then.go b/test/fixtures/then.go
index 68a9d822..813d873d 100644
--- a/test/fixtures/then.go
+++ b/test/fixtures/then.go
@@ -56,6 +56,18 @@ func (t *Then) ExpectRolloutStatus(expectedStatus string) *Then {
 	return t
 }
 
+func (t *Then) ExpectRolloutMessage(expectedMessage string) *Then {
+	ro, err := t.rolloutClient.ArgoprojV1alpha1().Rollouts(t.namespace).Get(t.Context, t.rollout.GetName(), metav1.GetOptions{})
+	t.CheckError(err)
+	_, message := rolloututil.GetRolloutPhase(ro)
+	if message != expectedMessage {
+		t.log.Errorf("Rollout message expected to be '%s'. actual: %s", expectedMessage, message)
+		t.t.FailNow()
+	}
+	t.log.Infof("Rollout expectation status=%s met", expectedMessage)
+	return t
+}
+
 func (t *Then) ExpectReplicaCounts(desired, current, updated, ready, available any) *Then {
 	ro, err := t.rolloutClient.ArgoprojV1alpha1().Rollouts(t.namespace).Get(t.Context, t.rollout.GetName(), metav1.GetOptions{})
 	t.CheckError(err)
diff --git a/test/fixtures/when.go b/test/fixtures/when.go
index 4f8159e5..b4b7ed24 100644
--- a/test/fixtures/when.go
+++ b/test/fixtures/when.go
@@ -278,6 +278,14 @@ func (w *When) WaitForRolloutStatus(status string, timeout ...time.Duration) *Wh
 	return w.WaitForRolloutCondition(checkStatus, fmt.Sprintf("status=%s", status), timeout...)
 }
 
+func (w *When) WaitForRolloutMessage(message string, timeout ...time.Duration) *When {
+	checkStatus := func(ro *rov1.Rollout) bool {
+		_, m := rolloututil.GetRolloutPhase(ro)
+		return m == message
+	}
+	return w.WaitForRolloutCondition(checkStatus, fmt.Sprintf("message=%s", message), timeout...)
+}
+
 func (w *When) MarkPodsReady(revision string, count int, timeouts ...time.Duration) *When {
 	ticker := time.NewTicker(time.Second)
 	defer ticker.Stop()