From ee6c5a7fe41f430a401516ee07b1d1de34863d6d Mon Sep 17 00:00:00 2001
From: Karl Isenberg <karlisenberg@google.com>
Date: Fri, 15 Oct 2021 17:42:59 -0700
Subject: [PATCH] Return error & send error event on cancel/timeout

---
 cmd/apply/cmdapply.go               | 4 ----
 cmd/destroy/cmddestroy.go           | 4 ----
 cmd/preview/cmdpreview.go           | 4 ----
 cmd/status/cmdstatus.go             | 2 +-
 cmd/status/cmdstatus_test.go        | 7 +++++--
 pkg/apply/applier_test.go           | 7 +++++++
 pkg/apply/destroyer_test.go         | 7 +++++++
 pkg/apply/taskrunner/runner.go      | 3 +--
 pkg/apply/taskrunner/runner_test.go | 2 ++
 9 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/cmd/apply/cmdapply.go b/cmd/apply/cmdapply.go
index 84e3c876..c1d1918c 100644
--- a/cmd/apply/cmdapply.go
+++ b/cmd/apply/cmdapply.go
@@ -91,11 +91,7 @@ type ApplyRunner struct {
 }
 
 func (r *ApplyRunner) RunE(cmd *cobra.Command, args []string) error {
-	// If specified, use command context.
 	ctx := cmd.Context()
-	if ctx == nil {
-		ctx = context.Background()
-	}
 	// If specified, cancel with timeout.
 	if r.timeout != 0 {
 		var cancel context.CancelFunc
diff --git a/cmd/destroy/cmddestroy.go b/cmd/destroy/cmddestroy.go
index 3502a0c9..7b1600bb 100644
--- a/cmd/destroy/cmddestroy.go
+++ b/cmd/destroy/cmddestroy.go
@@ -77,11 +77,7 @@ type DestroyRunner struct {
 }
 
 func (r *DestroyRunner) RunE(cmd *cobra.Command, args []string) error {
-	// If specified, use command context.
 	ctx := cmd.Context()
-	if ctx == nil {
-		ctx = context.Background()
-	}
 	// If specified, cancel with timeout.
 	if r.timeout != 0 {
 		var cancel context.CancelFunc
diff --git a/cmd/preview/cmdpreview.go b/cmd/preview/cmdpreview.go
index 8d3abb75..785d2efe 100644
--- a/cmd/preview/cmdpreview.go
+++ b/cmd/preview/cmdpreview.go
@@ -88,11 +88,7 @@ type PreviewRunner struct {
 
 // RunE is the function run from the cobra command.
 func (r *PreviewRunner) RunE(cmd *cobra.Command, args []string) error {
-	// If specified, use command context.
 	ctx := cmd.Context()
-	if ctx == nil {
-		ctx = context.Background()
-	}
 	// If specified, cancel with timeout.
 	if r.timeout != 0 {
 		var cancel context.CancelFunc
diff --git a/cmd/status/cmdstatus.go b/cmd/status/cmdstatus.go
index 3cbabe35..8cafe866 100644
--- a/cmd/status/cmdstatus.go
+++ b/cmd/status/cmdstatus.go
@@ -127,7 +127,7 @@ func (r *StatusRunner) runE(cmd *cobra.Command, args []string) error {
 
 	// If the user has specified a timeout, we create a context with timeout,
 	// otherwise we create a context with cancel.
-	ctx := context.Background()
+	ctx := cmd.Context()
 	var cancel func()
 	if r.timeout != 0 {
 		ctx, cancel = context.WithTimeout(ctx, r.timeout)
diff --git a/cmd/status/cmdstatus_test.go b/cmd/status/cmdstatus_test.go
index 2edcf97f..71cdc2f0 100644
--- a/cmd/status/cmdstatus_test.go
+++ b/cmd/status/cmdstatus_test.go
@@ -236,12 +236,15 @@ deployment.apps/foo is InProgress: inProgress
 				timeout:   tc.timeout,
 			}
 
-			cmd := &cobra.Command{}
+			cmd := &cobra.Command{
+				RunE: runner.runE,
+			}
 			cmd.SetIn(strings.NewReader(tc.input))
 			var buf bytes.Buffer
 			cmd.SetOut(&buf)
+			cmd.SetArgs([]string{})
 
-			err := runner.runE(cmd, []string{})
+			err := cmd.Execute()
 
 			if tc.expectedErrMsg != "" {
 				if !assert.Error(t, err) {
diff --git a/pkg/apply/applier_test.go b/pkg/apply/applier_test.go
index 694d8c17..1958f881 100644
--- a/pkg/apply/applier_test.go
+++ b/pkg/apply/applier_test.go
@@ -709,6 +709,13 @@ func TestApplierCancel(t *testing.T) {
 				// 		Type:      event.Finished,
 				// 	},
 				// },
+				{
+					// Error
+					EventType: event.ErrorType,
+					ErrorEvent: &testutil.ExpErrorEvent{
+						Err: context.DeadlineExceeded,
+					},
+				},
 			},
 		},
 		"completed with timeout": {
diff --git a/pkg/apply/destroyer_test.go b/pkg/apply/destroyer_test.go
index 99084891..a9f3f41a 100644
--- a/pkg/apply/destroyer_test.go
+++ b/pkg/apply/destroyer_test.go
@@ -143,6 +143,13 @@ func TestDestroyerCancel(t *testing.T) {
 				},
 				// Inventory cannot be deleted, because the objects still exist,
 				// even tho they've been deleted (ex: blocked by finalizer).
+				{
+					// Error
+					EventType: event.ErrorType,
+					ErrorEvent: &testutil.ExpErrorEvent{
+						Err: context.DeadlineExceeded,
+					},
+				},
 			},
 		},
 		"completed with timeout": {
diff --git a/pkg/apply/taskrunner/runner.go b/pkg/apply/taskrunner/runner.go
index 1048570c..0b9e06f7 100644
--- a/pkg/apply/taskrunner/runner.go
+++ b/pkg/apply/taskrunner/runner.go
@@ -9,7 +9,6 @@ import (
 	"sort"
 	"time"
 
-	"k8s.io/klog/v2"
 	"sigs.k8s.io/cli-utils/pkg/apply/cache"
 	"sigs.k8s.io/cli-utils/pkg/apply/event"
 	"sigs.k8s.io/cli-utils/pkg/apply/poller"
@@ -257,7 +256,7 @@ func (b *baseRunner) run(ctx context.Context, taskQueue chan Task,
 		case <-doneCh:
 			doneCh = nil // Set doneCh to nil so we don't enter a busy loop.
 			abort = true
-			klog.V(3).Info("taskrunner cancelled by caller")
+			abortReason = ctx.Err() // always non-nil when doneCh is closed
 			completeIfWaitTask(currentTask, taskContext)
 		}
 	}
diff --git a/pkg/apply/taskrunner/runner_test.go b/pkg/apply/taskrunner/runner_test.go
index 8a24a009..b5366918 100644
--- a/pkg/apply/taskrunner/runner_test.go
+++ b/pkg/apply/taskrunner/runner_test.go
@@ -297,6 +297,7 @@ func TestBaseRunnerCancellation(t *testing.T) {
 				},
 			},
 			contextTimeout: 2 * time.Second,
+			expectedError:  context.Canceled,
 			expectedEventTypes: []event.Type{
 				event.ActionGroupType,
 				event.ApplyType,
@@ -315,6 +316,7 @@ func TestBaseRunnerCancellation(t *testing.T) {
 				},
 			},
 			contextTimeout: 2 * time.Second,
+			expectedError:  context.Canceled,
 			expectedEventTypes: []event.Type{
 				event.ActionGroupType,
 				event.ActionGroupType,