CLOUDSTACK-10057: listNetworkOfferings now returns the correct number of offerings. #2250

sgoeminn · 2017-08-29T13:33:56Z

https://issues.apache.org/jira/browse/CLOUDSTACK-10057

rohityadavcloud · 2017-08-31T07:53:02Z

@blueorangutan package

blueorangutan · 2017-08-31T07:53:07Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2017-08-31T10:03:07Z

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1057

rohityadavcloud · 2017-08-31T11:04:03Z

@blueorangutan test

blueorangutan · 2017-08-31T11:05:07Z

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

blueorangutan · 2017-08-31T22:19:47Z

Trillian test result (tid-1475)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40440 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2250-t1475-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Intermitten failure detected: /marvin/tests/smoke/test_iso.py
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Test completed. 57 look OK, 5 have error(s)

Test	Result	Time (s)	Test File
test_01_vpc_remote_access_vpn	`Failure`	65.74	test_vpc_vpn.py
test_04_rvpc_privategw_static_routes	`Failure`	359.40	test_privategw_acl.py
test_05_iso_permissions	`Failure`	0.04	test_iso.py
test_02_edit_iso	`Failure`	0.04	test_iso.py
test_hostha_enable_ha_when_host_disabled	`Failure`	5.44	test_hostha_kvm.py
test_ha_kvm_host_degraded	`Error`	26.89	test_hostha_kvm.py
ContextSuite context=TestDeployVirtioSCSIVM>:teardown	`Error`	38.66	test_deploy_virtio_scsi_vm.py
test_change_service_offering_for_vm_with_snapshots	Skipped	0.00	test_vm_snapshots.py
test_09_copy_delete_template	Skipped	0.01	test_templates.py
test_06_copy_template	Skipped	0.00	test_templates.py
test_static_role_account_acls	Skipped	0.02	test_staticroles.py
test_11_ss_nfs_version_on_ssvm	Skipped	0.02	test_ssvm.py
test_01_scale_vm	Skipped	0.00	test_scale_vm.py
test_01_primary_storage_iscsi	Skipped	0.02	test_primary_storage.py
test_vm_nic_adapter_vmxnet3	Skipped	0.00	test_nic_adapter_type.py
test_nested_virtualization_vmware	Skipped	0.00	test_nested_virtualization.py
test_06_copy_iso	Skipped	0.00	test_iso.py
test_list_ha_for_host_valid	Skipped	0.01	test_hostha_simulator.py
test_list_ha_for_host_invalid	Skipped	0.01	test_hostha_simulator.py
test_list_ha_for_host	Skipped	0.01	test_hostha_simulator.py
test_hostha_enable_feature_without_setting_provider	Skipped	0.01	test_hostha_simulator.py
test_hostha_enable_feature_valid	Skipped	0.01	test_hostha_simulator.py
test_hostha_disable_feature_valid	Skipped	0.02	test_hostha_simulator.py
test_hostha_configure_invalid_provider	Skipped	0.01	test_hostha_simulator.py
test_hostha_configure_default_driver	Skipped	0.01	test_hostha_simulator.py
test_ha_verify_fsm_recovering	Skipped	0.01	test_hostha_simulator.py
test_ha_verify_fsm_fenced	Skipped	0.01	test_hostha_simulator.py
test_ha_verify_fsm_degraded	Skipped	0.01	test_hostha_simulator.py
test_ha_verify_fsm_available	Skipped	0.01	test_hostha_simulator.py
test_ha_multiple_mgmt_server_ownership	Skipped	0.01	test_hostha_simulator.py
test_ha_list_providers	Skipped	0.01	test_hostha_simulator.py
test_ha_enable_feature_invalid	Skipped	0.01	test_hostha_simulator.py
test_ha_disable_feature_invalid	Skipped	0.01	test_hostha_simulator.py
test_ha_configure_enabledisable_across_clusterzones	Skipped	0.01	test_hostha_simulator.py
test_configure_ha_provider_valid	Skipped	0.01	test_hostha_simulator.py
test_configure_ha_provider_invalid	Skipped	0.02	test_hostha_simulator.py
test_deploy_vgpu_enabled_vm	Skipped	0.02	test_deploy_vgpu_enabled_vm.py
test_3d_gpu_support	Skipped	0.02	test_deploy_vgpu_enabled_vm.py

