Skip to content

CLOUDSTACK-9811: fix duplicated nics on VR caused by nic name p<slot_number>p<port_number> - #2011

Merged
asfgit merged 1 commit into
apache:4.9from
ustcweizhou:fix-issue-p55p1
Mar 23, 2017
Merged

CLOUDSTACK-9811: fix duplicated nics on VR caused by nic name p<slot_number>p<port_number>#2011
asfgit merged 1 commit into
apache:4.9from
ustcweizhou:fix-issue-p55p1

Conversation

@ustcweizhou

@ustcweizhouustcweizhou commented Mar 20, 2017

Copy link
Copy Markdown
Contributor

No description provided.

@ustcweizhou
ustcweizhou changed the base branch from master to 4.9March 20, 2017 06:32
@borisstoyanov

borisstoyanov commented Mar 20, 2017

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

blueorangutan commented Mar 20, 2017

Copy link
Copy Markdown

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

@borisstoyanov

borisstoyanov commented Mar 20, 2017

Copy link
Copy Markdown
Contributor

Hi @ustcweizhou, out of curiosity why 4.9 instead of master?

@ustcweizhou

ustcweizhou commented Mar 20, 2017

Copy link
Copy Markdown
ContributorAuthor

@borisstoyanov first 4.9, then merge into master.
this patch can also be applied to master.

wget --no-check-certificate https://github.com/apache/cloudstack/pull/2011.patch
git am 2011.patch (or patch -p1 <2011.patch)

@blueorangutan

blueorangutan commented Mar 20, 2017

Copy link
Copy Markdown

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-596

@borisstoyanov

borisstoyanov commented Mar 20, 2017

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

blueorangutan commented Mar 20, 2017

Copy link
Copy Markdown

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

@blueorangutan

blueorangutan commented Mar 20, 2017

Copy link
Copy Markdown

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

@borisstoyanov

borisstoyanov commented Mar 20, 2017

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

blueorangutan commented Mar 20, 2017

Copy link
Copy Markdown

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

@karuturikaruturi mentioned this pull request Mar 22, 2017
Merged
@borisstoyanov

borisstoyanov commented Mar 22, 2017

Copy link
Copy Markdown
Contributor

Looks like tests were aborted, will restart now
@blueorangutan test

@blueorangutan

blueorangutan commented Mar 22, 2017

Copy link
Copy Markdown

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

borisstoyanov
borisstoyanov approved these changes Mar 22, 2017
View reviewed changes

@borisstoyanovborisstoyanov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM based on code review

@yadvr

yadvr commented Mar 22, 2017

Copy link
Copy Markdown
Member

LGTM. Can we check travis failures?

@DaanHoogland

DaanHoogland commented Mar 22, 2017•
edited
Loading

Copy link
Copy Markdown
Contributor

@ustcweizhou this enables looking for a nic by a name like a1b2c3 (\D+\d+\D*\d*\D*\d*) it will also allow all shorter as long as they start with which makes sense. But to satisfy @karuturi's question in #2003, could you briefly describe how this prevents extra nics from being created? otherwise LGTM

@blueorangutan

blueorangutan commented Mar 22, 2017

Copy link
Copy Markdown

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

