Skip to content

Commit

Permalink
fix: prevent hot loop when fully promoted rollout is aborted (argopro…
Browse files Browse the repository at this point in the history 
…j#3064)

* fix: prevent hot loop when fully promoted rollout is aborted

Signed-off-by: Jesse Suen <jesse@akuity.io>

* test: change expectations of abort tests

Signed-off-by: Jesse Suen <jesse@akuity.io>

---------

Signed-off-by: Jesse Suen <jesse@akuity.io>
Signed-off-by: Philip Clark <philip.clark@iterable.com>
  • Loading branch information
@jessesuen @phclark
jessesuen authored and phclark committed Oct 13, 2023
1 parent 27212d0 commit 202b97c
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 85 deletions.
6 changes: 3 additions & 3 deletions experiments/controller_test.go
View file
Expand Up @@ -805,7 +805,7 @@ func TestAddInvalidSpec(t *testing.T) {
"status":{
}
}`, nil, cond)
assert.Equal(t, expectedPatch, patch)
assert.JSONEq(t, expectedPatch, patch)
}

func TestKeepInvalidSpec(t *testing.T) {
Expand Down Expand Up @@ -852,7 +852,7 @@ func TestUpdateInvalidSpec(t *testing.T) {
"status":{
}
}`, nil, cond)
assert.Equal(t, expectedPatch, patch)
assert.JSONEq(t, expectedPatch, patch)

}

Expand Down Expand Up @@ -892,7 +892,7 @@ func TestRemoveInvalidSpec(t *testing.T) {
"status":{
}
}`, templateStatus, cond)
assert.Equal(t, expectedPatch, patch)
assert.JSONEq(t, expectedPatch, patch)
}

func TestRun(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion experiments/experiment_test.go
View file
Expand Up @@ -282,7 +282,7 @@ func TestSuccessAfterDurationPasses(t *testing.T) {
"phase": "Successful"
}
}`, templateStatuses, cond)
assert.Equal(t, expectedPatch, patch)
assert.JSONEq(t, expectedPatch, patch)
}

// TestDontRequeueWithoutDuration verifies we don't requeue if an experiment does not have
Expand Down
4 changes: 2 additions & 2 deletions experiments/replicaset_test.go
View file
Expand Up @@ -42,7 +42,7 @@ func TestCreateMultipleRS(t *testing.T) {
"status":{
}
}`, templateStatus, cond)
assert.Equal(t, expectedPatch, patch)
assert.JSONEq(t, expectedPatch, patch)
}

func TestCreateMissingRS(t *testing.T) {
Expand Down Expand Up @@ -72,7 +72,7 @@ func TestCreateMissingRS(t *testing.T) {
generateTemplatesStatus("bar", 0, 0, v1alpha1.TemplateStatusProgressing, now()),
generateTemplatesStatus("baz", 0, 0, v1alpha1.TemplateStatusProgressing, now()),
}
assert.Equal(t, calculatePatch(e, expectedPatch, templateStatuses, cond), patch)
assert.JSONEq(t, calculatePatch(e, expectedPatch, templateStatuses, cond), patch)
}

func TestTemplateHasMultipleRS(t *testing.T) {
Expand Down
64 changes: 32 additions & 32 deletions rollout/analysis_test.go
View file
Expand Up @@ -180,7 +180,7 @@ func TestCreateBackgroundAnalysisRun(t *testing.T) {
}
}
}`
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch)
}

func TestCreateBackgroundAnalysisRunWithTemplates(t *testing.T) {
Expand Down Expand Up @@ -241,7 +241,7 @@ func TestCreateBackgroundAnalysisRunWithTemplates(t *testing.T) {
}
}
}`
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch)
}

func TestCreateBackgroundAnalysisRunWithClusterTemplates(t *testing.T) {
Expand Down Expand Up @@ -303,7 +303,7 @@ func TestCreateBackgroundAnalysisRunWithClusterTemplates(t *testing.T) {
}
}
}`
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch)
}

func TestInvalidSpecMissingClusterTemplatesBackgroundAnalysis(t *testing.T) {
Expand Down Expand Up @@ -339,7 +339,7 @@ func TestInvalidSpecMissingClusterTemplatesBackgroundAnalysis(t *testing.T) {
expectedPatch := fmt.Sprintf(expectedPatchWithoutSub, progressingCond, string(invalidSpecBytes), strings.ReplaceAll(errmsg, "\"", "\\\""))

patch := f.getPatchedRollout(patchIndex)
assert.Equal(t, calculatePatch(r, expectedPatch), patch)
assert.JSONEq(t, calculatePatch(r, expectedPatch), patch)
}

func TestCreateBackgroundAnalysisRunWithClusterTemplatesAndTemplate(t *testing.T) {
Expand Down Expand Up @@ -416,7 +416,7 @@ func TestCreateBackgroundAnalysisRunWithClusterTemplatesAndTemplate(t *testing.T
}
}
}`
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch)
}

// TestCreateAnalysisRunWithCollision ensures we will create an new analysis run with a new name
Expand Down Expand Up @@ -487,7 +487,7 @@ func TestCreateAnalysisRunWithCollision(t *testing.T) {
}
}
}`
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedAR.Name)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedAR.Name)), patch)
}

