Skip to content

Conversation

@weng271190436
Copy link
Collaborator

@weng271190436 weng271190436 commented Dec 5, 2025

Description of your changes

I hit an issue with webhook informer cache not synced while working on another PR #366 where I enabled multiple webhook replicas and hit this issue frequently in e2e tests

The error I encountered is saying Namespace is not found to be a valid schema. This is due to some replicas are still getting api resources data from kube api server but they already report ready and start serving requests.

  [FAILED] Failed to create cluster resource placement
  Expected success, but got an error:
      <*errors.StatusError | 0xc0011c80a0>: 
      admission webhook "fleet.clusterresourceplacementv1beta1.validating" denied the request: deny create/update v1beta1 CRP has invalid fields the resource is not found in schema (please retry) or it is not a cluster scoped resource: /v1, Kind=Namespace
      {
          ErrStatus: {
              TypeMeta: {Kind: "", APIVersion: ""},
              ListMeta: {
                  SelfLink: "",
                  ResourceVersion: "",
                  Continue: "",
                  RemainingItemCount: nil,
              },
              Status: "Failure",
              Message: "admission webhook \"fleet.clusterresourceplacementv1beta1.validating\" denied the request: deny create/update v1beta1 CRP has invalid fields the resource is not found in schema (please retry) or it is not a cluster scoped resource: /v1, Kind=Namespace",
              Reason: "Forbidden",
              Details: nil,
              Code: 403,
          },
      }

I have:

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Special notes for your reviewer

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds a webhook readiness check that prevents the webhook from accepting requests until all resource informer caches are synced. This addresses an issue where webhook replicas could report ready and start serving requests before the discovery cache was fully populated, causing validation failures for valid resources.

Key Changes:

  • Adds a new ResourceInformerReadinessChecker function that verifies all informer caches are synced before marking the webhook pod as ready
  • Implements GetAllResources() method in the informer manager interface to retrieve all watched resources (both cluster-scoped and namespace-scoped)
  • Integrates the readiness check into the hub agent startup sequence after controllers are initialized

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/webhook/readiness.go New file implementing the readiness check function that validates all informer caches are synced
pkg/webhook/readiness_test.go Comprehensive unit tests for the readiness checker covering nil informer, no resources, synced/unsynced states
pkg/utils/informer/informermanager.go Adds GetAllResources() method to return all present resources (cluster + namespace scoped) with proper locking
pkg/utils/informer/informermanager_test.go Unit tests for GetAllResources() including edge cases and concurrency testing
test/utils/informer/manager.go Implements GetAllResources() in the fake manager for testing purposes
cmd/hubagent/main.go Integrates the readiness check after controller setup to ensure ResourceInformer is initialized

@weng271190436 weng271190436 force-pushed the weiweng/wait-for-webhook-informer-cache branch from 5527613 to 1b9dd16 Compare December 5, 2025 20:39
@codecov
Copy link

codecov bot commented Dec 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@weng271190436 weng271190436 force-pushed the weiweng/wait-for-webhook-informer-cache branch 2 times, most recently from 686f3e2 to 3048a51 Compare December 8, 2025 21:08
@weng271190436 weng271190436 force-pushed the weiweng/wait-for-webhook-informer-cache branch 2 times, most recently from a6a7d9e to 44004f9 Compare December 10, 2025 18:03
@weng271190436 weng271190436 force-pushed the weiweng/wait-for-webhook-informer-cache branch from 7d474b3 to a798337 Compare December 10, 2025 21:52
Wei Weng added 3 commits December 11, 2025 01:37
Signed-off-by: Wei Weng <Wei.Weng@microsoft.com>
Signed-off-by: Wei Weng <Wei.Weng@microsoft.com>
Signed-off-by: Wei Weng <Wei.Weng@microsoft.com>
@weng271190436 weng271190436 force-pushed the weiweng/wait-for-webhook-informer-cache branch from a798337 to 27ebb6b Compare December 11, 2025 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants