CLOUDSTACK-9811: fixed an issue if the dev is not in the databag by swill · Pull Request #2003 · apache/cloudstack

swill · 2017-03-13T19:40:06Z

Defend against the specified dev not being in the databag.

borisstoyanov · 2017-03-13T20:15:51Z

Thanks for this fix @swill
@blueorangutan package

blueorangutan · 2017-03-13T20:16:27Z

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

blueorangutan · 2017-03-13T20:39:03Z

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

borisstoyanov · 2017-03-13T21:22:11Z

@blueorangutan test

blueorangutan · 2017-03-13T21:22:27Z

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

karuturi · 2017-03-14T04:10:59Z

Thanks Will. Can you please add bug id(CLOUDSTACK-9811) to the PR and commit message?

blueorangutan · 2017-03-14T06:35:39Z

Trillian test result (tid-954)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33186 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2003-t954-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_network.py
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_snapshots.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Test completed. 45 look ok, 4 have error(s)

Test	Result	Time (s)	Test File
test_05_rvpc_multi_tiers	`Failure`	197.06	test_vpc_redundant.py
test_04_rvpc_network_garbage_collector_nics	`Failure`	1226.73	test_vpc_redundant.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL	`Failure`	367.39	test_vpc_redundant.py
test_04_rvpc_privategw_static_routes	`Failure`	308.87	test_privategw_acl.py
test_network_rules_acquired_public_ip_3_Load_Balancer_Rule	`Failure`	41.60	test_network.py
test_network_rules_acquired_public_ip_2_nat_rule	`Failure`	31.61	test_network.py
test_network_rules_acquired_public_ip_1_static_nat_rule	`Failure`	93.45	test_network.py
test_02_list_snapshots_with_removed_data_store	`Error`	0.03	test_snapshots.py
test_01_vpc_site2site_vpn	Success	154.57	test_vpc_vpn.py
test_01_vpc_remote_access_vpn	Success	60.82	test_vpc_vpn.py
test_01_redundant_vpc_site2site_vpn	Success	245.94	test_vpc_vpn.py
test_02_VPC_default_routes	Success	275.74	test_vpc_router_nics.py
test_01_VPC_nics_after_destroy	Success	501.27	test_vpc_router_nics.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers	Success	541.72	test_vpc_redundant.py
test_02_redundant_VPC_default_routes	Success	742.74	test_vpc_redundant.py
test_09_delete_detached_volume	Success	151.17	test_volumes.py
test_08_resize_volume	Success	156.07	test_volumes.py
test_07_resize_fail	Success	161.42	test_volumes.py
test_06_download_detached_volume	Success	156.57	test_volumes.py
test_05_detach_volume	Success	155.55	test_volumes.py
test_04_delete_attached_volume	Success	151.12	test_volumes.py
test_03_download_attached_volume	Success	156.10	test_volumes.py
test_02_attach_volume	Success	89.55	test_volumes.py
test_01_create_volume	Success	710.80	test_volumes.py
test_03_delete_vm_snapshots	Success	275.19	test_vm_snapshots.py
test_02_revert_vm_snapshots	Success	95.66	test_vm_snapshots.py
test_01_create_vm_snapshots	Success	159.69	test_vm_snapshots.py
test_deploy_vm_multiple	Success	256.90	test_vm_life_cycle.py
test_deploy_vm	Success	0.02	test_vm_life_cycle.py
test_advZoneVirtualRouter	Success	0.02	test_vm_life_cycle.py
test_10_attachAndDetach_iso	Success	26.48	test_vm_life_cycle.py
test_09_expunge_vm	Success	125.15	test_vm_life_cycle.py
test_08_migrate_vm	Success	30.59	test_vm_life_cycle.py
test_07_restore_vm	Success	0.09	test_vm_life_cycle.py
test_06_destroy_vm	Success	130.63	test_vm_life_cycle.py
test_03_reboot_vm	Success	125.63	test_vm_life_cycle.py
test_02_start_vm	Success	10.12	test_vm_life_cycle.py
test_01_stop_vm	Success	40.24	test_vm_life_cycle.py
test_CreateTemplateWithDuplicateName	Success	50.38	test_templates.py
test_08_list_system_templates	Success	0.02	test_templates.py
test_07_list_public_templates	Success	0.03	test_templates.py
test_05_template_permissions	Success	0.04	test_templates.py
test_04_extract_template	Success	5.11	test_templates.py
test_03_delete_template	Success	5.08	test_templates.py
test_02_edit_template	Success	90.16	test_templates.py
test_01_create_template	Success	20.24	test_templates.py
test_10_destroy_cpvm	Success	161.54	test_ssvm.py
test_09_destroy_ssvm	Success	138.36	test_ssvm.py
test_08_reboot_cpvm	Success	101.44	test_ssvm.py
test_07_reboot_ssvm	Success	133.44	test_ssvm.py
test_06_stop_cpvm	Success	132.03	test_ssvm.py
test_05_stop_ssvm	Success	163.51	test_ssvm.py
test_04_cpvm_internals	Success	1.15	test_ssvm.py
test_03_ssvm_internals	Success	3.27	test_ssvm.py
test_02_list_cpvm_vm	Success	0.08	test_ssvm.py
test_01_list_sec_storage_vm	Success	0.09	test_ssvm.py
test_01_snapshot_root_disk	Success	10.92	test_snapshots.py
test_04_change_offering_small	Success	234.25	test_service_offerings.py
test_03_delete_service_offering	Success	0.03	test_service_offerings.py
test_02_edit_service_offering	Success	0.04	test_service_offerings.py
test_01_create_service_offering	Success	0.07	test_service_offerings.py
test_02_sys_template_ready	Success	0.09	test_secondary_storage.py
test_01_sys_vm_start	Success	0.12	test_secondary_storage.py
test_09_reboot_router	Success	40.25	test_routers.py
test_08_start_router	Success	30.20	test_routers.py
test_07_stop_router	Success	10.12	test_routers.py
test_06_router_advanced	Success	0.04	test_routers.py
test_05_router_basic	Success	0.03	test_routers.py
test_04_restart_network_wo_cleanup	Success	5.65	test_routers.py
test_03_restart_network_cleanup	Success	60.39	test_routers.py
test_02_router_internal_adv	Success	1.04	test_routers.py
test_01_router_internal_basic	Success	0.56	test_routers.py
test_router_dns_guestipquery	Success	76.67	test_router_dns.py
test_router_dns_externalipquery	Success	0.06	test_router_dns.py
test_router_dhcphosts	Success	286.44	test_router_dhcphosts.py
test_router_dhcp_opts	Success	21.51	test_router_dhcphosts.py
test_01_updatevolumedetail	Success	0.05	test_resource_detail.py
test_01_reset_vm_on_reboot	Success	145.83	test_reset_vm_on_reboot.py
test_createRegion	Success	0.03	test_regions.py
test_create_pvlan_network	Success	5.14	test_pvlan.py
test_dedicatePublicIpRange	Success	0.30	test_public_ip_range.py
test_03_vpc_privategw_restart_vpc_cleanup	Success	473.19	test_privategw_acl.py
test_02_vpc_privategw_static_routes	Success	348.33	test_privategw_acl.py
test_01_vpc_privategw_acl	Success	81.51	test_privategw_acl.py
test_01_primary_storage_nfs	Success	35.65	test_primary_storage.py
test_createPortablePublicIPRange	Success	15.14	test_portable_publicip.py
test_createPortablePublicIPAcquire	Success	15.40	test_portable_publicip.py
test_isolate_network_password_server	Success	90.19	test_password_server.py
test_UpdateStorageOverProvisioningFactor	Success	0.10	test_over_provisioning.py
test_oobm_zchange_password	Success	25.44	test_outofbandmanagement.py
test_oobm_multiple_mgmt_server_ownership	Success	16.27	test_outofbandmanagement.py
test_oobm_issue_power_status	Success	10.18	test_outofbandmanagement.py
test_oobm_issue_power_soft	Success	10.22	test_outofbandmanagement.py
test_oobm_issue_power_reset	Success	15.38	test_outofbandmanagement.py
test_oobm_issue_power_on	Success	15.24	test_outofbandmanagement.py
test_oobm_issue_power_off	Success	10.22	test_outofbandmanagement.py
test_oobm_issue_power_cycle	Success	15.24	test_outofbandmanagement.py
test_oobm_enabledisable_across_clusterzones	Success	87.20	test_outofbandmanagement.py
test_oobm_enable_feature_valid	Success	5.12	test_outofbandmanagement.py
test_oobm_enable_feature_invalid	Success	0.26	test_outofbandmanagement.py
test_oobm_disable_feature_valid	Success	5.13	test_outofbandmanagement.py
test_oobm_disable_feature_invalid	Success	0.07	test_outofbandmanagement.py
test_oobm_configure_invalid_driver	Success	0.06	test_outofbandmanagement.py
test_oobm_configure_default_driver	Success	0.05	test_outofbandmanagement.py
test_oobm_background_powerstate_sync	Success	23.29	test_outofbandmanagement.py
test_extendPhysicalNetworkVlan	Success	15.41	test_non_contigiousvlan.py
test_01_nic	Success	553.91	test_nic.py
test_releaseIP	Success	252.16	test_network.py
test_reboot_router	Success	397.49	test_network.py
test_public_ip_user_account	Success	10.19	test_network.py
test_public_ip_admin_account	Success	40.22	test_network.py
test_delete_account	Success	281.99	test_network.py
test_02_port_fwd_on_non_src_nat	Success	55.46	test_network.py
test_01_port_fwd_on_src_nat	Success	111.57	test_network.py
test_nic_secondaryip_add_remove	Success	216.96	test_multipleips_per_nic.py
login_test_saml_user	Success	17.82	test_login.py
test_assign_and_removal_lb	Success	148.45	test_loadbalance.py
test_02_create_lb_rule_non_nat	Success	202.12	test_loadbalance.py
test_01_create_lb_rule_src_nat	Success	218.04	test_loadbalance.py
test_03_list_snapshots	Success	0.04	test_list_ids_parameter.py
test_02_list_templates	Success	0.03	test_list_ids_parameter.py
test_01_list_volumes	Success	0.02	test_list_ids_parameter.py
test_07_list_default_iso	Success	0.04	test_iso.py
test_05_iso_permissions	Success	0.04	test_iso.py
test_04_extract_Iso	Success	5.11	test_iso.py
test_03_delete_iso	Success	95.11	test_iso.py
test_02_edit_iso	Success	0.04	test_iso.py
test_01_create_iso	Success	20.70	test_iso.py
test_04_rvpc_internallb_haproxy_stats_on_all_interfaces	Success	197.91	test_internal_lb.py
test_03_vpc_internallb_haproxy_stats_on_all_interfaces	Success	137.18	test_internal_lb.py
test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80	Success	536.15	test_internal_lb.py
test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80	Success	439.18	test_internal_lb.py
test_dedicateGuestVlanRange	Success	10.19	test_guest_vlan_range.py
test_UpdateConfigParamWithScope	Success	0.10	test_global_settings.py
test_rolepermission_lifecycle_update	Success	5.79	test_dynamicroles.py
test_rolepermission_lifecycle_list	Success	5.68	test_dynamicroles.py
test_rolepermission_lifecycle_delete	Success	5.58	test_dynamicroles.py
test_rolepermission_lifecycle_create	Success	5.60	test_dynamicroles.py
test_rolepermission_lifecycle_concurrent_updates	Success	5.69	test_dynamicroles.py
test_role_lifecycle_update_role_inuse	Success	5.61	test_dynamicroles.py
test_role_lifecycle_update	Success	10.67	test_dynamicroles.py
test_role_lifecycle_list	Success	5.61	test_dynamicroles.py
test_role_lifecycle_delete	Success	10.63	test_dynamicroles.py
test_role_lifecycle_create	Success	5.62	test_dynamicroles.py
test_role_inuse_deletion	Success	5.59	test_dynamicroles.py
test_role_account_acls_multiple_mgmt_servers	Success	7.03	test_dynamicroles.py
test_role_account_acls	Success	7.06	test_dynamicroles.py
test_default_role_deletion	Success	5.65	test_dynamicroles.py
test_04_create_fat_type_disk_offering	Success	0.05	test_disk_offerings.py
test_03_delete_disk_offering	Success	0.03	test_disk_offerings.py
test_02_edit_disk_offering	Success	0.04	test_disk_offerings.py
test_02_create_sparse_type_disk_offering	Success	0.05	test_disk_offerings.py
test_01_create_disk_offering	Success	0.07	test_disk_offerings.py
test_deployvm_userdispersing	Success	20.40	test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_userconcentrated	Success	20.40	test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_firstfit	Success	60.49	test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_userdata_post	Success	10.31	test_deploy_vm_with_userdata.py
test_deployvm_userdata	Success	60.59	test_deploy_vm_with_userdata.py
test_02_deploy_vm_root_resize	Success	5.66	test_deploy_vm_root_resize.py
test_01_deploy_vm_root_resize	Success	5.66	test_deploy_vm_root_resize.py
test_00_deploy_vm_root_resize	Success	206.73	test_deploy_vm_root_resize.py
test_deploy_vm_from_iso	Success	206.85	test_deploy_vm_iso.py
test_DeployVmAntiAffinityGroup	Success	60.74	test_affinity_groups.py
test_change_service_offering_for_vm_with_snapshots	Skipped	0.00	test_vm_snapshots.py
test_01_test_vm_volume_snapshot	Skipped	0.00	test_vm_snapshots.py
test_06_copy_template	Skipped	0.00	test_templates.py
test_static_role_account_acls	Skipped	0.01	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.03	test_primary_storage.py
test_nested_virtualization_vmware	Skipped	0.00	test_nested_virtualization.py
test_06_copy_iso	Skipped	0.00	test_iso.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