// TestCreateAnalysisRunWithCollisionAndSemanticEquality will ensure we do not create an extra
Expand Down Expand Up @@ -550,7 +550,7 @@ func TestCreateAnalysisRunWithCollisionAndSemanticEquality(t *testing.T) {
}
}
}`
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, ar.Name)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, ar.Name)), patch)
}

func TestCreateAnalysisRunOnAnalysisStep(t *testing.T) {
Expand Down Expand Up @@ -611,7 +611,7 @@ func TestCreateAnalysisRunOnAnalysisStep(t *testing.T) {
}
}
}`
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch)
}

func TestFailCreateStepAnalysisRunIfInvalidTemplateRef(t *testing.T) {
Expand Down Expand Up @@ -653,7 +653,7 @@ func TestFailCreateStepAnalysisRunIfInvalidTemplateRef(t *testing.T) {
expectedPatch := fmt.Sprintf(expectedPatchWithoutSub, progressingCond, string(invalidSpecBytes), strings.ReplaceAll(errmsg, "\"", "\\\""))

patch := f.getPatchedRollout(patchIndex)
assert.Equal(t, calculatePatch(r, expectedPatch), patch)
assert.JSONEq(t, calculatePatch(r, expectedPatch), patch)
}

func TestFailCreateBackgroundAnalysisRunIfInvalidTemplateRef(t *testing.T) {
Expand Down Expand Up @@ -698,7 +698,7 @@ func TestFailCreateBackgroundAnalysisRunIfInvalidTemplateRef(t *testing.T) {
expectedPatch := fmt.Sprintf(expectedPatchWithoutSub, progressingCond, string(invalidSpecBytes), strings.ReplaceAll(errmsg, "\"", "\\\""))

patch := f.getPatchedRollout(patchIndex)
assert.Equal(t, calculatePatch(r, expectedPatch), patch)
assert.JSONEq(t, calculatePatch(r, expectedPatch), patch)
}

func TestFailCreateBackgroundAnalysisRunIfMetricRepeated(t *testing.T) {
Expand Down Expand Up @@ -745,7 +745,7 @@ func TestFailCreateBackgroundAnalysisRunIfMetricRepeated(t *testing.T) {
expectedPatch := fmt.Sprintf(expectedPatchWithoutSub, progressingCond, string(invalidSpecBytes), strings.ReplaceAll(errmsg, "\"", "\\\""))

patch := f.getPatchedRollout(patchIndex)
assert.Equal(t, calculatePatch(r, expectedPatch), patch)
assert.JSONEq(t, calculatePatch(r, expectedPatch), patch)
}

func TestDoNothingWithAnalysisRunsWhileBackgroundAnalysisRunRunning(t *testing.T) {
Expand Down Expand Up @@ -798,7 +798,7 @@ func TestDoNothingWithAnalysisRunsWhileBackgroundAnalysisRunRunning(t *testing.T
patchIndex := f.expectPatchRolloutAction(r2)
f.run(getKey(r2, t))
patch := f.getPatchedRollout(patchIndex)
assert.Equal(t, calculatePatch(r2, OnlyObservedGenerationPatch), patch)
assert.JSONEq(t, calculatePatch(r2, OnlyObservedGenerationPatch), patch)
}

func TestDoNothingWhileStepBasedAnalysisRunRunning(t *testing.T) {
Expand Down Expand Up @@ -847,7 +847,7 @@ func TestDoNothingWhileStepBasedAnalysisRunRunning(t *testing.T) {
patchIndex := f.expectPatchRolloutAction(r2)
f.run(getKey(r2, t))
patch := f.getPatchedRollout(patchIndex)
assert.Equal(t, calculatePatch(r2, OnlyObservedGenerationPatch), patch)
assert.JSONEq(t, calculatePatch(r2, OnlyObservedGenerationPatch), patch)
}

func TestCancelOlderAnalysisRuns(t *testing.T) {
Expand Down Expand Up @@ -915,7 +915,7 @@ func TestCancelOlderAnalysisRuns(t *testing.T) {
}
}
}`
assert.Equal(t, calculatePatch(r2, expectedPatch), patch)
assert.JSONEq(t, calculatePatch(r2, expectedPatch), patch)
}

func TestDeleteAnalysisRunsWithNoMatchingRS(t *testing.T) {
Expand Down Expand Up @@ -971,7 +971,7 @@ func TestDeleteAnalysisRunsWithNoMatchingRS(t *testing.T) {
deletedAr := f.getDeletedAnalysisRun(deletedIndex)
assert.Equal(t, deletedAr, arWithDiffPodHash.Name)
patch := f.getPatchedRollout(patchIndex)
assert.Equal(t, calculatePatch(r2, OnlyObservedGenerationPatch), patch)
assert.JSONEq(t, calculatePatch(r2, OnlyObservedGenerationPatch), patch)
}

func TestDeleteAnalysisRunsAfterRSDelete(t *testing.T) {
Expand Down Expand Up @@ -1083,7 +1083,7 @@ func TestIncrementStepAfterSuccessfulAnalysisRun(t *testing.T) {
}`
condition := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs2, false, "", false)

assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition)), patch)
}

