From ee8d8408540512682a1fdc2e5411b99830162183 Mon Sep 17 00:00:00 2001 From: Rohit Agrawal Date: Thu, 7 Jul 2022 09:54:34 -0700 Subject: [PATCH] fix(analysis): Fix Analysis Terminal Decision For Dry-Run Metrics Signed-off-by: Rohit Agrawal --- analysis/analysis_test.go | 8 ++++---- utils/analysis/helpers.go | 4 +++- utils/analysis/helpers_test.go | 14 ++++++++++++++ 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/analysis/analysis_test.go b/analysis/analysis_test.go index d4763f3e4b..30e3b7adb9 100644 --- a/analysis/analysis_test.go +++ b/analysis/analysis_test.go @@ -1500,7 +1500,7 @@ func TestAssessRunStatusErrorMessageAnalysisPhaseFail(t *testing.T) { func TestAssessRunStatusErrorMessageAnalysisPhaseFailInDryRunMode(t *testing.T) { status, message, dryRunSummary := StartAssessRunStatusErrorMessageAnalysisPhaseFail(t, true) - assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, status) + assert.Equal(t, v1alpha1.AnalysisPhaseRunning, status) assert.Equal(t, "", message) expectedDryRunSummary := v1alpha1.RunSummary{ Count: 2, @@ -1545,7 +1545,7 @@ func TestAssessRunStatusErrorMessageFromProvider(t *testing.T) { func TestAssessRunStatusErrorMessageFromProviderInDryRunMode(t *testing.T) { providerMessage := "Provider Error" status, message, dryRunSummary := StartAssessRunStatusErrorMessageFromProvider(t, providerMessage, true) - assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, status) + assert.Equal(t, v1alpha1.AnalysisPhaseRunning, status) assert.Equal(t, "", message) expectedDryRunSummary := v1alpha1.RunSummary{ Count: 2, @@ -1587,7 +1587,7 @@ func TestAssessRunStatusMultipleFailures(t *testing.T) { func TestAssessRunStatusMultipleFailuresInDryRunMode(t *testing.T) { status, message, dryRunSummary := StartAssessRunStatusMultipleFailures(t, true) - assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, status) + assert.Equal(t, v1alpha1.AnalysisPhaseRunning, status) assert.Equal(t, "", message) expectedDryRunSummary := v1alpha1.RunSummary{ Count: 2, @@ -1623,7 +1623,7 @@ func TestAssessRunStatusWorstMessageInReconcileAnalysisRun(t *testing.T) { func TestAssessRunStatusWorstMessageInReconcileAnalysisRunInDryRunMode(t *testing.T) { newRun := StartAssessRunStatusWorstMessageInReconcileAnalysisRun(t, true) - assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, newRun.Status.Phase) + assert.Equal(t, v1alpha1.AnalysisPhaseRunning, newRun.Status.Phase) assert.Equal(t, "", newRun.Status.Message) expectedDryRunSummary := v1alpha1.RunSummary{ Count: 2, diff --git a/utils/analysis/helpers.go b/utils/analysis/helpers.go index 64506ebae0..77583136af 100644 --- a/utils/analysis/helpers.go +++ b/utils/analysis/helpers.go @@ -87,7 +87,9 @@ func IsTerminating(run *v1alpha1.AnalysisRun) bool { for _, res := range run.Status.MetricResults { switch res.Phase { case v1alpha1.AnalysisPhaseFailed, v1alpha1.AnalysisPhaseError, v1alpha1.AnalysisPhaseInconclusive: - return true + // If this metric is running in the dryRun mode then we don't care about the failures and hence the terminal + // decision shouldn't be affected. + return !res.DryRun } } return false diff --git a/utils/analysis/helpers_test.go b/utils/analysis/helpers_test.go index c7bd003c18..f9baf89f14 100644 --- a/utils/analysis/helpers_test.go +++ b/utils/analysis/helpers_test.go @@ -62,20 +62,34 @@ func TestIsFastFailTerminating(t *testing.T) { Name: "success-rate", Phase: v1alpha1.AnalysisPhaseRunning, }, + { + Name: "dry-run-metric", + Phase: v1alpha1.AnalysisPhaseRunning, + DryRun: true, + }, }, }, } + // Verify that when the metric is not failing or in the error state then we don't terminate. successRate := run.Status.MetricResults[1] assert.False(t, IsTerminating(run)) + // Metric failing in the dryRun mode shouldn't impact the terminal decision. + dryRunMetricResult := run.Status.MetricResults[2] + dryRunMetricResult.Phase = v1alpha1.AnalysisPhaseError + run.Status.MetricResults[2] = dryRunMetricResult + assert.False(t, IsTerminating(run)) + // Verify that a wet run metric failure/error results in terminal decision. successRate.Phase = v1alpha1.AnalysisPhaseError run.Status.MetricResults[1] = successRate assert.True(t, IsTerminating(run)) successRate.Phase = v1alpha1.AnalysisPhaseFailed run.Status.MetricResults[1] = successRate assert.True(t, IsTerminating(run)) + // Verify that an inconclusive wet run metric results in terminal decision. successRate.Phase = v1alpha1.AnalysisPhaseInconclusive run.Status.MetricResults[1] = successRate assert.True(t, IsTerminating(run)) + // Verify that we don't terminate when there are no metric results or when the status is empty. run.Status.MetricResults = nil assert.False(t, IsTerminating(run)) run.Status = v1alpha1.AnalysisRunStatus{}