webhook readiness wait for informer cache sync

Signed-off-by: Wei Weng <Wei.Weng@microsoft.com>

Author: Wei Weng

cmd/hubagent/main.go

@@ -46,6 +46,7 @@ import (
 	"github.com/kubefleet-dev/kubefleet/cmd/hubagent/options"
 	"github.com/kubefleet-dev/kubefleet/cmd/hubagent/workload"
 	mcv1beta1 "github.com/kubefleet-dev/kubefleet/pkg/controllers/membercluster/v1beta1"
+	"github.com/kubefleet-dev/kubefleet/pkg/utils/validator"
 	"github.com/kubefleet-dev/kubefleet/pkg/webhook"
 	// +kubebuilder:scaffold:imports
 )
@@ -164,10 +165,19 @@ func main() {
 
 	ctx := ctrl.SetupSignalHandler()
 	if err := workload.SetupControllers(ctx, &wg, mgr, config, opts); err != nil {
-		klog.ErrorS(err, "unable to set up ready check")
+		klog.ErrorS(err, "unable to set up controllers")
 		exitWithErrorFunc()
 	}
 
+	// Add webhook readiness check AFTER controllers are set up (when ResourceInformer is initialized)
+	// This prevents webhook from accepting requests before discovery cache is populated
+	if opts.EnableWebhook {
+		if err := mgr.AddReadyzCheck("webhook-cache", webhook.ResourceInformerReadinessChecker(validator.ResourceInformer)); err != nil {
+			klog.ErrorS(err, "unable to set up webhook readiness check")
+			exitWithErrorFunc()
+		}
+	}
+
 	// +kubebuilder:scaffold:builder
 
 	wg.Add(1)

pkg/utils/informer/informermanager.go

@@ -61,6 +61,9 @@ type Manager interface {
 	// GetNameSpaceScopedResources returns the list of namespace scoped resources we are watching.
 	GetNameSpaceScopedResources() []schema.GroupVersionResource
 
+	// GetAllResources returns the list of all resources (both cluster-scoped and namespace-scoped) we are watching.
+	GetAllResources() []schema.GroupVersionResource
+
 	// IsClusterScopedResources returns if a resource is cluster scoped.
 	IsClusterScopedResources(resource schema.GroupVersionKind) bool
 
@@ -224,6 +227,19 @@ func (s *informerManagerImpl) GetNameSpaceScopedResources() []schema.GroupVersio
 	return res
 }
 
+func (s *informerManagerImpl) GetAllResources() []schema.GroupVersionResource {
+	s.resourcesLock.RLock()
+	defer s.resourcesLock.RUnlock()
+
+	res := make([]schema.GroupVersionResource, 0, len(s.apiResources))
+	for _, resource := range s.apiResources {
+		if resource.isPresent {
+			res = append(res, resource.GroupVersionResource)
+		}
+	}
+	return res
+}
+
 func (s *informerManagerImpl) IsClusterScopedResources(gvk schema.GroupVersionKind) bool {
 	s.resourcesLock.RLock()
 	defer s.resourcesLock.RUnlock()

pkg/utils/informer/informermanager_test.go

@@ -0,0 +1,324 @@
+/*
+Copyright 2025 The KubeFleet Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package informer
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/dynamic/fake"
+	"k8s.io/client-go/kubernetes/scheme"
+)
+
+func TestGetAllResources(t *testing.T) {
+	tests := []struct {
+		name                     string
+		namespaceScopedResources []APIResourceMeta
+		clusterScopedResources   []APIResourceMeta
+		staticResources          []APIResourceMeta
+		expectedResourceCount    int
+		expectedNamespacedCount  int
+	}{
+		{
+			name: "mixed cluster and namespace scoped resources",
+			namespaceScopedResources: []APIResourceMeta{
+				{
+					GroupVersionKind: schema.GroupVersionKind{
+						Group:   "",
+						Version: "v1",
+						Kind:    "ConfigMap",
+					},
+					GroupVersionResource: schema.GroupVersionResource{
+						Group:    "",
+						Version:  "v1",
+						Resource: "configmaps",
+					},
+					IsClusterScoped: false,
+				},
+				{
+					GroupVersionKind: schema.GroupVersionKind{
+						Group:   "",
+						Version: "v1",
+						Kind:    "Secret",
+					},
+					GroupVersionResource: schema.GroupVersionResource{
+						Group:    "",
+						Version:  "v1",
+						Resource: "secrets",
+					},
+					IsClusterScoped: false,
+				},
+			},
+			clusterScopedResources: []APIResourceMeta{
+				{
+					GroupVersionKind: schema.GroupVersionKind{
+						Group:   "",
+						Version: "v1",
+						Kind:    "Namespace",
+					},
+					GroupVersionResource: schema.GroupVersionResource{
+						Group:    "",
+						Version:  "v1",
+						Resource: "namespaces",
+					},
+					IsClusterScoped: true,
+				},
+			},
+			staticResources: []APIResourceMeta{
+				{
+					GroupVersionKind: schema.GroupVersionKind{
+						Group:   "",
+						Version: "v1",
+						Kind:    "Node",
+					},
+					GroupVersionResource: schema.GroupVersionResource{
+						Group:    "",
+						Version:  "v1",
+						Resource: "nodes",
+					},
+					IsClusterScoped:  true,
+					isStaticResource: true,
+				},
+			},
+			expectedResourceCount:   4, // All resources including static
+			expectedNamespacedCount: 2, // Only namespace-scoped, excluding static
+		},
+		{
+			name:                    "no resources",
+			expectedResourceCount:   0,
+			expectedNamespacedCount: 0,
+		},
+		{
+			name: "only namespace scoped resources",
+			namespaceScopedResources: []APIResourceMeta{
+				{
+					GroupVersionKind: schema.GroupVersionKind{
+						Group:   "apps",
+						Version: "v1",
+						Kind:    "Deployment",
+					},
+					GroupVersionResource: schema.GroupVersionResource{
+						Group:    "apps",
+						Version:  "v1",
+						Resource: "deployments",
+					},
+					IsClusterScoped: false,
+				},
+			},
+			expectedResourceCount:   1,
+			expectedNamespacedCount: 1,
+		},
+		{
+			name: "only cluster scoped resources",
+			clusterScopedResources: []APIResourceMeta{
+				{
+					GroupVersionKind: schema.GroupVersionKind{
+						Group:   "rbac.authorization.k8s.io",
+						Version: "v1",
+						Kind:    "ClusterRole",
+					},
+					GroupVersionResource: schema.GroupVersionResource{
+						Group:    "rbac.authorization.k8s.io",
+						Version:  "v1",
+						Resource: "clusterroles",
+					},
+					IsClusterScoped: true,
+				},
+			},
+			expectedResourceCount:   1,
+			expectedNamespacedCount: 0,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create a fake dynamic client
+			fakeClient := fake.NewSimpleDynamicClient(scheme.Scheme)
+			stopCh := make(chan struct{})
+			defer close(stopCh)
+
+			mgr := NewInformerManager(fakeClient, 0, stopCh)
+			implMgr := mgr.(*informerManagerImpl)
+
+			// Add namespace-scoped resources
+			for _, res := range tt.namespaceScopedResources {
+				res.isPresent = true
+				implMgr.apiResources[res.GroupVersionKind] = &res
+			}
+
+			// Add cluster-scoped resources
+			for _, res := range tt.clusterScopedResources {
+				res.isPresent = true
+				implMgr.apiResources[res.GroupVersionKind] = &res
+			}
+
+			// Add static resources
+			for _, res := range tt.staticResources {
+				res.isPresent = true
+				implMgr.apiResources[res.GroupVersionKind] = &res
+			}
+
+			// Test GetAllResources
+			allResources := mgr.GetAllResources()
+			assert.Equal(t, tt.expectedResourceCount, len(allResources), "GetAllResources should return correct count")
+
+			// Verify all expected resources are present
+			resourceMap := make(map[schema.GroupVersionResource]bool)
+			for _, gvr := range allResources {
+				resourceMap[gvr] = true
+			}
+
+			for _, res := range tt.namespaceScopedResources {
+				assert.True(t, resourceMap[res.GroupVersionResource], "namespace-scoped resource %v should be in GetAllResources", res.GroupVersionResource)
+			}
+
+			for _, res := range tt.clusterScopedResources {
+				assert.True(t, resourceMap[res.GroupVersionResource], "cluster-scoped resource %v should be in GetAllResources", res.GroupVersionResource)
+			}
+
+			for _, res := range tt.staticResources {
+				assert.True(t, resourceMap[res.GroupVersionResource], "static resource %v should be in GetAllResources", res.GroupVersionResource)
+			}
+
+			// Test GetNameSpaceScopedResources
+			namespacedResources := mgr.GetNameSpaceScopedResources()
+			assert.Equal(t, tt.expectedNamespacedCount, len(namespacedResources), "GetNameSpaceScopedResources should return correct count")
+
+			// Verify only namespace-scoped, non-static resources are present
+			namespacedMap := make(map[schema.GroupVersionResource]bool)
+			for _, gvr := range namespacedResources {
+				namespacedMap[gvr] = true
+			}
+
+			for _, res := range tt.namespaceScopedResources {
+				assert.True(t, namespacedMap[res.GroupVersionResource], "namespace-scoped resource %v should be in GetNameSpaceScopedResources", res.GroupVersionResource)
+			}
+
+			// Verify cluster-scoped and static resources are NOT in namespace-scoped list
+			for _, res := range tt.clusterScopedResources {
+				assert.False(t, namespacedMap[res.GroupVersionResource], "cluster-scoped resource %v should NOT be in GetNameSpaceScopedResources", res.GroupVersionResource)
+			}
+
+			for _, res := range tt.staticResources {
+				assert.False(t, namespacedMap[res.GroupVersionResource], "static resource %v should NOT be in GetNameSpaceScopedResources", res.GroupVersionResource)
+			}
+		})
+	}
+}
+
+func TestGetAllResources_NotPresent(t *testing.T) {
+	// Test that resources marked as not present are excluded
+	fakeClient := fake.NewSimpleDynamicClient(scheme.Scheme)
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+
+	mgr := NewInformerManager(fakeClient, 0, stopCh)
+	implMgr := mgr.(*informerManagerImpl)
+
+	// Add a resource that is present
+	presentRes := APIResourceMeta{
+		GroupVersionKind: schema.GroupVersionKind{
+			Group:   "",
+			Version: "v1",
+			Kind:    "ConfigMap",
+		},
+		GroupVersionResource: schema.GroupVersionResource{
+			Group:    "",
+			Version:  "v1",
+			Resource: "configmaps",
+		},
+		IsClusterScoped: false,
+		isPresent:       true,
+	}
+	implMgr.apiResources[presentRes.GroupVersionKind] = &presentRes
+
+	// Add a resource that is NOT present (deleted)
+	notPresentRes := APIResourceMeta{
+		GroupVersionKind: schema.GroupVersionKind{
+			Group:   "",
+			Version: "v1",
+			Kind:    "Secret",
+		},
+		GroupVersionResource: schema.GroupVersionResource{
+			Group:    "",
+			Version:  "v1",
+			Resource: "secrets",
+		},
+		IsClusterScoped: false,
+		isPresent:       false,
+	}
+	implMgr.apiResources[notPresentRes.GroupVersionKind] = &notPresentRes
+
+	allResources := mgr.GetAllResources()
+	assert.Equal(t, 1, len(allResources), "should only return present resources")
+	assert.Equal(t, presentRes.GroupVersionResource, allResources[0], "should return the present resource")
+}
+
+func TestInformerManagerConcurrency(t *testing.T) {
+	// Test that GetAllResources and GetNameSpaceScopedResources are safe for concurrent access
+	fakeClient := fake.NewSimpleDynamicClient(scheme.Scheme)
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+
+	mgr := NewInformerManager(fakeClient, 0, stopCh)
+	implMgr := mgr.(*informerManagerImpl)
+
+	// Add some resources
+	for i := 0; i < 10; i++ {
+		res := APIResourceMeta{
+			GroupVersionKind: schema.GroupVersionKind{
+				Group:   "",
+				Version: "v1",
+				Kind:    "Resource" + string(rune(i)),
+			},
+			GroupVersionResource: schema.GroupVersionResource{
+				Group:    "",
+				Version:  "v1",
+				Resource: "resource" + string(rune(i)) + "s",
+			},
+			IsClusterScoped: i%2 == 0, // Alternate between cluster and namespace scoped
+			isPresent:       true,
+		}
+		implMgr.apiResources[res.GroupVersionKind] = &res
+	}
+
+	// Run concurrent reads
+	done := make(chan bool, 100)
+	for i := 0; i < 50; i++ {
+		go func() {
+			mgr.GetAllResources()
+			done <- true
+		}()
+		go func() {
+			mgr.GetNameSpaceScopedResources()
+			done <- true
+		}()
+	}
+
+	// Wait for all goroutines with timeout
+	timeout := time.After(5 * time.Second)
+	for i := 0; i < 100; i++ {
+		select {
+		case <-done:
+			// Success
+		case <-timeout:
+			t.Fatal("Timeout waiting for concurrent reads to complete")
+		}
+	}
+}