diff --git a/rollout/analysis.go b/rollout/analysis.go index f23f680ce0..2c616beaef 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 diff --git a/test/e2e/analysis_test.go b/test/e2e/analysis_test.go index 13b6d19b60..c58ce1c520 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 0000000000..bc7a16100d --- /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 0000000000..932655fb46 --- /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 d39e054d34..2a63465dbc 100644 --- a/test/fixtures/then.go +++ b/test/fixtures/then.go @@ -55,6 +55,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 a5806befb9..10b4b5b7fe 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()