From 1d3083db86a9baef1c2fe0a5c0e10743fee76eb6 Mon Sep 17 00:00:00 2001 From: Spaceman1984 <49917670+Spaceman1984@users.noreply.github.com> Date: Sat, 28 Aug 2021 14:04:50 +0200 Subject: [PATCH] Added support for removing unused port groups on VMWare (#4701) * Added support for removing unused port groups on VMWare * Fixed error handling around unavailable portgroup name * Review changes, defaulting glovbal var to false, added warning to description, changed if statement. * Cleanup unused network port groups on all the hosts. Co-authored-by: Suresh Kumar Anaparti Co-authored-by: nicolas --- .../vmware/manager/VmwareManager.java | 3 + .../vmware/manager/VmwareManagerImpl.java | 2 +- .../vmware/resource/VmwareResource.java | 60 +++++++++++++++---- .../resource/VmwareStorageProcessor.java | 5 +- .../hypervisor/vmware/mo/DatacenterMO.java | 14 +++++ 5 files changed, 70 insertions(+), 14 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManager.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManager.java index 6d1de67963c..8bd4a67209c 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManager.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManager.java @@ -46,6 +46,9 @@ public interface VmwareManager { static final ConfigKey s_vmwareOVAPackageTimeout = new ConfigKey(Integer.class, "vmware.package.ova.timeout", "Advanced", "3600", "Vmware script timeout for ova packaging process", true, ConfigKey.Scope.Global, 1000); + static final ConfigKey s_vmwareCleanupPortGroups = new ConfigKey("Advanced", Boolean.class, "vmware.cleanup.port.groups", "false", + "When set to true, the unused port groups are removed from VMware hypervisor hosts. WARNING: Native VMware HA might not work when enabled.", true, ConfigKey.Scope.Global); + public static final ConfigKey VMWARE_STATS_TIME_WINDOW = new ConfigKey("Advanced", Integer.class, "vmware.stats.time.window", "300", "VMware interval window (in seconds) to collect metrics. If this is set to less than 20, then default (300 seconds) will be used. The interval used must be enabled in vCenter for this change to work, " + "otherwise the collection of metrics will result in an error. Check VMWare docs to know how to enable metrics interval.", true); diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java index 04c6bc3f9be..8411d251a4b 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java @@ -275,7 +275,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {s_vmwareNicHotplugWaitTimeout, s_vmwareCleanOldWorderVMs, templateCleanupInterval, s_vmwareSearchExcludeFolder, s_vmwareOVAPackageTimeout, VMWARE_STATS_TIME_WINDOW}; + return new ConfigKey[] {s_vmwareNicHotplugWaitTimeout, s_vmwareCleanOldWorderVMs, templateCleanupInterval, s_vmwareSearchExcludeFolder, s_vmwareOVAPackageTimeout, s_vmwareCleanupPortGroups, VMWARE_STATS_TIME_WINDOW}; } @Override public boolean configure(String name, Map params) throws ConfigurationException { diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 54aed10713c..2aba1e7f6db 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -48,6 +48,7 @@ import java.util.stream.Collectors; import javax.naming.ConfigurationException; import javax.xml.datatype.XMLGregorianCalendar; +import com.cloud.hypervisor.vmware.mo.NetworkMO; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.storage.command.CopyCommand; import org.apache.cloudstack.storage.command.StorageSubSystemCommand; @@ -5830,17 +5831,54 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa } } - public void cleanupNetwork(HostMO hostMo, NetworkDetails netDetails) { - // we will no longer cleanup VLAN networks in order to support native VMware HA - /* - * assert(netDetails.getName() != null); try { synchronized(this) { NetworkMO networkMo = new - * NetworkMO(hostMo.getContext(), netDetails.getNetworkMor()); ManagedObjectReference[] vms = - * networkMo.getVMsOnNetwork(); if(vms == null || vms.length == 0) { if(s_logger.isInfoEnabled()) { - * s_logger.info("Cleanup network as it is currently not in use: " + netDetails.getName()); } - * - * hostMo.deletePortGroup(netDetails.getName()); } } } catch(Throwable e) { - * s_logger.warn("Unable to cleanup network due to exception, skip for next time"); } - */ + public void cleanupNetwork(DatacenterMO dcMO, NetworkDetails netDetails) { + if (!VmwareManager.s_vmwareCleanupPortGroups.value()){ + return; + } + + try { + synchronized(this) { + if (!areVMsOnNetwork(dcMO, netDetails)) { + cleanupPortGroup(dcMO, netDetails.getName()); + } + } + } catch(Throwable e) { + s_logger.warn("Unable to cleanup network due to exception: " + e.getMessage(), e); + } + } + + private void cleanupPortGroup(DatacenterMO dcMO, String portGroupName) throws Exception { + if (StringUtils.isBlank(portGroupName)) { + s_logger.debug("Unspecified network port group, couldn't cleanup"); + return; + } + + List hosts = dcMO.getAllHostsOnDatacenter(); + if (!CollectionUtils.isEmpty(hosts)) { + for (HostMO host : hosts) { + host.deletePortGroup(portGroupName); + } + } + } + + private boolean areVMsOnNetwork(DatacenterMO dcMO, NetworkDetails netDetails) throws Exception { + if (netDetails == null || netDetails.getName() == null) { + throw new CloudRuntimeException("Unspecified network details / port group, couldn't check VMs on network port group"); + } + + List hosts = dcMO.getAllHostsOnDatacenter(); + if (!CollectionUtils.isEmpty(hosts)) { + for (HostMO host : hosts) { + NetworkMO networkMo = new NetworkMO(host.getContext(), netDetails.getNetworkMor()); + List vms = networkMo.getVMsOnNetwork(); + if (!CollectionUtils.isEmpty(vms)) { + s_logger.debug("Network port group: " + netDetails.getName() + " is in use"); + return true; + } + } + } + + return false; } @Override diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java index 97c32867496..30e0191bb22 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -2650,6 +2650,8 @@ public class VmwareStorageProcessor implements StorageProcessor { DatastoreMO dsMo = new DatastoreMO(context, morDs); ManagedObjectReference morDc = hyperHost.getHyperHostDatacenter(); + DatacenterMO dcMo = new DatacenterMO(context, morDc); + ManagedObjectReference morCluster = hyperHost.getHyperHostCluster(); ClusterMO clusterMo = new ClusterMO(context, morCluster); @@ -2660,7 +2662,6 @@ public class VmwareStorageProcessor implements StorageProcessor { VirtualMachineMO vmMo = clusterMo.findVmOnHyperHost(vmName); if (vmMo == null) { // Volume might be on a zone-wide storage pool, look for VM in datacenter - DatacenterMO dcMo = new DatacenterMO(context, morDc); vmMo = dcMo.findVm(vmName); } @@ -2715,7 +2716,7 @@ public class VmwareStorageProcessor implements StorageProcessor { for (NetworkDetails netDetails : networks) { if (netDetails.getGCTag() != null && netDetails.getGCTag().equalsIgnoreCase("true")) { if (netDetails.getVMMorsOnNetwork() == null || netDetails.getVMMorsOnNetwork().length == 1) { - resource.cleanupNetwork(hostMo, netDetails); + resource.cleanupNetwork(dcMo, netDetails); } } } diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/DatacenterMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/DatacenterMO.java index af89757d521..ade82275376 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/DatacenterMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/DatacenterMO.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.apache.commons.collections.CollectionUtils; import org.apache.log4j.Logger; import com.vmware.vim25.CustomFieldStringValue; @@ -172,6 +173,19 @@ public class DatacenterMO extends BaseMO { return vms; } + public List getAllHostsOnDatacenter() throws Exception { + List hosts = new ArrayList<>(); + + List ocs = getHostPropertiesOnDatacenterHostFolder(new String[] {"name"}); + if (CollectionUtils.isNotEmpty(ocs)) { + for (ObjectContent oc : ocs) { + hosts.add(new HostMO(getContext(), oc.getObj())); + } + } + + return hosts; + } + public ManagedObjectReference findDatastore(String name) throws Exception { assert (name != null);