TestResultTime (s)Test File
test_04_rvpc_privategw_static_routesFailure359.28test_privategw_acl.py
test_02_list_snapshots_with_removed_data_storeError0.03test_snapshots.py
test_01_vpc_site2site_vpnSuccess164.78test_vpc_vpn.py
test_01_vpc_remote_access_vpnSuccess66.08test_vpc_vpn.py
test_01_redundant_vpc_site2site_vpnSuccess255.18test_vpc_vpn.py
test_02_VPC_default_routesSuccess265.84test_vpc_router_nics.py
test_01_VPC_nics_after_destroySuccess527.08test_vpc_router_nics.py
test_05_rvpc_multi_tiersSuccess504.56test_vpc_redundant.py
test_04_rvpc_network_garbage_collector_nicsSuccess1403.26test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routersSuccess557.21test_vpc_redundant.py
test_02_redundant_VPC_default_routesSuccess752.65test_vpc_redundant.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACLSuccess1300.22test_vpc_redundant.py
test_09_delete_detached_volumeSuccess156.34test_volumes.py
test_08_resize_volumeSuccess156.52test_volumes.py
test_07_resize_failSuccess161.15test_volumes.py
test_06_download_detached_volumeSuccess156.03test_volumes.py
test_05_detach_volumeSuccess155.78test_volumes.py
test_04_delete_attached_volumeSuccess150.94test_volumes.py
test_03_download_attached_volumeSuccess156.59test_volumes.py
test_02_attach_volumeSuccess89.67test_volumes.py
test_01_create_volumeSuccess711.12test_volumes.py
test_deploy_vm_multipleSuccess277.07test_vm_life_cycle.py
test_deploy_vmSuccess0.02test_vm_life_cycle.py
test_advZoneVirtualRouterSuccess0.02test_vm_life_cycle.py
test_10_attachAndDetach_isoSuccess26.52test_vm_life_cycle.py
test_09_expunge_vmSuccess125.20test_vm_life_cycle.py
test_08_migrate_vmSuccess40.67test_vm_life_cycle.py
test_07_restore_vmSuccess0.07test_vm_life_cycle.py
test_06_destroy_vmSuccess125.63test_vm_life_cycle.py
test_03_reboot_vmSuccess125.67test_vm_life_cycle.py
test_02_start_vmSuccess10.12test_vm_life_cycle.py
test_01_stop_vmSuccess40.25test_vm_life_cycle.py
test_CreateTemplateWithDuplicateNameSuccess50.40test_templates.py
test_08_list_system_templatesSuccess0.02test_templates.py
test_07_list_public_templatesSuccess0.02test_templates.py
test_05_template_permissionsSuccess0.04test_templates.py
test_04_extract_templateSuccess5.12test_templates.py
test_03_delete_templateSuccess5.08test_templates.py
test_02_edit_templateSuccess90.17test_templates.py
test_01_create_templateSuccess50.36test_templates.py
test_10_destroy_cpvmSuccess161.58test_ssvm.py
test_09_destroy_ssvmSuccess163.45test_ssvm.py
test_08_reboot_cpvmSuccess131.54test_ssvm.py
test_07_reboot_ssvmSuccess163.49test_ssvm.py
test_06_stop_cpvmSuccess131.59test_ssvm.py
test_05_stop_ssvmSuccess138.58test_ssvm.py
test_04_cpvm_internalsSuccess1.26test_ssvm.py
test_03_ssvm_internalsSuccess3.83test_ssvm.py
test_02_list_cpvm_vmSuccess0.09test_ssvm.py
test_01_list_sec_storage_vmSuccess0.10test_ssvm.py
test_01_snapshot_root_diskSuccess10.95test_snapshots.py
test_04_change_offering_smallSuccess239.49test_service_offerings.py
test_03_delete_service_offeringSuccess0.03test_service_offerings.py
test_02_edit_service_offeringSuccess0.04test_service_offerings.py
test_01_create_service_offeringSuccess0.08test_service_offerings.py
test_02_sys_template_readySuccess0.09test_secondary_storage.py
test_01_sys_vm_startSuccess0.13test_secondary_storage.py
test_09_reboot_routerSuccess40.25test_routers.py
test_08_start_routerSuccess30.21test_routers.py
test_07_stop_routerSuccess10.12test_routers.py
test_06_router_advancedSuccess0.04test_routers.py
test_05_router_basicSuccess0.03test_routers.py
test_04_restart_network_wo_cleanupSuccess5.64test_routers.py
test_03_restart_network_cleanupSuccess60.48test_routers.py
test_02_router_internal_advSuccess1.05test_routers.py
test_01_router_internal_basicSuccess0.57test_routers.py
test_router_dns_guestipquerySuccess76.66test_router_dns.py
test_router_dns_externalipquerySuccess0.08test_router_dns.py
test_router_dhcphostsSuccess276.39test_router_dhcphosts.py
test_router_dhcp_optsSuccess21.49test_router_dhcphosts.py
test_01_updatevolumedetailSuccess0.06test_resource_detail.py
test_01_reset_vm_on_rebootSuccess130.70test_reset_vm_on_reboot.py
test_createRegionSuccess0.03test_regions.py
test_create_pvlan_networkSuccess5.16test_pvlan.py
test_dedicatePublicIpRangeSuccess0.31test_public_ip_range.py
test_03_vpc_privategw_restart_vpc_cleanupSuccess518.33test_privategw_acl.py
test_02_vpc_privategw_static_routesSuccess363.47test_privategw_acl.py
test_01_vpc_privategw_aclSuccess91.55test_privategw_acl.py
test_01_primary_storage_nfsSuccess35.77test_primary_storage.py
test_createPortablePublicIPRangeSuccess15.14test_portable_publicip.py
test_createPortablePublicIPAcquireSuccess15.31test_portable_publicip.py
test_isolate_network_password_serverSuccess89.27test_password_server.py
test_UpdateStorageOverProvisioningFactorSuccess0.09test_over_provisioning.py
test_oobm_zchange_passwordSuccess30.48test_outofbandmanagement.py
test_oobm_multiple_mgmt_server_ownershipSuccess16.24test_outofbandmanagement.py
test_oobm_issue_power_statusSuccess10.18test_outofbandmanagement.py
test_oobm_issue_power_softSuccess10.23test_outofbandmanagement.py
test_oobm_issue_power_resetSuccess15.24test_outofbandmanagement.py
test_oobm_issue_power_onSuccess15.28test_outofbandmanagement.py
test_oobm_issue_power_offSuccess10.23test_outofbandmanagement.py
test_oobm_issue_power_cycleSuccess15.23test_outofbandmanagement.py
test_oobm_enabledisable_across_clusterzonesSuccess67.09test_outofbandmanagement.py
test_oobm_enable_feature_validSuccess5.12test_outofbandmanagement.py
test_oobm_enable_feature_invalidSuccess0.07test_outofbandmanagement.py
test_oobm_disable_feature_validSuccess5.14test_outofbandmanagement.py
test_oobm_disable_feature_invalidSuccess0.07test_outofbandmanagement.py
test_oobm_configure_invalid_driverSuccess0.06test_outofbandmanagement.py
test_oobm_configure_default_driverSuccess0.06test_outofbandmanagement.py
test_oobm_background_powerstate_syncSuccess23.30test_outofbandmanagement.py
test_extendPhysicalNetworkVlanSuccess15.23test_non_contigiousvlan.py
test_01_nicSuccess438.49test_nic.py
test_releaseIPSuccess247.24test_network.py
test_reboot_routerSuccess412.92test_network.py
test_public_ip_user_accountSuccess10.19test_network.py
test_public_ip_admin_accountSuccess40.22test_network.py
test_network_rules_acquired_public_ip_3_Load_Balancer_RuleSuccess66.62test_network.py
test_network_rules_acquired_public_ip_2_nat_ruleSuccess61.57test_network.py
test_network_rules_acquired_public_ip_1_static_nat_ruleSuccess124.05test_network.py
test_delete_accountSuccess287.12test_network.py
test_02_port_fwd_on_non_src_natSuccess55.49test_network.py
test_01_port_fwd_on_src_natSuccess111.56test_network.py
test_nic_secondaryip_add_removeSuccess217.07test_multipleips_per_nic.py
login_test_saml_userSuccess17.75test_login.py
test_assign_and_removal_lbSuccess133.35test_loadbalance.py
test_02_create_lb_rule_non_natSuccess187.26test_loadbalance.py
test_01_create_lb_rule_src_natSuccess217.72test_loadbalance.py
test_03_list_snapshotsSuccess0.04test_list_ids_parameter.py
test_02_list_templatesSuccess0.03test_list_ids_parameter.py
test_01_list_volumesSuccess0.02test_list_ids_parameter.py
test_07_list_default_isoSuccess0.04test_iso.py
test_05_iso_permissionsSuccess0.04test_iso.py
test_04_extract_IsoSuccess5.15test_iso.py
test_03_delete_isoSuccess95.17test_iso.py
test_02_edit_isoSuccess0.04test_iso.py
test_01_create_isoSuccess20.68test_iso.py
test_04_rvpc_internallb_haproxy_stats_on_all_interfacesSuccess207.27test_internal_lb.py
test_03_vpc_internallb_haproxy_stats_on_all_interfacesSuccess147.77test_internal_lb.py
test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80Success519.07test_internal_lb.py
test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80Success428.65test_internal_lb.py
test_dedicateGuestVlanRangeSuccess10.20test_guest_vlan_range.py
test_UpdateConfigParamWithScopeSuccess0.10test_global_settings.py
test_rolepermission_lifecycle_updateSuccess5.79test_dynamicroles.py
test_rolepermission_lifecycle_listSuccess5.67test_dynamicroles.py
test_rolepermission_lifecycle_deleteSuccess5.57test_dynamicroles.py
test_rolepermission_lifecycle_createSuccess5.59test_dynamicroles.py
test_rolepermission_lifecycle_concurrent_updatesSuccess5.66test_dynamicroles.py
test_role_lifecycle_update_role_inuseSuccess5.60test_dynamicroles.py
test_role_lifecycle_updateSuccess10.64test_dynamicroles.py
test_role_lifecycle_listSuccess5.59test_dynamicroles.py
test_role_lifecycle_deleteSuccess10.62test_dynamicroles.py
test_role_lifecycle_createSuccess5.59test_dynamicroles.py
test_role_inuse_deletionSuccess5.58test_dynamicroles.py
test_role_account_acls_multiple_mgmt_serversSuccess6.86test_dynamicroles.py
test_role_account_aclsSuccess7.01test_dynamicroles.py
test_default_role_deletionSuccess5.66test_dynamicroles.py
test_04_create_fat_type_disk_offeringSuccess0.05test_disk_offerings.py
test_03_delete_disk_offeringSuccess0.03test_disk_offerings.py
test_02_edit_disk_offeringSuccess0.04test_disk_offerings.py
test_02_create_sparse_type_disk_offeringSuccess0.05test_disk_offerings.py
test_01_create_disk_offeringSuccess0.08test_disk_offerings.py
test_deployvm_userdispersingSuccess20.38test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_userconcentratedSuccess35.47test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_firstfitSuccess55.45test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_userdata_postSuccess10.31test_deploy_vm_with_userdata.py
test_deployvm_userdataSuccess55.55test_deploy_vm_with_userdata.py
test_02_deploy_vm_root_resizeSuccess5.64test_deploy_vm_root_resize.py
test_01_deploy_vm_root_resizeSuccess5.67test_deploy_vm_root_resize.py
test_00_deploy_vm_root_resizeSuccess206.78test_deploy_vm_root_resize.py
test_deploy_vm_from_isoSuccess207.26test_deploy_vm_iso.py
test_DeployVmAntiAffinityGroupSuccess60.62test_affinity_groups.py
test_03_delete_vm_snapshotsSkipped0.00test_vm_snapshots.py
test_02_revert_vm_snapshotsSkipped0.00test_vm_snapshots.py
test_01_test_vm_volume_snapshotSkipped0.00test_vm_snapshots.py
test_01_create_vm_snapshotsSkipped0.00test_vm_snapshots.py
test_06_copy_templateSkipped0.00test_templates.py
test_static_role_account_aclsSkipped0.02test_staticroles.py
test_01_scale_vmSkipped0.00test_scale_vm.py
test_01_primary_storage_iscsiSkipped0.03test_primary_storage.py
test_06_copy_isoSkipped0.00test_iso.py
test_deploy_vgpu_enabled_vmSkipped0.01test_deploy_vgpu_enabled_vm.py

