Skip to content

Commit

Permalink
Return error & send error event on cancel/timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
karlkfi committed Oct 19, 2021
1 parent 066fd77 commit ee6c5a7
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 17 deletions.
4 changes: 0 additions & 4 deletions cmd/apply/cmdapply.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions cmd/destroy/cmddestroy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions cmd/preview/cmdpreview.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cmd/status/cmdstatus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 5 additions & 2 deletions cmd/status/cmdstatus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 7 additions & 0 deletions pkg/apply/applier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
7 changes: 7 additions & 0 deletions pkg/apply/destroyer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
3 changes: 1 addition & 2 deletions pkg/apply/taskrunner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/apply/taskrunner/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ func TestBaseRunnerCancellation(t *testing.T) {
},
},
contextTimeout: 2 * time.Second,
expectedError: context.Canceled,
expectedEventTypes: []event.Type{
event.ActionGroupType,
event.ApplyType,
Expand All @@ -315,6 +316,7 @@ func TestBaseRunnerCancellation(t *testing.T) {
},
},
contextTimeout: 2 * time.Second,
expectedError: context.Canceled,
expectedEventTypes: []event.Type{
event.ActionGroupType,
event.ActionGroupType,
Expand Down

0 comments on commit ee6c5a7

Please sign in to comment.