Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: prevent hot loop when fully promoted rollout is aborted #3064

Merged
merged 2 commits into from
Sep 29, 2023
Merged

fix: prevent hot loop when fully promoted rollout is aborted #3064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a903a88
fix: prevent hot loop when fully promoted rollout is aborted
jessesuen Sep 28, 2023
2c6e8b5
test: change expectations of abort tests
jessesuen Sep 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions experiments/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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