func TestPausedOnInconclusiveBackgroundAnalysisRun(t *testing.T) {
Expand Down Expand Up @@ -1152,7 +1152,7 @@ func TestPausedOnInconclusiveBackgroundAnalysisRun(t *testing.T) {
}`
condition := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "", false)

assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, v1alpha1.PauseReasonInconclusiveAnalysis, now, v1alpha1.PauseReasonInconclusiveAnalysis)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, v1alpha1.PauseReasonInconclusiveAnalysis, now, v1alpha1.PauseReasonInconclusiveAnalysis)), patch)
}

func TestPausedStepAfterInconclusiveAnalysisRun(t *testing.T) {
Expand Down Expand Up @@ -1215,7 +1215,7 @@ func TestPausedStepAfterInconclusiveAnalysisRun(t *testing.T) {
}
}`
condition := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "", false)
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, v1alpha1.PauseReasonInconclusiveAnalysis, now, v1alpha1.PauseReasonInconclusiveAnalysis)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, v1alpha1.PauseReasonInconclusiveAnalysis, now, v1alpha1.PauseReasonInconclusiveAnalysis)), patch)
}

func TestErrorConditionAfterErrorAnalysisRunStep(t *testing.T) {
Expand Down Expand Up @@ -1282,7 +1282,7 @@ func TestErrorConditionAfterErrorAnalysisRunStep(t *testing.T) {
errmsg := fmt.Sprintf(conditions.RolloutAbortedMessage, 2) + ": " + ar.Status.Message
condition := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, errmsg, false)
expectedPatch = fmt.Sprintf(expectedPatch, condition, now, errmsg)
assert.Equal(t, calculatePatch(r2, expectedPatch), patch)
assert.JSONEq(t, calculatePatch(r2, expectedPatch), patch)
}

func TestErrorConditionAfterErrorAnalysisRunBackground(t *testing.T) {
Expand Down Expand Up @@ -1358,7 +1358,7 @@ func TestErrorConditionAfterErrorAnalysisRunBackground(t *testing.T) {
condition := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, "", false)

now := timeutil.Now().UTC().Format(time.RFC3339)
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, now, errmsg)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, now, errmsg)), patch)
}

func TestCancelAnalysisRunsWhenAborted(t *testing.T) {
Expand Down Expand Up @@ -1419,7 +1419,7 @@ func TestCancelAnalysisRunsWhenAborted(t *testing.T) {
}`
errmsg := fmt.Sprintf(conditions.RolloutAbortedMessage, 2)
now := timeutil.Now().UTC().Format(time.RFC3339)
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, newConditions, now, errmsg)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, newConditions, now, errmsg)), patch)
}

func TestCancelBackgroundAnalysisRunWhenRolloutIsCompleted(t *testing.T) {
Expand Down Expand Up @@ -1521,7 +1521,7 @@ func TestDoNotCreateBackgroundAnalysisRunAfterInconclusiveRun(t *testing.T) {
f.run(getKey(r2, t))

patch := f.getPatchedRollout(patchIndex)
assert.Equal(t, calculatePatch(r2, OnlyObservedGenerationPatch), patch)
assert.JSONEq(t, calculatePatch(r2, OnlyObservedGenerationPatch), patch)
}

func TestDoNotCreateBackgroundAnalysisRunOnNewCanaryRollout(t *testing.T) {
Expand Down Expand Up @@ -1647,7 +1647,7 @@ func TestCreatePrePromotionAnalysisRun(t *testing.T) {
}
}
}`, ar.Name)
assert.Equal(t, calculatePatch(r2, expectedPatch), patch)
assert.JSONEq(t, calculatePatch(r2, expectedPatch), patch)
}

// TestDoNotCreatePrePromotionAnalysisProgressedRollout ensures a pre-promotion analysis is not created after a Rollout
Expand Down Expand Up @@ -1771,7 +1771,7 @@ func TestDoNotCreatePrePromotionAnalysisRunOnNotReadyReplicaSet(t *testing.T) {
f.run(getKey(r2, t))

patch := f.getPatchedRollout(patchRolloutIndex)
assert.Equal(t, calculatePatch(r2, OnlyObservedGenerationPatch), patch)
assert.JSONEq(t, calculatePatch(r2, OnlyObservedGenerationPatch), patch)
}

