Skip to content

Commit 1e8e528

Browse files
inteoninteon
committed
replace deprecated wait calls with their non-deprecated replacements
Signed-off-by: Tim Ramlot <[email protected]>
1 parent b64cad8 commit 1e8e528

File tree

6 files changed

+75
-74
lines changed

6 files changed

+75
-74
lines changed

‎manager/manager.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ func (m *Manager) handleRequest(ctx context.Context, volumeID string, meta metad
512512

513513
// Poll every 200ms for the CertificateRequest to be ready
514514
lastFailureReason := ""
515-
if err := wait.PollUntilWithContext(ctx, time.Millisecond*200, func(ctx context.Context) (done bool, err error) {
515+
if err := wait.PollUntilContextCancel(ctx, time.Millisecond*200, true, func(ctx context.Context) (done bool, err error) {
516516
log.V(4).Info("Reading CertificateRequest from lister cache")
517517
updatedReq, err := m.lister.CertificateRequests(req.Namespace).Get(req.Name)
518518
if apierrors.IsNotFound(err) {
@@ -582,7 +582,7 @@ func (m *Manager) handleRequest(ctx context.Context, volumeID string, meta metad
582582
req = updatedReq
583583
return true, nil
584584
}); err != nil {
585-
if errors.Is(err, wait.ErrWaitTimeout) {
585+
if wait.Interrupted(err) {
586586
// try and return a more helpful error message than "timed out waiting for the condition"
587587
return fmt.Errorf("waiting for request: %s", lastFailureReason)
588588
}
@@ -720,7 +720,7 @@ func (m *Manager) submitRequest(ctx context.Context, meta metadata.Metadata, csr
720720
// This ensures callers that read from the lister/cache do not enter a confused state
721721
// where the CertificateRequest does not exist after calling submitRequest due to
722722
// cache timing issues.
723-
wait.PollUntil(time.Millisecond*50, func() (bool, error) {
723+
wait.PollUntilContextCancel(ctx, time.Millisecond*50, true, func(ctx context.Context) (bool, error) {
724724
_, err := m.lister.CertificateRequests(csrBundle.Namespace).Get(req.Name)
725725
if apierrors.IsNotFound(err) {
726726
return false, nil
@@ -729,7 +729,7 @@ func (m *Manager) submitRequest(ctx context.Context, meta metadata.Metadata, csr
729729
return false, err
730730
}
731731
return true, nil
732-
}, ctx.Done())
732+
})
733733

734734
return req, nil
735735
}
@@ -835,7 +835,7 @@ func (m *Manager) startRenewalRoutine(volumeID string) (started bool) {
835835
}
836836
return true, nil
837837
}); err != nil {
838-
if errors.Is(err, wait.ErrWaitTimeout) || errors.Is(err, context.DeadlineExceeded) {
838+
if wait.Interrupted(err) {
839839
continue
840840
}
841841
// this should never happen as the function above never actually returns errors

‎manager/manager_test.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ import (
3030
)
3131

3232
func TestManager_ManageVolumeImmediate_issueOnceAndSucceed(t *testing.T) {
33-
ctx := context.Background()
33+
ctx, cancel := context.WithCancel(context.Background())
34+
defer cancel()
3435

3536
opts := newDefaultTestOptions(t)
3637
m, err := NewManager(opts)
@@ -39,9 +40,7 @@ func TestManager_ManageVolumeImmediate_issueOnceAndSucceed(t *testing.T) {
3940
}
4041

4142
// Setup a goroutine to issue one CertificateRequest
42-
stopCh := make(chan struct{})
43-
go testutil.IssueOneRequest(t, opts.Client, defaultTestNamespace, stopCh, selfSignedExampleCertificate, []byte("ca bytes"))
44-
defer close(stopCh)
43+
go testutil.IssueOneRequest(ctx, t, opts.Client, defaultTestNamespace, selfSignedExampleCertificate, []byte("ca bytes"))
4544

4645
// Register a new volume with the metadata store
4746
store := opts.MetadataReader.(storage.Interface)
@@ -200,7 +199,7 @@ func TestManager_ResumesManagementOfExistingVolumes(t *testing.T) {
200199

201200
m.requestNameGenerator = func() string { return "certificaterequest-name" }
202201
// Automatically issue the request once created
203-
go testutil.IssueOneRequest(t, opts.Client, defaultTestNamespace, ctx.Done(), selfSignedExampleCertificate, []byte("ca bytes"))
202+
go testutil.IssueOneRequest(ctx, t, opts.Client, defaultTestNamespace, selfSignedExampleCertificate, []byte("ca bytes"))
204203

205204
// Register a new volume with the metadata store
206205
meta := metadata.Metadata{
@@ -264,7 +263,7 @@ func TestManager_ManageVolume_beginsManagingAndProceedsIfNotReady(t *testing.T)
264263
t.Errorf("expected management to have started, but it did not")
265264
}
266265

267-
if err := wait.PollUntilWithContext(ctx, time.Millisecond*500, func(ctx context.Context) (done bool, err error) {
266+
if err := wait.PollUntilContextCancel(ctx, time.Millisecond*500, true, func(ctx context.Context) (done bool, err error) {
268267
reqs, err := opts.Client.CertmanagerV1().CertificateRequests("").List(ctx, metav1.ListOptions{})
269268
if err != nil {
270269
return false, err
@@ -423,7 +422,7 @@ func TestManager_cleanupStaleRequests(t *testing.T) {
423422
}
424423

425424
// make sure client cache is in sync
426-
if err := wait.PollUntilWithContext(ctx, 5*time.Millisecond, func(context.Context) (done bool, err error) {
425+
if err := wait.PollUntilContextCancel(ctx, 5*time.Millisecond, false, func(context.Context) (done bool, err error) {
427426
list, err := m.client.CertmanagerV1().CertificateRequests(defaultTestNamespace).List(ctx, metav1.ListOptions{})
428427
if err != nil {
429428
return false, err

‎test/integration/issuance_test.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ vnEIALrtIClFU6D/mTU5wyHhN29llwfjUgJrmYWqoWTZSiwGS6YmZpry
6262
-----END CERTIFICATE-----`)
6363

6464
func TestIssuesCertificate(t *testing.T) {
65+
ctx, cancel := context.WithCancel(context.Background())
66+
defer cancel()
67+
6568
store := storage.NewMemoryFS()
6669
clock := fakeclock.NewFakeClock(time.Now())
6770
opts, cl, stop := testdriver.Run(t, testdriver.Options{
@@ -90,16 +93,14 @@ func TestIssuesCertificate(t *testing.T) {
9093
})
9194
defer stop()
9295

93-
stopCh := make(chan struct{})
94-
go testutil.IssueOneRequest(t, opts.Client, "certificaterequest-namespace", stopCh, selfSignedExampleCertificate, []byte("ca bytes"))
95-
defer close(stopCh)
96+
go testutil.IssueOneRequest(ctx, t, opts.Client, "certificaterequest-namespace", selfSignedExampleCertificate, []byte("ca bytes"))
9697

9798
tmpDir, err := os.MkdirTemp("", "*")
9899
if err != nil {
99100
t.Fatal(err)
100101
}
101102
defer os.RemoveAll(tmpDir)
102-
_, err = cl.NodePublishVolume(context.TODO(), &csi.NodePublishVolumeRequest{
103+
_, err = cl.NodePublishVolume(ctx, &csi.NodePublishVolumeRequest{
103104
VolumeId: "test-vol",
104105
VolumeContext: map[string]string{
105106
"csi.storage.k8s.io/ephemeral": "true",
@@ -126,6 +127,9 @@ func TestIssuesCertificate(t *testing.T) {
126127
}
127128

128129
func TestManager_CleansUpOldRequests(t *testing.T) {
130+
ctx, cancel := context.WithCancel(context.Background())
131+
defer cancel()
132+
129133
store := storage.NewMemoryFS()
130134
clock := fakeclock.NewFakeClock(time.Now())
131135

@@ -165,23 +169,21 @@ func TestManager_CleansUpOldRequests(t *testing.T) {
165169
Spec: cmapi.CertificateRequestSpec{},
166170
Status: cmapi.CertificateRequestStatus{},
167171
}
168-
cr, err := opts.Client.CertmanagerV1().CertificateRequests(cr.Namespace).Create(context.TODO(), cr, metav1.CreateOptions{})
172+
cr, err := opts.Client.CertmanagerV1().CertificateRequests(cr.Namespace).Create(ctx, cr, metav1.CreateOptions{})
169173
if err != nil {
170174
t.Fatal(err)
171175
}
172176

173177
// Set up a goroutine that automatically issues all CertificateRequests
174-
stopCh := make(chan struct{})
175-
go testutil.IssueAllRequests(t, opts.Client, "testns", stopCh, selfSignedExampleCertificate, []byte("ca bytes"))
176-
defer close(stopCh)
178+
go testutil.IssueAllRequests(ctx, t, opts.Client, "testns", selfSignedExampleCertificate, []byte("ca bytes"))
177179

178180
// Call NodePublishVolume
179181
tmpDir, err := os.MkdirTemp("", "*")
180182
if err != nil {
181183
t.Fatal(err)
182184
}
183185
defer os.RemoveAll(tmpDir)
184-
_, err = cl.NodePublishVolume(context.TODO(), &csi.NodePublishVolumeRequest{
186+
_, err = cl.NodePublishVolume(ctx, &csi.NodePublishVolumeRequest{
185187
VolumeId: "volume-id",
186188
VolumeContext: map[string]string{
187189
"csi.storage.k8s.io/ephemeral": "true",
@@ -195,12 +197,12 @@ func TestManager_CleansUpOldRequests(t *testing.T) {
195197
t.Fatal(err)
196198
}
197199

198-
_, err = opts.Client.CertmanagerV1().CertificateRequests("testns").Get(context.TODO(), "test-cr", metav1.GetOptions{})
200+
_, err = opts.Client.CertmanagerV1().CertificateRequests("testns").Get(ctx, "test-cr", metav1.GetOptions{})
199201
if !apierrors.IsNotFound(err) {
200202
t.Error("Expected 'test-cr' resource to be deleted but it still exists")
201203
}
202204

203-
all, err := opts.Client.CertmanagerV1().CertificateRequests("testns").List(context.TODO(), metav1.ListOptions{})
205+
all, err := opts.Client.CertmanagerV1().CertificateRequests("testns").List(ctx, metav1.ListOptions{})
204206
if err != nil {
205207
t.Fatal(err)
206208
}

‎test/integration/ready_to_request_test.go

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ import (
4343
// Tests to ensure that if a certificate is immediately ready to be requested when NodePublishVolume
4444
// is called, the call will be blocking until the volume is actually ready.
4545
func Test_PublishCallBlocksIfReadyToRequestDespiteContinueOnNotReadyTrue(t *testing.T) {
46+
ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
47+
defer cancel()
48+
4649
store := storage.NewMemoryFS()
4750
clock := fakeclock.NewFakeClock(time.Now())
4851

@@ -75,17 +78,13 @@ func Test_PublishCallBlocksIfReadyToRequestDespiteContinueOnNotReadyTrue(t *test
7578
defer stop()
7679

7780
// Setup a routine to issue/sign the request IF it is created
78-
stopCh := make(chan struct{})
79-
go testutil.IssueAllRequests(t, opts.Client, "certificaterequest-namespace", stopCh, selfSignedExampleCertificate, []byte("ca bytes"))
80-
defer close(stopCh)
81+
go testutil.IssueAllRequests(ctx, t, opts.Client, "certificaterequest-namespace", selfSignedExampleCertificate, []byte("ca bytes"))
8182

8283
tmpDir, err := os.MkdirTemp("", "*")
8384
if err != nil {
8485
t.Fatal(err)
8586
}
8687
defer os.RemoveAll(tmpDir)
87-
ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
88-
defer cancel()
8988
// This call will block until an issuance is successfully completed.
9089
_, err = cl.NodePublishVolume(ctx, &csi.NodePublishVolumeRequest{
9190
VolumeId: "test-vol",
@@ -116,6 +115,9 @@ func Test_PublishCallBlocksIfReadyToRequestDespiteContinueOnNotReadyTrue(t *test
116115
}
117116

118117
func Test_CompletesIfNotReadyToRequest_ContinueOnNotReadyEnabled(t *testing.T) {
118+
ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
119+
defer cancel()
120+
119121
store := storage.NewMemoryFS()
120122
clock := fakeclock.NewFakeClock(time.Now())
121123

@@ -156,17 +158,13 @@ func Test_CompletesIfNotReadyToRequest_ContinueOnNotReadyEnabled(t *testing.T) {
156158
defer stop()
157159

158160
// Setup a routine to issue/sign the request IF it is created
159-
stopCh := make(chan struct{})
160-
go testutil.IssueAllRequests(t, opts.Client, "certificaterequest-namespace", stopCh, selfSignedExampleCertificate, []byte("ca bytes"))
161-
defer close(stopCh)
161+
go testutil.IssueAllRequests(ctx, t, opts.Client, "certificaterequest-namespace", selfSignedExampleCertificate, []byte("ca bytes"))
162162

163163
tmpDir, err := os.MkdirTemp("", "*")
164164
if err != nil {
165165
t.Fatal(err)
166166
}
167167
defer os.RemoveAll(tmpDir)
168-
ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
169-
defer cancel()
170168
_, err = cl.NodePublishVolume(ctx, &csi.NodePublishVolumeRequest{
171169
VolumeId: "test-vol",
172170
VolumeContext: map[string]string{
@@ -181,7 +179,7 @@ func Test_CompletesIfNotReadyToRequest_ContinueOnNotReadyEnabled(t *testing.T) {
181179
t.Fatal(err)
182180
}
183181

184-
if err := wait.PollUntil(time.Second, func() (done bool, err error) {
182+
if err := wait.PollUntilContextCancel(ctx, time.Second, true, func(ctx context.Context) (done bool, err error) {
185183
files, err := store.ReadFiles("test-vol")
186184
if errors.Is(err, storage.ErrNotFound) || len(files) <= 1 {
187185
return false, nil
@@ -196,12 +194,15 @@ func Test_CompletesIfNotReadyToRequest_ContinueOnNotReadyEnabled(t *testing.T) {
196194
return false, fmt.Errorf("unexpected certificate data: %v", files["cert"])
197195
}
198196
return true, nil
199-
}, ctx.Done()); err != nil {
197+
}); err != nil {
200198
t.Error(err)
201199
}
202200
}
203201

204202
func TestFailsIfNotReadyToRequest_ContinueOnNotReadyDisabled(t *testing.T) {
203+
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
204+
defer cancel()
205+
205206
store := storage.NewMemoryFS()
206207
clock := fakeclock.NewFakeClock(time.Now())
207208

@@ -236,16 +237,12 @@ func TestFailsIfNotReadyToRequest_ContinueOnNotReadyDisabled(t *testing.T) {
236237
defer stop()
237238

238239
// Setup a routine to issue/sign the request IF it is created
239-
stopCh := make(chan struct{})
240-
go testutil.IssueAllRequests(t, opts.Client, "certificaterequest-namespace", stopCh, selfSignedExampleCertificate, []byte("ca bytes"))
241-
defer close(stopCh)
240+
go testutil.IssueAllRequests(ctx, t, opts.Client, "certificaterequest-namespace", selfSignedExampleCertificate, []byte("ca bytes"))
242241
tmpDir, err := os.MkdirTemp("", "*")
243242
if err != nil {
244243
t.Fatal(err)
245244
}
246245
defer os.RemoveAll(tmpDir)
247-
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
248-
defer cancel()
249246
_, err = cl.NodePublishVolume(ctx, &csi.NodePublishVolumeRequest{
250247
VolumeId: "test-vol",
251248
VolumeContext: map[string]string{
@@ -265,13 +262,13 @@ func TestFailsIfNotReadyToRequest_ContinueOnNotReadyDisabled(t *testing.T) {
265262
// being cleaned up of the persisted metadata file.
266263
ctx, cancel2 := context.WithTimeout(context.Background(), time.Second)
267264
defer cancel2()
268-
if err := wait.PollUntil(time.Millisecond*100, func() (bool, error) {
265+
if err := wait.PollUntilContextCancel(ctx, time.Millisecond*100, true, func(ctx context.Context) (bool, error) {
269266
_, err := store.ReadFiles("test-vol")
270267
if err != storage.ErrNotFound {
271268
return false, nil
272269
}
273270
return true, nil
274-
}, ctx.Done()); err != nil {
271+
}); err != nil {
275272
t.Errorf("failed to wait for storage backend to return NotFound: %v", err)
276273
}
277274
}

‎test/integration/resume_request_test.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ const (
4545
)
4646

4747
func testResumesExistingRequest(t *testing.T, issueBeforeCall WhenToCallIssue) {
48+
// create a root, non-expiring context
49+
ctx, cancel := context.WithCancel(context.Background())
50+
t.Cleanup(cancel)
51+
4852
store := storage.NewMemoryFS()
4953
ns := "certificaterequest-namespace"
5054
clock := fakeclock.NewFakeClock(time.Now())
@@ -76,9 +80,6 @@ func testResumesExistingRequest(t *testing.T, issueBeforeCall WhenToCallIssue) {
7680

7781
tmpDir := t.TempDir()
7882

79-
// create a root, non-expiring context
80-
ctx := context.Background()
81-
8283
// We are going to submit this request multiple times, so lets just write it out once
8384
nodePublishVolumeRequest := &csi.NodePublishVolumeRequest{
8485
VolumeId: "test-vol",
@@ -120,17 +121,15 @@ func testResumesExistingRequest(t *testing.T, issueBeforeCall WhenToCallIssue) {
120121
// ensure the same certificaterequest object still exists
121122
ensureOneRequestExists(ctx, t, opts.Client, ns, existingRequestUID)
122123

123-
stopCh := make(chan struct{})
124-
defer close(stopCh)
125124
if issueBeforeCall {
126125
// we don't run this in a goroutine so we can be sure the certificaterequest is completed BEFORE the issue loop is entered
127-
testutil.IssueOneRequest(t, opts.Client, "certificaterequest-namespace", stopCh, selfSignedExampleCertificate, []byte("ca bytes"))
126+
testutil.IssueOneRequest(ctx, t, opts.Client, "certificaterequest-namespace", selfSignedExampleCertificate, []byte("ca bytes"))
128127
} else {
129128
go func() {
130129
// allow 500ms before actually issuing the request so we can be sure we're within the issue() function call
131130
// when the certificaterequest is finally completed
132131
time.Sleep(time.Millisecond * 500)
133-
testutil.IssueOneRequest(t, opts.Client, "certificaterequest-namespace", stopCh, selfSignedExampleCertificate, []byte("ca bytes"))
132+
testutil.IssueOneRequest(ctx, t, opts.Client, "certificaterequest-namespace", selfSignedExampleCertificate, []byte("ca bytes"))
134133
}()
135134
}
136135

0 commit comments

Comments
 (0)