apache
Restore listNetworks behavior & clean up the code
Description
PR #9184 attempted to optimize the listNetworks API by converting the multiple queries that were performed into a single one. This was done by building multiple search criterias for the individual queries, OR-adding them to an intermediary search criteria, and AND-adding the intermediary to a final one. However, this did not work as intended, because the code does not carry the joins' conditions when adding a search criteria into another one.
This PR fixes the issue, making the final query work as intended by #9184. This was done by explicitly adding the join parameters to each query's WHERE clause, which will then be included in the intermediary search criteria and in the final query. I also cleaned up the related code.
Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [X] Cleanup (Code refactoring and cleanup, that may add test cases)
Feature/Enhancement Scale or Bug Severity
Bug Severity
- [X] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
Screenshots (if appropriate):
How Has This Been Tested?
My environment had the following domains, accounts, projects and networks (networks beginning with an "i" are isolated, and with a "s" are shared):
- ROOT: admin (root admin; networks iadmin, created sadmin), ur (user; network iur), pr (project; network ipr; has accounts admin, ur)
- ROOT/d1: d1 (domain admin; networks id1, created sd1), ud1 (user; network iud1), pd1 (project; network ipd1; has account d1)
- ROOT/d2: d2 (domain admin; network id2, created sd2)
Through the UI, in every account, I listed the networks using the all/account/domain/shared filters with the project toggle on/off. I compared the behavior for the same environment in a commit before #9184, and verified that it was the same. Below is a table showing which networks were listed for each account and filter combination.
| Account/filter | All, project off | Account, project off | Domain, project off | Shared, project off | All, project on | Account, project on | Domain, project on | Shared, project on |
|---|---|---|---|---|---|---|---|---|
| admin | id2,iud1,id1,iur,iadmin,sd2,sd1,sadmin | id2,iud1,id1,iur,iadmin | See [1] | Empty, see [2] | ipd1,ipr,sd2,sd1,sadmin | ipd1,ipr | See [1] | Empty, see [2] |
| ur | iur,sadmin | iur | See [1] | Empty, see [2] | ipr,sadmin | ipr | See [1] | Empty, see [2] |
| d1 | iud1,id1,sd1,sadmin | iud1,id1 | See [1] | Empty, see [2] | ipd1,sd1,sadmin | ipd1 | See [1] | Empty, see [2] |
| ud1 | iud1,sd1,sadmin | iud1 | See [1] | Empty, see [2] | sd1,sadmin | Empty | See [1] | Empty, see [2] |
| d2 | id2,sd2,sadmin | id2 | See [1] | Empty, see [2] | sd2,sadmin | Empty | See [1] | Empty, see [2] |
[1] When filtering by domain, I got Invalid value of networkfilter: domainpath. This was already present before #9184.
[2] This is probably not the intended behavior, and was already present before #9184. It can be fixed in a separate PR, as this one only aims to restore the original filtering behavior.
@winterhazel 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.
Codecov Report
Attention: Patch coverage is 0% with 94 lines in your changes missing coverage. Please review.
Project coverage is 15.08%. Comparing base (
bf11676) to head (8cf2987). Report is 33 commits behind head on 4.19.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| ...ain/java/com/cloud/network/NetworkServiceImpl.java | 0.00% | 94 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## 4.19 #9461 +/- ##
==========================================
Coverage 15.08% 15.08%
Complexity 11189 11189
==========================================
Files 5406 5406
Lines 472828 472830 +2
Branches 59879 60053 +174
==========================================
+ Hits 71346 71350 +4
+ Misses 393537 393536 -1
+ Partials 7945 7944 -1
| Flag | Coverage Δ | |
|---|---|---|
| uitests | 4.30% <ø> (ø) |
|
| unittests | 15.80% <0.00%> (+<0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10496
@shwstppr a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
@winterhazel 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.
@shwstppr could we run the CI again? There seems to have been a problem with the last run
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10549
@shwstppr a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
@shwstppr a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
[SF] Trillian test result (tid-11025) Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8 Total time taken: 48187 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9461-t11025-kvm-ol8.zip Smoke tests completed. 131 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below:
| Test | Result | Time (s) | Test File |
|---|---|---|---|
| test_01_secure_vm_migration | Error |
133.32 | test_vm_life_cycle.py |
| test_01_secure_vm_migration | Error |
133.33 | test_vm_life_cycle.py |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@shwstppr 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 10613
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
[SF] Trillian test result (tid-11057) Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8 Total time taken: 47970 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9461-t11057-kvm-ol8.zip Smoke tests completed. 133 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below:
| Test | Result | Time (s) | Test File |
|---|
cc @kiranchavala @rajujith @NuxRo @vladimirpetrov @borisstoyanov @JoaoJandre @GutoVeronezi or others - anybody able to manually QA this? Thanks.
@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 11027
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
[SF] Trillian test result (tid-11402) Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8 Total time taken: 46714 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9461-t11402-kvm-ol8.zip Smoke tests completed. 132 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below:
| Test | Result | Time (s) | Test File |
|---|---|---|---|
| test_04_nonsecured_to_secured_vm_migration | Error |
375.77 | test_vm_life_cycle.py |