[PR #6452] [management] fix flaky Test_SaveAccount_Large from random IP collision #29551

Closed
opened 2026-08-05 08:08:18 -04:00 by saavagebueno · 0 comments
Owner

Original Pull Request: https://github.com/netbirdio/netbird/pull/6452

State: closed
Merged: Yes


Describe your changes

Test_SaveAccount_Large was flaky on the mysql (and any) engine, failing intermittently with:

sql_store_test.go:79: expecting Account to have 6000 peers stored after SaveAccount(), got 5999

Root cause: each of the 6000 test peers got its IP from randomIPv4(), which
returned 4 fully random bytes with no de-duplication. The peers table has a
unique index idx_account_ip ON peers (account_id, ip), and since every peer
belongs to the same account, two colliding random IPv4 addresses caused the
unique index to drop one peer on insert — leaving 5999 instead of 6000. With
6000 addresses in a 32-bit space the birthday-collision probability is ~0.4%
per run.

Fix: replace randomIPv4() with sequentialIPv4(n), which derives a unique,
deterministic address (10.0.0.0 + n, base 0x0A000000) per peer index. With
6000 peers the range stays inside 10.0.0.0/8 (max 10.0.23.111) and every IP
is unique, so the unique index never drops a row. Removed the now-unused
math/rand import.

No public issue. Internal flaky-test fix for the management store suite; the
collision happens in randomIPv4 used by runLargeTest in
management/server/store/sql_store_test.go.

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Test-only change. No public CLI, API, configuration, or behavior change — only
the test's peer-IP generation is made deterministic to remove a pre-existing
flake.

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

N/A

Summary by CodeRabbit

  • Tests
    • Improved the large-account test data generator to use predictable, non-colliding IPv4 addresses.
    • Reduced the chance of intermittent uniqueness-related test failures during large save scenarios.
**Original Pull Request:** https://github.com/netbirdio/netbird/pull/6452 **State:** closed **Merged:** Yes --- ## Describe your changes `Test_SaveAccount_Large` was flaky on the `mysql` (and any) engine, failing intermittently with: sql_store_test.go:79: expecting Account to have 6000 peers stored after SaveAccount(), got 5999 Root cause: each of the 6000 test peers got its IP from `randomIPv4()`, which returned 4 fully random bytes with no de-duplication. The `peers` table has a unique index `idx_account_ip ON peers (account_id, ip)`, and since every peer belongs to the same account, two colliding random IPv4 addresses caused the unique index to drop one peer on insert — leaving 5999 instead of 6000. With 6000 addresses in a 32-bit space the birthday-collision probability is ~0.4% per run. Fix: replace `randomIPv4()` with `sequentialIPv4(n)`, which derives a unique, deterministic address (`10.0.0.0 + n`, base `0x0A000000`) per peer index. With 6000 peers the range stays inside `10.0.0.0/8` (max `10.0.23.111`) and every IP is unique, so the unique index never drops a row. Removed the now-unused `math/rand` import. ## Issue ticket number and link No public issue. Internal flaky-test fix for the management store suite; the collision happens in `randomIPv4` used by `runLargeTest` in `management/server/store/sql_store_test.go`. ## Stack <!-- branch-stack --> ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) Test-only change. No public CLI, API, configuration, or behavior change — only the test's peer-IP generation is made deterministic to remove a pre-existing flake. ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: N/A <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved the large-account test data generator to use predictable, non-colliding IPv4 addresses. * Reduced the chance of intermittent uniqueness-related test failures during large save scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
saavagebueno added the pull-request label 2026-08-05 08:08:18 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#29551