fix(experiments): Update replicaset on experiment retry. Fixes #4578

experiments/experiment.go

@@ -3,6 +3,7 @@ package experiments
 import (
 	"context"
 	"fmt"
+	"reflect"
 	"time"
 
 	"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
@@ -128,6 +129,23 @@ func (ec *experimentContext) reconcileTemplate(template v1alpha1.TemplateSpec) {
 
 	rs := ec.templateRSs[template.Name]
 
+	// If experiment is being retried, check if template RS pod template has changed and delete RS to recreate
+	if rs != nil && isExperimentRetried(ec.ex) {
+		expectedRS := newReplicaSetFromTemplate(ec.ex, template, experimentutil.GetCollisionCountForTemplate(ec.ex, template))
+		if !replicasetutil.PodTemplateEqualIgnoreHash(&rs.Spec.Template, &expectedRS.Spec.Template) {
+			logCtx.Infof("Template changed; deleting ReplicaSet '%s' to recreate", rs.Name)
+			if err := ec.deleteReplicaSet(rs); err != nil {
+				templateStatus.Status = v1alpha1.TemplateStatusError
+				templateStatus.Message = fmt.Sprintf("Failed to delete ReplicaSet '%s' for template '%s': %v", rs.Name, template.Name, err)
+				experimentutil.SetTemplateStatus(ec.newStatus, *templateStatus)
+			} else {
+				ec.templateRSs[template.Name] = nil
+				ec.enqueueExperimentAfter(ec.ex, time.Second)
+			}
+			return
+		}
+	}
+
 	// Create ReplicaSet if does not exist
 	if rs == nil {
 		ec.createReplicaSetForTemplate(template, templateStatus, logCtx, now)
@@ -236,6 +254,10 @@ func (ec *experimentContext) reconcileTemplate(template v1alpha1.TemplateSpec) {
 	experimentutil.SetTemplateStatus(ec.newStatus, *templateStatus)
 }
 
+func isExperimentRetried(ex *v1alpha1.Experiment) bool {
+	return reflect.DeepEqual(ex.Status, v1alpha1.ExperimentStatus{})
+}
+
 func (ec *experimentContext) addScaleDownDelayToTemplateRS(rs *appsv1.ReplicaSet, templateName string) {
 	rsIsUpdated, err := ec.addScaleDownDelay(rs)
 	if err != nil {

experiments/experiment_test.go

@@ -436,6 +436,89 @@ func TestFailReplicaSetCreation(t *testing.T) {
 	assert.Equal(t, newStatus.Phase, v1alpha1.AnalysisPhaseError)
 }
 
+func TestRetryDeleteReplicaSetErrorUpdatesStatus(t *testing.T) {
+	templates := generateTemplates("bar")
+	oldTemplate := templates[0]
+	newTemplate := oldTemplate.DeepCopy()
+	newTemplate.Template.Spec.Containers[0].Image = "bar-v2"
+
+	ex := newExperiment("foo", []v1alpha1.TemplateSpec{*newTemplate}, "")
+	ex.Status = v1alpha1.ExperimentStatus{}
+
+	oldRS := templateToRS(ex, oldTemplate, 1)
+	exCtx := newTestContext(ex, oldRS)
+	exCtx.templateRSs[oldTemplate.Name] = oldRS
+
+	fakeClient := exCtx.kubeclientset.(*k8sfake.Clientset)
+	fakeClient.PrependReactor("delete", "replicasets", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
+		return true, nil, errors.New("intentional delete error")
+	})
+
+	newStatus := exCtx.reconcile()
+	if assert.Len(t, newStatus.TemplateStatuses, 1) {
+		assert.Equal(t, v1alpha1.TemplateStatusError, newStatus.TemplateStatuses[0].Status)
+		assert.Contains(t, newStatus.TemplateStatuses[0].Message, "Failed to delete ReplicaSet")
+	}
+}
+
+func TestDeleteReplicaSetIgnoresNotFound(t *testing.T) {
+	templates := generateTemplates("bar")
+	ex := newExperiment("foo", templates, "")
+	exCtx := newTestContext(ex)
+
+	rs := templateToRS(ex, templates[0], 0)
+	fakeClient := exCtx.kubeclientset.(*k8sfake.Clientset)
+	fakeClient.PrependReactor("delete", "replicasets", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
+		return true, nil, k8serrors.NewNotFound(schema.GroupResource{Resource: "replicasets"}, rs.Name)
+	})
+
+	err := exCtx.deleteReplicaSet(rs)
+	assert.NoError(t, err)
+}
+
+func TestRetryRecreatesReplicaSetOnTemplateDiff(t *testing.T) {
+	templates := generateTemplates("bar")
+	oldTemplate := templates[0]
+	newTemplate := oldTemplate.DeepCopy()
+	newTemplate.Template.Spec.Containers[0].Image = "bar-v2"
+
+	ex := newExperiment("foo", []v1alpha1.TemplateSpec{*newTemplate}, "")
+	ex.Status = v1alpha1.ExperimentStatus{}
+
+	oldRS := templateToRS(ex, oldTemplate, 1)
+	exCtx := newTestContext(ex, oldRS)
+	exCtx.templateRSs[oldTemplate.Name] = oldRS
+
+	fakeClient := exCtx.kubeclientset.(*k8sfake.Clientset)
+	exCtx.reconcile()
+
+	deleted := false
+	for _, action := range fakeClient.Actions() {
+		if action.Matches("delete", "replicasets") {
+			deleted = true
+			break
+		}
+	}
+	assert.True(t, deleted)
+
+	exCtx.reconcile()
+	var createdRS *appsv1.ReplicaSet
+	for _, action := range fakeClient.Actions() {
+		if action.Matches("create", "replicasets") {
+			createAction, ok := action.(kubetesting.CreateAction)
+			assert.True(t, ok)
+			obj := createAction.GetObject()
+			rs, ok := obj.(*appsv1.ReplicaSet)
+			assert.True(t, ok)
+			createdRS = rs
+			break
+		}
+	}
+	if assert.NotNil(t, createdRS) {
+		assert.Equal(t, "bar-v2", createdRS.Spec.Template.Spec.Containers[0].Image)
+	}
+}
+
 func TestFailServiceCreation(t *testing.T) {
 	templates := generateTemplates("bad")
 	setExperimentService(&templates[0])

experiments/replicaset.go

@@ -303,6 +303,15 @@ func (ec *experimentContext) scaleReplicaSet(rs *appsv1.ReplicaSet, newScale int
 	return scaled, rs, err
 }
 
+func (ec *experimentContext) deleteReplicaSet(rs *appsv1.ReplicaSet) error {
+	ctx := context.TODO()
+	err := ec.kubeclientset.AppsV1().ReplicaSets(rs.Namespace).Delete(ctx, rs.Name, metav1.DeleteOptions{})
+	if err != nil && !k8serrors.IsNotFound(err) {
+		return err
+	}
+	return nil
+}
+
 func newReplicaSetAnnotations(experimentName, templateName string) map[string]string {
 	return map[string]string{
 		v1alpha1.ExperimentNameAnnotationKey:         experimentName,