Fix LC migration informer relist flake - #4264
Merged
Merged
Conversation
…sions A LogicalClusterMigration moves a logical cluster to a shard with an independent resource version space. Informers/watchers that were established before the migration relist after their watch is severed and send their last-observed (origin-shard) resource version to the destination shard. The migrating filter answered every blocked request with 503 + Retry-After, which the REST client silently retries. The reflector therefore never dropped its foreign resource version, and the destination shard's watch cache blocked on resourceVersionMatch=NotOlderThan until its own unrelated resource version counter happened to catch up. This raced the 30s test timeout, making TestFullMigration/PreMigrateInformer flake (#4250). For list/watch requests carrying a resourceVersion, return 410 Gone (Expired) without a Retry-After header instead. The REST client surfaces the error, the reflector resets its resource version and relists from scratch, which resolves cleanly against the destination shard. Other requests keep the 503 Retry-After behaviour so they transparently retry once the migration completes.
mjudeikis
force-pushed
the
fix/lc-migration-informer-relist-4250
branch
from
July 14, 2026 07:48
23714dc to
f76fdfe
Compare
Contributor
Author
|
/retest |
Contributor
Author
|
/retest |
Contributor
|
LGTM label has been added. DetailsGit tree hash: 1e31e9ce91d9b3d4d1f9973c5c61b6849a737dd0 |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: xrstf The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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
Fixes the
TestFullMigration/PreMigrateInformerflake tracked in #4250.A
LogicalClusterMigrationmoves a logical cluster to a shard with an independent resource version space. An informer/watcher established before the migration has its watch severed (cancelLogicalClusterConnections) and then relists, sending its last-observed origin-shard resource version to the destination shard.The migrating filter (
WithBlockMigratingLogicalClusters) previously answered every blocked request with503 + Retry-After: 1. The client-go REST layer silently retries429/5xxresponses that carry aRetry-Afterheader (with_retry.gocheckWait), so the error never surfaced to the reflector and it never dropped its foreign resource version. The destination shard's watch cache then blocked onresourceVersionMatch=NotOlderThanuntil its own, unrelated resource version counter organically caught up — which races the 30s test timeout and is inherently non-deterministic (passes when the destination RV is already ahead, hangs when it is behind).Fix
For list/watch requests that carry a
resourceVersion, the filter now returns410 Gone(Expired) without aRetry-Afterheader. The REST client surfaces this instead of swallowing it, so the reflector resets its resource version and relists from scratch (RV="", a consistent read), which resolves cleanly against the destination shard once migration completes.All other requests keep the
503 Retry-Afterbehaviour so they transparently retry once migration finishes. Thesystem:kcp:external-logical-cluster-adminbypass (used by the destination shard to pull the dump) is unchanged.Test
TestWithBlockMigratingLogicalClusterscovering the pass-through, external-admin,503 Retry-After, and410 Expiredpaths.test/e2e/logicalclustermigrationPreMigrateInformersubtests./kind flake
Fixes #4250
Release Notes