fix: retarget blue-green preview service before scaling up preview re…

…plicaset Signed-off-by: Jesse Suen <jesse_suen@intuit.com>
argoproj · Jul 30, 2021 · 45da217 · 45da217
1 parent 4e993ed
commit 45da217
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 21 deletions.
diff --git a/rollout/bluegreen.go b/rollout/bluegreen.go
@@ -25,6 +25,12 @@ func (c *rolloutContext) rolloutBlueGreen() error {
 		return err
 	}
 
+	// This must happen right after the new replicaset is created
+	err = c.reconcilePreviewService(previewSvc)
+	if err != nil {
+		return err
+	}
+
 	if replicasetutil.CheckPodSpecChange(c.rollout, c.newRS) {
 		return c.syncRolloutStatusBlueGreen(previewSvc, activeSvc)
 	}
@@ -39,11 +45,6 @@ func (c *rolloutContext) rolloutBlueGreen() error {
 		return err
 	}
 
-	err = c.reconcilePreviewService(previewSvc)
-	if err != nil {
-		return err
-	}
-
 	c.reconcileBlueGreenPause(activeSvc, previewSvc)
 
 	err = c.reconcileActiveService(activeSvc)

diff --git a/rollout/ephemeralmetadata_test.go b/rollout/ephemeralmetadata_test.go
@@ -209,6 +209,7 @@ func TestSyncBlueGreenEphemeralMetadataSecondRevision(t *testing.T) {
 	f.expectUpdateRolloutStatusAction(r2)              // Update Rollout conditions
 	rs2idx := f.expectCreateReplicaSetAction(rs2)      // Create revision 2 ReplicaSet
 	f.expectPatchServiceAction(previewSvc, rs2PodHash) // Update preview service to point at revision 2 replicaset
+	f.expectPatchReplicaSetAction(rs2)                 // scale revision 2 ReplicaSet up
 	f.expectListPodAction(r1.Namespace)                // list pods to patch ephemeral data on revision 1 ReplicaSets pods`
 	podIdx := f.expectUpdatePodAction(&pod)            // Update pod with ephemeral data
 	rs1idx := f.expectUpdateReplicaSetAction(rs1)      // update stable replicaset with stable metadata

diff --git a/rollout/sync.go b/rollout/sync.go
@@ -159,13 +159,7 @@ func (c *rolloutContext) createDesiredReplicaSet() (*appsv1.ReplicaSet, error) {
 			Template:        newRSTemplate,
 		},
 	}
-	allRSs := append(c.allRSs, newRS)
-	newReplicasCount, err := replicasetutil.NewRSNewReplicas(c.rollout, allRSs, newRS)
-	if err != nil {
-		return nil, err
-	}
-
-	newRS.Spec.Replicas = pointer.Int32Ptr(newReplicasCount)
+	newRS.Spec.Replicas = pointer.Int32Ptr(0)
 	// Set new replica set's annotation
 	annotations.SetNewReplicaSetAnnotations(c.rollout, newRS, newRevision, false)
 
@@ -250,12 +244,10 @@ func (c *rolloutContext) createDesiredReplicaSet() (*appsv1.ReplicaSet, error) {
 		return nil, err
 	}
 
