diff --git a/manifests/crds/rollout-crd.yaml b/manifests/crds/rollout-crd.yaml index ae0fdea244..7a4a52b30b 100644 --- a/manifests/crds/rollout-crd.yaml +++ b/manifests/crds/rollout-crd.yaml @@ -2623,6 +2623,8 @@ spec: HPAReplicas: format: int32 type: integer + abort: + type: boolean availableReplicas: format: int32 type: integer diff --git a/manifests/install.yaml b/manifests/install.yaml index f77da00497..f6cca5af95 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -10350,6 +10350,8 @@ spec: HPAReplicas: format: int32 type: integer + abort: + type: boolean availableReplicas: format: int32 type: integer diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index 45d58c4fc0..b1919e6c4b 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -10350,6 +10350,8 @@ spec: HPAReplicas: format: int32 type: integer + abort: + type: boolean availableReplicas: format: int32 type: integer diff --git a/pkg/apis/rollouts/v1alpha1/openapi_generated.go b/pkg/apis/rollouts/v1alpha1/openapi_generated.go index d3dc91e322..865acb1c15 100644 --- a/pkg/apis/rollouts/v1alpha1/openapi_generated.go +++ b/pkg/apis/rollouts/v1alpha1/openapi_generated.go @@ -1808,6 +1808,13 @@ func schema_pkg_apis_rollouts_v1alpha1_RolloutStatus(ref common.ReferenceCallbac SchemaProps: spec.SchemaProps{ Description: "RolloutStatus is the status for a Rollout resource", Properties: map[string]spec.Schema{ + "abort": { + SchemaProps: spec.SchemaProps{ + Description: "Abort cancel the current rollout progression", + Type: []string{"boolean"}, + Format: "", + }, + }, "pauseConditions": { SchemaProps: spec.SchemaProps{ Description: "PauseConditions indicates why the rollout is currently paused", diff --git a/pkg/apis/rollouts/v1alpha1/types.go b/pkg/apis/rollouts/v1alpha1/types.go index ed6c5b8978..2ff4f15812 100644 --- a/pkg/apis/rollouts/v1alpha1/types.go +++ b/pkg/apis/rollouts/v1alpha1/types.go @@ -274,6 +274,8 @@ type PauseCondition struct { // RolloutStatus is the status for a Rollout resource type RolloutStatus struct { + // Abort cancel the current rollout progression + Abort bool `json:"abort,omitempty"` // PauseConditions indicates why the rollout is currently paused PauseConditions []PauseCondition `json:"pauseConditions,omitempty"` //ControllerPause indicates the controller has paused the rollout diff --git a/pkg/kubectl-argo-rollouts/cmd/abort/abort.go b/pkg/kubectl-argo-rollouts/cmd/abort/abort.go new file mode 100644 index 0000000000..cb00a6e28e --- /dev/null +++ b/pkg/kubectl-argo-rollouts/cmd/abort/abort.go @@ -0,0 +1,48 @@ +package abort + +import ( + "fmt" + + "github.com/spf13/cobra" + types "k8s.io/apimachinery/pkg/types" + + "github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/options" +) + +const ( + example = ` + # Abort a rollout + %[1]s abort guestbook +` +) + +const ( + abortPatch = `{"status":{"abort":true}}` +) + +// NewCmdAbort returns a new instance of an `rollouts abort` command +func NewCmdAbort(o *options.ArgoRolloutsOptions) *cobra.Command { + var cmd = &cobra.Command{ + Use: "abort ROLLOUT", + Short: "Abort a rollout", + Example: o.Example(example), + SilenceUsage: true, + RunE: func(c *cobra.Command, args []string) error { + if len(args) == 0 { + return o.UsageErr(c) + } + ns := o.Namespace() + rolloutIf := o.RolloutsClientset().ArgoprojV1alpha1().Rollouts(ns) + for _, name := range args { + ro, err := rolloutIf.Patch(name, types.MergePatchType, []byte(abortPatch)) + if err != nil { + return err + } + fmt.Fprintf(o.Out, "rollout '%s' aborted\n", ro.Name) + } + return nil + }, + } + o.AddKubectlFlags(cmd) + return cmd +} diff --git a/pkg/kubectl-argo-rollouts/cmd/abort/abort_test.go b/pkg/kubectl-argo-rollouts/cmd/abort/abort_test.go new file mode 100644 index 0000000000..0bfb98f7a5 --- /dev/null +++ b/pkg/kubectl-argo-rollouts/cmd/abort/abort_test.go @@ -0,0 +1,78 @@ +package abort + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + kubetesting "k8s.io/client-go/testing" + + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" + fakeroclient "github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/fake" + options "github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/options/fake" +) + +func TestAbortCmdUsage(t *testing.T) { + tf, o := options.NewFakeArgoRolloutsOptions() + defer tf.Cleanup() + cmd := NewCmdAbort(o) + cmd.PersistentPreRunE = o.PersistentPreRunE + cmd.SetArgs([]string{}) + err := cmd.Execute() + assert.Error(t, err) + stdout := o.Out.(*bytes.Buffer).String() + stderr := o.ErrOut.(*bytes.Buffer).String() + assert.Empty(t, stdout) + assert.Contains(t, stderr, "Usage:") + assert.Contains(t, stderr, "abort ROLLOUT") +} + +func TestAbortCmd(t *testing.T) { + ro := v1alpha1.Rollout{ + ObjectMeta: metav1.ObjectMeta{ + Name: "guestbook", + Namespace: "test", + }, + } + + tf, o := options.NewFakeArgoRolloutsOptions(&ro) + defer tf.Cleanup() + fakeClient := o.RolloutsClient.(*fakeroclient.Clientset) + fakeClient.ReactionChain = nil + fakeClient.AddReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { + if patchAction, ok := action.(kubetesting.PatchAction); ok { + if string(patchAction.GetPatch()) == abortPatch { + ro.Status.Abort = true + } + } + return true, &ro, nil + }) + + cmd := NewCmdAbort(o) + cmd.PersistentPreRunE = o.PersistentPreRunE + cmd.SetArgs([]string{"guestbook", "-n", "test"}) + err := cmd.Execute() + assert.Nil(t, err) + + assert.True(t, ro.Status.Abort) + stdout := o.Out.(*bytes.Buffer).String() + stderr := o.ErrOut.(*bytes.Buffer).String() + assert.Equal(t, stdout, "rollout 'guestbook' aborted\n") + assert.Empty(t, stderr) +} + +func TestAbortCmdError(t *testing.T) { + tf, o := options.NewFakeArgoRolloutsOptions(&v1alpha1.Rollout{}) + defer tf.Cleanup() + cmd := NewCmdAbort(o) + cmd.PersistentPreRunE = o.PersistentPreRunE + cmd.SetArgs([]string{"doesnotexist", "-n", "test"}) + err := cmd.Execute() + assert.Error(t, err) + stdout := o.Out.(*bytes.Buffer).String() + stderr := o.ErrOut.(*bytes.Buffer).String() + assert.Empty(t, stdout) + assert.Equal(t, "Error: rollouts.argoproj.io \"doesnotexist\" not found\n", stderr) +} diff --git a/pkg/kubectl-argo-rollouts/cmd/cmd.go b/pkg/kubectl-argo-rollouts/cmd/cmd.go index 7a4c9f36dd..b8f9ebdcf8 100644 --- a/pkg/kubectl-argo-rollouts/cmd/cmd.go +++ b/pkg/kubectl-argo-rollouts/cmd/cmd.go @@ -3,9 +3,11 @@ package cmd import ( "github.com/spf13/cobra" + "github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/cmd/abort" "github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/cmd/list" "github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/cmd/pause" "github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/cmd/resume" + "github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/cmd/retry" "github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/cmd/version" "github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/options" ) @@ -35,5 +37,7 @@ func NewCmdArgoRollouts(o *options.ArgoRolloutsOptions) *cobra.Command { cmd.AddCommand(pause.NewCmdPause(o)) cmd.AddCommand(resume.NewCmdResume(o)) cmd.AddCommand(version.NewCmdVersion(o)) + cmd.AddCommand(abort.NewCmdAbort(o)) + cmd.AddCommand(retry.NewCmdRetry(o)) return cmd } diff --git a/pkg/kubectl-argo-rollouts/cmd/retry/retry.go b/pkg/kubectl-argo-rollouts/cmd/retry/retry.go new file mode 100644 index 0000000000..38043a5256 --- /dev/null +++ b/pkg/kubectl-argo-rollouts/cmd/retry/retry.go @@ -0,0 +1,48 @@ +package retry + +import ( + "fmt" + + "github.com/spf13/cobra" + types "k8s.io/apimachinery/pkg/types" + + "github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/options" +) + +const ( + example = ` + # Retry a rollout + %[1]s retry guestbook +` +) + +const ( + retryPatch = `{"status":{"abort":false}}` +) + +// NewCmdRetry returns a new instance of an `rollouts retry` command +func NewCmdRetry(o *options.ArgoRolloutsOptions) *cobra.Command { + var cmd = &cobra.Command{ + Use: "retry ROLLOUT", + Short: "Retry a rollout", + Example: o.Example(example), + SilenceUsage: true, + RunE: func(c *cobra.Command, args []string) error { + if len(args) == 0 { + return o.UsageErr(c) + } + ns := o.Namespace() + rolloutIf := o.RolloutsClientset().ArgoprojV1alpha1().Rollouts(ns) + for _, name := range args { + ro, err := rolloutIf.Patch(name, types.MergePatchType, []byte(retryPatch)) + if err != nil { + return err + } + fmt.Fprintf(o.Out, "rollout '%s' retried\n", ro.Name) + } + return nil + }, + } + o.AddKubectlFlags(cmd) + return cmd +} diff --git a/pkg/kubectl-argo-rollouts/cmd/retry/retry_test.go b/pkg/kubectl-argo-rollouts/cmd/retry/retry_test.go new file mode 100644 index 0000000000..9a2bc0dde5 --- /dev/null +++ b/pkg/kubectl-argo-rollouts/cmd/retry/retry_test.go @@ -0,0 +1,78 @@ +package retry + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + kubetesting "k8s.io/client-go/testing" + + "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" + fakeroclient "github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/fake" + options "github.com/argoproj/argo-rollouts/pkg/kubectl-argo-rollouts/options/fake" +) + +func TestRetryCmdUsage(t *testing.T) { + tf, o := options.NewFakeArgoRolloutsOptions() + defer tf.Cleanup() + cmd := NewCmdRetry(o) + cmd.PersistentPreRunE = o.PersistentPreRunE + cmd.SetArgs([]string{}) + err := cmd.Execute() + assert.Error(t, err) + stdout := o.Out.(*bytes.Buffer).String() + stderr := o.ErrOut.(*bytes.Buffer).String() + assert.Empty(t, stdout) + assert.Contains(t, stderr, "Usage:") + assert.Contains(t, stderr, "retry ROLLOUT") +} + +func TestRetryCmd(t *testing.T) { + ro := v1alpha1.Rollout{ + ObjectMeta: metav1.ObjectMeta{ + Name: "guestbook", + Namespace: "test", + }, + } + + tf, o := options.NewFakeArgoRolloutsOptions(&ro) + defer tf.Cleanup() + fakeClient := o.RolloutsClient.(*fakeroclient.Clientset) + fakeClient.ReactionChain = nil + fakeClient.AddReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { + if patchAction, ok := action.(kubetesting.PatchAction); ok { + if string(patchAction.GetPatch()) == retryPatch { + ro.Status.Abort = true + } + } + return true, &ro, nil + }) + + cmd := NewCmdRetry(o) + cmd.PersistentPreRunE = o.PersistentPreRunE + cmd.SetArgs([]string{"guestbook", "-n", "test"}) + err := cmd.Execute() + assert.Nil(t, err) + + assert.True(t, ro.Status.Abort) + stdout := o.Out.(*bytes.Buffer).String() + stderr := o.ErrOut.(*bytes.Buffer).String() + assert.Equal(t, stdout, "rollout 'guestbook' retried\n") + assert.Empty(t, stderr) +} + +func TestRetryCmdError(t *testing.T) { + tf, o := options.NewFakeArgoRolloutsOptions(&v1alpha1.Rollout{}) + defer tf.Cleanup() + cmd := NewCmdRetry(o) + cmd.PersistentPreRunE = o.PersistentPreRunE + cmd.SetArgs([]string{"doesnotexist", "-n", "test"}) + err := cmd.Execute() + assert.Error(t, err) + stdout := o.Out.(*bytes.Buffer).String() + stderr := o.ErrOut.(*bytes.Buffer).String() + assert.Empty(t, stdout) + assert.Equal(t, "Error: rollouts.argoproj.io \"doesnotexist\" not found\n", stderr) +} diff --git a/rollout/analysis.go b/rollout/analysis.go index 164068eec3..1815251cc3 100644 --- a/rollout/analysis.go +++ b/rollout/analysis.go @@ -42,11 +42,12 @@ func (c *RolloutController) getAnalysisRunsForRollout(rollout *v1alpha1.Rollout) } func (c *RolloutController) reconcileAnalysisRuns(roCtx *canaryContext) error { - rollout := roCtx.Rollout() otherArs := roCtx.OtherAnalysisRuns() - if len(rollout.Status.PauseConditions) > 0 { - return nil + if roCtx.PauseContext().IsAborted() { + allArs := append(roCtx.CurrentAnalysisRuns(), otherArs...) + return c.cancelAnalysisRuns(roCtx, allArs) } + newCurrentAnalysisRuns := []*v1alpha1.AnalysisRun{} stepAnalysisRun, err := c.reconcileStepBasedAnalysisRun(roCtx) @@ -64,6 +65,9 @@ func (c *RolloutController) reconcileAnalysisRuns(roCtx *canaryContext) error { if backgroundAnalysisRun != nil { newCurrentAnalysisRuns = append(newCurrentAnalysisRuns, backgroundAnalysisRun) } + if roCtx.PauseContext().HasAddPause() { + return nil + } err = c.cancelAnalysisRuns(roCtx, otherArs) if err != nil { @@ -102,6 +106,14 @@ func (c *RolloutController) reconcileBackgroundAnalysisRun(roCtx *canaryContext) } return currentAr, err } + if currentAr.Status != nil { + switch currentAr.Status.Status { + case v1alpha1.AnalysisStatusInconclusive: + roCtx.PauseContext().AddPauseCondition(v1alpha1.PauseReasonInconclusiveAnalysis) + case v1alpha1.AnalysisStatusError, v1alpha1.AnalysisStatusFailed: + roCtx.PauseContext().AddAbort() + } + } return currentAr, nil } @@ -147,8 +159,13 @@ func (c *RolloutController) reconcileStepBasedAnalysisRun(roCtx *canaryContext) return currentAr, err } - if currentAr.Status != nil && currentAr.Status.Status == v1alpha1.AnalysisStatusInconclusive { - roCtx.PauseContext().AddPauseCondition(v1alpha1.PauseReasonInconclusiveAnalysis) + if currentAr.Status != nil { + switch currentAr.Status.Status { + case v1alpha1.AnalysisStatusInconclusive: + roCtx.PauseContext().AddPauseCondition(v1alpha1.PauseReasonInconclusiveAnalysis) + case v1alpha1.AnalysisStatusError, v1alpha1.AnalysisStatusFailed: + roCtx.PauseContext().AddAbort() + } } return currentAr, nil diff --git a/rollout/analysis_test.go b/rollout/analysis_test.go index 96a6a63740..a9985611ff 100644 --- a/rollout/analysis_test.go +++ b/rollout/analysis_test.go @@ -505,6 +505,63 @@ func TestIncrementStepAfterSuccessfulAnalysisRun(t *testing.T) { assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition)), patch) } +func TestPausedOnInconclusiveBackgroundAnalysisRun(t *testing.T) { + f := newFixture(t) + defer f.Close() + + at := analysisTemplate("bar") + steps := []v1alpha1.CanaryStep{ + {SetWeight: pointer.Int32Ptr(10)}, + {SetWeight: pointer.Int32Ptr(20)}, + {SetWeight: pointer.Int32Ptr(30)}, + } + + r1 := newCanaryRollout("foo", 1, nil, steps, pointer.Int32Ptr(0), intstr.FromInt(0), intstr.FromInt(1)) + r2 := bumpVersion(r1) + ar := analysisRun(at, v1alpha1.RolloutTypeBackgroundRunLabel, r2) + r2.Spec.Strategy.Canary.Analysis = &v1alpha1.RolloutAnalysisStep{ + TemplateName: at.Name, + } + ar.Status = &v1alpha1.AnalysisRunStatus{ + Status: v1alpha1.AnalysisStatusInconclusive, + } + + rs1 := newReplicaSetWithStatus(r1, 1, 1) + rs2 := newReplicaSetWithStatus(r2, 0, 0) + f.kubeobjects = append(f.kubeobjects, rs1, rs2) + f.replicaSetLister = append(f.replicaSetLister, rs1, rs2) + rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] + + r2 = updateCanaryRolloutStatus(r2, rs1PodHash, 1, 0, 1, false) + r2.Status.Canary.CurrentBackgroundAnalysisRun = ar.Name + + f.rolloutLister = append(f.rolloutLister, r2) + f.analysisTemplateLister = append(f.analysisTemplateLister, at) + f.analysisRunLister = append(f.analysisRunLister, ar) + f.objects = append(f.objects, r2, at, ar) + + patchIndex := f.expectPatchRolloutAction(r2) + f.run(getKey(r2, t)) + patch := f.getPatchedRollout(patchIndex) + now := metav1.Now().UTC().Format(time.RFC3339) + expectedPatch := `{ + "status": { + "conditions": %s, + "canary": { + "currentBackgroundAnalysisRun": null + }, + "pauseConditions": [{ + "reason": "%s", + "startTime": "%s" + }], + "controllerPause": true + } + }` + condition := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, r2, false) + + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, v1alpha1.PauseReasonInconclusiveAnalysis, now)), patch) +} + func TestPausedStepAfterInconclusiveAnalysisRun(t *testing.T) { f := newFixture(t) defer f.Close() @@ -559,7 +616,7 @@ func TestPausedStepAfterInconclusiveAnalysisRun(t *testing.T) { assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition, v1alpha1.PauseReasonInconclusiveAnalysis, now)), patch) } -func TestErrorConditionAfterErrorAnalysisRun(t *testing.T) { +func TestErrorConditionAfterErrorAnalysisRunStep(t *testing.T) { f := newFixture(t) defer f.Close() @@ -599,10 +656,121 @@ func TestErrorConditionAfterErrorAnalysisRun(t *testing.T) { patch := f.getPatchedRollout(patchIndex) expectedPatch := `{ "status": { - "conditions": %s + "canary":{ + "currentStepAnalysisRun": null + }, + "conditions": %s, + "abort": true + } + }` + condition := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false) + + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition)), patch) +} + +func TestErrorConditionAfterErrorAnalysisRunBackground(t *testing.T) { + f := newFixture(t) + defer f.Close() + + at := analysisTemplate("bar") + steps := []v1alpha1.CanaryStep{ + {SetWeight: pointer.Int32Ptr(10)}, + {SetWeight: pointer.Int32Ptr(20)}, + {SetWeight: pointer.Int32Ptr(40)}, + } + + r1 := newCanaryRollout("foo", 10, nil, steps, pointer.Int32Ptr(0), intstr.FromInt(0), intstr.FromInt(1)) + r2 := bumpVersion(r1) + r2.Spec.Strategy.Canary.Analysis = &v1alpha1.RolloutAnalysisStep{ + TemplateName: at.Name, + } + ar := analysisRun(at, v1alpha1.RolloutTypeBackgroundRunLabel, r2) + ar.Status = &v1alpha1.AnalysisRunStatus{ + Status: v1alpha1.AnalysisStatusError, + MetricResults: []v1alpha1.MetricResult{{ + Status: v1alpha1.AnalysisStatusError, + }}, + } + + rs1 := newReplicaSetWithStatus(r1, 9, 9) + rs2 := newReplicaSetWithStatus(r2, 1, 1) + f.kubeobjects = append(f.kubeobjects, rs1, rs2) + f.replicaSetLister = append(f.replicaSetLister, rs1, rs2) + rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] + + r2 = updateCanaryRolloutStatus(r2, rs1PodHash, 10, 1, 10, false) + r2.Status.Canary.CurrentBackgroundAnalysisRun = ar.Name + + f.rolloutLister = append(f.rolloutLister, r2) + f.analysisTemplateLister = append(f.analysisTemplateLister, at) + f.analysisRunLister = append(f.analysisRunLister, ar) + f.objects = append(f.objects, r2, at, ar) + + patchIndex := f.expectPatchRolloutAction(r2) + f.run(getKey(r2, t)) + patch := f.getPatchedRollout(patchIndex) + expectedPatch := `{ + "status": { + "canary":{ + "currentBackgroundAnalysisRun": null + }, + "conditions": %s, + "abort": true } }` - condition := generateConditionsPatch(true, conditions.RolloutAnalysisRunFailedReason, r2, false) + condition := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false) assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, condition)), patch) } + +func TestCancelAnalysisRunsWhenAborted(t *testing.T) { + f := newFixture(t) + defer f.Close() + + at := analysisTemplate("bar") + steps := []v1alpha1.CanaryStep{{ + Analysis: &v1alpha1.RolloutAnalysisStep{ + TemplateName: at.Name, + }, + }} + + r1 := newCanaryRollout("foo", 1, nil, steps, pointer.Int32Ptr(0), intstr.FromInt(0), intstr.FromInt(1)) + r2 := bumpVersion(r1) + ar := analysisRun(at, v1alpha1.RolloutTypeStepLabel, r2) + olderAr := ar.DeepCopy() + olderAr.Name = "older-analysis-run" + + rs1 := newReplicaSetWithStatus(r1, 1, 1) + rs2 := newReplicaSetWithStatus(r2, 0, 0) + f.kubeobjects = append(f.kubeobjects, rs1, rs2) + f.replicaSetLister = append(f.replicaSetLister, rs1, rs2) + rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] + + r2 = updateCanaryRolloutStatus(r2, rs1PodHash, 1, 0, 1, false) + r2.Status.Abort = true + r2.Status.Canary.CurrentStepAnalysisRun = ar.Name + + f.rolloutLister = append(f.rolloutLister, r2) + f.analysisTemplateLister = append(f.analysisTemplateLister, at) + f.analysisRunLister = append(f.analysisRunLister, ar, olderAr) + f.objects = append(f.objects, r2, at, ar, olderAr) + + cancelCurrentAr := f.expectPatchAnalysisRunAction(ar) + cancelOldAr := f.expectPatchAnalysisRunAction(olderAr) + patchIndex := f.expectPatchRolloutAction(r2) + f.run(getKey(r2, t)) + + assert.True(t, f.verifyPatchedAnalysisRun(cancelOldAr, olderAr)) + assert.True(t, f.verifyPatchedAnalysisRun(cancelCurrentAr, ar)) + patch := f.getPatchedRollout(patchIndex) + newConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false) + expectedPatch := `{ + "status": { + "canary": { + "currentStepAnalysisRun":null + }, + "conditions": %s + } + }` + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, newConditions)), patch) +} diff --git a/rollout/canary.go b/rollout/canary.go index 4ecc9b02f6..05eb6f6ef5 100644 --- a/rollout/canary.go +++ b/rollout/canary.go @@ -64,7 +64,12 @@ func (c *RolloutController) rolloutCanary(rollout *v1alpha1.Rollout, rsList []*a } logCtx.Info("Reconciling AnalysisRun step") - if err = c.reconcileAnalysisRuns(roCtx); err != nil { + err = c.reconcileAnalysisRuns(roCtx) + if roCtx.PauseContext().HasAddPause() { + logCtx.Info("Detected pause due to inconclusive AnalysisRun") + return c.syncRolloutStatusCanary(roCtx) + } + if err != nil { return err } @@ -271,6 +276,7 @@ func (c *RolloutController) syncRolloutStatusCanary(roCtx *canaryContext) error } } roCtx.PauseContext().ClearPauseConditions() + roCtx.PauseContext().RemoveAbort() newStatus = c.calculateRolloutConditions(roCtx, newStatus) return c.persistRolloutStatus(roCtx, &newStatus) } @@ -287,22 +293,40 @@ func (c *RolloutController) syncRolloutStatusCanary(roCtx *canaryContext) error } roCtx.PauseContext().ClearPauseConditions() + roCtx.PauseContext().RemoveAbort() newStatus = c.calculateRolloutConditions(roCtx, newStatus) return c.persistRolloutStatus(roCtx, &newStatus) } currBackgroundAr := analysisutil.GetCurrentBackgroundAnalysisRun(currArs) - if currBackgroundAr != nil { - if currBackgroundAr.Status == nil || !currBackgroundAr.Status.Status.Completed() || analysisutil.IsTerminating(currBackgroundAr) { + if currBackgroundAr != nil && !roCtx.PauseContext().IsAborted() { + if currBackgroundAr.Status == nil || !currBackgroundAr.Status.Status.Completed() { newStatus.Canary.CurrentBackgroundAnalysisRun = currBackgroundAr.Name } } + currStepAr := analysisutil.GetCurrentStepAnalysisRun(currArs) + if currStepAr != nil && !roCtx.PauseContext().IsAborted() { + if currStepAr.Status == nil || !currStepAr.Status.Status.Completed() { + newStatus.Canary.CurrentStepAnalysisRun = currStepAr.Name + } + } + + if roCtx.PauseContext().IsAborted() { + if stepCount > int32(0) { + if newStatus.Canary.StableRS == newStatus.CurrentPodHash { + newStatus.CurrentStepIndex = &stepCount + } else { + newStatus.CurrentStepIndex = pointer.Int32Ptr(0) + } + } + newStatus = c.calculateRolloutConditions(roCtx, newStatus) + return c.persistRolloutStatus(roCtx, &newStatus) + } if currentStepIndex != nil && *currentStepIndex == stepCount { logCtx.Info("Rollout has executed every step") newStatus.CurrentStepIndex = &stepCount if newRS != nil && newRS.Status.AvailableReplicas == defaults.GetReplicasOrDefault(r.Spec.Replicas) { - //TODO(dthomson) cancel background analysis here not when we reach currentStepIndex == stepCount logCtx.Info("New RS has successfully progressed") newStatus.Canary.StableRS = newStatus.CurrentPodHash } @@ -324,6 +348,7 @@ func (c *RolloutController) syncRolloutStatusCanary(roCtx *canaryContext) error if completedCurrentCanaryStep(roCtx) { *currentStepIndex++ newStatus.CurrentStepIndex = currentStepIndex + newStatus.Canary.CurrentStepAnalysisRun = "" if int(*currentStepIndex) == len(r.Spec.Strategy.Canary.Steps) { c.recorder.Event(r, corev1.EventTypeNormal, "SettingStableRS", "Completed all steps") } @@ -334,14 +359,6 @@ func (c *RolloutController) syncRolloutStatusCanary(roCtx *canaryContext) error return c.persistRolloutStatus(roCtx, &newStatus) } - currStepAr := analysisutil.GetCurrentStepAnalysisRun(currArs) - if currStepAr != nil { - if currStepAr.Status == nil || !currStepAr.Status.Status.Completed() || analysisutil.IsTerminating(currStepAr) { - newStatus.Canary.CurrentStepAnalysisRun = currStepAr.Name - } - - } - if currExp != nil { newStatus.Canary.CurrentExperiment = currExp.Name if currExp.Status.Status.Completed() && currExp.Status.Status != v1alpha1.AnalysisStatusSuccessful { diff --git a/rollout/canary_test.go b/rollout/canary_test.go index efd2a0496f..4f2366c816 100644 --- a/rollout/canary_test.go +++ b/rollout/canary_test.go @@ -1135,3 +1135,79 @@ func TestHandleNilNewRSOnScaleAndImageChange(t *testing.T) { patch := f.getPatchedRollout(patchIndex) assert.Equal(t, calculatePatch(r2, OnlyObservedGenerationPatch), patch) } + +func TestHandleCanaryAbort(t *testing.T) { + t.Run("Scale up stable ReplicaSet", func(t *testing.T) { + f := newFixture(t) + defer f.Close() + + steps := []v1alpha1.CanaryStep{ + {SetWeight: int32Ptr(10)}, + {SetWeight: int32Ptr(20)}, + {SetWeight: int32Ptr(30)}, + } + r1 := newCanaryRollout("foo", 10, nil, steps, int32Ptr(1), intstr.FromInt(1), intstr.FromInt(0)) + rs1 := newReplicaSetWithStatus(r1, 9, 9) + rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] + r2 := bumpVersion(r1) + rs2 := newReplicaSetWithStatus(r2, 1, 1) + + f.kubeobjects = append(f.kubeobjects, rs1, rs2) + f.replicaSetLister = append(f.replicaSetLister, rs1, rs2) + + r2 = updateCanaryRolloutStatus(r2, rs1PodHash, 10, 1, 10, false) + r2.Status.Abort = true + f.rolloutLister = append(f.rolloutLister, r2) + f.objects = append(f.objects, r2) + + rsIndex := f.expectUpdateReplicaSetAction(rs2) + patchIndex := f.expectPatchRolloutAction(r2) + f.run(getKey(r2, t)) + + updatedRS := f.getUpdatedReplicaSet(rsIndex) + assert.Equal(t, int32(10), *updatedRS.Spec.Replicas) + + patch := f.getPatchedRollout(patchIndex) + expectedPatch := `{ + "status":{ + "currentStepIndex": 0, + "conditions": %s + } + }` + newConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r2, false) + assert.Equal(t, calculatePatch(r2, fmt.Sprintf(expectedPatch, newConditions)), patch) + }) + + t.Run("Do not reset currentStepCount if newRS is stableRS", func(t *testing.T) { + f := newFixture(t) + defer f.Close() + + steps := []v1alpha1.CanaryStep{ + {SetWeight: int32Ptr(10)}, + {SetWeight: int32Ptr(20)}, + {SetWeight: int32Ptr(30)}, + } + r1 := newCanaryRollout("foo", 2, nil, steps, int32Ptr(3), intstr.FromInt(1), intstr.FromInt(0)) + r1.Status.Abort = true + rs1 := newReplicaSetWithStatus(r1, 2, 2) + rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] + r1 = updateCanaryRolloutStatus(r1, rs1PodHash, 2, 2, 2, false) + + f.kubeobjects = append(f.kubeobjects, rs1) + f.replicaSetLister = append(f.replicaSetLister, rs1) + + f.rolloutLister = append(f.rolloutLister, r1) + f.objects = append(f.objects, r1) + + patchIndex := f.expectPatchRolloutAction(r1) + f.run(getKey(r1, t)) + patch := f.getPatchedRollout(patchIndex) + expectedPatch := `{ + "status":{ + "conditions": %s + } + }` + newConditions := generateConditionsPatch(true, conditions.RolloutAbortedReason, r1, false) + assert.Equal(t, calculatePatch(r1, fmt.Sprintf(expectedPatch, newConditions)), patch) + }) +} diff --git a/rollout/controller_test.go b/rollout/controller_test.go index 296c01058c..e2de43771e 100644 --- a/rollout/controller_test.go +++ b/rollout/controller_test.go @@ -156,6 +156,10 @@ func newProgressingCondition(reason string, resourceObj runtime.Object) (v1alpha if reason == conditions.ReplicaSetUpdatedReason { msg = fmt.Sprintf(conditions.RolloutProgressingMessage, resource.Name) } + if reason == conditions.RolloutAbortedReason { + msg = conditions.RolloutAbortedMessage + status = corev1.ConditionFalse + } if reason == conditions.RolloutExperimentFailedReason { exName := fmt.Sprintf("%s%s", experimentutil.ExperimentGeneratedNameFromRollout(resource), MockGeneratedNameSuffix) msg = fmt.Sprintf(conditions.RolloutExperimentFailedMessage, exName, resource.Name) @@ -163,10 +167,11 @@ func newProgressingCondition(reason string, resourceObj runtime.Object) (v1alpha } if reason == conditions.RolloutAnalysisRunFailedReason { atName := "" - if resource.Spec.Strategy.Canary.Steps != nil && resource.Status.CurrentStepIndex != nil { + if resource.Spec.Strategy.Canary.Analysis != nil { + atName = resource.Spec.Strategy.Canary.Analysis.TemplateName + } else if resource.Spec.Strategy.Canary.Steps != nil && resource.Status.CurrentStepIndex != nil { atName = resource.Spec.Strategy.Canary.Steps[*resource.Status.CurrentStepIndex].Analysis.TemplateName } - //TODO(dthomson) Add support for parellel analysisRuns too arName := fmt.Sprintf("%s-%s-%s-%s", resource.Name, atName, resource.Status.CurrentPodHash, MockGeneratedNameSuffix) msg = fmt.Sprintf(conditions.RolloutAnalysisRunFailedMessage, arName, resource.Name) status = corev1.ConditionFalse diff --git a/rollout/experiment.go b/rollout/experiment.go index 9736e98f0e..399b3544a7 100644 --- a/rollout/experiment.go +++ b/rollout/experiment.go @@ -105,15 +105,14 @@ func (c *RolloutController) reconcileExperiments(roCtx *canaryContext) error { stableRS := roCtx.StableRS() otherExs := roCtx.OtherExperiments() - for _, otherEx := range otherExs { - if !otherEx.Status.Status.Completed() { - logCtx.Infof("Canceling other running experiment '%s' owned by rollout", otherEx.Name) - experimentIf := c.argoprojclientset.ArgoprojV1alpha1().Experiments(otherEx.Namespace) - err := experimentutil.Terminate(experimentIf, otherEx.Name) - if err != nil { - return err - } - } + if roCtx.PauseContext().IsAborted() { + allExs := append(otherExs, roCtx.CurrentExperiment()) + return c.cancelExperiments(roCtx, allExs) + } + + err := c.cancelExperiments(roCtx, otherExs) + if err != nil { + return err } step, _ := replicasetutil.GetCurrentCanaryStep(rollout) @@ -143,7 +142,7 @@ func (c *RolloutController) reconcileExperiments(roCtx *canaryContext) error { } exsToDelete := experimentutil.FilterExperimentsToDelete(otherExs, roCtx.AllRSs()) - err := c.deleteExperiments(roCtx, exsToDelete) + err = c.deleteExperiments(roCtx, exsToDelete) if err != nil { return err } @@ -151,6 +150,23 @@ func (c *RolloutController) reconcileExperiments(roCtx *canaryContext) error { return nil } +func (c *RolloutController) cancelExperiments(roCtx *canaryContext, exs []*v1alpha1.Experiment) error { + for i := range exs { + ex := exs[i] + if ex == nil { + continue + } + if !ex.Spec.Terminate && !experimentutil.HasFinished(ex) { + roCtx.Log().Infof("Canceling other running experiment '%s' owned by rollout", ex.Name) + err := experimentutil.Terminate(c.argoprojclientset.ArgoprojV1alpha1().Experiments(ex.Namespace), ex.Name) + if err != nil { + return err + } + } + } + return nil +} + func (c *RolloutController) deleteExperiments(roCtx rolloutContext, exs []*v1alpha1.Experiment) error { for i := range exs { ex := exs[i] diff --git a/rollout/experiment_test.go b/rollout/experiment_test.go index 5fadbcdb55..1548e674ae 100644 --- a/rollout/experiment_test.go +++ b/rollout/experiment_test.go @@ -450,3 +450,36 @@ func TestDeleteExperimentsAfterRSDelete(t *testing.T) { deletedEx := f.getDeletedExperiment(deletedIndex) assert.Equal(t, deletedEx, exToDelete.Name) } + +func TestCancelExperimentWhenAborted(t *testing.T) { + f := newFixture(t) + defer f.Close() + + steps := []v1alpha1.CanaryStep{{ + Experiment: &v1alpha1.RolloutExperimentStep{}, + }} + + r1 := newCanaryRollout("foo", 1, nil, steps, pointer.Int32Ptr(0), intstr.FromInt(0), intstr.FromInt(1)) + r2 := bumpVersion(r1) + + rs1 := newReplicaSetWithStatus(r1, 1, 1) + rs2 := newReplicaSetWithStatus(r2, 0, 0) + rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] + + ex, _ := GetExperimentFromTemplate(r2, rs1, rs2) + ex.Name = "test" + ex.Status.Status = v1alpha1.AnalysisStatusRunning + + r2 = updateCanaryRolloutStatus(r2, rs1PodHash, 1, 0, 1, false) + r2.Status.Abort = true + f.kubeobjects = append(f.kubeobjects, rs1, rs2) + f.replicaSetLister = append(f.replicaSetLister, rs1, rs2) + + f.rolloutLister = append(f.rolloutLister, r2) + f.experimentLister = append(f.experimentLister, ex) + f.objects = append(f.objects, r2, ex) + + f.expectPatchExperimentAction(ex) + f.expectPatchRolloutAction(r2) + f.run(getKey(r2, t)) +} diff --git a/rollout/pause.go b/rollout/pause.go index f53751c7d4..b9485bf8a9 100644 --- a/rollout/pause.go +++ b/rollout/pause.go @@ -17,6 +17,30 @@ type pauseContext struct { addPauseReasons []v1alpha1.PauseReason removePauseReasons []v1alpha1.PauseReason clearPauseConditions bool + addAbort bool + removeAbort bool +} + +func (pCtx *pauseContext) HasAddPause() bool { + return len(pCtx.addPauseReasons) > 0 +} + +func (pCtx *pauseContext) IsAborted() bool { + if pCtx.removeAbort { + return false + } + if pCtx.addAbort || pCtx.rollout.Status.Abort { + return true + } + return false +} + +func (pCtx *pauseContext) AddAbort() { + pCtx.addAbort = true +} + +func (pCtx *pauseContext) RemoveAbort() { + pCtx.removeAbort = true } func (pCtx *pauseContext) AddPauseCondition(reason v1alpha1.PauseReason) { @@ -31,6 +55,16 @@ func (pCtx *pauseContext) ClearPauseConditions() { } func (pCtx *pauseContext) CalculatePauseStatus(newStatus *v1alpha1.RolloutStatus) { + if pCtx.addAbort { + newStatus.Abort = true + return + } + if !pCtx.removeAbort && pCtx.rollout.Status.Abort { + newStatus.Abort = true + return + } + newStatus.Abort = false + if pCtx.clearPauseConditions { return } diff --git a/rollout/sync.go b/rollout/sync.go index b476e3f986..3b786b3f9a 100644 --- a/rollout/sync.go +++ b/rollout/sync.go @@ -497,6 +497,9 @@ func (c *RolloutController) calculateRolloutConditions(roCtx rolloutContext, new // Check for progress only if the latest rollout hasn't completed yet. if !isCompleteRollout { switch { + case roCtx.PauseContext().IsAborted(): + condition := conditions.NewRolloutCondition(v1alpha1.RolloutProgressing, corev1.ConditionFalse, conditions.RolloutAbortedReason, conditions.RolloutAbortedMessage) + conditions.SetRolloutCondition(&newStatus, *condition) case conditions.RolloutComplete(r, &newStatus): // Update the rollout conditions with a message for the new replica set that // was successfully deployed. If the condition already exists, we ignore this update. diff --git a/utils/conditions/conditions.go b/utils/conditions/conditions.go index 9f6a4b214b..8fdde984cf 100644 --- a/utils/conditions/conditions.go +++ b/utils/conditions/conditions.go @@ -81,6 +81,11 @@ const ( // FoundNewRSMessage is added in a rollout when it adopts an existing replica set. FoundNewRSMessage = "Found new replica set %q" + // RolloutAbortedReason indicates that the rollout was aborted + RolloutAbortedReason = "RolloutAborted" + // RolloutAbortedMessage indicates that the rollout was aborted + RolloutAbortedMessage = "Rollout is aborted" + // NewRSAvailableReason is added in a rollout when its newest replica set is made available // ie. the number of new pods that have passed readiness checks and run for at least minReadySeconds // is at least the minimum available pods that need to run for the rollout. diff --git a/utils/replicaset/canary.go b/utils/replicaset/canary.go index 0fb0658295..fe2d56c179 100644 --- a/utils/replicaset/canary.go +++ b/utils/replicaset/canary.go @@ -240,6 +240,9 @@ func GetCurrentCanaryStep(rollout *v1alpha1.Rollout) (*v1alpha1.CanaryStep, *int // until it finds a setWeight step. The controller defaults to 100 if it iterates through all the steps with no // setWeight or if there is no current step (i.e. the controller has already stepped through all the steps). func GetCurrentSetWeight(rollout *v1alpha1.Rollout) int32 { + if rollout.Status.Abort { + return 0 + } currentStep, currentStepIndex := GetCurrentCanaryStep(rollout) if currentStep == nil { return 100 diff --git a/utils/replicaset/canary_test.go b/utils/replicaset/canary_test.go index 3a0524eadf..89adac03b6 100644 --- a/utils/replicaset/canary_test.go +++ b/utils/replicaset/canary_test.go @@ -420,6 +420,10 @@ func TestGetCurrentSetWeight(t *testing.T) { setWeight = GetCurrentSetWeight(rollout) assert.Equal(t, setWeight, int32(10)) + rollout.Status.Abort = true + setWeight = GetCurrentSetWeight(rollout) + assert.Equal(t, setWeight, int32(0)) + } func TestGetCurrentExperiment(t *testing.T) {