Skip to content

Commit ad73c86

Browse files
committed
test(metrics): cover reconcile-condition metric across reconcilers
Adds test coverage for the per-resource reconcile-condition gauge flagged by Codecov as low patch coverage: - lbc: exercise MockCollector condition set/delete recording - service: NotFound-delete branch + condition assertions on reconcile - targetGroupBinding: NotFound, cleanup-success, and deferred branches - globalAccelerator: NotFound-delete branch (first tests for the package) - gateway: NotFound-delete branch via the canonical NewMockCollector - ingress: members-present (condition set) and empty-group (delete) branches
1 parent 5f6e17b commit ad73c86

6 files changed

Lines changed: 560 additions & 14 deletions

File tree

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package controllers
2+
3+
import (
4+
"context"
5+
"testing"
6+
"time"
7+
8+
"github.com/go-logr/logr"
9+
"github.com/stretchr/testify/assert"
10+
"k8s.io/apimachinery/pkg/runtime"
11+
"k8s.io/apimachinery/pkg/types"
12+
agaapi "sigs.k8s.io/aws-load-balancer-controller/v3/apis/aga/v1beta1"
13+
testclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
14+
"sigs.k8s.io/controller-runtime/pkg/reconcile"
15+
)
16+
17+
// recordedCondition captures the arguments of a reconcile-condition observation/deletion.
18+
type recordedCondition struct {
19+
controller string
20+
namespace string
21+
name string
22+
reconciled bool
23+
}
24+
25+
// recordingMetricCollector records the reconcile-condition calls so tests can assert on the
26+
// per-resource condition metric. ObserveControllerReconcileLatency invokes the passed fn so the
27+
// wrapped reconcile stages actually run.
28+
type recordingMetricCollector struct {
29+
conditions []recordedCondition
30+
deletes []recordedCondition
31+
}
32+
33+
func (m *recordingMetricCollector) ObservePodReadinessGateReady(_ string, _ string, _ time.Duration) {
34+
}
35+
func (m *recordingMetricCollector) ObserveQUICTargetMissingServerId(_ string, _ string) {}
36+
func (m *recordingMetricCollector) ObserveControllerReconcileError(_ string, _ string) {}
37+
func (m *recordingMetricCollector) ObserveControllerReconcileCondition(controller string, namespace string, name string, reconciled bool) {
38+
m.conditions = append(m.conditions, recordedCondition{controller, namespace, name, reconciled})
39+
}
40+
func (m *recordingMetricCollector) DeleteControllerReconcileCondition(controller string, namespace string, name string) {
41+
m.deletes = append(m.deletes, recordedCondition{controller: controller, namespace: namespace, name: name})
42+
}
43+
func (m *recordingMetricCollector) ObserveControllerReconcileLatency(_ string, _ string, fn func()) {
44+
fn()
45+
}
46+
func (m *recordingMetricCollector) ObserveWebhookValidationError(_ string, _ string) {}
47+
func (m *recordingMetricCollector) ObserveWebhookMutationError(_ string, _ string) {}
48+
func (m *recordingMetricCollector) StartCollectTopTalkers(_ context.Context) {}
49+
func (m *recordingMetricCollector) StartCollectCacheSize(_ context.Context) {}
50+
51+
// TestGlobalAcceleratorReconciler_Reconcile_NotFound covers the branch where the GlobalAccelerator
52+
// has already been deleted: the fetch returns NotFound and the reconcile drops the condition series.
53+
func TestGlobalAcceleratorReconciler_Reconcile_NotFound(t *testing.T) {
54+
scheme := runtime.NewScheme()
55+
_ = agaapi.AddToScheme(scheme)
56+
57+
// no object seeded into the fake client, so the Get returns NotFound
58+
k8sClient := testclient.NewClientBuilder().WithScheme(scheme).Build()
59+
60+
metricsCollector := &recordingMetricCollector{}
61+
r := &globalAcceleratorReconciler{
62+
k8sClient: k8sClient,
63+
logger: logr.Discard(),
64+
metricsCollector: metricsCollector,
65+
}
66+
67+
err := r.reconcile(context.Background(), reconcile.Request{
68+
NamespacedName: types.NamespacedName{Namespace: "default", Name: "gone-aga"},
69+
})
70+
71+
assert.NoError(t, err)
72+
assert.Empty(t, metricsCollector.conditions)
73+
assert.Equal(t, []recordedCondition{{controller: "globalAccelerator", namespace: "default", name: "gone-aga"}}, metricsCollector.deletes)
74+
}

controllers/elbv2/targetgroupbinding_controller_test.go

Lines changed: 170 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,30 @@ func (m *mockMetricCollector) ObserveWebhookMutationError(webhookName string, er
6464
func (m *mockMetricCollector) StartCollectTopTalkers(ctx context.Context) {}
6565
func (m *mockMetricCollector) StartCollectCacheSize(ctx context.Context) {}
6666

67+
// recordedCondition captures the arguments of a reconcile-condition observation/deletion.
68+
type recordedCondition struct {
69+
controller string
70+
namespace string
71+
name string
72+
reconciled bool
73+
}
74+
75+
// recordingMetricCollector records the reconcile-condition calls so tests can assert on the
76+
// per-resource condition metric, delegating every other method to mockMetricCollector.
77+
type recordingMetricCollector struct {
78+
mockMetricCollector
79+
conditions []recordedCondition
80+
deletes []recordedCondition
81+
}
82+
83+
func (m *recordingMetricCollector) ObserveControllerReconcileCondition(controller string, namespace string, name string, reconciled bool) {
84+
m.conditions = append(m.conditions, recordedCondition{controller, namespace, name, reconciled})
85+
}
86+
87+
func (m *recordingMetricCollector) DeleteControllerReconcileCondition(controller string, namespace string, name string) {
88+
m.deletes = append(m.deletes, recordedCondition{controller: controller, namespace: namespace, name: name})
89+
}
90+
6791
// --- Test ---
6892

6993
func TestTargetGroupBindingReconciler_Delete_Stuck(t *testing.T) {
@@ -230,14 +254,15 @@ func TestTargetGroupBindingReconciler_Reconcile_Success(t *testing.T) {
230254

231255
fakeRecorder := record.NewFakeRecorder(10)
232256

257+
metricsCollector := &recordingMetricCollector{}
233258
reconciler := &targetGroupBindingReconciler{
234259
k8sClient: k8sClient,
235260
eventRecorder: fakeRecorder,
236261
finalizerManager: mockFinalizerManager,
237262
tgbResourceManager: mockResMgr,
238263
deferredTargetGroupBindingReconciler: &mockDeferredReconciler{},
239264
logger: log.Log.WithName("controllers").WithName("TargetGroupBinding"),
240-
metricsCollector: &mockMetricCollector{},
265+
metricsCollector: metricsCollector,
241266
reconcileCounters: metricsutil.NewReconcileCounters(),
242267
}
243268

@@ -262,4 +287,148 @@ func TestTargetGroupBindingReconciler_Reconcile_Success(t *testing.T) {
262287
default:
263288
t.Fatal("expected SuccessfullyReconciled event but none was emitted")
264289
}
290+
291+
// a successful reconcile marks the resource's condition as healthy
292+
assert.Equal(t, []recordedCondition{{controller: "targetGroupBinding", namespace: "default", name: "test-tgb", reconciled: true}}, metricsCollector.conditions)
293+
assert.Empty(t, metricsCollector.deletes)
294+
}
295+
296+
// TestTargetGroupBindingReconciler_Reconcile_NotFound covers the branch where the TGB has already
297+
// been deleted: the fetch returns NotFound and the reconcile drops the condition series.
298+
func TestTargetGroupBindingReconciler_Reconcile_NotFound(t *testing.T) {
299+
scheme := runtime.NewScheme()
300+
clientgoscheme.AddToScheme(scheme)
301+
elbv2api.AddToScheme(scheme)
302+
303+
// no TGB seeded into the client, so the Get returns NotFound
304+
k8sClient := testclient.NewClientBuilder().WithScheme(scheme).Build()
305+
306+
ctrl := gomock.NewController(t)
307+
defer ctrl.Finish()
308+
309+
metricsCollector := &recordingMetricCollector{}
310+
reconciler := &targetGroupBindingReconciler{
311+
k8sClient: k8sClient,
312+
eventRecorder: record.NewFakeRecorder(10),
313+
finalizerManager: k8s.NewMockFinalizerManager(ctrl),
314+
tgbResourceManager: targetgroupbinding.NewMockResourceManager(ctrl),
315+
deferredTargetGroupBindingReconciler: &mockDeferredReconciler{},
316+
logger: log.Log.WithName("controllers").WithName("TargetGroupBinding"),
317+
metricsCollector: metricsCollector,
318+
reconcileCounters: metricsutil.NewReconcileCounters(),
319+
}
320+
321+
req := reconcile.Request{
322+
NamespacedName: client.ObjectKey{Namespace: "default", Name: "gone-tgb"},
323+
}
324+
325+
_, err := reconciler.Reconcile(context.Background(), req)
326+
327+
assert.NoError(t, err)
328+
assert.Empty(t, metricsCollector.conditions)
329+
assert.Equal(t, []recordedCondition{{controller: "targetGroupBinding", namespace: "default", name: "gone-tgb"}}, metricsCollector.deletes)
330+
}
331+
332+
// TestTargetGroupBindingReconciler_Reconcile_CleanupSuccess covers a successful delete: after
333+
// cleanup the condition series must be removed.
334+
func TestTargetGroupBindingReconciler_Reconcile_CleanupSuccess(t *testing.T) {
335+
scheme := runtime.NewScheme()
336+
clientgoscheme.AddToScheme(scheme)
337+
elbv2api.AddToScheme(scheme)
338+
339+
tgb := &elbv2api.TargetGroupBinding{
340+
ObjectMeta: metav1.ObjectMeta{
341+
Name: "test-tgb",
342+
Namespace: "default",
343+
Finalizers: []string{"elbv2.k8s.aws/resources"},
344+
DeletionTimestamp: &metav1.Time{Time: time.Now()},
345+
},
346+
Spec: elbv2api.TargetGroupBindingSpec{
347+
TargetGroupARN: "arn:aws:elasticloadbalancing:us-west-2:123456789012:targetgroup/test/123",
348+
},
349+
}
350+
351+
k8sClient := testclient.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(tgb).WithRuntimeObjects(tgb).Build()
352+
353+
ctrl := gomock.NewController(t)
354+
defer ctrl.Finish()
355+
356+
mockFinalizerManager := k8s.NewMockFinalizerManager(ctrl)
357+
mockFinalizerManager.EXPECT().RemoveFinalizers(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
358+
359+
mockResMgr := targetgroupbinding.NewMockResourceManager(ctrl)
360+
mockResMgr.EXPECT().Cleanup(gomock.Any(), gomock.Any()).Return(nil)
361+
362+
metricsCollector := &recordingMetricCollector{}
363+
reconciler := &targetGroupBindingReconciler{
364+
k8sClient: k8sClient,
365+
eventRecorder: record.NewFakeRecorder(10),
366+
finalizerManager: mockFinalizerManager,
367+
tgbResourceManager: mockResMgr,
368+
deferredTargetGroupBindingReconciler: &mockDeferredReconciler{},
369+
logger: log.Log.WithName("controllers").WithName("TargetGroupBinding"),
370+
metricsCollector: metricsCollector,
371+
reconcileCounters: metricsutil.NewReconcileCounters(),
372+
}
373+
374+
req := reconcile.Request{
375+
NamespacedName: client.ObjectKey{Namespace: "default", Name: "test-tgb"},
376+
}
377+
378+
_, err := reconciler.Reconcile(context.Background(), req)
379+
380+
assert.NoError(t, err)
381+
assert.Empty(t, metricsCollector.conditions)
382+
assert.Equal(t, []recordedCondition{{controller: "targetGroupBinding", namespace: "default", name: "test-tgb"}}, metricsCollector.deletes)
383+
}
384+
385+
// TestTargetGroupBindingReconciler_Reconcile_Deferred covers a deferred reconcile: the pass itself
386+
// did not fail, so the condition must be marked healthy even though the work is enqueued.
387+
func TestTargetGroupBindingReconciler_Reconcile_Deferred(t *testing.T) {
388+
scheme := runtime.NewScheme()
389+
clientgoscheme.AddToScheme(scheme)
390+
elbv2api.AddToScheme(scheme)
391+
392+
tgb := &elbv2api.TargetGroupBinding{
393+
ObjectMeta: metav1.ObjectMeta{
394+
Name: "test-tgb",
395+
Namespace: "default",
396+
},
397+
Spec: elbv2api.TargetGroupBindingSpec{
398+
TargetGroupARN: "arn:aws:elasticloadbalancing:us-west-2:123456789012:targetgroup/test/123",
399+
},
400+
}
401+
402+
k8sClient := testclient.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(tgb).WithRuntimeObjects(tgb).Build()
403+
404+
ctrl := gomock.NewController(t)
405+
defer ctrl.Finish()
406+
407+
mockFinalizerManager := k8s.NewMockFinalizerManager(ctrl)
408+
mockFinalizerManager.EXPECT().AddFinalizers(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
409+
410+
mockResMgr := targetgroupbinding.NewMockResourceManager(ctrl)
411+
mockResMgr.EXPECT().Reconcile(gomock.Any(), gomock.Any()).Return(true, nil)
412+
413+
metricsCollector := &recordingMetricCollector{}
414+
reconciler := &targetGroupBindingReconciler{
415+
k8sClient: k8sClient,
416+
eventRecorder: record.NewFakeRecorder(10),
417+
finalizerManager: mockFinalizerManager,
418+
tgbResourceManager: mockResMgr,
419+
deferredTargetGroupBindingReconciler: &mockDeferredReconciler{},
420+
logger: log.Log.WithName("controllers").WithName("TargetGroupBinding"),
421+
metricsCollector: metricsCollector,
422+
reconcileCounters: metricsutil.NewReconcileCounters(),
423+
}
424+
425+
req := reconcile.Request{
426+
NamespacedName: client.ObjectKey{Namespace: "default", Name: "test-tgb"},
427+
}
428+
429+
_, err := reconciler.Reconcile(context.Background(), req)
430+
431+
assert.NoError(t, err)
432+
assert.Equal(t, []recordedCondition{{controller: "targetGroupBinding", namespace: "default", name: "test-tgb", reconciled: true}}, metricsCollector.conditions)
433+
assert.Empty(t, metricsCollector.deletes)
265434
}

controllers/gateway/gateway_controller_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,16 @@ import (
1010
"github.com/go-logr/logr"
1111
"github.com/stretchr/testify/assert"
1212
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/apimachinery/pkg/types"
1314
"k8s.io/client-go/tools/record"
1415
ctrlerrors "sigs.k8s.io/aws-load-balancer-controller/v3/pkg/error"
1516
gateway_constants "sigs.k8s.io/aws-load-balancer-controller/v3/pkg/gateway/constants"
1617
"sigs.k8s.io/aws-load-balancer-controller/v3/pkg/gateway/routeutils"
1718
"sigs.k8s.io/aws-load-balancer-controller/v3/pkg/k8s"
19+
lbcmetrics "sigs.k8s.io/aws-load-balancer-controller/v3/pkg/metrics/lbc"
1820
elbv2model "sigs.k8s.io/aws-load-balancer-controller/v3/pkg/model/elbv2"
1921
"sigs.k8s.io/aws-load-balancer-controller/v3/pkg/testutils"
22+
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2023
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
2124
)
2225

@@ -182,3 +185,28 @@ func Test_updateGatewayStatusSuccess_normalizesDNSNameToLowercase(t *testing.T)
182185
assert.NotNil(t, updatedGW.Status.Addresses[0].Type)
183186
assert.Equal(t, gwv1.HostnameAddressType, *updatedGW.Status.Addresses[0].Type)
184187
}
188+
189+
// Test_reconcileHelper_NotFound covers the branch where the Gateway has already been deleted: the
190+
// fetch returns NotFound and reconcileHelper drops the condition series without erroring.
191+
func Test_reconcileHelper_NotFound(t *testing.T) {
192+
k8sClient := testutils.GenerateTestClient()
193+
194+
collector := lbcmetrics.NewMockCollector()
195+
reconciler := &gatewayReconciler{
196+
k8sClient: k8sClient,
197+
logger: logr.Discard(),
198+
eventRecorder: record.NewFakeRecorder(10),
199+
controllerName: "gateway.k8s.aws/nlb",
200+
metricsCollector: collector,
201+
}
202+
203+
// no Gateway seeded into the client, so the Get returns NotFound
204+
err := reconciler.reconcileHelper(context.Background(), reconcile.Request{
205+
NamespacedName: types.NamespacedName{Namespace: "test-ns", Name: "gone-gw"},
206+
})
207+
208+
assert.NoError(t, err)
209+
// the resource's condition series is removed (a single delete invocation, no set)
210+
mc := collector.(*lbcmetrics.MockCollector)
211+
assert.Equal(t, 1, len(mc.Invocations[lbcmetrics.MetricControllerReconcileCondition]))
212+
}

0 commit comments

Comments
 (0)