From b4ad04badfbc35cd11f761866b59eb475fbd479f Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Wed, 18 Dec 2024 13:42:28 +0530 Subject: [PATCH] Allow config drive deletion of migrated VM, on host maintenance (#10045) --- .../java/com/cloud/vm/VmDetailConstants.java | 1 + .../com/cloud/agent/manager/AgentAttache.java | 10 ++--- .../resource/LibvirtComputingResource.java | 33 +++++++++------- .../kvm/storage/KVMStoragePoolManager.java | 3 +- .../element/ConfigDriveNetworkElement.java | 39 +++++++++++++++---- 5 files changed, 59 insertions(+), 27 deletions(-) diff --git a/api/src/main/java/com/cloud/vm/VmDetailConstants.java b/api/src/main/java/com/cloud/vm/VmDetailConstants.java index 9338cc11cd4..f7d87ab18f1 100644 --- a/api/src/main/java/com/cloud/vm/VmDetailConstants.java +++ b/api/src/main/java/com/cloud/vm/VmDetailConstants.java @@ -73,6 +73,7 @@ public interface VmDetailConstants { String ENCRYPTED_PASSWORD = "Encrypted.Password"; String CONFIG_DRIVE_LOCATION = "configDriveLocation"; + String LAST_CONFIG_DRIVE_LOCATION = "lastConfigDriveLocation"; String SKIP_DRS = "skipFromDRS"; diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java index 2d8d6f1c48e..4d9ee0a9ab4 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java @@ -46,6 +46,7 @@ import com.cloud.agent.api.CleanupNetworkRulesCmd; import com.cloud.agent.api.Command; import com.cloud.agent.api.CreateStoragePoolCommand; import com.cloud.agent.api.DeleteStoragePoolCommand; +import com.cloud.agent.api.HandleConfigDriveIsoCommand; import com.cloud.agent.api.MaintainCommand; import com.cloud.agent.api.MigrateCommand; import com.cloud.agent.api.ModifySshKeysCommand; @@ -119,11 +120,10 @@ public abstract class AgentAttache { public final static String[] s_commandsAllowedInMaintenanceMode = new String[] { MaintainCommand.class.toString(), MigrateCommand.class.toString(), StopCommand.class.toString(), CheckVirtualMachineCommand.class.toString(), PingTestCommand.class.toString(), CheckHealthCommand.class.toString(), - ReadyCommand.class.toString(), ShutdownCommand.class.toString(), SetupCommand.class.toString(), - CleanupNetworkRulesCmd.class.toString(), CheckNetworkCommand.class.toString(), PvlanSetupCommand.class.toString(), CheckOnHostCommand.class.toString(), - ModifyTargetsCommand.class.toString(), ModifySshKeysCommand.class.toString(), - CreateStoragePoolCommand.class.toString(), DeleteStoragePoolCommand.class.toString(), ModifyStoragePoolCommand.class.toString(), - SetupMSListCommand.class.toString(), RollingMaintenanceCommand.class.toString(), CleanupPersistentNetworkResourceCommand.class.toString()}; + ReadyCommand.class.toString(), ShutdownCommand.class.toString(), SetupCommand.class.toString(), CleanupNetworkRulesCmd.class.toString(), + CheckNetworkCommand.class.toString(), PvlanSetupCommand.class.toString(), CheckOnHostCommand.class.toString(), ModifyTargetsCommand.class.toString(), + ModifySshKeysCommand.class.toString(), CreateStoragePoolCommand.class.toString(), DeleteStoragePoolCommand.class.toString(), ModifyStoragePoolCommand.class.toString(), + SetupMSListCommand.class.toString(), RollingMaintenanceCommand.class.toString(), CleanupPersistentNetworkResourceCommand.class.toString(), HandleConfigDriveIsoCommand.class.toString()}; protected final static String[] s_commandsNotAllowedInConnectingMode = new String[] { StartCommand.class.toString(), CreateCommand.class.toString() }; static { Arrays.sort(s_commandsAllowedInMaintenanceMode); 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 a4d7bd524ad..f9d56f8301d 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 @@ -2985,9 +2985,10 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv public static boolean useBLOCKDiskType(KVMPhysicalDisk physicalDisk) { return physicalDisk != null && - physicalDisk.getPool().getType() == StoragePoolType.Linstor && + physicalDisk.getPool() != null && + StoragePoolType.Linstor.equals(physicalDisk.getPool().getType()) && physicalDisk.getFormat() != null && - physicalDisk.getFormat()== PhysicalDiskFormat.RAW; + PhysicalDiskFormat.RAW.equals(physicalDisk.getFormat()); } public static DiskDef.DiskType getDiskType(KVMPhysicalDisk physicalDisk) { @@ -3402,13 +3403,15 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } if (configdrive != null) { try { + s_logger.debug(String.format("Detaching ConfigDrive ISO of the VM %s, at path %s", vmName, configdrive.getDiskPath())); String result = attachOrDetachISO(conn, vmName, configdrive.getDiskPath(), false, CONFIG_DRIVE_ISO_DEVICE_ID); if (result != null) { - s_logger.warn("Detach ConfigDrive ISO with result: " + result); + s_logger.warn(String.format("Detach ConfigDrive ISO of the VM %s, at path %s with %s: ", vmName, configdrive.getDiskPath(), result)); } + s_logger.debug(String.format("Attaching ConfigDrive ISO of the VM %s, at path %s", vmName, configdrive.getDiskPath())); result = attachOrDetachISO(conn, vmName, configdrive.getDiskPath(), true, CONFIG_DRIVE_ISO_DEVICE_ID); if (result != null) { - s_logger.warn("Attach ConfigDrive ISO with result: " + result); + s_logger.warn(String.format("Attach ConfigDrive ISO of the VM %s, at path %s with %s: ", vmName, configdrive.getDiskPath(), result)); } } catch (final LibvirtException | InternalErrorException | URISyntaxException e) { final String msg = "Detach and attach ConfigDrive ISO failed due to " + e.toString(); @@ -3420,16 +3423,20 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv public synchronized String attachOrDetachISO(final Connect conn, final String vmName, String isoPath, final boolean isAttach, final Integer diskSeq) throws LibvirtException, URISyntaxException, InternalErrorException { final DiskDef iso = new DiskDef(); - if (isoPath != null && isAttach) { - final int index = isoPath.lastIndexOf("/"); - final String path = isoPath.substring(0, index); - final String name = isoPath.substring(index + 1); - final KVMStoragePool secondaryPool = storagePoolManager.getStoragePoolByURI(path); - final KVMPhysicalDisk isoVol = secondaryPool.getPhysicalDisk(name); - final DiskDef.DiskType diskType = getDiskType(isoVol); - isoPath = isoVol.getPath(); + if (isAttach && StringUtils.isNotBlank(isoPath) && isoPath.lastIndexOf("/") > 0) { + if (isoPath.startsWith(getConfigPath() + "/" + ConfigDrive.CONFIGDRIVEDIR) && isoPath.contains(vmName)) { + iso.defISODisk(isoPath, diskSeq, DiskDef.DiskType.FILE); + } else { + final int index = isoPath.lastIndexOf("/"); + final String path = isoPath.substring(0, index); + final String name = isoPath.substring(index + 1); + final KVMStoragePool storagePool = storagePoolManager.getStoragePoolByURI(path); + final KVMPhysicalDisk isoVol = storagePool.getPhysicalDisk(name); + final DiskDef.DiskType diskType = getDiskType(isoVol); + isoPath = isoVol.getPath(); - iso.defISODisk(isoPath, diskSeq, diskType); + iso.defISODisk(isoPath, diskSeq, diskType); + } } else { iso.defISODisk(null, diskSeq, DiskDef.DiskType.FILE); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java index ee23e489951..da6a192f129 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java @@ -287,6 +287,7 @@ public class KVMStoragePoolManager { URI storageUri = null; try { + s_logger.debug("Get storage pool by uri: " + uri); storageUri = new URI(uri); } catch (URISyntaxException e) { throw new CloudRuntimeException(e.toString()); @@ -296,7 +297,7 @@ public class KVMStoragePoolManager { String uuid = null; String sourceHost = ""; StoragePoolType protocol = null; - final String scheme = storageUri.getScheme().toLowerCase(); + final String scheme = (storageUri.getScheme() != null) ? storageUri.getScheme().toLowerCase() : ""; List acceptedSchemes = List.of("nfs", "networkfilesystem", "filesystem"); if (acceptedSchemes.contains(scheme)) { sourcePath = storageUri.getPath(); diff --git a/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java b/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java index 83900ff2d43..38d71b9c507 100644 --- a/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java +++ b/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java @@ -341,10 +341,10 @@ public class ConfigDriveNetworkElement extends AdapterBase implements NetworkEle try { if (isConfigDriveIsoOnHostCache(vm.getId())) { vm.setConfigDriveLocation(Location.HOST); - configureConfigDriveData(vm, nic, dest); - - // Create the config drive on dest host cache - createConfigDriveIsoOnHostCache(vm, dest.getHost().getId()); + if (configureConfigDriveData(vm, nic, dest)) { + // Create the config drive on dest host cache + createConfigDriveIsoOnHostCache(vm, dest.getHost().getId()); + } } else { vm.setConfigDriveLocation(getConfigDriveLocation(vm.getId())); addPasswordAndUserdata(network, nic, vm, dest, context); @@ -373,7 +373,7 @@ public class ConfigDriveNetworkElement extends AdapterBase implements NetworkEle @Override public void commitMigration(NicProfile nic, Network network, VirtualMachineProfile vm, ReservationContext src, ReservationContext dst) { try { - if (isConfigDriveIsoOnHostCache(vm.getId())) { + if (isLastConfigDriveIsoOnHostCache(vm.getId())) { vm.setConfigDriveLocation(Location.HOST); // Delete the config drive on src host cache deleteConfigDriveIsoOnHostCache(vm.getVirtualMachine(), vm.getHostId()); @@ -530,6 +530,17 @@ public class ConfigDriveNetworkElement extends AdapterBase implements NetworkEle return false; } + private boolean isLastConfigDriveIsoOnHostCache(long vmId) { + final UserVmDetailVO vmDetailLastConfigDriveLocation = _userVmDetailsDao.findDetail(vmId, VmDetailConstants.LAST_CONFIG_DRIVE_LOCATION); + if (vmDetailLastConfigDriveLocation == null) { + return isConfigDriveIsoOnHostCache(vmId); + } + if (Location.HOST.toString().equalsIgnoreCase(vmDetailLastConfigDriveLocation.getValue())) { + return true; + } + return false; + } + private boolean createConfigDriveIsoOnHostCache(VirtualMachineProfile profile, Long hostId) throws ResourceUnavailableException { if (hostId == null) { throw new ResourceUnavailableException("Config drive iso creation failed, dest host not available", @@ -556,7 +567,7 @@ public class ConfigDriveNetworkElement extends AdapterBase implements NetworkEle } profile.setConfigDriveLocation(answer.getConfigDriveLocation()); - _userVmDetailsDao.addDetail(profile.getId(), VmDetailConstants.CONFIG_DRIVE_LOCATION, answer.getConfigDriveLocation().toString(), false); + updateConfigDriveLocationInVMDetails(profile.getId(), answer.getConfigDriveLocation()); addConfigDriveDisk(profile, null); return true; } @@ -618,11 +629,23 @@ public class ConfigDriveNetworkElement extends AdapterBase implements NetworkEle answer.getDetails()), ConfigDriveNetworkElement.class, 0L); } profile.setConfigDriveLocation(answer.getConfigDriveLocation()); - _userVmDetailsDao.addDetail(profile.getId(), VmDetailConstants.CONFIG_DRIVE_LOCATION, answer.getConfigDriveLocation().toString(), false); + updateConfigDriveLocationInVMDetails(profile.getId(), answer.getConfigDriveLocation()); addConfigDriveDisk(profile, dataStore); return true; } + private void updateConfigDriveLocationInVMDetails(long vmId, NetworkElement.Location configDriveLocation) { + final UserVmDetailVO vmDetailConfigDriveLocation = _userVmDetailsDao.findDetail(vmId, VmDetailConstants.CONFIG_DRIVE_LOCATION); + if (vmDetailConfigDriveLocation != null) { + if (!configDriveLocation.toString().equalsIgnoreCase(vmDetailConfigDriveLocation.getValue())) { + _userVmDetailsDao.addDetail(vmId, VmDetailConstants.LAST_CONFIG_DRIVE_LOCATION, vmDetailConfigDriveLocation.getValue(), false); + } else { + _userVmDetailsDao.removeDetail(vmId, VmDetailConstants.LAST_CONFIG_DRIVE_LOCATION); + } + } + _userVmDetailsDao.addDetail(vmId, VmDetailConstants.CONFIG_DRIVE_LOCATION, configDriveLocation.toString(), false); + } + private Map getVMCustomUserdataParamMap(long vmId) { UserVmVO userVm = _userVmDao.findById(vmId); String userDataDetails = userVm.getUserDataDetails(); @@ -737,7 +760,7 @@ public class ConfigDriveNetworkElement extends AdapterBase implements NetworkEle private boolean configureConfigDriveData(final VirtualMachineProfile profile, final NicProfile nic, final DeployDestination dest) { final UserVmVO vm = _userVmDao.findById(profile.getId()); - if (vm.getType() != VirtualMachine.Type.User) { + if (vm == null || vm.getType() != VirtualMachine.Type.User) { return false; } final Nic defaultNic = _networkModel.getDefaultNic(vm.getId());