fix: rollout experiment template changing reference rs template labels.

Fixes #1596 (#1597) * fix: rollout experiment template changing reference rs template labels Signed-off-by: Flavio Lemos <flavio.lemos@farfetch.com> * docs: Add Farfetch to USERS.md Signed-off-by: Flavio Lemos <flavio.lemos@farfetch.com>
argoproj · Nov 3, 2021 · 9d2ccb9 · 9d2ccb9
1 parent fe87bdd
commit 9d2ccb9
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 2 deletions.
diff --git a/USERS.md b/USERS.md
@@ -11,6 +11,7 @@ Organizations below are **officially** using Argo Rollouts. Please send a PR wit
 1. [Codefresh](https://codefresh.io/)
 1. [Databricks](https://github.com/databricks)
 1. [Devtron Labs](https://github.com/devtron-labs/devtron)
+1. [Farfetch](https://www.farfetch.com/)
 1. [Intuit](https://www.intuit.com/)
 1. [New Relic](https://newrelic.com/)
 1. [Nitro](https://www.gonitro.com)

diff --git a/rollout/experiment.go b/rollout/experiment.go
@@ -69,9 +69,9 @@ func GetExperimentFromTemplate(r *v1alpha1.Rollout, stableRS, newRS *appsv1.Repl
 		templateRS := &appsv1.ReplicaSet{}
 		switch templateStep.SpecRef {
 		case v1alpha1.CanarySpecRef:
-			templateRS = newRS
+			templateRS = newRS.DeepCopy()
 		case v1alpha1.StableSpecRef:
-			templateRS = stableRS
+			templateRS = stableRS.DeepCopy()
 		default:
 			return nil, fmt.Errorf("Invalid template step SpecRef: must be canary or stable")
 		}

diff --git a/rollout/experiment_test.go b/rollout/experiment_test.go
@@ -584,6 +584,41 @@ func TestGetExperimentFromTemplate(t *testing.T) {
 	assert.Nil(t, err)
 }
 
+func TestGetExperimentFromTemplateModifiedLabelsDoesntChangeRefReplicatSet(t *testing.T) {
+	steps := []v1alpha1.CanaryStep{{
+		Experiment: &v1alpha1.RolloutExperimentStep{
+			Templates: []v1alpha1.RolloutExperimentTemplate{{
+				Name:     "stable-template",
+				SpecRef:  v1alpha1.StableSpecRef,
+				Replicas: pointer.Int32Ptr(1),
+			}},
+		},
+	}}
+
+	r1 := newCanaryRollout("foo", 1, nil, steps, pointer.Int32Ptr(0), intstr.FromInt(0), intstr.FromInt(1))
+	r2 := bumpVersion(r1)
+	r2.Spec.Strategy.Canary.Steps[0].Experiment.Templates[0].Metadata.Annotations = map[string]string{"abc": "def"}
+	r2.Spec.Strategy.Canary.Steps[0].Experiment.Templates[0].Metadata.Labels = map[string]string{"123": "456"}
+
+	rs1 := newReplicaSetWithStatus(r1, 1, 1)
+	rs2 := newReplicaSetWithStatus(r2, 1, 1)
+	stableRsTemplate := rs1.Spec.Template.DeepCopy()
+	canaryRsTemplate := rs2.Spec.Template.DeepCopy()
+	rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]
+
+	r2.Status.CurrentStepIndex = pointer.Int32Ptr(0)
+	r2.Status.StableRS = rs1PodHash
+
+	_, err := GetExperimentFromTemplate(r2, rs1, rs2)
+	assert.Nil(t, err)
+	assert.Equal(t, stableRsTemplate, &rs1.Spec.Template)
+
+	r2.Spec.Strategy.Canary.Steps[0].Experiment.Templates[0].SpecRef = v1alpha1.CanarySpecRef
+	_, err = GetExperimentFromTemplate(r2, rs1, rs2)
+	assert.Nil(t, err)
+	assert.Equal(t, canaryRsTemplate, &rs2.Spec.Template)
+}
+
 func TestDeleteExperimentWithNoMatchingRS(t *testing.T) {
 	f := newFixture(t)
 	defer f.Close()