Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimize IPAM pool selections #2207

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

Icarus9913
Copy link
Collaborator

@Icarus9913 Icarus9913 commented Aug 21, 2023

The Spiderpool IPAM has the ability to allocate IP addresses from an array of IPPools. If you use ipam.spidernet.io/ippool or ipam.spidernet.io/ippools annotation to specify IPPools or you use CNI configuration default_ipv4_ippool to specify IPPools, the system would allocate IPs from these IPPools in order.
For instance:

annotations:
  ipam.spidernet.io/ippools: |-
    {"ipv4": ["pool1","pool2","pool3"]}

But once if you do not set any annotation or CNI configuration rules, the backend system will search all SpiderIPPool resources with "Spec.Default == true". In this case, the backend system can't control the sequence for the IPPool candidates array and may lead to #2042 .

Consequently, I optimize the backend logic and sort the IPPool candidates sequence with the filter rules.
And it's suitable for all IPPool allocation rules.

For example:

If you have these IPPools:
Pool1, PodAffinity
Pool2, NodeName
Pool3, NamespaceName
Pool4, MultusName
Pool5

And you will get the IPPool candidates just like: ["pool1", "pool3", "pool2", "pool4", "pool5"]

or

If you have these IPPools:
Pool1, PodAffinity
Pool2, NodeName
Pool3, NodeName && NamespaceName
Pool4, NamespaceName && MultusName

And you will get the IPPool candidates just like: ["pool1", "pool3", "pool2", "pool4"]

Signed-off-by: Icarus9913 [email protected]

What this PR does / why we need it:
new feature

Which issue(s) this PR fixes:
close #2042

@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Merging #2207 (c17f374) into main (aa2d201) will increase coverage by 0.14%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2207      +/-   ##
==========================================
+ Coverage   80.99%   81.14%   +0.14%     
==========================================
  Files          49       49              
  Lines        5236     5277      +41     
==========================================
+ Hits         4241     4282      +41     
  Misses        832      832              
  Partials      163      163              
Flag Coverage Δ
unittests 81.14% <100.00%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
pkg/ippoolmanager/utils.go 92.77% <100.00%> (+7.05%) ⬆️

@Icarus9913 Icarus9913 added the pr/not-ready not ready for merging label Aug 21, 2023
@Icarus9913 Icarus9913 force-pushed the feat/wk/pool-selection branch from f73c9f4 to 6604ece Compare August 22, 2023 07:52
@Icarus9913 Icarus9913 force-pushed the feat/wk/pool-selection branch from 6604ece to c17f374 Compare August 22, 2023 08:37
@Icarus9913 Icarus9913 added pr/ready-review This pull is ready for review and removed pr/not-ready not ready for merging labels Aug 22, 2023
@weizhoublue weizhoublue merged commit 93677f1 into spidernet-io:main Aug 23, 2023
@Icarus9913 Icarus9913 deleted the feat/wk/pool-selection branch September 8, 2023 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready-review This pull is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

confused selection for ippool
2 participants