From 4c69edd5ffb1c42625cec72976aed51d1aa7c47e Mon Sep 17 00:00:00 2001
From: Jack Andersen <jandersen@plaid.com>
Date: Tue, 8 Nov 2022 17:36:32 -0500
Subject: [PATCH] Use a different method of propogating information about
 delayed service swaps

Signed-off-by: Jack Andersen <jandersen@plaid.com>
---
 rollout/canary.go         |  6 ------
 rollout/context.go        |  4 ++++
 rollout/service.go        |  4 +++-
 rollout/service_test.go   | 13 ++++++-------
 rollout/trafficrouting.go |  4 ++++
 5 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/rollout/canary.go b/rollout/canary.go
index acc35e6dce..87228499ab 100644
--- a/rollout/canary.go
+++ b/rollout/canary.go
@@ -1,7 +1,6 @@
 package rollout
 
 import (
-	"errors"
 	"sort"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -51,11 +50,6 @@ func (c *rolloutContext) rolloutCanary() error {
 	}
 
 	if err := c.reconcileStableAndCanaryService(); err != nil {
-		// if we cannot reconcile the stable and canary services then
-		// we should not continue to adjust traffic routing
-		if errors.Is(err, DelayServiceSelectorSwapError) {
-			return nil
-		}
 		return err
 	}
 
diff --git a/rollout/context.go b/rollout/context.go
index f8fa5b5f03..14283e3a83 100644
--- a/rollout/context.go
+++ b/rollout/context.go
@@ -49,6 +49,10 @@ type rolloutContext struct {
 	// (e.g. a setWeight step, after a blue-green active switch, after stable service switch),
 	// since we do not want to continually verify weight in case it could incur rate-limiting or other expenses.
 	targetsVerified *bool
+
+	// skippedSelectorSwap indicates that we have skipped swapping selectors
+	// and are not ready to perform any final actions of moving to 100%
+	skippedSelectorSwap *bool
 }
 
 func (c *rolloutContext) reconcile() error {
diff --git a/rollout/service.go b/rollout/service.go
index 8a657efb8d..0337c8352b 100644
--- a/rollout/service.go
+++ b/rollout/service.go
@@ -290,8 +290,10 @@ func (c *rolloutContext) ensureSVCTargets(svcName string, rs *appsv1.ReplicaSet,
 		if checkRsAvailability && !replicasetutil.IsReplicaSetAvailable(rs) {
 			logCtx := c.log.WithField(logutil.ServiceKey, svc.Name)
 			logCtx.Infof("delaying service switch from %s to %s: ReplicaSet not fully available", currSelector, desiredSelector)
-			return DelayServiceSelectorSwapError
+			c.skippedSelectorSwap = pointer.BoolPtr(true)
+			return nil
 		}
+		c.skippedSelectorSwap = pointer.BoolPtr(false)
 		err = c.switchServiceSelector(svc, desiredSelector, c.rollout)
 		if err != nil {
 			return err
diff --git a/rollout/service_test.go b/rollout/service_test.go
index ba34b3c31a..c14506ab3c 100644
--- a/rollout/service_test.go
+++ b/rollout/service_test.go
@@ -781,12 +781,16 @@ func TestDelayCanaryStableServiceLabelInjection(t *testing.T) {
 		roCtx.stableRS = newReplicaSetWithStatus(ro2, 3, 0)
 
 		err = roCtx.reconcileStableAndCanaryService()
-		// an error is returned because we are delaying
-		assert.Equal(t, err, DelayServiceSelectorSwapError)
+		assert.NoError(t, err)
 		_, canaryInjected := canarySvc.Spec.Selector[v1alpha1.DefaultRolloutUniqueLabelKey]
 		assert.False(t, canaryInjected)
 		_, stableInjected := stableSvc.Spec.Selector[v1alpha1.DefaultRolloutUniqueLabelKey]
 		assert.False(t, stableInjected)
+
+		assert.NotNil(t, roCtx.skippedSelectorSwap)
+		if roCtx.skippedSelectorSwap != nil {
+			assert.True(t, *roCtx.skippedSelectorSwap)
+		}
 	}
 	{
 		// next ensure we do update service because new/stable are now available
@@ -806,8 +810,3 @@ func TestDelayCanaryStableServiceLabelInjection(t *testing.T) {
 	}
 
 }
-
-func TestDelayServiceSelectorSwapError(t *testing.T) {
-	assert.Error(t, DelayServiceSelectorSwapError)
-	assert.NotEqual(t, DelayServiceSelectorSwapError.Error(), "")
-}
diff --git a/rollout/trafficrouting.go b/rollout/trafficrouting.go
index ebc22b9704..f539b44c8c 100644
--- a/rollout/trafficrouting.go
+++ b/rollout/trafficrouting.go
@@ -220,6 +220,10 @@ func (c *rolloutContext) reconcileTrafficRouting() error {
 			return err
 		}
 
+		if c.skippedSelectorSwap != nil && *c.skippedSelectorSwap {
+			continue
+		}
+
 		err = reconciler.SetWeight(desiredWeight, weightDestinations...)
 		if err != nil {
 			c.recorder.Warnf(c.rollout, record.EventOptions{EventReason: "TrafficRoutingError"}, err.Error())