Skip to content

Commit ca8b171

Browse files
sebrandon1sebrandon1
committed
Use resource kind in ApplyResource error messages for better debuggability
Derive the Kubernetes kind name from the concrete type parameter using reflect, replacing the generic 'resource' string. Error messages now read e.g. 'failed to apply Service' instead of 'failed to apply resource'. Also use client.ObjectKeyFromObject for consistent resource name formatting.
1 parent f8b7c81 commit ca8b171

13 files changed

Lines changed: 52 additions & 54 deletions

‎pkg/controller/common/applier.go‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package common
22

33
import (
44
"context"
5-
"fmt"
5+
"reflect"
66

77
"github.com/go-logr/logr"
88

@@ -30,27 +30,25 @@ func ApplyResource[T client.Object](
3030
fieldOwner string,
3131
hasChanged func(desired, existing T) bool,
3232
) error {
33-
resourceName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
34-
if resourceName == "/" {
35-
resourceName = desired.GetName()
36-
}
33+
key := client.ObjectKeyFromObject(desired)
34+
kind := reflect.TypeOf(desired).Elem().Name()
3735

38-
log.V(4).Info("reconciling resource", "name", resourceName)
36+
log.V(4).Info("reconciling resource", "kind", kind, "name", key)
3937

40-
exists, err := c.Exists(ctx, client.ObjectKeyFromObject(desired), existing)
38+
exists, err := c.Exists(ctx, key, existing)
4139
if err != nil {
42-
return FromClientError(err, "failed to check if resource %q exists", resourceName)
40+
return FromClientError(err, "failed to check if %s %q exists", kind, key)
4341
}
4442
if exists && !hasChanged(desired, existing) {
45-
log.V(4).Info("resource exists and is in desired state", "name", resourceName)
43+
log.V(4).Info("resource exists and is in desired state", "kind", kind, "name", key)
4644
return nil
4745
}
4846

49-
log.V(2).Info("applying resource to desired state", "name", resourceName)
47+
log.V(2).Info("applying resource to desired state", "kind", kind, "name", key)
5048
if err := c.Patch(ctx, desired, client.Apply, client.FieldOwner(fieldOwner), client.ForceOwnership); err != nil {
51-
return FromClientError(err, "failed to apply resource %q", resourceName)
49+
return FromClientError(err, "failed to apply %s %q", kind, key)
5250
}
5351

54-
recorder.Eventf(owner, corev1.EventTypeNormal, "Reconciled", "resource %s applied", resourceName)
52+
recorder.Eventf(owner, corev1.EventTypeNormal, "Reconciled", "%s %s applied", kind, key)
5553
return nil
5654
}

‎pkg/controller/istiocsr/certificates_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func TestCreateOrApplyCertificates(t *testing.T) {
3939
return true, nil
4040
})
4141
},
42-
wantErr: `failed to check if resource "istio-test-ns/istiod" exists: test client error`,
42+
wantErr: `failed to check if Certificate "istio-test-ns/istiod" exists: test client error`,
4343
},
4444
{
4545
name: "reconciliation of certificate fails while restoring to expected state",
@@ -63,7 +63,7 @@ func TestCreateOrApplyCertificates(t *testing.T) {
6363
})
6464
m.PatchReturns(errTestClient)
6565
},
66-
wantErr: `failed to apply resource "istio-test-ns/istiod": test client error`,
66+
wantErr: `failed to apply Certificate "istio-test-ns/istiod": test client error`,
6767
},
6868
{
6969
name: "reconciliation of certificate which already exists in expected state",
@@ -106,7 +106,7 @@ func TestCreateOrApplyCertificates(t *testing.T) {
106106
})
107107
m.PatchReturns(errTestClient)
108108
},
109-
wantErr: `failed to apply resource "istio-test-ns/istiod": test client error`,
109+
wantErr: `failed to apply Certificate "istio-test-ns/istiod": test client error`,
110110
},
111111
{
112112
name: "reconciliation of certificate when revisions are configured",

‎pkg/controller/istiocsr/install_instiocsr_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func TestReconcileIstioCSRDeployment(t *testing.T) {
9595
return nil
9696
})
9797
},
98-
wantErr: `failed to apply resource "istiocsr-test-ns/cert-manager-istio-csr": test client error`,
98+
wantErr: `failed to apply ServiceAccount "istiocsr-test-ns/cert-manager-istio-csr": test client error`,
9999
},
100100
{
101101
name: "istiocsr reconciliation fails with role apply error",
@@ -116,7 +116,7 @@ func TestReconcileIstioCSRDeployment(t *testing.T) {
116116
return nil
117117
})
118118
},
119-
wantErr: `failed to apply resource "istio-test-ns/cert-manager-istio-csr": test client error`,
119+
wantErr: `failed to apply Role "istio-test-ns/cert-manager-istio-csr": test client error`,
120120
},
121121
{
122122
name: "istiocsr reconciliation fails with certificate apply error",
@@ -137,7 +137,7 @@ func TestReconcileIstioCSRDeployment(t *testing.T) {
137137
return nil
138138
})
139139
},
140-
wantErr: `failed to apply resource "istio-test-ns/istiod": test client error`,
140+
wantErr: `failed to apply Certificate "istio-test-ns/istiod": test client error`,
141141
},
142142
}
143143

