Skip to content

fix cached resource replication after leader election loss - #4552

Open
nfarhadian wants to merge 1 commit into
kcp-dev:mainfrom
nfarhadian:fix-4451-ccr-replication-lease-loss
Open

nfarhadian wants to merge 1 commit into
kcp-dev:mainfrom
nfarhadian:fix-4451-ccr-replication-lease-loss

Conversation

@nfarhadian

Copy link
Copy Markdown

Summary

ClusterCachedResource replication stops after the shard loses the leader lease, and status stays Ready.

Global (cache) informers were run with Run(ctx.Done()) on the leader election context. On lease loss they stop but stay in CachePartialMetadataDDSIF, so on the next term ForResource returns the stopped informer, AddEventHandler fails with "stopped already" and nothing is replicated anymore. Same happens silently when the CRD is deleted and recreated: the local informer is stopped, the replication controller keeps it and nothing requeues the ClusterCachedResource.

Changes:

  • informers are started with the factories' Start, so they are not tied to leadership
  • the replication controller adds its event handlers in Start and removes them on exit, and stops when one of its informers is stopped. The ClusterCachedResource is requeued and the controller is recreated
  • we don't create a controller for a resource the local factory doesn't serve (REST mapper can be behind discovery)
  • ReplicationStarted and phase are now set on every reconcile from the controller state, instead of once when the controller is registered
  • when a CRD comes back, matching ClusterCachedResources are enqueued with their backoff reset, otherwise they can wait up to 1000s

Notes:

  • CachePartialMetadataDDSIF still has no worker. Adding one would start metadata informers for every cache server CRD on every shard. The cost is that global informers run until the process exits; if a cache CRD is deleted its reflector keeps retrying.
  • On startup each ClusterCachedResource can log one "not served by local informers" error until local discovery catches up. It is retried with backoff.
  • Phase is Initializing while replication isn't running, there is no better phase in the API today.
  • Not fixed here, found during review and exists on main: listener removal in AddGVRLifecycleHandler can race with updateInformers (pkg/informer/informer.go), and this PR adds one more listener that goes away on lease loss.

Tested with unit tests and manually with kcp start --feature-gates=CacheAPIs=true --enable-leader-election=true: lease taken away and back, CRD deleted and recreated, CRD absent for 45s and recreated. Replication resumes in each case and status goes Initializing/NotRunning while the CRD is gone.

What Type of PR Is This?

/kind bug

Related Issue(s)

Fixes #4451

Release Notes

Fix ClusterCachedResource replication stopping after leader election loss or CRD recreation while status stayed Ready.

Signed-off-by: Naeem Farhadian <n.f.azizi@gmail.com>
@kcp-ci-bot kcp-ci-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. dco-signoff: yes Indicates the PR's author has signed the DCO. labels Nov 8, 2026
@kcp-ci-bot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign xmudrii 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:

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

@kcp-ci-bot kcp-ci-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 8, 2026
@kcp-ci-bot

Copy link
Copy Markdown
Contributor

Hi @nfarhadian. Thanks for your PR.

I'm waiting for a kcp-dev 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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-signoff: yes Indicates the PR's author has signed the DCO. kind/bug Categorizes issue or PR as related to a bug. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: ClusterCachedResource replication stops after a leader-lease loss; status stays Ready

2 participants