Skip to content

Commit a6a7d9e

Browse files
Wei WengWei Weng
authored andcommitted
move readiness check function to informer manager package
Signed-off-by: Wei Weng <Wei.Weng@microsoft.com>
1 parent a49f887 commit a6a7d9e

File tree

5 files changed

+223
-202
lines changed

5 files changed

+223
-202
lines changed

cmd/hubagent/main.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import (
4646
"github.com/kubefleet-dev/kubefleet/cmd/hubagent/options"
4747
"github.com/kubefleet-dev/kubefleet/cmd/hubagent/workload"
4848
mcv1beta1 "github.com/kubefleet-dev/kubefleet/pkg/controllers/membercluster/v1beta1"
49+
"github.com/kubefleet-dev/kubefleet/pkg/utils/informer"
4950
"github.com/kubefleet-dev/kubefleet/pkg/utils/validator"
5051
"github.com/kubefleet-dev/kubefleet/pkg/webhook"
5152
// +kubebuilder:scaffold:imports
@@ -175,7 +176,7 @@ func main() {
175176
// which is critical for all controllers that rely on dynamic resource discovery.
176177
// AddReadyzCheck adds additional readiness check instead of replacing the one registered earlier provided the name is different.
177178
// Both registered checks need to pass for the manager to be considered ready.
178-
if err := mgr.AddReadyzCheck("informer-cache", webhook.ResourceInformerReadinessChecker(validator.ResourceInformer)); err != nil {
179+
if err := mgr.AddReadyzCheck("informer-cache", informer.ReadinessChecker(validator.ResourceInformer)); err != nil {
179180
klog.ErrorS(err, "unable to set up informer cache readiness check")
180181
exitWithErrorFunc()
181182
}

pkg/utils/informer/informermanager.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package informer
1919
import (
2020
"context"
2121
"fmt"
22+
"net/http"
2223
"sync"
2324
"time"
2425

@@ -198,6 +199,40 @@ func (s *informerManagerImpl) IsInformerSynced(resource schema.GroupVersionResou
198199
return s.informerFactory.ForResource(resource).Informer().HasSynced()
199200
}
200201

202+
// ReadinessChecker creates a readiness check function that verifies
203+
// all resource informer caches are synced before marking the pod as ready.
204+
// This prevents components from processing requests before the discovery cache is populated.
205+
func ReadinessChecker(resourceInformer Manager) func(*http.Request) error {
206+
return func(_ *http.Request) error {
207+
if resourceInformer == nil {
208+
return fmt.Errorf("resource informer not initialized")
209+
}
210+
211+
// Require ALL informer caches to be synced before marking ready
212+
allResources := resourceInformer.GetAllResources()
213+
if len(allResources) == 0 {
214+
// This can happen during startup when the ResourceInformer is created but the ChangeDetector
215+
// hasn't discovered and registered any resources yet via AddDynamicResources().
216+
return fmt.Errorf("resource informer not ready: no resources registered")
217+
}
218+
219+
// Check that ALL informers have synced
220+
unsyncedResources := []schema.GroupVersionResource{}
221+
for _, gvr := range allResources {
222+
if !resourceInformer.IsInformerSynced(gvr) {
223+
unsyncedResources = append(unsyncedResources, gvr)
224+
}
225+
}
226+
227+
if len(unsyncedResources) > 0 {
228+
return fmt.Errorf("resource informer not ready: %d/%d informers not synced yet", len(unsyncedResources), len(allResources))
229+
}
230+
231+
klog.V(5).InfoS("All resource informers synced", "totalInformers", len(allResources))
232+
return nil
233+
}
234+
}
235+
201236
func (s *informerManagerImpl) Lister(resource schema.GroupVersionResource) cache.GenericLister {
202237
return s.informerFactory.ForResource(resource).Lister()
203238
}
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
/*
2+
Copyright 2025 The KubeFleet Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package informer
18+
19+
import (
20+
"testing"
21+
22+
"github.com/stretchr/testify/assert"
23+
"k8s.io/apimachinery/pkg/runtime/schema"
24+
"k8s.io/client-go/dynamic"
25+
"k8s.io/client-go/tools/cache"
26+
)
27+
28+
// mockInformerManager is a simple mock for testing
29+
type mockInformerManager struct {
30+
allResources []schema.GroupVersionResource
31+
syncedMap map[schema.GroupVersionResource]bool
32+
}
33+
34+
func (m *mockInformerManager) AddDynamicResources(resources []APIResourceMeta, handler cache.ResourceEventHandler, listComplete bool) {
35+
}
36+
func (m *mockInformerManager) AddStaticResource(resource APIResourceMeta, handler cache.ResourceEventHandler) {
37+
}
38+
func (m *mockInformerManager) IsInformerSynced(resource schema.GroupVersionResource) bool {
39+
if m.syncedMap == nil {
40+
return false
41+
}
42+
synced, exists := m.syncedMap[resource]
43+
return exists && synced
44+
}
45+
func (m *mockInformerManager) Start() {}
46+
func (m *mockInformerManager) Stop() {}
47+
func (m *mockInformerManager) Lister(resource schema.GroupVersionResource) cache.GenericLister {
48+
return nil
49+
}
50+
func (m *mockInformerManager) GetNameSpaceScopedResources() []schema.GroupVersionResource { return nil }
51+
func (m *mockInformerManager) GetAllResources() []schema.GroupVersionResource {
52+
return m.allResources
53+
}
54+
func (m *mockInformerManager) IsClusterScopedResources(resource schema.GroupVersionKind) bool {
55+
return false
56+
}
57+
func (m *mockInformerManager) WaitForCacheSync() {}
58+
func (m *mockInformerManager) GetClient() dynamic.Interface { return nil }
59+
60+
func TestReadinessChecker(t *testing.T) {
61+
tests := []struct {
62+
name string
63+
resourceInformer Manager
64+
expectError bool
65+
errorContains string
66+
}{
67+
{
68+
name: "nil informer",
69+
resourceInformer: nil,
70+
expectError: true,
71+
errorContains: "resource informer not initialized",
72+
},
73+
{
74+
name: "no resources registered",
75+
resourceInformer: &mockInformerManager{
76+
allResources: []schema.GroupVersionResource{},
77+
},
78+
expectError: true,
79+
errorContains: "no resources registered",
80+
},
81+
{
82+
name: "all informers synced",
83+
resourceInformer: &mockInformerManager{
84+
allResources: []schema.GroupVersionResource{
85+
{Group: "", Version: "v1", Resource: "configmaps"},
86+
{Group: "", Version: "v1", Resource: "secrets"},
87+
{Group: "", Version: "v1", Resource: "namespaces"},
88+
},
89+
syncedMap: map[schema.GroupVersionResource]bool{
90+
{Group: "", Version: "v1", Resource: "configmaps"}: true,
91+
{Group: "", Version: "v1", Resource: "secrets"}: true,
92+
{Group: "", Version: "v1", Resource: "namespaces"}: true,
93+
},
94+
},
95+
expectError: false,
96+
},
97+
{
98+
name: "some informers not synced",
99+
resourceInformer: &mockInformerManager{
100+
allResources: []schema.GroupVersionResource{
101+
{Group: "", Version: "v1", Resource: "configmaps"},
102+
{Group: "", Version: "v1", Resource: "secrets"},
103+
{Group: "", Version: "v1", Resource: "namespaces"},
104+
},
105+
syncedMap: map[schema.GroupVersionResource]bool{
106+
{Group: "", Version: "v1", Resource: "configmaps"}: true,
107+
{Group: "", Version: "v1", Resource: "secrets"}: false,
108+
{Group: "", Version: "v1", Resource: "namespaces"}: true,
109+
},
110+
},
111+
expectError: true,
112+
errorContains: "informers not synced yet",
113+
},
114+
}
115+
116+
for _, tt := range tests {
117+
t.Run(tt.name, func(t *testing.T) {
118+
checker := ReadinessChecker(tt.resourceInformer)
119+
err := checker(nil)
120+
121+
if tt.expectError {
122+
assert.Error(t, err)
123+
if tt.errorContains != "" {
124+
assert.Contains(t, err.Error(), tt.errorContains)
125+
}
126+
} else {
127+
assert.NoError(t, err)
128+
}
129+
})
130+
}
131+
}
132+
133+
func TestReadinessChecker_PartialSync(t *testing.T) {
134+
// Test the case where we have multiple resources but only some are synced
135+
mockManager := &mockInformerManager{
136+
allResources: []schema.GroupVersionResource{
137+
{Group: "", Version: "v1", Resource: "configmaps"},
138+
{Group: "", Version: "v1", Resource: "secrets"},
139+
{Group: "apps", Version: "v1", Resource: "deployments"},
140+
{Group: "", Version: "v1", Resource: "namespaces"},
141+
},
142+
syncedMap: map[schema.GroupVersionResource]bool{
143+
{Group: "", Version: "v1", Resource: "configmaps"}: false,
144+
{Group: "", Version: "v1", Resource: "secrets"}: false,
145+
{Group: "apps", Version: "v1", Resource: "deployments"}: false,
146+
{Group: "", Version: "v1", Resource: "namespaces"}: false,
147+
},
148+
}
149+
150+
checker := ReadinessChecker(mockManager)
151+
err := checker(nil)
152+
153+
assert.Error(t, err)
154+
assert.Contains(t, err.Error(), "informers not synced yet")
155+
// Should report 4 unsynced
156+
assert.Contains(t, err.Error(), "4/4")
157+
}
158+
159+
func TestReadinessChecker_AllSyncedMultipleResources(t *testing.T) {
160+
// Test with many resources all synced
161+
mockManager := &mockInformerManager{
162+
allResources: []schema.GroupVersionResource{
163+
{Group: "", Version: "v1", Resource: "configmaps"},
164+
{Group: "", Version: "v1", Resource: "secrets"},
165+
{Group: "", Version: "v1", Resource: "services"},
166+
{Group: "apps", Version: "v1", Resource: "deployments"},
167+
{Group: "apps", Version: "v1", Resource: "statefulsets"},
168+
{Group: "", Version: "v1", Resource: "namespaces"},
169+
{Group: "rbac.authorization.k8s.io", Version: "v1", Resource: "clusterroles"},
170+
},
171+
syncedMap: map[schema.GroupVersionResource]bool{
172+
{Group: "", Version: "v1", Resource: "configmaps"}: true,
173+
{Group: "", Version: "v1", Resource: "secrets"}: true,
174+
{Group: "", Version: "v1", Resource: "services"}: true,
175+
{Group: "apps", Version: "v1", Resource: "deployments"}: true,
176+
{Group: "apps", Version: "v1", Resource: "statefulsets"}: true,
177+
{Group: "", Version: "v1", Resource: "namespaces"}: true,
178+
{Group: "rbac.authorization.k8s.io", Version: "v1", Resource: "clusterroles"}: true,
179+
},
180+
}
181+
182+
checker := ReadinessChecker(mockManager)
183+
err := checker(nil)
184+
185+
assert.NoError(t, err, "Should be ready when all informers are synced")
186+
}

pkg/webhook/readiness.go

Lines changed: 0 additions & 61 deletions
This file was deleted.

0 commit comments

Comments
 (0)