From e681a790584ada4c0a7813f32698fc3ff0784305 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 11 Dec 2024 12:57:40 +0100 Subject: [PATCH] apimachinery wait: support contextual logging By passing the context, if available, down into the actual wait loop it becomes possible to use contextual logging when handling a crash. That then logs more information about which component encountered the problem (WithName e.g. in kube-controller-manager) and what it was doing at the time (WithValues e.g. in kube-scheduler). --- .../apimachinery/pkg/util/wait/backoff.go | 50 ++++++++++++------- .../k8s.io/apimachinery/pkg/util/wait/loop.go | 4 +- .../k8s.io/apimachinery/pkg/util/wait/wait.go | 3 +- 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/wait/backoff.go b/staging/src/k8s.io/apimachinery/pkg/util/wait/backoff.go index 4187619256e..177be09a957 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/wait/backoff.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/wait/backoff.go @@ -157,6 +157,8 @@ func (b Backoff) DelayWithReset(c clock.Clock, resetInterval time.Duration) Dela // Until is syntactic sugar on top of JitterUntil with zero jitter factor and // with sliding = true (which means the timer for period starts after the f // completes). +// +// Contextual logging: UntilWithContext should be used instead of Until in code which supports contextual logging. func Until(f func(), period time.Duration, stopCh <-chan struct{}) { JitterUntil(f, period, 0.0, true, stopCh) } @@ -176,6 +178,8 @@ func UntilWithContext(ctx context.Context, f func(context.Context), period time. // NonSlidingUntil is syntactic sugar on top of JitterUntil with zero jitter // factor, with sliding = false (meaning the timer for period starts at the same // time as the function starts). +// +// Contextual logging: NonSlidingUntilWithContext should be used instead of NonSlidingUntil in code which supports contextual logging. func NonSlidingUntil(f func(), period time.Duration, stopCh <-chan struct{}) { JitterUntil(f, period, 0.0, false, stopCh) } @@ -200,19 +204,44 @@ func NonSlidingUntilWithContext(ctx context.Context, f func(context.Context), pe // // Close stopCh to stop. f may not be invoked if stop channel is already // closed. Pass NeverStop to if you don't want it stop. +// +// Contextual logging: JitterUntilWithContext should be used instead of JitterUntil in code which supports contextual logging. func JitterUntil(f func(), period time.Duration, jitterFactor float64, sliding bool, stopCh <-chan struct{}) { BackoffUntil(f, NewJitteredBackoffManager(period, jitterFactor, &clock.RealClock{}), sliding, stopCh) } +// JitterUntilWithContext loops until context is done, running f every period. +// +// If jitterFactor is positive, the period is jittered before every run of f. +// If jitterFactor is not positive, the period is unchanged and not jittered. +// +// If sliding is true, the period is computed after f runs. If it is false then +// period includes the runtime for f. +// +// Cancel context to stop. f may not be invoked if context is already done. +func JitterUntilWithContext(ctx context.Context, f func(context.Context), period time.Duration, jitterFactor float64, sliding bool) { + BackoffUntilWithContext(ctx, f, NewJitteredBackoffManager(period, jitterFactor, &clock.RealClock{}), sliding) +} + // BackoffUntil loops until stop channel is closed, run f every duration given by BackoffManager. // // If sliding is true, the period is computed after f runs. If it is false then // period includes the runtime for f. +// +// Contextual logging: BackoffUntilWithContext should be used instead of BackoffUntil in code which supports contextual logging. func BackoffUntil(f func(), backoff BackoffManager, sliding bool, stopCh <-chan struct{}) { + BackoffUntilWithContext(ContextForChannel(stopCh), func(context.Context) { f() }, backoff, sliding) +} + +// BackoffUntilWithContext loops until context is done, run f every duration given by BackoffManager. +// +// If sliding is true, the period is computed after f runs. If it is false then +// period includes the runtime for f. +func BackoffUntilWithContext(ctx context.Context, f func(ctx context.Context), backoff BackoffManager, sliding bool) { var t clock.Timer for { select { - case <-stopCh: + case <-ctx.Done(): return default: } @@ -222,8 +251,8 @@ func BackoffUntil(f func(), backoff BackoffManager, sliding bool, stopCh <-chan } func() { - defer runtime.HandleCrash() - f() + defer runtime.HandleCrashWithContext(ctx) + f(ctx) }() if sliding { @@ -236,7 +265,7 @@ func BackoffUntil(f func(), backoff BackoffManager, sliding bool, stopCh <-chan // In order to mitigate we re-check stopCh at the beginning // of every loop to prevent extra executions of f(). select { - case <-stopCh: + case <-ctx.Done(): if !t.Stop() { <-t.C() } @@ -246,19 +275,6 @@ func BackoffUntil(f func(), backoff BackoffManager, sliding bool, stopCh <-chan } } -// JitterUntilWithContext loops until context is done, running f every period. -// -// If jitterFactor is positive, the period is jittered before every run of f. -// If jitterFactor is not positive, the period is unchanged and not jittered. -// -// If sliding is true, the period is computed after f runs. If it is false then -// period includes the runtime for f. -// -// Cancel context to stop. f may not be invoked if context is already expired. -func JitterUntilWithContext(ctx context.Context, f func(context.Context), period time.Duration, jitterFactor float64, sliding bool) { - JitterUntil(func() { f(ctx) }, period, jitterFactor, sliding, ctx.Done()) -} - // backoffManager provides simple backoff behavior in a threadsafe manner to a caller. type backoffManager struct { backoff Backoff diff --git a/staging/src/k8s.io/apimachinery/pkg/util/wait/loop.go b/staging/src/k8s.io/apimachinery/pkg/util/wait/loop.go index 107bfc132fd..9f9b929ffac 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/wait/loop.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/wait/loop.go @@ -49,7 +49,7 @@ func loopConditionUntilContext(ctx context.Context, t Timer, immediate, sliding // if we haven't requested immediate execution, delay once if immediate { if ok, err := func() (bool, error) { - defer runtime.HandleCrash() + defer runtime.HandleCrashWithContext(ctx) return condition(ctx) }(); err != nil || ok { return err @@ -83,7 +83,7 @@ func loopConditionUntilContext(ctx context.Context, t Timer, immediate, sliding t.Next() } if ok, err := func() (bool, error) { - defer runtime.HandleCrash() + defer runtime.HandleCrashWithContext(ctx) return condition(ctx) }(); err != nil || ok { return err diff --git a/staging/src/k8s.io/apimachinery/pkg/util/wait/wait.go b/staging/src/k8s.io/apimachinery/pkg/util/wait/wait.go index 6805e8cf948..a65d7708185 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/wait/wait.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/wait/wait.go @@ -141,6 +141,7 @@ func (c channelContext) Value(key any) any { return nil } // // Deprecated: Will be removed when the legacy polling methods are removed. func runConditionWithCrashProtection(condition ConditionFunc) (bool, error) { + //nolint:logcheck // Already deprecated. defer runtime.HandleCrash() return condition() } @@ -150,7 +151,7 @@ func runConditionWithCrashProtection(condition ConditionFunc) (bool, error) { // // Deprecated: Will be removed when the legacy polling methods are removed. func runConditionWithCrashProtectionWithContext(ctx context.Context, condition ConditionWithContextFunc) (bool, error) { - defer runtime.HandleCrash() + defer runtime.HandleCrashWithContext(ctx) return condition(ctx) }