Cleaning up code and enhancing a few IP management logs#4714
Cleaning up code and enhancing a few IP management logs#4714DaanHoogland merged 5 commits intoapache:mainfrom
Conversation
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
code looks good and no functional changes to be seen
|
@blueorangutan package |
|
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2817 |
| fetchFromDedicatedRange = true; | ||
| sc.setParameters("vlanId", dedicatedVlanDbIds.toArray()); | ||
| errorMessage.append(", vlanId id=" + Arrays.toString(dedicatedVlanDbIds.toArray())); | ||
| } else if (nonDedicatedVlanDbIds != null && !nonDedicatedVlanDbIds.isEmpty()) { |
There was a problem hiding this comment.
Null check not needed, the list has been created at line 793.
List<Long> nonDedicatedVlanDbIds = new ArrayList<Long>();
There was a problem hiding this comment.
yes, you could have told me before i checked it ;)
|
A conflict has emerged, I will reserve some time to fix it. |
|
Conflicts have been fixed. |
|
@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
@blueorangutan package |
|
@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✖️ centos7 ✖️ centos8 ✖️ debian. SL-JID 535 |
|
Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 536 |
|
more conflicts @GabrielBrascher |
|
Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 569 |
|
Trillian test result (tid-630)
|
|
Trillian test result (tid-655)
|
|
Thanks for the review, @sureshanaparti. |
|
@blueorangutan package |
|
@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 19 |
|
@blueorangutan test |
|
@shwstppr a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-684)
|
|
triggering the tests again to check the latest results. |
|
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
| if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { | ||
| ResourceAllocationException ex = new ResourceAllocationException("Cannot perform this operation, " + "Zone is currently disabled" + "zoneId=" + zone.getUuid(), | ||
| ResourceType.network); | ||
| ResourceAllocationException ex = new ResourceAllocationException(generateErrorMessageForOperationOnDisabledZone("allocate Pod IP addresses", zone), ResourceType.network); |
There was a problem hiding this comment.
Good observation; updated code breaking line.
|
Trillian test result (tid-1100)
|
|
@blueorangutan package |
|
@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 379 |
|
@blueorangutan test |
|
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-1133)
|
|
@GabrielBrascher I have no opinion to speak of ;
However, can you provide insight into your testing, i'm trusting the smoke tests a bit only. Any other testing needed? |
@DaanHoogland the However, as it is not populated, the conditional For reference, the code: |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 693 |
|
Trillian test result (tid-1416)
|
5 participants
Description
This PR removes some unnecessary code and enhances a few log messages at IpAddressManagerImpl.
Additionally, method
com.cloud.network.NetworkModelImpl.areServicesEnabledInZonedoes not use listcheckedProvider; the respective list should either be removed or populated.I have chosen to add provider names at
checkedProvider, instead of removing it; however, I would appreciate any opinion/suggestion.Types of changes