Added support for removing unused port groups on VMWare by Spaceman1984 · Pull Request #4701 · apache/cloudstack

Spaceman1984 · 2021-02-18T10:34:55Z

Description

This PR adds a global setting (vmware.cleanup.port.groups) to toggle removing unused port groups from VMware hypervisor hosts.

Fixes: #3779

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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

This has been manually tested, with the below steps:

Set global setting (vmware.cleanup.port.groups) to true / false.
Create an instance on a new network.
Check network config for hosts in vSphere client for the newly created port group.
Destroy and expunge the instance.
Destroy the router.
Check network config for hosts in vSphere client if the port group has been removed or not, depending on global setting (vmware.cleanup.port.groups)

Spaceman1984 · 2021-02-18T10:35:27Z

@blueorangutan package

blueorangutan · 2021-02-18T10:36:40Z

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

blueorangutan · 2021-02-18T11:27:27Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2735

Spaceman1984 · 2021-02-18T11:48:37Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2021-02-18T11:49:46Z

@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests

blueorangutan · 2021-02-18T23:05:55Z

Trillian test result (tid-3570)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38865 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4701-t3570-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Smoke tests completed. 85 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_03_deploy_and_upgrade_kubernetes_cluster	`Failure`	776.06	test_kubernetes_clusters.py

Spaceman1984 · 2021-03-11T09:26:44Z

@blueorangutan package

blueorangutan · 2021-03-11T09:27:03Z

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

blueorangutan · 2021-03-11T09:48:25Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 88

DaanHoogland · 2021-03-12T08:21:39Z

code looks good @Spaceman1984 , but i don't see how it is being called. Am I missing something?

sureshanaparti · 2021-03-17T07:41:13Z

            "Vmware script timeout for ova packaging process", true, ConfigKey.Scope.Global, 1000);

+    static final ConfigKey<Boolean> s_vmwareCleanupPortGroups = new ConfigKey<Boolean>("Advanced", Boolean.class, "vmware.cleanup.port.groups",  "false",
+            "Remove unused port groups. WARNING: When set to true, native VMware HA might not work.", true, ConfigKey.Scope.Global);


@Spaceman1984 If my understanding is correct, cleanup port groups happens only on the network cleanup only and the old uncleaned port groups of other networks (when this setting is false) still remains. If so, Can you mention in the setting description that port groups cleanup happens on the network cleanup for that network only.

Is it possible to clean old port groups of other networks through any garbage collector, when this setting is true.

To my knowledge, this setting is not used through garbage collection, it is checked when an instance is destroyed. I'm not sure if there is some kind of garbage collection that would destroy a VR, if there is, then I suppose this will also apply.

nvazquez · 2021-06-23T15:13:23Z

Hi @Spaceman1984 there are some open questions on this PR, can you please advise?

Spaceman1984 · 2021-06-24T06:03:18Z

Hi @Spaceman1984 there are some open questions on this PR, can you please advise?

Sure, @nvazquez

Spaceman1984 · 2021-06-24T06:05:48Z

code looks good @Spaceman1984 , but i don't see how it is being called. Am I missing something?

This setting is checked when an instance is destroyed @DaanHoogland. If the instance being destroyed is the last instance (including VR) using a specific network, the port group will be removed.

DaanHoogland

clgtm

nvazquez · 2021-07-26T16:30:56Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2021-07-26T16:32:06Z

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

blueorangutan · 2021-07-27T03:09:11Z

Trillian test result (tid-1380)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36550 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4701-t1380-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_pvlan.py
Smoke tests completed. 88 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_create_pvlan_network	`Error`	0.03	test_pvlan.py

sureshanaparti · 2021-07-30T12:36:15Z

@Spaceman1984 Tested this manually with multiple vSphere hosts (and Standard vSwitches), I noticed the port groups are removed only on one host where the last destroyed VM (usually VR) resides. Make sure the unused port groups are removed from all the hosts. Also, verify this with Distributed vSwitch. Thanks.

yadvr · 2021-08-09T08:45:07Z

@sureshanaparti @andrijapanicsb @borisstoyanov @vladimirpetrov @nvazquez are we lgtm to merge this?

nvazquez · 2021-08-10T05:27:08Z

@Spaceman1984 could you check @sureshanaparti's last comment?

sureshanaparti · 2021-08-10T11:02:49Z

@sureshanaparti @andrijapanicsb @borisstoyanov @vladimirpetrov @nvazquez are we lgtm to merge this?

no @rhtyd

…cription, changed if statement.

sureshanaparti · 2021-08-24T13:45:03Z

@blueorangutan package

blueorangutan · 2021-08-24T13:46:02Z

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

sureshanaparti · 2021-08-24T13:47:55Z

@Spaceman1984 Tested this manually with multiple vSphere hosts (and Standard vSwitches), I noticed the port groups are removed only on one host where the last destroyed VM (usually VR) resides. Make sure the unused port groups are removed from all the hosts. Also, verify this with Distributed vSwitch. Thanks.

Addressed the changes for cleaning up of the unused port groups on all the hosts. Tested this manually with multiple vSphere hosts, network configured through standard vSwitches on them.

blueorangutan · 2021-08-24T14:12:08Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 997