func TestRolloutPrePromotionAnalysisBecomesInconclusive(t *testing.T) {
Expand Down Expand Up @@ -1841,7 +1841,7 @@ func TestRolloutPrePromotionAnalysisBecomesInconclusive(t *testing.T) {
}
}
}`, now, now)
assert.Equal(t, calculatePatch(r2, expectedPatch), patch)
assert.JSONEq(t, calculatePatch(r2, expectedPatch), patch)
}

func TestRolloutPrePromotionAnalysisSwitchServiceAfterSuccess(t *testing.T) {
Expand Down Expand Up @@ -1905,7 +1905,7 @@ func TestRolloutPrePromotionAnalysisSwitchServiceAfterSuccess(t *testing.T) {
"message": null
}
}`, rs2PodHash, rs2PodHash, rs2PodHash)
assert.Equal(t, calculatePatch(r2, expectedPatch), patch)
assert.JSONEq(t, calculatePatch(r2, expectedPatch), patch)
}

func TestRolloutPrePromotionAnalysisHonorAutoPromotionSeconds(t *testing.T) {
Expand Down Expand Up @@ -1971,7 +1971,7 @@ func TestRolloutPrePromotionAnalysisHonorAutoPromotionSeconds(t *testing.T) {
"message": null
}
}`, rs2PodHash, rs2PodHash, rs2PodHash)
assert.Equal(t, calculatePatch(r2, expectedPatch), patch)
assert.JSONEq(t, calculatePatch(r2, expectedPatch), patch)
}

func TestRolloutPrePromotionAnalysisDoNothingOnInconclusiveAnalysis(t *testing.T) {
Expand Down Expand Up @@ -2096,7 +2096,7 @@ func TestAbortRolloutOnErrorPrePromotionAnalysis(t *testing.T) {
now := timeutil.MetaNow().UTC().Format(time.RFC3339)
progressingFalseAborted, _ := newProgressingCondition(conditions.RolloutAbortedReason, r2, "")
newConditions := updateConditionsPatch(*r2, progressingFalseAborted)
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, now, newConditions, conditions.RolloutAbortedReason, progressingFalseAborted.Message)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, now, newConditions, conditions.RolloutAbortedReason, progressingFalseAborted.Message)), patch)
}

func TestCreatePostPromotionAnalysisRun(t *testing.T) {
Expand Down Expand Up @@ -2143,7 +2143,7 @@ func TestCreatePostPromotionAnalysisRun(t *testing.T) {
}
}
}`, ar.Name)
assert.Equal(t, calculatePatch(r2, expectedPatch), patch)
assert.JSONEq(t, calculatePatch(r2, expectedPatch), patch)
}

func TestRolloutPostPromotionAnalysisSuccess(t *testing.T) {
Expand Down Expand Up @@ -2199,7 +2199,7 @@ func TestRolloutPostPromotionAnalysisSuccess(t *testing.T) {
"message": null
}
}`, rs2PodHash)
assert.Equal(t, calculatePatch(r2, expectedPatch), patch)
assert.JSONEq(t, calculatePatch(r2, expectedPatch), patch)
}

// TestPostPromotionAnalysisRunHandleInconclusive ensures that the Rollout does not scale down a old ReplicaSet if
Expand Down Expand Up @@ -2264,7 +2264,7 @@ func TestPostPromotionAnalysisRunHandleInconclusive(t *testing.T) {
"message": "InconclusiveAnalysisRun"
}
}`)
assert.Equal(t, calculatePatch(r2, expectedPatch), patch)
assert.JSONEq(t, calculatePatch(r2, expectedPatch), patch)
}

func TestAbortRolloutOnErrorPostPromotionAnalysis(t *testing.T) {
Expand Down Expand Up @@ -2334,7 +2334,7 @@ func TestAbortRolloutOnErrorPostPromotionAnalysis(t *testing.T) {
now := timeutil.MetaNow().UTC().Format(time.RFC3339)
progressingFalseAborted, _ := newProgressingCondition(conditions.RolloutAbortedReason, r2, "")
newConditions := updateConditionsPatch(*r2, progressingFalseAborted)
assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, now, newConditions, conditions.RolloutAbortedReason, progressingFalseAborted.Message)), patch)
assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, now, newConditions, conditions.RolloutAbortedReason, progressingFalseAborted.Message)), patch)
}

func TestCreateAnalysisRunWithCustomAnalysisRunMetadataAndROCopyLabels(t *testing.T) {
Expand Down

0 comments on commit 202b97c

Please sign in to comment.