From 5ab23cd9c971b555cb80befe9c521dfb25f01fee Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 21 Jun 2024 10:28:18 +0530 Subject: [PATCH] Timeout config to copy the disks of remote KVM instance while importing the instance from an external host (#9213) * Added timeout config to copy the disks of remote KVM instance while importing the instance from an external host * Updated copy config units to mins * Cleanup remote converted file and local file when copy failed --- .../cloudstack/vm/UnmanagedVMsManager.java | 9 +++++++ .../agent/api/CopyRemoteVolumeCommand.java | 3 --- .../resource/LibvirtComputingResource.java | 27 +++++++++++++------ ...LibvirtCopyRemoteVolumeCommandWrapper.java | 17 ++++++------ .../ConfigurationManagerImpl.java | 2 ++ .../vm/UnmanagedVMsManagerImpl.java | 21 ++++++++------- .../java/com/cloud/utils/ssh/SshHelper.java | 8 +++++- 7 files changed, 58 insertions(+), 29 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManager.java b/api/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManager.java index 53aece94964..e3a484b35b3 100644 --- a/api/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManager.java +++ b/api/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManager.java @@ -30,6 +30,15 @@ public interface UnmanagedVMsManager extends VmImportService, UnmanageVMService, "If set to true, do not remove VM nics (and its MAC addresses) when unmanaging a VM, leaving them allocated but not reserved. " + "If set to false, nics are removed and MAC addresses can be reassigned", true, ConfigKey.Scope.Zone); + ConfigKey RemoteKvmInstanceDisksCopyTimeout = new ConfigKey<>(Integer.class, + "remote.kvm.instance.disks.copy.timeout", + "Advanced", + "30", + "Timeout (in mins) to prepare and copy the disks of remote KVM instance while importing the instance from an external host", + true, + ConfigKey.Scope.Global, + null); + static boolean isSupported(Hypervisor.HypervisorType hypervisorType) { return hypervisorType == VMware || hypervisorType == KVM; } diff --git a/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java b/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java index 82bc4d7cb45..88e5669f3b1 100644 --- a/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java +++ b/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java @@ -23,14 +23,11 @@ import com.cloud.agent.api.to.StorageFilerTO; @LogLevel(LogLevel.Log4jLevel.Trace) public class CopyRemoteVolumeCommand extends Command { - String remoteIp; String username; String password; String srcFile; - String tmpPath; - StorageFilerTO storageFilerTO; public CopyRemoteVolumeCommand(String remoteIp, String username, String password) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 37aba35bb9c..76e4efdfd7f 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -5336,20 +5336,31 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv /* Scp volume from remote host to local directory */ - public String copyVolume(String srcIp, String username, String password, String localDir, String remoteFile, String tmpPath) { + public String copyVolume(String srcIp, String username, String password, String localDir, String remoteFile, String tmpPath, int timeoutInSecs) { + String outputFile = UUID.randomUUID().toString(); try { - String outputFile = UUID.randomUUID().toString(); StringBuilder command = new StringBuilder("qemu-img convert -O qcow2 "); command.append(remoteFile); - command.append(" "+tmpPath); + command.append(" " + tmpPath); command.append(outputFile); - s_logger.debug("Converting remoteFile: "+remoteFile); - SshHelper.sshExecute(srcIp, 22, username, null, password, command.toString()); - s_logger.debug("Copying remoteFile to: "+localDir); - SshHelper.scpFrom(srcIp, 22, username, null, password, localDir, tmpPath+outputFile); - s_logger.debug("Successfully copyied remoteFile to: "+localDir+"/"+outputFile); + s_logger.debug(String.format("Converting remote disk file: %s, output file: %s%s (timeout: %d secs)", remoteFile, tmpPath, outputFile, timeoutInSecs)); + SshHelper.sshExecute(srcIp, 22, username, null, password, command.toString(), timeoutInSecs * 1000); + s_logger.debug("Copying converted remote disk file " + outputFile + " to: " + localDir); + SshHelper.scpFrom(srcIp, 22, username, null, password, localDir, tmpPath + outputFile); + s_logger.debug("Successfully copied converted remote disk file to: " + localDir + "/" + outputFile); return outputFile; } catch (Exception e) { + try { + String deleteRemoteConvertedFileCmd = String.format("rm -f %s%s", tmpPath, outputFile); + SshHelper.sshExecute(srcIp, 22, username, null, password, deleteRemoteConvertedFileCmd); + } catch (Exception ignored) { + } + + try { + FileUtils.deleteQuietly(new File(localDir + "/" + outputFile)); + } catch (Exception ignored) { + } + throw new RuntimeException(e); } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyRemoteVolumeCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyRemoteVolumeCommandWrapper.java index e48edd8eec0..a5e1716da2e 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyRemoteVolumeCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyRemoteVolumeCommandWrapper.java @@ -46,7 +46,6 @@ public final class LibvirtCopyRemoteVolumeCommandWrapper extends CommandWrapper< @Override public Answer execute(final CopyRemoteVolumeCommand command, final LibvirtComputingResource libvirtComputingResource) { - String result = null; String srcIp = command.getRemoteIp(); String username = command.getUsername(); String password = command.getPassword(); @@ -56,23 +55,25 @@ public final class LibvirtCopyRemoteVolumeCommandWrapper extends CommandWrapper< KVMStoragePoolManager poolMgr = libvirtComputingResource.getStoragePoolMgr(); KVMStoragePool pool = poolMgr.getStoragePool(storageFilerTO.getType(), storageFilerTO.getUuid()); String dstPath = pool.getLocalPath(); + int timeoutInSecs = command.getWait(); try { if (storageFilerTO.getType() == Storage.StoragePoolType.Filesystem || storageFilerTO.getType() == Storage.StoragePoolType.NetworkFilesystem) { - String filename = libvirtComputingResource.copyVolume(srcIp, username, password, dstPath, srcFile, tmpPath); - s_logger.debug("Volume Copy Successful"); + String filename = libvirtComputingResource.copyVolume(srcIp, username, password, dstPath, srcFile, tmpPath, timeoutInSecs); + s_logger.debug("Volume " + srcFile + " copy successful, copied to file: " + filename); final KVMPhysicalDisk vol = pool.getPhysicalDisk(filename); final String path = vol.getPath(); long size = getVirtualSizeFromFile(path); - return new CopyRemoteVolumeAnswer(command, "", filename, size); + return new CopyRemoteVolumeAnswer(command, "", filename, size); } else { - return new Answer(command, false, "Unsupported Storage Pool"); + String msg = "Unsupported storage pool type: " + storageFilerTO.getType().toString() + ", only local and NFS pools are supported"; + return new Answer(command, false, msg); } - } catch (final Exception e) { - s_logger.error("Error while copying file from remote host: "+ e.getMessage()); - return new Answer(command, false, result); + s_logger.error("Error while copying volume file from remote host: " + e.getMessage(), e); + String msg = "Failed to copy volume due to: " + e.getMessage(); + return new Answer(command, false, msg); } } diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index f65dffb18cd..eb72a626036 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -135,6 +135,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.userdata.UserDataManager; import org.apache.cloudstack.utils.jsinterpreter.TagAsRuleHelper; import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; +import org.apache.cloudstack.vm.UnmanagedVMsManager; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.ObjectUtils; @@ -557,6 +558,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati configValuesForValidation.add(StorageManager.STORAGE_POOL_CLIENT_TIMEOUT.key()); configValuesForValidation.add(StorageManager.STORAGE_POOL_CLIENT_MAX_CONNECTIONS.key()); configValuesForValidation.add(UserDataManager.VM_USERDATA_MAX_LENGTH_STRING); + configValuesForValidation.add(UnmanagedVMsManager.RemoteKvmInstanceDisksCopyTimeout.key()); } private void weightBasedParametersForValidation() { diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index ed4f377a896..6e7c0245ebc 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -794,13 +794,19 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { } } copyRemoteVolumeCommand.setTempPath(tmpPath); + int copyTimeout = UnmanagedVMsManager.RemoteKvmInstanceDisksCopyTimeout.value(); + if (copyTimeout <= 0) { + copyTimeout = Integer.valueOf(UnmanagedVMsManager.RemoteKvmInstanceDisksCopyTimeout.defaultValue()); + } + int copyTimeoutInSecs = copyTimeout * 60; + copyRemoteVolumeCommand.setWait(copyTimeoutInSecs); Answer answer = agentManager.easySend(dest.getHost().getId(), copyRemoteVolumeCommand); if (!(answer instanceof CopyRemoteVolumeAnswer)) { - throw new CloudRuntimeException("Error while copying volume"); + throw new CloudRuntimeException("Error while copying volume of remote instance: " + answer.getDetails()); } CopyRemoteVolumeAnswer copyRemoteVolumeAnswer = (CopyRemoteVolumeAnswer) answer; if(!copyRemoteVolumeAnswer.getResult()) { - throw new CloudRuntimeException("Error while copying volume"); + throw new CloudRuntimeException("Unable to copy volume of remote instance"); } diskProfile.setSize(copyRemoteVolumeAnswer.getSize()); DiskProfile profile = volumeManager.updateImportedVolume(type, diskOffering, vm, template, deviceId, @@ -813,7 +819,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { Volume.Type type, VirtualMachineTemplate template, Long deviceId, Long hostId, String diskPath, DiskProfile diskProfile) { List storagePools = primaryDataStoreDao.findLocalStoragePoolsByHostAndTags(hostId, null); - if(storagePools.size() < 1) { throw new CloudRuntimeException("Local Storage not found for host"); } @@ -826,7 +831,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { return new Pair<>(profile, storagePool); } - private Pair importKVMSharedDisk(VirtualMachine vm, DiskOffering diskOffering, Volume.Type type, VirtualMachineTemplate template, Long deviceId, Long poolId, String diskPath, DiskProfile diskProfile) { @@ -838,7 +842,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { return new Pair<>(profile, storagePool); } - private Pair importDisk(UnmanagedInstanceTO.Disk disk, VirtualMachine vm, Cluster cluster, DiskOffering diskOffering, Volume.Type type, String name, Long diskSize, Long minIops, Long maxIops, VirtualMachineTemplate template, Account owner, Long deviceId) { @@ -2335,7 +2338,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Import failed for Vm: %s. Suitable deployment destination not found", userVm.getInstanceName())); } - Map storage = dest.getStorageForDisks(); Volume volume = volumeDao.findById(diskProfile.getVolumeId()); StoragePool storagePool = storage.get(volume); @@ -2355,7 +2357,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { } diskProfile.setSize(checkVolumeAnswer.getSize()); - List> diskProfileStoragePoolList = new ArrayList<>(); try { long deviceId = 1L; @@ -2376,7 +2377,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { return userVm; } - private NetworkVO getDefaultNetwork(DataCenter zone, Account owner, boolean selectAny) throws InsufficientCapacityException, ResourceAllocationException { NetworkVO defaultNetwork = null; @@ -2503,6 +2503,9 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[]{UnmanageVMPreserveNic}; + return new ConfigKey[]{ + UnmanageVMPreserveNic, + RemoteKvmInstanceDisksCopyTimeout + }; } } diff --git a/utils/src/main/java/com/cloud/utils/ssh/SshHelper.java b/utils/src/main/java/com/cloud/utils/ssh/SshHelper.java index 6a2aa827856..1856c0b3838 100644 --- a/utils/src/main/java/com/cloud/utils/ssh/SshHelper.java +++ b/utils/src/main/java/com/cloud/utils/ssh/SshHelper.java @@ -38,12 +38,18 @@ import com.cloud.utils.Pair; public class SshHelper { private static final int DEFAULT_CONNECT_TIMEOUT = 180000; private static final int DEFAULT_KEX_TIMEOUT = 60000; + private static final int DEFAULT_WAIT_RESULT_TIMEOUT = 120000; private static final Logger s_logger = Logger.getLogger(SshHelper.class); public static Pair sshExecute(String host, int port, String user, File pemKeyFile, String password, String command) throws Exception { - return sshExecute(host, port, user, pemKeyFile, password, command, DEFAULT_CONNECT_TIMEOUT, DEFAULT_KEX_TIMEOUT, 120000); + return sshExecute(host, port, user, pemKeyFile, password, command, DEFAULT_CONNECT_TIMEOUT, DEFAULT_KEX_TIMEOUT, DEFAULT_WAIT_RESULT_TIMEOUT); + } + + public static Pair sshExecute(String host, int port, String user, File pemKeyFile, String password, String command, int waitResultTimeoutInMs) throws Exception { + + return sshExecute(host, port, user, pemKeyFile, password, command, DEFAULT_CONNECT_TIMEOUT, DEFAULT_KEX_TIMEOUT, waitResultTimeoutInMs); } public static void scpTo(String host, int port, String user, File pemKeyFile, String password, String remoteTargetDirectory, String localFile, String fileMode)