sureshanaparti · 2021-08-24T14:14:31Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2021-08-24T14:15:03Z

@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests

blueorangutan · 2021-08-25T08:24:02Z

Trillian test result (tid-1763)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 64141 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4701-t1763-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 87 look OK, 2 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80	`Failure`	1074.85	test_internal_lb.py
test_09_expunge_vm	`Failure`	423.54	test_vm_life_cycle.py

DaanHoogland · 2021-08-27T08:21:51Z

@Spaceman1984, can you look at the conflicts here

nvazquez · 2021-08-27T17:21:16Z

Conflicts fixed

@blueorangutan package

blueorangutan · 2021-08-27T17:22:02Z

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

blueorangutan · 2021-08-27T17:48:25Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1044

nvazquez · 2021-08-27T18:15:16Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2021-08-27T18:16:02Z

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

blueorangutan · 2021-08-28T06:26:19Z

Trillian test result (tid-1817)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37999 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4701-t1817-vmware-67u3.zip
Smoke tests completed. 89 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

nvazquez

LGTM

Spaceman1984 requested review from harikrishna-patnala and yadvr February 24, 2021 06:41

Spaceman1984 marked this pull request as ready for review February 24, 2021 06:56

GutoVeronezi reviewed Mar 9, 2021

View reviewed changes

Comment thread ...ns/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java Outdated

GutoVeronezi reviewed Mar 9, 2021

View reviewed changes

Comment thread ...ns/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java Outdated

GabrielBrascher reviewed Mar 9, 2021

View reviewed changes

Comment thread ...ns/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java Outdated

Comment thread ...ns/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java Outdated

sureshanaparti reviewed Mar 10, 2021

View reviewed changes

Comment thread plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManager.java Outdated

sureshanaparti reviewed Mar 10, 2021

View reviewed changes

Comment thread ...ns/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

DaanHoogland added Severity:Major type:bug type:cleanup type:enhancement labels Mar 12, 2021

DaanHoogland added this to the 4.16.0.0 milestone Mar 12, 2021

sureshanaparti reviewed Mar 17, 2021

View reviewed changes

DaanHoogland requested changes Jun 24, 2021

View reviewed changes

Comment thread ...ns/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java Outdated

DaanHoogland approved these changes Jun 25, 2021

View reviewed changes

sureshanaparti requested a review from nvazquez June 27, 2021 16:29

Spaceman1984 and others added 4 commits August 24, 2021 19:05

Added support for removing unused port groups on VMWare

a8af41c

Fixed error handling around unavailable portgroup name

b50a350

Review changes, defaulting glovbal var to false, added warning to des…

69fed99

…cription, changed if statement.

Cleanup unused network port groups on all the hosts.

e0a3f40

sureshanaparti force-pushed the vmware-cleanup-port-groups branch from 5cc8fb6 to e0a3f40 Compare August 24, 2021 13:44

sureshanaparti approved these changes Aug 24, 2021

View reviewed changes

Merge branch 'main' into vmware-cleanup-port-groups

8908091

nvazquez approved these changes Aug 28, 2021

View reviewed changes

nvazquez merged commit 1d3083d into apache:main Aug 28, 2021

Conversation

Spaceman1984 commented Feb 18, 2021 • edited by sureshanaparti Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

Spaceman1984 commented Feb 18, 2021

Uh oh!

blueorangutan commented Feb 18, 2021

Uh oh!

blueorangutan commented Feb 18, 2021

Uh oh!

Spaceman1984 commented Feb 18, 2021

Uh oh!

blueorangutan commented Feb 18, 2021

Uh oh!

blueorangutan commented Feb 18, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Spaceman1984 commented Mar 11, 2021

Uh oh!

blueorangutan commented Mar 11, 2021

Uh oh!

blueorangutan commented Mar 11, 2021

Uh oh!

DaanHoogland commented Mar 12, 2021

Uh oh!

sureshanaparti Mar 17, 2021 • 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!

Spaceman1984 Jun 24, 2021 • 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!

nvazquez commented Jun 23, 2021

Uh oh!

Spaceman1984 commented Jun 24, 2021

Uh oh!

Spaceman1984 commented Jun 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Jul 26, 2021

Uh oh!

blueorangutan commented Jul 26, 2021

Uh oh!

blueorangutan commented Jul 27, 2021

Uh oh!

sureshanaparti commented Jul 30, 2021

Uh oh!

yadvr commented Aug 9, 2021

Uh oh!

nvazquez commented Aug 10, 2021

Uh oh!

sureshanaparti commented Aug 10, 2021

Uh oh!

sureshanaparti commented Aug 24, 2021

Uh oh!

blueorangutan commented Aug 24, 2021

Uh oh!

sureshanaparti commented Aug 24, 2021

Uh oh!

blueorangutan commented Aug 24, 2021

Uh oh!

sureshanaparti commented Aug 24, 2021

Uh oh!

Spaceman1984 commented Feb 18, 2021 •

edited by sureshanaparti

Loading

sureshanaparti Mar 17, 2021 •

edited

Loading

Spaceman1984 Jun 24, 2021 •

edited

Loading

Spaceman1984 commented Jun 24, 2021 •

edited

Loading