‎pkg/controller/istiocsr/rbacs_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) {
6767
return true, nil
6868
})
6969
},
70-
wantErr: `failed to check if resource "istio-test-ns/cert-manager-istio-csr" exists: test client error`,
70+
wantErr: `failed to check if Role "istio-test-ns/cert-manager-istio-csr" exists: test client error`,
7171
},
7272
{
7373
name: "rolebindings reconciliation fails while checking if exists",
@@ -80,7 +80,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) {
8080
return true, nil
8181
})
8282
},
83-
wantErr: `failed to check if resource "istio-test-ns/cert-manager-istio-csr" exists: test client error`,
83+
wantErr: `failed to check if RoleBinding "istio-test-ns/cert-manager-istio-csr" exists: test client error`,
8484
},
8585
{
8686
name: "role-leases reconciliation fails while checking if exists",
@@ -95,7 +95,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) {
9595
return true, nil
9696
})
9797
},
98-
wantErr: `failed to check if resource "istio-test-ns/cert-manager-istio-csr-leases" exists: test client error`,
98+
wantErr: `failed to check if Role "istio-test-ns/cert-manager-istio-csr-leases" exists: test client error`,
9999
},
100100
{
101101
name: "rolebindings-leases reconciliation fails while checking if exists",
@@ -110,7 +110,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) {
110110
return true, nil
111111
})
112112
},
113-
wantErr: `failed to check if resource "istio-test-ns/cert-manager-istio-csr-leases" exists: test client error`,
113+
wantErr: `failed to check if RoleBinding "istio-test-ns/cert-manager-istio-csr-leases" exists: test client error`,
114114
},
115115
{
116116
name: "clusterrolebindings reconciliation fails while listing existing resources",
@@ -472,7 +472,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) {
472472
return nil
473473
})
474474
},
475-
wantErr: `failed to apply resource "istio-test-ns/cert-manager-istio-csr": test client error`,
475+
wantErr: `failed to apply Role "istio-test-ns/cert-manager-istio-csr": test client error`,
476476
},
477477
{
478478
name: "role reconciliation creation fails",
@@ -492,7 +492,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) {
492492
return nil
493493
})
494494
},
495-
wantErr: `failed to apply resource "istio-test-ns/cert-manager-istio-csr": test client error`,
495+
wantErr: `failed to apply Role "istio-test-ns/cert-manager-istio-csr": test client error`,
496496
},
497497
{
498498
name: "role-leases reconciliation applying to desired state fails",
@@ -518,7 +518,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) {
518518
return nil
519519
})
520520
},
521-
wantErr: `failed to apply resource "istio-test-ns/cert-manager-istio-csr-leases": test client error`,
521+
wantErr: `failed to apply Role "istio-test-ns/cert-manager-istio-csr-leases": test client error`,
522522
},
523523
{
524524
name: "role-leases reconciliation creation fails",
@@ -542,7 +542,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) {
542542
return nil
543543
})
544544
},
545-
wantErr: `failed to apply resource "istio-test-ns/cert-manager-istio-csr-leases": test client error`,
545+
wantErr: `failed to apply Role "istio-test-ns/cert-manager-istio-csr-leases": test client error`,
546546
},
547547
{
548548
name: "rolebindings reconciliation applying to desired state fails",
@@ -564,7 +564,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) {
564564
return nil
565565
})
566566
},
567-
wantErr: `failed to apply resource "istio-test-ns/cert-manager-istio-csr": test client error`,
567+
wantErr: `failed to apply RoleBinding "istio-test-ns/cert-manager-istio-csr": test client error`,
568568
},
569569
{
570570
name: "rolebindings reconciliation creation fails",
@@ -584,7 +584,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) {
584584
return nil
585585
})
586586
},
587-
wantErr: `failed to apply resource "istio-test-ns/cert-manager-istio-csr": test client error`,
587+
wantErr: `failed to apply RoleBinding "istio-test-ns/cert-manager-istio-csr": test client error`,
588588
},
589589
{
590590
name: "rolebinding-leases reconciliation applying to desired state fails",
@@ -610,7 +610,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) {
610610
return nil
611611
})
612612
},
613-
wantErr: `failed to apply resource "istio-test-ns/cert-manager-istio-csr-leases": test client error`,
613+
wantErr: `failed to apply RoleBinding "istio-test-ns/cert-manager-istio-csr-leases": test client error`,
614614
},
615615
{
616616
name: "rolebinding-leases reconciliation creation fails",
@@ -634,7 +634,7 @@ func TestCreateOrApplyRBACResource(t *testing.T) {
634634
return nil
635635
})
636636
},
637-
wantErr: `failed to apply resource "istio-test-ns/cert-manager-istio-csr-leases": test client error`,
637+
wantErr: `failed to apply RoleBinding "istio-test-ns/cert-manager-istio-csr-leases": test client error`,
638638
},
639639
}
640640

