-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Flexibilize public IP selection #11076
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
base: 4.22
Are you sure you want to change the base?
Flexibilize public IP selection #11076
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11076 +/- ##
===========================================
Coverage 16.57% 16.57%
- Complexity 13868 13987 +119
===========================================
Files 5719 5745 +26
Lines 507178 510872 +3694
Branches 61571 62141 +570
===========================================
+ Hits 84085 84698 +613
- Misses 413674 416699 +3025
- Partials 9419 9475 +56
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
engine/schema/src/main/java/com/cloud/network/dao/PublicIpQuarantineDaoImpl.java
Outdated
Show resolved
Hide resolved
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14153 |
|
@blueorangutan LLtest |
|
@DaanHoogland a [LL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
@erikbocks , do you still want to move this forwards? |
|
Hello, @DaanHoogland Yes, I wish to move forward with this PR. It is currently waiting for reviews. |
engine/schema/src/main/java/com/cloud/network/dao/PublicIpQuarantineDao.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/network/dao/PublicIpQuarantineDaoImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
Outdated
Show resolved
Hide resolved
|
@erikbocks can this go into 4.20.3? |
|
@bernardodemarco I checked the code for the @winterhazel, sure, I will re-target the PR. |
4975e0b to
d5d4aa9
Compare
Description
Currently, when a public IP allocation request is made, the
quarantined_ipstable is ignored during the selection process. Instead, the system selects an IP without checking its quarantine status, and only validates if the IP is quarantined afterward, throwing an exception if it is.This causes repeated failures for automatic IP selections, as the same IP will be chosen over and over, until the quarantine period ends.
This PR fixes this behaviour by considering the
quarantined_ipstable during the selection process, and choosing only an allocatable IP. It also brings a minor improvement to the IP quarantine logic, where IPs that were not removed either in active quarantine, were not set removed when allocated.Now, when these IPs are allocated, the removed column is updated and the removal reason is set to:
IP was removed from quarantine because it was no longer in quarantine. The PR also does a little code refactor, improving its legibility and dropping the usage of deprecated methods.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
How Has This Been Tested?
I made the following tests: