Skip to content

Conversation

@robmry
Copy link
Contributor

@robmry robmry commented Jun 28, 2024

- What I did

Keep allocated subnets in-order, so that they're not mistakenly reallocated due to a gap in the list where misplaced subnets should have been.

- How I did it

The iterator over allocated subnets was incremented too early, this change moves it past three clauses in addrSpace.allocatePredefinedPool().

- How to verify it

The three new unit tests correspond to a separate failure caused by incrementing before each of them.

Without the fix, the three new tests fail like this ...

=== FAIL: libnetwork/ipams/defaultipam TestPoolAllocateAndRelease/allocate_after_reserved (0.00s)
    address_space_test.go:475: assertion failed: reserved.Overlaps(subnet) is true: subnet 10.0.1.0/24 allocated in reserved range 10.0.0.0/16

=== FAIL: libnetwork/ipams/defaultipam TestPoolAllocateAndRelease/reallocate_first_subnet (0.00s)
    address_space_test.go:472: assertion failed: exists is true: subnet 10.0.0.0/24 allocated to n4, reallocated for n5

=== FAIL: libnetwork/ipams/defaultipam TestPoolAllocateAndRelease/reallocate_after_release (0.00s)
    address_space_test.go:472: assertion failed: exists is true: subnet 10.0.1.0/24 allocated to n5, reallocated for n6

As a follow-up, it might be good to add something to the tests to assert that allocated is ordered (and maybe to the production code to log an error if the ordering breaks).

- Description for the changelog

Fix a regression that caused duplicate subnet allocations when creating networks.

@robmry robmry added this to the 27.0.3 milestone Jun 28, 2024
@robmry robmry self-assigned this Jun 28, 2024
@robmry robmry requested review from akerouanton and corhere June 28, 2024 11:19
@robmry robmry force-pushed the 48069_fix_overlapping_subnets branch from faca22b to 4849786 Compare June 28, 2024 11:27
Keep allocated subnets in-order, so that they're not mistakenly
reallocated due to a gap in the list where misplaced subnets should
have been.

Introduced in 9d288b5.

The iterator over allocated subnets was incremented too early, this
change moves it past three clauses in addrSpace.allocatePredefinedPool().
The three new unit tests correspond to a separate failure caused by
incrementing before each of them.

Signed-off-by: Rob Murray <[email protected]>
@robmry robmry force-pushed the 48069_fix_overlapping_subnets branch from 4849786 to 4de54ee Compare June 28, 2024 12:34
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

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

Projects

None yet

3 participants