From 0be27de282fbea88fc08eed2059c59e3ef68c741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20O=CC=88zc=CC=A7elik?= Date: Thu, 23 Jul 2026 10:30:41 -0700 Subject: [PATCH 1/2] fix flaky TestKubeCertAgent by waiting for informer's watch action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hasDeploymentSynced` asserts the exact order of the fake client's deployment actions, expecting the informer's initial `"list"` and `"watch"` to precede any actions caused by `Sync.WaitForCacheSync()` can return before the reflector issues its "watch" request, since the cache is considered synced once the initial "list" is processed. Wait for the `"watch"` action to be recorded before proceeding, since this function runs at the start of every Sync. Signed-off-by: Volkan Özçelik --- .../kubecertagent/kubecertagent_test.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/internal/controller/kubecertagent/kubecertagent_test.go b/internal/controller/kubecertagent/kubecertagent_test.go index 0de3cd3f5..b1a846801 100644 --- a/internal/controller/kubecertagent/kubecertagent_test.go +++ b/internal/controller/kubecertagent/kubecertagent_test.go @@ -22,7 +22,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/cache" "k8s.io/client-go/informers" - "k8s.io/client-go/kubernetes" kubefake "k8s.io/client-go/kubernetes/fake" coretesting "k8s.io/client-go/testing" clocktesting "k8s.io/utils/clock/testing" @@ -2097,9 +2096,22 @@ done: return errorMessages } -func hasDeploymentSynced(client kubernetes.Interface, kubeInformers informers.SharedInformerFactory) func(ctx context.Context, t *testing.T) { +func hasDeploymentSynced(client *kubefake.Clientset, kubeInformers informers.SharedInformerFactory) func(ctx context.Context, t *testing.T) { return func(ctx context.Context, t *testing.T) { testlib.RequireEventually(t, func(requireEventually *require.Assertions) { + // The test asserts the exact order of the deployment actions' verbs (wantDeploymentActionVerbs), + // expecting the informer's initial "list" and "watch" to appear before any verbs caused by Sync. + // However, WaitForCacheSync() can return before the informer's reflector has issued its "watch" + // request, because the cache is considered synced once the initial "list" results have been + // processed. Since this function is called at the start of every Sync, waiting here for the + // "watch" action guarantees that it is recorded before any of Sync's actions. + requireEventually.True( + slices.ContainsFunc(client.Actions(), func(a coretesting.Action) bool { + return a.GetVerb() == "watch" && a.GetResource().Resource == "deployments" + }), + "informer has not started watching deployments yet", + ) + realDep, realErr := client.AppsV1().Deployments("concierge"). Get(ctx, "pinniped-concierge-kube-cert-agent", metav1.GetOptions{}) From 1e7023bfb22563598472520c0fa0bd9b07ec2f21 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 23 Jul 2026 11:29:38 -0700 Subject: [PATCH 2/2] fix flake in TestLegacyPodCleanerController Signed-off-by: Ryan Richard --- .../kubecertagent/kubecertagent_test.go | 2 +- .../kubecertagent/legacypodcleaner_test.go | 24 +++++++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/internal/controller/kubecertagent/kubecertagent_test.go b/internal/controller/kubecertagent/kubecertagent_test.go index b1a846801..1fb55466a 100644 --- a/internal/controller/kubecertagent/kubecertagent_test.go +++ b/internal/controller/kubecertagent/kubecertagent_test.go @@ -2126,6 +2126,6 @@ func hasDeploymentSynced(client *kubefake.Clientset, kubeInformers informers.Sha requireEventually.NoError(cachedErr) requireEventually.Equal(realDep, cachedDep) - }, 2*time.Second, 100*time.Millisecond) + }, 5*time.Second, 100*time.Millisecond) } } diff --git a/internal/controller/kubecertagent/legacypodcleaner_test.go b/internal/controller/kubecertagent/legacypodcleaner_test.go index de1777d1b..fbf6441e1 100644 --- a/internal/controller/kubecertagent/legacypodcleaner_test.go +++ b/internal/controller/kubecertagent/legacypodcleaner_test.go @@ -1,4 +1,4 @@ -// Copyright 2021-2025 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2026 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package kubecertagent @@ -6,10 +6,12 @@ package kubecertagent import ( "context" "fmt" + "slices" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -21,6 +23,7 @@ import ( "go.pinniped.dev/internal/kubeclient" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/testutil" + "go.pinniped.dev/test/testlib" ) func TestLegacyPodCleanerController(t *testing.T) { @@ -162,10 +165,27 @@ func TestLegacyPodCleanerController(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - errorMessages := runControllerUntilQuiet(ctx, t, controller, func(_ context.Context, _ *testing.T) {}, kubeInformers) + errorMessages := runControllerUntilQuiet(ctx, t, controller, hasPodsSynced(kubeClientset), kubeInformers) assert.Equal(t, tt.wantDistinctErrors, deduplicate(errorMessages), "unexpected errors") assert.Equal(t, tt.wantDistinctLogs, deduplicate(testutil.SplitByNewline(log.String())), "unexpected logs") assert.Equal(t, tt.wantActions, kubeClientset.Actions()[2:], "unexpected actions") }) } } + +func hasPodsSynced(client *kubefake.Clientset) func(ctx context.Context, t *testing.T) { + return func(_ context.Context, t *testing.T) { + testlib.RequireEventually(t, func(requireEventually *require.Assertions) { + // WaitForCacheSync() can return before the informer's reflector has issued its "watch" request, + // because the cache is considered synced once the initial "list" results have been processed. + // Since this function is called at the start of every Sync, waiting here for the "watch" action + // guarantees that it is recorded before any of Sync's actions. + requireEventually.True( + slices.ContainsFunc(client.Actions(), func(a coretesting.Action) bool { + return a.GetVerb() == "watch" && a.GetResource().Resource == "pods" + }), + "informer has not started watching pods yet", + ) + }, 5*time.Second, 100*time.Millisecond) + } +}