@borisstoyanov

borisstoyanov commented Mar 22, 2017

Copy link
Copy Markdown
Contributor

thank you guys, tests are looking good as well.
I think we have enough LGTM to merge this now
cc: @karuturi
tag:mergeready

@ustcweizhou

ustcweizhou commented Mar 22, 2017

Copy link
Copy Markdown
ContributorAuthor

@DaanHoogland@karuturi@borisstoyanov

Here is a short explanation why the issue happened:
Normally getBroadcastUriFromBridge method in KVM plugin will get the URI from a bridge name. for example, breth1-100 -> vlan://100, brem1-200 -> vlan://200.
However, for the bridge like brp55p1-100, it will return vlan://1 which is obviously wrong.

The method getBroadcastUriFromBridge is used in prepareNetworkElementCommand for command IpAssocCommand in VR. Each time when a new ip is associated, it will check the existing nics by getting the URIs from bridge name of nics. If the URIs do not contain the uri of te new ip, then a new nic will be plugged and new ip will be associated to the new nic.

The issue happened when cloudstack try to set more information of eth2 (public nic) of a VR, as for the error described in part 1 , the URI of eth2 return wrong result (vlan://1) which is different from uri of ip (it is vlan://7 from StartCommand), so eth3 is added.

@DaanHoogland

DaanHoogland commented Mar 23, 2017

Copy link
Copy Markdown
Contributor

thanks @ustcweizhou
@karuturi can we merge this? and merge forward?

@karuturi

karuturi commented Mar 23, 2017

Copy link
Copy Markdown
Member

ok. Thanks everyone. I am merging this.

asfgit pushed a commit that referenced this pull request Mar 23, 2017
@karuturi
Merge pull request#2011from ustcweizhou/fix-issue-p55p1
9bf4281
CLOUDSTACK-9811: fix duplicated nics on VR caused by nic name p<slot_number>p<port_number>
* pr/2011:
CLOUDSTACK-9811: fix duplicated nics on VR caused by nic name p<slot_number>p<port_number>
Signed-off-by: Rajani Karuturi <rajani.karuturi@accelerite.com>
@asfgit
asfgit merged commit bf93b63 into apache:4.9Mar 23, 2017
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@borisstoyanovborisstoyanovborisstoyanov approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@ustcweizhou@borisstoyanov@blueorangutan@yadvr@DaanHoogland@karuturi@asfgit