Skip to content

Conversation

@rbtr
Copy link
Collaborator

@rbtr rbtr commented Dec 1, 2023

Reason for Change:

v2 IPAM Pool Monitor introduces idempotent scaling math, migrates to an event-driven instead of polling architecture, and leverages the Pod watcher for IP demand instead of using the "free IPs in the pool" to trigger scaling.

Notably this change improves pool scaling performance from O(n) to O(1).

Testing on a single Linux node DS2v2 with a 100 Pod pause container scale-up yields:

100 pod scaleup test CNS mode typical time to full IP availability
IPAMv1 1:30
IPAMv2 0:13

Issue Fixed:

Requirements:

Notes: not to be confused with SwiftV2, this is applicable to all dynamic Pod IP Swift scenarios

Design docs for reference: #2013

@rbtr rbtr requested a review from a team as a code owner December 1, 2023 18:16
@rbtr rbtr requested a review from thatmattlong December 1, 2023 18:16
@rbtr rbtr self-assigned this Dec 1, 2023
@rbtr rbtr requested a review from behzad-mir December 1, 2023 18:16
@rbtr rbtr changed the title feat: v2 ipampool feat: v2 swift ipampool Dec 1, 2023
@rbtr rbtr added cns Related to CNS. swift Related to SWIFT networking. release/latest Change affects latest release train labels Dec 1, 2023
@rbtr rbtr requested a review from a team as a code owner December 1, 2023 18:24
@rbtr rbtr requested a review from nddq December 1, 2023 18:38
@rbtr rbtr force-pushed the ipam-v2 branch 6 times, most recently from 57fdbc9 to c0ceb7b Compare December 5, 2023 18:46
@rbtr rbtr enabled auto-merge (squash) December 5, 2023 22:36
@rbtr rbtr force-pushed the ipam-v2 branch 3 times, most recently from 8d4b17a to 57edeeb Compare December 8, 2023 15:56
auto-merge was automatically disabled December 19, 2023 23:02

Merge queue setting changed

@github-actions github-actions bot added the stale Stale due to inactivity. label Dec 23, 2023
@rbtr rbtr removed the stale Stale due to inactivity. label Dec 23, 2023
@rbtr rbtr force-pushed the ipam-v2 branch 2 times, most recently from 7226270 to bc4e879 Compare January 3, 2024 21:18
@rbtr rbtr enabled auto-merge January 3, 2024 21:20
@Azure Azure deleted a comment from github-actions bot Jan 3, 2024
@rbtr rbtr force-pushed the ipam-v2 branch 2 times, most recently from 1b01e20 to 92c4f63 Compare January 11, 2024 18:21
Copy link
Member

@nddq nddq left a comment

Choose a reason for hiding this comment

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

Left some comments, I'm wondering if we could try and do some stress tests on an actual cluster (or you have done it already 🙂)

@rbtr
Copy link
Collaborator Author

rbtr commented Jan 18, 2024

I'm wondering if we could try and do some stress tests on an actual cluster (or you have done it already 🙂)

I don't have numbers, but I validated that it works as expected and reaches target state faster for big swings in Pod scheduling 🙂
The really interesting results will be testing this is combination with @behzad-mir's stateless CNI changes. I expect the time-to-Pod-ready for big Pod scale-ups to be much much lower.

Copy link
Member

@nddq nddq left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@rbtr rbtr added this pull request to the merge queue Jan 26, 2024
@rbtr rbtr removed this pull request from the merge queue due to a manual request Jan 26, 2024
@rbtr rbtr added this pull request to the merge queue Jan 26, 2024
@rbtr
Copy link
Collaborator Author

rbtr commented Jan 26, 2024

going to move this one forward, and consider the feedback while running it for real to refine it.

Merged via the queue into master with commit 51d7c5c Jan 26, 2024
@rbtr rbtr deleted the ipam-v2 branch January 26, 2024 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cns Related to CNS. release/latest Change affects latest release train swift Related to SWIFT networking.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants