Skip to content

Commit 36de77c

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#125142 from TommyStarK/unit-tests/kubelet-util-manager
kubelet/util/manager: small cleanup and start replacing deprecated functions wait.Poll/wait.PollImmediate
2 parents 9125473 + 2ed556a commit 36de77c

File tree

2 files changed

+55
-17
lines changed

2 files changed

+55
-17
lines changed

‎pkg/kubelet/util/manager/cache_based_manager_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,31 @@ func TestCacheInvalidation(t *testing.T) {
437437
fakeClient.ClearActions()
438438
}
439439

440+
func TestResourceContentExpired(t *testing.T) {
441+
fakeClient := &fake.Clientset{}
442+
fakeClock := testingclock.NewFakeClock(time.Now())
443+
store := newSecretStore(fakeClient, fakeClock, noObjectTTL, time.Minute)
444+
manager := newCacheBasedSecretManager(store)
445+
446+
// Create a pod with some secrets.
447+
s1 := secretsToAttach{
448+
imagePullSecretNames: []string{"s1"},
449+
containerEnvSecrets: []envSecrets{
450+
{envVarNames: []string{"s1"}, envFromNames: []string{"s10"}},
451+
{envVarNames: []string{"s2"}},
452+
},
453+
}
454+
455+
// emulate a requested resource content that has expired from the server
456+
manager.RegisterPod(podWithSecrets("dummy-ns", "dummy-name", s1))
457+
fakeClient.PrependReactor("get", "secrets", func(action core.Action) (bool, runtime.Object, error) {
458+
return true, &v1.Secret{}, apierrors.NewResourceExpired("expired")
459+
})
460+
// should fail to fetch the latest object
461+
_, err := manager.GetObject("dummy-ns", "s1")
462+
assert.Error(t, err)
463+
}
464+
440465
func TestRegisterIdempotence(t *testing.T) {
441466
fakeClient := &fake.Clientset{}
442467
fakeClock := testingclock.NewFakeClock(time.Now())

‎pkg/kubelet/util/manager/watch_based_manager_test.go‎

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838

3939
corev1 "k8s.io/kubernetes/pkg/apis/core/v1"
4040

41+
"k8s.io/kubernetes/test/utils/ktesting"
4142
"k8s.io/utils/clock"
4243
testingclock "k8s.io/utils/clock/testing"
4344

@@ -105,7 +106,7 @@ func TestSecretCache(t *testing.T) {
105106
ObjectMeta: metav1.ObjectMeta{Name: "name", Namespace: "ns", ResourceVersion: "125"},
106107
}
107108
fakeWatch.Add(secret)
108-
getFn := func() (bool, error) {
109+
getFn := func(_ context.Context) (bool, error) {
109110
object, err := store.Get("ns", "name")
110111
if err != nil {
111112
if apierrors.IsNotFound(err) {
@@ -119,13 +120,15 @@ func TestSecretCache(t *testing.T) {
119120
}
120121
return true, nil
121122
}
122-
if err := wait.PollImmediate(10*time.Millisecond, time.Second, getFn); err != nil {
123+
124+
tCtx := ktesting.Init(t)
125+
if err := wait.PollUntilContextCancel(tCtx, 10*time.Millisecond, true, getFn); err != nil {
123126
t.Errorf("unexpected error: %v", err)
124127
}
125128

126129
// Eventually we should observer secret deletion.
127130
fakeWatch.Delete(secret)
128-
getFn = func() (bool, error) {
131+
getFn = func(_ context.Context) (bool, error) {
129132
_, err := store.Get("ns", "name")
130133
if err != nil {
131134
if apierrors.IsNotFound(err) {
@@ -135,7 +138,9 @@ func TestSecretCache(t *testing.T) {
135138
}
136139
return false, nil
137140
}
138-
if err := wait.PollImmediate(10*time.Millisecond, time.Second, getFn); err != nil {
141+
deadlineCtx, deadlineCancel := context.WithTimeout(tCtx, time.Second)
142+
defer deadlineCancel()
143+
if err := wait.PollUntilContextCancel(deadlineCtx, 10*time.Millisecond, true, getFn); err != nil {
139144
t.Errorf("unexpected error: %v", err)
140145
}
141146

@@ -166,7 +171,7 @@ func TestSecretCacheMultipleRegistrations(t *testing.T) {
166171

167172
store.AddReference("ns", "name", "pod")
168173
// This should trigger List and Watch actions eventually.
169-
actionsFn := func() (bool, error) {
174+
actionsFn := func(_ context.Context) (bool, error) {
170175
actions := fakeClient.Actions()
171176
if len(actions) > 2 {
172177
return false, fmt.Errorf("too many actions: %v", actions)
@@ -179,7 +184,8 @@ func TestSecretCacheMultipleRegistrations(t *testing.T) {
179184
}
180185
return true, nil
181186
}
182-
if err := wait.PollImmediate(10*time.Millisecond, time.Second, actionsFn); err != nil {
187+
tCtx := ktesting.Init(t)
188+
if err := wait.PollUntilContextCancel(tCtx, 10*time.Millisecond, true, actionsFn); err != nil {
183189
t.Errorf("unexpected error: %v", err)
184190
}
185191

@@ -271,7 +277,7 @@ func TestImmutableSecretStopsTheReflector(t *testing.T) {
271277
store := newSecretCache(fakeClient, fakeClock, time.Minute)
272278

273279
key := objectKey{namespace: "ns", name: "name"}
274-
itemExists := func() (bool, error) {
280+
itemExists := func(_ context.Context) (bool, error) {
275281
store.lock.Lock()
276282
defer store.lock.Unlock()
277283
_, ok := store.items[key]
@@ -289,7 +295,8 @@ func TestImmutableSecretStopsTheReflector(t *testing.T) {
289295

290296
// AddReference should start reflector.
291297
store.AddReference("ns", "name", "pod")
292-
if err := wait.Poll(10*time.Millisecond, time.Second, itemExists); err != nil {
298+
tCtx := ktesting.Init(t)
299+
if err := wait.PollUntilContextCancel(tCtx, 10*time.Millisecond, false, itemExists); err != nil {
293300
t.Errorf("item wasn't added to cache")
294301
}
295302

@@ -309,7 +316,7 @@ func TestImmutableSecretStopsTheReflector(t *testing.T) {
309316
fakeWatch.Add(tc.eventual)
310317

311318
// Eventually Get should return that secret.
312-
getFn := func() (bool, error) {
319+
getFn := func(_ context.Context) (bool, error) {
313320
object, err := store.Get("ns", "name")
314321
if err != nil {
315322
if apierrors.IsNotFound(err) {
@@ -320,7 +327,9 @@ func TestImmutableSecretStopsTheReflector(t *testing.T) {
320327
secret := object.(*v1.Secret)
321328
return apiequality.Semantic.DeepEqual(tc.eventual, secret), nil
322329
}
323-
if err := wait.PollImmediate(10*time.Millisecond, time.Second, getFn); err != nil {
330+
deadlineCtx, deadlineCancel := context.WithTimeout(tCtx, time.Second)
331+
defer deadlineCancel()
332+
if err := wait.PollUntilContextCancel(deadlineCtx, 10*time.Millisecond, true, getFn); err != nil {
324333
t.Errorf("unexpected error: %v", err)
325334
}
326335

@@ -358,7 +367,7 @@ func TestMaxIdleTimeStopsTheReflector(t *testing.T) {
358367
store := newSecretCache(fakeClient, fakeClock, time.Minute)
359368

360369
key := objectKey{namespace: "ns", name: "name"}
361-
itemExists := func() (bool, error) {
370+
itemExists := func(_ context.Context) (bool, error) {
362371
store.lock.Lock()
363372
defer store.lock.Unlock()
364373
_, ok := store.items[key]
@@ -377,7 +386,8 @@ func TestMaxIdleTimeStopsTheReflector(t *testing.T) {
377386

378387
// AddReference should start reflector.
379388
store.AddReference("ns", "name", "pod")
380-
if err := wait.Poll(10*time.Millisecond, 10*time.Second, itemExists); err != nil {
389+
tCtx := ktesting.Init(t)
390+
if err := wait.PollUntilContextCancel(tCtx, 10*time.Millisecond, false, itemExists); err != nil {
381391
t.Errorf("item wasn't added to cache")
382392
}
383393

@@ -440,7 +450,7 @@ func TestReflectorNotStoppedOnSlowInitialization(t *testing.T) {
440450
store := newSecretCache(fakeClient, fakeClock, time.Minute)
441451

442452
key := objectKey{namespace: "ns", name: "name"}
443-
itemExists := func() (bool, error) {
453+
itemExists := func(_ context.Context) (bool, error) {
444454
store.lock.Lock()
445455
defer store.lock.Unlock()
446456
_, ok := store.items[key]
@@ -457,7 +467,7 @@ func TestReflectorNotStoppedOnSlowInitialization(t *testing.T) {
457467
return !item.stopped
458468
}
459469

460-
reflectorInitialized := func() (bool, error) {
470+
reflectorInitialized := func(_ context.Context) (bool, error) {
461471
store.lock.Lock()
462472
defer store.lock.Unlock()
463473
item := store.items[key]
@@ -469,7 +479,8 @@ func TestReflectorNotStoppedOnSlowInitialization(t *testing.T) {
469479

470480
// AddReference should start reflector.
471481
store.AddReference("ns", "name", "pod")
472-
if err := wait.Poll(10*time.Millisecond, 10*time.Second, itemExists); err != nil {
482+
tCtx := ktesting.Init(t)
483+
if err := wait.PollUntilContextCancel(tCtx, 10*time.Millisecond, false, itemExists); err != nil {
473484
t.Errorf("item wasn't added to cache")
474485
}
475486

@@ -479,7 +490,7 @@ func TestReflectorNotStoppedOnSlowInitialization(t *testing.T) {
479490
// Reflector didn't yet initialize, so it shouldn't be stopped.
480491
// However, Get should still be failing.
481492
assert.True(t, reflectorRunning())
482-
initialized, _ := reflectorInitialized()
493+
initialized, _ := reflectorInitialized(tCtx)
483494
assert.False(t, initialized)
484495
_, err := store.Get("ns", "name")
485496
if err == nil || !strings.Contains(err.Error(), "failed to sync") {
@@ -488,7 +499,9 @@ func TestReflectorNotStoppedOnSlowInitialization(t *testing.T) {
488499

489500
// Initialization should successfully finish.
490501
fakeClock.Step(30 * time.Second)
491-
if err := wait.Poll(10*time.Millisecond, time.Second, reflectorInitialized); err != nil {
502+
deadlineCtx, deadlineCancel := context.WithTimeout(tCtx, time.Second)
503+
defer deadlineCancel()
504+
if err := wait.PollUntilContextCancel(deadlineCtx, 10*time.Millisecond, false, reflectorInitialized); err != nil {
492505
t.Errorf("reflector didn't iniailize correctly")
493506
}
494507

0 commit comments

Comments
 (0)