Use a different method of propogating information about delayed servi…

…ce swaps

Signed-off-by: Jack Andersen <[email protected]>

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
 	}
 

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 {

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

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(), "")
-}

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())