From a151e574b54187a3ee14de0b601654b4e31ae01e Mon Sep 17 00:00:00 2001 From: Danny Thomson Date: Tue, 7 Jul 2020 15:29:40 -0700 Subject: [PATCH 1/4] Refactor current ar tracking in Rollout --- rollout/analysis.go | 32 ++++++++++------------- rollout/bluegreen.go | 6 ++--- rollout/canary.go | 4 +-- rollout/context.go | 48 ++++++++++++++++------------------- rollout/pause.go | 3 +-- utils/analysis/filter.go | 36 +++++++++++++++----------- utils/analysis/filter_test.go | 14 +++++----- utils/analysis/helpers.go | 25 ++++++++++++++++++ 8 files changed, 93 insertions(+), 75 deletions(-) diff --git a/rollout/analysis.go b/rollout/analysis.go index a23d42d622..572591c594 100644 --- a/rollout/analysis.go +++ b/rollout/analysis.go @@ -46,44 +46,38 @@ func (c *Controller) getAnalysisRunsForRollout(rollout *v1alpha1.Rollout) ([]*v1 func (c *Controller) reconcileAnalysisRuns(roCtx rolloutContext) error { otherArs := roCtx.OtherAnalysisRuns() if roCtx.PauseContext().IsAborted() { - allArs := append(roCtx.CurrentAnalysisRuns(), otherArs...) + allArs := append(roCtx.CurrentAnalysisRuns().ToArray(), otherArs...) return c.cancelAnalysisRuns(roCtx, allArs) } - newCurrentAnalysisRuns := []*v1alpha1.AnalysisRun{} + newCurrentAnalysisRuns := analysisutil.CurrentAnalysisRuns{} rollout := roCtx.Rollout() if rollout.Spec.Strategy.Canary != nil { stepAnalysisRun, err := c.reconcileStepBasedAnalysisRun(roCtx) if err != nil { return err } - if stepAnalysisRun != nil { - newCurrentAnalysisRuns = append(newCurrentAnalysisRuns, stepAnalysisRun) - } + newCurrentAnalysisRuns.CanaryStep = stepAnalysisRun backgroundAnalysisRun, err := c.reconcileBackgroundAnalysisRun(roCtx) if err != nil { return err } - if backgroundAnalysisRun != nil { - newCurrentAnalysisRuns = append(newCurrentAnalysisRuns, backgroundAnalysisRun) - } + newCurrentAnalysisRuns.CanaryBackground = backgroundAnalysisRun + } if rollout.Spec.Strategy.BlueGreen != nil { prePromotionAr, err := c.reconcilePrePromotionAnalysisRun(roCtx) if err != nil { return err } - if prePromotionAr != nil { - newCurrentAnalysisRuns = append(newCurrentAnalysisRuns, prePromotionAr) - } + newCurrentAnalysisRuns.BlueGreenPrePromotion = prePromotionAr + postPromotionAr, err := c.reconcilePostPromotionAnalysisRun(roCtx) if err != nil { return err } - if postPromotionAr != nil { - newCurrentAnalysisRuns = append(newCurrentAnalysisRuns, postPromotionAr) - } + newCurrentAnalysisRuns.BlueGreenPostPromotion = postPromotionAr } roCtx.SetCurrentAnalysisRuns(newCurrentAnalysisRuns) @@ -93,7 +87,7 @@ func (c *Controller) reconcileAnalysisRuns(roCtx rolloutContext) error { // To prevent us from terminating the runs that we just created moments ago, rebuild otherArs // to ensure it does not include the newly created runs. otherArs, _ = analysisutil.FilterAnalysisRuns(otherArs, func(ar *v1alpha1.AnalysisRun) bool { - for _, curr := range newCurrentAnalysisRuns { + for _, curr := range newCurrentAnalysisRuns.ToArray() { if ar.Name == curr.Name { roCtx.Log().Infof("Rescued %s from inadvertent termination", ar.Name) return false @@ -121,7 +115,7 @@ func (c *Controller) reconcilePrePromotionAnalysisRun(roCtx rolloutContext) (*v1 rollout := roCtx.Rollout() newRS := roCtx.NewRS() currentArs := roCtx.CurrentAnalysisRuns() - currentAr := analysisutil.FilterAnalysisRunsByName(currentArs, rollout.Status.BlueGreen.PrePromotionAnalysisRun) + currentAr := currentArs.BlueGreenPrePromotion if rollout.Spec.Strategy.BlueGreen.PrePromotionAnalysis == nil { err := c.cancelAnalysisRuns(roCtx, []*v1alpha1.AnalysisRun{currentAr}) return nil, err @@ -163,7 +157,7 @@ func (c *Controller) reconcilePostPromotionAnalysisRun(roCtx rolloutContext) (*v rollout := roCtx.Rollout() newRS := roCtx.NewRS() currentArs := roCtx.CurrentAnalysisRuns() - currentAr := analysisutil.FilterAnalysisRunsByName(currentArs, rollout.Status.BlueGreen.PostPromotionAnalysisRun) + currentAr := currentArs.BlueGreenPostPromotion if rollout.Spec.Strategy.BlueGreen.PostPromotionAnalysis == nil { err := c.cancelAnalysisRuns(roCtx, []*v1alpha1.AnalysisRun{currentAr}) return nil, err @@ -206,7 +200,7 @@ func (c *Controller) reconcileBackgroundAnalysisRun(roCtx rolloutContext) (*v1al rollout := roCtx.Rollout() newRS := roCtx.NewRS() currentArs := roCtx.CurrentAnalysisRuns() - currentAr := analysisutil.FilterAnalysisRunsByName(currentArs, rollout.Status.Canary.CurrentBackgroundAnalysisRun) + currentAr := currentArs.CanaryBackground if rollout.Spec.Strategy.Canary.Analysis == nil { err := c.cancelAnalysisRuns(roCtx, []*v1alpha1.AnalysisRun{currentAr}) return nil, err @@ -261,7 +255,7 @@ func (c *Controller) reconcileStepBasedAnalysisRun(roCtx rolloutContext) (*v1alp currentArs := roCtx.CurrentAnalysisRuns() newRS := roCtx.NewRS() step, index := replicasetutil.GetCurrentCanaryStep(rollout) - currentAr := analysisutil.FilterAnalysisRunsByName(currentArs, rollout.Status.Canary.CurrentStepAnalysisRun) + currentAr := currentArs.CanaryStep if getPauseCondition(rollout, v1alpha1.PauseReasonInconclusiveAnalysis) != nil { return currentAr, nil diff --git a/rollout/bluegreen.go b/rollout/bluegreen.go index 9c67c3c21c..08915a45bf 100644 --- a/rollout/bluegreen.go +++ b/rollout/bluegreen.go @@ -11,7 +11,6 @@ import ( "k8s.io/kubernetes/pkg/controller" "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" - analysisutil "github.com/argoproj/argo-rollouts/utils/analysis" "github.com/argoproj/argo-rollouts/utils/defaults" replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset" serviceutil "github.com/argoproj/argo-rollouts/utils/service" @@ -207,7 +206,7 @@ func (c *Controller) scaleDownOldReplicaSetsForBlueGreen(oldRSs []*appsv1.Replic return false, nil } if rollout.Spec.Strategy.BlueGreen.PostPromotionAnalysis != nil && rollout.Spec.Strategy.BlueGreen.ScaleDownDelaySeconds == nil { - currentPostAr := analysisutil.GetCurrentAnalysisRunByType(roCtx.CurrentAnalysisRuns(), v1alpha1.RolloutTypePostPromotionLabel) + currentPostAr := roCtx.CurrentAnalysisRuns().BlueGreenPostPromotion if currentPostAr == nil || currentPostAr.Status.Phase != v1alpha1.AnalysisPhaseSuccessful { logCtx.Infof("Cannot scale down old ReplicaSets while Analysis is running and no ScaleDownDelaySeconds") return false, nil @@ -299,8 +298,7 @@ func (c *Controller) syncRolloutStatusBlueGreen(previewSvc *corev1.Service, acti } postAnalysisRunFinished := false if r.Spec.Strategy.BlueGreen.PostPromotionAnalysis != nil { - ars := roCtx.CurrentAnalysisRuns() - currentPostPromotionAnalysisRun := analysisutil.GetCurrentAnalysisRunByType(ars, v1alpha1.RolloutTypePostPromotionLabel) + currentPostPromotionAnalysisRun := roCtx.CurrentAnalysisRuns().BlueGreenPostPromotion if currentPostPromotionAnalysisRun != nil { postAnalysisRunFinished = currentPostPromotionAnalysisRun.Status.Phase == v1alpha1.AnalysisPhaseSuccessful } diff --git a/rollout/canary.go b/rollout/canary.go index f7841abce1..d8129beae4 100644 --- a/rollout/canary.go +++ b/rollout/canary.go @@ -11,7 +11,6 @@ import ( "k8s.io/utils/pointer" "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" - analysisutil "github.com/argoproj/argo-rollouts/utils/analysis" "github.com/argoproj/argo-rollouts/utils/conditions" "github.com/argoproj/argo-rollouts/utils/defaults" logutil "github.com/argoproj/argo-rollouts/utils/log" @@ -243,8 +242,7 @@ func completedCurrentCanaryStep(roCtx *canaryContext) bool { if currentStep.Experiment != nil && experiment != nil && experiment.Status.Phase.Completed() && experiment.Status.Phase == v1alpha1.AnalysisPhaseSuccessful { return true } - currentArs := roCtx.CurrentAnalysisRuns() - currentStepAr := analysisutil.GetCurrentAnalysisRunByType(currentArs, v1alpha1.RolloutTypeStepLabel) + currentStepAr := roCtx.CurrentAnalysisRuns().CanaryStep analysisExistsAndCompleted := currentStepAr != nil && currentStepAr.Status.Phase.Completed() if currentStep.Analysis != nil && analysisExistsAndCompleted && currentStepAr.Status.Phase == v1alpha1.AnalysisPhaseSuccessful { return true diff --git a/rollout/context.go b/rollout/context.go index e83e049ea7..2ae0ec8a11 100644 --- a/rollout/context.go +++ b/rollout/context.go @@ -19,14 +19,14 @@ type rolloutContext interface { StableRS() *appsv1.ReplicaSet AllRSs() []*appsv1.ReplicaSet - CurrentAnalysisRuns() []*v1alpha1.AnalysisRun + CurrentAnalysisRuns() analysisutil.CurrentAnalysisRuns OtherAnalysisRuns() []*v1alpha1.AnalysisRun CurrentExperiment() *v1alpha1.Experiment OtherExperiments() []*v1alpha1.Experiment PauseContext() *pauseContext NewStatus() v1alpha1.RolloutStatus - SetCurrentAnalysisRuns([]*v1alpha1.AnalysisRun) + SetCurrentAnalysisRuns(analysisutil.CurrentAnalysisRuns) SetRestartedAt() } @@ -39,7 +39,7 @@ type blueGreenContext struct { olderRSs []*appsv1.ReplicaSet allRSs []*appsv1.ReplicaSet - currentArs []*v1alpha1.AnalysisRun + currentArs analysisutil.CurrentAnalysisRuns otherArs []*v1alpha1.AnalysisRun newStatus v1alpha1.RolloutStatus @@ -55,7 +55,7 @@ type canaryContext struct { olderRSs []*appsv1.ReplicaSet allRSs []*appsv1.ReplicaSet - currentArs []*v1alpha1.AnalysisRun + currentArs analysisutil.CurrentAnalysisRuns otherArs []*v1alpha1.AnalysisRun currentEx *v1alpha1.Experiment @@ -120,7 +120,7 @@ func (bgCtx *blueGreenContext) AllRSs() []*appsv1.ReplicaSet { func (bgCtx *blueGreenContext) CurrentExperiment() *v1alpha1.Experiment { return nil } -func (bgCtx *blueGreenContext) CurrentAnalysisRuns() []*v1alpha1.AnalysisRun { +func (bgCtx *blueGreenContext) CurrentAnalysisRuns() analysisutil.CurrentAnalysisRuns { return bgCtx.currentArs } @@ -128,20 +128,18 @@ func (bgCtx *blueGreenContext) OtherAnalysisRuns() []*v1alpha1.AnalysisRun { return bgCtx.otherArs } -func (bgCtx *blueGreenContext) SetCurrentAnalysisRuns(ars []*v1alpha1.AnalysisRun) { - bgCtx.currentArs = ars - currPrePromoAr := analysisutil.GetCurrentAnalysisRunByType(ars, v1alpha1.RolloutTypePrePromotionLabel) - if currPrePromoAr != nil && !bgCtx.PauseContext().IsAborted() { - switch currPrePromoAr.Status.Phase { +func (bgCtx *blueGreenContext) SetCurrentAnalysisRuns(currAr analysisutil.CurrentAnalysisRuns) { + bgCtx.currentArs = currAr + if currAr.BlueGreenPrePromotion != nil && !bgCtx.PauseContext().IsAborted() { + switch currAr.BlueGreenPrePromotion.Status.Phase { case v1alpha1.AnalysisPhasePending, v1alpha1.AnalysisPhaseRunning, v1alpha1.AnalysisPhaseSuccessful, "": - bgCtx.newStatus.BlueGreen.PrePromotionAnalysisRun = currPrePromoAr.Name + bgCtx.newStatus.BlueGreen.PrePromotionAnalysisRun = currAr.BlueGreenPrePromotion.Name } } - currPostPromoAr := analysisutil.GetCurrentAnalysisRunByType(ars, v1alpha1.RolloutTypePostPromotionLabel) - if currPostPromoAr != nil && !bgCtx.PauseContext().IsAborted() { - switch currPostPromoAr.Status.Phase { + if currAr.BlueGreenPostPromotion != nil && !bgCtx.PauseContext().IsAborted() { + switch currAr.BlueGreenPostPromotion.Status.Phase { case v1alpha1.AnalysisPhasePending, v1alpha1.AnalysisPhaseRunning, v1alpha1.AnalysisPhaseSuccessful, "": - bgCtx.newStatus.BlueGreen.PostPromotionAnalysisRun = currPostPromoAr.Name + bgCtx.newStatus.BlueGreen.PostPromotionAnalysisRun = currAr.BlueGreenPostPromotion.Name } } } @@ -219,25 +217,23 @@ func (cCtx *canaryContext) AllRSs() []*appsv1.ReplicaSet { return cCtx.allRSs } -func (cCtx *canaryContext) SetCurrentAnalysisRuns(ars []*v1alpha1.AnalysisRun) { - cCtx.currentArs = ars - currBackgroundAr := analysisutil.GetCurrentAnalysisRunByType(ars, v1alpha1.RolloutTypeBackgroundRunLabel) - if currBackgroundAr != nil && !cCtx.PauseContext().IsAborted() { - switch currBackgroundAr.Status.Phase { +func (cCtx *canaryContext) SetCurrentAnalysisRuns(currARs analysisutil.CurrentAnalysisRuns) { + cCtx.currentArs = currARs + if currARs.CanaryBackground != nil && !cCtx.PauseContext().IsAborted() { + switch currARs.CanaryBackground.Status.Phase { case v1alpha1.AnalysisPhasePending, v1alpha1.AnalysisPhaseRunning, v1alpha1.AnalysisPhaseSuccessful, "": - cCtx.newStatus.Canary.CurrentBackgroundAnalysisRun = currBackgroundAr.Name + cCtx.newStatus.Canary.CurrentBackgroundAnalysisRun = currARs.CanaryBackground.Name } } - currStepAr := analysisutil.GetCurrentAnalysisRunByType(ars, v1alpha1.RolloutTypeStepLabel) - if currStepAr != nil && !cCtx.PauseContext().IsAborted() { - if !currStepAr.Status.Phase.Completed() { - cCtx.newStatus.Canary.CurrentStepAnalysisRun = currStepAr.Name + if currARs.CanaryStep != nil && !cCtx.PauseContext().IsAborted() { + if !currARs.CanaryStep.Status.Phase.Completed() { + cCtx.newStatus.Canary.CurrentStepAnalysisRun = currARs.CanaryStep.Name } } } -func (cCtx *canaryContext) CurrentAnalysisRuns() []*v1alpha1.AnalysisRun { +func (cCtx *canaryContext) CurrentAnalysisRuns() analysisutil.CurrentAnalysisRuns { return cCtx.currentArs } func (cCtx *canaryContext) OtherAnalysisRuns() []*v1alpha1.AnalysisRun { diff --git a/rollout/pause.go b/rollout/pause.go index b8e066e2f8..876736820b 100644 --- a/rollout/pause.go +++ b/rollout/pause.go @@ -7,7 +7,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" - analysisutil "github.com/argoproj/argo-rollouts/utils/analysis" logutil "github.com/argoproj/argo-rollouts/utils/log" ) @@ -137,7 +136,7 @@ func completedPrePromotionAnalysis(roCtx *blueGreenContext) bool { return false } - currentAr := analysisutil.FilterAnalysisRunsByName(roCtx.CurrentAnalysisRuns(), rollout.Status.BlueGreen.PrePromotionAnalysisRun) + currentAr := roCtx.CurrentAnalysisRuns().BlueGreenPrePromotion if currentAr != nil && currentAr.Status.Phase == v1alpha1.AnalysisPhaseSuccessful { return true } diff --git a/utils/analysis/filter.go b/utils/analysis/filter.go index b119f13745..87ed427f98 100644 --- a/utils/analysis/filter.go +++ b/utils/analysis/filter.go @@ -18,22 +18,28 @@ func GetCurrentAnalysisRunByType(currentArs []*v1alpha1.AnalysisRun, kind string } // FilterCurrentRolloutAnalysisRuns returns analysisRuns that match the analysisRuns listed in the rollout status -func FilterCurrentRolloutAnalysisRuns(analysisRuns []*v1alpha1.AnalysisRun, r *v1alpha1.Rollout) ([]*v1alpha1.AnalysisRun, []*v1alpha1.AnalysisRun) { - return FilterAnalysisRuns(analysisRuns, func(ar *v1alpha1.AnalysisRun) bool { - if ar.Name == r.Status.Canary.CurrentStepAnalysisRun { - return true - } - if ar.Name == r.Status.Canary.CurrentBackgroundAnalysisRun { - return true - } - if ar.Name == r.Status.BlueGreen.PrePromotionAnalysisRun { - return true - } - if ar.Name == r.Status.BlueGreen.PostPromotionAnalysisRun { - return true +func FilterCurrentRolloutAnalysisRuns(analysisRuns []*v1alpha1.AnalysisRun, r *v1alpha1.Rollout) (CurrentAnalysisRuns, []*v1alpha1.AnalysisRun) { + currArs := CurrentAnalysisRuns{} + otherArs := []*v1alpha1.AnalysisRun{} + for i := range analysisRuns { + ar := analysisRuns[i] + if ar != nil { + + switch ar.Name { + case r.Status.Canary.CurrentStepAnalysisRun: + currArs.CanaryStep = ar + case r.Status.Canary.CurrentBackgroundAnalysisRun: + currArs.CanaryBackground = ar + case r.Status.BlueGreen.PrePromotionAnalysisRun: + currArs.BlueGreenPrePromotion = ar + case r.Status.BlueGreen.PostPromotionAnalysisRun: + currArs.BlueGreenPostPromotion = ar + default: + otherArs = append(otherArs, ar) + } } - return false - }) + } + return currArs, otherArs } // FilterAnalysisRunsByRolloutType returns a list of analysisRuns that have the rollout-type of the typeFilter diff --git a/utils/analysis/filter_test.go b/utils/analysis/filter_test.go index a2019f0364..c9567c807c 100644 --- a/utils/analysis/filter_test.go +++ b/utils/analysis/filter_test.go @@ -67,10 +67,11 @@ func TestFilterCurrentRolloutAnalysisRuns(t *testing.T) { }, } currentArs, nonCurrentArs := FilterCurrentRolloutAnalysisRuns(ars, r) - assert.Len(t, currentArs, 2) assert.Len(t, nonCurrentArs, 1) - assert.Contains(t, currentArs, ars[0]) - assert.Contains(t, currentArs, ars[1]) + assert.Equal(t, currentArs.CanaryStep, ars[0]) + assert.Equal(t, currentArs.CanaryBackground, ars[1]) + assert.Nil(t, currentArs.BlueGreenPostPromotion) + assert.Nil(t, currentArs.BlueGreenPrePromotion) }) t.Run("BlueGreen", func(t *testing.T) { @@ -83,10 +84,11 @@ func TestFilterCurrentRolloutAnalysisRuns(t *testing.T) { }, } currentArs, nonCurrentArs := FilterCurrentRolloutAnalysisRuns(ars, r) - assert.Len(t, currentArs, 2) assert.Len(t, nonCurrentArs, 1) - assert.Contains(t, currentArs, ars[0]) - assert.Contains(t, currentArs, ars[1]) + assert.Equal(t, currentArs.BlueGreenPrePromotion, ars[0]) + assert.Equal(t, currentArs.BlueGreenPostPromotion, ars[1]) + assert.Nil(t, currentArs.CanaryBackground) + assert.Nil(t, currentArs.CanaryStep) }) } diff --git a/utils/analysis/helpers.go b/utils/analysis/helpers.go index 3d0ff21f0d..5084f5ca5f 100644 --- a/utils/analysis/helpers.go +++ b/utils/analysis/helpers.go @@ -16,6 +16,31 @@ import ( argoprojclient "github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/typed/rollouts/v1alpha1" ) +// CurrentAnalysisRuns holds all the current analysis runs for a Rollout +type CurrentAnalysisRuns struct { + BlueGreenPrePromotion *v1alpha1.AnalysisRun + BlueGreenPostPromotion *v1alpha1.AnalysisRun + CanaryStep *v1alpha1.AnalysisRun + CanaryBackground *v1alpha1.AnalysisRun +} + +func (c CurrentAnalysisRuns) ToArray() []*v1alpha1.AnalysisRun { + currentAnalysisRuns := []*v1alpha1.AnalysisRun{} + if c.BlueGreenPostPromotion != nil { + currentAnalysisRuns = append(currentAnalysisRuns, c.BlueGreenPostPromotion) + } + if c.BlueGreenPrePromotion != nil { + currentAnalysisRuns = append(currentAnalysisRuns, c.BlueGreenPrePromotion) + } + if c.CanaryStep != nil { + currentAnalysisRuns = append(currentAnalysisRuns, c.CanaryStep) + } + if c.CanaryBackground != nil { + currentAnalysisRuns = append(currentAnalysisRuns, c.CanaryBackground) + } + return currentAnalysisRuns +} + // analysisStatusOrder is a list of completed analysis sorted in best to worst condition var analysisStatusOrder = []v1alpha1.AnalysisPhase{ v1alpha1.AnalysisPhaseSuccessful, From 00bca844f12facd82ddd6db237a7638e79d6741f Mon Sep 17 00:00:00 2001 From: Danny Thomson Date: Tue, 7 Jul 2020 15:58:23 -0700 Subject: [PATCH 2/4] Add more analysis info to Rollout --- manifests/crds/rollout-crd.yaml | 48 +++++ manifests/install.yaml | 48 +++++ manifests/namespace-install.yaml | 48 +++++ .../rollouts/v1alpha1/openapi_generated.go | 60 +++++- pkg/apis/rollouts/v1alpha1/types.go | 23 +++ .../v1alpha1/zz_generated.deepcopy.go | 38 +++- rollout/analysis.go | 99 +++++++++- rollout/analysis_test.go | 180 ++++++++++++++---- rollout/bluegreen.go | 5 +- rollout/bluegreen_test.go | 1 + rollout/canary.go | 2 + rollout/canary_test.go | 2 + rollout/context.go | 44 +++-- rollout/controller_test.go | 2 + rollout/experiment_test.go | 1 + rollout/pause.go | 3 + rollout/sync.go | 2 + utils/analysis/filter.go | 1 - 18 files changed, 543 insertions(+), 64 deletions(-) diff --git a/manifests/crds/rollout-crd.yaml b/manifests/crds/rollout-crd.yaml index d8c1f7c174..ea567f5a81 100644 --- a/manifests/crds/rollout-crd.yaml +++ b/manifests/crds/rollout-crd.yaml @@ -2918,8 +2918,32 @@ spec: type: string postPromotionAnalysisRun: type: string + postPromotionAnalysisRunStatus: + properties: + message: + type: string + name: + type: string + status: + type: string + required: + - name + - status + type: object prePromotionAnalysisRun: type: string + prePromotionAnalysisRunStatus: + properties: + message: + type: string + name: + type: string + status: + type: string + required: + - name + - status + type: object previewSelector: type: string previousActiveSelector: @@ -2934,10 +2958,34 @@ spec: properties: currentBackgroundAnalysisRun: type: string + currentBackgroundAnalysisRunStatus: + properties: + message: + type: string + name: + type: string + status: + type: string + required: + - name + - status + type: object currentExperiment: type: string currentStepAnalysisRun: type: string + currentStepAnalysisRunStatus: + properties: + message: + type: string + name: + type: string + status: + type: string + required: + - name + - status + type: object stableRS: type: string type: object diff --git a/manifests/install.yaml b/manifests/install.yaml index e2bd3b4c85..6440c3ceff 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -13867,8 +13867,32 @@ spec: type: string postPromotionAnalysisRun: type: string + postPromotionAnalysisRunStatus: + properties: + message: + type: string + name: + type: string + status: + type: string + required: + - name + - status + type: object prePromotionAnalysisRun: type: string + prePromotionAnalysisRunStatus: + properties: + message: + type: string + name: + type: string + status: + type: string + required: + - name + - status + type: object previewSelector: type: string previousActiveSelector: @@ -13883,10 +13907,34 @@ spec: properties: currentBackgroundAnalysisRun: type: string + currentBackgroundAnalysisRunStatus: + properties: + message: + type: string + name: + type: string + status: + type: string + required: + - name + - status + type: object currentExperiment: type: string currentStepAnalysisRun: type: string + currentStepAnalysisRunStatus: + properties: + message: + type: string + name: + type: string + status: + type: string + required: + - name + - status + type: object stableRS: type: string type: object diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index 1d263d3aa2..4c04323037 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -13867,8 +13867,32 @@ spec: type: string postPromotionAnalysisRun: type: string + postPromotionAnalysisRunStatus: + properties: + message: + type: string + name: + type: string + status: + type: string + required: + - name + - status + type: object prePromotionAnalysisRun: type: string + prePromotionAnalysisRunStatus: + properties: + message: + type: string + name: + type: string + status: + type: string + required: + - name + - status + type: object previewSelector: type: string previousActiveSelector: @@ -13883,10 +13907,34 @@ spec: properties: currentBackgroundAnalysisRun: type: string + currentBackgroundAnalysisRunStatus: + properties: + message: + type: string + name: + type: string + status: + type: string + required: + - name + - status + type: object currentExperiment: type: string currentStepAnalysisRun: type: string + currentStepAnalysisRunStatus: + properties: + message: + type: string + name: + type: string + status: + type: string + required: + - name + - status + type: object stableRS: type: string type: object diff --git a/pkg/apis/rollouts/v1alpha1/openapi_generated.go b/pkg/apis/rollouts/v1alpha1/openapi_generated.go index 7616462c70..5336004ddd 100644 --- a/pkg/apis/rollouts/v1alpha1/openapi_generated.go +++ b/pkg/apis/rollouts/v1alpha1/openapi_generated.go @@ -74,6 +74,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.Rollout": schema_pkg_apis_rollouts_v1alpha1_Rollout(ref), "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutAnalysis": schema_pkg_apis_rollouts_v1alpha1_RolloutAnalysis(ref), "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutAnalysisBackground": schema_pkg_apis_rollouts_v1alpha1_RolloutAnalysisBackground(ref), + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutAnalysisRunStatus": schema_pkg_apis_rollouts_v1alpha1_RolloutAnalysisRunStatus(ref), "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutAnalysisTemplates": schema_pkg_apis_rollouts_v1alpha1_RolloutAnalysisTemplates(ref), "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutCondition": schema_pkg_apis_rollouts_v1alpha1_RolloutCondition(ref), "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutExperimentStep": schema_pkg_apis_rollouts_v1alpha1_RolloutExperimentStep(ref), @@ -640,6 +641,12 @@ func schema_pkg_apis_rollouts_v1alpha1_BlueGreenStatus(ref common.ReferenceCallb Format: "", }, }, + "prePromotionAnalysisRunStatus": { + SchemaProps: spec.SchemaProps{ + Description: "PrePromotionAnalysisRunStatus indicates the status of the current prepromotion analysis run", + Ref: ref("github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutAnalysisRunStatus"), + }, + }, "postPromotionAnalysisRun": { SchemaProps: spec.SchemaProps{ Description: "PostPromotionAnalysisRun is the current analysis run running after the active service promotion", @@ -647,11 +654,17 @@ func schema_pkg_apis_rollouts_v1alpha1_BlueGreenStatus(ref common.ReferenceCallb Format: "", }, }, + "postPromotionAnalysisRunStatus": { + SchemaProps: spec.SchemaProps{ + Description: "PostPromotionAnalysisRunStatus indicates the status of the current post promotion analysis run", + Ref: ref("github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutAnalysisRunStatus"), + }, + }, }, }, }, Dependencies: []string{ - "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutAnalysisRunStatus", "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, } } @@ -759,6 +772,12 @@ func schema_pkg_apis_rollouts_v1alpha1_CanaryStatus(ref common.ReferenceCallback Format: "", }, }, + "currentStepAnalysisRunStatus": { + SchemaProps: spec.SchemaProps{ + Description: "CurrentStepAnalysisRunStatus indicates the status of the current step analysis run", + Ref: ref("github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutAnalysisRunStatus"), + }, + }, "currentBackgroundAnalysisRun": { SchemaProps: spec.SchemaProps{ Description: "CurrentBackgroundAnalysisRun indicates the analysisRun for the Background step", @@ -766,6 +785,12 @@ func schema_pkg_apis_rollouts_v1alpha1_CanaryStatus(ref common.ReferenceCallback Format: "", }, }, + "currentBackgroundAnalysisRunStatus": { + SchemaProps: spec.SchemaProps{ + Description: "CurrentStepAnalysisRunStatus indicates the status of the current background analysis run", + Ref: ref("github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutAnalysisRunStatus"), + }, + }, "currentExperiment": { SchemaProps: spec.SchemaProps{ Description: "CurrentExperiment indicates the running experiment", @@ -776,6 +801,8 @@ func schema_pkg_apis_rollouts_v1alpha1_CanaryStatus(ref common.ReferenceCallback }, }, }, + Dependencies: []string{ + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutAnalysisRunStatus"}, } } @@ -2220,6 +2247,37 @@ func schema_pkg_apis_rollouts_v1alpha1_RolloutAnalysisBackground(ref common.Refe } } +func schema_pkg_apis_rollouts_v1alpha1_RolloutAnalysisRunStatus(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "name": { + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + "status": { + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + "message": { + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + }, + Required: []string{"name", "status"}, + }, + }, + } +} + func schema_pkg_apis_rollouts_v1alpha1_RolloutAnalysisTemplates(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ diff --git a/pkg/apis/rollouts/v1alpha1/types.go b/pkg/apis/rollouts/v1alpha1/types.go index 24c6d290d7..aaae5659b4 100644 --- a/pkg/apis/rollouts/v1alpha1/types.go +++ b/pkg/apis/rollouts/v1alpha1/types.go @@ -476,6 +476,11 @@ type RolloutStatus struct { PauseConditions []PauseCondition `json:"pauseConditions,omitempty"` //ControllerPause indicates the controller has paused the rollout ControllerPause bool `json:"controllerPause,omitempty"` + // ReconciledAbort indicates the controller reconciled an aborted rollout. The controller uses this to understand if + // the controller needs to do some specific work when a Rollout is aborted. For example, the reconcileAbort is used + // to indicate if the Rollout should enter an aborted state when the latest AnalysisRun is a failure, or the controller + // has already put the Rollout into an aborted and should create a new AnalysisRun. + ReconciledAbort bool `json:"reconciledAbort,omitempty"` // CurrentPodHash the hash of the current pod template // +optional CurrentPodHash string `json:"currentPodHash,omitempty"` @@ -550,9 +555,15 @@ type BlueGreenStatus struct { // +optional ScaleUpPreviewCheckPoint bool `json:"scaleUpPreviewCheckPoint,omitempty"` // PrePromotionAnalysisRun is the current analysis run running before the active service promotion + // TODO(Depreciated): Remove in v0.10 PrePromotionAnalysisRun string `json:"prePromotionAnalysisRun,omitempty"` + // PrePromotionAnalysisRunStatus indicates the status of the current prepromotion analysis run + PrePromotionAnalysisRunStatus *RolloutAnalysisRunStatus `json:"prePromotionAnalysisRunStatus,omitempty"` // PostPromotionAnalysisRun is the current analysis run running after the active service promotion + // TODO(Depreciated): Remove in v0.10 PostPromotionAnalysisRun string `json:"postPromotionAnalysisRun,omitempty"` + // PostPromotionAnalysisRunStatus indicates the status of the current post promotion analysis run + PostPromotionAnalysisRunStatus *RolloutAnalysisRunStatus `json:"postPromotionAnalysisRunStatus,omitempty"` } // CanaryStatus status fields that only pertain to the canary rollout @@ -561,13 +572,25 @@ type CanaryStatus struct { // +optional StableRS string `json:"stableRS,omitempty"` // CurrentStepAnalysisRun indicates the analysisRun for the current step index + // TODO(Depreciated): Remove in v0.10 CurrentStepAnalysisRun string `json:"currentStepAnalysisRun,omitempty"` + // CurrentStepAnalysisRunStatus indicates the status of the current step analysis run + CurrentStepAnalysisRunStatus *RolloutAnalysisRunStatus `json:"currentStepAnalysisRunStatus,omitempty"` // CurrentBackgroundAnalysisRun indicates the analysisRun for the Background step + // TODO(Depreciated): Remove in v0.10 CurrentBackgroundAnalysisRun string `json:"currentBackgroundAnalysisRun,omitempty"` + // CurrentStepAnalysisRunStatus indicates the status of the current background analysis run + CurrentBackgroundAnalysisRunStatus *RolloutAnalysisRunStatus `json:"currentBackgroundAnalysisRunStatus,omitempty"` // CurrentExperiment indicates the running experiment CurrentExperiment string `json:"currentExperiment,omitempty"` } +type RolloutAnalysisRunStatus struct { + Name string `json:"name"` + Status AnalysisPhase `json:"status"` + Message string `json:"message,omitempty"` +} + // RolloutConditionType defines the conditions of Rollout type RolloutConditionType string diff --git a/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go index b65ff9a67d..545157ce92 100644 --- a/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go @@ -351,6 +351,16 @@ func (in *BlueGreenStatus) DeepCopyInto(out *BlueGreenStatus) { in, out := &in.ScaleDownDelayStartTime, &out.ScaleDownDelayStartTime *out = (*in).DeepCopy() } + if in.PrePromotionAnalysisRunStatus != nil { + in, out := &in.PrePromotionAnalysisRunStatus, &out.PrePromotionAnalysisRunStatus + *out = new(RolloutAnalysisRunStatus) + **out = **in + } + if in.PostPromotionAnalysisRunStatus != nil { + in, out := &in.PostPromotionAnalysisRunStatus, &out.PostPromotionAnalysisRunStatus + *out = new(RolloutAnalysisRunStatus) + **out = **in + } return } @@ -423,6 +433,16 @@ func (in *BlueGreenStrategy) DeepCopy() *BlueGreenStrategy { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CanaryStatus) DeepCopyInto(out *CanaryStatus) { *out = *in + if in.CurrentStepAnalysisRunStatus != nil { + in, out := &in.CurrentStepAnalysisRunStatus, &out.CurrentStepAnalysisRunStatus + *out = new(RolloutAnalysisRunStatus) + **out = **in + } + if in.CurrentBackgroundAnalysisRunStatus != nil { + in, out := &in.CurrentBackgroundAnalysisRunStatus, &out.CurrentBackgroundAnalysisRunStatus + *out = new(RolloutAnalysisRunStatus) + **out = **in + } return } @@ -1201,6 +1221,22 @@ func (in *RolloutAnalysisBackground) DeepCopy() *RolloutAnalysisBackground { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RolloutAnalysisRunStatus) DeepCopyInto(out *RolloutAnalysisRunStatus) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RolloutAnalysisRunStatus. +func (in *RolloutAnalysisRunStatus) DeepCopy() *RolloutAnalysisRunStatus { + if in == nil { + return nil + } + out := new(RolloutAnalysisRunStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RolloutAnalysisTemplates) DeepCopyInto(out *RolloutAnalysisTemplates) { *out = *in @@ -1438,7 +1474,7 @@ func (in *RolloutStatus) DeepCopyInto(out *RolloutStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - out.Canary = in.Canary + in.Canary.DeepCopyInto(&out.Canary) in.BlueGreen.DeepCopyInto(&out.BlueGreen) if in.RestartedAt != nil { in, out := &in.RestartedAt, &out.RestartedAt diff --git a/rollout/analysis.go b/rollout/analysis.go index 572591c594..2d99e58893 100644 --- a/rollout/analysis.go +++ b/rollout/analysis.go @@ -5,6 +5,8 @@ import ( "strconv" "strings" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -47,6 +49,7 @@ func (c *Controller) reconcileAnalysisRuns(roCtx rolloutContext) error { otherArs := roCtx.OtherAnalysisRuns() if roCtx.PauseContext().IsAborted() { allArs := append(roCtx.CurrentAnalysisRuns().ToArray(), otherArs...) + roCtx.SetCurrentAnalysisRuns(roCtx.CurrentAnalysisRuns()) return c.cancelAnalysisRuns(roCtx, allArs) } @@ -108,9 +111,80 @@ func (c *Controller) reconcileAnalysisRuns(roCtx rolloutContext) error { return err } + c.reconcileAnalysisRunStatusChanges(roCtx, newCurrentAnalysisRuns) return nil } +func needsNewAnalysisRun(currentAr *v1alpha1.AnalysisRun, rollout *v1alpha1.Rollout) bool { + if currentAr == nil { + return true + } + + // Here the controller is checking that rollout has already paused for a inconclusive analysis run. If it has paused + // for the inconclusive analysisrun, the controller needs to create a new AnalysisRun. Otherwise, the controller has + // not processed the inconclusive run yet (and needs to add a pause). It checks this by seeing if the controllerPause + // is set and then seeing if the last status was inconclusive. + // There is an additional check for the BlueGreen Pause because the prepromotion analysis always has the BlueGreen + // Pause and that causes controllerPause to be set. The extra check for the BlueGreen Pause ensures that a new Analysis + // Run is created only when the previous AnalysisRun is inconclusive + if rollout.Status.ControllerPause && getPauseCondition(rollout, v1alpha1.PauseReasonBlueGreenPause) == nil { + return currentAr.Status.Phase == v1alpha1.AnalysisPhaseInconclusive + } + return rollout.Status.ReconciledAbort +} + +// emitAnalysisRunStatusChanges emits a Kubernetes event if the analysis run of that type has changed status +func (c *Controller) emitAnalysisRunStatusChanges(r *v1alpha1.Rollout, prevStatus *v1alpha1.RolloutAnalysisRunStatus, ar *v1alpha1.AnalysisRun, arType string) { + if ar != nil { + if prevStatus == nil || prevStatus.Name == ar.Name && prevStatus.Status != ar.Status.Phase { + prevStatusStr := "NoPreviousStatus" + if prevStatus != nil { + prevStatusStr = string(prevStatus.Status) + } + + eventType := corev1.EventTypeNormal + if ar.Status.Phase == v1alpha1.AnalysisPhaseFailed || ar.Status.Phase == v1alpha1.AnalysisPhaseError { + eventType = corev1.EventTypeWarning + } + msg := fmt.Sprintf("%s Analysis Run '%s' Status New: '%s' Previous: '%s'", arType, ar.Name, ar.Status.Phase, prevStatusStr) + c.recorder.Event(r, eventType, "AnalysisRunStatusChange", msg) + } + } +} + +// reconcileAnalysisRunStatusChanges for each analysisRun type, the controller checks if the analysis run status has changed +// for that type +func (c *Controller) reconcileAnalysisRunStatusChanges(ctx rolloutContext, currARs analysisutil.CurrentAnalysisRuns) { + rollout := ctx.Rollout() + c.emitAnalysisRunStatusChanges( + rollout, + rollout.Status.BlueGreen.PostPromotionAnalysisRunStatus, + currARs.BlueGreenPostPromotion, + v1alpha1.RolloutTypePostPromotionLabel, + ) + + c.emitAnalysisRunStatusChanges( + rollout, + rollout.Status.BlueGreen.PrePromotionAnalysisRunStatus, + currARs.BlueGreenPrePromotion, + v1alpha1.RolloutTypePrePromotionLabel, + ) + + c.emitAnalysisRunStatusChanges( + rollout, + rollout.Status.Canary.CurrentStepAnalysisRunStatus, + currARs.CanaryStep, + v1alpha1.RolloutTypeStepLabel, + ) + + c.emitAnalysisRunStatusChanges( + rollout, + rollout.Status.Canary.CurrentBackgroundAnalysisRunStatus, + currARs.CanaryBackground, + v1alpha1.RolloutTypeBackgroundRunLabel, + ) +} + func (c *Controller) reconcilePrePromotionAnalysisRun(roCtx rolloutContext) (*v1alpha1.AnalysisRun, error) { rollout := roCtx.Rollout() newRS := roCtx.NewRS() @@ -134,7 +208,7 @@ func (c *Controller) reconcilePrePromotionAnalysisRun(roCtx rolloutContext) (*v1 return currentAr, nil } - if currentAr == nil { + if needsNewAnalysisRun(currentAr, rollout) { podHash := replicasetutil.GetPodTemplateHash(newRS) instanceID := analysisutil.GetInstanceID(rollout) prePromotionLabels := analysisutil.PrePromotionLabels(podHash, instanceID) @@ -153,6 +227,15 @@ func (c *Controller) reconcilePrePromotionAnalysisRun(roCtx rolloutContext) (*v1 return currentAr, nil } +// needPostPromotionAnalysisRun indicates if the controller needs to create an analysis run by checking that the desired +// ReplicaSet is the stable ReplicaSet, the active service promotion has not happened, the rollout was just created, or +// the newRS is not saturated +func needPostPromotionAnalysisRun(rollout *v1alpha1.Rollout, newRS *appsv1.ReplicaSet) bool { + currentPodHash := rollout.Status.CurrentPodHash + activeSelector := rollout.Status.BlueGreen.ActiveSelector + return rollout.Status.StableRS == currentPodHash || activeSelector != currentPodHash || currentPodHash == "" || !annotations.IsSaturated(rollout, newRS) +} + func (c *Controller) reconcilePostPromotionAnalysisRun(roCtx rolloutContext) (*v1alpha1.AnalysisRun, error) { rollout := roCtx.Rollout() newRS := roCtx.NewRS() @@ -164,11 +247,7 @@ func (c *Controller) reconcilePostPromotionAnalysisRun(roCtx rolloutContext) (*v } roCtx.Log().Info("Reconciling Post Promotion Analysis") - currentPodHash := rollout.Status.CurrentPodHash - activeSelector := rollout.Status.BlueGreen.ActiveSelector - // Do not create an analysis run if the desired ReplicaSet is the stable ReplicaSet, the active service promotion - // has not happened, the rollout was just created, or the newRS is not saturated - if rollout.Status.StableRS == currentPodHash || activeSelector != currentPodHash || currentPodHash == "" || !annotations.IsSaturated(rollout, newRS) { + if needPostPromotionAnalysisRun(rollout, newRS) { err := c.cancelAnalysisRuns(roCtx, []*v1alpha1.AnalysisRun{currentAr}) return nil, err } @@ -177,7 +256,7 @@ func (c *Controller) reconcilePostPromotionAnalysisRun(roCtx rolloutContext) (*v return currentAr, nil } - if currentAr == nil { + if needsNewAnalysisRun(currentAr, rollout) { podHash := replicasetutil.GetPodTemplateHash(newRS) instanceID := analysisutil.GetInstanceID(rollout) postPromotionLabels := analysisutil.PostPromotionLabels(podHash, instanceID) @@ -215,7 +294,7 @@ func (c *Controller) reconcileBackgroundAnalysisRun(roCtx rolloutContext) (*v1al return currentAr, nil } - if currentAr == nil { + if needsNewAnalysisRun(currentAr, rollout) { podHash := replicasetutil.GetPodTemplateHash(newRS) instanceID := analysisutil.GetInstanceID(rollout) backgroundLabels := analysisutil.BackgroundLabels(podHash, instanceID) @@ -257,7 +336,7 @@ func (c *Controller) reconcileStepBasedAnalysisRun(roCtx rolloutContext) (*v1alp step, index := replicasetutil.GetCurrentCanaryStep(rollout) currentAr := currentArs.CanaryStep - if getPauseCondition(rollout, v1alpha1.PauseReasonInconclusiveAnalysis) != nil { + if len(rollout.Status.PauseConditions) > 0 || rollout.Status.Abort { return currentAr, nil } @@ -265,7 +344,7 @@ func (c *Controller) reconcileStepBasedAnalysisRun(roCtx rolloutContext) (*v1alp err := c.cancelAnalysisRuns(roCtx, []*v1alpha1.AnalysisRun{currentAr}) return nil, err } - if currentAr == nil { + if needsNewAnalysisRun(currentAr, rollout) { podHash := replicasetutil.GetPodTemplateHash(newRS) instanceID := analysisutil.GetInstanceID(rollout) stepLabels := analysisutil.StepLabels(*index, podHash, instanceID) diff --git a/rollout/analysis_test.go b/rollout/analysis_test.go index 22fb15c1c9..a115436bf1 100644 --- a/rollout/analysis_test.go +++ b/rollout/analysis_test.go @@ -153,11 +153,15 @@ func TestCreateBackgroundAnalysisRun(t *testing.T) { expectedPatch := `{ "status": { "canary": { - "currentBackgroundAnalysisRun": "%s" + "currentBackgroundAnalysisRun": "%s", + "currentBackgroundAnalysisRunStatus": { + "name": "%s", + "status": "" + } } } }` - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName, expectedArName)), patch) } func TestCreateBackgroundAnalysisRunWithTemplates(t *testing.T) { @@ -377,11 +381,15 @@ func TestCreateBackgroundAnalysisRunWithClusterTemplatesAndTemplate(t *testing.T expectedPatch := `{ "status": { "canary": { - "currentBackgroundAnalysisRun": "%s" + "currentBackgroundAnalysisRun": "%s", + "currentBackgroundAnalysisRunStatus": { + "name": "%s", + "status": "" + } } } }` - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName, expectedArName)), patch) } // TestCreateAnalysisRunWithCollision ensures we will create an new analysis run with a new name @@ -439,11 +447,15 @@ func TestCreateAnalysisRunWithCollision(t *testing.T) { expectedPatch := `{ "status": { "canary": { - "currentBackgroundAnalysisRun": "%s" + "currentBackgroundAnalysisRun": "%s", + "currentBackgroundAnalysisRunStatus": { + "name": "%s", + "status": "" + } } } }` - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedAR.Name)), patch) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedAR.Name, expectedAR.Name)), patch) } // TestCreateAnalysisRunWithCollisionAndSemanticEquality will ensure we do not create an extra @@ -493,11 +505,15 @@ func TestCreateAnalysisRunWithCollisionAndSemanticEquality(t *testing.T) { expectedPatch := `{ "status": { "canary": { - "currentBackgroundAnalysisRun": "%s" + "currentBackgroundAnalysisRun": "%s", + "currentBackgroundAnalysisRunStatus": { + "name": "%s", + "status": "" + } } } }` - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, ar.Name)), patch) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, ar.Name, ar.Name)), patch) } func TestCreateAnalysisRunOnAnalysisStep(t *testing.T) { @@ -514,6 +530,7 @@ func TestCreateAnalysisRunOnAnalysisStep(t *testing.T) { r1 := newCanaryRollout("foo", 1, nil, steps, pointer.Int32Ptr(0), intstr.FromInt(0), intstr.FromInt(1)) r2 := bumpVersion(r1) ar := analysisRun(at, v1alpha1.RolloutTypeStepLabel, r2) + ar.Status.Phase = v1alpha1.AnalysisPhaseRunning rs1 := newReplicaSetWithStatus(r1, 1, 1) rs2 := newReplicaSetWithStatus(r2, 0, 0) @@ -544,11 +561,15 @@ func TestCreateAnalysisRunOnAnalysisStep(t *testing.T) { expectedPatch := `{ "status": { "canary": { - "currentStepAnalysisRun": "%s" + "currentStepAnalysisRun": "%s", + "currentStepAnalysisRunStatus": { + "name": "%s", + "status": "" + } } } }` - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName, expectedArName)), patch) } func TestFailCreateStepAnalysisRunIfInvalidTemplateRef(t *testing.T) { @@ -571,6 +592,10 @@ func TestFailCreateStepAnalysisRunIfInvalidTemplateRef(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] r2 = updateCanaryRolloutStatus(r2, rs1PodHash, 1, 0, 1, false) + r2.Status.Canary.CurrentStepAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: ar.Name, + Status: "", + } progressingCondition, _ := newProgressingCondition(conditions.ReplicaSetUpdatedReason, rs2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) @@ -714,6 +739,7 @@ func TestDoNothingWithAnalysisRunsWhileBackgroundAnalysisRunRunning(t *testing.T }, } ar := analysisRun(at, v1alpha1.RolloutTypeBackgroundRunLabel, r2) + ar.Status.Phase = v1alpha1.AnalysisPhaseRunning rs1 := newReplicaSetWithStatus(r1, 1, 1) rs2 := newReplicaSetWithStatus(r2, 0, 0) @@ -727,6 +753,10 @@ func TestDoNothingWithAnalysisRunsWhileBackgroundAnalysisRunRunning(t *testing.T availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) r2.Status.Canary.CurrentBackgroundAnalysisRun = ar.Name + r2.Status.Canary.CurrentBackgroundAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: ar.Name, + Status: v1alpha1.AnalysisPhaseRunning, + } f.rolloutLister = append(f.rolloutLister, r2) f.analysisTemplateLister = append(f.analysisTemplateLister, at) @@ -767,6 +797,10 @@ func TestDoNothingWhileStepBasedAnalysisRunRunning(t *testing.T) { availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) r2.Status.Canary.CurrentStepAnalysisRun = ar.Name + r2.Status.Canary.CurrentStepAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: ar.Name, + Status: v1alpha1.AnalysisPhaseRunning, + } f.rolloutLister = append(f.rolloutLister, r2) f.analysisTemplateLister = append(f.analysisTemplateLister, at) @@ -810,7 +844,15 @@ func TestCancelOlderAnalysisRuns(t *testing.T) { availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) r2.Status.Canary.CurrentStepAnalysisRun = ar.Name + r2.Status.Canary.CurrentStepAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: ar.Name, + Status: "", + } r2.Status.Canary.CurrentBackgroundAnalysisRun = oldBackgroundAr.Name + r2.Status.Canary.CurrentBackgroundAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: oldBackgroundAr.Name, + Status: "", + } f.rolloutLister = append(f.rolloutLister, r2) f.analysisTemplateLister = append(f.analysisTemplateLister, at) @@ -828,7 +870,8 @@ func TestCancelOlderAnalysisRuns(t *testing.T) { expectedPatch := `{ "status": { "canary": { - "currentBackgroundAnalysisRun": null + "currentBackgroundAnalysisRun": null, + "currentBackgroundAnalysisRunStatus":null } } }` @@ -1028,7 +1071,10 @@ func TestPausedOnInconclusiveBackgroundAnalysisRun(t *testing.T) { "status": { "conditions": %s, "canary": { - "currentBackgroundAnalysisRun": null + "currentBackgroundAnalysisRunStatus": { + "name": "%s", + "status": "Inconclusive" + } }, "pauseConditions": [{ "reason": "%s", @@ -1039,7 +1085,7 @@ func TestPausedOnInconclusiveBackgroundAnalysisRun(t *testing.T) { }` condition := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "") - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, v1alpha1.PauseReasonInconclusiveAnalysis, now)), patch) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, ar.Name, v1alpha1.PauseReasonInconclusiveAnalysis, now)), patch) } func TestPausedStepAfterInconclusiveAnalysisRun(t *testing.T) { @@ -1082,7 +1128,10 @@ func TestPausedStepAfterInconclusiveAnalysisRun(t *testing.T) { "status": { "conditions": %s, "canary": { - "currentStepAnalysisRun": null + "currentStepAnalysisRunStatus": { + "name": "%s", + "status": "Inconclusive" + } }, "pauseConditions": [{ "reason": "%s", @@ -1092,7 +1141,7 @@ func TestPausedStepAfterInconclusiveAnalysisRun(t *testing.T) { } }` condition := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false, "") - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, v1alpha1.PauseReasonInconclusiveAnalysis, now)), patch) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, ar.Name, v1alpha1.PauseReasonInconclusiveAnalysis, now)), patch) } func TestErrorConditionAfterErrorAnalysisRunStep(t *testing.T) { @@ -1137,15 +1186,20 @@ func TestErrorConditionAfterErrorAnalysisRunStep(t *testing.T) { expectedPatch := `{ "status": { "canary":{ - "currentStepAnalysisRun": null + "currentStepAnalysisRunStatus": { + "name": "%s", + "status": "Error", + "message": "Error" + } }, "conditions": %s, - "abort": true + "abort": true, + "reconciledAbort": true } }` condition := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, ar.Status.Message) - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition)), patch) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, ar.Name, condition)), patch) } func TestErrorConditionAfterErrorAnalysisRunBackground(t *testing.T) { @@ -1166,6 +1220,7 @@ func TestErrorConditionAfterErrorAnalysisRunBackground(t *testing.T) { TemplateName: at.Name, }, } + ar := analysisRun(at, v1alpha1.RolloutTypeBackgroundRunLabel, r2) ar.Status = v1alpha1.AnalysisRunStatus{ Phase: v1alpha1.AnalysisPhaseError, @@ -1174,6 +1229,11 @@ func TestErrorConditionAfterErrorAnalysisRunBackground(t *testing.T) { }}, } + r2.Status.Canary.CurrentBackgroundAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: ar.Name, + Status: v1alpha1.AnalysisPhaseRunning, + } + rs1 := newReplicaSetWithStatus(r1, 9, 9) rs2 := newReplicaSetWithStatus(r2, 1, 1) f.kubeobjects = append(f.kubeobjects, rs1, rs2) @@ -1194,9 +1254,12 @@ func TestErrorConditionAfterErrorAnalysisRunBackground(t *testing.T) { expectedPatch := `{ "status": { "canary":{ - "currentBackgroundAnalysisRun": null + "currentBackgroundAnalysisRunStatus": { + "status": "Error" + } }, "conditions": %s, + "reconciledAbort": true, "abort": true } }` @@ -1231,6 +1294,10 @@ func TestCancelAnalysisRunsWhenAborted(t *testing.T) { r2 = updateCanaryRolloutStatus(r2, rs1PodHash, 1, 0, 1, false) r2.Status.Abort = true r2.Status.Canary.CurrentStepAnalysisRun = ar.Name + r2.Status.Canary.CurrentStepAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: ar.Name, + Status: "", + } f.rolloutLister = append(f.rolloutLister, r2) f.analysisTemplateLister = append(f.analysisTemplateLister, at) @@ -1248,10 +1315,8 @@ func TestCancelAnalysisRunsWhenAborted(t *testing.T) { newConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, "") expectedPatch := `{ "status": { - "canary": { - "currentStepAnalysisRun":null - }, - "conditions": %s + "conditions": %s, + "reconciledAbort": true } }` assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, newConditions)), patch) @@ -1410,10 +1475,14 @@ func TestCreatePrePromotionAnalysisRun(t *testing.T) { expectedPatch := fmt.Sprintf(`{ "status": { "blueGreen": { - "prePromotionAnalysisRun": "%s" + "prePromotionAnalysisRun": "%s", + "prePromotionAnalysisRunStatus": { + "name": "%s", + "status": "" + } } } - }`, ar.Name) + }`, ar.Name, ar.Name) assert.Equal(t, calculatePatch(r2, expectedPatch), patch) } @@ -1544,13 +1613,17 @@ func TestRolloutPrePromotionAnalysisBecomesInconclusive(t *testing.T) { } ar := analysisRun(at, v1alpha1.RolloutTypePrePromotionLabel, r2) ar.Status.Phase = v1alpha1.AnalysisPhaseInconclusive - r2.Status.BlueGreen.PrePromotionAnalysisRun = ar.Name rs1 := newReplicaSetWithStatus(r1, 1, 1) rs2 := newReplicaSetWithStatus(r2, 1, 1) rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2.Status.BlueGreen.PrePromotionAnalysisRun = ar.Name + r2.Status.BlueGreen.PrePromotionAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: ar.Name, + Status: v1alpha1.AnalysisPhaseRunning, + } pausedCondition, _ := newProgressingCondition(conditions.PausedRolloutReason, r2, "") conditions.SetRolloutCondition(&r2.Status, pausedCondition) @@ -1581,7 +1654,9 @@ func TestRolloutPrePromotionAnalysisBecomesInconclusive(t *testing.T) { } ], "blueGreen": { - "prePromotionAnalysisRun": null + "prePromotionAnalysisRunStatus": { + "status": "Inconclusive" + } } } }`, now, now) @@ -1611,6 +1686,10 @@ func TestRolloutPrePromotionAnalysisSwitchServiceAfterSuccess(t *testing.T) { rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] r2 = updateBlueGreenRolloutStatus(r2, "", rs1PodHash, rs1PodHash, 1, 1, 2, 1, true, true) + r2.Status.BlueGreen.PrePromotionAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: ar.Name, + Status: v1alpha1.AnalysisPhaseRunning, + } pausedCondition, _ := newProgressingCondition(conditions.PausedRolloutReason, r2, "") conditions.SetRolloutCondition(&r2.Status, pausedCondition) @@ -1633,7 +1712,9 @@ func TestRolloutPrePromotionAnalysisSwitchServiceAfterSuccess(t *testing.T) { expectedPatch := fmt.Sprintf(`{ "status": { "blueGreen": { - "activeSelector": "%s" + "activeSelector": "%s", + "prePromotionAnalysisRunStatus": null, + "prePromotionAnalysisRun": null }, "pauseConditions": null, "controllerPause": null, @@ -1660,6 +1741,10 @@ func TestRolloutPrePromotionAnalysisHonorAutoPromotionSeconds(t *testing.T) { ar := analysisRun(at, v1alpha1.RolloutTypePrePromotionLabel, r2) ar.Status.Phase = v1alpha1.AnalysisPhaseRunning r2.Status.BlueGreen.PrePromotionAnalysisRun = ar.Name + r2.Status.BlueGreen.PrePromotionAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: ar.Name, + Status: v1alpha1.AnalysisPhaseRunning, + } rs1 := newReplicaSetWithStatus(r1, 1, 1) rs2 := newReplicaSetWithStatus(r2, 1, 1) @@ -1763,6 +1848,10 @@ func TestAbortRolloutOnErrorPrePromotionAnalysis(t *testing.T) { ar := analysisRun(at, v1alpha1.RolloutTypePrePromotionLabel, r2) ar.Status.Phase = v1alpha1.AnalysisPhaseError r2.Status.BlueGreen.PrePromotionAnalysisRun = ar.Name + r2.Status.BlueGreen.PrePromotionAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: ar.Name, + Status: v1alpha1.AnalysisPhaseRunning, + } rs1 := newReplicaSetWithStatus(r1, 1, 1) rs2 := newReplicaSetWithStatus(r2, 1, 1) @@ -1786,13 +1875,17 @@ func TestAbortRolloutOnErrorPrePromotionAnalysis(t *testing.T) { patchIndex := f.expectPatchRolloutActionWithPatch(r2, OnlyObservedGenerationPatch) f.run(getKey(r2, t)) patch := f.getPatchedRollout(patchIndex) + // is wrong expectedPatch := `{ "status": { "abort": true, + "reconciledAbort": true, "pauseConditions": null, "controllerPause":null, "blueGreen": { - "prePromotionAnalysisRun": null + "prePromotionAnalysisRunStatus": { + "status": "Error" + } } } }` @@ -1837,10 +1930,14 @@ func TestCreatePostPromotionAnalysisRun(t *testing.T) { expectedPatch := fmt.Sprintf(`{ "status": { "blueGreen": { - "postPromotionAnalysisRun": "%s" + "postPromotionAnalysisRun": "%s", + "postPromotionAnalysisRunStatus":{ + "name": "%s", + "status": "" + } } } - }`, ar.Name) + }`, ar.Name, ar.Name) assert.Equal(t, calculatePatch(r2, expectedPatch), patch) } @@ -1859,6 +1956,10 @@ func TestRolloutPostPromotionAnalysisSuccess(t *testing.T) { ar := analysisRun(at, v1alpha1.RolloutTypePostPromotionLabel, r2) ar.Status.Phase = v1alpha1.AnalysisPhaseSuccessful r2.Status.BlueGreen.PostPromotionAnalysisRun = ar.Name + r2.Status.BlueGreen.PostPromotionAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: ar.Name, + Status: v1alpha1.AnalysisPhaseRunning, + } rs1 := newReplicaSetWithStatus(r1, 0, 0) rs2 := newReplicaSetWithStatus(r2, 1, 1) @@ -1883,7 +1984,11 @@ func TestRolloutPostPromotionAnalysisSuccess(t *testing.T) { patch := f.getPatchedRollout(patchIndex) expectedPatch := fmt.Sprintf(`{ "status": { - "stableRS": "%s" + "stableRS": "%s", + "blueGreen": { + "postPromotionAnalysisRunStatus": null, + "postPromotionAnalysisRun": null + } } }`, rs2PodHash) assert.Equal(t, calculatePatch(r2, expectedPatch), patch) @@ -1956,9 +2061,13 @@ func TestAbortRolloutOnErrorPostPromotionAnalysis(t *testing.T) { TemplateName: at.Name, }}, } - ar := analysisRun(at, v1alpha1.RolloutTypePrePromotionLabel, r2) + ar := analysisRun(at, v1alpha1.RolloutTypePostPromotionLabel, r2) ar.Status.Phase = v1alpha1.AnalysisPhaseError r2.Status.BlueGreen.PostPromotionAnalysisRun = ar.Name + r2.Status.BlueGreen.PostPromotionAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: ar.Name, + Status: v1alpha1.AnalysisPhaseRunning, + } rs1 := newReplicaSetWithStatus(r1, 1, 1) rs1.Annotations[v1alpha1.DefaultReplicaSetScaleDownDeadlineAnnotationKey] = nowFn().Add(10 * time.Second).Format(time.RFC3339) @@ -1987,10 +2096,13 @@ func TestAbortRolloutOnErrorPostPromotionAnalysis(t *testing.T) { expectedPatch := `{ "status": { "abort": true, + "reconciledAbort": true, "pauseConditions": null, "controllerPause":null, "blueGreen": { - "postPromotionAnalysisRun": null + "postPromotionAnalysisRunStatus": { + "status": "Error" + } } } }` diff --git a/rollout/bluegreen.go b/rollout/bluegreen.go index 08915a45bf..c8a3fe223b 100644 --- a/rollout/bluegreen.go +++ b/rollout/bluegreen.go @@ -200,12 +200,13 @@ func (c *Controller) reconcileBlueGreenPause(activeSvc, previewSvc *corev1.Servi // scaleDownOldReplicaSetsForBlueGreen scales down old replica sets when rollout strategy is "Blue Green". func (c *Controller) scaleDownOldReplicaSetsForBlueGreen(oldRSs []*appsv1.ReplicaSet, roCtx *blueGreenContext) (bool, error) { rollout := roCtx.Rollout() + newRS := roCtx.NewRS() logCtx := roCtx.Log() if getPauseCondition(rollout, v1alpha1.PauseReasonInconclusiveAnalysis) != nil { logCtx.Infof("Cannot scale down old ReplicaSets while paused with inconclusive Analysis ") return false, nil } - if rollout.Spec.Strategy.BlueGreen.PostPromotionAnalysis != nil && rollout.Spec.Strategy.BlueGreen.ScaleDownDelaySeconds == nil { + if rollout.Spec.Strategy.BlueGreen.PostPromotionAnalysis != nil && rollout.Spec.Strategy.BlueGreen.ScaleDownDelaySeconds == nil && !needPostPromotionAnalysisRun(rollout, newRS) { currentPostAr := roCtx.CurrentAnalysisRuns().BlueGreenPostPromotion if currentPostAr == nil || currentPostAr.Status.Phase != v1alpha1.AnalysisPhaseSuccessful { logCtx.Infof("Cannot scale down old ReplicaSets while Analysis is running and no ScaleDownDelaySeconds") @@ -265,6 +266,8 @@ func (c *Controller) syncRolloutStatusBlueGreen(previewSvc *corev1.Service, acti roCtx.PauseContext().ClearPauseConditions() roCtx.PauseContext().RemoveAbort() roCtx.SetRestartedAt() + newStatus.BlueGreen.PrePromotionAnalysisRunStatus = nil + newStatus.BlueGreen.PostPromotionAnalysisRunStatus = nil } newStatus.AvailableReplicas = replicasetutil.GetAvailableReplicaCountForReplicaSets([]*appsv1.ReplicaSet{newRS}) diff --git a/rollout/bluegreen_test.go b/rollout/bluegreen_test.go index 4ad7f2ebf1..61b69932d9 100644 --- a/rollout/bluegreen_test.go +++ b/rollout/bluegreen_test.go @@ -1196,6 +1196,7 @@ func TestBlueGreenAbort(t *testing.T) { r1 := newBlueGreenRollout("foo", 1, nil, "bar", "") r2 := bumpVersion(r1) r2.Status.Abort = true + r2.Status.ReconciledAbort = true rs1 := newReplicaSetWithStatus(r1, 1, 1) rs2 := newReplicaSetWithStatus(r2, 1, 1) diff --git a/rollout/canary.go b/rollout/canary.go index d8129beae4..acaad8f964 100644 --- a/rollout/canary.go +++ b/rollout/canary.go @@ -284,6 +284,8 @@ func (c *Controller) syncRolloutStatusCanary(roCtx *canaryContext) error { roCtx.PauseContext().RemoveAbort() roCtx.SetRestartedAt() newStatus = c.calculateRolloutConditions(roCtx, newStatus) + newStatus.Canary.CurrentStepAnalysisRunStatus = nil + newStatus.Canary.CurrentBackgroundAnalysisRunStatus = nil return c.persistRolloutStatus(roCtx, &newStatus) } diff --git a/rollout/canary_test.go b/rollout/canary_test.go index 250bfe2c5a..ae1a7ebfe8 100644 --- a/rollout/canary_test.go +++ b/rollout/canary_test.go @@ -1258,6 +1258,7 @@ func TestHandleCanaryAbort(t *testing.T) { r2 = updateCanaryRolloutStatus(r2, rs1PodHash, 10, 1, 10, false) r2.Status.Abort = true + r2.Status.ReconciledAbort = true f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2) @@ -1290,6 +1291,7 @@ func TestHandleCanaryAbort(t *testing.T) { } r1 := newCanaryRollout("foo", 2, nil, steps, int32Ptr(3), intstr.FromInt(1), intstr.FromInt(0)) r1.Status.Abort = true + r1.Status.ReconciledAbort = true rs1 := newReplicaSetWithStatus(r1, 2, 2) rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] r1 = updateCanaryRolloutStatus(r1, rs1PodHash, 2, 2, 2, false) diff --git a/rollout/context.go b/rollout/context.go index 2ae0ec8a11..29b3e6d411 100644 --- a/rollout/context.go +++ b/rollout/context.go @@ -130,16 +130,22 @@ func (bgCtx *blueGreenContext) OtherAnalysisRuns() []*v1alpha1.AnalysisRun { func (bgCtx *blueGreenContext) SetCurrentAnalysisRuns(currAr analysisutil.CurrentAnalysisRuns) { bgCtx.currentArs = currAr - if currAr.BlueGreenPrePromotion != nil && !bgCtx.PauseContext().IsAborted() { - switch currAr.BlueGreenPrePromotion.Status.Phase { - case v1alpha1.AnalysisPhasePending, v1alpha1.AnalysisPhaseRunning, v1alpha1.AnalysisPhaseSuccessful, "": - bgCtx.newStatus.BlueGreen.PrePromotionAnalysisRun = currAr.BlueGreenPrePromotion.Name + currPrePromoAr := currAr.BlueGreenPrePromotion + if currPrePromoAr != nil && currPrePromoAr.Status.Phase != v1alpha1.AnalysisPhaseSuccessful { + bgCtx.newStatus.BlueGreen.PrePromotionAnalysisRun = currPrePromoAr.Name + bgCtx.newStatus.BlueGreen.PrePromotionAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: currPrePromoAr.Name, + Status: currPrePromoAr.Status.Phase, + Message: currPrePromoAr.Status.Message, } } - if currAr.BlueGreenPostPromotion != nil && !bgCtx.PauseContext().IsAborted() { - switch currAr.BlueGreenPostPromotion.Status.Phase { - case v1alpha1.AnalysisPhasePending, v1alpha1.AnalysisPhaseRunning, v1alpha1.AnalysisPhaseSuccessful, "": - bgCtx.newStatus.BlueGreen.PostPromotionAnalysisRun = currAr.BlueGreenPostPromotion.Name + currPostPromoAr := currAr.BlueGreenPostPromotion + if currPostPromoAr != nil && currPostPromoAr.Status.Phase != v1alpha1.AnalysisPhaseSuccessful { + bgCtx.newStatus.BlueGreen.PostPromotionAnalysisRun = currPostPromoAr.Name + bgCtx.newStatus.BlueGreen.PostPromotionAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: currPostPromoAr.Name, + Status: currPostPromoAr.Status.Phase, + Message: currPostPromoAr.Status.Message, } } } @@ -219,18 +225,24 @@ func (cCtx *canaryContext) AllRSs() []*appsv1.ReplicaSet { func (cCtx *canaryContext) SetCurrentAnalysisRuns(currARs analysisutil.CurrentAnalysisRuns) { cCtx.currentArs = currARs - if currARs.CanaryBackground != nil && !cCtx.PauseContext().IsAborted() { - switch currARs.CanaryBackground.Status.Phase { - case v1alpha1.AnalysisPhasePending, v1alpha1.AnalysisPhaseRunning, v1alpha1.AnalysisPhaseSuccessful, "": - cCtx.newStatus.Canary.CurrentBackgroundAnalysisRun = currARs.CanaryBackground.Name + currBackgroundAr := currARs.CanaryBackground + if currBackgroundAr != nil && currBackgroundAr.Status.Phase != v1alpha1.AnalysisPhaseSuccessful { + cCtx.newStatus.Canary.CurrentBackgroundAnalysisRun = currBackgroundAr.Name + cCtx.newStatus.Canary.CurrentBackgroundAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: currBackgroundAr.Name, + Status: currBackgroundAr.Status.Phase, + Message: currBackgroundAr.Status.Message, } } - if currARs.CanaryStep != nil && !cCtx.PauseContext().IsAborted() { - if !currARs.CanaryStep.Status.Phase.Completed() { - cCtx.newStatus.Canary.CurrentStepAnalysisRun = currARs.CanaryStep.Name + currStepAr := currARs.CanaryStep + if currStepAr != nil && currStepAr.Status.Phase != v1alpha1.AnalysisPhaseSuccessful { + cCtx.newStatus.Canary.CurrentStepAnalysisRun = currStepAr.Name + cCtx.newStatus.Canary.CurrentStepAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: currStepAr.Name, + Status: currStepAr.Status.Phase, + Message: currStepAr.Status.Message, } } - } func (cCtx *canaryContext) CurrentAnalysisRuns() analysisutil.CurrentAnalysisRuns { diff --git a/rollout/controller_test.go b/rollout/controller_test.go index 084c010299..4d0bf66042 100644 --- a/rollout/controller_test.go +++ b/rollout/controller_test.go @@ -335,10 +335,12 @@ func newReplicaSet(r *v1alpha1.Rollout, replicas int) *appsv1.ReplicaSet { func calculatePatch(ro *v1alpha1.Rollout, patch string) string { origBytes, err := json.Marshal(ro) if err != nil { + fmt.Println(patch) panic(err) } newBytes, err := strategicpatch.StrategicMergePatch(origBytes, []byte(patch), v1alpha1.Rollout{}) if err != nil { + fmt.Println(patch) panic(err) } newRO := &v1alpha1.Rollout{} diff --git a/rollout/experiment_test.go b/rollout/experiment_test.go index c889d912c5..fd3acc4bec 100644 --- a/rollout/experiment_test.go +++ b/rollout/experiment_test.go @@ -299,6 +299,7 @@ func TestAbortRolloutAfterFailedExperiment(t *testing.T) { expectedPatch := `{ "status": { "abort": true, + "reconciledAbort": true, "conditions": %s, "canary": { "currentExperiment": null diff --git a/rollout/pause.go b/rollout/pause.go index 876736820b..1ed3fd5758 100644 --- a/rollout/pause.go +++ b/rollout/pause.go @@ -59,13 +59,16 @@ func (pCtx *pauseContext) ClearPauseConditions() { func (pCtx *pauseContext) CalculatePauseStatus(newStatus *v1alpha1.RolloutStatus) { if pCtx.addAbort { newStatus.Abort = true + newStatus.ReconciledAbort = true return } if !pCtx.removeAbort && pCtx.rollout.Status.Abort { newStatus.Abort = true + newStatus.ReconciledAbort = true return } newStatus.Abort = false + newStatus.ReconciledAbort = false if pCtx.clearPauseConditions { return diff --git a/rollout/sync.go b/rollout/sync.go index 0142d2d625..5fcb7c2a4a 100644 --- a/rollout/sync.go +++ b/rollout/sync.go @@ -232,6 +232,8 @@ func (c *Controller) syncReplicasOnly(r *v1alpha1.Rollout, rsList []*appsv1.Repl if r.Spec.Strategy.BlueGreen != nil { roCtx := newBlueGreenCtx(r, newRS, oldRSs, arList) previewSvc, activeSvc, err := c.getPreviewAndActiveServices(r) + // Keep existing analysis runs if the rollout is paused + roCtx.SetCurrentAnalysisRuns(roCtx.CurrentAnalysisRuns()) if err != nil { return nil } diff --git a/utils/analysis/filter.go b/utils/analysis/filter.go index 87ed427f98..5656510a98 100644 --- a/utils/analysis/filter.go +++ b/utils/analysis/filter.go @@ -24,7 +24,6 @@ func FilterCurrentRolloutAnalysisRuns(analysisRuns []*v1alpha1.AnalysisRun, r *v for i := range analysisRuns { ar := analysisRuns[i] if ar != nil { - switch ar.Name { case r.Status.Canary.CurrentStepAnalysisRun: currArs.CanaryStep = ar From c59beed801ed3260ed5ed99e58d0e73db2f64f8d Mon Sep 17 00:00:00 2001 From: Danny Thomson Date: Fri, 31 Jul 2020 07:38:42 -0700 Subject: [PATCH 3/4] Address PR --- manifests/crds/rollout-crd.yaml | 2 ++ .../rollouts/v1alpha1/openapi_generated.go | 9 ++++++- pkg/apis/rollouts/v1alpha1/types.go | 14 +++++------ rollout/analysis.go | 2 +- rollout/analysis_test.go | 25 +++++++++++-------- rollout/bluegreen_test.go | 3 ++- rollout/canary_test.go | 6 +++-- rollout/experiment_test.go | 5 ++-- rollout/pause.go | 8 +++--- 9 files changed, 47 insertions(+), 27 deletions(-) diff --git a/manifests/crds/rollout-crd.yaml b/manifests/crds/rollout-crd.yaml index ea567f5a81..e3d0ef6337 100644 --- a/manifests/crds/rollout-crd.yaml +++ b/manifests/crds/rollout-crd.yaml @@ -2909,6 +2909,8 @@ spec: type: integer abort: type: boolean + abortedAt: + type: boolean availableReplicas: format: int32 type: integer diff --git a/pkg/apis/rollouts/v1alpha1/openapi_generated.go b/pkg/apis/rollouts/v1alpha1/openapi_generated.go index 5336004ddd..8c87275554 100644 --- a/pkg/apis/rollouts/v1alpha1/openapi_generated.go +++ b/pkg/apis/rollouts/v1alpha1/openapi_generated.go @@ -787,7 +787,7 @@ func schema_pkg_apis_rollouts_v1alpha1_CanaryStatus(ref common.ReferenceCallback }, "currentBackgroundAnalysisRunStatus": { SchemaProps: spec.SchemaProps{ - Description: "CurrentStepAnalysisRunStatus indicates the status of the current background analysis run", + Description: "CurrentBackgroundAnalysisRunStatus indicates the status of the current background analysis run", Ref: ref("github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1.RolloutAnalysisRunStatus"), }, }, @@ -2702,6 +2702,13 @@ func schema_pkg_apis_rollouts_v1alpha1_RolloutStatus(ref common.ReferenceCallbac Format: "", }, }, + "abortedAt": { + SchemaProps: spec.SchemaProps{ + Description: "AbortedAt indicates the controller reconciled an aborted rollout. The controller uses this to understand if the controller needs to do some specific work when a Rollout is aborted. For example, the reconcileAbort is used to indicate if the Rollout should enter an aborted state when the latest AnalysisRun is a failure, or the controller has already put the Rollout into an aborted and should create a new AnalysisRun.", + Type: []string{"boolean"}, + Format: "", + }, + }, "currentPodHash": { SchemaProps: spec.SchemaProps{ Description: "CurrentPodHash the hash of the current pod template", diff --git a/pkg/apis/rollouts/v1alpha1/types.go b/pkg/apis/rollouts/v1alpha1/types.go index aaae5659b4..fcf0149848 100644 --- a/pkg/apis/rollouts/v1alpha1/types.go +++ b/pkg/apis/rollouts/v1alpha1/types.go @@ -476,11 +476,11 @@ type RolloutStatus struct { PauseConditions []PauseCondition `json:"pauseConditions,omitempty"` //ControllerPause indicates the controller has paused the rollout ControllerPause bool `json:"controllerPause,omitempty"` - // ReconciledAbort indicates the controller reconciled an aborted rollout. The controller uses this to understand if + // AbortedAt indicates the controller reconciled an aborted rollout. The controller uses this to understand if // the controller needs to do some specific work when a Rollout is aborted. For example, the reconcileAbort is used // to indicate if the Rollout should enter an aborted state when the latest AnalysisRun is a failure, or the controller // has already put the Rollout into an aborted and should create a new AnalysisRun. - ReconciledAbort bool `json:"reconciledAbort,omitempty"` + AbortedAt *metav1.Time `json:"abortedAt,omitempty"` // CurrentPodHash the hash of the current pod template // +optional CurrentPodHash string `json:"currentPodHash,omitempty"` @@ -555,12 +555,12 @@ type BlueGreenStatus struct { // +optional ScaleUpPreviewCheckPoint bool `json:"scaleUpPreviewCheckPoint,omitempty"` // PrePromotionAnalysisRun is the current analysis run running before the active service promotion - // TODO(Depreciated): Remove in v0.10 + // TODO(Deprecated): Remove in v0.10 PrePromotionAnalysisRun string `json:"prePromotionAnalysisRun,omitempty"` // PrePromotionAnalysisRunStatus indicates the status of the current prepromotion analysis run PrePromotionAnalysisRunStatus *RolloutAnalysisRunStatus `json:"prePromotionAnalysisRunStatus,omitempty"` // PostPromotionAnalysisRun is the current analysis run running after the active service promotion - // TODO(Depreciated): Remove in v0.10 + // TODO(Deprecated): Remove in v0.10 PostPromotionAnalysisRun string `json:"postPromotionAnalysisRun,omitempty"` // PostPromotionAnalysisRunStatus indicates the status of the current post promotion analysis run PostPromotionAnalysisRunStatus *RolloutAnalysisRunStatus `json:"postPromotionAnalysisRunStatus,omitempty"` @@ -572,14 +572,14 @@ type CanaryStatus struct { // +optional StableRS string `json:"stableRS,omitempty"` // CurrentStepAnalysisRun indicates the analysisRun for the current step index - // TODO(Depreciated): Remove in v0.10 + // TODO(Deprecated): Remove in v0.10 CurrentStepAnalysisRun string `json:"currentStepAnalysisRun,omitempty"` // CurrentStepAnalysisRunStatus indicates the status of the current step analysis run CurrentStepAnalysisRunStatus *RolloutAnalysisRunStatus `json:"currentStepAnalysisRunStatus,omitempty"` // CurrentBackgroundAnalysisRun indicates the analysisRun for the Background step - // TODO(Depreciated): Remove in v0.10 + // TODO(Deprecated): Remove in v0.10 CurrentBackgroundAnalysisRun string `json:"currentBackgroundAnalysisRun,omitempty"` - // CurrentStepAnalysisRunStatus indicates the status of the current background analysis run + // CurrentBackgroundAnalysisRunStatus indicates the status of the current background analysis run CurrentBackgroundAnalysisRunStatus *RolloutAnalysisRunStatus `json:"currentBackgroundAnalysisRunStatus,omitempty"` // CurrentExperiment indicates the running experiment CurrentExperiment string `json:"currentExperiment,omitempty"` diff --git a/rollout/analysis.go b/rollout/analysis.go index 2d99e58893..f126434829 100644 --- a/rollout/analysis.go +++ b/rollout/analysis.go @@ -130,7 +130,7 @@ func needsNewAnalysisRun(currentAr *v1alpha1.AnalysisRun, rollout *v1alpha1.Roll if rollout.Status.ControllerPause && getPauseCondition(rollout, v1alpha1.PauseReasonBlueGreenPause) == nil { return currentAr.Status.Phase == v1alpha1.AnalysisPhaseInconclusive } - return rollout.Status.ReconciledAbort + return rollout.Status.AbortedAt != nil } // emitAnalysisRunStatusChanges emits a Kubernetes event if the analysis run of that type has changed status diff --git a/rollout/analysis_test.go b/rollout/analysis_test.go index a115436bf1..5dfee4d50f 100644 --- a/rollout/analysis_test.go +++ b/rollout/analysis_test.go @@ -1194,12 +1194,13 @@ func TestErrorConditionAfterErrorAnalysisRunStep(t *testing.T) { }, "conditions": %s, "abort": true, - "reconciledAbort": true + "abortedAt": "%s" } }` + now := metav1.Now().UTC().Format(time.RFC3339) condition := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, ar.Status.Message) - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, ar.Name, condition)), patch) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, ar.Name, condition, now)), patch) } func TestErrorConditionAfterErrorAnalysisRunBackground(t *testing.T) { @@ -1259,13 +1260,14 @@ func TestErrorConditionAfterErrorAnalysisRunBackground(t *testing.T) { } }, "conditions": %s, - "reconciledAbort": true, + "abortedAt": "%s", "abort": true } }` condition := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, "") - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition)), patch) + now := metav1.Now().UTC().Format(time.RFC3339) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, now)), patch) } func TestCancelAnalysisRunsWhenAborted(t *testing.T) { @@ -1316,10 +1318,11 @@ func TestCancelAnalysisRunsWhenAborted(t *testing.T) { expectedPatch := `{ "status": { "conditions": %s, - "reconciledAbort": true + "abortedAt": "%s" } }` - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, newConditions)), patch) + now := metav1.Now().UTC().Format(time.RFC3339) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, newConditions, now)), patch) } func TestCancelBackgroundAnalysisRunWhenRolloutIsCompleted(t *testing.T) { @@ -1879,7 +1882,7 @@ func TestAbortRolloutOnErrorPrePromotionAnalysis(t *testing.T) { expectedPatch := `{ "status": { "abort": true, - "reconciledAbort": true, + "abortedAt": "%s", "pauseConditions": null, "controllerPause":null, "blueGreen": { @@ -1889,7 +1892,8 @@ func TestAbortRolloutOnErrorPrePromotionAnalysis(t *testing.T) { } } }` - assert.Equal(t, calculatePatch(r2, expectedPatch), patch) + now := metav1.Now().UTC().Format(time.RFC3339) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, now)), patch) } func TestCreatePostPromotionAnalysisRun(t *testing.T) { @@ -2096,7 +2100,7 @@ func TestAbortRolloutOnErrorPostPromotionAnalysis(t *testing.T) { expectedPatch := `{ "status": { "abort": true, - "reconciledAbort": true, + "abortedAt": "%s", "pauseConditions": null, "controllerPause":null, "blueGreen": { @@ -2106,5 +2110,6 @@ func TestAbortRolloutOnErrorPostPromotionAnalysis(t *testing.T) { } } }` - assert.Equal(t, calculatePatch(r2, expectedPatch), patch) + now := metav1.Now().UTC().Format(time.RFC3339) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, now)), patch) } diff --git a/rollout/bluegreen_test.go b/rollout/bluegreen_test.go index 61b69932d9..f954f60766 100644 --- a/rollout/bluegreen_test.go +++ b/rollout/bluegreen_test.go @@ -1196,7 +1196,8 @@ func TestBlueGreenAbort(t *testing.T) { r1 := newBlueGreenRollout("foo", 1, nil, "bar", "") r2 := bumpVersion(r1) r2.Status.Abort = true - r2.Status.ReconciledAbort = true + now := metav1.Now() + r2.Status.AbortedAt = &now rs1 := newReplicaSetWithStatus(r1, 1, 1) rs2 := newReplicaSetWithStatus(r2, 1, 1) diff --git a/rollout/canary_test.go b/rollout/canary_test.go index ae1a7ebfe8..35742b69e1 100644 --- a/rollout/canary_test.go +++ b/rollout/canary_test.go @@ -1258,7 +1258,8 @@ func TestHandleCanaryAbort(t *testing.T) { r2 = updateCanaryRolloutStatus(r2, rs1PodHash, 10, 1, 10, false) r2.Status.Abort = true - r2.Status.ReconciledAbort = true + now := metav1.Now() + r2.Status.AbortedAt = &now f.rolloutLister = append(f.rolloutLister, r2) f.objects = append(f.objects, r2) @@ -1291,7 +1292,8 @@ func TestHandleCanaryAbort(t *testing.T) { } r1 := newCanaryRollout("foo", 2, nil, steps, int32Ptr(3), intstr.FromInt(1), intstr.FromInt(0)) r1.Status.Abort = true - r1.Status.ReconciledAbort = true + now := metav1.Now() + r1.Status.AbortedAt = &now rs1 := newReplicaSetWithStatus(r1, 2, 2) rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] r1 = updateCanaryRolloutStatus(r1, rs1PodHash, 2, 2, 2, false) diff --git a/rollout/experiment_test.go b/rollout/experiment_test.go index fd3acc4bec..1999a6c1fe 100644 --- a/rollout/experiment_test.go +++ b/rollout/experiment_test.go @@ -299,15 +299,16 @@ func TestAbortRolloutAfterFailedExperiment(t *testing.T) { expectedPatch := `{ "status": { "abort": true, - "reconciledAbort": true, + "abortedAt": "%s", "conditions": %s, "canary": { "currentExperiment": null } } }` + now := metav1.Now().UTC().Format(time.RFC3339) generatedConditons := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false, "") - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, generatedConditons)), patch) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, now, generatedConditons)), patch) } func TestPauseRolloutAfterInconclusiveExperiment(t *testing.T) { diff --git a/rollout/pause.go b/rollout/pause.go index 1ed3fd5758..07f4bed5db 100644 --- a/rollout/pause.go +++ b/rollout/pause.go @@ -59,16 +59,18 @@ func (pCtx *pauseContext) ClearPauseConditions() { func (pCtx *pauseContext) CalculatePauseStatus(newStatus *v1alpha1.RolloutStatus) { if pCtx.addAbort { newStatus.Abort = true - newStatus.ReconciledAbort = true + now := metav1.Now() + newStatus.AbortedAt = &now return } if !pCtx.removeAbort && pCtx.rollout.Status.Abort { newStatus.Abort = true - newStatus.ReconciledAbort = true + now := metav1.Now() + newStatus.AbortedAt = &now return } newStatus.Abort = false - newStatus.ReconciledAbort = false + newStatus.AbortedAt = nil if pCtx.clearPauseConditions { return From 39b5d3155f79c565b02c549863995062fea8016a Mon Sep 17 00:00:00 2001 From: Danny Thomson Date: Fri, 31 Jul 2020 08:33:24 -0700 Subject: [PATCH 4/4] Fix tests --- rollout/analysis_test.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/rollout/analysis_test.go b/rollout/analysis_test.go index 5dfee4d50f..b73aba62ef 100644 --- a/rollout/analysis_test.go +++ b/rollout/analysis_test.go @@ -213,11 +213,15 @@ func TestCreateBackgroundAnalysisRunWithTemplates(t *testing.T) { expectedPatch := `{ "status": { "canary": { - "currentBackgroundAnalysisRun": "%s" + "currentBackgroundAnalysisRun": "%s", + "currentBackgroundAnalysisRunStatus": { + "name": "%s", + "status": "" + } } } }` - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName, expectedArName)), patch) } func TestCreateBackgroundAnalysisRunWithClusterTemplates(t *testing.T) { @@ -270,11 +274,15 @@ func TestCreateBackgroundAnalysisRunWithClusterTemplates(t *testing.T) { expectedPatch := `{ "status": { "canary": { - "currentBackgroundAnalysisRun": "%s" + "currentBackgroundAnalysisRun": "%s", + "currentBackgroundAnalysisRunStatus": { + "name": "%s", + "status": "" + } } } }` - assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName, expectedArName)), patch) } func TestCreateBackgroundAnalysisRunErrorWithMissingClusterTemplates(t *testing.T) { @@ -592,10 +600,6 @@ func TestFailCreateStepAnalysisRunIfInvalidTemplateRef(t *testing.T) { rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] r2 = updateCanaryRolloutStatus(r2, rs1PodHash, 1, 0, 1, false) - r2.Status.Canary.CurrentStepAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ - Name: ar.Name, - Status: "", - } progressingCondition, _ := newProgressingCondition(conditions.ReplicaSetUpdatedReason, rs2, "") conditions.SetRolloutCondition(&r2.Status, progressingCondition) availableCondition, _ := newAvailableCondition(true) @@ -784,6 +788,7 @@ func TestDoNothingWhileStepBasedAnalysisRunRunning(t *testing.T) { r1 := newCanaryRollout("foo", 1, nil, steps, pointer.Int32Ptr(0), intstr.FromInt(0), intstr.FromInt(1)) r2 := bumpVersion(r1) ar := analysisRun(at, v1alpha1.RolloutTypeStepLabel, r2) + ar.Status.Phase = v1alpha1.AnalysisPhaseRunning rs1 := newReplicaSetWithStatus(r1, 1, 1) rs2 := newReplicaSetWithStatus(r2, 0, 0) @@ -910,6 +915,9 @@ func TestDeleteAnalysisRunsWithNoMatchingRS(t *testing.T) { availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) r2.Status.Canary.CurrentStepAnalysisRun = ar.Name + r2.Status.Canary.CurrentStepAnalysisRunStatus = &v1alpha1.RolloutAnalysisRunStatus{ + Name: ar.Name, + } f.rolloutLister = append(f.rolloutLister, r2) f.analysisTemplateLister = append(f.analysisTemplateLister, at)