fix: updates to replicas and pod template at the same time causes rol…

…lout to get stuck (#3272) * fix: fix the rollout stuck when pod/replicas changed together or canary strategy. Signed-off-by: Liming Liu <[email protected]> * add one unit test case for empty canary service. Signed-off-by: Liming Liu <[email protected]> * use rollout status to get the replicaset hash instead of service Signed-off-by: Zach Aller <[email protected]> --------- Signed-off-by: Liming Liu <[email protected]> Signed-off-by: Zach Aller <[email protected]> Co-authored-by: Liming Liu <[email protected]>
argoproj · Jan 2, 2024 · 192009a · 192009a
1 parent ede86a8
commit 192009a
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 0 deletions.
diff --git a/rollout/canary.go b/rollout/canary.go
@@ -435,6 +435,13 @@ func (c *rolloutContext) reconcileCanaryReplicaSets() (bool, error) {
 		return true, nil
 	}
 
+	// 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

diff --git a/rollout/canary_test.go b/rollout/canary_test.go
@@ -1,6 +1,7 @@
 package rollout
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"strconv"
@@ -2009,3 +2010,46 @@ func TestIsDynamicallyRollingBackToStable(t *testing.T) {
 		})
 	}
 }
+
+func TestCanaryReplicaAndSpecChangedTogether(t *testing.T) {
+	f := newFixture(t)
+	defer f.Close()
+
+	originReplicas := 3
+	r1 := newCanaryRollout("foo", originReplicas, nil, nil, nil, intstr.FromInt(1), intstr.FromInt(0))
+	canarySVCName := "canary"
+	stableSVCName := "stable"
+	r1.Spec.Strategy.Canary.CanaryService = canarySVCName
+	r1.Spec.Strategy.Canary.StableService = stableSVCName
+
+	stableRS := newReplicaSetWithStatus(r1, originReplicas, originReplicas)
+	stableSVC := newService(stableSVCName, 80,
+		map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: stableRS.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]}, r1)
+
+	r2 := bumpVersion(r1)
+	canaryRS := newReplicaSetWithStatus(r2, originReplicas, originReplicas)
+	canarySVC := newService(canarySVCName, 80,
+		map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: canaryRS.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]}, r2)
+
+	f.replicaSetLister = append(f.replicaSetLister, canaryRS, stableRS)
+	f.serviceLister = append(f.serviceLister, canarySVC, stableSVC)
+
+	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)
+	f.objects = append(f.objects, r3)
+
+	ctrl, _, _ := f.newController(noResyncPeriodFunc)
+	roCtx, err := ctrl.newRolloutContext(r3)
+	assert.NoError(t, err)
+	err = roCtx.reconcile()
+	assert.NoError(t, err)
+	updated, err := f.kubeclient.AppsV1().ReplicaSets(r3.Namespace).Get(context.Background(), canaryRS.Name, metav1.GetOptions{})
+	assert.NoError(t, err)
+	// check the canary one is updated
+	assert.NotEqual(t, originReplicas, int(*updated.Spec.Replicas))
+}