Bugfix: an incomplete list response must not delete cached objects - #2746
Open
Rad710 wants to merge 1 commit into
Open
Bugfix: an incomplete list response must not delete cached objects#2746Rad710 wants to merge 1 commit into
Rad710 wants to merge 1 commit into
Conversation
saveListObject routed every multi-object list into ReplaceComponentList, which deletes cached objects absent from the response. But a ?fieldSelector=metadata.name= list or a single page of a paginated list carries only a subset: an empty name-selector response wiped the whole cached collection, and one page deleted the objects on the other pages. Route these to a no-delete merge path. A genuinely complete list still prunes (TestCompleteListStillPrunes). Trade-off: an always-paginating component keeps objects deleted in the cloud while disconnected, which beats serving a truncated collection after an offline reboot. Signed-off-by: Rolando Medina Rosner <rolmedro@gmail.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
1 participant
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.
What type of PR is this?
/kind bug
What this PR does / why we need it:
saveListObjectrouted every multi-object list response intoReplaceComponentList, which deletes every cached object of that type that is absent from the response — i.e. it treats the response as the complete, authoritative list. Two common list shapes carry only a subset, so this silently truncated the cache:?fieldSelector=metadata.name=<x>list (the named object does not exist) deleted the whole cached collection for that type.This routes those shapes to a no-delete merge path that writes the carried objects and prunes nothing. A genuinely complete list (an informer relist with no name selector, no pagination and no
resourceVersionMatch=Exact) still prunes, as before.Trade-off worth stating: a component that always paginates now keeps objects that were deleted in the cloud while yurthub was disconnected (GC still collects pods and events). Serving a stale-but-complete cached view is preferable to handing a truncated collection to a consumer.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
New
cache_truncation_test.gocovers the empty name-selector wipe, odd name-selector values, first/last page pagination, a cold cache, point-in-time lists, andTestCompleteListStillPrunes(a genuinely complete list must still prune). Each fails on the current code and passes with the fix.Does this PR introduce a user-facing change?