use rollout status to get the replicaset hash instead of service

Signed-off-by: Zach Aller <zachaller@users.noreply.github.com>

rollout/canary.go

@@ -435,13 +435,13 @@ func (c *rolloutContext) reconcileCanaryReplicaSets() (bool, error) {
 		return true, nil
 	}
 
-	if c.newRS == nil {
-		canaryRS, err := c.getCanaryReplicaSet()
-		if err != nil {
-			return false, err
-		}
-		c.newRS = canaryRS
+	// If we have updated both the replica count and the pod template hash c.newRS will be nil we want to reconcile the newRS so we look at the
+	// rollout status to get the newRS to reconcile it.
+	if c.newRS == nil && c.rollout.Status.CurrentPodHash != c.rollout.Status.StableRS {
+		rs, _ := replicasetutil.GetReplicaSetByTemplateHash(c.allRSs, c.rollout.Status.CurrentPodHash)
+		c.newRS = rs
 	}
+
 	scaledNewRS, err := c.reconcileNewReplicaSet()
 	if err != nil {
 		return false, err

rollout/canary_test.go

@@ -2037,6 +2037,7 @@ func TestCanaryReplicaAndSpecChangedTogether(t *testing.T) {
 	r3 := bumpVersion(r2)
 	r3.Spec.Replicas = pointer.Int32(int32(originReplicas) + 5)
 	r3.Status.StableRS = stableRS.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]
+	r3.Status.CurrentPodHash = canaryRS.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]
 
 	f.rolloutLister = append(f.rolloutLister, r3)
 	f.kubeobjects = append(f.kubeobjects, canaryRS, stableRS, canarySVC, stableSVC)

rollout/service.go

@@ -17,7 +17,6 @@ import (
 	serviceutil "github.com/argoproj/argo-rollouts/utils/service"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
-	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	patchtypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/pointer"
@@ -285,27 +284,6 @@ func (c *rolloutContext) reconcileStableAndCanaryService() error {
 	return nil
 }
 
-func (c *rolloutContext) getCanaryReplicaSet() (*appsv1.ReplicaSet, error) {
-	if c.rollout.Spec.Strategy.Canary == nil || c.rollout.Spec.Strategy.Canary.CanaryService == "" {
-		return nil, nil
-	}
-	svc, err := c.servicesLister.Services(c.rollout.Namespace).Get(c.rollout.Spec.Strategy.Canary.CanaryService)
-	if err != nil {
-		if k8serrors.IsNotFound(err) {
-			return nil, nil
-		}
-		return nil, err
-	}
-	if svc != nil {
-		for _, rs := range c.allRSs {
-			if serviceutil.GetRolloutSelectorLabel(svc) == rs.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] {
-				return rs, nil
-			}
-		}
-	}
-	return nil, nil
-}
-
 // ensureSVCTargets updates the service with the given name to point to the given ReplicaSet,
 // but only if that ReplicaSet has proper availability. There is still an edge case with this function if
 // in the small window of time between a rollout being completed, and we try to update the service selector, we lose 100%

rollout/service_test.go

@@ -879,92 +879,3 @@ func TestDelayCanaryStableServiceDelayOnAdoptedService(t *testing.T) {
 	})
 
 }
-
-func TestGetCanaryReplicaSet(t *testing.T) {
-	t.Run("Not Canary Strategy", func(t *testing.T) {
-		f := newFixture(t)
-		defer f.Close()
-		ctrl, _, _ := f.newController(noResyncPeriodFunc)
-		ro := newRollout("foo", 3, nil, nil)
-		roCtx, err := ctrl.newRolloutContext(ro)
-		assert.NoError(t, err)
-		rs, err := roCtx.getCanaryReplicaSet()
-		assert.NoError(t, err)
-		assert.Nil(t, rs)
-	},
-	)
-
-	t.Run("Empty Canary Service", func(t *testing.T) {
-		f := newFixture(t)
-		defer f.Close()
-		ctrl, _, _ := f.newController(noResyncPeriodFunc)
-		ro := newCanaryRollout("foo", 3, nil, nil, nil, intstr.FromInt(1), intstr.FromInt(1))
-		ro.Spec.Strategy.Canary.CanaryService = ""
-		roCtx, err := ctrl.newRolloutContext(ro)
-		assert.NoError(t, err)
-		rs, err := roCtx.getCanaryReplicaSet()
-		assert.NoError(t, err)
-		assert.Nil(t, rs)
-	},
-	)
-
-	t.Run("No Canary SVC", func(t *testing.T) {
-		f := newFixture(t)
-		defer f.Close()
-		ctrl, _, _ := f.newController(noResyncPeriodFunc)
-		ro := newCanaryRollout("foo", 3, nil, nil, nil, intstr.FromInt(1), intstr.FromInt(1))
-		ro.Spec.Strategy.Canary.CanaryService = "canary"
-		roCtx, err := ctrl.newRolloutContext(ro)
-		assert.NoError(t, err)
-		rs, err := roCtx.getCanaryReplicaSet()
-		assert.Nil(t, rs)
-		assert.NoError(t, err)
-	},
-	)
-
-	t.Run("Have Canary SVC", func(t *testing.T) {
-		f := newFixture(t)
-		defer f.Close()
-		ro := newCanaryRollout("foo", 3, nil, nil, nil, intstr.FromInt(1), intstr.FromInt(1))
-		canarySVCName := "canary"
-		ro.Spec.Strategy.Canary.CanaryService = canarySVCName
-		canaryRS := newReplicaSetWithStatus(ro, 3, 3)
-		canarySVC := newService(canarySVCName, 80,
-			map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: canaryRS.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]}, ro)
-		ro.Spec.Strategy.Canary.CanaryService = canarySVCName
-		f.rolloutLister = append(f.rolloutLister, ro)
-		f.kubeobjects = append(f.kubeobjects, canaryRS, canarySVC)
-		f.replicaSetLister = append(f.replicaSetLister, canaryRS)
-		f.serviceLister = append(f.serviceLister, canarySVC)
-
-		ctrl, _, _ := f.newController(noResyncPeriodFunc)
-		roCtx, err := ctrl.newRolloutContext(ro)
-		assert.NoError(t, err)
-		rs, err := roCtx.getCanaryReplicaSet()
-		assert.NoError(t, err)
-		assert.NotNil(t, rs)
-	})
-
-	t.Run("No Matched Replicaset", func(t *testing.T) {
-		f := newFixture(t)
-		defer f.Close()
-		ro := newCanaryRollout("foo", 3, nil, nil, nil, intstr.FromInt(1), intstr.FromInt(1))
-		canarySVCName := "canary"
-		ro.Spec.Strategy.Canary.CanaryService = canarySVCName
-		canaryRS := newReplicaSetWithStatus(ro, 3, 0)
-		canarySVC := newService(canarySVCName, 80,
-			map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: "youknowthisisdifferent"}, ro)
-		ro.Spec.Strategy.Canary.CanaryService = canarySVCName
-		f.rolloutLister = append(f.rolloutLister, ro)
-		f.kubeobjects = append(f.kubeobjects, canaryRS, canarySVC)
-		f.replicaSetLister = append(f.replicaSetLister, canaryRS)
-		f.serviceLister = append(f.serviceLister, canarySVC)
-
-		ctrl, _, _ := f.newController(noResyncPeriodFunc)
-		roCtx, err := ctrl.newRolloutContext(ro)
-		assert.NoError(t, err)
-		rs, err := roCtx.getCanaryReplicaSet()
-		assert.NoError(t, err)
-		assert.Nil(t, rs)
-	})
-}