Skip to content

test(cni): Pin the empty egress group when no shard resolves - #1376

Merged
scotwells merged 1 commit into
mainfrom
claude/project-thread-pmmhz6
Nov 7, 2026
Merged

scotwells merged 1 commit into
mainfrom
claude/project-thread-pmmhz6

Conversation

@scotwells

Copy link
Copy Markdown
Contributor

Summary

Since #1372, a shard whose SID matches only a default route counts as unreachable, so a node that has lost every shard of a group now treats all of them as down at once. This adds two cases to the standby table test to pin what happens then: the group publishes no active shard, holds no NAT64 standby, and leaves every Maglev slot empty, so tenants hit group_empty instead of a dead shard. It covers a NAT64 group with a standby and a plain NAT66 group. The review on #1372 also asked for a log line, but none is added, because the existing "egress shard unreachable" warning already prints the default-route reason.

Test plan

  • The standby table test passes with the new cases
  • Lint passes for the egress route package
  • CI unit and root jobs pass
  • CI E2E passes


Generated by Claude Code

When every shard of a group fails to resolve, as happens when each one
matches only a default route, the group must publish no active shard, hold
no standby and leave every Maglev slot empty, so tenants hit group_empty
rather than a dead shard.
@scotwells
scotwells requested a review from a team as a code owner November 7, 2026 17:42
@scotwells
scotwells requested review from ecv and kevwilliams November 7, 2026 17:42

Copy link
Copy Markdown
Contributor Author

ci / Build and test failed in TestRetirementBacksOffWhileGatewayNeverAcknowledges (internal/serviceaddress) with first wait = 1.000112143s, want 1s. That package isn't part of this diff, which only adds cases to an egress route test.

The likely cause is in the test itself: the retirement backoff is time.Since(LastTransitionTime), and the fake client stores condition times at second precision. A transition late in a second is read back up to a second earlier, so the first wait can come out just over the 1s minimum. It passed 50 times in a row locally under -race. I've re-run the job once.

No fix exists on main yet. A suggested patch, kept out of this PR: have the test assert the first wait is between minRetirementBackoff and minRetirementBackoff + time.Second, or have the reconciler return the minimum for the first wait.


Generated by Claude Code

@ecv ecv 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.

LGTM. Both cases pin the all-unreachable outcome #1372 created: empty group, no NAT64 standby held, so tenants hit group_empty rather than a dead shard. Unit job green at this head.

@scotwells
scotwells merged commit 6b19317 into main Nov 7, 2026
28 of 29 checks passed
@scotwells
scotwells deleted the claude/project-thread-pmmhz6 branch November 7, 2026 18:40
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.

2 participants