Use klog.ErrorS instead of klog.V(2).InfoS for decode failures in tombstone handlers - #8173
Open
Archong-Liu wants to merge 1 commit into
Open
Conversation
Commit 1ba8d62 (antrea-io#7964) added cache.DeletedFinalStateUnknown tombstone handling to these 5 controller-side delete handlers, but logged the "both assertions failed" branches at klog.V(2).InfoS. Every other decode-failure handler in the codebase (networkpolicy, grouping, stats, ipam, supportbundlecollection, and the two agent-side handlers fixed in antrea-io#8161) uses klog.Errorf/ErrorS for this exact situation -- these 5 files are the only place V(2).InfoS shows up for this pattern anywhere in the tree. These branches only fire when both the direct type assertion and the tombstone-unwrapped assertion fail, meaning the informer delivered a completely unexpected object type -- a real bug (miswired informer, wrong indexer), not a routine event like a normal tombstone recovery. That's an actionable failure, matching what antrea-io#8161's review discussion concluded for the two agent-side handlers with the same bug. No behavior change beyond the log level; full test suites for all 5 affected packages pass. Signed-off-by: ArchongLiu <aaarrchong@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adjusts logging severity for tombstone / type-assertion decode failures in several controller delete handlers, aligning them with existing project logging guidance by using klog.ErrorS instead of klog.V(2).InfoS for these unexpected conditions.
Changes:
- Updated Traceflow controller delete handler to log unexpected object / tombstone types at error level.
- Updated LabelIdentity, ExternalNode, ExternalIPPool, and Egress controller delete handlers to log decode failures at error level.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/controller/traceflow/controller.go | Promotes unexpected delete-object decode logs from verbose info to klog.ErrorS. |
| pkg/controller/labelidentity/controller.go | Promotes invalid-type delete decode logs to klog.ErrorS. |
| pkg/controller/externalnode/controller.go | Promotes invalid-type delete decode logs to klog.ErrorS. |
| pkg/controller/externalippool/controller.go | Promotes invalid-type delete decode logs to klog.ErrorS. |
| pkg/controller/egress/controller.go | Promotes invalid-type delete decode logs to klog.ErrorS. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
@antoninbas a simple fix, would appreciate a quick review. Thank you! |
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.
Follow-up to #8161 antoninbas pointed out those handlers logged decode failures at
V(2).InfoSwhereErrorSreflects the behavior better. Same fix, applied to the other 5 files #7964 touched, which had the identical inconsistency.No behavior change, just log level. Tests pass.