fix: undo referenced object for workloadRef rollout (argoproj#1275)

huikang · web-flow · commit a2de87520760 · 2021-06-24T12:03:12.000-07:00
Signed-off-by: Hui Kang &lt;hui.kang@salesforce.com&gt;
diff --git a/pkg/kubectl-argo-rollouts/cmd/undo/undo.go b/pkg/kubectl-argo-rollouts/cmd/undo/undo.go
@@ -17,6 +17,7 @@ import (
 
 	"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
 	"github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/options"
+	routils "github.com/argoproj/argo-rollouts/utils/unstructured"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
@@ -67,6 +68,8 @@ func NewCmdUndo(o *options.ArgoRolloutsOptions) *cobra.Command {
 // RunUndoRollout performs the execution of 'rollouts undo' sub command
 func RunUndoRollout(rolloutIf dynamic.ResourceInterface, c kubernetes.Interface, name string, toRevision int64) (string, error) {
 	ctx := context.TODO()
+	var err error
+
 	ro, err := rolloutIf.Get(ctx, name, metav1.GetOptions{})
 	if err != nil {
 		return "", err
@@ -93,13 +96,41 @@ func RunUndoRollout(rolloutIf dynamic.ResourceInterface, c kubernetes.Interface,
 		return "", fmt.Errorf("failed restoring revision %d: %v", toRevision, err)
 	}
 
-	// Restore revision
-	if _, err = rolloutIf.Patch(ctx, name, patchType, patch, metav1.PatchOptions{}); err != nil {
+	// Restore revision depending on whether workload ref is nil
+	rollout := routils.ObjectToRollout(ro)
+	if rollout == nil {
+		return "", fmt.Errorf("Invalid rollout object")
+	}
+	if rollout.Spec.WorkloadRef != nil {
+		err = undoWorkloadRef(c, rollout, patchType, patch)
+	} else {
+		_, err = rolloutIf.Patch(ctx, name, patchType, patch, metav1.PatchOptions{})
+	}
+	if err != nil {
 		return "", fmt.Errorf("failed restoring revision %d: %v", toRevision, err)
 	}
 	return fmt.Sprintf("rollout '%s' undo\n", ro.GetName()), nil
 }
 
+func undoWorkloadRef(c kubernetes.Interface, rollout *v1alpha1.Rollout, patchType types.PatchType, patch []byte) error {
+	var err error
+
+	refName := rollout.Spec.WorkloadRef.Name
+	namespace := rollout.GetNamespace()
+
+	switch rollout.Spec.WorkloadRef.Kind {
+	case "Deployment":
+		_, err = c.AppsV1().Deployments(namespace).Patch(context.TODO(), refName, patchType, patch, metav1.PatchOptions{})
+	case "ReplicaSet":
+		_, err = c.AppsV1().ReplicaSets(namespace).Patch(context.TODO(), refName, patchType, patch, metav1.PatchOptions{})
+	case "PodTemplate":
+		_, err = c.CoreV1().PodTemplates(namespace).Patch(context.TODO(), refName, patchType, patch, metav1.PatchOptions{})
+	default:
+		return fmt.Errorf("workload of type %s is not supported", rollout.Spec.WorkloadRef.Kind)
+	}
+	return err
+}
+
 func rolloutRevision(ro *unstructured.Unstructured, c kubernetes.Interface, toRevision int64) (*appsv1.ReplicaSet, error) {
 	allRSs, err := getAllReplicaSets(ro, c.AppsV1())
 	if err != nil {
diff --git a/pkg/kubectl-argo-rollouts/cmd/undo/undo_test.go b/pkg/kubectl-argo-rollouts/cmd/undo/undo_test.go
@@ -2,6 +2,7 @@ package undo
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"testing"
@@ -118,6 +119,51 @@ func TestUndoCmdToRevision(t *testing.T) {
 	assert.Empty(t, stderr)
 }
 
+func TestUndoCmdToRevisionOfWorkloadRef(t *testing.T) {
+
+	roTests := []struct {
+		idx     int
+		refName string
+		refType string
+	}{
+		{1, "canary-demo-65fb5ffc84", "ReplicaSet"},
+		{2, "canary-demo-deploy", "Deployment"},
+	}
+
+	for _, roTest := range roTests {
+		rolloutObjs := testdata.NewCanaryRollout()
+		ro := rolloutObjs.Rollouts[roTest.idx]
+		ro.Spec.Template = corev1.PodTemplateSpec{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "test",
+			},
+		}
+		tf, o := options.NewFakeArgoRolloutsOptions(rolloutObjs.AllObjects()...)
+		o.RESTClientGetter = tf.WithNamespace(ro.Namespace)
+		defer tf.Cleanup()
+		cmd := NewCmdUndo(o)
+		cmd.PersistentPreRunE = o.PersistentPreRunE
+		cmd.SetArgs([]string{ro.Name, "--to-revision=29"})
+
+		err := cmd.Execute()
+		assert.Nil(t, err)
+
+		// Verify the current RS has been patched by the oldRS's template
+		switch roTest.refType {
+		case "Deployment":
+			currentRs, _ := o.KubeClient.AppsV1().Deployments(ro.Namespace).Get(context.TODO(), "canary-demo-deploy", metav1.GetOptions{})
+			assert.Equal(t, "argoproj/rollouts-demo:asdf", currentRs.Spec.Template.Spec.Containers[0].Image)
+		case "ReplicaSet":
+			currentRs, _ := o.KubeClient.AppsV1().ReplicaSets(ro.Namespace).Get(context.TODO(), "canary-demo-65fb5ffc84", metav1.GetOptions{})
+			assert.Equal(t, "argoproj/rollouts-demo:asdf", currentRs.Spec.Template.Spec.Containers[0].Image)
+		}
+		stdout := o.Out.(*bytes.Buffer).String()
+		stderr := o.ErrOut.(*bytes.Buffer).String()
+		assert.Equal(t, fmt.Sprintf("rollout '%s' undo\n", ro.Name), stdout)
+		assert.Empty(t, stderr)
+	}
+}
+
 func TestUndoCmdToRevisionSkipCurrent(t *testing.T) {
 	rolloutObjs := testdata.NewCanaryRollout()
 	ro := rolloutObjs.Rollouts[0]
diff --git a/pkg/kubectl-argo-rollouts/info/testdata/canary/canary-ref-deploy.yaml b/pkg/kubectl-argo-rollouts/info/testdata/canary/canary-ref-deploy.yaml
@@ -0,0 +1,26 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: canary-demo-deploy
+  namespace: jesse-test
+spec:
+  selector:
+    matchLabels:
+      app: canary-demo-deploy
+  replicas: 0
+  template:
+    metadata:
+      labels:
+        app: canary-demo-deploy
+    spec:
+      containers:
+        - name: canary-demo-deploy
+          image: argoproj/rollouts-demo:red
+          ports:
+            - name: http
+              containerPort: 8080
+              protocol: TCP
+          resources:
+            requests:
+              memory: 32Mi
+              cpu: 5m
diff --git a/pkg/kubectl-argo-rollouts/info/testdata/canary/canary-rollout2.yaml b/pkg/kubectl-argo-rollouts/info/testdata/canary/canary-rollout2.yaml
@@ -0,0 +1,68 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Rollout
+metadata:
+  annotations:
+    rollout.argoproj.io/revision: "31"
+  creationTimestamp: "2019-10-25T06:07:18Z"
+  generation: 429
+  labels:
+    app: canary-demo
+    app.kubernetes.io/instance: jesse-test
+  name: canary-demo-workloadRef
+  namespace: jesse-test
+  resourceVersion: "28253567"
+  selfLink: /apis/argoproj.io/v1alpha1/namespaces/jesse-test/rollouts/canary-demo
+  uid: b350ba76-f6ed-11e9-a15b-42010aa80033
+spec:
+  progressDeadlineSeconds: 30
+  replicas: 5
+  revisionHistoryLimit: 3
+  selector:
+    matchLabels:
+      app: canary-demo
+  strategy:
+    canary:
+      canaryService: canary-demo-preview
+      steps:
+      - setWeight: 20
+      - pause: {}
+      - setWeight: 40
+      - pause:
+          duration: 10s
+      - setWeight: 60
+      - pause:
+          duration: 10s
+      - setWeight: 80
+      - pause:
+          duration: 10s
+  workloadRef:
+    apiVersion: apps/v1
+    kind: ReplicaSet
+    name: canary-demo-65fb5ffc84
+status:
+  HPAReplicas: 6
+  availableReplicas: 5
+  blueGreen: {}
+  canary: {}
+  stableRS: 877894d5b
+  conditions:
+  - lastTransitionTime: "2019-10-25T06:07:29Z"
+    lastUpdateTime: "2019-10-25T06:07:29Z"
+    message: Rollout has minimum availability
+    reason: AvailableReason
+    status: "True"
+    type: Available
+  - lastTransitionTime: "2019-10-28T04:52:55Z"
+    lastUpdateTime: "2019-10-28T04:52:55Z"
+    message: ReplicaSet "canary-demo-65fb5ffc84" has timed out progressing.
+    reason: ProgressDeadlineExceeded
+    status: "False"
+    type: Progressing
+  currentPodHash: 65fb5ffc84
+  currentStepHash: f64cdc9d
+  currentStepIndex: 0
+  observedGeneration: "429"
+  readyReplicas: 5
+  replicas: 6
+  selector: app=canary-demo
+  updatedReplicas: 1
diff --git a/pkg/kubectl-argo-rollouts/info/testdata/canary/canary-rollout3.yaml b/pkg/kubectl-argo-rollouts/info/testdata/canary/canary-rollout3.yaml
@@ -0,0 +1,68 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Rollout
+metadata:
+  annotations:
+    rollout.argoproj.io/revision: "31"
+  creationTimestamp: "2019-10-25T06:07:18Z"
+  generation: 429
+  labels:
+    app: canary-demo
+    app.kubernetes.io/instance: jesse-test
+  name: canary-demo-workloadRef-deploy
+  namespace: jesse-test
+  resourceVersion: "28253567"
+  selfLink: /apis/argoproj.io/v1alpha1/namespaces/jesse-test/rollouts/canary-demo
+  uid: b350ba76-f6ed-11e9-a15b-42010aa80033
+spec:
+  progressDeadlineSeconds: 30
+  replicas: 5
+  revisionHistoryLimit: 3
+  selector:
+    matchLabels:
+      app: canary-demo
+  strategy:
+    canary:
+      canaryService: canary-demo-preview
+      steps:
+      - setWeight: 20
+      - pause: {}
+      - setWeight: 40
+      - pause:
+          duration: 10s
+      - setWeight: 60
+      - pause:
+          duration: 10s
+      - setWeight: 80
+      - pause:
+          duration: 10s
+  workloadRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: canary-demo-deploy
+status:
+  HPAReplicas: 6
+  availableReplicas: 5
+  blueGreen: {}
+  canary: {}
+  stableRS: 877894d5b
+  conditions:
+  - lastTransitionTime: "2019-10-25T06:07:29Z"
+    lastUpdateTime: "2019-10-25T06:07:29Z"
+    message: Rollout has minimum availability
+    reason: AvailableReason
+    status: "True"
+    type: Available
+  - lastTransitionTime: "2019-10-28T04:52:55Z"
+    lastUpdateTime: "2019-10-28T04:52:55Z"
+    message: ReplicaSet "canary-demo-65fb5ffc84" has timed out progressing.
+    reason: ProgressDeadlineExceeded
+    status: "False"
+    type: Progressing
+  currentPodHash: 65fb5ffc84
+  currentStepHash: f64cdc9d
+  currentStepIndex: 0
+  observedGeneration: "429"
+  readyReplicas: 5
+  replicas: 6
+  selector: app=canary-demo
+  updatedReplicas: 1
diff --git a/pkg/kubectl-argo-rollouts/info/testdata/canary/canary-rs.yaml b/pkg/kubectl-argo-rollouts/info/testdata/canary/canary-rs.yaml
@@ -1,4 +1,4 @@
-apiVersion: extensions/v1beta1
+apiVersion: apps/v1
 kind: ReplicaSet
 metadata:
   annotations:
diff --git a/pkg/kubectl-argo-rollouts/info/testdata/canary/old-rs.yaml b/pkg/kubectl-argo-rollouts/info/testdata/canary/old-rs.yaml
@@ -1,4 +1,4 @@
-apiVersion: extensions/v1beta1
+apiVersion: apps/v1
 kind: ReplicaSet
 metadata:
   annotations:
diff --git a/pkg/kubectl-argo-rollouts/info/testdata/canary/stable-rs.yaml b/pkg/kubectl-argo-rollouts/info/testdata/canary/stable-rs.yaml
@@ -1,4 +1,4 @@
-apiVersion: extensions/v1beta1
+apiVersion: apps/v1
 kind: ReplicaSet
 metadata:
   annotations:
diff --git a/pkg/kubectl-argo-rollouts/info/testdata/testdata.go b/pkg/kubectl-argo-rollouts/info/testdata/testdata.go
@@ -29,6 +29,7 @@ type RolloutObjects struct {
 	Pods         []*corev1.Pod
 	Experiments  []*v1alpha1.Experiment
 	AnalysisRuns []*v1alpha1.AnalysisRun
+	Deployments  []*appsv1.Deployment
 }
 
 func (r *RolloutObjects) AllObjects() []k8sruntime.Object {
@@ -39,6 +40,9 @@ func (r *RolloutObjects) AllObjects() []k8sruntime.Object {
 	for _, o := range r.ReplicaSets {
 		objs = append(objs, o)
 	}
+	for _, o := range r.Deployments {
+		objs = append(objs, o)
+	}
 	for _, o := range r.Pods {
 		objs = append(objs, o)
 	}
@@ -107,6 +111,14 @@ func discoverObjects(path string) *RolloutObjects {
 			}
 			rs.CreationTimestamp = aWeekAgo
 			objs.ReplicaSets = append(objs.ReplicaSets, &rs)
+		case "Deployment":
+			var de appsv1.Deployment
+			err = yaml.UnmarshalStrict(yamlBytes, &de, yaml.DisallowUnknownFields)
+			if err != nil {
+				panic(err)
+			}
+			de.CreationTimestamp = aWeekAgo
+			objs.Deployments = append(objs.Deployments, &de)
 		case "Pod":
 			var pod corev1.Pod
 			err = yaml.UnmarshalStrict(yamlBytes, &pod, yaml.DisallowUnknownFields)

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-apiVersion: extensions/v1beta1`
	`1`	`+apiVersion: apps/v1`
`2`	`2`	`kind: ReplicaSet`
`3`	`3`	`metadata:`
`4`	`4`	`annotations:`