Fix to enable Advanced zones with Security groups and VXLAN isolation type by nvazquez · Pull Request #2693 · apache/cloudstack

nvazquez · 2018-06-06T01:24:43Z

Description

Not possible to deploy an Advanced zone with Security Groups, and VXLAN isolation method on KVM.
Exception: "Unable to convert network offering with specified id to network profile" is logged.

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)

GitHub Issue/PRs

Screenshots (if appropriate):

How Has This Been Tested?

Tested on KVM. Deploy an Advanced zone, enabling Security Groups, VXLAN isolation for Guest traffic

Checklist:

I have read the CONTRIBUTING document.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
Testing
I have added tests to cover my changes.
All relevant new and existing integration tests have passed.
A full integration testsuite with all test that can run on my environment has passed.

Fixes: #2701

… type

nvazquez · 2018-06-06T01:25:21Z

@blueorangutan package

blueorangutan · 2018-06-06T01:25:43Z

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

blueorangutan · 2018-06-06T01:55:31Z

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

nvazquez · 2018-06-06T03:15:20Z

@blueorangutan test

blueorangutan · 2018-06-06T03:15:43Z

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

yadvr

Lgtm, just drop the My in the method name or simplify the method name

DaanHoogland

looks good. very specific. is it only for vxlan for instance?

to quick

DaanHoogland · 2018-06-06T07:18:36Z

@rhtyd @nvazquez shouldn't vxlan be added to the list of _isolationMethods in the constructor instead?

blueorangutan · 2018-06-06T11:41:08Z

Trillian test result (tid-2725)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 28512 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2693-t2725-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_public_ip_range.py
Intermitten failure detected: /marvin/tests/smoke/test_templates.py
Intermitten failure detected: /marvin/tests/smoke/test_usage.py
Intermitten failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermitten failure detected: /marvin/tests/smoke/test_volumes.py
Intermitten failure detected: /marvin/tests/smoke/test_host_maintenance.py
Smoke tests completed. 60 look OK, 7 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestDeployVirtioSCSIVM>:setup	`Error`	0.00	test_deploy_virtio_scsi_vm.py
test_03_vpc_privategw_restart_vpc_cleanup	`Error`	119.74	test_privategw_acl.py
test_04_extract_template	`Failure`	128.38	test_templates.py
ContextSuite context=TestISOUsage>:setup	`Error`	0.00	test_usage.py
test_01_secured_vm_migration	`Error`	1.15	test_vm_life_cycle.py
test_02_not_secured_vm_migration	`Error`	1.17	test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration	`Error`	1.16	test_vm_life_cycle.py
test_04_nonsecured_to_secured_vm_migration	`Error`	1.15	test_vm_life_cycle.py
test_08_migrate_vm	`Error`	5.46	test_vm_life_cycle.py
test_06_download_detached_volume	`Failure`	138.18	test_volumes.py
test_01_cancel_host_maintenace_with_no_migration_jobs	`Failure`	0.15	test_host_maintenance.py
test_02_cancel_host_maintenace_with_migration_jobs	`Error`	31.13	test_host_maintenance.py

yadvr

LGTM, let's keep the isMy... method as a similar method already exists.

yadvr · 2018-06-07T05:08:58Z

@blueorangutan package

blueorangutan · 2018-06-07T05:09:43Z

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

yadvr · 2018-06-07T05:51:24Z

@DaanHoogland @PaulAngus have you tested this and are you LGTM on it?

DaanHoogland · 2018-06-07T05:54:34Z

See my question above @rhtyd

yadvr · 2018-06-07T05:59:27Z

I think that's a question for @nvazquez to reply.

blueorangutan · 2018-06-07T06:30:47Z

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

yadvr · 2018-06-07T07:33:37Z

@blueorangutan package

blueorangutan · 2018-06-07T07:34:45Z

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

PaulAngus · 2018-06-07T08:09:58Z

it's very specific to using vxlan as the isolation method in advanced zones. i'm mid-test but i'm tied up in meetings this morning #overshare.

blueorangutan · 2018-06-07T08:11:02Z

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

yadvr · 2018-06-07T08:55:48Z

