Tolerate missing config ConfigMaps at webhook startup by rishabhsvats · Pull Request #16686 · knative/serving

rishabhsvats · 2026-09-02T10:10:38Z

The config Store registers its ConfigMaps with the InformedWatcher using plain Watch(), so the watcher fatals at startup if config-defaults, config-features, or config-autoscaler is absent. On a cold start this deadlocks against the config validating webhook (failurePolicy: Fail), which cannot admit the ConfigMaps while the webhook itself is down.

Proposed Changes

Override Store.WatchConfigs to use WatchWithDefault when the watcher implements DefaultingWatcher, registering a synthetic empty default for each ConfigMap.
The InformedWatcher then treats a missing ConfigMap as "use the default" instead of failing, so the webhook boots on built-in defaults and can then admit the real ConfigMaps.
Real values are picked up via OnConfigChanged as soon as the ConfigMaps appear. Watchers that aren't DefaultingWatcher fall back to the previous Watch() behaviour.

Release Note

Fix a webhook startup deadlock where the config ConfigMaps could not be created because the webhook was waiting on those same ConfigMaps to start.

The config Store registers its ConfigMaps with the InformedWatcher using plain Watch(), so the watcher fatals at startup if config-defaults, config-features, or config-autoscaler is absent. On a cold start this deadlocks against the config validating webhook (failurePolicy: Fail), which cannot admit the ConfigMaps while the webhook itself is down. Override Store.WatchConfigs to use WatchWithDefault when the watcher is a DefaultingWatcher, registering an empty default per ConfigMap so startup no longer fails. Real ConfigMap values are observed as soon as they appear. Fixes knative#16658 Signed-off-by: Rishabh Singh <rishabhsvats@gmail.com>

linux-foundation-easycla · 2026-09-02T10:10:46Z

The committers listed above are authorized under a signed CLA.

✅ login: rishabhsvats / name: Rishabh Singh (050d265)

knative-prow · 2026-09-02T10:10:49Z

Welcome @rishabhsvats! It looks like this is your first PR to knative/serving 🎉

knative-prow · 2026-09-02T10:10:49Z

Hi @rishabhsvats. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

dsimansk · 2026-09-04T07:07:57Z

/ok-to-test

codecov · 2026-09-04T07:14:24Z

Codecov Report

❌ Patch coverage is 11.11111% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.33%. Comparing base (618b0c2) to head (050d265).

Files with missing lines	Patch %	Lines
pkg/apis/config/store.go	11.11%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #16686      +/-   ##
==========================================
+ Coverage   80.29%   80.33%   +0.04%     
==========================================
  Files         217      217              
  Lines       13560    13564       +4     
==========================================
+ Hits        10888    10897       +9     
+ Misses       2309     2306       -3     
+ Partials      363      361       -2

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dsimansk · 2026-09-04T11:10:20Z

+		// This prevents Start() from failing if ConfigMaps don't exist
+		for name := range configConstructors {
+			dw.WatchWithDefault(corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Name: name},


It seems that the watcher wouldn't be executed due to missing Namespace: system.Namespace(). Could be verified by adding unit tests to cover chhanges.

dsimansk

Nit 1: missing namespace
Nit 2: no test coverage for the changed default behavior

I'm concerned that the change is not scoped to webhook only, as the store impl is used in revision controller as well. And inherently revision controller might be explicitly relaying on rejecting early mechanism not to start on missing config.

Could you chime in folks?
/cc @dprotaso @linkvt

**Webhook-specific scoping:** - Renamed WatchConfigs -> WatchConfigsWithDefaults - Revision controller continues using WatchConfigs (fail-fast behavior) **Fixes:** - Nit 1: Add namespace parameter, pass system.Namespace() - Nit 2: Add unit tests for the new default configuration watcher behavior. The tests verify that all required ConfigMaps are registered with the watcher, the correct namespace is used, default configuration updates the Store through the registered callbacks, real ConfigMaps replace the defaults, and watchers that do not support defaulting continue to use the existing Watch behavior. Signed-off-by: Rishabh Singh <rishabhsvats@gmail.com>

knative-prow · 2026-09-05T17:20:57Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rishabhsvats
Once this PR has been reviewed and has the lgtm label, please assign dprotaso for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS
pkg/apis/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rishabhsvats · 2026-09-05T17:26:19Z

Nit 1 (missing namespace): Added namespace parameter to WatchConfigsWithDefaults and passing system.Namespace() from webhook.

Nit 2 (no test coverage): Added 5 tests covering tests to verify that all required ConfigMaps are registered with the watcher, the correct namespace is used, default configuration updates the Store through the registered callbacks, real ConfigMaps replace the defaults, and watchers that do not support defaulting continue to use the existing Watch behavior.

This change is now webhook specific. Created new method WatchConfigsWithDefaults used only by webhook controllers. Revision controller continues using existing behavior.

knative-prow Bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 2, 2026

knative-prow Bot requested review from dsimansk and skonto September 2, 2026 10:11

knative-prow Bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 4, 2026

dsimansk reviewed Sep 4, 2026

View reviewed changes

dsimansk requested changes Sep 4, 2026

View reviewed changes

knative-prow Bot requested review from dprotaso and linkvt September 4, 2026 11:13

knative-prow Bot assigned dsimansk Sep 4, 2026

knative-prow Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 5, 2026

Navigation Menu

Uh oh!

Tolerate missing config ConfigMaps at webhook startup - #16686

Tolerate missing config ConfigMaps at webhook startup#16686
rishabhsvats wants to merge 2 commits into
knative:mainfrom
rishabhsvats:fix-16658-webhook-configmap-deadlock

rishabhsvats commented Sep 2, 2026

Uh oh!

linux-foundation-easycla Bot commented Sep 2, 2026 •

edited

Loading

Uh oh!

knative-prow Bot commented Sep 2, 2026

Uh oh!

knative-prow Bot commented Sep 2, 2026

Uh oh!

dsimansk commented Sep 4, 2026

Uh oh!

codecov Bot commented Sep 4, 2026

Uh oh!

dsimansk Sep 4, 2026 •

edited

Loading

Uh oh!

dsimansk left a comment •

edited

Loading

Uh oh!

knative-prow Bot commented Sep 5, 2026

Uh oh!

rishabhsvats commented Sep 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

rishabhsvats commented Sep 2, 2026

Proposed Changes

Uh oh!

linux-foundation-easycla Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knative-prow Bot commented Sep 2, 2026

Uh oh!

knative-prow Bot commented Sep 2, 2026

Uh oh!

dsimansk commented Sep 4, 2026

Uh oh!

codecov Bot commented Sep 4, 2026

Codecov Report

Uh oh!

dsimansk Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsimansk left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knative-prow Bot commented Sep 5, 2026

Uh oh!

rishabhsvats commented Sep 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

linux-foundation-easycla Bot commented Sep 2, 2026 •

edited

Loading

dsimansk Sep 4, 2026 •

edited

Loading

dsimansk left a comment •

edited

Loading