From 76cfc0e58892ed9759c1a732e243f75aed2fc6bd Mon Sep 17 00:00:00 2001 From: Zach Aller Date: Wed, 24 Aug 2022 16:16:43 -0500 Subject: [PATCH] fix: change completed condition so it only triggers on pod hash changes also adds an event for when it does changes. (#2203) * feat: add healthy event/condition and fix completed event/condition Signed-off-by: zachaller * fix unit tests Signed-off-by: zachaller * rename vars to make more sense and remove healthy event becase it will never be consistent Signed-off-by: zachaller * fix unit tests Signed-off-by: zachaller * possible fix for e2e Signed-off-by: zachaller * fix e2e Signed-off-by: zachaller * unit test for complete function Signed-off-by: zachaller * small cleanup and changes to not check generation Signed-off-by: zachaller * fix unit test and proper behavior Signed-off-by: zachaller * Fix e2e Signed-off-by: zachaller * rename and fix one unit test Signed-off-by: zachaller * fix unit tests Signed-off-by: zachaller * fix unit test Signed-off-by: zachaller * add seperate test for TestRolloutComplete Signed-off-by: zachaller * renames Signed-off-by: zachaller * fix e2e Signed-off-by: zachaller * Set Completed to false Signed-off-by: zachaller * Add event Signed-off-by: zachaller * fix e2e Signed-off-by: zachaller * refactor Signed-off-by: zachaller * Fix all but one unit test Signed-off-by: zachaller * fix last unit test Signed-off-by: zachaller * lint Signed-off-by: zachaller * cleanup Signed-off-by: zachaller * Rename Signed-off-by: zachaller * More renames Signed-off-by: zachaller * small comment change Signed-off-by: zachaller Signed-off-by: zachaller --- pkg/apis/rollouts/v1alpha1/types.go | 7 +- rollout/analysis_test.go | 81 +++++++++++++----- rollout/bluegreen_test.go | 127 +++++++++++++++++----------- rollout/canary_test.go | 38 ++++++--- rollout/controller_test.go | 76 ++++++++++++++--- rollout/experiment_test.go | 14 +-- rollout/service_test.go | 32 ++++--- rollout/sync.go | 47 +++++++--- rollout/trafficrouting_test.go | 2 + test/e2e/functional_test.go | 27 +++--- utils/conditions/conditions.go | 22 ++++- utils/conditions/rollouts_test.go | 28 +++++- 12 files changed, 358 insertions(+), 143 deletions(-) diff --git a/pkg/apis/rollouts/v1alpha1/types.go b/pkg/apis/rollouts/v1alpha1/types.go index b5124b2f2d..746758c60f 100644 --- a/pkg/apis/rollouts/v1alpha1/types.go +++ b/pkg/apis/rollouts/v1alpha1/types.go @@ -987,8 +987,13 @@ const ( RolloutReplicaFailure RolloutConditionType = "ReplicaFailure" // RolloutPaused means that rollout is in a paused state. It is still progressing at this point. RolloutPaused RolloutConditionType = "Paused" - // RolloutCompleted means that rollout is in a completed state. It is still progressing at this point. + // RolloutCompleted indicates that the rollout completed its update to the desired revision and is not in the middle + // of any update. Note that a Completed rollout could also be considered Progressing or Degraded, if its Pods become + // unavailable sometime after the update completes. RolloutCompleted RolloutConditionType = "Completed" + // RolloutHealthy means that rollout is in a completed state and is healthy. Which means that all the pods have been updated + // and are passing their health checks and are ready to serve traffic. + RolloutHealthy RolloutConditionType = "Healthy" ) // RolloutCondition describes the state of a rollout at a certain point. diff --git a/rollout/analysis_test.go b/rollout/analysis_test.go index ed21a4c848..46fe961fec 100644 --- a/rollout/analysis_test.go +++ b/rollout/analysis_test.go @@ -153,6 +153,8 @@ func TestCreateBackgroundAnalysisRun(t *testing.T) { conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completeCond, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completeCond) f.rolloutLister = append(f.rolloutLister, r2) f.analysisTemplateLister = append(f.analysisTemplateLister, at) @@ -212,6 +214,8 @@ func TestCreateBackgroundAnalysisRunWithTemplates(t *testing.T) { conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completeCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completeCondition) f.rolloutLister = append(f.rolloutLister, r2) f.analysisTemplateLister = append(f.analysisTemplateLister, at) @@ -272,6 +276,8 @@ func TestCreateBackgroundAnalysisRunWithClusterTemplates(t *testing.T) { conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) f.rolloutLister = append(f.rolloutLister, r2) f.clusterAnalysisTemplateLister = append(f.clusterAnalysisTemplateLister, cat) @@ -381,6 +387,8 @@ func TestCreateBackgroundAnalysisRunWithClusterTemplatesAndTemplate(t *testing.T conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) f.rolloutLister = append(f.rolloutLister, r2) f.clusterAnalysisTemplateLister = append(f.clusterAnalysisTemplateLister, cat) @@ -446,6 +454,8 @@ func TestCreateAnalysisRunWithCollision(t *testing.T) { conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) ar.Status.Phase = v1alpha1.AnalysisPhaseFailed @@ -514,6 +524,8 @@ func TestCreateAnalysisRunWithCollisionAndSemanticEquality(t *testing.T) { conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) f.rolloutLister = append(f.rolloutLister, r2) f.analysisRunLister = append(f.analysisRunLister, ar) @@ -573,6 +585,8 @@ func TestCreateAnalysisRunOnAnalysisStep(t *testing.T) { conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) f.rolloutLister = append(f.rolloutLister, r2) f.analysisTemplateLister = append(f.analysisTemplateLister, at) @@ -768,6 +782,8 @@ func TestDoNothingWithAnalysisRunsWhileBackgroundAnalysisRunRunning(t *testing.T conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) r2.Status.Canary.CurrentBackgroundAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ Name: ar.Name, Status: v1alpha1.AnalysisPhaseRunning, @@ -816,6 +832,8 @@ func TestDoNothingWhileStepBasedAnalysisRunRunning(t *testing.T) { conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) r2.Status.Canary.CurrentStepAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ Name: ar.Name, Status: v1alpha1.AnalysisPhaseRunning, @@ -866,6 +884,8 @@ func TestCancelOlderAnalysisRuns(t *testing.T) { conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) r2.Status.Canary.CurrentStepAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ Name: ar.Name, Status: "", @@ -933,6 +953,8 @@ func TestDeleteAnalysisRunsWithNoMatchingRS(t *testing.T) { conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) r2.Status.Canary.CurrentStepAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ Name: ar.Name, } @@ -1059,7 +1081,7 @@ func TestIncrementStepAfterSuccessfulAnalysisRun(t *testing.T) { "conditions": %s } }` - condition := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs2, false, "") + condition := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs2, false, "", false) assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition)), patch) } @@ -1128,7 +1150,7 @@ func TestPausedOnInconclusiveBackgroundAnalysisRun(t *testing.T) { "message": "%s" } }` - condition := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "") + condition := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "", false) assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, v1alpha1.PauseReasonInconclusiveAnalysis, now, v1alpha1.PauseReasonInconclusiveAnalysis)), patch) } @@ -1192,7 +1214,7 @@ func TestPausedStepAfterInconclusiveAnalysisRun(t *testing.T) { "message": "%s" } }` - condition := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "") + condition := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "", false) assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, v1alpha1.PauseReasonInconclusiveAnalysis, now, v1alpha1.PauseReasonInconclusiveAnalysis)), patch) } @@ -1258,7 +1280,7 @@ func TestErrorConditionAfterErrorAnalysisRunStep(t *testing.T) { }` now := timeutil.MetaNow().UTC().Format(time.RFC3339) errmsg := fmt.Sprintf(conditions.RolloutAbortedMessage, 2) + ": " + ar.Status.Message - condition := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, errmsg) + condition := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, errmsg, false) expectedPatch = fmt.Sprintf(expectedPatch, condition, now, errmsg) assert.Equal(t, calculatePatch(r2, expectedPatch), patch) } @@ -1333,7 +1355,7 @@ func TestErrorConditionAfterErrorAnalysisRunBackground(t *testing.T) { } }` errmsg := fmt.Sprintf(conditions.RolloutAbortedMessage, 2) - condition := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, "") + 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) @@ -1386,7 +1408,7 @@ func TestCancelAnalysisRunsWhenAborted(t *testing.T) { assert.True(t, f.verifyPatchedAnalysisRun(cancelOldAr, olderAr)) assert.True(t, f.verifyPatchedAnalysisRun(cancelCurrentAr, ar)) patch := f.getPatchedRollout(patchIndex) - newConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, "") + newConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, "", false) expectedPatch := `{ "status": { "conditions": %s, @@ -1488,6 +1510,9 @@ func TestDoNotCreateBackgroundAnalysisRunAfterInconclusiveRun(t *testing.T) { availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) + f.rolloutLister = append(f.rolloutLister, r2) f.analysisTemplateLister = append(f.analysisTemplateLister, at) f.objects = append(f.objects, r2, at) @@ -1586,13 +1611,16 @@ func TestCreatePrePromotionAnalysisRun(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true, false) progressingCondition, _ := newProgressingCondition(conditions.RolloutPausedReason, r2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) pausedCondition, _ := newPausedCondition(true) conditions.SetRolloutCondition(&r2.Status, pausedCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) + previewSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash} previewSvc := newService("preview", 80, previewSelector, r2) activeSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash} @@ -1650,7 +1678,7 @@ func TestDoNotCreatePrePromotionAnalysisAfterPromotionRollout(t *testing.T) { f.analysisTemplateLister = append(f.analysisTemplateLister, at) f.objects = append(f.objects, at) - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 1, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 1, 1, false, true, true) r2.Status.ObservedGeneration = strconv.Itoa(int(r2.Generation)) f.rolloutLister = append(f.rolloutLister, r2) @@ -1661,7 +1689,7 @@ func TestDoNotCreatePrePromotionAnalysisAfterPromotionRollout(t *testing.T) { f.run(getKey(r2, t)) - newConditions := generateConditionsPatchWithComplete(true, conditions.NewRSAvailableReason, rs2, true, "", true) + newConditions := generateConditionsPatchWithHealthy(true, conditions.NewRSAvailableReason, rs2, true, "", true, true) expectedPatch := fmt.Sprintf(`{ "status":{ "conditions":%s @@ -1723,7 +1751,7 @@ func TestDoNotCreatePrePromotionAnalysisRunOnNotReadyReplicaSet(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 2, 2, 4, 2, false, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 2, 2, 4, 2, false, true, false) activeSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash} activeSvc := newService("active", 80, activeSelector, r2) @@ -1766,7 +1794,7 @@ func TestRolloutPrePromotionAnalysisBecomesInconclusive(t *testing.T) { rs2 := newReplicaSetWithStatus(r2, 1, 1) rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true, false) r2.Status.BlueGreen.PrePromotionAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ Name: ar.Name, Status: v1alpha1.AnalysisPhaseRunning, @@ -1777,6 +1805,9 @@ func TestRolloutPrePromotionAnalysisBecomesInconclusive(t *testing.T) { pausedCondition, _ := newPausedCondition(true) conditions.SetRolloutCondition(&r2.Status, pausedCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) + activeSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash} activeSvc := newService("active", 80, activeSelector, r2) @@ -1834,7 +1865,7 @@ func TestRolloutPrePromotionAnalysisSwitchServiceAfterSuccess(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true, false) r2.Status.BlueGreen.PrePromotionAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ Name: ar.Name, Status: v1alpha1.AnalysisPhaseRunning, @@ -1903,7 +1934,7 @@ func TestRolloutPrePromotionAnalysisHonorAutoPromotionSeconds(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true, false) now := metav1.NewTime(timeutil.MetaNow().Add(-10 * time.Second)) r2.Status.PauseConditions[0].StartTime = now progressingCondition, _ := newProgressingCondition(conditions.RolloutPausedReason, r2, "") @@ -1966,7 +1997,7 @@ func TestRolloutPrePromotionAnalysisDoNothingOnInconclusiveAnalysis(t *testing.T rs2 := newReplicaSetWithStatus(r2, 1, 1) rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true, false) inconclusivePauseCondition := v1alpha1.PauseCondition{ Reason: v1alpha1.PauseReasonInconclusiveAnalysis, StartTime: timeutil.MetaNow(), @@ -2021,12 +2052,15 @@ func TestAbortRolloutOnErrorPrePromotionAnalysis(t *testing.T) { rs2 := newReplicaSetWithStatus(r2, 1, 1) rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true, false) progressingCondition, _ := newProgressingCondition(conditions.RolloutPausedReason, r2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) pausedCondition, _ := newPausedCondition(true) conditions.SetRolloutCondition(&r2.Status, pausedCondition) + + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) r2.Status.Phase, r2.Status.Message = rolloututil.CalculateRolloutPhase(r2.Spec, r2.Status) activeSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash} @@ -2083,7 +2117,7 @@ func TestCreatePostPromotionAnalysisRun(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 1, 1, 2, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 1, 1, 2, 1, false, true, false) activeSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash} activeSvc := newService("active", 80, activeSelector, r2) @@ -2136,11 +2170,14 @@ func TestRolloutPostPromotionAnalysisSuccess(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 1, 1, 1, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 1, 1, 1, 1, false, true, false) activeSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash} activeSvc := newService("active", 80, activeSelector, r2) + cond, _ := newCompletedCondition(true) + conditions.SetRolloutCondition(&r2.Status, cond) + f.objects = append(f.objects, r2, at, ar) f.kubeobjects = append(f.kubeobjects, activeSvc, rs1, rs2) f.rolloutLister = append(f.rolloutLister, r2) @@ -2190,7 +2227,7 @@ func TestPostPromotionAnalysisRunHandleInconclusive(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 1, 1, 2, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 1, 1, 2, 1, false, true, false) r2.Status.PauseConditions = []v1alpha1.PauseCondition{{ Reason: v1alpha1.PauseReasonInconclusiveAnalysis, StartTime: timeutil.MetaNow(), @@ -2201,6 +2238,9 @@ func TestPostPromotionAnalysisRunHandleInconclusive(t *testing.T) { pausedCondition, _ := newPausedCondition(true) conditions.SetRolloutCondition(&r2.Status, pausedCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) + activeSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash} activeSvc := newService("active", 80, activeSelector, r2) @@ -2251,13 +2291,16 @@ func TestAbortRolloutOnErrorPostPromotionAnalysis(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 1, 1, 2, 1, true, true, false) progressingCondition, _ := newProgressingCondition(conditions.RolloutPausedReason, r2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) pausedCondition, _ := newPausedCondition(true) conditions.SetRolloutCondition(&r2.Status, pausedCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) + activeSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash} activeSvc := newService("active", 80, activeSelector, r2) diff --git a/rollout/bluegreen_test.go b/rollout/bluegreen_test.go index 5cae668da3..8ad0b1f838 100644 --- a/rollout/bluegreen_test.go +++ b/rollout/bluegreen_test.go @@ -38,15 +38,17 @@ func newBlueGreenRollout(name string, replicas int, revisionHistoryLimit *int32, return rollout } -func TestBlueGreenComplateRolloutRestart(t *testing.T) { +func TestBlueGreenCompletedRolloutRestart(t *testing.T) { f := newFixture(t) defer f.Close() r := newBlueGreenRollout("foo", 1, nil, "active", "preview") r.Status.Conditions = []v1alpha1.RolloutCondition{} - completedCond := conditions.NewRolloutCondition(v1alpha1.RolloutCompleted, corev1.ConditionTrue, conditions.RolloutCompletedReason, conditions.RolloutCompletedReason) - conditions.SetRolloutCondition(&r.Status, *completedCond) + completedHealthyCond := conditions.NewRolloutCondition(v1alpha1.RolloutHealthy, corev1.ConditionFalse, conditions.RolloutHealthyReason, conditions.RolloutNotHealthyMessage) + conditions.SetRolloutCondition(&r.Status, *completedHealthyCond) + completedCond, _ := newCompletedCondition(true) + conditions.SetRolloutCondition(&r.Status, completedCond) f.rolloutLister = append(f.rolloutLister, r) f.objects = append(f.objects, r) @@ -57,7 +59,7 @@ func TestBlueGreenComplateRolloutRestart(t *testing.T) { rs := newReplicaSet(r, 1) rsPodHash := rs.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - generatedConditions := generateConditionsPatchWithComplete(false, conditions.ReplicaSetNotAvailableReason, rs, false, "", false) + generatedConditions := generateConditionsPatchWithCompletedHealthy(false, conditions.ReplicaSetUpdatedReason, rs, false, "", false, true) f.expectCreateReplicaSetAction(rs) servicePatchIndex := f.expectPatchServiceAction(previewSvc, rsPodHash) @@ -107,7 +109,7 @@ func TestBlueGreenCreatesReplicaSet(t *testing.T) { rs := newReplicaSet(r, 1) rsPodHash := rs.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - generatedConditions := generateConditionsPatch(false, conditions.ReplicaSetUpdatedReason, rs, false, "") + generatedConditions := generateConditionsPatchWithCompleted(false, conditions.ReplicaSetUpdatedReason, rs, false, "", true) f.expectCreateReplicaSetAction(rs) servicePatchIndex := f.expectPatchServiceAction(previewSvc, rsPodHash) @@ -271,7 +273,7 @@ func TestBlueGreenHandlePause(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 2, 2, 4, 2, false, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 2, 2, 4, 2, false, true, false) activeSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash} activeSvc := newService("active", 80, activeSelector, r2) @@ -302,7 +304,7 @@ func TestBlueGreenHandlePause(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true, false) previewSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash} previewSvc := newService("preview", 80, previewSelector, r2) activeSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash} @@ -350,7 +352,7 @@ func TestBlueGreenHandlePause(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true, false) previewSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash} previewSvc := newService("preview", 80, previewSelector, r2) @@ -373,7 +375,7 @@ func TestBlueGreenHandlePause(t *testing.T) { "conditions": %s } }` - addedConditions := generateConditionsPatchWithPause(true, conditions.RolloutPausedReason, rs2, true, "", true) + addedConditions := generateConditionsPatchWithPause(true, conditions.RolloutPausedReason, rs2, true, "", true, false) assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, addedConditions)), patch) }) @@ -390,12 +392,15 @@ func TestBlueGreenHandlePause(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true, false) progressingCondition, _ := newProgressingCondition(conditions.RolloutPausedReason, r2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) pausedCondition, _ := newPausedCondition(true) conditions.SetRolloutCondition(&r2.Status, pausedCondition) + + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) r2.Status.Phase, r2.Status.Message = rolloututil.CalculateRolloutPhase(r2.Spec, r2.Status) previewSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash} @@ -433,7 +438,7 @@ func TestBlueGreenHandlePause(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true, false) now := timeutil.MetaNow() r2.Status.PauseConditions = append(r2.Status.PauseConditions, v1alpha1.PauseCondition{ Reason: v1alpha1.PauseReasonInconclusiveAnalysis, @@ -479,12 +484,15 @@ func TestBlueGreenHandlePause(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true, false) progressingCondition, _ := newProgressingCondition(conditions.RolloutPausedReason, r2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) pausedCondition, _ := newPausedCondition(true) conditions.SetRolloutCondition(&r2.Status, pausedCondition) + + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) r2.Status.Phase, r2.Status.Message = rolloututil.CalculateRolloutPhase(r2.Spec, r2.Status) previewSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash} @@ -517,7 +525,7 @@ func TestBlueGreenHandlePause(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true, false) now := timeutil.MetaNow() before := metav1.NewTime(now.Add(-1 * time.Minute)) r2.Status.PauseConditions[0].StartTime = before @@ -575,7 +583,7 @@ func TestBlueGreenHandlePause(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true, false) now := timeutil.MetaNow() before := metav1.NewTime(now.Add(-1 * time.Minute)) r2.Status.PauseConditions[0].StartTime = before @@ -586,6 +594,9 @@ func TestBlueGreenHandlePause(t *testing.T) { pausedCondition, _ := newPausedCondition(true) conditions.SetRolloutCondition(&r2.Status, pausedCondition) + + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) r2.Status.Phase, r2.Status.Message = rolloututil.CalculateRolloutPhase(r2.Spec, r2.Status) activeSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash} @@ -619,7 +630,7 @@ func TestBlueGreenHandlePause(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true, false) r2.Spec.Strategy.BlueGreen.ScaleDownDelaySeconds = pointer.Int32Ptr(10) progressingCondition, _ := newProgressingCondition(conditions.NewReplicaSetReason, rs2, "") @@ -635,7 +646,7 @@ func TestBlueGreenHandlePause(t *testing.T) { servicePatchIndex := f.expectPatchServiceAction(activeSvc, rs2PodHash) - generatedConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs2, true, "") + generatedConditions := generateConditionsPatchWithCompleted(true, conditions.ReplicaSetUpdatedReason, rs2, true, "", true) newSelector := metav1.FormatLabelSelector(rs2.Spec.Selector) expectedPatchWithoutSubs := `{ "status": { @@ -670,10 +681,13 @@ func TestBlueGreenHandlePause(t *testing.T) { rs2 := newReplicaSetWithStatus(r2, 1, 1) rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true, false) progressingCondition, _ := newProgressingCondition(conditions.NewReplicaSetReason, rs2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) + + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) activeSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash} activeSvc := newService("active", 80, activeSelector, r2) @@ -713,7 +727,7 @@ func TestBlueGreenHandlePause(t *testing.T) { rs1 := newReplicaSetWithStatus(r1, 1, 1) rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r1 = updateBlueGreenRolloutStatus(r1, "", "", "", 1, 1, 1, 1, false, false) + r1 = updateBlueGreenRolloutStatus(r1, "", "", "", 1, 1, 1, 1, false, false, false) activeSelector := map[string]string{"foo": "bar"} @@ -739,7 +753,7 @@ func TestBlueGreenHandlePause(t *testing.T) { } }` - generateConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs1, false, "") + generateConditions := generateConditionsPatchWithCompleted(true, conditions.ReplicaSetUpdatedReason, rs1, false, "", true) newSelector := metav1.FormatLabelSelector(rs1.Spec.Selector) expectedPatch := calculatePatch(r1, fmt.Sprintf(expectedPatchWithoutSubs, rs1PodHash, rs1PodHash, generateConditions, newSelector)) patchRolloutIndex := f.expectPatchRolloutActionWithPatch(r1, expectedPatch) @@ -765,8 +779,10 @@ func TestBlueGreenHandlePause(t *testing.T) { rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] r2.Spec.Strategy.BlueGreen.ScaleDownDelaySeconds = pointer.Int32Ptr(10) - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true, false) r2.Status.ControllerPause = true + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) pausedCondition, _ := newProgressingCondition(conditions.RolloutPausedReason, rs2, "") conditions.SetRolloutCondition(&r2.Status, pausedCondition) @@ -788,7 +804,10 @@ func TestBlueGreenHandlePause(t *testing.T) { f.verifyPatchedService(servicePatchIndex, rs2PodHash, "") unpausePatch := f.getPatchedRollout(unpausePatchIndex) - unpauseConditions := generateConditionsPatch(true, conditions.RolloutResumedReason, rs2, true, "") + _, availableCondition := newAvailableCondition(true) + _, progressingCondition := newProgressingCondition(conditions.RolloutResumedReason, rs2, "") + _, compCondition := newCompletedCondition(false) + unpauseConditions := fmt.Sprintf("[%s, %s, %s]", availableCondition, compCondition, progressingCondition) expectedUnpausePatch := `{ "status": { "conditions": %s @@ -796,7 +815,7 @@ func TestBlueGreenHandlePause(t *testing.T) { }` assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedUnpausePatch, unpauseConditions)), unpausePatch) - generatedConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs2, true, "") + generatedConditions := generateConditionsPatchWithCompleted(true, conditions.ReplicaSetUpdatedReason, rs2, true, "", true) expected2ndPatchWithoutSubs := `{ "status": { "blueGreen": { @@ -835,7 +854,7 @@ func TestBlueGreenAddScaleDownDelayToPreviousActiveReplicaSet(t *testing.T) { f.replicaSetLister = append(f.replicaSetLister, rs1, rs2) r2.Spec.Strategy.BlueGreen.ScaleDownDelaySeconds = pointer.Int32Ptr(10) - r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true, false) f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2) f.serviceLister = append(f.serviceLister, s) @@ -858,7 +877,7 @@ func TestBlueGreenAddScaleDownDelayToPreviousActiveReplicaSet(t *testing.T) { } }` newSelector := metav1.FormatLabelSelector(rs2.Spec.Selector) - expectedCondition := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs2, true, "") + expectedCondition := generateConditionsPatchWithCompleted(true, conditions.ReplicaSetUpdatedReason, rs2, true, "", true) expectedPatch := calculatePatch(r2, fmt.Sprintf(expectedPatchWithoutSubs, rs2PodHash, rs2PodHash, expectedCondition, newSelector)) assert.Equal(t, expectedPatch, patch) } @@ -879,7 +898,7 @@ func TestBlueGreenRolloutStatusHPAStatusFieldsActiveSelectorSet(t *testing.T) { previewSvc := newService("preview", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash}, r2) activeSvc := newService("active", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash}, r2) - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 0, 0, 0, 0, true, false) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 0, 0, 0, 0, true, false, false) r2.Status.Selector = "" progressingCondition, _ := newProgressingCondition(conditions.RolloutPausedReason, rs2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) @@ -940,6 +959,7 @@ func TestBlueGreenRolloutStatusHPAStatusFieldsNoActiveSelector(t *testing.T) { assert.Len(t, f.client.Actions(), 1) result := f.client.Actions()[0].(core.PatchAction).GetPatch() _, availableStr := newAvailableCondition(false) + _, compCond := newCompletedCondition(true) expectedPatchWithoutSub := `{ "status":{ "HPAReplicas":1, @@ -947,11 +967,11 @@ func TestBlueGreenRolloutStatusHPAStatusFieldsNoActiveSelector(t *testing.T) { "availableReplicas": 1, "updatedReplicas":1, "replicas":1, - "conditions":[%s, %s], + "conditions":[%s, %s, %s], "selector":"foo=bar" } }` - expectedPatch := calculatePatch(ro, fmt.Sprintf(expectedPatchWithoutSub, progressingConditionStr, availableStr)) + expectedPatch := calculatePatch(ro, fmt.Sprintf(expectedPatchWithoutSub, progressingConditionStr, availableStr, compCond)) assert.Equal(t, expectedPatch, string(result)) } @@ -973,7 +993,7 @@ func TestBlueGreenRolloutScaleUpdateActiveRS(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 1, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 1, 1, false, true, false) f.objects = append(f.objects, r2) previewSvc := newService("preview", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash}, r2) activeSvc := newService("active", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash}, r2) @@ -1006,7 +1026,7 @@ func TestPreviewReplicaCountHandleScaleUpPreviewCheckPoint(t *testing.T) { activeSvc := newService("active", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash}, r2) - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 3, 3, 8, 5, false, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 3, 3, 8, 5, false, true, false) f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2) f.kubeobjects = append(f.kubeobjects, activeSvc) @@ -1038,7 +1058,7 @@ func TestPreviewReplicaCountHandleScaleUpPreviewCheckPoint(t *testing.T) { activeSvc := newService("active", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash}, r2) - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 5, 5, 8, 5, false, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 5, 5, 8, 5, false, true, false) r2.Status.BlueGreen.ScaleUpPreviewCheckPoint = true f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2) @@ -1069,7 +1089,7 @@ func TestPreviewReplicaCountHandleScaleUpPreviewCheckPoint(t *testing.T) { activeSvc := newService("active", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash}, r2) - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 5, 5, 8, 5, false, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 5, 5, 8, 5, false, true, false) r2.Status.BlueGreen.ScaleUpPreviewCheckPoint = true f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2) @@ -1103,7 +1123,7 @@ func TestBlueGreenRolloutIgnoringScalingUsePreviewRSCount(t *testing.T) { previewSvc := newService("preview", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash}, r2) activeSvc := newService("active", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash}, r2) - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 2, 1, 1, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 2, 1, 1, 1, false, true, true) // Scaling up the rollout r2.Spec.Replicas = pointer.Int32Ptr(2) f.rolloutLister = append(f.rolloutLister, r2) @@ -1136,7 +1156,7 @@ func TestBlueGreenRolloutCompleted(t *testing.T) { s := newService("bar", 80, serviceSelector, r2) f.kubeobjects = append(f.kubeobjects, s) - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 1, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 1, 1, false, true, true) r2.Status.ObservedGeneration = strconv.Itoa(int(r2.Generation)) f.rolloutLister = append(f.rolloutLister, r2) @@ -1147,7 +1167,7 @@ func TestBlueGreenRolloutCompleted(t *testing.T) { f.run(getKey(r2, t)) - newConditions := generateConditionsPatchWithComplete(true, conditions.NewRSAvailableReason, rs2, true, "", true) + newConditions := generateConditionsPatchWithHealthy(true, conditions.NewRSAvailableReason, rs2, true, "", true, true) expectedPatch := fmt.Sprintf(`{ "status":{ "conditions":%s @@ -1162,7 +1182,7 @@ func TestBlueGreenRolloutCompletedFalse(t *testing.T) { defer f.Close() r1 := newBlueGreenRollout("foo", 1, nil, "bar", "") - completedCondition, _ := newCompletedCondition(true) + completedCondition, _ := newHealthyCondition(true) conditions.SetRolloutCondition(&r1.Status, completedCondition) r2 := bumpVersion(r1) @@ -1182,7 +1202,7 @@ func TestBlueGreenRolloutCompletedFalse(t *testing.T) { s := newService("bar", 80, serviceSelector, r2) f.kubeobjects = append(f.kubeobjects, s) - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 1, 1, true, false) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 1, 1, true, false, false) r2.Status.ObservedGeneration = strconv.Itoa(int(r2.Generation)) f.rolloutLister = append(f.rolloutLister, r2) @@ -1197,8 +1217,8 @@ func TestBlueGreenRolloutCompletedFalse(t *testing.T) { err := json.Unmarshal([]byte(patch), &rolloutPatch) assert.NoError(t, err) - index := len(rolloutPatch.Status.Conditions) - 2 - assert.Equal(t, v1alpha1.RolloutCompleted, rolloutPatch.Status.Conditions[index].Type) + index := len(rolloutPatch.Status.Conditions) - 3 + assert.Equal(t, v1alpha1.RolloutHealthy, rolloutPatch.Status.Conditions[index].Type) assert.Equal(t, corev1.ConditionFalse, rolloutPatch.Status.Conditions[index].Status) } @@ -1220,7 +1240,7 @@ func TestBlueGreenUnableToReadScaleDownAt(t *testing.T) { f.kubeobjects = append(f.kubeobjects, s, rs1, rs2) f.replicaSetLister = append(f.replicaSetLister, rs1, rs2) - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 2, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 2, 1, false, true, true) f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2) f.serviceLister = append(f.serviceLister, s) @@ -1257,7 +1277,7 @@ func TestBlueGreenNotReadyToScaleDownOldReplica(t *testing.T) { f.kubeobjects = append(f.kubeobjects, s, rs1, rs2) f.replicaSetLister = append(f.replicaSetLister, rs1, rs2) - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 2, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 2, 1, false, true, true) f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2) f.serviceLister = append(f.serviceLister, s) @@ -1290,7 +1310,7 @@ func TestBlueGreenReadyToScaleDownOldReplica(t *testing.T) { f.kubeobjects = append(f.kubeobjects, s, rs1, rs2) f.replicaSetLister = append(f.replicaSetLister, rs1, rs2) - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 2, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 2, 1, false, true, true) f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2) f.serviceLister = append(f.serviceLister, s) @@ -1335,7 +1355,7 @@ func TestFastRollback(t *testing.T) { r2.Status.CurrentPodHash = rs1PodHash rs1.Annotations[annotations.RevisionAnnotation] = "3" - r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, false, true, true) f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2) f.serviceLister = append(f.serviceLister, s) @@ -1373,7 +1393,7 @@ func TestBlueGreenScaleDownLimit(t *testing.T) { f.kubeobjects = append(f.kubeobjects, s, rs1, rs2, rs3) f.replicaSetLister = append(f.replicaSetLister, rs1, rs2, rs3) - r3 = updateBlueGreenRolloutStatus(r3, "", rs3PodHash, rs3PodHash, 1, 1, 3, 1, false, true) + r3 = updateBlueGreenRolloutStatus(r3, "", rs3PodHash, rs3PodHash, 1, 1, 3, 1, false, true, true) f.rolloutLister = append(f.rolloutLister, r3) f.objects = append(f.objects, r3) f.serviceLister = append(f.serviceLister, s) @@ -1412,7 +1432,7 @@ func TestBlueGreenAbort(t *testing.T) { f.kubeobjects = append(f.kubeobjects, s, rs1, rs2) f.replicaSetLister = append(f.replicaSetLister, rs1, rs2) - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 1, 1, 2, 1, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 1, 1, 2, 1, false, true, true) f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2) f.serviceLister = append(f.serviceLister, s) @@ -1420,7 +1440,7 @@ func TestBlueGreenAbort(t *testing.T) { f.expectPatchServiceAction(s, rs1PodHash) patchIndex := f.expectPatchRolloutAction(r2) f.run(getKey(r2, t)) - expectedConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, true, "") + expectedConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, true, "", false) expectedPatch := fmt.Sprintf(`{ "status": { "blueGreen": { @@ -1449,7 +1469,7 @@ func TestBlueGreenHandlePauseAutoPromoteWithConditions(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2 = updateBlueGreenRolloutStatus(r2, rs2PodHash, rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true, true) now := timeutil.MetaNow() before := metav1.NewTime(now.Add(-1 * time.Minute)) r2.Status.PauseConditions[0].StartTime = before @@ -1463,6 +1483,8 @@ func TestBlueGreenHandlePauseAutoPromoteWithConditions(t *testing.T) { availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + //completedCondition, _ := newCompletedCondition(true) + //conditions.SetRolloutCondition(&r2.Status, completedCondition) r2.Status.Phase, r2.Status.Message = rolloututil.CalculateRolloutPhase(r2.Spec, r2.Status) activeSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash} @@ -1481,7 +1503,7 @@ func TestBlueGreenHandlePauseAutoPromoteWithConditions(t *testing.T) { "blueGreen": { "activeSelector": "%s" }, - "conditions": [%s, %s, %s], + "conditions": [%s, %s, %s, %s], "stableRS": "%s", "pauseConditions": null, "controllerPause": null, @@ -1495,9 +1517,12 @@ func TestBlueGreenHandlePauseAutoPromoteWithConditions(t *testing.T) { updatedProgressingCond, _ := newProgressingCondition(conditions.ReplicaSetUpdatedReason, rs2, fmt.Sprintf("ReplicaSet \"%s\" is progressing.", rs2.Name)) progressingCondBytes, err := json.Marshal(updatedProgressingCond) assert.Nil(t, err) - pausedCondBytes, err := json.Marshal(r2.Status.Conditions[2]) + pausedCondBytes, err := json.Marshal(r2.Status.Conditions[3]) + assert.Nil(t, err) + completeCond, _ := newCompletedCondition(true) + completeCondBytes, err := json.Marshal(completeCond) assert.Nil(t, err) - expectedPatch := calculatePatch(r2, fmt.Sprintf(expectedPatchWithoutSubs, rs2PodHash, string(availableCondBytes), string(pausedCondBytes), string(progressingCondBytes), rs2PodHash, rs2PodHash)) + expectedPatch := calculatePatch(r2, fmt.Sprintf(expectedPatchWithoutSubs, rs2PodHash, string(availableCondBytes), string(completeCondBytes), string(pausedCondBytes), string(progressingCondBytes), rs2PodHash, rs2PodHash)) f.expectPatchServiceAction(activeSvc, rs2PodHash) patchRolloutIndex := f.expectPatchRolloutActionWithPatch(r2, expectedPatch) f.run(getKey(r2, t)) @@ -1521,8 +1546,8 @@ func TestBlueGreenAddScaleDownDelay(t *testing.T) { rs2 := newReplicaSetWithStatus(r2, 1, 1) rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] r2.Status.ObservedGeneration = strconv.Itoa(int(r2.Generation)) - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 2, 1, false, true) - completedCondition, _ := newCompletedCondition(true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 2, 1, false, true, true) + completedCondition, _ := newHealthyCondition(true) conditions.SetRolloutCondition(&r2.Status, completedCondition) progressingCondition, _ := newProgressingCondition(conditions.NewRSAvailableReason, rs2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) diff --git a/rollout/canary_test.go b/rollout/canary_test.go index 8cc1c3b0df..56445f3978 100644 --- a/rollout/canary_test.go +++ b/rollout/canary_test.go @@ -178,7 +178,7 @@ func TestCanaryRolloutEnterPauseState(t *testing.T) { } }` - conditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "") + conditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "", false) now := timeutil.MetaNow().UTC().Format(time.RFC3339) expectedPatchWithoutObservedGen := fmt.Sprintf(expectedPatchTemplate, v1alpha1.PauseReasonCanaryPauseStep, now, conditions, v1alpha1.PauseReasonCanaryPauseStep) expectedPatch := calculatePatch(r2, expectedPatchWithoutObservedGen) @@ -239,6 +239,9 @@ func TestCanaryRolloutUpdatePauseConditionWhilePaused(t *testing.T) { availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) + rs1 := newReplicaSetWithStatus(r1, 10, 10) rs2 := newReplicaSetWithStatus(r2, 0, 0) @@ -337,7 +340,7 @@ func TestCanaryRolloutIncrementStepAfterUnPaused(t *testing.T) { "currentStepIndex": 1 } }` - generatedConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs2, false, "") + generatedConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs2, false, "", false) expectedPatch := calculatePatch(r2, fmt.Sprintf(expectedPatchTemplate, generatedConditions)) assert.Equal(t, expectedPatch, patch) } @@ -379,7 +382,7 @@ func TestCanaryRolloutUpdateStatusWhenAtEndOfSteps(t *testing.T) { } }` - expectedPatch := fmt.Sprintf(expectedPatchWithoutStableRS, expectedStableRS, generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs2, false, "")) + expectedPatch := fmt.Sprintf(expectedPatchWithoutStableRS, expectedStableRS, generateConditionsPatchWithCompleted(true, conditions.ReplicaSetUpdatedReason, rs2, false, "", true)) assert.Equal(t, calculatePatch(r2, expectedPatch), patch) } @@ -421,7 +424,7 @@ func TestResetCurrentStepIndexOnStepChange(t *testing.T) { "conditions": %s } }` - newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "") + newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "", false) expectedPatch := fmt.Sprintf(expectedPatchWithoutPodHash, expectedCurrentPodHash, expectedCurrentStepHash, newConditions) assert.Equal(t, calculatePatch(r2, expectedPatch), patch) @@ -462,7 +465,7 @@ func TestResetCurrentStepIndexOnPodSpecChange(t *testing.T) { "conditions": %s } }` - newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "") + newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "", false) expectedPatch := fmt.Sprintf(expectedPatchWithoutPodHash, expectedCurrentPodHash, newConditions) assert.Equal(t, calculatePatch(r2, expectedPatch), patch) @@ -502,7 +505,7 @@ func TestCanaryRolloutCreateFirstReplicasetNoSteps(t *testing.T) { } }` - newConditions := generateConditionsPatch(false, conditions.ReplicaSetUpdatedReason, rs, false, "") + newConditions := generateConditionsPatchWithCompleted(false, conditions.ReplicaSetUpdatedReason, rs, false, "", true) assert.Equal(t, calculatePatch(r, fmt.Sprintf(expectedPatch, newConditions)), patch) } @@ -542,7 +545,7 @@ func TestCanaryRolloutCreateFirstReplicasetWithSteps(t *testing.T) { "conditions": %s } }` - expectedPatch := fmt.Sprintf(expectedPatchWithSub, generateConditionsPatch(false, conditions.ReplicaSetUpdatedReason, rs, false, "")) + expectedPatch := fmt.Sprintf(expectedPatchWithSub, generateConditionsPatchWithCompleted(false, conditions.ReplicaSetUpdatedReason, rs, false, "", true)) assert.Equal(t, calculatePatch(r, expectedPatch), patch) } @@ -840,7 +843,7 @@ func TestRollBackToStable(t *testing.T) { "conditions": %s } }` - newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs1, false, "") + newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs1, false, "", true) expectedPatch := fmt.Sprintf(expectedPatchWithoutSub, hash.ComputePodTemplateHash(&r2.Spec.Template, r2.Status.CollisionCount), newConditions) patch := f.getPatchedRollout(patchIndex) assert.Equal(t, calculatePatch(r2, expectedPatch), patch) @@ -883,7 +886,7 @@ func TestGradualShiftToNewStable(t *testing.T) { "conditions": %s } }` - newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "") + newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "", false) expectedPatch := fmt.Sprintf(expectedPatchWithoutSub, newConditions) patch := f.getPatchedRollout(patchIndex) assert.Equal(t, calculatePatch(r2, expectedPatch), patch) @@ -931,7 +934,7 @@ func TestRollBackToStableAndStepChange(t *testing.T) { }` newPodHash := hash.ComputePodTemplateHash(&r2.Spec.Template, r2.Status.CollisionCount) newStepHash := conditions.ComputeStepHash(r2) - newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs1, false, "") + newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs1, false, "", true) expectedPatch := fmt.Sprintf(expectedPatchWithoutSub, newPodHash, newStepHash, newConditions) patch := f.getPatchedRollout(patchIndex) assert.Equal(t, calculatePatch(r2, expectedPatch), patch) @@ -969,7 +972,7 @@ func TestCanaryRolloutIncrementStepIfSetWeightsAreCorrect(t *testing.T) { "conditions": %s } }` - newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs3, false, "") + newConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs3, false, "", false) assert.Equal(t, calculatePatch(r3, fmt.Sprintf(expectedPatch, newConditions)), patch) } @@ -1005,6 +1008,9 @@ func TestSyncRolloutWaitAddToQueue(t *testing.T) { availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) + r2.Status.ObservedGeneration = strconv.Itoa(int(r2.Generation)) f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2) @@ -1053,6 +1059,9 @@ func TestSyncRolloutIgnoreWaitOutsideOfReconciliationPeriod(t *testing.T) { availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) + f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2) @@ -1617,6 +1626,9 @@ func TestHandleNilNewRSOnScaleAndImageChange(t *testing.T) { availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) + f.kubeobjects = append(f.kubeobjects, rs1) f.replicaSetLister = append(f.replicaSetLister, rs1) f.rolloutLister = append(f.rolloutLister, r2) @@ -1672,7 +1684,7 @@ func TestHandleCanaryAbort(t *testing.T) { } }` errmsg := fmt.Sprintf(conditions.RolloutAbortedMessage, 2) - newConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, "") + newConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, "", false) assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, newConditions, conditions.RolloutAbortedReason, errmsg)), patch) }) @@ -1710,7 +1722,7 @@ func TestHandleCanaryAbort(t *testing.T) { } }` errmsg := fmt.Sprintf(conditions.RolloutAbortedMessage, 1) - newConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r1, false, "") + newConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r1, false, "", true) assert.Equal(t, calculatePatch(r1, fmt.Sprintf(expectedPatch, newConditions, conditions.RolloutAbortedReason, errmsg)), patch) }) } diff --git a/rollout/controller_test.go b/rollout/controller_test.go index 7f324ea10b..0e97a405e9 100644 --- a/rollout/controller_test.go +++ b/rollout/controller_test.go @@ -193,6 +193,28 @@ func newPausedCondition(isPaused bool) (v1alpha1.RolloutCondition, string) { return condition, string(conditionBytes) } +func newHealthyCondition(isHealthy bool) (v1alpha1.RolloutCondition, string) { + status := corev1.ConditionTrue + msg := conditions.RolloutHealthyMessage + if !isHealthy { + status = corev1.ConditionFalse + msg = conditions.RolloutNotHealthyMessage + } + condition := v1alpha1.RolloutCondition{ + LastTransitionTime: timeutil.MetaNow(), + LastUpdateTime: timeutil.MetaNow(), + Message: msg, + Reason: conditions.RolloutHealthyReason, + Status: status, + Type: v1alpha1.RolloutHealthy, + } + conditionBytes, err := json.Marshal(condition) + if err != nil { + panic(err) + } + return condition, string(conditionBytes) +} + func newCompletedCondition(isCompleted bool) (v1alpha1.RolloutCondition, string) { status := corev1.ConditionTrue if !isCompleted { @@ -315,33 +337,57 @@ func newAvailableCondition(available bool) (v1alpha1.RolloutCondition, string) { return condition, string(conditionBytes) } -func generateConditionsPatch(available bool, progressingReason string, progressingResource runtime.Object, availableConditionFirst bool, progressingMessage string) string { +func generateConditionsPatch(available bool, progressingReason string, progressingResource runtime.Object, availableConditionFirst bool, progressingMessage string, isCompleted bool) string { _, availableCondition := newAvailableCondition(available) _, progressingCondition := newProgressingCondition(progressingReason, progressingResource, progressingMessage) + _, completedCondition := newCompletedCondition(isCompleted) if availableConditionFirst { - return fmt.Sprintf("[%s, %s]", availableCondition, progressingCondition) + return fmt.Sprintf("[%s, %s, %s]", availableCondition, progressingCondition, completedCondition) } - return fmt.Sprintf("[%s, %s]", progressingCondition, availableCondition) + return fmt.Sprintf("[%s, %s, %s]", progressingCondition, availableCondition, completedCondition) } -func generateConditionsPatchWithPause(available bool, progressingReason string, progressingResource runtime.Object, availableConditionFirst bool, progressingMessage string, isPaused bool) string { +func generateConditionsPatchWithPause(available bool, progressingReason string, progressingResource runtime.Object, availableConditionFirst bool, progressingMessage string, isPaused bool, isCompleted bool) string { _, availableCondition := newAvailableCondition(available) _, progressingCondition := newProgressingCondition(progressingReason, progressingResource, progressingMessage) _, pauseCondition := newPausedCondition(isPaused) + _, completedCondition := newCompletedCondition(isCompleted) + if availableConditionFirst { + return fmt.Sprintf("[%s, %s, %s, %s]", availableCondition, completedCondition, progressingCondition, pauseCondition) + } + return fmt.Sprintf("[%s, %s, %s, %s]", progressingCondition, pauseCondition, availableCondition, completedCondition) +} + +func generateConditionsPatchWithHealthy(available bool, progressingReason string, progressingResource runtime.Object, availableConditionFirst bool, progressingMessage string, isHealthy bool, isCompleted bool) string { + _, availableCondition := newAvailableCondition(available) + _, progressingCondition := newProgressingCondition(progressingReason, progressingResource, progressingMessage) + _, healthyCondition := newHealthyCondition(isHealthy) + _, completedCondition := newCompletedCondition(isCompleted) if availableConditionFirst { - return fmt.Sprintf("[%s, %s, %s]", availableCondition, progressingCondition, pauseCondition) + return fmt.Sprintf("[%s, %s, %s, %s]", availableCondition, completedCondition, healthyCondition, progressingCondition) } - return fmt.Sprintf("[%s, %s, %s]", progressingCondition, pauseCondition, availableCondition) + return fmt.Sprintf("[%s, %s, %s, %s]", completedCondition, healthyCondition, progressingCondition, availableCondition) } -func generateConditionsPatchWithComplete(available bool, progressingReason string, progressingResource runtime.Object, availableConditionFirst bool, progressingMessage string, isCompleted bool) string { +func generateConditionsPatchWithCompleted(available bool, progressingReason string, progressingResource runtime.Object, availableConditionFirst bool, progressingMessage string, isCompleted bool) string { _, availableCondition := newAvailableCondition(available) _, progressingCondition := newProgressingCondition(progressingReason, progressingResource, progressingMessage) _, completeCondition := newCompletedCondition(isCompleted) if availableConditionFirst { - return fmt.Sprintf("[%s, %s, %s]", availableCondition, completeCondition, progressingCondition) + return fmt.Sprintf("[%s, %s, %s]", availableCondition, progressingCondition, completeCondition) + } + return fmt.Sprintf("[%s, %s, %s]", progressingCondition, availableCondition, completeCondition) +} + +func generateConditionsPatchWithCompletedHealthy(available bool, progressingReason string, progressingResource runtime.Object, availableConditionFirst bool, progressingMessage string, isHealthy bool, isCompleted bool) string { + _, completedCondition := newCompletedCondition(isCompleted) + _, availableCondition := newAvailableCondition(available) + _, progressingCondition := newProgressingCondition(progressingReason, progressingResource, progressingMessage) + _, healthyCondition := newHealthyCondition(isHealthy) + if availableConditionFirst { + return fmt.Sprintf("[%s, %s, %s, %s]", availableCondition, healthyCondition, completedCondition, progressingCondition) } - return fmt.Sprintf("[%s, %s, %s]", completeCondition, progressingCondition, availableCondition) + return fmt.Sprintf("[%s, %s, %s, %s]", healthyCondition, completedCondition, progressingCondition, availableCondition) } func updateConditionsPatch(r v1alpha1.Rollout, newCondition v1alpha1.RolloutCondition) string { @@ -351,7 +397,7 @@ func updateConditionsPatch(r v1alpha1.Rollout, newCondition v1alpha1.RolloutCond } // func updateBlueGreenRolloutStatus(r *v1alpha1.Rollout, preview, active string, availableReplicas, updatedReplicas, hpaReplicas int32, pause bool, available bool, progressingStatus string) *v1alpha1.Rollout { -func updateBlueGreenRolloutStatus(r *v1alpha1.Rollout, preview, active, stable string, availableReplicas, updatedReplicas, totalReplicas, hpaReplicas int32, pause bool, available bool) *v1alpha1.Rollout { +func updateBlueGreenRolloutStatus(r *v1alpha1.Rollout, preview, active, stable string, availableReplicas, updatedReplicas, totalReplicas, hpaReplicas int32, pause bool, available bool, isCompleted bool) *v1alpha1.Rollout { newRollout := updateBaseRolloutStatus(r, availableReplicas, updatedReplicas, totalReplicas, hpaReplicas) selector := newRollout.Spec.Selector.DeepCopy() if active != "" { @@ -363,6 +409,8 @@ func updateBlueGreenRolloutStatus(r *v1alpha1.Rollout, preview, active, stable s newRollout.Status.StableRS = stable cond, _ := newAvailableCondition(available) newRollout.Status.Conditions = append(newRollout.Status.Conditions, cond) + completeCond, _ := newCompletedCondition(isCompleted) + newRollout.Status.Conditions = append(newRollout.Status.Conditions, completeCond) if pause { now := timeutil.MetaNow() cond := v1alpha1.PauseCondition{ @@ -1390,6 +1438,8 @@ func TestComputeHashChangeTolerationBlueGreen(t *testing.T) { conditions.SetRolloutCondition(&r.Status, availableCondition) progressingCondition, _ := newProgressingCondition(conditions.ReplicaSetUpdatedReason, rs, "") conditions.SetRolloutCondition(&r.Status, progressingCondition) + completedCondition, _ := newCompletedCondition(true) + conditions.SetRolloutCondition(&r.Status, completedCondition) r.Status.Phase, r.Status.Message = rolloututil.CalculateRolloutPhase(r.Spec, r.Status) podTemplate := corev1.PodTemplate{ @@ -1434,6 +1484,8 @@ func TestComputeHashChangeTolerationCanary(t *testing.T) { conditions.SetRolloutCondition(&r.Status, availableCondition) progressingCondition, _ := newProgressingCondition(conditions.ReplicaSetUpdatedReason, rs, "") conditions.SetRolloutCondition(&r.Status, progressingCondition) + completedCondition, _ := newCompletedCondition(true) + conditions.SetRolloutCondition(&r.Status, completedCondition) podTemplate := corev1.PodTemplate{ Template: rs.Spec.Template, @@ -1461,7 +1513,7 @@ func TestSwitchBlueGreenToCanary(t *testing.T) { activeSvc := newService("active", 80, nil, r) rs := newReplicaSetWithStatus(r, 1, 1) rsPodHash := rs.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] - r = updateBlueGreenRolloutStatus(r, "", rsPodHash, rsPodHash, 1, 1, 1, 1, false, true) + r = updateBlueGreenRolloutStatus(r, "", rsPodHash, rsPodHash, 1, 1, 1, 1, false, true, false) // StableRS is set to avoid running the migration code. When .status.canary.stableRS is removed, the line below can be deleted //r.Status.Canary.StableRS = rsPodHash r.Spec.Strategy.BlueGreen = nil @@ -1479,7 +1531,7 @@ func TestSwitchBlueGreenToCanary(t *testing.T) { f.run(getKey(r, t)) patch := f.getPatchedRollout(i) - addedConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs, true, "") + addedConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs, true, "", true) expectedPatch := fmt.Sprintf(`{ "status": { "blueGreen": { diff --git a/rollout/experiment_test.go b/rollout/experiment_test.go index c8b610a303..e2e50c1f28 100644 --- a/rollout/experiment_test.go +++ b/rollout/experiment_test.go @@ -68,7 +68,7 @@ func TestRolloutCreateExperiment(t *testing.T) { "conditions": %s } }` - conds := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "") + conds := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "", false) assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, ex.Name, conds)), patch) } @@ -125,7 +125,7 @@ func TestRolloutCreateClusterTemplateExperiment(t *testing.T) { "conditions": %s } }` - conds := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "") + conds := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "", false) assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, ex.Name, conds)), patch) } @@ -177,7 +177,7 @@ func TestCreateExperimentWithCollision(t *testing.T) { "conditions": %s } }` - conds := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "") + conds := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "", false) assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, createdEx.Name, conds)), patch) } @@ -228,7 +228,7 @@ func TestCreateExperimentWithCollisionAndSemanticEquality(t *testing.T) { "conditions": %s } }` - conds := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "") + conds := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "", false) assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, ex.Name, conds)), patch) } @@ -256,6 +256,8 @@ func TestRolloutExperimentProcessingDoNothing(t *testing.T) { conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) f.rolloutLister = append(f.rolloutLister, r2) f.experimentLister = append(f.experimentLister, ex) @@ -311,7 +313,7 @@ func TestAbortRolloutAfterFailedExperiment(t *testing.T) { } }` now := timeutil.Now().UTC().Format(time.RFC3339) - generatedConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, "") + generatedConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, "", false) assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, now, generatedConditions, conditions.RolloutAbortedReason, fmt.Sprintf(conditions.RolloutAbortedMessage, 2))), patch) } @@ -477,7 +479,7 @@ func TestRolloutExperimentFinishedIncrementStep(t *testing.T) { "conditions": %s } }` - generatedConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs2, false, "") + generatedConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs2, false, "", false) assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, generatedConditions)), patch) } diff --git a/rollout/service_test.go b/rollout/service_test.go index aa4f1d020b..8a402d001c 100644 --- a/rollout/service_test.go +++ b/rollout/service_test.go @@ -302,13 +302,15 @@ func TestBlueGreenAWSVerifyTargetGroupsNotYetReady(t *testing.T) { rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] svc := newService("active", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash}, r2) - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 3, 3, 6, 3, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 3, 3, 6, 3, false, true, false) r2.Status.Message = "" r2.Status.ObservedGeneration = strconv.Itoa(int(r2.Generation)) - completedCondition, _ := newCompletedCondition(true) - conditions.SetRolloutCondition(&r2.Status, completedCondition) + completedHealthyCondition, _ := newHealthyCondition(true) + conditions.SetRolloutCondition(&r2.Status, completedHealthyCondition) progressingCondition, _ := newProgressingCondition(conditions.NewRSAvailableReason, rs2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) + completedCondition, _ := newCompletedCondition(false) + conditions.SetRolloutCondition(&r2.Status, completedCondition) f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2, tgb) @@ -385,13 +387,15 @@ func TestBlueGreenAWSVerifyTargetGroupsReady(t *testing.T) { rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] svc := newService("active", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash}, r2) - r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 3, 3, 6, 3, false, true) + r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs1PodHash, 3, 3, 6, 3, false, true, false) r2.Status.Message = "waiting for post-promotion verification to complete" r2.Status.ObservedGeneration = strconv.Itoa(int(r2.Generation)) - completedCondition, _ := newCompletedCondition(true) + completedCondition, _ := newHealthyCondition(true) conditions.SetRolloutCondition(&r2.Status, completedCondition) progressingCondition, _ := newProgressingCondition(conditions.NewRSAvailableReason, rs2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) + completedCond := conditions.NewRolloutCondition(v1alpha1.RolloutCompleted, corev1.ConditionTrue, conditions.RolloutCompletedReason, conditions.RolloutCompletedReason) + conditions.SetRolloutCondition(&r2.Status, *completedCond) f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2, tgb) @@ -489,10 +493,12 @@ func TestCanaryAWSVerifyTargetGroupsNotYetReady(t *testing.T) { r2.Status.StableRS = rs2PodHash availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) - completedCondition, _ := newCompletedCondition(false) - conditions.SetRolloutCondition(&r2.Status, completedCondition) + healthyCondition, _ := newHealthyCondition(false) + conditions.SetRolloutCondition(&r2.Status, healthyCondition) progressingCondition, _ := newProgressingCondition(conditions.NewRSAvailableReason, rs2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) + completedCondition, _ := newCompletedCondition(true) + conditions.SetRolloutCondition(&r2.Status, completedCondition) _, r2.Status.Canary.Weights = calculateWeightStatus(r2, rs2PodHash, rs2PodHash, 0) f.rolloutLister = append(f.rolloutLister, r2) @@ -585,10 +591,12 @@ func TestCanaryAWSVerifyTargetGroupsReady(t *testing.T) { r2.Status.StableRS = rs2PodHash availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) - completedCondition, _ := newCompletedCondition(false) - conditions.SetRolloutCondition(&r2.Status, completedCondition) + healthyCondition, _ := newHealthyCondition(false) + conditions.SetRolloutCondition(&r2.Status, healthyCondition) progressingCondition, _ := newProgressingCondition(conditions.NewRSAvailableReason, rs2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) + completedCondition, _ := newCompletedCondition(true) + conditions.SetRolloutCondition(&r2.Status, completedCondition) _, r2.Status.Canary.Weights = calculateWeightStatus(r2, rs2PodHash, rs2PodHash, 0) f.rolloutLister = append(f.rolloutLister, r2) @@ -646,10 +654,12 @@ func TestCanaryAWSVerifyTargetGroupsSkip(t *testing.T) { r2.Status.StableRS = rs2PodHash availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) - completedCondition, _ := newCompletedCondition(false) - conditions.SetRolloutCondition(&r2.Status, completedCondition) + healthyCondition, _ := newHealthyCondition(false) + conditions.SetRolloutCondition(&r2.Status, healthyCondition) progressingCondition, _ := newProgressingCondition(conditions.NewRSAvailableReason, rs2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) + completedCondition, _ := newCompletedCondition(true) + conditions.SetRolloutCondition(&r2.Status, completedCondition) _, r2.Status.Canary.Weights = calculateWeightStatus(r2, rs2PodHash, rs2PodHash, 0) f.rolloutLister = append(f.rolloutLister, r2) diff --git a/rollout/sync.go b/rollout/sync.go index fc260f8633..b9bc616552 100644 --- a/rollout/sync.go +++ b/rollout/sync.go @@ -547,15 +547,19 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt isPaused := len(c.rollout.Status.PauseConditions) > 0 || c.rollout.Spec.Paused isAborted := c.pauseContext.IsAborted() - var becameIncomplete bool // remember if we transitioned from completed - completeCond := conditions.GetRolloutCondition(c.rollout.Status, v1alpha1.RolloutCompleted) - if !isPaused && conditions.RolloutComplete(c.rollout, &newStatus) { - updateCompletedCond := conditions.NewRolloutCondition(v1alpha1.RolloutCompleted, corev1.ConditionTrue, conditions.RolloutCompletedReason, conditions.RolloutCompletedReason) - conditions.SetRolloutCondition(&newStatus, *updateCompletedCond) + var becameUnhealthy bool // remember if we transitioned from healthy to unhealthy + completeCond := conditions.GetRolloutCondition(c.rollout.Status, v1alpha1.RolloutHealthy) + if !isPaused && conditions.RolloutHealthy(c.rollout, &newStatus) { + updateHealthyCond := conditions.NewRolloutCondition(v1alpha1.RolloutHealthy, corev1.ConditionTrue, conditions.RolloutHealthyReason, conditions.RolloutHealthyMessage) + conditions.SetRolloutCondition(&newStatus, *updateHealthyCond) + // If we ever wanted to emit a healthy event here it would be noisy and somewhat unpredictable for tests and so should probably be skipped + // when checking in e2e and unit tests. + //c.recorder.Warnf(c.rollout, record.EventOptions{EventReason: conditions.RolloutHealthyReason}, conditions.RolloutHealthyMessage) } else { if completeCond != nil { - updateCompletedCond := conditions.NewRolloutCondition(v1alpha1.RolloutCompleted, corev1.ConditionFalse, conditions.RolloutCompletedReason, conditions.RolloutCompletedReason) - becameIncomplete = conditions.SetRolloutCondition(&newStatus, *updateCompletedCond) + updateHealthyCond := conditions.NewRolloutCondition(v1alpha1.RolloutHealthy, corev1.ConditionFalse, conditions.RolloutHealthyReason, conditions.RolloutNotHealthyMessage) + becameUnhealthy = conditions.SetRolloutCondition(&newStatus, *updateHealthyCond) + //c.recorder.Warnf(c.rollout, record.EventOptions{EventReason: conditions.RolloutHealthyReason}, conditions.RolloutNotHealthyMessage) } } @@ -576,11 +580,11 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt // In such a case, we should simply not estimate any progress for this rollout. currentCond := conditions.GetRolloutCondition(c.rollout.Status, v1alpha1.RolloutProgressing) - isCompleteRollout := newStatus.Replicas == newStatus.AvailableReplicas && currentCond != nil && currentCond.Reason == conditions.NewRSAvailableReason && currentCond.Type != v1alpha1.RolloutProgressing + isHealthyRollout := newStatus.Replicas == newStatus.AvailableReplicas && currentCond != nil && currentCond.Reason == conditions.NewRSAvailableReason && currentCond.Type != v1alpha1.RolloutProgressing // Check for progress. Only do this if the latest rollout hasn't completed yet and it is not aborted - if !isCompleteRollout && !isAborted { + if !isHealthyRollout && !isAborted { switch { - case conditions.RolloutComplete(c.rollout, &newStatus): + case conditions.RolloutHealthy(c.rollout, &newStatus): // Update the rollout conditions with a message for the new replica set that // was successfully deployed. If the condition already exists, we ignore this update. rsName := "" @@ -590,7 +594,7 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt msg := fmt.Sprintf(conditions.ReplicaSetCompletedMessage, rsName) progressingCondition := conditions.NewRolloutCondition(v1alpha1.RolloutProgressing, corev1.ConditionTrue, conditions.NewRSAvailableReason, msg) conditions.SetRolloutCondition(&newStatus, *progressingCondition) - case conditions.RolloutProgressing(c.rollout, &newStatus) || becameIncomplete: + case conditions.RolloutProgressing(c.rollout, &newStatus) || becameUnhealthy: // If there is any progress made, continue by not checking if the rollout failed. This // behavior emulates the rolling updater progressDeadline check. msg := fmt.Sprintf(conditions.RolloutProgressingMessage, c.rollout.Name) @@ -599,7 +603,7 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt } var reason string - if newStatus.StableRS == newStatus.CurrentPodHash && becameIncomplete { + if newStatus.StableRS == newStatus.CurrentPodHash && becameUnhealthy { // When a fully promoted rollout becomes Incomplete, e.g., due to the ReplicaSet status changes like // pod restarts, evicted -> recreated, we'll need to reset the rollout's condition to `PROGRESSING` to // avoid any timeouts. @@ -672,6 +676,22 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt } else { conditions.RemoveRolloutCondition(&newStatus, v1alpha1.RolloutReplicaFailure) } + + if conditions.RolloutCompleted(c.rollout, &newStatus) { + // The event gets triggered in function promoteStable + updateCompletedCond := conditions.NewRolloutCondition(v1alpha1.RolloutCompleted, corev1.ConditionTrue, + conditions.RolloutCompletedReason, conditions.RolloutCompletedReason) + conditions.SetRolloutCondition(&newStatus, *updateCompletedCond) + } else { + updateCompletedCond := conditions.NewRolloutCondition(v1alpha1.RolloutCompleted, corev1.ConditionFalse, + conditions.RolloutCompletedReason, conditions.RolloutCompletedReason) + if conditions.SetRolloutCondition(&newStatus, *updateCompletedCond) { + revision, _ := replicasetutil.Revision(c.rollout) + c.recorder.Eventf(c.rollout, record.EventOptions{EventReason: conditions.RolloutNotCompletedReason}, + conditions.RolloutNotCompletedMessage, revision+1, newStatus.CurrentPodHash) + } + } + return newStatus } @@ -754,7 +774,7 @@ func (c *rolloutContext) requeueStuckRollout(newStatus v1alpha1.RolloutStatus) t } // No need to estimate progress if the rollout is complete or already timed out. isPaused := len(c.rollout.Status.PauseConditions) > 0 || c.rollout.Spec.Paused - if conditions.RolloutComplete(c.rollout, &newStatus) || currentCond.Reason == conditions.TimedOutReason || isPaused || c.rollout.Status.Abort || isIndefiniteStep(c.rollout) { + if conditions.RolloutHealthy(c.rollout, &newStatus) || currentCond.Reason == conditions.TimedOutReason || isPaused || c.rollout.Status.Abort || isIndefiniteStep(c.rollout) { return time.Duration(-1) } // If there is no sign of progress at this point then there is a high chance that the @@ -923,6 +943,7 @@ func (c *rolloutContext) promoteStable(newStatus *v1alpha1.RolloutStatus, reason } } newStatus.StableRS = newStatus.CurrentPodHash + revision, _ := replicasetutil.Revision(c.rollout) c.recorder.Eventf(c.rollout, record.EventOptions{EventReason: conditions.RolloutCompletedReason}, conditions.RolloutCompletedMessage, revision, newStatus.CurrentPodHash, reason) diff --git a/rollout/trafficrouting_test.go b/rollout/trafficrouting_test.go index abb0cc09f6..6c1e49f476 100644 --- a/rollout/trafficrouting_test.go +++ b/rollout/trafficrouting_test.go @@ -739,6 +739,8 @@ func TestCanaryWithTrafficRoutingAddScaleDownDelay(t *testing.T) { r2.Status.ObservedGeneration = strconv.Itoa(int(r2.Generation)) availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) + completedCondition, _ := newCompletedCondition(true) + conditions.SetRolloutCondition(&r2.Status, completedCondition) _, r2.Status.Canary.Weights = calculateWeightStatus(r2, rs2PodHash, rs2PodHash, 0) selector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash} diff --git a/test/e2e/functional_test.go b/test/e2e/functional_test.go index f4380c3562..8ca5a026ac 100644 --- a/test/e2e/functional_test.go +++ b/test/e2e/functional_test.go @@ -90,10 +90,12 @@ spec: ExpectRevisionPodCount("1", 0). ExpectRevisionPodCount("2", 1). ExpectRolloutEvents([]string{ + "RolloutNotCompleted", // Rollout not completed, started update to revision 0 (7fd9b5545c) "RolloutUpdated", // Rollout updated to revision 1 "NewReplicaSetCreated", // Created ReplicaSet abort-retry-promote-698fbfb9dc (revision 1) "ScalingReplicaSet", // Scaled up ReplicaSet abort-retry-promote-698fbfb9dc (revision 1) from 0 to 1 "RolloutCompleted", // Rollout completed update to revision 1 (698fbfb9dc): Initial deploy + "RolloutNotCompleted", "RolloutUpdated", // Rollout updated to revision 2 "NewReplicaSetCreated", // Created ReplicaSet abort-retry-promote-75dcb5ddd6 (revision 2) "ScalingReplicaSet", // Scaled up ReplicaSet abort-retry-promote-75dcb5ddd6 (revision 2) from 0 to 1 @@ -706,6 +708,7 @@ func (s *FunctionalSuite) TestBlueGreenUpdate() { "SwitchService", // Switched selector for service 'bluegreen' from '' to '7dcd8f8869' "RolloutUpdated", // Rollout updated to revision 2 "NewReplicaSetCreated", // Created ReplicaSet bluegreen-5498785cd6 (revision 2) + "RolloutNotCompleted", // Rollout went to not completed state started update to revision 2 (85c6899) "ScalingReplicaSet", // Scaled up ReplicaSet bluegreen-5498785cd6 (revision 2) from 0 to 3 "SwitchService", // Switched selector for service 'bluegreen' from '7dcd8f8869' to '6c779b88b6' "RolloutCompleted", // Rollout completed update to revision 2 (6c779b88b6): Completed blue-green update @@ -959,7 +962,7 @@ spec: Then(). ExpectRevisionPodCount("2", 0). ExpectRollout("Abort=True", func(r *v1alpha1.Rollout) bool { - return r.Status.Abort == true && len(r.Status.Conditions) == 3 + return r.Status.Abort == true && len(r.Status.Conditions) == 4 }) } @@ -1120,10 +1123,10 @@ func (s *FunctionalSuite) TestKubectlWaitForCompleted() { kind: Service apiVersion: v1 metadata: - name: kubectl-wait-completed + name: kubectl-wait-healthy spec: selector: - app: kubectl-wait-completed + app: kubectl-wait-healthy ports: - protocol: TCP port: 80 @@ -1132,19 +1135,19 @@ spec: apiVersion: argoproj.io/v1alpha1 kind: Rollout metadata: - name: kubectl-wait-completed + name: kubectl-wait-healthy spec: replicas: 1 selector: matchLabels: - app: kubectl-wait-completed + app: kubectl-wait-healthy template: metadata: labels: - app: kubectl-wait-completed + app: kubectl-wait-healthy spec: containers: - - name: kubectl-wait-completed + - name: kubectl-wait-healthy image: nginx:1.19-alpine imagePullPolicy: Always ports: @@ -1158,21 +1161,21 @@ spec: strategy: blueGreen: - activeService: kubectl-wait-completed + activeService: kubectl-wait-healthy autoPromotionEnabled: true `). When(). UpdateSpec(). Then(). - ExpectRollout("Completed=False", func(r *v1alpha1.Rollout) bool { - cmd := exec.Command("kubectl", "wait", "--for=condition=Completed=False", fmt.Sprintf("rollout/%s", r.Name)) + ExpectRollout("Healthy=False", func(r *v1alpha1.Rollout) bool { + cmd := exec.Command("kubectl", "wait", "--for=condition=Healthy=False", fmt.Sprintf("rollout/%s", r.Name)) out, err := cmd.CombinedOutput() return err == nil && strings.Contains(string(out), fmt.Sprintf("rollout.argoproj.io/%s condition met", r.Name)) }). ExpectRolloutStatus("Progressing"). ExpectActiveRevision("1"). - ExpectRollout("Completed=True", func(r *v1alpha1.Rollout) bool { - cmd := exec.Command("kubectl", "wait", "--for=condition=Completed=True", fmt.Sprintf("rollout/%s", r.Name)) + ExpectRollout("Healthy=True", func(r *v1alpha1.Rollout) bool { + cmd := exec.Command("kubectl", "wait", "--for=condition=Healthy=True", fmt.Sprintf("rollout/%s", r.Name)) out, err := cmd.CombinedOutput() return err == nil && strings.Contains(string(out), fmt.Sprintf("rollout.argoproj.io/%s condition met", r.Name)) }). diff --git a/utils/conditions/conditions.go b/utils/conditions/conditions.go index 99b6d59ddf..c2aaece787 100644 --- a/utils/conditions/conditions.go +++ b/utils/conditions/conditions.go @@ -68,6 +68,17 @@ const ( RolloutCompletedReason = "RolloutCompleted" // RolloutCompletedMessage is added when the rollout is completed RolloutCompletedMessage = "Rollout completed update to revision %d (%s): %s" + // RolloutNotCompletedReason is added in a rollout when it is completed. + RolloutNotCompletedReason = "RolloutNotCompleted" + // RolloutNotCompletedMessage is added when the rollout is completed + RolloutNotCompletedMessage = "Rollout not completed, started update to revision %d (%s)" + + // RolloutHealthyReason is added in a rollout when it is healthy. + RolloutHealthyReason = "RolloutHealthy" + // RolloutHealthyMessage is added when the rollout is completed and is healthy or not. + RolloutHealthyMessage = "Rollout is healthy" + // RolloutNotHealthyMessage is added when the rollout is completed and is healthy or not. + RolloutNotHealthyMessage = "Rollout is not healthy" // RolloutAbortedReason indicates that the rollout was aborted RolloutAbortedReason = "RolloutAborted" @@ -125,7 +136,7 @@ const ( // TimedOutReason is added in a rollout when its newest replica set fails to show any progress // within the given deadline (progressDeadlineSeconds). TimedOutReason = "ProgressDeadlineExceeded" - // RolloutTimeOutMessage is is added in a rollout when the rollout fails to show any progress + // RolloutTimeOutMessage is added in a rollout when the rollout fails to show any progress // within the given deadline (progressDeadlineSeconds). RolloutTimeOutMessage = "Rollout %q has timed out progressing." @@ -257,9 +268,9 @@ func RolloutProgressing(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutSt strategySpecificProgress } -// RolloutComplete considers a rollout to be complete once all of its desired replicas +// RolloutHealthy considers a rollout to be healthy once all of its desired replicas // are updated, available, and receiving traffic from the active service, and no old pods are running. -func RolloutComplete(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutStatus) bool { +func RolloutHealthy(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutStatus) bool { completedStrategy := true replicas := defaults.GetReplicasOrDefault(rollout.Spec.Replicas) @@ -288,6 +299,11 @@ func RolloutComplete(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutStatu completedStrategy } +// RolloutCompleted considers a rollout to be complete once StableRS == CurrentPodHash +func RolloutCompleted(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutStatus) bool { + return newStatus.StableRS != "" && newStatus.StableRS == newStatus.CurrentPodHash +} + // ComputeStepHash returns a hash value calculated from the Rollout's steps. The hash will // be safe encoded to avoid bad words. func ComputeStepHash(rollout *v1alpha1.Rollout) string { diff --git a/utils/conditions/rollouts_test.go b/utils/conditions/rollouts_test.go index f6e3e02550..842f01f594 100644 --- a/utils/conditions/rollouts_test.go +++ b/utils/conditions/rollouts_test.go @@ -350,7 +350,7 @@ func TestRolloutProgressing(t *testing.T) { } -func TestRolloutComplete(t *testing.T) { +func TestRolloutHealthy(t *testing.T) { rollout := func(desired, current, updated, available int32, correctObservedGeneration bool) *v1alpha1.Rollout { r := &v1alpha1.Rollout{ Spec: v1alpha1.RolloutSpec{ @@ -475,12 +475,36 @@ func TestRolloutComplete(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - assert.Equal(t, test.expected, RolloutComplete(test.r, &test.r.Status)) + assert.Equal(t, test.expected, RolloutHealthy(test.r, &test.r.Status)) }) } } +func TestRolloutComplete(t *testing.T) { + rollout := func(desired, current, updated, available int32) *v1alpha1.Rollout { + r := &v1alpha1.Rollout{ + Spec: v1alpha1.RolloutSpec{ + Replicas: &desired, + }, + Status: v1alpha1.RolloutStatus{ + Replicas: current, + UpdatedReplicas: updated, + AvailableReplicas: available, + }, + } + podHash := hash.ComputePodTemplateHash(&r.Spec.Template, r.Status.CollisionCount) + r.Status.CurrentPodHash = podHash + r.Status.StableRS = podHash + return r + } + r := rollout(5, 5, 5, 5) + assert.Equal(t, true, RolloutCompleted(r, &r.Status)) + + r.Status.StableRS = "not-current-pod-hash" + assert.Equal(t, false, RolloutCompleted(r, &r.Status)) +} + func TestRolloutTimedOut(t *testing.T) { before := metav1.Time{