Skip to content

Commit 26cb76f

Browse files
ullbergmullbergm
committed
refactor(tests): Replace client variable with cl in cleanup job tests for consistency
Signed-off-by: Magnus Ullberg <magnus@ullberg.us>
1 parent c7267cd commit 26cb76f

2 files changed

Lines changed: 24 additions & 22 deletions

File tree

‎pkg/controllers/lease_controller_test.go‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ func defaultAnn() Annotations {
4646
}
4747
}
4848

49+
const testOnDeleteJob = "object-lease-controller.ullberg.io/on-delete-job"
50+
4951
type stubMgr struct{ c client.Client }
5052

5153
func (s stubMgr) GetClient() client.Client { return s.c }
@@ -481,7 +483,7 @@ func TestHandleExpired_CreatesCleanupJob_FireAndForget(t *testing.T) {
481483
obj.SetAnnotations(map[string]string{
482484
defaultAnn().TTL: "1s",
483485
defaultAnn().LeaseStart: time.Now().UTC().Add(-time.Hour).Format(time.RFC3339),
484-
"object-lease-controller.ullberg.io/on-delete-job": "scripts-cm/cleanup.sh",
486+
testOnDeleteJob: "scripts-cm/cleanup.sh",
485487
})
486488

487489
// Need a scheme that understands Jobs
@@ -494,7 +496,7 @@ func TestHandleExpired_CreatesCleanupJob_FireAndForget(t *testing.T) {
494496
// rebuild client with Job types registered
495497
cl2 := fake.NewClientBuilder().WithScheme(scheme).WithObjects(obj).Build()
496498
r.Client = cl2
497-
r.Annotations.OnDeleteJob = "object-lease-controller.ullberg.io/on-delete-job"
499+
r.Annotations.OnDeleteJob = testOnDeleteJob
498500
reg := withIsolatedRegistry(t)
499501
r.Metrics = ometrics.NewLeaseMetrics(gvk)
500502
r.Recorder = record.NewFakeRecorder(10)
@@ -507,7 +509,7 @@ func TestHandleExpired_CreatesCleanupJob_FireAndForget(t *testing.T) {
507509

508510
// job should exist in client - search by label
509511
jl := &batchv1.JobList{}
510-
if err := r.Client.List(context.Background(), jl, client.InNamespace("default")); err != nil {
512+
if err := r.List(context.Background(), jl, client.InNamespace("default")); err != nil {
511513
t.Fatalf("list jobs failed: %v", err)
512514
}
513515
if len(jl.Items) == 0 {
@@ -539,7 +541,7 @@ func TestHandleExpired_CleanupJobCreateFails(t *testing.T) {
539541
obj.SetAnnotations(map[string]string{
540542
defaultAnn().TTL: "1s",
541543
defaultAnn().LeaseStart: time.Now().UTC().Add(-time.Hour).Format(time.RFC3339),
542-
"object-lease-controller.ullberg.io/on-delete-job": "scripts-cm/cleanup.sh",
544+
testOnDeleteJob: "scripts-cm/cleanup.sh",
543545
})
544546

545547
scheme := runtime.NewScheme()
@@ -551,7 +553,7 @@ func TestHandleExpired_CleanupJobCreateFails(t *testing.T) {
551553
// client that returns error on Create
552554

553555
r, _, _ := newWatcher(t, gvk, obj)
554-
r.Annotations.OnDeleteJob = "object-lease-controller.ullberg.io/on-delete-job"
556+
r.Annotations.OnDeleteJob = testOnDeleteJob
555557
r.Client = &failCreateClient{Client: base}
556558
r.Recorder = record.NewFakeRecorder(10)
557559

@@ -601,8 +603,8 @@ func TestHandleExpired_CleanupJobWaitFailure(t *testing.T) {
601603
obj.SetAnnotations(map[string]string{
602604
defaultAnn().TTL: "1s",
603605
defaultAnn().LeaseStart: time.Now().UTC().Add(-time.Hour).Format(time.RFC3339),
604-
"object-lease-controller.ullberg.io/on-delete-job": "scripts-cm/cleanup.sh",
605-
"object-lease-controller.ullberg.io/job-wait": "true",
606+
testOnDeleteJob: "scripts-cm/cleanup.sh",
607+
"object-lease-controller.ullberg.io/job-wait": "true",
606608
})
607609

608610
scheme := runtime.NewScheme()
@@ -611,7 +613,7 @@ func TestHandleExpired_CleanupJobWaitFailure(t *testing.T) {
611613
base := fake.NewClientBuilder().WithScheme(scheme).WithObjects(obj).Build()
612614

613615
r, _, _ := newWatcher(t, gvk, obj)
614-
r.Annotations.OnDeleteJob = "object-lease-controller.ullberg.io/on-delete-job"
616+
r.Annotations.OnDeleteJob = testOnDeleteJob
615617
r.Annotations.JobWait = "object-lease-controller.ullberg.io/job-wait"
616618
r.Client = &failCompleteClient{Client: base}
617619
r.Recorder = record.NewFakeRecorder(10)
@@ -662,11 +664,11 @@ func TestHandleExpired_InvalidCleanupConfigEmitsEvent(t *testing.T) {
662664
obj.SetAnnotations(map[string]string{
663665
defaultAnn().TTL: "1s",
664666
defaultAnn().LeaseStart: time.Now().UTC().Add(-time.Hour).Format(time.RFC3339),
665-
"object-lease-controller.ullberg.io/on-delete-job": "missingSlash",
667+
testOnDeleteJob: "missingSlash",
666668
})
667669

668670
r, _, _ := newWatcher(t, gvk, obj)
669-
r.Annotations.OnDeleteJob = "object-lease-controller.ullberg.io/on-delete-job"
671+
r.Annotations.OnDeleteJob = testOnDeleteJob
670672
r.Recorder = record.NewFakeRecorder(10)
671673

672674
_, err := r.handleExpired(context.Background(), obj, time.Now().UTC())
@@ -698,7 +700,7 @@ func TestExecuteCleanupJob_LeaseStartFallback(t *testing.T) {
698700
obj.SetAnnotations(map[string]string{
699701
defaultAnn().TTL: "1s",
700702
defaultAnn().LeaseStart: "not-a-time",
701-
"object-lease-controller.ullberg.io/on-delete-job": "scripts-cm/cleanup.sh",
703+
testOnDeleteJob: "scripts-cm/cleanup.sh",
702704
})
703705

704706
scheme := runtime.NewScheme()
@@ -707,7 +709,7 @@ func TestExecuteCleanupJob_LeaseStartFallback(t *testing.T) {
707709
base := fake.NewClientBuilder().WithScheme(scheme).WithObjects(obj).Build()
708710

709711
r, _, _ := newWatcher(t, gvk, obj)
710-
r.Annotations.OnDeleteJob = "object-lease-controller.ullberg.io/on-delete-job"
712+
r.Annotations.OnDeleteJob = testOnDeleteJob
711713
r.Client = base
712714
r.Recorder = record.NewFakeRecorder(10)
713715

@@ -733,7 +735,7 @@ func TestExecuteCleanupJob_LeaseStartFallback(t *testing.T) {
733735

734736
// find created job and check LEASE_STARTED_AT env is RFC3339 (approx now)
735737
jl := &batchv1.JobList{}
736-
if err := r.Client.List(context.Background(), jl, client.InNamespace("default")); err != nil {
738+
if err := r.List(context.Background(), jl, client.InNamespace("default")); err != nil {
737739
t.Fatalf("list jobs failed: %v", err)
738740
}
739741
if len(jl.Items) == 0 {
@@ -764,8 +766,8 @@ func TestHandleExpired_CleanupJobWaitSuccess(t *testing.T) {
764766
obj.SetAnnotations(map[string]string{
765767
defaultAnn().TTL: "1s",
766768
defaultAnn().LeaseStart: time.Now().UTC().Add(-time.Hour).Format(time.RFC3339),
767-
"object-lease-controller.ullberg.io/on-delete-job": "scripts-cm/cleanup.sh",
768-
"object-lease-controller.ullberg.io/job-wait": "true",
769+
testOnDeleteJob: "scripts-cm/cleanup.sh",
770+
"object-lease-controller.ullberg.io/job-wait": "true",
769771
})
770772

771773
scheme := runtime.NewScheme()
@@ -778,7 +780,7 @@ func TestHandleExpired_CleanupJobWaitSuccess(t *testing.T) {
778780
// use top-level completeCreateClient
779781

780782
r, _, _ := newWatcher(t, gvk, obj)
781-
r.Annotations.OnDeleteJob = "object-lease-controller.ullberg.io/on-delete-job"
783+
r.Annotations.OnDeleteJob = testOnDeleteJob
782784
r.Annotations.JobWait = "object-lease-controller.ullberg.io/job-wait"
783785

784786
r.Client = &completeCreateClient{Client: base}

‎pkg/util/cleanup_job_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ func TestCreateCleanupJob(t *testing.T) {
205205
_ = batchv1.AddToScheme(scheme)
206206
_ = corev1.AddToScheme(scheme)
207207

208-
client := fake.NewClientBuilder().WithScheme(scheme).Build()
208+
cl := fake.NewClientBuilder().WithScheme(scheme).Build()
209209

210210
obj := &unstructured.Unstructured{}
211211
obj.SetName("test-obj")
@@ -235,7 +235,7 @@ func TestCreateCleanupJob(t *testing.T) {
235235
leaseStart := time.Now().Add(-1 * time.Hour)
236236
leaseExpire := time.Now()
237237

238-
job, err := CreateCleanupJob(context.Background(), client, obj, gvk, config, leaseStart, leaseExpire)
238+
job, err := CreateCleanupJob(context.Background(), cl, obj, gvk, config, leaseStart, leaseExpire)
239239
if err != nil {
240240
t.Fatalf("Failed to create cleanup job: %v", err)
241241
}
@@ -321,7 +321,7 @@ func TestCreateCleanupJob_LabelsMarshalError(t *testing.T) {
321321
_ = batchv1.AddToScheme(scheme)
322322
_ = corev1.AddToScheme(scheme)
323323

324-
client := fake.NewClientBuilder().WithScheme(scheme).Build()
324+
cl := fake.NewClientBuilder().WithScheme(scheme).Build()
325325

326326
obj := &unstructured.Unstructured{}
327327
obj.SetName("test-obj")
@@ -336,7 +336,7 @@ func TestCreateCleanupJob_LabelsMarshalError(t *testing.T) {
336336
jsonMarshal = func(v interface{}) ([]byte, error) { return nil, fmt.Errorf("json error") }
337337
defer func() { jsonMarshal = old }()
338338

339-
if _, err := CreateCleanupJob(context.Background(), client, obj, gvk, cfg, time.Now(), time.Now()); err == nil {
339+
if _, err := CreateCleanupJob(context.Background(), cl, obj, gvk, cfg, time.Now(), time.Now()); err == nil {
340340
t.Fatalf("expected error creating job when labels JSON marshal fails")
341341
}
342342
}
@@ -346,7 +346,7 @@ func TestCreateCleanupJob_AnnotationsMarshalError(t *testing.T) {
346346
_ = batchv1.AddToScheme(scheme)
347347
_ = corev1.AddToScheme(scheme)
348348

349-
client := fake.NewClientBuilder().WithScheme(scheme).Build()
349+
cl := fake.NewClientBuilder().WithScheme(scheme).Build()
350350

351351
obj := &unstructured.Unstructured{}
352352
obj.SetName("test-obj")
@@ -368,7 +368,7 @@ func TestCreateCleanupJob_AnnotationsMarshalError(t *testing.T) {
368368
}
369369
defer func() { jsonMarshal = old }()
370370

371-
if _, err := CreateCleanupJob(context.Background(), client, obj, gvk, cfg, time.Now(), time.Now()); err == nil {
371+
if _, err := CreateCleanupJob(context.Background(), cl, obj, gvk, cfg, time.Now(), time.Now()); err == nil {
372372
t.Fatalf("expected error creating job when annotations JSON marshal fails")
373373
}
374374
}

0 commit comments

Comments
 (0)