fix!: subresource scale selector changed for traffic routed canary

rollout/canary.go

@@ -360,6 +360,16 @@ func (c *rolloutContext) syncRolloutStatusCanary() error {
 	newStatus.AvailableReplicas = replicasetutil.GetAvailableReplicaCountForReplicaSets(c.allRSs)
 	newStatus.HPAReplicas = replicasetutil.GetActualReplicaCountForReplicaSets(c.allRSs)
 	newStatus.Selector = metav1.FormatLabelSelector(c.rollout.Spec.Selector)
+
+	if c.rollout.Spec.Strategy.Canary != nil && c.rollout.Spec.Strategy.Canary.TrafficRouting != nil && !c.rollout.Spec.Strategy.Canary.DynamicStableScale && c.stableRS != nil {
+		// When using traffic routed canary without scaling down the stable replicaset, the number of pods
+		// selected by the rollout selector will be up to twice the amount of desired spec.replicas.
+		// We update the selector to select the stable pods since the Scale subresource expects a
+		// label that queries over pods that should match the replicas count, because that is the number
+		// of pods that will be receiving traffic.
+		newStatus.Selector = metav1.FormatLabelSelector(c.stableRS.Spec.Selector)
+	}
+
 	newStatus.Canary.StablePingPong = c.rollout.Status.Canary.StablePingPong
 	newStatus.Canary.StepPluginStatuses = c.rollout.Status.Canary.StepPluginStatuses
 	c.stepPluginContext.updateStatus(&newStatus)

rollout/service_test.go

@@ -495,10 +495,14 @@ func TestCanaryAWSVerifyTargetGroupsNotYetReady(t *testing.T) {
 	f.ingressLister = append(f.ingressLister, ingressutil.NewLegacyIngress(ing))
 
 	f.expectGetEndpointsAction(ep)
+	rolloutPatchIndex := f.expectPatchRolloutAction(r2)
 	f.run(getKey(r2, t))
 	f.assertEvents([]string{
 		conditions.TargetGroupUnverifiedReason,
 	})
+	patch := f.getPatchedRollout(rolloutPatchIndex)
+	expectedPatch := `{"status":{"selector":"foo=bar,rollouts-pod-template-hash=58c48fdff5"}}`
+	assert.JSONEq(t, expectedPatch, patch)
 }
 
 // TestCanaryAWSVerifyTargetGroupsReady verifies we proceed with scale down of old
@@ -595,11 +599,16 @@ func TestCanaryAWSVerifyTargetGroupsReady(t *testing.T) {
 
 	f.expectGetEndpointsAction(ep)
 	scaleDownRSIndex := f.expectPatchReplicaSetAction(rs1)
+
+	rolloutPatchIndex := f.expectPatchRolloutAction(r2)
 	f.run(getKey(r2, t))
 	f.verifyPatchedReplicaSet(scaleDownRSIndex, 30)
 	f.assertEvents([]string{
 		conditions.TargetGroupVerifiedReason,
 	})
+	patch := f.getPatchedRollout(rolloutPatchIndex)
+	expectedPatch := `{"status":{"selector":"foo=bar,rollouts-pod-template-hash=58c48fdff5"}}`
+	assert.JSONEq(t, expectedPatch, patch)
 }
 
 // TestCanaryAWSVerifyTargetGroupsSkip verifies we skip unnecessary verification if scaledown
@@ -657,8 +666,13 @@ func TestCanaryAWSVerifyTargetGroupsSkip(t *testing.T) {
 	f.serviceLister = append(f.serviceLister, rootSvc, canarySvc, stableSvc)
 	f.ingressLister = append(f.ingressLister, ingressutil.NewLegacyIngress(ing))
 
+	patchIndex := f.expectPatchRolloutAction(r2)
 	f.run(getKey(r2, t)) // there should be no api calls
 	f.assertEvents(nil)
+
+	patch := f.getPatchedRollout(patchIndex)
+	expectedPatch := `{"status":{"selector":"foo=bar,rollouts-pod-template-hash=58c48fdff5"}}`
+	assert.Equal(t, expectedPatch, patch)
 }
 
 // TestShouldVerifyTargetGroups returns whether or not we should verify the target group

rollout/trafficrouting_test.go

@@ -839,10 +839,14 @@ func TestCanaryWithTrafficRoutingAddScaleDownDelay(t *testing.T) {
 	f.rolloutLister = append(f.rolloutLister, r2)
 	f.objects = append(f.objects, r2)
 
-	rs1Patch := f.expectPatchReplicaSetAction(rs1) // set scale-down-deadline annotation
+	rs1Patch := f.expectPatchReplicaSetAction(rs1)      // set scale-down-deadline annotation
+	rolloutPatchIndex := f.expectPatchRolloutAction(r2) // patch to update rollout status, hpa selector
 	f.run(getKey(r2, t))
 
 	f.verifyPatchedReplicaSet(rs1Patch, 30)
+	updatedRollout := f.getPatchedRollout(rolloutPatchIndex)
+	expectedRolloutPatch := `{"status":{"selector":"foo=bar,rollouts-pod-template-hash=58c48fdff5"}}`
+	assert.JSONEq(t, expectedRolloutPatch, updatedRollout)
 }
 
 // Verifies with a canary using traffic routing, we scale down old ReplicaSets which exceed our limit