Skip to content

[v0.35] fix: CRD sync race in vcluster HA (#3993)#4002

Open
loft-bot wants to merge 1 commit into
v0.35from
backport/v0.35/pr-3993
Open

[v0.35] fix: CRD sync race in vcluster HA (#3993)#4002
loft-bot wants to merge 1 commit into
v0.35from
backport/v0.35/pr-3993

Conversation

@loft-bot

@loft-bot loft-bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Backport from main to v0.35

resolves ENGCP-939

Original PR Nr.: #3993

Backported Commits:

@loft-bot loft-bot requested a review from a team as a code owner June 12, 2026 12:21
@loft-bot loft-bot mentioned this pull request Jun 12, 2026
3 tasks
@loft-bot loft-bot force-pushed the backport/v0.35/pr-3993 branch from a809623 to 2a7418c Compare June 12, 2026 12:21
@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown

E2E Ginkgo Tests

Status Commit Run
Passed 3d228d781b05a8297876e9b722401434987b4c79 View run #27510037294

@jjaferson jjaferson force-pushed the backport/v0.35/pr-3993 branch from 2a7418c to 3d228d7 Compare June 14, 2026 19:46

@sowmyav27 sowmyav27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — the fix is correct and the core scenario is covered (first CRD update hits a conflict → retried → succeeds, no crash-loop).

Test coverage note (non-blocking — fine as a follow-up PR, not a blocker for this backport):

  1. The already-converged skip path isn't exercised. The GET stub returns the same un-updated CRD on every call, so apiequality.Semantic.DeepEqual(current, desired) is always false and the return nil early-return (skip the update when a peer replica already added the version) never runs — even though the test name says "…AndConverges". A case where the re-GET returns the already-updated CRD (and asserts a single update) would cover the realistic race-loser path, plus the re-GET→fresh-ResourceVersion retry mechanic.

  2. Error/edge branches of crdUpdateWithNewVersion are untested: version-not-found (newVersion == nil), GET error inside the retry loop, and non-conflict / exhausted-retry terminal error.

Both are missing-coverage items only — nothing is broken. They can land in a follow-up so this PR isn't held up.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants