diff --git a/rollout/analysis.go b/rollout/analysis.go index 9d7f86bbf2..e0ba91db62 100644 --- a/rollout/analysis.go +++ b/rollout/analysis.go @@ -56,7 +56,13 @@ func (c *RolloutController) reconcileAnalysisRuns(rollout *v1alpha1.Rollout, cur newCurrentAnalysisRuns = append(newCurrentAnalysisRuns, stepAnalysisRun) } - //TODO(dthomson) implement reconcileBackgroundBasedAnalysisRun + backgroundAnalysisRun, err := c.reconcileBackgroundAnalysisRun(rollout, currentArs, stableRS, newRS) + if err != nil { + return currentArs, err + } + if backgroundAnalysisRun != nil { + newCurrentAnalysisRuns = append(newCurrentAnalysisRuns, backgroundAnalysisRun) + } err = c.cancelAnalysisRuns(rollout, otherArs) if err != nil { @@ -66,6 +72,43 @@ func (c *RolloutController) reconcileAnalysisRuns(rollout *v1alpha1.Rollout, cur return newCurrentAnalysisRuns, nil } +func (c *RolloutController) reconcileBackgroundAnalysisRun(rollout *v1alpha1.Rollout, currentArs []*v1alpha1.AnalysisRun, stableRS, newRS *appsv1.ReplicaSet) (*v1alpha1.AnalysisRun, error) { + currentAr := analysisutil.FilterAnalysisRunsByName(currentArs, rollout.Status.Canary.CurrentBackgroundAnalysisRun) + if rollout.Spec.Strategy.CanaryStrategy.Analysis == nil { + err := c.cancelAnalysisRuns(rollout, []*v1alpha1.AnalysisRun{currentAr}) + if err != nil { + return nil, err + } + return nil, err + } + if currentAr == nil { + return c.createBackgroundAnalysisRun(rollout, rollout.Spec.Strategy.CanaryStrategy.Analysis, stableRS, newRS) + } + return currentAr, nil +} + +func (c *RolloutController) createBackgroundAnalysisRun(rollout *v1alpha1.Rollout, rolloutAnalysisStep *v1alpha1.RolloutAnalysisStep, stableRS, newRS *appsv1.ReplicaSet) (*v1alpha1.AnalysisRun, error) { + podHash := controller.ComputeHash(&rollout.Spec.Template, rollout.Status.CollisionCount) + // Since the compute hash function is not guaranteed to be stable, we will use the podHash attached the newRS if possible. + if newRS != nil { + if newRsPodHash, ok := newRS.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]; ok { + podHash = newRsPodHash + } + } + analysisRunLabels := analysisutil.BackgroundLabels(podHash) + args := analysisutil.BuildArgumentsForRolloutAnalysisRun(rolloutAnalysisStep, stableRS, newRS) + ar, err := c.getAnalysisRunFromRollout(rollout, rolloutAnalysisStep, args, podHash, analysisRunLabels) + if err != nil { + return nil, err + } + ar, err = c.argoprojclientset.ArgoprojV1alpha1().AnalysisRuns(ar.Namespace).Create(ar) + if err != nil { + return nil, err + } + logutil.WithRollout(rollout).WithField(logutil.AnalysisRunKey, ar.Name).Info("Created background AnalysisRun") + return ar, nil +} + func (c *RolloutController) reconcileStepBasedAnalysisRun(rollout *v1alpha1.Rollout, currentArs []*v1alpha1.AnalysisRun, stableRS, newRS *appsv1.ReplicaSet) (*v1alpha1.AnalysisRun, error) { step, index := replicasetutil.GetCurrentCanaryStep(rollout) currentAr := analysisutil.FilterAnalysisRunsByName(currentArs, rollout.Status.Canary.CurrentStepAnalysisRun) @@ -91,7 +134,7 @@ func (c *RolloutController) createStepBasedAnalysisRun(rollout *v1alpha1.Rollout podHash = newRsPodHash } } - analysisRunLabels := analysisutil.StepLabels(rollout, index, podHash) + analysisRunLabels := analysisutil.StepLabels(index, podHash) args := analysisutil.BuildArgumentsForRolloutAnalysisRun(rolloutAnalysisStep, stableRS, newRS) ar, err := c.getAnalysisRunFromRollout(rollout, rolloutAnalysisStep, args, podHash, analysisRunLabels) if err != nil { @@ -138,7 +181,7 @@ func (c *RolloutController) getAnalysisRunFromRollout(r *v1alpha1.Rollout, rollo ar := v1alpha1.AnalysisRun{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: fmt.Sprintf("%s-%s-%s", r.Name, rolloutAnalysisStep.TemplateName, podHash), + GenerateName: fmt.Sprintf("%s-%s-%s-", r.Name, rolloutAnalysisStep.TemplateName, podHash), Namespace: r.Namespace, Labels: labels, Annotations: map[string]string{ diff --git a/rollout/analysis_test.go b/rollout/analysis_test.go index 825d710ea0..76d4d57033 100644 --- a/rollout/analysis_test.go +++ b/rollout/analysis_test.go @@ -35,7 +35,9 @@ func analysisRun(at *v1alpha1.AnalysisTemplate, analysisRunType string, r *v1alp labels := map[string]string{} podHash := controller.ComputeHash(&r.Spec.Template, r.Status.CollisionCount) if analysisRunType == v1alpha1.RolloutTypeStepLabel { - labels = analysisutil.StepLabels(r, *r.Status.CurrentStepIndex, podHash) + labels = analysisutil.StepLabels(*r.Status.CurrentStepIndex, podHash) + } else if analysisRunType == v1alpha1.RolloutTypeBackgroundRunLabel { + labels = analysisutil.BackgroundLabels(podHash) } return &v1alpha1.AnalysisRun{ ObjectMeta: metav1.ObjectMeta{ @@ -50,6 +52,59 @@ func analysisRun(at *v1alpha1.AnalysisTemplate, analysisRunType string, r *v1alp } } +func TestCreateBackgroundAnalysisRun(t *testing.T) { + f := newFixture(t) + defer f.Close() + + steps := []v1alpha1.CanaryStep{{ + SetWeight: int32Ptr(10), + }} + at := analysisTemplate("bar") + r1 := newCanaryRollout("foo", 10, nil, steps, pointer.Int32Ptr(0), intstr.FromInt(0), intstr.FromInt(1)) + r2 := bumpVersion(r1) + ar := analysisRun(at, v1alpha1.RolloutTypeBackgroundRunLabel, r2) + r2.Spec.Strategy.CanaryStrategy.Analysis = &v1alpha1.RolloutAnalysisStep{ + TemplateName: at.Name, + } + + rs1 := newReplicaSetWithStatus(r1, 10, 10) + 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, 10, 0, 10, false) + progressingCondition, _ := newProgressingCondition(conditions.ReplicaSetUpdatedReason, rs2) + conditions.SetRolloutCondition(&r2.Status, progressingCondition) + availableCondition, _ := newAvailableCondition(true) + conditions.SetRolloutCondition(&r2.Status, availableCondition) + + f.rolloutLister = append(f.rolloutLister, r2) + f.analysisTemplateLister = append(f.analysisTemplateLister, at) + f.objects = append(f.objects, r2, at) + + createdIndex := f.expectCreateAnalysisRunAction(ar) + f.expectUpdateReplicaSetAction(rs2) + index := f.expectPatchRolloutAction(r1) + + f.run(getKey(r2, t)) + createdAr := f.getCreatedAnalysisRun(createdIndex) + expectedArGeneratedName := fmt.Sprintf("%s-%s-%s-", r2.Name, at.Name, rs2PodHash) + expectedArName := fmt.Sprintf("%s%s", expectedArGeneratedName, MockGeneratedNameSuffix) + assert.Equal(t, expectedArGeneratedName, createdAr.GenerateName) + + patch := f.getPatchedRollout(index) + expectedPatch := `{ + "status": { + "canary": { + "currentBackgroundAnalysisRun": "%s" + } + } + }` + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch) +} + func TestCreateAnalysisRunOnAnalysisStep(t *testing.T) { f := newFixture(t) defer f.Close() @@ -87,8 +142,8 @@ func TestCreateAnalysisRunOnAnalysisStep(t *testing.T) { f.run(getKey(r2, t)) createdAr := f.getCreatedAnalysisRun(createdIndex) - expectedArGeneratedName := fmt.Sprintf("%s-%s-%s", r2.Name, at.Name, rs2PodHash) - expectedArName := fmt.Sprintf("%s-%s", expectedArGeneratedName, MockGeneratedNameSuffix) + expectedArGeneratedName := fmt.Sprintf("%s-%s-%s-", r2.Name, at.Name, rs2PodHash) + expectedArName := fmt.Sprintf("%s%s", expectedArGeneratedName, MockGeneratedNameSuffix) assert.Equal(t, expectedArGeneratedName, createdAr.GenerateName) patch := f.getPatchedRollout(index) @@ -102,7 +157,7 @@ func TestCreateAnalysisRunOnAnalysisStep(t *testing.T) { assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, expectedArName)), patch) } -func TestFailCreateAnalysisRunIfInvalidTemplateRef(t *testing.T) { +func TestFailCreateStepAnalysisRunIfInvalidTemplateRef(t *testing.T) { f := newFixture(t) defer f.Close() @@ -133,7 +188,81 @@ func TestFailCreateAnalysisRunIfInvalidTemplateRef(t *testing.T) { f.runExpectError(getKey(r2, t), true) } -func TestDoNothingWhileAnalysisRunRunning(t *testing.T) { +func TestFailCreateBackgroundAnalysisRunIfInvalidTemplateRef(t *testing.T) { + f := newFixture(t) + defer f.Close() + + steps := []v1alpha1.CanaryStep{{ + SetWeight: pointer.Int32Ptr(10), + }} + + r1 := newCanaryRollout("foo", 1, nil, steps, pointer.Int32Ptr(0), intstr.FromInt(0), intstr.FromInt(1)) + r2 := bumpVersion(r1) + r2.Spec.Strategy.CanaryStrategy.Analysis = &v1alpha1.RolloutAnalysisStep{ + TemplateName: "invalid-template-ref", + } + + 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] + + 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) + + f.rolloutLister = append(f.rolloutLister, r2) + f.objects = append(f.objects, r2) + + f.runExpectError(getKey(r2, t), true) +} + +func TestDoNothingWithAnalysisRunsWhileBackgroundAnalysisRunRunning(t *testing.T) { + f := newFixture(t) + defer f.Close() + + at := analysisTemplate("bar") + steps := []v1alpha1.CanaryStep{{ + SetWeight: pointer.Int32Ptr(10), + }} + + r1 := newCanaryRollout("foo", 1, nil, steps, pointer.Int32Ptr(0), intstr.FromInt(0), intstr.FromInt(1)) + r2 := bumpVersion(r1) + r2.Spec.Strategy.CanaryStrategy.Analysis = &v1alpha1.RolloutAnalysisStep{ + TemplateName: at.Name, + } + ar := analysisRun(at, v1alpha1.RolloutTypeBackgroundRunLabel, r2) + + 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] + + 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) + r2.Status.Canary.CurrentBackgroundAnalysisRun = ar.Name + + f.rolloutLister = append(f.rolloutLister, r2) + f.analysisTemplateLister = append(f.analysisTemplateLister, at) + f.analysisRunLister = append(f.analysisRunLister, ar) + f.objects = append(f.objects, r2, at, ar) + + f.expectUpdateReplicaSetAction(rs2) + patchIndex := f.expectPatchRolloutAction(r2) + f.run(getKey(r2, t)) + patch := f.getPatchedRollout(patchIndex) + assert.Equal(t, calculatePatch(r2, OnlyObservedGenerationPatch), patch) +} + +func TestDoNothingWhileStepBasedAnalysisRunRunning(t *testing.T) { f := newFixture(t) defer f.Close() @@ -188,6 +317,8 @@ func TestCancelOlderAnalysisRuns(t *testing.T) { ar := analysisRun(at, v1alpha1.RolloutTypeStepLabel, r2) olderAr := ar.DeepCopy() olderAr.Name = "older-analysis-run" + oldBackgroundAr := analysisRun(at, v1alpha1.RolloutTypeBackgroundRunLabel, r2) + oldBackgroundAr.Name = "old-background-run" rs1 := newReplicaSetWithStatus(r1, 1, 1) rs2 := newReplicaSetWithStatus(r2, 0, 0) @@ -201,19 +332,29 @@ func TestCancelOlderAnalysisRuns(t *testing.T) { availableCondition, _ := newAvailableCondition(true) conditions.SetRolloutCondition(&r2.Status, availableCondition) r2.Status.Canary.CurrentStepAnalysisRun = ar.Name + r2.Status.Canary.CurrentBackgroundAnalysisRun = oldBackgroundAr.Name f.rolloutLister = append(f.rolloutLister, r2) f.analysisTemplateLister = append(f.analysisTemplateLister, at) - f.analysisRunLister = append(f.analysisRunLister, ar, olderAr) - f.objects = append(f.objects, r2, at, ar, olderAr) + f.analysisRunLister = append(f.analysisRunLister, ar, olderAr, oldBackgroundAr) + f.objects = append(f.objects, r2, at, ar, olderAr, oldBackgroundAr) cancelOldAr := f.expectPatchAnalysisRunAction(olderAr) + cancelBackgroundAr := f.expectPatchAnalysisRunAction(oldBackgroundAr) patchIndex := f.expectPatchRolloutAction(r2) f.run(getKey(r2, t)) assert.True(t, f.verifyPatchedAnalysisRun(cancelOldAr, olderAr)) + assert.True(t, f.verifyPatchedAnalysisRun(cancelBackgroundAr, oldBackgroundAr)) patch := f.getPatchedRollout(patchIndex) - assert.Equal(t, calculatePatch(r2, OnlyObservedGenerationPatch), patch) + expectedPatch := `{ + "status": { + "canary": { + "currentBackgroundAnalysisRun": null + } + } + }` + assert.Equal(t, calculatePatch(r2, expectedPatch), patch) } func TestIncrementStepAfterSuccessfulAnalysisRun(t *testing.T) { diff --git a/rollout/canary.go b/rollout/canary.go index b31e62e3fa..e07526f39c 100644 --- a/rollout/canary.go +++ b/rollout/canary.go @@ -268,6 +268,13 @@ func (c *RolloutController) syncRolloutStatusCanary(olderRSs []*appsv1.ReplicaSe newStatus.Canary.CurrentStepAnalysisRun = currStepAr.Name } + } + currBackgroundAr := analysisutil.GetCurrentBackgroundAnalysisRun(currArs) + if currBackgroundAr != nil { + if currBackgroundAr.Status == nil || !currBackgroundAr.Status.Status.Completed() || analysisutil.IsTerminating(currBackgroundAr) { + newStatus.Canary.CurrentBackgroundAnalysisRun = currBackgroundAr.Name + } + } if !r.Spec.Paused { @@ -292,7 +299,6 @@ func (c *RolloutController) syncRolloutStatusCanary(olderRSs []*appsv1.ReplicaSe return c.persistRolloutStatus(r, &newStatus, pointer.BoolPtr(false)) } - //TODO(dthomson): Add steps to store CurrentBackgroundAnalysisRun if completedCurrentCanaryStep(olderRSs, newRS, stableRS, currExp, currStepAr, r) { *currentStepIndex++ newStatus.CurrentStepIndex = currentStepIndex diff --git a/rollout/controller_test.go b/rollout/controller_test.go index 7e59860024..99112ee062 100644 --- a/rollout/controller_test.go +++ b/rollout/controller_test.go @@ -415,7 +415,7 @@ func (f *fixture) newController(resync resyncFunc) (*RolloutController, informer converter := runtime.NewTestUnstructuredConverter(equality.Semantic) objMap, _ := converter.ToUnstructured(createAction.GetObject()) runtime.NewTestUnstructuredConverter(equality.Semantic).FromUnstructured(objMap, ar) - ar.Name = ar.GenerateName + "-" + MockGeneratedNameSuffix + ar.Name = ar.GenerateName + MockGeneratedNameSuffix return true, ar.DeepCopyObject(), nil }) diff --git a/utils/analysis/factory.go b/utils/analysis/factory.go index 743c1c9785..b75eea295a 100644 --- a/utils/analysis/factory.go +++ b/utils/analysis/factory.go @@ -33,8 +33,16 @@ func BuildArgumentsForRolloutAnalysisRun(rolloutAnalysisRun *v1alpha1.RolloutAna return arguments } +// BackgroundLabels returns a map[string]string of common labels for the background analysis +func BackgroundLabels(podHash string) map[string]string { + return map[string]string{ + v1alpha1.DefaultRolloutUniqueLabelKey: podHash, + v1alpha1.RolloutTypeLabel: v1alpha1.RolloutTypeBackgroundRunLabel, + } +} + // StepLabels returns a map[string]string of common labels for analysisruns created from an analysis step -func StepLabels(r *v1alpha1.Rollout, index int32, podHash string) map[string]string { +func StepLabels(index int32, podHash string) map[string]string { indexStr := strconv.Itoa(int(index)) return map[string]string{ v1alpha1.DefaultRolloutUniqueLabelKey: podHash, diff --git a/utils/analysis/factory_test.go b/utils/analysis/factory_test.go index 90f87659de..6df1b55462 100644 --- a/utils/analysis/factory_test.go +++ b/utils/analysis/factory_test.go @@ -54,18 +54,23 @@ func TestBuildArgumentsForRolloutAnalysisRun(t *testing.T) { } func TestStepLabels(t *testing.T) { - ro := &v1alpha1.Rollout{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - }, - } podHash := "abcd123" expected := map[string]string{ v1alpha1.DefaultRolloutUniqueLabelKey: podHash, v1alpha1.RolloutTypeLabel: v1alpha1.RolloutTypeStepLabel, v1alpha1.RolloutCanaryStepIndexLabel: "1", } - generated := StepLabels(ro, 1, podHash) + generated := StepLabels(1, podHash) + assert.Equal(t, expected, generated) +} + +func TestBackgroundLabels(t *testing.T) { + podHash := "abcd123" + expected := map[string]string{ + v1alpha1.DefaultRolloutUniqueLabelKey: podHash, + v1alpha1.RolloutTypeLabel: v1alpha1.RolloutTypeBackgroundRunLabel, + } + generated := BackgroundLabels(podHash) assert.Equal(t, expected, generated) } diff --git a/utils/analysis/filter.go b/utils/analysis/filter.go index 577e5a3a6b..876cbf1d77 100644 --- a/utils/analysis/filter.go +++ b/utils/analysis/filter.go @@ -16,6 +16,18 @@ func GetCurrentStepAnalysisRun(currentArs []*v1alpha1.AnalysisRun) *v1alpha1.Ana return nil } +//GetCurrentBackgroundAnalysisRun filters the currentArs and returns the background based analysis run +func GetCurrentBackgroundAnalysisRun(currentArs []*v1alpha1.AnalysisRun) *v1alpha1.AnalysisRun { + for i := range currentArs { + ar := currentArs[i] + rolloutType, ok := ar.Labels[v1alpha1.RolloutTypeLabel] + if ok && rolloutType == v1alpha1.RolloutTypeBackgroundRunLabel { + return ar + } + } + return nil +} + // 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 { diff --git a/utils/analysis/filter_test.go b/utils/analysis/filter_test.go index d966907f7a..da8172f373 100644 --- a/utils/analysis/filter_test.go +++ b/utils/analysis/filter_test.go @@ -9,6 +9,33 @@ import ( "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" ) +func TestGetCurrentBackgroundAnalysisRun(t *testing.T) { + arsWithBackground := []*v1alpha1.AnalysisRun{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Labels: map[string]string{ + v1alpha1.RolloutTypeLabel: v1alpha1.RolloutTypeBackgroundRunLabel, + }, + }, + }, + } + currAr := GetCurrentBackgroundAnalysisRun(arsWithBackground) + assert.Equal(t, arsWithBackground[0], currAr) + arsWithNoBackground := []*v1alpha1.AnalysisRun{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Labels: map[string]string{ + v1alpha1.RolloutTypeLabel: v1alpha1.RolloutTypeStepLabel, + }, + }, + }, + } + currAr = GetCurrentBackgroundAnalysisRun(arsWithNoBackground) + assert.Nil(t, currAr) +} + func TestGetCurrentStepAnalysisRun(t *testing.T) { arsWithSteps := []*v1alpha1.AnalysisRun{ {