Set StableRS hash to current if replicaset does not actually exist

rollout/canary.go

@@ -1,6 +1,7 @@
 package rollout
 
 import (
+	"fmt"
 	"sort"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -34,22 +35,20 @@ func (c *RolloutController) rolloutCanary(rollout *v1alpha1.Rollout, rsList []*a
 		if err != nil {
 			return err
 		}
-		stableRS, oldRSs := replicasetutil.GetStableRS(rollout, newRS, previousRSs)
-		roCtx := newCanaryCtx(rollout, newRS, stableRS, oldRSs, exList, arList)
+		roCtx := newCanaryCtx(rollout, newRS, previousRSs, exList, arList)
 		return c.syncRolloutStatusCanary(roCtx)
 	}
 
 	newRS, previousRSs, err := c.getAllReplicaSetsAndSyncRevision(rollout, rsList, true)
 	if err != nil {
 		return err
 	}
-	stableRS, oldRSs := replicasetutil.GetStableRS(rollout, newRS, previousRSs)
 
-	roCtx := newCanaryCtx(rollout, newRS, stableRS, oldRSs, exList, arList)
+	roCtx := newCanaryCtx(rollout, newRS, previousRSs, exList, arList)
 	logCtx := roCtx.Log()
 
 	logCtx.Info("Cleaning up old replicasets, experiments, and analysis runs")
-	if err := c.cleanupRollouts(oldRSs, roCtx); err != nil {
+	if err := c.cleanupRollouts(roCtx.OlderRSs(), roCtx); err != nil {
 		return err
 	}
 
@@ -251,6 +250,7 @@ func (c *RolloutController) syncRolloutStatusCanary(roCtx *canaryContext) error
 	r := roCtx.Rollout()
 	logCtx := roCtx.Log()
 	newRS := roCtx.NewRS()
+	stableRS := roCtx.StableRS()
 	allRSs := roCtx.AllRSs()
 
 	newStatus := c.calculateBaseStatus(roCtx)
@@ -279,8 +279,8 @@ func (c *RolloutController) syncRolloutStatusCanary(roCtx *canaryContext) error
 		return c.persistRolloutStatus(roCtx, &newStatus)
 	}
 
-	if r.Status.Canary.StableRS == "" {
-		msg := "Setting StableRS to CurrentPodHash as it is empty beforehand"
+	if stableRS == nil {
+		msg := fmt.Sprintf("Setting StableRS to CurrentPodHash: StableRS hash: %s", newStatus.CurrentPodHash)
 		logCtx.Info(msg)
 		newStatus.Canary.StableRS = newStatus.CurrentPodHash
 		if stepCount > 0 {

rollout/canary_test.go

@@ -62,14 +62,14 @@ func TestReconcileCanaryStepsHandleBaseCases(t *testing.T) {
 
 	// Handle case with no steps
 	r := newCanaryRollout("test", 1, nil, nil, nil, intstr.FromInt(0), intstr.FromInt(1))
-	roCtx := newCanaryCtx(r, nil, nil, nil, nil, nil)
+	roCtx := newCanaryCtx(r, nil, nil, nil, nil)
 	stepResult := controller.reconcileCanaryPause(roCtx)
 	assert.False(t, stepResult)
 	assert.Len(t, fake.Actions(), 0)
 
 	r2 := newCanaryRollout("test", 1, nil, []v1alpha1.CanaryStep{{SetWeight: int32Ptr(10)}}, nil, intstr.FromInt(0), intstr.FromInt(1))
 	r2.Status.CurrentStepIndex = int32Ptr(1)
-	roCtx2 := newCanaryCtx(r2, nil, nil, nil, nil, nil)
+	roCtx2 := newCanaryCtx(r2, nil, nil, nil, nil)
 	stepResult = controller.reconcileCanaryPause(roCtx2)
 	assert.False(t, stepResult)
 	assert.Len(t, fake.Actions(), 0)
@@ -1118,7 +1118,7 @@ func TestNoResumeAfterPauseDurationIfUserPaused(t *testing.T) {
 	r1 := newCanaryRollout("foo", 1, nil, steps, pointer.Int32Ptr(1), intstr.FromInt(1), intstr.FromInt(1))
 	rs1 := newReplicaSetWithStatus(r1, 1, 1)
 	rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]
-	r1 = updateCanaryRolloutStatus(r1, rs1PodHash, 2, 1, 2, true)
+	r1 = updateCanaryRolloutStatus(r1, rs1PodHash, 1, 1, 1, true)
 	overAMinuteAgo := metav1.Time{Time: time.Now().Add(-61 * time.Second)}
 	r1.Status.ObservedGeneration = conditions.ComputeGenerationHash(r1.Spec)
 	r1.Status.PauseConditions = []v1alpha1.PauseCondition{{

rollout/context.go

@@ -9,6 +9,7 @@ import (
 	analysisutil "github.com/argoproj/argo-rollouts/utils/analysis"
 	experimentutil "github.com/argoproj/argo-rollouts/utils/experiment"
 	logutil "github.com/argoproj/argo-rollouts/utils/log"
+	replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
 )
 
 type rolloutContext interface {
@@ -119,11 +120,9 @@ func (bgCtx *blueGreenContext) NewStatus() v1alpha1.RolloutStatus {
 	return bgCtx.newStatus
 }
 
-func newCanaryCtx(r *v1alpha1.Rollout, newRS *appsv1.ReplicaSet, stableRS *appsv1.ReplicaSet, olderRSs []*appsv1.ReplicaSet, exList []*v1alpha1.Experiment, arList []*v1alpha1.AnalysisRun) *canaryContext {
-	allRSs := append(olderRSs, newRS)
-	if stableRS != nil {
-		allRSs = append(allRSs, stableRS)
-	}
+func newCanaryCtx(r *v1alpha1.Rollout, newRS *appsv1.ReplicaSet, otherRSs []*appsv1.ReplicaSet, exList []*v1alpha1.Experiment, arList []*v1alpha1.AnalysisRun) *canaryContext {
+	allRSs := append(otherRSs, newRS)
+	stableRS, oldRSs := replicasetutil.GetStableRS(r, newRS, otherRSs)
 
 	currentArs, otherArs := analysisutil.FilterCurrentRolloutAnalysisRuns(arList, r)
 	currentEx := experimentutil.GetCurrentExperiment(r, exList)
@@ -134,7 +133,7 @@ func newCanaryCtx(r *v1alpha1.Rollout, newRS *appsv1.ReplicaSet, stableRS *appsv
 		log:      logCtx,
 		newRS:    newRS,
 		stableRS: stableRS,
-		olderRSs: olderRSs,
+		olderRSs: oldRSs,
 		allRSs:   allRSs,
 
 		currentArs: currentArs,

rollout/sync.go

@@ -241,8 +241,7 @@ func (c *RolloutController) syncReplicasOnly(r *v1alpha1.Rollout, rsList []*apps
 			return err
 		}
 
-		stableRS, oldRSs := replicasetutil.GetStableRS(r, newRS, rsList)
-		roCtx := newCanaryCtx(r, newRS, stableRS, oldRSs, exList, arList)
+		roCtx := newCanaryCtx(r, newRS, oldRSs, exList, arList)
 
 		if isScaling {
 			if _, err := c.reconcileCanaryReplicaSets(roCtx); err != nil {

utils/replicaset/canary.go

@@ -261,6 +261,9 @@ func GetStableRS(rollout *v1alpha1.Rollout, newRS *appsv1.ReplicaSet, rslist []*
 	if rollout.Status.Canary.StableRS == "" {
 		return nil, rslist
 	}
+	if newRS != nil && newRS.Labels != nil && newRS.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] == rollout.Status.Canary.StableRS {
+		return newRS, rslist
+	}
 	olderRSs := []*appsv1.ReplicaSet{}
 	var stableRS *appsv1.ReplicaSet
 	for i := range rslist {

utils/replicaset/canary_test.go

@@ -388,6 +388,36 @@ func TestCalculateReplicaCountsForCanaryStableRSdEdgeCases(t *testing.T) {
 	assert.Equal(t, int32(0), stableRSReplicaCount)
 }
 
+func TestGetStableRS(t *testing.T) {
+	rs := func(podHash string) appsv1.ReplicaSet {
+		return appsv1.ReplicaSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: podHash,
+				Labels: map[string]string{
+					v1alpha1.DefaultRolloutUniqueLabelKey: podHash,
+				},
+			},
+		}
+	}
+
+	rollout := &v1alpha1.Rollout{}
+	rs1 := rs("1")
+	rs2 := rs("2")
+	rs3 := rs("3")
+	noStable, rsList := GetStableRS(rollout, &rs1, []*appsv1.ReplicaSet{&rs2, &rs3})
+	assert.Nil(t, noStable)
+	assert.Len(t, rsList, 2)
+
+	rollout.Status.Canary.StableRS = "1"
+	sameAsNewRS, rsList := GetStableRS(rollout, &rs1, []*appsv1.ReplicaSet{&rs2, &rs3})
+	assert.Equal(t, *sameAsNewRS, rs1)
+	assert.Len(t, rsList, 2)
+
+	stableInOtherRSs, rsList := GetStableRS(rollout, &rs2, []*appsv1.ReplicaSet{&rs1, &rs2, &rs3})
+	assert.Equal(t, *stableInOtherRSs, rs1)
+	assert.Len(t, rsList, 1)
+
+}
 func TestGetCurrentCanaryStep(t *testing.T) {
 	rollout := newRollout(10, 10, intstr.FromInt(0), intstr.FromInt(1), "", "")
 	rollout.Spec.Strategy.Canary.Steps = nil