ustcweizhou · 2017-03-14T07:08:21Z

not tested it. but it seems the 'else' part also need to be changed.

borisstoyanov

I've tested the changes and the VR does come up now, but I'm really concerned from the test failures we're getting

swill · 2017-03-14T13:26:13Z

@ustcweizhou the else case did not change from the original code: https://github.com/apache/cloudstack/pull/1741/files#diff-a7d6f7150cca74029f23c19b72ad0622L49

@karuturi I have updated the PR title and I updated the commit message to include the jira ticket.

@borisstoyanov do we have a run of these tests just against master recently to review the difference in output?

ustcweizhou · 2017-03-14T13:34:49Z

@swill what I mean is , if you do not want to change the logic, do you need to change 'else:' to 'elif index == -1:' ?

swill · 2017-03-14T14:04:46Z

I have to admit, I am not sure how the IP was found when looping through the list initially, but then when trying to update the index for the IP that was found, it is not there. The only way I can think of that would cause this is if the IP <-> dev mapping is broken somehow. It found the matching IP, but apparently that IP is not on the dev defined by the nic_dev_id (which comes from either: nic_dev_id = address['nic_dev_id'] (from the databag), which can then be overridden by nic_dev_id = ip['nic_dev_id'] (from the passed in IP) ).

Yes, I think you are right that the else case does not work right now. This is because we are appending the IP to this dev, but we never removed it from the other dev it was apparently found in. This logic is SUPPOSED to find the existing IP's index and then if found, update that index with the updated info.