‎pkg/controller/istiocsr/serviceaccounts_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestCreateOrApplyServiceAccounts(t *testing.T) {
4848
return false, nil
4949
})
5050
},
51-
wantErr: `failed to check if resource "istiocsr-test-ns/cert-manager-istio-csr" exists: test client error`,
51+
wantErr: `failed to check if ServiceAccount "istiocsr-test-ns/cert-manager-istio-csr" exists: test client error`,
5252
},
5353
{
5454
name: "serviceaccount reconciliation fails while updating status",
@@ -96,7 +96,7 @@ func TestCreateOrApplyServiceAccounts(t *testing.T) {
9696
})
9797
m.PatchReturns(errTestClient)
9898
},
99-
wantErr: `failed to apply resource "istiocsr-test-ns/cert-manager-istio-csr": test client error`,
99+
wantErr: `failed to apply ServiceAccount "istiocsr-test-ns/cert-manager-istio-csr": test client error`,
100100
},
101101
{
102102
name: "serviceaccount created when it does not exist",

‎pkg/controller/istiocsr/services_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func TestCreateOrApplyServices(t *testing.T) {
5050
return false, nil
5151
})
5252
},
53-
wantErr: `failed to check if resource "istiocsr-test-ns/cert-manager-istio-csr" exists: test client error`,
53+
wantErr: `failed to check if Service "istiocsr-test-ns/cert-manager-istio-csr" exists: test client error`,
5454
},
5555
{
5656
name: "service reconciliation fails while applying to desired state",
@@ -67,14 +67,14 @@ func TestCreateOrApplyServices(t *testing.T) {
6767
return false, nil
6868
})
6969
},
70-
wantErr: `failed to apply resource "istiocsr-test-ns/cert-manager-istio-csr": test client error`,
70+
wantErr: `failed to apply Service "istiocsr-test-ns/cert-manager-istio-csr": test client error`,
7171
},
7272
{
7373
name: "service reconciliation fails while creating",
7474
preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) {
7575
m.PatchReturns(errTestClient)
7676
},
77-
wantErr: `failed to apply resource "istiocsr-test-ns/cert-manager-istio-csr": test client error`,
77+
wantErr: `failed to apply Service "istiocsr-test-ns/cert-manager-istio-csr": test client error`,
7878
},
7979
{
8080
name: "service reconciliation when server config is not empty",

‎pkg/controller/trustmanager/certificates_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ func TestIssuerReconciliation(t *testing.T) {
233233
return false, errTestClient
234234
})
235235
},
236-
wantErr: "failed to check if resource",
236+
wantErr: "failed to check if Issuer",
237237
wantExistsCount: 1,
238238
wantPatchCount: 0,
239239
},
@@ -247,7 +247,7 @@ func TestIssuerReconciliation(t *testing.T) {
247247
return errTestClient
248248
})
249249
},
250-
wantErr: "failed to apply resource",
250+
wantErr: "failed to apply Issuer",
251251
wantExistsCount: 1,
252252
wantPatchCount: 1,
253253
},
@@ -376,7 +376,7 @@ func TestCertificateReconciliation(t *testing.T) {
376376
return false, errTestClient
377377
})
378378
},
379-
wantErr: "failed to check if resource",
379+
wantErr: "failed to check if Certificate",
380380
wantExistsCount: 1,
381381
wantPatchCount: 0,
382382
},
@@ -390,7 +390,7 @@ func TestCertificateReconciliation(t *testing.T) {
390390
return errTestClient
391391
})
392392
},
393-
wantErr: "failed to apply resource",
393+
wantErr: "failed to apply Certificate",
394394
wantExistsCount: 1,
395395
wantPatchCount: 1,
396396
},

