From fc285e23f38e75dff27e563d0eceb36c546bf049 Mon Sep 17 00:00:00 2001 From: sureshanaparti <12028987+sureshanaparti@users.noreply.github.com> Date: Thu, 2 Sep 2021 11:02:20 +0530 Subject: [PATCH] vmware: Cancel the pending tasks for worker VM before destroying it (#5374) Co-authored-by: nicolas --- .../manager/VmwareStorageManagerImpl.java | 15 ++---- .../vmware/resource/VmwareResource.java | 4 +- .../resource/VmwareStorageProcessor.java | 43 +++++---------- .../vmware/mo/HypervisorHostHelper.java | 3 +- .../vmware/mo/VirtualMachineMO.java | 34 +++++++++++- .../hypervisor/vmware/util/VmwareClient.java | 52 +++++++++++++++++++ 6 files changed, 106 insertions(+), 45 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java index a98032c7fdd..9ed1f7d1579 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java @@ -380,8 +380,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager { try { if (workerVm != null) { // detach volume and destroy worker vm - workerVm.detachAllDisks(); - workerVm.destroy(); + workerVm.detachAllDisksAndDestroy(); } } catch (Throwable e) { s_logger.warn("Failed to destroy worker VM: " + workerVMName); @@ -670,8 +669,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager { } finally { if (clonedVm != null) { - clonedVm.detachAllDisks(); - clonedVm.destroy(); + clonedVm.detachAllDisksAndDestroy(); } vmMo.removeSnapshot(templateUniqueName, false); @@ -923,8 +921,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager { clonedVm.detachAllDisks(); } finally { if (clonedVm != null) { - clonedVm.detachAllDisks(); - clonedVm.destroy(); + clonedVm.detachAllDisksAndDestroy(); } } } @@ -980,8 +977,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager { } } finally { if (clonedVm != null) { - clonedVm.detachAllDisks(); - clonedVm.destroy(); + clonedVm.detachAllDisksAndDestroy(); } } } @@ -1037,8 +1033,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager { } if (workerVm != null) { //detach volume and destroy worker vm - workerVm.detachAllDisks(); - workerVm.destroy(); + workerVm.detachAllDisksAndDestroy(); } } } 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 8e41ed118b2..cafdc74591f 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 @@ -6099,9 +6099,9 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa if (recycle) { s_logger.info("Recycle pending worker VM: " + vmMo.getName()); + vmMo.cancelPendingTasks(); vmMo.powerOff(); - vmMo.detachAllDisks(); - vmMo.destroy(); + vmMo.detachAllDisksAndDestroy(); } } } 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 0f221521f47..6536bb7f466 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 @@ -429,8 +429,7 @@ public class VmwareStorageProcessor implements StorageProcessor { virtualDeviceBackingInfo = backingInfo.getParent(); } - vmMo.detachAllDisks(); - vmMo.destroy(); + vmMo.detachAllDisksAndDestroy(); VmwareStorageLayoutHelper.moveVolumeToRootFolder(dcMo, backingFiles); @@ -820,8 +819,7 @@ public class VmwareStorageProcessor implements StorageProcessor { if (volume.getDeviceId().equals(0L)) { if (existingVm != null) { s_logger.info("Found existing VM " + vmName + " before cloning from template, destroying it"); - existingVm.detachAllDisks(); - existingVm.destroy(); + existingVm.detachAllDisksAndDestroy(); } s_logger.info("ROOT Volume from deploy-as-is template, cloning template"); cloneVMFromTemplate(hyperHost, template.getPath(), vmName, primaryStore.getUuid()); @@ -853,8 +851,7 @@ public class VmwareStorageProcessor implements StorageProcessor { s_logger.info("Destroy dummy VM after volume creation"); if (vmMo != null) { s_logger.warn("Unable to destroy a null VM ManagedObjectReference"); - vmMo.detachAllDisks(); - vmMo.destroy(); + vmMo.detachAllDisksAndDestroy(); } } } else { @@ -926,8 +923,7 @@ public class VmwareStorageProcessor implements StorageProcessor { String vmdkFileBaseName = vmMo.getVmdkFileBaseNames().get(0); if (volume.getVolumeType() == Volume.Type.DATADISK) { s_logger.info("detach disks from volume-wrapper VM " + vmName); - vmMo.detachAllDisks(); - vmMo.destroy(); + vmMo.detachAllDisksAndDestroy(); } return vmdkFileBaseName; } @@ -961,11 +957,8 @@ public class VmwareStorageProcessor implements StorageProcessor { dsMo.moveDatastoreFile(vmwareLayoutFilePair[i], dcMo.getMor(), dsMo.getMor(), legacyCloudStackLayoutFilePair[i], dcMo.getMor(), true); } - s_logger.info("detach disks from volume-wrapper VM " + vmdkName); - vmMo.detachAllDisks(); - - s_logger.info("destroy volume-wrapper VM " + vmdkName); - vmMo.destroy(); + s_logger.info("detach disks from volume-wrapper VM and destroy" + vmdkName); + vmMo.detachAllDisksAndDestroy(); String srcFile = dsMo.getDatastorePath(vmdkName, true); @@ -1134,8 +1127,7 @@ public class VmwareStorageProcessor implements StorageProcessor { } if (workerVm != null) { //detach volume and destroy worker vm - workerVm.detachAllDisks(); - workerVm.destroy(); + workerVm.detachAllDisksAndDestroy(); } } } @@ -1272,8 +1264,7 @@ public class VmwareStorageProcessor implements StorageProcessor { } finally { if (clonedVm != null) { - clonedVm.detachAllDisks(); - clonedVm.destroy(); + clonedVm.detachAllDisksAndDestroy(); } } } @@ -1354,8 +1345,7 @@ public class VmwareStorageProcessor implements StorageProcessor { } finally { try { if (volume.getVmName() == null && workerVmMo != null) { - workerVmMo.detachAllDisks(); - workerVmMo.destroy(); + workerVmMo.detachAllDisksAndDestroy(); } } catch (Throwable e) { s_logger.error("Failed to destroy worker VM created for detached volume"); @@ -1665,8 +1655,7 @@ public class VmwareStorageProcessor implements StorageProcessor { workerVM.exportVm(installFullPath, exportName, false, false); - workerVM.detachAllDisks(); - workerVM.destroy(); + workerVM.detachAllDisksAndDestroy(); } private String getTemplateVmdkName(String installFullPath, String exportName) { @@ -1843,8 +1832,7 @@ public class VmwareStorageProcessor implements StorageProcessor { return new Pair<>(diskDevice, disks); } finally { if (clonedVm != null) { - clonedVm.detachAllDisks(); - clonedVm.destroy(); + clonedVm.detachAllDisksAndDestroy(); } } } @@ -2013,8 +2001,7 @@ public class VmwareStorageProcessor implements StorageProcessor { try { if (workerVm != null) { // detach volume and destroy worker vm - workerVm.detachAllDisks(); - workerVm.destroy(); + workerVm.detachAllDisksAndDestroy(); } } catch (Throwable e) { s_logger.warn("Failed to destroy worker VM: " + workerVMName); @@ -2556,8 +2543,7 @@ public class VmwareStorageProcessor implements StorageProcessor { } finally { s_logger.info("Destroy dummy VM after volume creation"); if (vmMo != null) { - vmMo.detachAllDisks(); - vmMo.destroy(); + vmMo.detachAllDisksAndDestroy(); } } } @@ -3768,8 +3754,7 @@ public class VmwareStorageProcessor implements StorageProcessor { return _storage.getSize(srcOVFFileName); } finally { if (clonedVm != null) { - clonedVm.detachAllDisks(); - clonedVm.destroy(); + clonedVm.detachAllDisksAndDestroy(); } } } diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java index aafd112b223..0c41263bcef 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java @@ -2115,8 +2115,7 @@ public class HypervisorHostHelper { throw e; } } finally { - workerVmMo.detachAllDisks(); - workerVmMo.destroy(); + workerVmMo.detachAllDisksAndDestroy(); } } diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java index 8db7ff9c963..30396f62fd5 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java @@ -39,6 +39,8 @@ import java.util.concurrent.Future; import com.cloud.utils.exception.CloudRuntimeException; import com.vmware.vim25.InvalidStateFaultMsg; import com.vmware.vim25.RuntimeFaultFaultMsg; +import com.vmware.vim25.TaskInfo; +import com.vmware.vim25.TaskInfoState; import com.vmware.vim25.VirtualMachineTicket; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang.StringUtils; @@ -1466,6 +1468,11 @@ public class VirtualMachineMO extends BaseMO { return chain; } + public void detachAllDisksAndDestroy() throws Exception { + detachAllDisks(); + destroy(); + } + public void detachAllDisks() throws Exception { if (s_logger.isTraceEnabled()) s_logger.trace("vCenter API trace - detachAllDisk(). target MOR: " + _mor.getValue()); @@ -2016,8 +2023,7 @@ public class VirtualMachineMO extends BaseMO { return clonedVmMo; } finally { if (!bSuccess) { - clonedVmMo.detachAllDisks(); - clonedVmMo.destroy(); + clonedVmMo.detachAllDisksAndDestroy(); } } } @@ -3568,6 +3574,30 @@ public class VirtualMachineMO extends BaseMO { return ticket.getTicket(); } + public void cancelPendingTasks() throws Exception { + String vmName = getVmName(); + s_logger.debug("Checking for pending tasks of the VM: " + vmName); + + ManagedObjectReference taskmgr = _context.getServiceContent().getTaskManager(); + List tasks = _context.getVimClient().getDynamicProperty(taskmgr, "recentTask"); + + int vmTasks = 0, vmPendingTasks = 0; + for (ManagedObjectReference task : tasks) { + TaskInfo info = (TaskInfo) (_context.getVimClient().getDynamicProperty(task, "info")); + if (info.getEntityName().equals(vmName)) { + vmTasks++; + if (!(info.getState().equals(TaskInfoState.SUCCESS) || info.getState().equals(TaskInfoState.ERROR))) { + String taskName = StringUtils.isNotBlank(info.getName()) ? info.getName() : "Unknown"; + s_logger.debug(taskName + " task pending for the VM: " + vmName + ", cancelling it"); + vmPendingTasks++; + _context.getVimClient().cancelTask(task); + } + } + } + + s_logger.debug(vmPendingTasks + " pending tasks for the VM: " + vmName + " found, out of " + vmTasks + " recent VM tasks"); + } + public void tagAsWorkerVM() throws Exception { setCustomFieldValue(CustomFieldConstants.CLOUD_WORKER, "true"); String workerTag = String.format("%d-%s", System.currentTimeMillis(), getContext().getStockObject("noderuninfo")); diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareClient.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareClient.java index 2395ccf5326..9d79e5466af 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareClient.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareClient.java @@ -42,6 +42,8 @@ import org.apache.cloudstack.utils.security.SecureSSLSocketFactory; import com.vmware.pbm.PbmPortType; import com.vmware.pbm.PbmService; import com.vmware.pbm.PbmServiceInstanceContent; + +import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; import org.w3c.dom.Element; @@ -779,4 +781,54 @@ public class VmwareClient { return vCenterSessionTimeout; } + public void cancelTask(ManagedObjectReference task) throws Exception { + TaskInfo info = (TaskInfo)(getDynamicProperty(task, "info")); + if (info == null) { + s_logger.warn("Unable to get the task info, so couldn't cancel the task"); + return; + } + + String taskName = StringUtils.isNotBlank(info.getName()) ? info.getName() : "Unknown"; + taskName += "(" + info.getKey() + ")"; + + String entityName = StringUtils.isNotBlank(info.getEntityName()) ? info.getEntityName() : ""; + + if (info.getState().equals(TaskInfoState.SUCCESS)) { + s_logger.debug(taskName + " task successfully completed for the entity " + entityName + ", can't cancel it"); + return; + } + + if (info.getState().equals(TaskInfoState.ERROR)) { + s_logger.debug(taskName + " task execution failed for the entity " + entityName + ", can't cancel it"); + return; + } + + s_logger.debug(taskName + " task pending for the entity " + entityName + ", trying to cancel"); + if (!info.isCancelable()) { + s_logger.warn(taskName + " task will continue to run on vCenter because it can't be cancelled"); + return; + } + + s_logger.debug("Cancelling task " + taskName + " of the entity " + entityName); + getService().cancelTask(task); + + // Since task cancellation is asynchronous, wait for the task to be cancelled + Object[] result = waitForValues(task, new String[] {"info.state", "info.error"}, new String[] {"state"}, + new Object[][] {new Object[] {TaskInfoState.SUCCESS, TaskInfoState.ERROR}}); + + if (result != null && result.length == 2) { //result for 2 properties: info.state, info.error + if (result[0].equals(TaskInfoState.SUCCESS)) { + s_logger.warn("Failed to cancel" + taskName + " task of the entity " + entityName + ", the task successfully completed"); + } + + if (result[1] instanceof LocalizedMethodFault) { + MethodFault fault = ((LocalizedMethodFault)result[1]).getFault(); + if (fault instanceof RequestCanceled) { + s_logger.debug(taskName + " task of the entity " + entityName + " was successfully cancelled"); + } + } else { + s_logger.warn("Couldn't cancel " + taskName + " task of the entity " + entityName + " due to " + ((LocalizedMethodFault)result[1]).getLocalizedMessage()); + } + } + } }