The old logic which just removed all "found" IPs and then re-added them, did not preserve the order, so the source nat IP could get reconfigured as a secondary IP on a nic instead of it being primary. This reordering of the IPs on VR reboot caused the VPN to fail because the source nat IP was no longer the primary IP.

So we know that removing all found IPs and then adding them does not work because it changes the order of the IPs causing the source nat IP to become a secondary IP on the nic.

The error described in CLOUDSTACK-9811 seems to be a situation where the IP is found on one nic, but then for some reason the dev which the IP is associated with is changed (I don't know why this would be happening), causing the index where the IP was found to not be valid because the IP is actually on a different dev.

To know how to fix this correctly, we need to understand why/how an IP can be associated with one dev (index found) and then get changed to be associated with a different dev.

We need to find a way to preserve IP order while being able to update the IP configurations.

@remibergsma do you have any ideas on this? Anyone else have ideas here?

swill · 2017-03-14T14:19:58Z

On the Jira ticket, Wei Zhou mentioned that this could potentially happen if you have two different public IP ranges. So if you associate a public IP which is in a different range than the source nat, a new nic will be created. If you disassociate it, that nic will be removed.

I guess my question is. Why is it not on thedbagvariable then associated with that dev? I think Wei is probably right in terms of context, but I am not sure how exactly to solve this yet.

@borisstoyanov can you confirm that you have two different public IP ranges in your setup?

borisstoyanov · 2017-03-14T14:40:43Z

@swill we're using one IP range for the physical guest public network. I think we could set this up if required?

ustcweizhou · 2017-03-14T14:42:32Z

@borisstoyanov are you using vpc ?

swill · 2017-03-14T15:37:05Z

@borisstoyanov we are still trying to understand how the IP was found when looping through the dbag but it is not associated with the expected dev when we try to update the index which it was found on. The only way I can see this could happen is if the dev requested it be configured on is different from the dev it is actually configured on. I still have not figured out how that case exists.

@ustcweizhou has some good ideas on this front, but I don't think we have gotten to a point where we understand how this problem is happening.

@borisstoyanov is there a chance you can post the ips.json and ip_association.json files in your environment so we can start to understand what the config is that is causing this?

borisstoyanov · 2017-03-14T17:46:36Z

@swill thanks, I found the ips.json, but I couldn't find the ip_association.json on the VR. I guess it's not processed yet. Can you advise how to force it to generate? Hope this helps.
ips.txt

borisstoyanov · 2017-03-14T17:47:09Z

@ustcweizhou no VPC

swill · 2017-03-14T17:51:35Z

@borisstoyanov I have a suspicion that the ip_association.json is a transient file which may be short lived. I am not sure because I have not seen it either, but it was the file being processed when you had your original error.

I will review your ips.json and see what I can find.

swill · 2017-03-14T18:49:37Z

This ips.json file confuses the hell out of me. From what I can tell both eth2 and eth3 both have the ip 10.1.35.83 twice for each (for a total of the IP being configured 4 times over two interfaces).

How was this IP configured?

 "eth2": [
{
"add": true,
"broadcast": "10.1.63.255",
"cidr": "10.1.35.83/19",
"device": "eth2",
"gateway": "10.1.63.254",
"netmask": "255.255.224.0",
"network": "10.1.32.0/19",
"nic_dev_id": "2",
"nw_type": "public",
"one_to_one_nat": false,
"public_ip": "10.1.35.83",
"size": "19",
"source_nat": false
},
{
"add": true,
"broadcast": "10.1.63.255",
"cidr": "10.1.35.83/19",
"device": "eth2",
"gateway": "10.1.63.254",
"netmask": "255.255.224.0",
"network": "10.1.32.0/19",
"nic_dev_id": "2",
"nw_type": "public",
"one_to_one_nat": false,
"public_ip": "10.1.35.83",
"size": "19",
"source_nat": false
}
],
"eth3": [
{
"add": true,
"broadcast": "10.1.63.255",
"cidr": "10.1.35.83/19",
"device": "eth3",
"first_i_p": true,
"gateway": "10.1.63.254",
"netmask": "255.255.224.0",
"network": "10.1.32.0/19",
"new_nic": true,
"nic_dev_id": 3,
"nw_type": "public",
"one_to_one_nat": false,
"public_ip": "10.1.35.83",
"size": "19",
"source_nat": true,
"vif_mac_address": "06:ee:46:00:00:03"
},
{
"add": true,
"broadcast": "10.1.63.255",
"cidr": "10.1.35.83/19",
"device": "eth3",
"first_i_p": true,
"gateway": "10.1.63.254",
"netmask": "255.255.224.0",
"network": "10.1.32.0/19",
"new_nic": true,
"nic_dev_id": 3,
"nw_type": "public",
"one_to_one_nat": false,
"public_ip": "10.1.35.83",
"size": "19",
"source_nat": true,
"vif_mac_address": "06:a4:80:00:00:03"
}
],

I will see if I can get to the bottom of this, but this is pretty confusing. Do you have a sequence of events which produced this configuration?

swill · 2017-03-14T19:05:49Z

So... Looking at this with a different perspective. If there is a bug elsewhere which would cause an IP to be duplicated in the dbag, the old implementation would have cleaned up this bug and would have removed all the duplicates (including the real one) and then would have only added back the real one. My implementation did not assume that I was having to clean up duplicates in the dbag variable and only focused on adding if it didn't exist or updating the existing entry if it did exist. I will have to think about this more...

ustcweizhou · 2017-03-14T20:35:37Z

the ips.json is very very weird

borisstoyanov · 2017-03-14T20:39:00Z

yes, agree with you @swill, it seems eth3 is duplicated eth2. The VR we're trying to bring up isn't supposed to have eth3 at all...

ustcweizhou · 2017-03-14T21:10:28Z

it seems nicDevId is not set correctly in java code.

ustcweizhou · 2017-03-14T21:13:49Z

@borisstoyanov what's the last commit in your code ?
could you check how many nics attached to the VR in database, select * from nics where instance_id= ?

swill · 2017-03-14T21:18:59Z

Ya, I am not sure where the problem is stemming from. The fact that my code is not defensive around this problem is a problem. That is for sure. But given my understanding of the moving parts here, I feel like there is something else going on as well. I will see if I can get an environment up and running tomorrow so I can do some more testing on this.

ustcweizhou · 2017-03-15T07:18:36Z

@borisstoyanov moreover, can you please restart the network with cleanup, to see if the new VRs can start ?

swill · 2017-03-16T16:04:25Z

I am considering changing my implementation to be the same as the old implementation (which removed the IP from the dbag in the initial loop of the merge), but if source_nat is present and it is true and the len(dbag[ip['device']]) > 0 (with all the checking required), then prepend else append. This should ensure that the source nat ip is the primary ip on the nic which the StrongSwan feature depends on.

If I do this and there is a bug elsewhere that will duplicate the IPs in the databag, then this will clean that up (as I assume it was doing before).

Not sure this will fix @borisstoyanov's current problem, but I think it is probably a safer implementation to keep the source nat IP as the first IP.

I will update this PR with that change later today...

borisstoyanov · 2017-03-17T14:18:09Z

Hi @swill, could you please let us know when do you expect to address the changes you have mentioned, so I could schedule the testing accordingly. Thanks.

ustcweizhou · 2017-03-17T17:10:33Z

@borisstoyanov could you please test with my suggestion? I think it should fix the duplicated nics in VR.

remibergsma · 2017-03-17T18:53:14Z

@swill FYI if you look for first_i_p as seen in the json on the Python side, then on the Java side it's called firstIP, as the gson lib replaces every capital with an underscore and then the lowercase letter.

swill · 2017-03-17T19:02:03Z

Thanks @remibergsma. I am going to use source_nat as it better represents what it is I am trying to establish and it seems to always be in the object where first_i_p does not always seem to be present (look at @borisstoyanov's output above for example).

@borisstoyanov I will work on getting you the new implementation today.

swill · 2017-03-17T19:10:50Z

@borisstoyanov alright, this is ready for you to start testing. Can you kick off CI on this as well?

I will be doing testing of this locally as well.

This implementation is very similar to how it was implemented before my original change. See: https://github.com/apache/cloudstack/pull/1741/files#diff-a7d6f7150cca74029f23c19b72ad0622L19

The only change from the original was that if the IP is a source_nat IP and there are already IPs associated with that dev, it will prepend instead of append so the source nat ip is set as the primary ip on the nic (required for VPN).

borisstoyanov · 2017-03-17T21:39:56Z

not sure about the CI tests @swill, I think the easiest way to kick Travis tests is the close/reopen the PR.
I'll pick it up as soon as possible. I'll rebuild and run the smoketests.
@blueorangutan package

blueorangutan · 2017-03-17T21:40:45Z

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

blueorangutan · 2017-03-17T22:03:20Z

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

borisstoyanov · 2017-03-17T22:06:13Z

@blueorangutan test

blueorangutan · 2017-03-17T22:06:45Z

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

swill · 2017-03-17T22:25:31Z

thank you sir. :)

borisstoyanov · 2017-03-18T08:28:55Z

@blueorangutan test

blueorangutan · 2017-03-18T08:29:47Z

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

blueorangutan · 2017-03-18T17:21:00Z

Trillian test result (tid-958)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31840 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2003-t958-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_snapshots.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Test completed. 46 look ok, 3 have error(s)

Test	Result	Time (s)	Test File
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL	`Failure`	369.05	test_vpc_redundant.py
test_04_rvpc_privategw_static_routes	`Failure`	335.61	test_privategw_acl.py
test_02_list_snapshots_with_removed_data_store	`Error`	0.04	test_snapshots.py
test_01_vpc_site2site_vpn	Success	145.03	test_vpc_vpn.py
test_01_vpc_remote_access_vpn	Success	66.12	test_vpc_vpn.py
test_01_redundant_vpc_site2site_vpn	Success	250.53	test_vpc_vpn.py
test_02_VPC_default_routes	Success	275.54	test_vpc_router_nics.py
test_01_VPC_nics_after_destroy	Success	547.92	test_vpc_router_nics.py
test_05_rvpc_multi_tiers	Success	508.38	test_vpc_redundant.py
test_04_rvpc_network_garbage_collector_nics	Success	1403.12	test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers	Success	543.21	test_vpc_redundant.py
test_02_redundant_VPC_default_routes	Success	750.68	test_vpc_redundant.py
test_09_delete_detached_volume	Success	156.45	test_volumes.py
test_08_resize_volume	Success	156.39	test_volumes.py
test_07_resize_fail	Success	161.50	test_volumes.py
test_06_download_detached_volume	Success	156.27	test_volumes.py
test_05_detach_volume	Success	150.72	test_volumes.py
test_04_delete_attached_volume	Success	151.19	test_volumes.py
test_03_download_attached_volume	Success	156.32	test_volumes.py
test_02_attach_volume	Success	94.54	test_volumes.py
test_01_create_volume	Success	711.06	test_volumes.py
test_03_delete_vm_snapshots	Success	275.20	test_vm_snapshots.py
test_02_revert_vm_snapshots	Success	95.66	test_vm_snapshots.py
test_01_create_vm_snapshots	Success	163.76	test_vm_snapshots.py
test_deploy_vm_multiple	Success	247.58	test_vm_life_cycle.py
test_deploy_vm	Success	0.03	test_vm_life_cycle.py
test_advZoneVirtualRouter	Success	0.02	test_vm_life_cycle.py
test_10_attachAndDetach_iso	Success	26.64	test_vm_life_cycle.py
test_09_expunge_vm	Success	125.24	test_vm_life_cycle.py
test_08_migrate_vm	Success	45.95	test_vm_life_cycle.py
test_07_restore_vm	Success	0.15	test_vm_life_cycle.py
test_06_destroy_vm	Success	125.83	test_vm_life_cycle.py
test_03_reboot_vm	Success	125.85	test_vm_life_cycle.py
test_02_start_vm	Success	10.16	test_vm_life_cycle.py
test_01_stop_vm	Success	40.31	test_vm_life_cycle.py
test_CreateTemplateWithDuplicateName	Success	30.38	test_templates.py
test_08_list_system_templates	Success	0.03	test_templates.py
test_07_list_public_templates	Success	0.04	test_templates.py
test_05_template_permissions	Success	0.06	test_templates.py
test_04_extract_template	Success	5.13	test_templates.py
test_03_delete_template	Success	5.13	test_templates.py
test_02_edit_template	Success	90.17	test_templates.py
test_01_create_template	Success	25.33	test_templates.py
test_10_destroy_cpvm	Success	161.59	test_ssvm.py
test_09_destroy_ssvm	Success	163.56	test_ssvm.py
test_08_reboot_cpvm	Success	101.50	test_ssvm.py
test_07_reboot_ssvm	Success	133.54	test_ssvm.py
test_06_stop_cpvm	Success	131.72	test_ssvm.py
test_05_stop_ssvm	Success	193.66	test_ssvm.py
test_04_cpvm_internals	Success	1.22	test_ssvm.py
test_03_ssvm_internals	Success	3.25	test_ssvm.py
test_02_list_cpvm_vm	Success	0.12	test_ssvm.py
test_01_list_sec_storage_vm	Success	0.13	test_ssvm.py
test_01_snapshot_root_disk	Success	11.12	test_snapshots.py
test_04_change_offering_small	Success	239.60	test_service_offerings.py
test_03_delete_service_offering	Success	0.04	test_service_offerings.py
test_02_edit_service_offering	Success	0.05	test_service_offerings.py
test_01_create_service_offering	Success	0.11	test_service_offerings.py
test_02_sys_template_ready	Success	0.12	test_secondary_storage.py
test_01_sys_vm_start	Success	0.18	test_secondary_storage.py
test_09_reboot_router	Success	35.33	test_routers.py
test_08_start_router	Success	30.27	test_routers.py
test_07_stop_router	Success	10.16	test_routers.py
test_06_router_advanced	Success	0.06	test_routers.py
test_05_router_basic	Success	0.04	test_routers.py
test_04_restart_network_wo_cleanup	Success	5.69	test_routers.py
test_03_restart_network_cleanup	Success	55.48	test_routers.py
test_02_router_internal_adv	Success	1.06	test_routers.py
test_01_router_internal_basic	Success	0.57	test_routers.py
test_router_dns_guestipquery	Success	76.73	test_router_dns.py
test_router_dns_externalipquery	Success	0.08	test_router_dns.py
test_router_dhcphosts	Success	277.82	test_router_dhcphosts.py
test_router_dhcp_opts	Success	21.71	test_router_dhcphosts.py
test_01_updatevolumedetail	Success	0.07	test_resource_detail.py
test_01_reset_vm_on_reboot	Success	130.88	test_reset_vm_on_reboot.py
test_createRegion	Success	0.04	test_regions.py
test_create_pvlan_network	Success	5.22	test_pvlan.py
test_dedicatePublicIpRange	Success	0.41	test_public_ip_range.py
test_03_vpc_privategw_restart_vpc_cleanup	Success	479.72	test_privategw_acl.py
test_02_vpc_privategw_static_routes	Success	370.17	test_privategw_acl.py
test_01_vpc_privategw_acl	Success	87.09	test_privategw_acl.py
test_01_primary_storage_nfs	Success	35.81	test_primary_storage.py
test_createPortablePublicIPRange	Success	15.18	test_portable_publicip.py
test_createPortablePublicIPAcquire	Success	15.42	test_portable_publicip.py
test_isolate_network_password_server	Success	89.31	test_password_server.py
test_UpdateStorageOverProvisioningFactor	Success	0.13	test_over_provisioning.py
test_oobm_zchange_password	Success	30.64	test_outofbandmanagement.py
test_oobm_multiple_mgmt_server_ownership	Success	16.34	test_outofbandmanagement.py
test_oobm_issue_power_status	Success	10.25	test_outofbandmanagement.py
test_oobm_issue_power_soft	Success	15.32	test_outofbandmanagement.py
test_oobm_issue_power_reset	Success	15.31	test_outofbandmanagement.py
test_oobm_issue_power_on	Success	15.32	test_outofbandmanagement.py
test_oobm_issue_power_off	Success	15.34	test_outofbandmanagement.py
test_oobm_issue_power_cycle	Success	15.35	test_outofbandmanagement.py
test_oobm_enabledisable_across_clusterzones	Success	82.49	test_outofbandmanagement.py
test_oobm_enable_feature_valid	Success	5.15	test_outofbandmanagement.py
test_oobm_enable_feature_invalid	Success	0.09	test_outofbandmanagement.py
test_oobm_disable_feature_valid	Success	5.17	test_outofbandmanagement.py
test_oobm_disable_feature_invalid	Success	0.10	test_outofbandmanagement.py
test_oobm_configure_invalid_driver	Success	0.08	test_outofbandmanagement.py
test_oobm_configure_default_driver	Success	0.07	test_outofbandmanagement.py
test_oobm_background_powerstate_sync	Success	23.39	test_outofbandmanagement.py
test_extendPhysicalNetworkVlan	Success	15.31	test_non_contigiousvlan.py
test_01_nic	Success	419.21	test_nic.py
test_releaseIP	Success	257.96	test_network.py
test_reboot_router	Success	413.50	test_network.py
test_public_ip_user_account	Success	10.25	test_network.py
test_public_ip_admin_account	Success	40.27	test_network.py
test_network_rules_acquired_public_ip_3_Load_Balancer_Rule	Success	66.91	test_network.py
test_network_rules_acquired_public_ip_2_nat_rule	Success	61.69	test_network.py
test_network_rules_acquired_public_ip_1_static_nat_rule	Success	123.73	test_network.py
test_delete_account	Success	287.98	test_network.py
test_02_port_fwd_on_non_src_nat	Success	55.76	test_network.py
test_01_port_fwd_on_src_nat	Success	111.70	test_network.py
test_nic_secondaryip_add_remove	Success	202.47	test_multipleips_per_nic.py
login_test_saml_user	Success	19.25	test_login.py
test_assign_and_removal_lb	Success	133.35	test_loadbalance.py
test_02_create_lb_rule_non_nat	Success	187.33	test_loadbalance.py
test_01_create_lb_rule_src_nat	Success	218.95	test_loadbalance.py
test_03_list_snapshots	Success	0.06	test_list_ids_parameter.py
test_02_list_templates	Success	0.04	test_list_ids_parameter.py
test_01_list_volumes	Success	0.03	test_list_ids_parameter.py
test_07_list_default_iso	Success	0.06	test_iso.py
test_05_iso_permissions	Success	0.06	test_iso.py
test_04_extract_Iso	Success	5.14	test_iso.py
test_03_delete_iso	Success	95.19	test_iso.py
test_02_edit_iso	Success	0.05	test_iso.py
test_01_create_iso	Success	21.01	test_iso.py
test_04_rvpc_internallb_haproxy_stats_on_all_interfaces	Success	198.50	test_internal_lb.py
test_03_vpc_internallb_haproxy_stats_on_all_interfaces	Success	147.65	test_internal_lb.py
test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80	Success	495.73	test_internal_lb.py
test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80	Success	430.60	test_internal_lb.py
test_dedicateGuestVlanRange	Success	10.26	test_guest_vlan_range.py
test_UpdateConfigParamWithScope	Success	0.14	test_global_settings.py
test_rolepermission_lifecycle_update	Success	6.17	test_dynamicroles.py
test_rolepermission_lifecycle_list	Success	5.98	test_dynamicroles.py
test_rolepermission_lifecycle_delete	Success	5.87	test_dynamicroles.py
test_rolepermission_lifecycle_create	Success	5.89	test_dynamicroles.py
test_rolepermission_lifecycle_concurrent_updates	Success	6.00	test_dynamicroles.py
test_role_lifecycle_update_role_inuse	Success	5.90	test_dynamicroles.py
test_role_lifecycle_update	Success	10.98	test_dynamicroles.py
test_role_lifecycle_list	Success	5.90	test_dynamicroles.py
test_role_lifecycle_delete	Success	10.93	test_dynamicroles.py
test_role_lifecycle_create	Success	5.89	test_dynamicroles.py
test_role_inuse_deletion	Success	5.86	test_dynamicroles.py
test_role_account_acls_multiple_mgmt_servers	Success	8.14	test_dynamicroles.py
test_role_account_acls	Success	8.20	test_dynamicroles.py
test_default_role_deletion	Success	5.97	test_dynamicroles.py
test_04_create_fat_type_disk_offering	Success	0.06	test_disk_offerings.py
test_03_delete_disk_offering	Success	0.04	test_disk_offerings.py
test_02_edit_disk_offering	Success	0.05	test_disk_offerings.py
test_02_create_sparse_type_disk_offering	Success	0.07	test_disk_offerings.py
test_01_create_disk_offering	Success	0.10	test_disk_offerings.py
test_deployvm_userdispersing	Success	20.56	test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_userconcentrated	Success	30.65	test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_firstfit	Success	50.63	test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_userdata_post	Success	10.48	test_deploy_vm_with_userdata.py
test_deployvm_userdata	Success	50.78	test_deploy_vm_with_userdata.py
test_02_deploy_vm_root_resize	Success	5.97	test_deploy_vm_root_resize.py
test_01_deploy_vm_root_resize	Success	5.99	test_deploy_vm_root_resize.py
test_00_deploy_vm_root_resize	Success	207.53	test_deploy_vm_root_resize.py
test_deploy_vm_from_iso	Success	207.46	test_deploy_vm_iso.py
test_DeployVmAntiAffinityGroup	Success	60.88	test_affinity_groups.py
test_change_service_offering_for_vm_with_snapshots	Skipped	0.00	test_vm_snapshots.py
test_01_test_vm_volume_snapshot	Skipped	0.00	test_vm_snapshots.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.04	test_primary_storage.py
test_nested_virtualization_vmware	Skipped	0.00	test_nested_virtualization.py
test_06_copy_iso	Skipped	0.00	test_iso.py
test_deploy_vgpu_enabled_vm	Skipped	0.03	test_deploy_vgpu_enabled_vm.py
test_3d_gpu_support	Skipped	0.03	test_deploy_vgpu_enabled_vm.py

borisstoyanov

Thanks for the fix @swill, tests are looking good, I've manually confirmed VR is coming up.
LGTM

borisstoyanov · 2017-03-20T08:15:20Z

ping @rhtyd @DaanHoogland @abhinandanprateek @PaulAngus for review.

DaanHoogland · 2017-03-20T08:57:28Z

code LGTM but @ustcweizhou 's patch still makes sense to me. Maybe you can add that separately? travis still fails!

borisstoyanov · 2017-03-20T09:03:44Z

@DaanHoogland you could review it as well it addressed in https://github.com/apache/cloudstack/pull/2011/files

karuturi · 2017-03-22T08:03:34Z

@ustcweizhou can you also review this fix? Is pr #2011 still required?

yadvr · 2017-03-22T09:48:56Z

LGTM, does this affect 4.9 as well @swill ? /cc @borisstoyanov

borisstoyanov · 2017-03-22T09:56:38Z

@rhtyd I think this was introduced with the StrongSwan implementation which was 4.10 I think.

ustcweizhou · 2017-03-22T09:59:29Z

@karuturi to be honest, I think PR#2011 fixes the issue, not this one.

swill · 2017-03-22T12:47:27Z

I think #2011 fixes the observed issue, but this PR improves the IP ordering fix originally added to the strongswan pr. This implementation ensures (again) that duplicate public ips will not be saved to the data bag while ensuring the source nat ip is primary on its nic.

In short, I think both PRs are worth merging.

borisstoyanov · 2017-03-22T12:55:36Z

@karuturi we have 3 LGTMs, I think we're good to merge this.
tag:mergeready

karuturi · 2017-03-23T09:45:30Z

ok. Thanks everyone. I am merging this.

CLOUDSTACK-9811: fixed an issue if the dev is not in the databagDefend against the specified dev not being in the databag. * pr/2003: changed the order fix to be closer to the original code CLOUDSTACK-9811: fixed an issue if the dev is not in the databag Signed-off-by: Rajani Karuturi <rajani.karuturi@accelerite.com>

borisstoyanov reviewed Mar 14, 2017
View reviewed changes

swill changed the title ~~fixed an issue if the dev is not in the databag~~CLOUDSTACK-9811: fixed an issue if the dev is not in the databagMar 14, 2017

CLOUDSTACK-9811: fixed an issue if the dev is not in the databag
8d4855b

swill force-pushed the cs_ip_fix branch from 6cde7cd to 8d4855bCompare March 14, 2017 13:24

changed the order fix to be closer to the original code
bb40877

borisstoyanov approved these changes Mar 20, 2017
View reviewed changes

DaanHoogland mentioned this pull request Mar 22, 2017
CLOUDSTACK-9811: fix duplicated nics on VR caused by nic name p<slot_number>p<port_number> #2011
Merged

asfgit merged commit bb40877 into apache:masterMar 23, 2017

Uh oh!

Conversation

swill commented Mar 13, 2017

Uh oh!

borisstoyanov commented Mar 13, 2017

Uh oh!

blueorangutan commented Mar 13, 2017

Uh oh!

blueorangutan commented Mar 13, 2017

Uh oh!

borisstoyanov commented Mar 13, 2017

Uh oh!

blueorangutan commented Mar 13, 2017

Uh oh!

karuturi commented Mar 14, 2017

Uh oh!

blueorangutan commented Mar 14, 2017

Uh oh!

ustcweizhou commented Mar 14, 2017

Uh oh!

borisstoyanov left a comment

Choose a reason for hiding this comment

Uh oh!

swill commented Mar 14, 2017

Uh oh!

ustcweizhou commented Mar 14, 2017

Uh oh!

swill commented Mar 14, 2017

Uh oh!

swill commented Mar 14, 2017

Uh oh!

borisstoyanov commented Mar 14, 2017

Uh oh!

ustcweizhou commented Mar 14, 2017

Uh oh!

swill commented Mar 14, 2017

Uh oh!

borisstoyanov commented Mar 14, 2017

Uh oh!

borisstoyanov commented Mar 14, 2017

Uh oh!

swill commented Mar 14, 2017

Uh oh!

swill commented Mar 14, 2017

Uh oh!

swill commented Mar 14, 2017

Uh oh!

ustcweizhou commented Mar 14, 2017

Uh oh!

borisstoyanov commented Mar 14, 2017

Uh oh!

ustcweizhou commented Mar 14, 2017

Uh oh!

ustcweizhou commented Mar 14, 2017

Uh oh!

swill commented Mar 14, 2017

Uh oh!

ustcweizhou commented Mar 15, 2017

Uh oh!

swill commented Mar 16, 2017

Uh oh!

borisstoyanov commented Mar 17, 2017

Uh oh!

ustcweizhou commented Mar 17, 2017

Uh oh!

remibergsma commented Mar 17, 2017

Uh oh!

swill commented Mar 17, 2017

Uh oh!

swill commented Mar 17, 2017

Uh oh!

borisstoyanov commented Mar 17, 2017

Uh oh!

blueorangutan commented Mar 17, 2017

Uh oh!

blueorangutan commented Mar 17, 2017

Uh oh!

borisstoyanov commented Mar 17, 2017

Uh oh!

blueorangutan commented Mar 17, 2017

swill commented Mar 22, 2017•
edited
Loading