CLOUDSTACK-10109: Enable dedication of public IPs to SSVM and CPVM by nvazquez · Pull Request #2295 · apache/cloudstack

nvazquez · 2017-10-12T17:30:33Z

JIRA Ticket: https://issues.apache.org/jira/browse/CLOUDSTACK-10109

Description

This feature allow admins to dedicate a range of public IP addresses to the SSVM and CPVM, such that they can be subject to specific external firewall rules. The option to dedicate a public IP range to the System VMs (SSVM & CPVM) is added to the createVlanIpRange API method and the UI.

Solution

Global setting 'system.vm.public.ip.reservation.mode.strictness' is added to determine if the use of the system VM reservation is strict (when true) or preferred (false), false by default.
When a range has been dedicated to System VMs, CloudStack should apply IPs from that range to
the public interfaces of the CPVM and the SSVM depending on global setting's value:

If the global setting is set to false: then CloudStack will use any unused and unreserved public IP
addresses for system VMs only when the pool of reserved IPs has been exhausted
If the global setting is set to true: then CloudStack will fail to deploy the system VM when the pool
of reserved IPs has been exhausted, citing the lack of available IPs.

UI Changes

Under Infrastructure -> Zone -> Physical Network -> Public -> IP Ranges, button 'Account' label is refactored to 'Set reservation':

When that button is clicked, dialog displayed is also refactored, including a new checkbox 'System VMs' which indicates if range should be dedicated for CPVM and SSVM, and a note indicating its usage:

When clicking on button for any created range, UI dialog displayed indicates whether IP range is dedicated for system vms or not:

nvazquez · 2017-10-12T17:33:20Z

@blueorangutan package

blueorangutan · 2017-10-12T17:33:30Z

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

blueorangutan · 2017-10-12T18:01:13Z

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

nvazquez · 2017-10-12T18:04:34Z

@blueorangutan test

blueorangutan · 2017-10-12T18:05:31Z

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

nvazquez · 2017-10-12T19:27:31Z

@borisstoyanov @rhtyd @DaanHoogland

Guys, one issue while porting this feature to 4.11 which was not present on 4.6. In this commit: 37301ed#diff-6adc687ad4a13d4b956fdc638299401bdomain_vlan_map is introduced. When adding a new range on domain, a new entry on domain_vlan_map is created. As IpAddressManagerImpl#fetchNewPublicIp lists non dedicated zone wide vlans, but always returns range corresponding to vlan id = 1 (as there's no entry for it on domain_vlan_map) - check ZoneWideNonDedicatedVlanSearch search builder on VlanDaoImpl.
What do you think will be the best solution for this case?

borisstoyanov · 2017-10-13T07:43:48Z

looks like the deployment failed waiting for default template to install, I'll restart
@blueorangutan test

blueorangutan · 2017-10-13T07:44:30Z

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

borisstoyanov · 2017-10-13T09:05:04Z

@blueorangutan test

blueorangutan · 2017-10-13T09:05:30Z

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

nvazquez · 2017-10-13T14:58:16Z

 // This VLAN is domain-wide, so create a DomainVlanMapVO entry
- final DomainVlanMapVO domainVlanMapVO = new DomainVlanMapVO(domain.getId(), vlan.getId());
- _domainVlanMapDao.persist(domainVlanMapVO);
+//final DomainVlanMapVO domainVlanMapVO = new DomainVlanMapVO(domain.getId(), vlan.getId());


Hi Wei @ustcweizhou,
I would like to ask you about persisting entries on doman_vlan_map table. These lines were due to a quick fix for fetching public IP for system vms range, as per search builder on VlanDaoImpl.listZoneWideNonDedicatedVlans does not consider vlans which are mapped on domain_vlan_map only returning range for vlan id = 1. Can you check my last comment on this PR as well?

I think it will break the something obviously.

blueorangutan · 2017-10-13T18:54:13Z

Trillian test result (tid-1589)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35319 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2295-t1589-kvm-centos7.zip
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_router_dhcphosts.py
Intermitten failure detected: /marvin/tests/smoke/test_ssvm.py
Intermitten failure detected: /marvin/tests/smoke/test_templates.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Test completed. 58 look OK, 4 have error(s)

