server: refactor listNetworks api database retrievals #9184

shwstppr · 2024-06-06T10:47:26Z

Description

This PR attempts to optimize database queries during listNetworks API call. It should fix pagination-related issues as well while querying desired networks from the database. Only in the following cases, it may query all networks:

API param canusefordeploy is passed as true
Any service specified with the supportedservices API parameter

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

codecov · 2024-06-06T10:55:52Z

Codecov Report

Attention: Patch coverage is 0% with 70 lines in your changes missing coverage. Please review.

Project coverage is 14.98%. Comparing base (631d6ad) to head (3a97fae).
Report is 102 commits behind head on 4.19.

Files	Patch %	Lines
...ain/java/com/cloud/network/NetworkServiceImpl.java	0.00%	70 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.19    #9184      +/-   ##
============================================
+ Coverage     14.96%   14.98%   +0.02%     
- Complexity    10991    11048      +57     
============================================
  Files          5373     5389      +16     
  Lines        469203   470603    +1400     
  Branches      60225    58405    -1820     
============================================
+ Hits          70198    70517     +319     
- Misses       391232   392251    +1019     
- Partials       7773     7835      +62

Flag	Coverage Δ
uitests	`4.28% <ø> (-0.03%)`	⬇️
unittests	`15.69% <0.00%> (+0.02%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

weizhouapache · 2024-06-06T11:12:15Z

good idea

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr · 2024-06-06T18:55:57Z

@blueorangutan package

blueorangutan · 2024-06-06T18:58:04Z

@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.

blueorangutan · 2024-06-06T20:05:11Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9810

shwstppr · 2024-06-06T20:08:59Z

@blueorangutan test

blueorangutan · 2024-06-06T20:10:04Z

@shwstppr a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2024-06-07T10:08:53Z

[SF] Trillian test result (tid-10382)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 48773 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9184-t10382-kvm-centos7.zip
Smoke tests completed. 131 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

weizhouapache · 2024-06-10T07:50:40Z

@blueorangutan package

blueorangutan · 2024-06-10T07:52:04Z

@weizhouapache 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.

blueorangutan · 2024-06-10T08:54:01Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9836

shwstppr · 2024-06-10T09:41:32Z

In my testing, listing networks is working fine with the changes and smoke tests didn't report any error. Marking it ready for review

rohityadavcloud

LGTM, didn't test it but needs testing on:

networks by roles: admin, domain admin, normal user
list networks by projects, accounts, domains
list networks in env with variety of networks across roles; isolated networks, VPC tier/network, shared network, L2 network and user-shared network

sureshanaparti

clgtm

sureshanaparti · 2024-06-24T07:40:42Z

@blueorangutan package

blueorangutan · 2024-06-24T07:42:03Z

@sureshanaparti 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.

vishesh92 · 2024-06-25T07:27:01Z

server/src/main/java/com/cloud/network/NetworkServiceImpl.java

- networksToReturn.addAll(listAccountSpecificNetworks(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
- aclType, skipProjectNetworks, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, permittedAccounts));
+ additionalSearchCriteria.addOr("id", SearchCriteria.Op.SC,
+ getAccountSpecificNetworksSearchCriteria(sb, permittedAccounts, skipProjectNetworks));


If getAccountSpecificNetworksSearchCriteria returns null, the request fails with a bad sql query.

Thanks @vishesh92 will check

@vishesh92 it was due to empty searchcriteria at line 2392. Should be fixed now.

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr · 2024-06-26T07:51:20Z

@blueorangutan package

blueorangutan · 2024-06-26T07:52:03Z

@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.

blueorangutan · 2024-06-26T08:07:27Z

Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10141

shwstppr · 2024-06-26T08:23:21Z

@blueorangutan package

blueorangutan · 2024-06-26T08:24:03Z

@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.

blueorangutan · 2024-06-26T09:26:19Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10145

rohityadavcloud · 2024-06-26T09:46:17Z

@blueorangutan test

blueorangutan · 2024-06-26T09:48:04Z

@rohityadavcloud a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

borisstoyanov

LGTM, checked manually for admin/user visibility of shared networks and for networks deployed within projects

blueorangutan · 2024-06-26T20:32:54Z

[SF] Trillian test result (tid-10639)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37134 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9184-t10639-kvm-centos7.zip
Smoke tests completed. 97 look OK, 34 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestTemplateHierarchy>:setup	`Error`	8.27	test_accounts.py
test_01_events_resource	`Failure`	0.02	test_events_resource.py
ContextSuite context=TestDeployVmWithAffinityGroup>:setup	`Error`	0.00	test_affinity_groups.py
ContextSuite context=TestMultipleVolumeAttach>:setup	`Error`	0.00	test_attach_multiple_volumes.py
ContextSuite context=TestDummyBackupAndRecovery>:setup	`Error`	0.00	test_backup_recovery_dummy.py
ContextSuite context=TestVeeamBackupAndRecovery>:setup	`Error`	0.00	test_backup_recovery_veeam.py
test_01_condensed_drs_algorithm	`Error`	0.00	test_cluster_drs.py
test_02_balanced_drs_algorithm	`Error`	0.00	test_cluster_drs.py
ContextSuite context=TestConsoleEndpoint>:setup	`Error`	0.00	test_console_endpoint.py
test_01_deploy_vm_with_extraconfig_throws_exception_kvm	`Error`	0.10	test_deploy_vm_extra_config_data.py
test_02_deploy_vm_with_extraconfig_kvm	`Error`	0.07	test_deploy_vm_extra_config_data.py
test_03_update_vm_with_extraconfig_kvm	`Error`	0.06	test_deploy_vm_extra_config_data.py
ContextSuite context=TestDeployVmRootSize>:setup	`Error`	0.00	test_deploy_vm_root_resize.py
ContextSuite context=TestHostControlState>:setup	`Error`	27.24	test_host_control_state.py
ContextSuite context=TestImportAndUnmanageVolumes>:setup	`Error`	0.00	test_import_unmanage_volumes.py
test_11_test_unmanaged_cluster_lifecycle	`Error`	2.30	test_kubernetes_clusters.py
ContextSuite context=TestListVolumes>:setup	`Error`	0.00	test_list_volumes.py
ContextSuite context=TestNetworkMigration>:setup	`Error`	0.00	test_migration.py
test_03_network_operations_on_created_vm_of_otheruser	`Error`	1.20	test_network_permissions.py
test_04_deploy_vm_for_other_user_and_test_vm_operations	`Failure`	0.05	test_network_permissions.py
ContextSuite context=TestSharedNetwork>:setup	`Error`	56.59	test_network.py
test_01_nic	`Error`	0.05	test_nic.py
test_01_non_strict_host_anti_affinity	`Error`	2.17	test_nonstrict_affinity_group.py
test_02_non_strict_host_affinity	`Error`	1.13	test_nonstrict_affinity_group.py
ContextSuite context=TestL2PersistentNetworks>:setup	`Error`	0.00	test_persistent_network.py
test_01_add_primary_storage_disabled_host	`Failure`	0.06	test_primary_storage.py
test_01_primary_storage_iscsi	`Failure`	0.06	test_primary_storage.py
test_01_primary_storage_nfs	`Failure`	0.06	test_primary_storage.py
ContextSuite context=TestStorageTags>:setup	`Error`	0.15	test_primary_storage.py
ContextSuite context=TestPrivateGwACLOvsGRE>:setup	`Error`	0.00	test_privategw_acl_ovs_gre.py
ContextSuite context=TestTemplates>:setup	`Error`	36.51	test_templates.py
test_01_positive_tests_usage	`Failure`	2.93	test_usage_events.py
ContextSuite context=TestLBRuleUsage>:setup	`Error`	2.24	test_usage.py
ContextSuite context=TestNatRuleUsage>:setup	`Error`	2.28	test_usage.py
ContextSuite context=TestPublicIPUsage>:setup	`Error`	2.32	test_usage.py
ContextSuite context=TestSnapshotUsage>:setup	`Error`	2.38	test_usage.py
ContextSuite context=TestTemplateUsage>:setup	`Error`	2.43	test_usage.py
ContextSuite context=TestVmUsage>:setup	`Error`	2.48	test_usage.py
ContextSuite context=TestVolumeUsage>:setup	`Error`	2.57	test_usage.py
ContextSuite context=TestVpnUsage>:setup	`Error`	2.62	test_usage.py
ContextSuite context=TestVmAutoScaling>:setup	`Error`	0.00	test_vm_autoscaling.py
test_01_deploy_vm_on_specific_host	`Error`	0.05	test_vm_deployment_planner.py
test_02_deploy_vm_on_specific_cluster	`Error`	0.04	test_vm_deployment_planner.py
test_03_deploy_vm_on_specific_pod	`Error`	0.07	test_vm_deployment_planner.py
test_04_deploy_vm_on_host_override_pod_and_cluster	`Error`	0.08	test_vm_deployment_planner.py
test_05_deploy_vm_on_cluster_override_pod	`Error`	0.05	test_vm_deployment_planner.py
ContextSuite context=TestDeployVM>:setup	`Error`	0.00	test_vm_life_cycle.py
ContextSuite context=TestKVMLiveMigration>:setup	`Error`	0.00	test_vm_life_cycle.py
ContextSuite context=TestMigrateVMwithVolume>:setup	`Error`	0.00	test_vm_life_cycle.py
ContextSuite context=TestSecuredVmMigration>:setup	`Error`	0.00	test_vm_life_cycle.py
ContextSuite context=TestVMLifeCycle>:setup	`Error`	0.06	test_vm_life_cycle.py
ContextSuite context=TestUnmanageVM>:setup	`Error`	0.00	test_vm_lifecycle_unmanage_import.py
ContextSuite context=TestVMSchedule>:setup	`Error`	0.00	test_vm_schedule.py
ContextSuite context=TestVmSnapshot>:setup	`Error`	0.07	test_vm_snapshots.py
ContextSuite context=TestVnfTemplates>:setup	`Error`	0.00	test_vnf_templates.py
ContextSuite context=TestCreateVolume>:setup	`Error`	0.00	test_volumes.py
ContextSuite context=TestVolumeEncryption>:setup	`Error`	0.00	test_volumes.py
ContextSuite context=TestVolumes>:setup	`Error`	0.00	test_volumes.py
test_02_cancel_host_maintenace_with_migration_jobs	`Failure`	0.14	test_host_maintenance.py
test_03_cancel_host_maintenace_with_migration_jobs_failure	`Error`	0.19	test_host_maintenance.py
ContextSuite context=TestHostMaintenanceAgents>:setup	`Error`	0.26	test_host_maintenance.py

sureshanaparti · 2024-06-28T10:31:27Z

@blueorangutan package

blueorangutan · 2024-06-28T10:32:03Z

@sureshanaparti 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.

blueorangutan · 2024-06-28T11:38:27Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10190

sureshanaparti · 2024-06-28T11:48:54Z

Merging based on manual tests & reviews.

* server: refactor listNetworks api database retrievals * fixes * remove unused methods * imports * fix empty searchcriteria issue * refactor Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

* server, api, ui: access improvements and assorted fixes Fixes domain-admin access check to prevent unauthorized access. Co-authored-by: Fabricio Duarte <fabricio.duarte.jr@gmail.com> Co-authored-by: nvazquez <nicovazquez90@gmail.com> Co-authored-by: Abhishek Kumar <abhishek.mrt22@gmail.com> * Revert "server: refactor listNetworks api database retrievals (#9184)" This reverts commit c7f1ba5. * Fix snapshot chain being deleted on XenServer (#9447) Using XenServer as the hypervisor, when deleting a snapshot that has a parent, that parent will also get erased on storage, causing data loss. This behavior was introduced with #7873, where the list of snapshot states that can be deleted was changed to add BackedUp snapshots. This PR changes the states list back to the original list, and swaps the while loop for a do while loop to account for the changes in #7873. Fixes #9446 * UI: Display Firewall, LB and Port Forwading rules tab for CKS clusters deployed on isolated networks (#9458) --------- Co-authored-by: nvazquez <nicovazquez90@gmail.com> Co-authored-by: Fabricio Duarte <fabricio.duarte.jr@gmail.com> Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com> Co-authored-by: Pearl Dsilva <pearl1594@gmail.com>

* server: refactor listNetworks api database retrievals * fixes * remove unused methods * imports * fix empty searchcriteria issue * refactor Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

* server, api, ui: access improvements and assorted fixes Fixes domain-admin access check to prevent unauthorized access. Co-authored-by: Fabricio Duarte <fabricio.duarte.jr@gmail.com> Co-authored-by: nvazquez <nicovazquez90@gmail.com> Co-authored-by: Abhishek Kumar <abhishek.mrt22@gmail.com> * Revert "server: refactor listNetworks api database retrievals (apache#9184)" This reverts commit c7f1ba5. * Fix snapshot chain being deleted on XenServer (apache#9447) Using XenServer as the hypervisor, when deleting a snapshot that has a parent, that parent will also get erased on storage, causing data loss. This behavior was introduced with apache#7873, where the list of snapshot states that can be deleted was changed to add BackedUp snapshots. This PR changes the states list back to the original list, and swaps the while loop for a do while loop to account for the changes in apache#7873. Fixes apache#9446 * UI: Display Firewall, LB and Port Forwading rules tab for CKS clusters deployed on isolated networks (apache#9458) --------- Co-authored-by: nvazquez <nicovazquez90@gmail.com> Co-authored-by: Fabricio Duarte <fabricio.duarte.jr@gmail.com> Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com> Co-authored-by: Pearl Dsilva <pearl1594@gmail.com>

* server: refactor listNetworks api database retrievals * fixes * remove unused methods * imports * fix empty searchcriteria issue * refactor Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

server: refactor listNetworks api database retrievals

e95b72d

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

boring-cyborg bot added the component:networking label Jun 6, 2024

shwstppr mentioned this pull request Jun 6, 2024

listNetworks optimization #9096

Closed

13 tasks

DaanHoogland added this to the 4.19.1.0 milestone Jun 6, 2024

fixes

f627ecd

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr added 2 commits June 10, 2024 12:43

remove unused methods

3f15867

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

imports

11ca762

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr changed the title ~~[WIP] server: refactor listNetworks api database retrievals~~ server: refactor listNetworks api database retrievals Jun 10, 2024

shwstppr marked this pull request as ready for review June 10, 2024 09:41

shwstppr requested review from abh1sar, rohityadavcloud and weizhouapache June 12, 2024 11:37

rohityadavcloud approved these changes Jun 12, 2024

View reviewed changes

rohityadavcloud added the status:needs-testing label Jun 12, 2024

rohityadavcloud assigned borisstoyanov Jun 12, 2024

sureshanaparti approved these changes Jun 24, 2024

View reviewed changes

vishesh92 reviewed Jun 25, 2024

View reviewed changes

shwstppr added 2 commits June 26, 2024 13:18

fix empty searchcriteria issue

038787f

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

refactor

3a97fae

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

borisstoyanov approved these changes Jun 26, 2024

View reviewed changes

sureshanaparti removed the status:needs-testing label Jun 26, 2024

apache deleted a comment from blueorangutan Jun 27, 2024

sureshanaparti merged commit c7f1ba5 into apache:4.19 Jun 28, 2024

DaanHoogland deleted the refactor-listnetworks-execution branch July 1, 2024 06:52

winterhazel mentioned this pull request Jul 29, 2024

Restore listNetworks behavior & clean up the code #9461

Merged

7 tasks

weizhouapache mentioned this pull request Aug 5, 2024

New feature: Dynamic and Static Routing #9470

Merged

14 tasks

server: refactor listNetworks api database retrievals #9184

server: refactor listNetworks api database retrievals #9184

Uh oh!

Conversation

shwstppr commented Jun 6, 2024

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

codecov bot commented Jun 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

weizhouapache commented Jun 6, 2024

Uh oh!

shwstppr commented Jun 6, 2024

Uh oh!

blueorangutan commented Jun 6, 2024

Uh oh!

blueorangutan commented Jun 6, 2024

Uh oh!

shwstppr commented Jun 6, 2024

Uh oh!

blueorangutan commented Jun 6, 2024

Uh oh!

blueorangutan commented Jun 7, 2024

Uh oh!

weizhouapache commented Jun 10, 2024

Uh oh!

blueorangutan commented Jun 10, 2024

Uh oh!

blueorangutan commented Jun 10, 2024

Uh oh!

shwstppr commented Jun 10, 2024

Uh oh!

rohityadavcloud left a comment

Choose a reason for hiding this comment

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

sureshanaparti commented Jun 24, 2024

Uh oh!

blueorangutan commented Jun 24, 2024

Uh oh!

vishesh92 Jun 25, 2024

Choose a reason for hiding this comment

Uh oh!

shwstppr Jun 25, 2024

Choose a reason for hiding this comment

Uh oh!

shwstppr Jun 26, 2024

Choose a reason for hiding this comment

Uh oh!

shwstppr commented Jun 26, 2024

Uh oh!

blueorangutan commented Jun 26, 2024

Uh oh!

blueorangutan commented Jun 26, 2024

Uh oh!

shwstppr commented Jun 26, 2024

Uh oh!

blueorangutan commented Jun 26, 2024

Uh oh!

blueorangutan commented Jun 26, 2024

Uh oh!

rohityadavcloud commented Jun 26, 2024

Uh oh!

blueorangutan commented Jun 26, 2024

Uh oh!

borisstoyanov left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jun 26, 2024

Uh oh!

codecov bot commented Jun 6, 2024 •

edited

Loading