@blueorangutan test

blueorangutan · 2018-06-07T08:56:43Z

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

nvazquez · 2018-06-07T10:56:31Z

Sorry @DaanHoogland and @rhtyd I missed out your questions.
@rhtyd no problem, I can rename the method's name
@DaanHoogland good point, I didn't add it to '_isolationMethods' as I assumed this was only allowed for VXLAN + security groups only (in advanced zone). Adding it will simplify the implementation but I'm not sure if it would be correct

PaulAngus · 2018-06-07T14:33:19Z

LGTM - This fixes reported bug, that zone creation fails - tested manually creating zone through UI.

subsequent bug found relating to cloudstack ignoring requested isolation type when deploying VM, will file separately.

nvazquez · 2018-06-07T17:30:24Z

Thanks @PaulAngus @rhtyd @DaanHoogland, we have 2 LGTM on this one, depending on test results I think we are good to merge this one if no objections

blueorangutan · 2018-06-07T18:19:17Z

Trillian test result (tid-2746)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31997 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2693-t2746-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_vm_life_cycle.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 63 look OK, 4 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestDeployVirtioSCSIVM>:setup	`Error`	0.00	test_deploy_virtio_scsi_vm.py
test_03_vpc_privategw_restart_vpc_cleanup	`Error`	167.78	test_privategw_acl.py
test_04_rvpc_network_garbage_collector_nics	`Failure`	298.66	test_vpc_redundant.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	3.42	test_hostha_kvm.py

yadvr · 2018-06-08T07:42:53Z

Merging this based on tests and 2+ LGTMs (Paul and Daan have commented LGTMs than use Github review/approval feature).

Fix to enable Advanced zones with Security groups and VXLAN isolation…

c4a62a9

… type

nvazquez added this to the 4.11.1.0 milestone Jun 6, 2018

nvazquez requested review from DaanHoogland and yadvr June 6, 2018 01:32

yadvr approved these changes Jun 6, 2018

View reviewed changes

DaanHoogland previously approved these changes Jun 6, 2018

View reviewed changes

yadvr approved these changes Jun 7, 2018

View reviewed changes

yadvr merged commit a5856a6 into apache:4.11 Jun 8, 2018

yadvr mentioned this pull request Aug 14, 2018

VXLAN as isolation method when using Advanced networks with security groups is ignored when deploying VMs #2701

Closed

nvazquez deleted the vxlansecuritygroups branch April 6, 2020 14:47

Conversation

nvazquez commented Jun 6, 2018 • edited by DaanHoogland Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

GitHub Issue/PRs

Screenshots (if appropriate):

How Has This Been Tested?

Checklist:

Uh oh!

nvazquez commented Jun 6, 2018

Uh oh!

blueorangutan commented Jun 6, 2018

Uh oh!

blueorangutan commented Jun 6, 2018

Uh oh!

nvazquez commented Jun 6, 2018

Uh oh!

blueorangutan commented Jun 6, 2018

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Jun 6, 2018

Uh oh!

blueorangutan commented Jun 6, 2018

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented Jun 7, 2018

Uh oh!

blueorangutan commented Jun 7, 2018

Uh oh!

yadvr commented Jun 7, 2018

Uh oh!

DaanHoogland commented Jun 7, 2018

Uh oh!

yadvr commented Jun 7, 2018

Uh oh!

blueorangutan commented Jun 7, 2018

Uh oh!

yadvr commented Jun 7, 2018

Uh oh!

blueorangutan commented Jun 7, 2018

Uh oh!

PaulAngus commented Jun 7, 2018

Uh oh!

blueorangutan commented Jun 7, 2018

Uh oh!

yadvr commented Jun 7, 2018

Uh oh!

blueorangutan commented Jun 7, 2018

Uh oh!

nvazquez commented Jun 7, 2018

Uh oh!

PaulAngus commented Jun 7, 2018

Uh oh!

nvazquez commented Jun 7, 2018

Uh oh!

blueorangutan commented Jun 7, 2018

Uh oh!

yadvr commented Jun 8, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nvazquez commented Jun 6, 2018 •

edited by DaanHoogland

Loading