-	if !alreadyExists && newReplicasCount > 0 {
+	if !alreadyExists {
 		revision, _ := replicasetutil.Revision(createdRS)
-		c.recorder.Eventf(c.rollout, record.EventOptions{EventReason: conditions.NewReplicaSetReason}, conditions.NewReplicaSetDetailedMessage, createdRS.Name, revision, newReplicasCount)
-	}
+		c.recorder.Eventf(c.rollout, record.EventOptions{EventReason: conditions.NewReplicaSetReason}, conditions.NewReplicaSetDetailedMessage, createdRS.Name, revision)
 
-	if !alreadyExists {
 		msg := fmt.Sprintf(conditions.NewReplicaSetMessage, createdRS.Name)
 		condition := conditions.NewRolloutCondition(v1alpha1.RolloutProgressing, corev1.ConditionTrue, conditions.NewReplicaSetReason, msg)
 		conditions.SetRolloutCondition(&c.rollout.Status, *condition)

diff --git a/test/e2e/functional_test.go b/test/e2e/functional_test.go
@@ -90,10 +90,12 @@ spec:
 		ExpectRevisionPodCount("2", 1).
 		ExpectRolloutEvents([]string{
 			"RolloutUpdated",       // Rollout updated to revision 1
-			"NewReplicaSetCreated", // Created ReplicaSet abort-retry-promote-698fbfb9dc (revision 1) with size 1
+			"NewReplicaSetCreated", // Created ReplicaSet abort-retry-promote-698fbfb9dc (revision 1)
+			"ScalingReplicaSet",    // Scaled up ReplicaSet abort-retry-promote-698fbfb9dc (revision 1) from 0 to 1
 			"RolloutCompleted",     // Rollout completed update to revision 1 (698fbfb9dc): Initial deploy
 			"RolloutUpdated",       // Rollout updated to revision 2
-			"NewReplicaSetCreated", // Created ReplicaSet abort-retry-promote-75dcb5ddd6 (revision 2) with size 1
+			"NewReplicaSetCreated", // Created ReplicaSet abort-retry-promote-75dcb5ddd6 (revision 2)
+			"ScalingReplicaSet",    // Scaled up ReplicaSet abort-retry-promote-75dcb5ddd6 (revision 2) from 0 to 1
 			"RolloutStepCompleted", // Rollout step 1/2 completed (setWeight: 50)
 			"RolloutPaused",        // Rollout is paused (CanaryPauseStep)
 			"ScalingReplicaSet",    // Scaled down ReplicaSet abort-retry-promote-75dcb5ddd6 (revision 2) from 1 to 0
@@ -696,11 +698,13 @@ func (s *FunctionalSuite) TestBlueGreenUpdate() {
 		ExpectReplicaCounts(3, 6, 3, 3, 3).
 		ExpectRolloutEvents([]string{
 			"RolloutUpdated",       // Rollout updated to revision 1
-			"NewReplicaSetCreated", // Created ReplicaSet bluegreen-7dcd8f8869 (revision 1) with size 3
+			"NewReplicaSetCreated", // Created ReplicaSet bluegreen-7dcd8f8869 (revision 1)
+			"ScalingReplicaSet",    // Scaled up ReplicaSet bluegreen-7dcd8f8869 (revision 1) from 0 to 3
 			"RolloutCompleted",     // Rollout completed update to revision 1 (7dcd8f8869): Initial deploy
 			"SwitchService",        // Switched selector for service 'bluegreen' from '' to '7dcd8f8869'
 			"RolloutUpdated",       // Rollout updated to revision 2
-			"NewReplicaSetCreated", // Created ReplicaSet bluegreen-5498785cd6 (revision 2) with size 3
+			"NewReplicaSetCreated", // Created ReplicaSet bluegreen-5498785cd6 (revision 2)
+			"ScalingReplicaSet",    // Scaled up ReplicaSet bluegreen-5498785cd6 (revision 2) from 0 to 3
 			"SwitchService",        // Switched selector for service 'bluegreen' from '7dcd8f8869' to '6c779b88b6'
 			"RolloutCompleted",     // Rollout completed update to revision 2 (6c779b88b6): Completed blue-green update
 		})

diff --git a/utils/conditions/conditions.go b/utils/conditions/conditions.go
@@ -52,7 +52,7 @@ const (
 	//NewReplicaSetMessage is added in a rollout when it creates a new replicas set.
 	NewReplicaSetMessage = "Created new replica set %q"
 	// NewReplicaSetDetailedMessage is a more detailed format message
-	NewReplicaSetDetailedMessage = "Created ReplicaSet %s (revision %d) with size %d"
+	NewReplicaSetDetailedMessage = "Created ReplicaSet %s (revision %d)"
 
 	// FoundNewRSReason is added in a rollout when it adopts an existing replica set.
 	FoundNewRSReason = "FoundNewReplicaSet"