Test	Result	Time (s)	Test File
test_01_vpc_remote_access_vpn	`Failure`	60.89	test_vpc_vpn.py
test_08_reboot_cpvm	`Failure`	0.03	test_ssvm.py
test_06_stop_cpvm	`Failure`	0.05	test_ssvm.py
test_04_cpvm_internals	`Failure`	0.04	test_ssvm.py
test_02_list_cpvm_vm	`Failure`	0.03	test_ssvm.py
test_04_rvpc_privategw_static_routes	`Failure`	471.52	test_privategw_acl.py
test_04_extract_Iso	`Failure`	5.10	test_iso.py
test_10_destroy_cpvm	`Error`	5.15	test_ssvm.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.08	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.02	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.02	test_hostha_simulator.py
test_hostha_disable_feature_valid	Skipped	0.02	test_hostha_simulator.py
test_hostha_configure_invalid_provider	Skipped	0.02	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.02	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.02	test_hostha_simulator.py
test_ha_list_providers	Skipped	0.02	test_hostha_simulator.py
test_ha_enable_feature_invalid	Skipped	0.02	test_hostha_simulator.py
test_ha_disable_feature_invalid	Skipped	0.02	test_hostha_simulator.py
test_ha_configure_enabledisable_across_clusterzones	Skipped	0.02	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.03	test_deploy_vgpu_enabled_vm.py
test_3d_gpu_support	Skipped	0.04	test_deploy_vgpu_enabled_vm.py

borisstoyanov

@nvazquez can you add the account/systemvm exclusiveness check here as well

nvazquez · 2017-10-18T12:25:05Z