‎pkg/controller/trustmanager/controller_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ func TestProcessReconcileRequest(t *testing.T) {
251251
Reason: v1alpha1.ReasonInProgress,
252252
},
253253
},
254-
wantErr: "failed to check if resource",
254+
wantErr: "failed to check if ServiceAccount",
255255
},
256256
{
257257
name: "trust namespace does not exist sets degraded true",

‎pkg/controller/trustmanager/deployments_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ func TestDeploymentReconciliation(t *testing.T) {
593593
return false, errTestClient
594594
})
595595
},
596-
wantErr: "failed to check if resource",
596+
wantErr: "failed to check if Deployment",
597597
wantExistsCount: 1,
598598
wantPatchCount: 0,
599599
},
@@ -608,7 +608,7 @@ func TestDeploymentReconciliation(t *testing.T) {
608608
return errTestClient
609609
})
610610
},
611-
wantErr: "failed to apply resource",
611+
wantErr: "failed to apply Deployment",
612612
wantExistsCount: 1,
613613
wantPatchCount: 1,
614614
},

‎pkg/controller/trustmanager/rbacs_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ func TestRBACReconciliation(t *testing.T) {
486486
return false, errTestClient
487487
})
488488
},
489-
wantErr: "failed to check if resource",
489+
wantErr: "failed to check if ClusterRole",
490490
wantExistsCount: 1,
491491
wantPatchCount: 0,
492492
},
@@ -503,7 +503,7 @@ func TestRBACReconciliation(t *testing.T) {
503503
return nil
504504
})
505505
},
506-
wantErr: "failed to apply resource",
506+
wantErr: "failed to apply ClusterRole",
507507
wantExistsCount: 1,
508508
wantPatchCount: 1,
509509
},
@@ -520,7 +520,7 @@ func TestRBACReconciliation(t *testing.T) {
520520
return nil
521521
})
522522
},
523-
wantErr: "failed to apply resource",
523+
wantErr: "failed to apply ClusterRoleBinding",
524524
wantExistsCount: 2,
525525
wantPatchCount: 2,
526526
},
@@ -537,7 +537,7 @@ func TestRBACReconciliation(t *testing.T) {
537537
return nil
538538
})
539539
},
540-
wantErr: "failed to apply resource",
540+
wantErr: "failed to apply Role",
541541
wantExistsCount: 3,
542542
wantPatchCount: 3,
543543
},
@@ -554,7 +554,7 @@ func TestRBACReconciliation(t *testing.T) {
554554
return nil
555555
})
556556
},
557-
wantErr: "failed to apply resource",
557+
wantErr: "failed to apply RoleBinding",
558558
wantExistsCount: 4,
559559
wantPatchCount: 4,
560560
},

0 commit comments

Comments
 (0)