From d7df29ffe975157a7c827aa1bbcc3f6c994fb1ea Mon Sep 17 00:00:00 2001 From: Hui Kang Date: Sun, 13 Jun 2021 12:18:11 -0400 Subject: [PATCH] fix: undo referenced object for workloadRef rollout - unit test is added Signed-off-by: Hui Kang --- pkg/kubectl-argo-rollouts/cmd/undo/undo.go | 35 +++++++++- .../cmd/undo/undo_test.go | 46 +++++++++++++ .../testdata/canary/canary-ref-deploy.yaml | 26 +++++++ .../info/testdata/canary/canary-rollout2.yaml | 68 +++++++++++++++++++ .../info/testdata/canary/canary-rollout3.yaml | 68 +++++++++++++++++++ .../info/testdata/canary/canary-rs.yaml | 2 +- .../info/testdata/canary/old-rs.yaml | 2 +- .../info/testdata/canary/stable-rs.yaml | 2 +- .../info/testdata/testdata.go | 12 ++++ 9 files changed, 256 insertions(+), 5 deletions(-) create mode 100644 pkg/kubectl-argo-rollouts/info/testdata/canary/canary-ref-deploy.yaml create mode 100644 pkg/kubectl-argo-rollouts/info/testdata/canary/canary-rollout2.yaml create mode 100644 pkg/kubectl-argo-rollouts/info/testdata/canary/canary-rollout3.yaml diff --git a/pkg/kubectl-argo-rollouts/cmd/undo/undo.go b/pkg/kubectl-argo-rollouts/cmd/undo/undo.go index 8b496a97cc..58bfe11b7c 100644 --- 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 index d3be3b9f7b..da9099d5de 100644 --- 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 new file mode 100644 index 0000000000..1d1b5f82fb --- /dev/null +++ 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 new file mode 100644 index 0000000000..46fee7fd71 --- /dev/null +++ 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 new file mode 100644 index 0000000000..57eda1077f --- /dev/null +++ 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 index 3476c982f6..39321d5d95 100644 --- 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 index 7fa649cf32..4aaa41912a 100644 --- 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 index f39f9198ed..8dae5cf0c7 100644 --- 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 index 228a4c395b..8ac7e5622f 100644 --- 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)