From ef75a536abcf98809f553f92292a0ebad173acda Mon Sep 17 00:00:00 2001 From: Oleksandr Saulyak Date: Mon, 11 Dec 2023 16:25:15 +0200 Subject: [PATCH] fix: canary step analysis run wasn't terminated as keep running after promote action being called. Fixes #3220 (#3221) * fix: canary step analysis run wasn't terminated as keep running after promote action being called Signed-off-by: oleksandr-codefresh * fix unit test: added test case for canary step analysis run wasn't terminated as keep running after promote action being called Signed-off-by: oleksandr-codefresh --------- Signed-off-by: oleksandr-codefresh Co-authored-by: pasha-codefresh (cherry picked from commit 0300af1a5047100d59c7495b7cacc4c1d12571a8) --- rollout/analysis.go | 5 ++- rollout/analysis_test.go | 78 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/rollout/analysis.go b/rollout/analysis.go index b2528aaf3b..49583287f6 100644 --- a/rollout/analysis.go +++ b/rollout/analysis.go @@ -372,7 +372,10 @@ func (c *rolloutContext) reconcileStepBasedAnalysisRun() (*v1alpha1.AnalysisRun, return currentAr, nil } - if step == nil || step.Analysis == nil || index == nil { + // for promotion cases + analysisRunFromPreviousStep := step != nil && step.Analysis != nil && currentAr != nil && currentAr.GetLabels()[v1alpha1.RolloutCanaryStepIndexLabel] != strconv.Itoa(int(*index)) + + if step == nil || step.Analysis == nil || index == nil || analysisRunFromPreviousStep { err := c.cancelAnalysisRuns([]*v1alpha1.AnalysisRun{currentAr}) return nil, err } diff --git a/rollout/analysis_test.go b/rollout/analysis_test.go index de9a5e1db3..624134fbb6 100644 --- a/rollout/analysis_test.go +++ b/rollout/analysis_test.go @@ -614,6 +614,84 @@ func TestCreateAnalysisRunOnAnalysisStep(t *testing.T) { assert.JSONEq(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch) } +func TestCreateAnalysisRunOnPromotedAnalysisStepIfPreviousStepWasAnalysisToo(t *testing.T) { + f := newFixture(t) + defer f.Close() + + at := analysisTemplate("bar") + steps := []v1alpha1.CanaryStep{{ + Analysis: &v1alpha1.RolloutAnalysis{ + Templates: []v1alpha1.RolloutAnalysisTemplate{ + { + TemplateName: at.Name, + }, + }, + }, + }, { + Analysis: &v1alpha1.RolloutAnalysis{ + Templates: []v1alpha1.RolloutAnalysisTemplate{ + { + TemplateName: at.Name, + }, + }, + }, + }} + + r1 := newCanaryRollout("foo", 1, nil, steps, pointer.Int32Ptr(0), intstr.FromInt(0), intstr.FromInt(1)) + r2 := bumpVersion(r1) + ar0Step := analysisRun(at, v1alpha1.RolloutTypeStepLabel, r2) + ar0Step.Status.Phase = v1alpha1.AnalysisPhaseRunning + + rs1 := newReplicaSetWithStatus(r1, 1, 1) + rs2 := newReplicaSetWithStatus(r2, 0, 0) + f.kubeobjects = append(f.kubeobjects, rs1, rs2) + f.replicaSetLister = append(f.replicaSetLister, rs1, rs2) + rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] + //rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] + + r2 = updateCanaryRolloutStatus(r2, rs1PodHash, 1, 0, 1, false) + progressingCondition, _ := newProgressingCondition(conditions.ReplicaSetUpdatedReason, rs2, "") + 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: ar0Step.Name, + Status: "", + } + + f.rolloutLister = append(f.rolloutLister, r2) + f.analysisTemplateLister = append(f.analysisTemplateLister, at) + f.analysisRunLister = append(f.analysisRunLister, ar0Step) + f.objects = append(f.objects, r2, at, ar0Step) + + patchOldAnalysisIndex := f.expectPatchAnalysisRunAction(ar0Step) + //createdIndex := f.expectCreateAnalysisRunAction(ar0Step) + index := f.expectPatchRolloutAction(r2) + + // simulate promote action + r2.Status.CurrentStepIndex = pointer.Int32Ptr(1) + + f.run(getKey(r2, t)) + + assert.True(t, f.verifyPatchedAnalysisRun(patchOldAnalysisIndex, ar0Step)) + // should terminate analysis run for old step + patchedOldAr := f.getPatchedAnalysisRun(patchOldAnalysisIndex) + assert.True(t, patchedOldAr.Spec.Terminate) + + // should patch rollout with relevant currentStepAnalysisRun + patch := f.getPatchedRollout(index) + expectedPatch := `{ + "status": { + "canary": { + "currentStepAnalysisRunStatus":null + } + } + }` + assert.JSONEq(t, calculatePatch(r2, expectedPatch), patch) +} + func TestFailCreateStepAnalysisRunIfInvalidTemplateRef(t *testing.T) { f := newFixture(t) defer f.Close()