Use informer lister, context-aware shutdown, and standard logging#50
Merged
Conversation
- Read namespaces from the informer's cached lister instead of issuing a fresh List API call on every event, and sort the result so the joined ConfigMap value is stable. Non-deterministic cache order could otherwise flip the value and trigger spurious container restarts. - Replace the ad-hoc stop channel and os.Exit(0) with signal.NotifyContext, and thread the context through the ConfigMap API calls so in-flight requests are cancelled on shutdown. - Surface a cache-sync failure as a fatal startup error instead of silently continuing. - Switch fmt.Printf/Println logging to the standard log package for timestamps, and use log.Fatalf for startup failures. Tests build and sync a namespace lister from the fake clientset, mirroring production, and assert the sorted ordering. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The three deferred code improvements to
main.go. Behaviour-preserving except where noted (the lister sort fixes a latent spurious-restart bug).Informer lister instead of repeated List calls
updateConfigMappreviously issued a freshNamespaces().List()API call on every informer event. It now reads from the informer's cached lister — the data is already in memory.Context-aware graceful shutdown
stopCh+signalCh+os.Exit(0)(which skipped deferred cleanup) withsignal.NotifyContext.Get/Update/Createcalls (no morecontext.TODO()), so in-flight requests are cancelled on shutdown.Standard logging
fmt.Printf/fmt.Printlnlogging to the standardlogpackage (timestamps), andlog.Fatalffor startup failures.Tests
main_test.gonow builds and syncs a namespace lister from the fake clientset (mirroring production) and asserts the sorted ordering. All existing assertions retained, including the nil-map panic regression test.Verification (local)
gofmt -l .→ cleango vet ./...✅make test✅ (4 tests pass)make build(-race) ✅🤖 Generated with Claude Code