Skip to content

Commit 19c4b14

Browse files
committed
pkg/cvo/sync_worker: Generalize CancelError to ContextError
With this commit, I drop contextIsCancelled in favor of Context.Err(). From the docs [1]: If Done is not yet closed, Err returns nil. If Done is closed, Err returns a non-nil error explaining why: Canceled if the context was canceled or DeadlineExceeded if the context's deadline passed. After Err returns a non-nil error, successive calls to Err return the same error. I dunno why we'd been checking Done() instead, but contextIsCancelled dates back to 961873d (sync: Do config syncing in the background, 2019-01-11, #82). I've also generalized a number of *Cancel* helpers to be *Context* to remind folks that Context.Err() can be DeadlineExceeded as well as Canceled, and the CVO uses both WithCancel and WithTimeout. The new error messages will be either: update context deadline exceeded at 1 of 2 or: update context canceled at 1 of 2 Instead of always claiming: update was cancelled at 1 of 2 [1]: https://golang.org/pkg/context/#Context
1 parent e318b86 commit 19c4b14

File tree

1 file changed

+16
-25
lines changed

1 file changed

+16
-25
lines changed

pkg/cvo/sync_worker.go

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -626,8 +626,8 @@ func (w *SyncWorker) apply(ctx context.Context, payloadUpdate *payload.Update, w
626626
if precreateObjects {
627627
payload.RunGraph(ctx, graph, 8, func(ctx context.Context, tasks []*payload.Task) error {
628628
for _, task := range tasks {
629-
if contextIsCancelled(ctx) {
630-
return cr.CancelError()
629+
if err := ctx.Err(); err != nil {
630+
return cr.ContextError(err)
631631
}
632632
if task.Manifest.GVK != configv1.SchemeGroupVersion.WithKind("ClusterOperator") {
633633
continue
@@ -645,8 +645,8 @@ func (w *SyncWorker) apply(ctx context.Context, payloadUpdate *payload.Update, w
645645
// update each object
646646
errs := payload.RunGraph(ctx, graph, maxWorkers, func(ctx context.Context, tasks []*payload.Task) error {
647647
for _, task := range tasks {
648-
if contextIsCancelled(ctx) {
649-
return cr.CancelError()
648+
if err := ctx.Err(); err != nil {
649+
return cr.ContextError(err)
650650
}
651651
cr.Update()
652652

@@ -690,11 +690,11 @@ func init() {
690690
)
691691
}
692692

693-
type errCanceled struct {
693+
type errContext struct {
694694
err error
695695
}
696696

697-
func (e errCanceled) Error() string { return e.err.Error() }
697+
func (e errContext) Error() string { return e.err.Error() }
698698

699699
// consistentReporter hides the details of calculating the status based on the progress
700700
// of the graph runner.
@@ -731,7 +731,7 @@ func (r *consistentReporter) Error(err error) {
731731
copied := r.status
732732
copied.Step = "ApplyResources"
733733
copied.Fraction = float32(r.done) / float32(r.total)
734-
if !isCancelledError(err) {
734+
if !isContextError(err) {
735735
copied.Failure = err
736736
}
737737
r.reporter.Report(copied)
@@ -752,10 +752,10 @@ func (r *consistentReporter) Errors(errs []error) error {
752752
return err
753753
}
754754

755-
func (r *consistentReporter) CancelError() error {
755+
func (r *consistentReporter) ContextError(err error) error {
756756
r.lock.Lock()
757757
defer r.lock.Unlock()
758-
return errCanceled{fmt.Errorf("update was cancelled at %d of %d", r.done, r.total)}
758+
return errContext{fmt.Errorf("update %s at %d of %d", err, r.done, r.total)}
759759
}
760760

761761
func (r *consistentReporter) Complete() {
@@ -771,11 +771,11 @@ func (r *consistentReporter) Complete() {
771771
r.reporter.Report(copied)
772772
}
773773

774-
func isCancelledError(err error) bool {
774+
func isContextError(err error) bool {
775775
if err == nil {
776776
return false
777777
}
778-
_, ok := err.(errCanceled)
778+
_, ok := err.(errContext)
779779
return ok
780780
}
781781

@@ -796,11 +796,12 @@ func isImageVerificationError(err error) bool {
796796
// not truly an error (cancellation).
797797
// TODO: take into account install vs upgrade
798798
func summarizeTaskGraphErrors(errs []error) error {
799-
// we ignore cancellation errors since they don't provide good feedback to users and are an internal
800-
// detail of the server
801-
err := errors.FilterOut(errors.NewAggregate(errs), isCancelledError)
799+
// we ignore context errors (canceled or timed out) since they don't
800+
// provide good feedback to users and are an internal detail of the
801+
// server
802+
err := errors.FilterOut(errors.NewAggregate(errs), isContextError)
802803
if err == nil {
803-
klog.V(4).Infof("All errors were cancellation errors: %v", errs)
804+
klog.V(4).Infof("All errors were context errors: %v", errs)
804805
return nil
805806
}
806807
agg, ok := err.(errors.Aggregate)
@@ -971,16 +972,6 @@ func ownerRefModifier(config *configv1.ClusterVersion) resourcebuilder.MetaV1Obj
971972
}
972973
}
973974

974-
// contextIsCancelled returns true if the provided context is cancelled.
975-
func contextIsCancelled(ctx context.Context) bool {
976-
select {
977-
case <-ctx.Done():
978-
return true
979-
default:
980-
return false
981-
}
982-
}
983-
984975
// runThrottledStatusNotifier invokes fn every time ch is updated, but no more often than once
985976
// every interval. If bucket is non-zero then the channel is throttled like a rate limiter bucket.
986977
func runThrottledStatusNotifier(stopCh <-chan struct{}, interval time.Duration, bucket int, ch <-chan SyncWorkerStatus, fn func()) {

0 commit comments

Comments
 (0)