rohityadavcloud · 2017-09-04T06:35:41Z

server/src/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -4788,7 +4788,7 @@ protected void validateNtwkOffDetails(final Map<Detail, String> details, final M
            // Now apply pagination
            final List<? extends NetworkOffering> wPagination = StringUtils.applyPagination(supportedOfferings, cmd.getStartIndex(), cmd.getPageSizeVal());
            if (wPagination != null) {
-                final Pair<List<? extends NetworkOffering>, Integer> listWPagination = new Pair<List<? extends NetworkOffering>, Integer>(wPagination, offerings.size());
+                final Pair<List<? extends NetworkOffering>, Integer> listWPagination = new Pair<List<? extends NetworkOffering>, Integer>(wPagination, supportedOfferings.size());


Additional review and testing requested, if we need to change this line, then 4798 may have similar code as well.

Line 4798 is correct. The supportedOfferings list is build/created within the if (parseOfferings) code block. In the else case we want to return the offerings list and the size of the offerings list (supportedOfferings is also not defined in that case).

@sgoeminn why not use wPagination.size(), or is it that you want to expose the count value equal to total found/supported offerings however only return the offerings in the current page?

@rhtyd we always want to return the number of total found offerings and not only the number of offerings in the page that is returned. This is the same behavior as all the other list cmds.

@sgoeminn thanks.

rohityadavcloud · 2017-10-10T04:43:16Z

@blueorangutan package

blueorangutan · 2017-10-10T04:43:31Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2017-10-10T05:25:59Z

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1138

rohityadavcloud · 2017-10-10T05:30:42Z

@blueorangutan test

blueorangutan · 2017-10-10T05:31:32Z

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

blueorangutan · 2017-10-10T19:39:49Z

Trillian test result (tid-1568)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 50271 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2250-t1568-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
Intermitten failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermitten failure detected: /marvin/tests/smoke/test_snapshots.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Test completed. 56 look OK, 6 have error(s)

Test	Result	Time (s)	Test File
test_01_vpc_site2site_vpn	`Failure`	165.14	test_vpc_vpn.py
test_01_vpc_remote_access_vpn	`Failure`	56.16	test_vpc_vpn.py
test_router_dhcphosts	`Failure`	104.43	test_router_dhcphosts.py
test_04_rvpc_privategw_static_routes	`Failure`	654.88	test_privategw_acl.py
ContextSuite context=TestRVPCSite2SiteVpn>:setup	`Error`	0.00	test_vpc_vpn.py
ContextSuite context=TestSnapshotRootDisk>:setup	`Error`	0.00	test_snapshots.py
ContextSuite context=TestRedundantIsolateNetworks>:setup	`Error`	1848.34	test_routers_network_ops.py
ContextSuite context=TestRouterDHCPHosts>:teardown	`Error`	144.90	test_router_dhcphosts.py
ContextSuite context=TestDeployVirtioSCSIVM>:setup	`Error`	0.00	test_deploy_virtio_scsi_vm.py
test_change_service_offering_for_vm_with_snapshots	Skipped	0.00	test_vm_snapshots.py
test_09_copy_delete_template	Skipped	0.02	test_templates.py
test_06_copy_template	Skipped	0.00	test_templates.py
test_static_role_account_acls	Skipped	0.02	test_staticroles.py
test_11_ss_nfs_version_on_ssvm	Skipped	0.03	test_ssvm.py
test_01_scale_vm	Skipped	0.00	test_scale_vm.py
test_01_primary_storage_iscsi	Skipped	0.10	test_primary_storage.py
test_vm_nic_adapter_vmxnet3	Skipped	0.00	test_nic_adapter_type.py
test_nested_virtualization_vmware	Skipped	0.00	test_nested_virtualization.py
test_06_copy_iso	Skipped	0.00	test_iso.py
test_list_ha_for_host_valid	Skipped	0.04	test_hostha_simulator.py
test_list_ha_for_host_invalid	Skipped	0.03	test_hostha_simulator.py
test_list_ha_for_host	Skipped	0.03	test_hostha_simulator.py
test_hostha_enable_feature_without_setting_provider	Skipped	0.03	test_hostha_simulator.py
test_hostha_enable_feature_valid	Skipped	0.03	test_hostha_simulator.py
test_hostha_disable_feature_valid	Skipped	0.03	test_hostha_simulator.py
test_hostha_configure_invalid_provider	Skipped	0.04	test_hostha_simulator.py
test_hostha_configure_default_driver	Skipped	0.03	test_hostha_simulator.py
test_ha_verify_fsm_recovering	Skipped	0.02	test_hostha_simulator.py
test_ha_verify_fsm_fenced	Skipped	0.03	test_hostha_simulator.py
test_ha_verify_fsm_degraded	Skipped	0.02	test_hostha_simulator.py
test_ha_verify_fsm_available	Skipped	0.03	test_hostha_simulator.py
test_ha_multiple_mgmt_server_ownership	Skipped	0.03	test_hostha_simulator.py
test_ha_list_providers	Skipped	0.03	test_hostha_simulator.py
test_ha_enable_feature_invalid	Skipped	0.04	test_hostha_simulator.py
test_ha_disable_feature_invalid	Skipped	0.03	test_hostha_simulator.py
test_ha_configure_enabledisable_across_clusterzones	Skipped	0.04	test_hostha_simulator.py
test_configure_ha_provider_valid	Skipped	0.03	test_hostha_simulator.py
test_configure_ha_provider_invalid	Skipped	0.03	test_hostha_simulator.py
test_deploy_vgpu_enabled_vm	Skipped	0.03	test_deploy_vgpu_enabled_vm.py
test_3d_gpu_support	Skipped	0.04	test_deploy_vgpu_enabled_vm.py

fmaximus · 2017-10-12T11:48:11Z

I've added an extra unit test for the changed code, to keep the history intact in relation to the Trillian Tests, I have not squashed yet. I will squash once we have approval.

rohityadavcloud · 2017-10-12T11:51:03Z

@blueorangutan package

blueorangutan · 2017-10-12T11:51:30Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2017-10-12T12:19:18Z

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1154

krissterckx · 2017-10-16T19:36:22Z

server/src/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -4788,7 +4788,7 @@ protected void validateNtwkOffDetails(final Map<Detail, String> details, final M
            // Now apply pagination
            final List<? extends NetworkOffering> wPagination = StringUtils.applyPagination(supportedOfferings, cmd.getStartIndex(), cmd.getPageSizeVal());
            if (wPagination != null) {
-                final Pair<List<? extends NetworkOffering>, Integer> listWPagination = new Pair<List<? extends NetworkOffering>, Integer>(wPagination, offerings.size());
+                final Pair<List<? extends NetworkOffering>, Integer> listWPagination = new Pair<List<? extends NetworkOffering>, Integer>(wPagination, supportedOfferings.size());


fmaximus · 2017-10-27T13:50:55Z

@rhtyd Do you agree to merge this? I have squashed both commits, but somehow Jenkins fails to report back its state, and the build is gone (hooray for only keeping 5 PR builds).

… of offerings.

krissterckx · 2017-10-30T15:34:27Z

@rhtyd @fmaximus Can this be merged in ? thanks

rohityadavcloud · 2017-10-31T09:34:46Z

LGTM, sure @fmaximus @krissterckx we can merge this.

rohityadavcloud reviewed Sep 4, 2017

View reviewed changes

sgoeminn force-pushed the CLOUDSTACK-10057 branch from 8ff721f to 96c6241 Compare September 29, 2017 11:26

fmaximus approved these changes Oct 9, 2017

View reviewed changes

krissterckx approved these changes Oct 16, 2017

View reviewed changes

fmaximus added the type:bug label Oct 26, 2017

fmaximus force-pushed the CLOUDSTACK-10057 branch from f5213fc to 9a0ec3e Compare October 26, 2017 13:36

fmaximus added the status:ready-for-merge label Oct 26, 2017

CLOUDSTACK-10057: listNetworkOfferings now returns the correct number…

8016a91

… of offerings.

fmaximus force-pushed the CLOUDSTACK-10057 branch from 9a0ec3e to 8016a91 Compare October 30, 2017 12:32

fmaximus added this to the 4.11 milestone Oct 30, 2017

rohityadavcloud merged commit 587b66d into apache:master Oct 31, 2017

fmaximus deleted the CLOUDSTACK-10057 branch November 2, 2017 17:52

DaanHoogland removed the status:ready-for-merge label Dec 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLOUDSTACK-10057: listNetworkOfferings now returns the correct number of offerings. #2250

CLOUDSTACK-10057: listNetworkOfferings now returns the correct number of offerings. #2250

sgoeminn commented Aug 29, 2017

rohityadavcloud commented Aug 31, 2017

blueorangutan commented Aug 31, 2017

blueorangutan commented Aug 31, 2017

rohityadavcloud commented Aug 31, 2017

blueorangutan commented Aug 31, 2017

blueorangutan commented Aug 31, 2017

rohityadavcloud Sep 4, 2017

sgoeminn Sep 4, 2017 •

edited

Loading

rohityadavcloud Oct 31, 2017 •

edited

Loading

sgoeminn Oct 31, 2017

rohityadavcloud Oct 31, 2017

rohityadavcloud commented Oct 10, 2017

blueorangutan commented Oct 10, 2017

blueorangutan commented Oct 10, 2017

rohityadavcloud commented Oct 10, 2017

blueorangutan commented Oct 10, 2017

blueorangutan commented Oct 10, 2017

fmaximus commented Oct 12, 2017

rohityadavcloud commented Oct 12, 2017

blueorangutan commented Oct 12, 2017

blueorangutan commented Oct 12, 2017

krissterckx Oct 16, 2017

fmaximus commented Oct 27, 2017

krissterckx commented Oct 30, 2017

rohityadavcloud commented Oct 31, 2017

CLOUDSTACK-10057: listNetworkOfferings now returns the correct number of offerings. #2250

CLOUDSTACK-10057: listNetworkOfferings now returns the correct number of offerings. #2250

Conversation

sgoeminn commented Aug 29, 2017

rohityadavcloud commented Aug 31, 2017

blueorangutan commented Aug 31, 2017

blueorangutan commented Aug 31, 2017

rohityadavcloud commented Aug 31, 2017

blueorangutan commented Aug 31, 2017

blueorangutan commented Aug 31, 2017

rohityadavcloud Sep 4, 2017

Choose a reason for hiding this comment

sgoeminn Sep 4, 2017 • edited Loading

Choose a reason for hiding this comment

rohityadavcloud Oct 31, 2017 • edited Loading

Choose a reason for hiding this comment

sgoeminn Oct 31, 2017

Choose a reason for hiding this comment

rohityadavcloud Oct 31, 2017

Choose a reason for hiding this comment

rohityadavcloud commented Oct 10, 2017

blueorangutan commented Oct 10, 2017

blueorangutan commented Oct 10, 2017

rohityadavcloud commented Oct 10, 2017

blueorangutan commented Oct 10, 2017

blueorangutan commented Oct 10, 2017

fmaximus commented Oct 12, 2017

rohityadavcloud commented Oct 12, 2017

blueorangutan commented Oct 12, 2017

blueorangutan commented Oct 12, 2017

krissterckx Oct 16, 2017

Choose a reason for hiding this comment

fmaximus commented Oct 27, 2017

krissterckx commented Oct 30, 2017

rohityadavcloud commented Oct 31, 2017

sgoeminn Sep 4, 2017 •

edited

Loading

rohityadavcloud Oct 31, 2017 •

edited

Loading