Skip to content

Commit 2edac36

Browse files
committed
client-go/rest: backoff with context support
The BackoffManager interface sleeps without considering the caller's context, i.e. cancellation is not supported. This alone is reason enough to deprecate it and to replace it with an interface that supports a context parameter. The other reason is that contextual logging needs that parameter.
1 parent b6309f7 commit 2edac36

File tree

26 files changed

+419
-51
lines changed

26 files changed

+419
-51
lines changed

staging/src/k8s.io/apiextensions-apiserver/go.sum

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

staging/src/k8s.io/apiserver/go.sum

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

staging/src/k8s.io/client-go/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ require (
5858
github.com/onsi/ginkgo/v2 v2.21.0 // indirect
5959
github.com/pkg/errors v0.9.1 // indirect
6060
github.com/pmezard/go-difflib v1.0.0 // indirect
61+
github.com/stretchr/objx v0.5.2 // indirect
6162
github.com/x448/float16 v0.8.4 // indirect
6263
golang.org/x/sys v0.28.0 // indirect
6364
golang.org/x/text v0.21.0 // indirect

staging/src/k8s.io/client-go/go.sum

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
dir: .
3+
filename: "mock_{{.InterfaceName | snakecase}}_test.go"
4+
boilerplate-file: ../../../../../hack/boilerplate/boilerplate.generatego.txt
5+
outpkg: rest
6+
with-expecter: true
7+
packages:
8+
k8s.io/client-go/rest:
9+
interfaces:
10+
BackoffManager:

staging/src/k8s.io/client-go/rest/client.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ type RESTClient struct {
9393
content requestClientContentConfigProvider
9494

9595
// creates BackoffManager that is passed to requests.
96-
createBackoffMgr func() BackoffManager
96+
createBackoffMgr func() BackoffManagerWithContext
9797

9898
// rateLimiter is shared among all requests created by this client unless specifically
9999
// overridden.
@@ -178,7 +178,7 @@ func (c *RESTClient) GetRateLimiter() flowcontrol.RateLimiter {
178178
// readExpBackoffConfig handles the internal logic of determining what the
179179
// backoff policy is. By default if no information is available, NoBackoff.
180180
// TODO Generalize this see #17727 .
181-
func readExpBackoffConfig() BackoffManager {
181+
func readExpBackoffConfig() BackoffManagerWithContext {
182182
backoffBase := os.Getenv(envBackoffBase)
183183
backoffDuration := os.Getenv(envBackoffDuration)
184184

staging/src/k8s.io/client-go/rest/client_test.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"k8s.io/apimachinery/pkg/types"
3636
"k8s.io/client-go/kubernetes/scheme"
3737
utiltesting "k8s.io/client-go/util/testing"
38+
"k8s.io/klog/v2/ktesting"
3839

3940
"github.com/google/go-cmp/cmp"
4041
)
@@ -335,36 +336,36 @@ func TestHTTPProxy(t *testing.T) {
335336
}
336337

337338
func TestCreateBackoffManager(t *testing.T) {
338-
339+
_, ctx := ktesting.NewTestContext(t)
339340
theUrl, _ := url.Parse("http://localhost")
340341

341342
// 1 second base backoff + duration of 2 seconds -> exponential backoff for requests.
342343
t.Setenv(envBackoffBase, "1")
343344
t.Setenv(envBackoffDuration, "2")
344345
backoff := readExpBackoffConfig()
345-
backoff.UpdateBackoff(theUrl, nil, 500)
346-
backoff.UpdateBackoff(theUrl, nil, 500)
347-
if backoff.CalculateBackoff(theUrl)/time.Second != 2 {
346+
backoff.UpdateBackoffWithContext(ctx, theUrl, nil, 500)
347+
backoff.UpdateBackoffWithContext(ctx, theUrl, nil, 500)
348+
if backoff.CalculateBackoffWithContext(ctx, theUrl)/time.Second != 2 {
348349
t.Errorf("Backoff env not working.")
349350
}
350351

351352
// 0 duration -> no backoff.
352353
t.Setenv(envBackoffBase, "1")
353354
t.Setenv(envBackoffDuration, "0")
354-
backoff.UpdateBackoff(theUrl, nil, 500)
355-
backoff.UpdateBackoff(theUrl, nil, 500)
355+
backoff.UpdateBackoffWithContext(ctx, theUrl, nil, 500)
356+
backoff.UpdateBackoffWithContext(ctx, theUrl, nil, 500)
356357
backoff = readExpBackoffConfig()
357-
if backoff.CalculateBackoff(theUrl)/time.Second != 0 {
358+
if backoff.CalculateBackoffWithContext(ctx, theUrl)/time.Second != 0 {
358359
t.Errorf("Zero backoff duration, but backoff still occurring.")
359360
}
360361

361362
// No env -> No backoff.
362363
t.Setenv(envBackoffBase, "")
363364
t.Setenv(envBackoffDuration, "")
364365
backoff = readExpBackoffConfig()
365-
backoff.UpdateBackoff(theUrl, nil, 500)
366-
backoff.UpdateBackoff(theUrl, nil, 500)
367-
if backoff.CalculateBackoff(theUrl)/time.Second != 0 {
366+
backoff.UpdateBackoffWithContext(ctx, theUrl, nil, 500)
367+
backoff.UpdateBackoffWithContext(ctx, theUrl, nil, 500)
368+
if backoff.CalculateBackoffWithContext(ctx, theUrl)/time.Second != 0 {
368369
t.Errorf("Backoff should have been 0.")
369370
}
370371

staging/src/k8s.io/client-go/rest/mock_backoff_manager_test.go

Lines changed: 168 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

staging/src/k8s.io/client-go/rest/request.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ type Request struct {
106106
warningHandler WarningHandlerWithContext
107107

108108
rateLimiter flowcontrol.RateLimiter
109-
backoff BackoffManager
109+
backoff BackoffManagerWithContext
110110
timeout time.Duration
111111
maxRetries int
112112

@@ -136,7 +136,7 @@ type Request struct {
136136

137137
// NewRequest creates a new request helper object for accessing runtime.Objects on a server.
138138
func NewRequest(c *RESTClient) *Request {
139-
var backoff BackoffManager
139+
var backoff BackoffManagerWithContext
140140
if c.createBackoffMgr != nil {
141141
backoff = c.createBackoffMgr()
142142
}
@@ -259,13 +259,27 @@ func (r *Request) Resource(resource string) *Request {
259259
}
260260

261261
// BackOff sets the request's backoff manager to the one specified,
262-
// or defaults to the stub implementation if nil is provided
262+
// or defaults to the stub implementation if nil is provided.
263+
//
264+
// Deprecated: BackoffManager.Sleep ignores the caller's context. Use BackOffWithContext and BackoffManagerWithContext instead.
263265
func (r *Request) BackOff(manager BackoffManager) *Request {
264266
if manager == nil {
265267
r.backoff = &NoBackoff{}
266268
return r
267269
}
268270

271+
r.backoff = &backoffManagerNopContext{BackoffManager: manager}
272+
return r
273+
}
274+
275+
// BackOffWithContext sets the request's backoff manager to the one specified,
276+
// or defaults to the stub implementation if nil is provided.
277+
func (r *Request) BackOffWithContext(manager BackoffManagerWithContext) *Request {
278+
if manager == nil {
279+
r.backoff = &NoBackoff{}
280+
return r
281+
}
282+
269283
r.backoff = manager
270284
return r
271285
}

0 commit comments

Comments
 (0)