Gracefully handle missing shipped ConfigMaps at startup #3308

anjalii-28 · 2026-01-13T10:17:08Z

This change updates sharedmain to handle missing shipped ConfigMaps more gracefully
at startup.

Behavior:
-> Missing shipped ConfigMaps are treated as empty (defaults used) instead of panicking

A warning is logged when the ConfigMap is not found
Watchers remain active and pick up the ConfigMap if it is created later
No changes to existing observability restart semantics

Fixes #3195

Testing:

Unit tests in pkg/injection/sharedmain pass
Local failures in pkg/test/upgrade/shell are due to macOS Bash 3.2, CI runs Bash ≥4

knative-prow · 2026-01-13T10:17:19Z

Hi @anjalii-28. 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.

knative-prow · 2026-01-13T10:17:40Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: anjalii-28
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

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

codecov · 2026-01-13T10:29:06Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.61%. Comparing base (93d6642) to head (8b44dc9).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3308      +/-   ##
==========================================
- Coverage   74.63%   74.61%   -0.02%     
==========================================
  Files         188      188              
  Lines        8195     8210      +15     
==========================================
+ Hits         6116     6126      +10     
- Misses       1840     1843       +3     
- Partials      239      241       +2

☔ View full report in Codecov by Sentry.
📢 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.

dprotaso · 2026-01-14T16:20:37Z

/ok-to-test

dprotaso · 2026-01-14T17:27:08Z

configmap/informer/informed_watcher.go

+				}
+				// ConfigMap doesn't exist and is not defaulted. Log a warning but don't fail.
+				// The watcher will pick up the ConfigMap if it's created later.
+				log.Printf("WARNING: ConfigMap %q in namespace %q not found, using defaults and watching for creation", k, i.Namespace)


shouldn't use log package. we use a zap logger we'll probably want a new constructor that accepts a logger if we want these errors

dprotaso · 2026-01-14T17:40:01Z

injection/sharedmain/main.go

+	if _, err := kubeclient.Get(ctx).CoreV1().ConfigMaps(system.Namespace()).Get(ctx, cmName,
 		metav1.GetOptions{}); err == nil {
-		cmw.Watch(logging.ConfigMapName(), logging.UpdateLevelFromConfigMap(logger, atomicLevel, component))
-	} else if !apierrors.IsNotFound(err) {
-		logger.Fatalw("Error reading ConfigMap "+logging.ConfigMapName(), zap.Error(err))
+		cmw.Watch(cmName, logging.UpdateLevelFromConfigMap(logger, atomicLevel, component))
+	} else if apierrors.IsNotFound(err) {
+		// ConfigMap doesn't exist, but we still register a watcher so updates are picked up if it's created later.
+		logger.Warnw("ConfigMap "+cmName+" not found, using defaults and watching for creation", zap.String("configmap", cmName))
+		cmw.Watch(cmName, logging.UpdateLevelFromConfigMap(logger, atomicLevel, component))
+	} else {
+		logger.Fatalw("Error reading ConfigMap "+cmName, zap.Error(err))
 	}


Seems like we can simply call Watch now? and not have these if clauses here

dprotaso · 2026-01-14T17:40:24Z

injection/sharedmain/main.go

+	} else if apierrors.IsNotFound(err) {
+		// ConfigMap doesn't exist, but we still register a watcher so updates are picked up if it's created later.
+		logger.Warnw("ConfigMap "+cmName+" not found, using defaults and watching for creation", zap.String("configmap", cmName))
+		cmw.Watch(cmName, observers...)
+	} else {


Likewise we can simplify this and just call Watch and not worry about fetching the config maps?

Also one thing we could do for the observability config map is maybe kill the main context thus triggering a shutdown of the component. Then Kubernetes would restart the pod.

dprotaso · 2026-01-14T21:38:10Z

configmap/informer/informed_watcher.go


+	// Check which ConfigMaps exist and mark missing ones (without defaults) as done in the synced callback
+	// so we don't wait for them indefinitely
+	i.markMissingConfigMapsAsDone(s)


This behaviour should probably be opt-in for now using an option. Someone might be relying checkObservedResourcesExist to return the error and we don't want to break that.

dprotaso · 2026-01-14T21:39:16Z

configmap/informer/informed_watcher.go

+					// so we don't wait for it. The watcher will pick it up if it's created later.
+					s.MarkKeyAsDone(k)
+				}
+			}


should probably return an error if Get returns something besides IsNotFound no?

dprotaso · 2026-01-14T21:39:48Z

configmap/informer/informed_watcher_test.go

-			case err.Error() != tc.expectErr:
-				t.Fatal("Unexpected error =", err)
+			// Missing ConfigMaps should not cause Start to fail
+			if err != nil {


revert this - we shouldn't change the default behaviour

We should add a separate test for the new behaviour

dprotaso · 2026-01-14T23:06:18Z

injection/sharedmain/main_test.go

+	cmw := cminformer.NewInformedWatcher(kc, system.Namespace())
+
+	// Track if the update handler was invoked
+	var handlerInvoked sync.WaitGroup


It would be simpler to just have a channel that returns ConfigMaps

Then you could drop handlerMutex as well

dprotaso · 2026-01-14T23:06:58Z

injection/sharedmain/main_test.go

+	}
+
+	// Give the watcher time to fully start and sync
+	time.Sleep(500 * time.Millisecond)


We should probably use the config map watchers Wait method here

dprotaso · 2026-01-14T23:08:45Z

injection/sharedmain/main_test.go

+		// Handler was invoked, verify the ConfigMap
+		handlerMutex.Lock()
+		defer handlerMutex.Unlock()
+		if receivedConfigMap == nil {


We use cmp.Diff to make this simpler - see other examples in the codebase

Gracefully handle missing shipped ConfigMaps at startup

272b714

knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 13, 2026

knative-prow bot requested review from Leo6Leo and creydr January 13, 2026 10:17

Run gofmt

8b44dc9

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 Jan 14, 2026

dprotaso reviewed Jan 14, 2026

View reviewed changes

Gracefully handle missing shipped ConfigMaps at startup #3308

Are you sure you want to change the base?

Gracefully handle missing shipped ConfigMaps at startup #3308

Conversation

anjalii-28 commented Jan 13, 2026

Uh oh!

knative-prow bot commented Jan 13, 2026

Uh oh!

knative-prow bot commented Jan 13, 2026

Uh oh!

codecov bot commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dprotaso commented Jan 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Jan 13, 2026 •

edited

Loading