@borisstoyanov done.
The only main concern for this community PR is fix for domain_vlan_map. @ustcweizhou could you please give your thoughts/suggestions about that fix (mentioning you as found a commit from you where domain vlan map is introduced and don't want to break your code :) ) ?

nitin-maharana · 2017-10-20T15:53:51Z

+ * @param vlanId vlan id
+ * @return true if VLAN IP range is dedicated to system vms
+ */
+ private boolean getIsForSystemVms(long vlanId){


@nvazquez, I would suggest using the name as only isForSystemVms. Thanks!

Done, thanks

nitin-maharana · 2017-10-20T16:05:03Z

- PublicIp ip = _ipAddrMgr.assignPublicIpAddress(dc.getId(), null, vm.getOwner(), VlanType.VirtualNetwork, null, null, false);
+ boolean forSystemVms = false;
+ if (vm.getType().equals(VirtualMachine.Type.ConsoleProxy) || vm.getType().equals(VirtualMachine.Type.SecondaryStorageVm)) {
+ forSystemVms = true;


Don't we need to check the value of SystemVmPublicIpReservationModeStrictness parameter before assigning the value as true for SSVM and CPVM?

Could be another alternative, actually logic is done on IpAddressManagerImpl.fetchNewPublicIp method using forSystemVms and SystemVmPublicIpReservationModeStrictness value.

@nvazquez, I just went with the flow and found out there already exists a column name ("is_system") in user_ip_address table. Can you please see if you can make use of that instead of creating a new one ("forsystemvms").

@nitin-maharana sorry for late response, been busy with other tasks. I checked usage of is_system column and are used for static nat or load balancing purposes. Best option should be including this new column.

nitin-maharana · 2017-10-20T16:31:43Z

+ return None
+
+ @attr(tags = ["advanced", "publiciprange", "dedicate", "release"], required_hardware="false")
+ def test_dedicate_public_ip_range_for_system_vms_cpvm(self):


It would be good if you add a test for SSVM as well.

Done, thanks

nitin-maharana · 2017-10-20T16:39:03Z

 var domain = json.listdomainsresponse.domain[0];

+ if (data.forSystemVms != null) {
+ systemvms = '<li>' + _l('label.system.vms') + ': ' + data.forSystemVms + '</li>'


I think this should be systemvms = data.forSystemVms, if I am not wrong.

Actually that refactoring will show only true or false in the UI, without knowing what would be it refered to. Please see last screenshot on PR description

nitin-maharana · 2017-10-20T16:40:51Z

- desc: '(optional) Please specify an account to be associated with this IP range.',
- fields: fields
+ title: 'label.set.reservation',
+ desc: '(optional) Please specify an account to be associated with this IP range.<br/><br/>'+


Can you please replace the sentence with a dictionary key here.

Done, thanks

nitin-maharana · 2017-10-20T16:54:47Z

@nvazquez, Thanks for the feature. Can you please put some snaps of the UI change, Particularly the set reservation form.

nvazquez · 2017-10-24T14:48:41Z

@nitin-maharana thanks for your review! I'll try addressing your comments asap, thanks!

nvazquez · 2017-10-26T03:40:14Z

@nitin-maharana thanks for your review and comments! UI screenshots are added on PR description and refactor is made based on your comments.

New test case added on marvin test for SSVM, results:

Test public IP range dedication ... === TestName: test_dedicatePublicIpRange | Status : SUCCESS ===
ok
Test public IP range dedication for SSVM and CPVM ... === TestName: test_dedicate_public_ip_range_for_system_vms | Status : SUCCESS ===
ok
Test CPVM Public IP ... === TestName: test_dedicate_public_ip_range_for_system_vms_cpvm | Status : SUCCESS ===
ok
Test SSVM Public IP ... === TestName: test_dedicate_public_ip_range_for_system_vms_ssvm | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 4 tests in 391.510s
OK

yadvr · 2018-01-04T05:33:21Z

@blueorangutan package

blueorangutan · 2018-01-04T05:33:31Z

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

blueorangutan · 2018-01-04T06:14:02Z

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

yadvr · 2018-01-05T06:21:12Z

@blueorangutan package

blueorangutan · 2018-01-05T06:21:32Z

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

blueorangutan · 2018-01-05T06:58:50Z

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

yadvr · 2018-01-05T07:29:20Z

@blueorangutan test

blueorangutan · 2018-01-05T07:29:31Z

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

yadvr

LGTM

borisstoyanov · 2018-01-05T15:43:01Z

@blueorangutan package

blueorangutan · 2018-01-05T15:43:31Z

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

blueorangutan · 2018-01-05T16:11:21Z

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

borisstoyanov · 2018-01-05T16:18:12Z

@blueorangutan test

blueorangutan · 2018-01-05T16:18:31Z

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

blueorangutan · 2018-01-05T18:29:26Z

Trillian test result (tid-2018)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39590 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2295-t2018-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_accounts.py
Intermitten failure detected: /marvin/tests/smoke/test_affinity_groups_projects.py
Intermitten failure detected: /marvin/tests/smoke/test_affinity_groups.py
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 64 look OK, 3 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestAccounts>:setup	`Error`	0.00	test_accounts.py
ContextSuite context=TestAddVmToSubDomain>:setup	`Error`	0.00	test_accounts.py
test_DeleteDomain	`Error`	1.59	test_accounts.py
test_forceDeleteDomain	`Error`	1.82	test_accounts.py
ContextSuite context=TestRemoveUserFromAccount>:setup	`Error`	21.96	test_accounts.py
ContextSuite context=TestDeployVmWithAffinityGroup>:setup	`Error`	0.00	test_affinity_groups_projects.py
ContextSuite context=TestDeployVmWithAffinityGroup>:setup	`Error`	0.00	test_affinity_groups.py

blueorangutan · 2018-01-06T00:40:27Z

Trillian test result (tid-2035)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 30115 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2295-t2035-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 66 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_redundant_vpc_site2site_vpn	`Failure`	228.90	test_vpc_vpn.py

borisstoyanov

tests LGTM

yadvr · 2018-01-06T17:43:44Z

Merging this based on test results and two code review lgtms.

This fixes regression introduced in PR apache#2295: - Pass assign=true to fetch new public IP - Use wait_until instead of sleep+wait in tests - Loop through list of public IP ranges to match the systemvm gateway Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

This fixes regression introduced in PR apache#2295: - Pass assign=true to fetch new public IP - Use wait_until instead of sleep+wait in tests - Loop through list of public IP ranges to match the systemvm gateway - Fix potential NPE seen when adding simulator host(s) Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

This fixes regression introduced in PR apache#2295: - Pass assign=true to fetch new public IP - Use wait_until instead of sleep+wait in tests - Loop through list of public IP ranges to match the systemvm gateway - Fix potential NPE seen when adding simulator host(s) - Removes aria2 installation from setup_agent.sh using yum, it's already dependency for cloudstack-agent package Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

This fixes regression introduced in PR #2295: - Pass assign=true to fetch new public IP - Use wait_until instead of sleep+wait in tests - Loop through list of public IP ranges to match the systemvm gateway - Fix potential NPE seen when adding simulator host(s) - Removes aria2 installation from setup_agent.sh using yum, it's already dependency for cloudstack-agent package Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

nvazquez self-assigned this Oct 12, 2017

nvazquez added component:console-proxy component:networking component:ssvm labels Oct 12, 2017

nvazquez force-pushed the systemvmpubliciprange branch 2 times, most recently from a85f65c to ca20150Compare October 13, 2017 14:56

nvazquez commented Oct 13, 2017
View reviewed changes

borisstoyanov reviewed Oct 18, 2017
View reviewed changes

nvazquez force-pushed the systemvmpubliciprange branch from ca20150 to a7855abCompare October 18, 2017 12:19

nitin-maharana reviewed Oct 20, 2017
View reviewed changes

nvazquez force-pushed the systemvmpubliciprange branch from a7855ab to c79c04cCompare October 26, 2017 03:24

yadvr approved these changes Jan 5, 2018
View reviewed changes

CLOUDSTACK-10109: Enable dedication of public IPs to SSVM and CPVM
c66c7fa

DaanHoogland force-pushed the systemvmpubliciprange branch from d741d88 to c66c7faCompare January 5, 2018 11:12

borisstoyanov approved these changes Jan 6, 2018
View reviewed changes

yadvr added 2 commits January 6, 2018 23:10

Merge branch 'master' into systemvmpubliciprange
1373d52

Merge branch 'master' into systemvmpubliciprange
8b2ae8a

yadvr merged commit 90ef67b into apache:masterJan 6, 2018

yadvr mentioned this pull request Jan 8, 2018
CLOUDSTACK-10109: Fix regression from PR #2295 #2394
Merged

yadvr mentioned this pull request Jan 14, 2018
CLOUDSTACK-9620: Enhancements for managed storage #2298
Merged

nvazquez deleted the systemvmpubliciprange branch April 6, 2020 14:52

Uh oh!

Conversation

nvazquez commented Oct 12, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Solution

UI Changes

Uh oh!

nvazquez commented Oct 12, 2017

Uh oh!

blueorangutan commented Oct 12, 2017

Uh oh!

blueorangutan commented Oct 12, 2017

Uh oh!

nvazquez commented Oct 12, 2017

Uh oh!

blueorangutan commented Oct 12, 2017

Uh oh!

nvazquez commented Oct 12, 2017

Uh oh!

borisstoyanov commented Oct 13, 2017

Uh oh!

blueorangutan commented Oct 13, 2017

Uh oh!

borisstoyanov commented Oct 13, 2017

Uh oh!

blueorangutan commented Oct 13, 2017

Uh oh!

nvazquezOct 13, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Oct 13, 2017

Uh oh!

borisstoyanov left a comment

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Oct 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nvazquezOct 26, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nitin-maharana commented Oct 20, 2017

Uh oh!

nvazquez commented Oct 24, 2017

Uh oh!

nvazquez commented Oct 26, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yadvr commented Jan 4, 2018

Uh oh!

blueorangutan commented Jan 4, 2018

Uh oh!

blueorangutan commented Jan 4, 2018

Uh oh!

nvazquez commented Oct 12, 2017•
edited
Loading

nvazquezOct 13, 2017•
edited
Loading

nvazquezOct 26, 2017•
edited
Loading

nvazquez commented Oct 26, 2017•
edited
Loading