This is an automated email from the ASF dual-hosted git repository. dahn pushed a commit to branch 4.20 in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 08ad1c70ba3bd4c0033f7b5249ea4c9b11a5bbbb Merge: c80b8860e49 e196275d5a6 Author: Daan Hoogland <[email protected]> AuthorDate: Mon Feb 24 14:21:14 2025 +0100 Merge branch '4.19' into 4.20 .../com/cloud/agent/api/to/RemoteInstanceTO.java | 8 ++- .../java/com/cloud/storage/VolumeApiService.java | 22 ++++--- .../apache/cloudstack/vm/UnmanagedInstanceTO.java | 10 +++ .../LibvirtConvertInstanceCommandWrapper.java | 7 +- .../IpmitoolOutOfBandManagementDriver.java | 8 +-- .../driver/ipmitool/IpmitoolWrapper.java | 55 +++++++++------- .../com/cloud/storage/VolumeApiServiceImpl.java | 36 +++++----- .../volume/VolumeImportUnmanageManagerImpl.java | 8 +-- .../cloudstack/vm/UnmanagedVMsManagerImpl.java | 76 +++++++++++++++------- .../cloud/storage/VolumeApiServiceImplTest.java | 20 +++--- .../VolumeImportUnmanageManagerImplTest.java | 2 +- .../cloudstack/vm/UnmanagedVMsManagerImplTest.java | 68 +++++++++---------- ui/public/locales/ar.json | 2 +- ui/public/locales/ca.json | 2 +- ui/public/locales/de_DE.json | 2 +- ui/public/locales/el_GR.json | 2 +- ui/public/locales/en.json | 6 +- ui/public/locales/es.json | 2 +- ui/public/locales/fr_FR.json | 2 +- ui/public/locales/hu.json | 2 +- ui/public/locales/it_IT.json | 2 +- ui/public/locales/ja_JP.json | 2 +- ui/public/locales/ko_KR.json | 2 +- ui/public/locales/nb_NO.json | 2 +- ui/public/locales/nl_NL.json | 2 +- ui/public/locales/pl.json | 2 +- ui/public/locales/pt_BR.json | 2 +- ui/public/locales/ru_RU.json | 2 +- ui/public/locales/zh_CN.json | 2 +- ui/src/config/section/infra/clusters.js | 6 +- ui/src/config/section/infra/hosts.js | 2 +- ui/src/views/tools/SelectVmwareVcenter.vue | 32 ++++++--- .../com/cloud/hypervisor/vmware/mo/BaseMO.java | 4 ++ .../hypervisor/vmware/mo/VirtualMachineMO.java | 26 ++++++++ .../cloud/hypervisor/vmware/util/VmwareHelper.java | 1 + 35 files changed, 264 insertions(+), 165 deletions(-) diff --cc api/src/main/java/com/cloud/storage/VolumeApiService.java index 6f4c7aa09e2,833ba442888..bb69b5b6650 --- a/api/src/main/java/com/cloud/storage/VolumeApiService.java +++ b/api/src/main/java/com/cloud/storage/VolumeApiService.java @@@ -22,7 -22,7 +22,12 @@@ import java.net.MalformedURLException import java.util.List; import java.util.Map; ++import com.cloud.exception.ResourceAllocationException; ++import com.cloud.offering.DiskOffering; ++import com.cloud.user.Account; import com.cloud.utils.Pair; ++import com.cloud.utils.fsm.NoTransitionException; ++ import org.apache.cloudstack.api.command.user.volume.AssignVolumeCmd; import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd; import org.apache.cloudstack.api.command.user.volume.ChangeOfferingForVolumeCmd; @@@ -37,13 -37,13 +42,9 @@@ import org.apache.cloudstack.api.comman import org.apache.cloudstack.api.response.GetUploadParamsResponse; import org.apache.cloudstack.framework.config.ConfigKey; --import com.cloud.exception.ResourceAllocationException; --import com.cloud.user.Account; --import com.cloud.utils.fsm.NoTransitionException; -- public interface VolumeApiService { -- ConfigKey<Long> ConcurrentMigrationsThresholdPerDatastore = new ConfigKey<Long>("Advanced" ++ ConfigKey<Long> ConcurrentMigrationsThresholdPerDatastore = new ConfigKey<>("Advanced" , Long.class , "concurrent.migrations.per.target.datastore" , "0" @@@ -51,7 -51,7 +52,7 @@@ , true // not sure if this is to be dynamic , ConfigKey.Scope.Global); -- ConfigKey<Boolean> UseHttpsToUpload = new ConfigKey<Boolean>("Advanced", ++ ConfigKey<Boolean> UseHttpsToUpload = new ConfigKey<>("Advanced", Boolean.class, "use.https.to.upload", "true", @@@ -85,7 -85,7 +86,7 @@@ * @param cmd * the API command wrapping the criteria * @return the volume object -- * @throws ResourceAllocationException ++ * @throws ResourceAllocationException no capacity to allocate the new volume size */ Volume resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationException; @@@ -169,7 -163,7 +170,8 @@@ * </body> * </table> */ - boolean doesTargetStorageSupportDiskOffering(StoragePool destPool, String diskOfferingTags); ++ boolean doesStoragePoolSupportDiskOffering(StoragePool destPool, DiskOffering diskOffering); + boolean doesStoragePoolSupportDiskOfferingTags(StoragePool destPool, String diskOfferingTags); Volume destroyVolume(long volumeId, Account caller, boolean expunge, boolean forceExpunge); diff --cc plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java index 3ba1bc11975,504edb9d888..a11730a1240 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java @@@ -69,8 -57,8 +69,7 @@@ public class LibvirtConvertInstanceComm RemoteInstanceTO sourceInstance = cmd.getSourceInstance(); Hypervisor.HypervisorType sourceHypervisorType = sourceInstance.getHypervisorType(); String sourceInstanceName = sourceInstance.getInstanceName(); - String sourceInstancePath = sourceInstance.getInstancePath(); Hypervisor.HypervisorType destinationHypervisorType = cmd.getDestinationHypervisorType(); - List<String> destinationStoragePools = cmd.getDestinationStoragePools(); DataStoreTO conversionTemporaryLocation = cmd.getConversionTemporaryLocation(); long timeout = (long) cmd.getWait() * 1000; @@@ -196,6 -182,11 +196,11 @@@ String encodedUsername = encodeUsername(username); String encodedPassword = encodeUsername(password); + if (StringUtils.isNotBlank(path)) { - s_logger.info("VM path: " + path); ++ logger.info("VM path: {}", path); + return String.format("vi://%s:%s@%s/%s/%s/%s", + encodedUsername, encodedPassword, vcenter, datacenter, path, vm); + } return String.format("vi://%s:%s@%s/%s/vm/%s", encodedUsername, encodedPassword, vcenter, datacenter, vm); } diff --cc plugins/outofbandmanagement-drivers/ipmitool/src/main/java/org/apache/cloudstack/outofbandmanagement/driver/ipmitool/IpmitoolWrapper.java index 86ec615e552,2004943c0c6..b0ded5350a3 --- a/plugins/outofbandmanagement-drivers/ipmitool/src/main/java/org/apache/cloudstack/outofbandmanagement/driver/ipmitool/IpmitoolWrapper.java +++ b/plugins/outofbandmanagement-drivers/ipmitool/src/main/java/org/apache/cloudstack/outofbandmanagement/driver/ipmitool/IpmitoolWrapper.java @@@ -25,8 -25,8 +25,9 @@@ import org.apache.cloudstack.outofbandm import org.apache.cloudstack.outofbandmanagement.driver.OutOfBandManagementDriverResponse; import org.apache.cloudstack.utils.process.ProcessResult; import org.apache.cloudstack.utils.process.ProcessRunner; -import org.apache.log4j.Logger; -import org.jetbrains.annotations.NotNull; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.LogManager; ++ import org.joda.time.Duration; import java.util.ArrayList; @@@ -34,7 -34,7 +35,7 @@@ import java.util.List import java.util.concurrent.ExecutorService; public final class IpmitoolWrapper { - protected Logger logger = LogManager.getLogger(getClass()); - public static final Logger LOG = Logger.getLogger(IpmitoolWrapper.class); ++ Logger logger = LogManager.getLogger(getClass()); private final ProcessRunner RUNNER; @@@ -110,12 -110,12 +111,12 @@@ return ipmiToolCommands.build(); } ++ /** ++ * Expected usersList string contains legends on first line and users on rest ++ * ID Name Callin Link Auth IPMI Msg Channel Priv Limit ++ * 1 admin true true true ADMINISTRATOR ++ */ public String findIpmiUser(final String usersList, final String username) { -- /** -- * Expected usersList string contains legends on first line and users on rest -- * ID Name Callin Link Auth IPMI Msg Channel Priv Limit -- * 1 admin true true true ADMINISTRATOR -- */ // Assuming user 'ID' index on 1st position int idIndex = 0; @@@ -156,26 -156,32 +157,32 @@@ public OutOfBandManagementDriverResponse executeCommands(final List<String> commands, final Duration timeOut) { final ProcessResult result = RUNNER.executeCommands(commands, timeOut); - if (LOG.isTraceEnabled()) { + if (logger.isTraceEnabled()) { - List<String> cleanedCommands = new ArrayList<String>(); - int maskNextCommand = 0; - for (String command : commands) { - if (maskNextCommand > 0) { - cleanedCommands.add("**** "); - maskNextCommand--; - continue; - } - if (command.equalsIgnoreCase("-P")) { - maskNextCommand = 1; - } else if (command.toLowerCase().endsWith("password")) { - maskNextCommand = 2; - } - cleanedCommands.add(command); - } - logger.trace("Executed ipmitool process with commands: " + StringUtils.join(cleanedCommands, ", ") + - "\nIpmitool execution standard output: " + result.getStdOutput() + - "\nIpmitool execution error output: " + result.getStdError()); + List<String> cleanedCommands = getSanatisedCommandStrings(commands); - LOG.trace("Executed ipmitool process with commands: " + StringUtils.join(cleanedCommands, ", ") + - "\nIpmitool execution standard output: " + result.getStdOutput() + - "\nIpmitool execution error output: " + result.getStdError()); ++ logger.trace("Executed ipmitool process with commands: {}\nIpmitool execution standard output: {}\nIpmitool execution error output: {}", ++ StringUtils.join(cleanedCommands, ", "), ++ result.getStdOutput(), ++ result.getStdError()); } return new OutOfBandManagementDriverResponse(result.getStdOutput(), result.getStdError(), result.isSuccess()); } + - @NotNull + List<String> getSanatisedCommandStrings(List<String> commands) { - List<String> cleanedCommands = new ArrayList<String>(); ++ List<String> cleanedCommands = new ArrayList<>(); + int maskNextCommand = 0; + for (String command : commands) { + if (maskNextCommand > 0) { + cleanedCommands.add("**** "); + maskNextCommand--; + continue; + } + if (command.equalsIgnoreCase("-P")) { + maskNextCommand = 1; + } else if (command.toLowerCase().endsWith("password")) { + maskNextCommand = 2; + } + cleanedCommands.add(command); + } + return cleanedCommands; + } } diff --cc server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 496643f0bdf,e2babeacc71..15978bf1dc7 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@@ -366,15 -365,15 +366,15 @@@ public class VolumeApiServiceImpl exten VmWorkJobHandlerProxy _jobHandlerProxy = new VmWorkJobHandlerProxy(this); -- static final ConfigKey<Long> VmJobCheckInterval = new ConfigKey<Long>("Advanced", Long.class, "vm.job.check.interval", "3000", "Interval in milliseconds to check if the job is complete", false); ++ static final ConfigKey<Long> VmJobCheckInterval = new ConfigKey<>("Advanced", Long.class, "vm.job.check.interval", "3000", "Interval in milliseconds to check if the job is complete", false); -- static final ConfigKey<Boolean> VolumeUrlCheck = new ConfigKey<Boolean>("Advanced", Boolean.class, "volume.url.check", "true", ++ static final ConfigKey<Boolean> VolumeUrlCheck = new ConfigKey<>("Advanced", Boolean.class, "volume.url.check", "true", "Check the url for a volume before downloading it from the management server. Set to false when your management has no internet access.", true); -- public static final ConfigKey<Boolean> AllowUserExpungeRecoverVolume = new ConfigKey<Boolean>("Advanced", Boolean.class, "allow.user.expunge.recover.volume", "true", ++ public static final ConfigKey<Boolean> AllowUserExpungeRecoverVolume = new ConfigKey<>("Advanced", Boolean.class, "allow.user.expunge.recover.volume", "true", "Determines whether users can expunge or recover their volume", true, ConfigKey.Scope.Account); -- public static final ConfigKey<Boolean> MatchStoragePoolTagsWithDiskOffering = new ConfigKey<Boolean>("Advanced", Boolean.class, "match.storage.pool.tags.with.disk.offering", "true", ++ public static final ConfigKey<Boolean> MatchStoragePoolTagsWithDiskOffering = new ConfigKey<>("Advanced", Boolean.class, "match.storage.pool.tags.with.disk.offering", "true", "If true, volume's disk offering can be changed only with the matched storage tags", true, ConfigKey.Scope.Zone); public static final ConfigKey<Long> WaitDetachDevice = new ConfigKey<>( @@@ -388,7 -387,7 +388,7 @@@ public static ConfigKey<Long> storageTagRuleExecutionTimeout = new ConfigKey<>("Advanced", Long.class, "storage.tag.rule.execution.timeout", "2000", "The maximum runtime," + " in milliseconds, to execute a storage tag rule; if it is reached, a timeout will happen.", true); -- public static final ConfigKey<Boolean> AllowCheckAndRepairVolume = new ConfigKey<Boolean>("Advanced", Boolean.class, "volume.check.and.repair.leaks.before.use", "false", ++ public static final ConfigKey<Boolean> AllowCheckAndRepairVolume = new ConfigKey<>("Advanced", Boolean.class, "volume.check.and.repair.leaks.before.use", "false", "To check and repair the volume if it has any leaks before performing volume attach or VM start operations", true, ConfigKey.Scope.StoragePool); private final StateMachine2<Volume.State, Volume.Event, Volume> _volStateMachine; @@@ -637,7 -633,7 +637,7 @@@ return Transaction.execute(new TransactionCallbackWithException<VolumeVO, CloudRuntimeException>() { @Override public VolumeVO doInTransaction(TransactionStatus status) { -- VolumeVO volume = new VolumeVO(volumeName, zoneId, -1, -1, -1, new Long(-1), null, null, Storage.ProvisioningType.THIN, 0, Volume.Type.DATADISK); ++ VolumeVO volume = new VolumeVO(volumeName, zoneId, -1, -1, -1, -1L, null, null, Storage.ProvisioningType.THIN, 0, Volume.Type.DATADISK); DataCenter zone = _dcDao.findById(zoneId); volume.setPoolId(null); volume.setDataCenterId(zoneId); @@@ -839,7 -838,7 +839,7 @@@ maxIops = diskOffering.getMaxIops(); } -- if (!validateVolumeSizeInBytes(size)) { ++ if (!validateVolumeSizeInBytes(size == null ? 0 : size)) { throw new InvalidParameterValueException(String.format("Invalid size for custom volume creation: %s, max volume size is: %s GB", NumbersUtil.toReadableSize(size), VolumeOrchestrationService.MaxVolumeSize.value())); } } @@@ -3647,9 -3577,9 +3647,9 @@@ * </body> * </table> */ - protected boolean doesTargetStorageSupportDiskOffering(StoragePool destPool, DiskOfferingVO diskOffering) { - String targetStoreTags = diskOffering.getTags(); - return doesTargetStorageSupportDiskOffering(destPool, targetStoreTags); - protected boolean doesStoragePoolSupportDiskOffering(StoragePool destPool, DiskOfferingVO diskOffering) { - String offeringTags = diskOffering.getTags(); ++ public boolean doesStoragePoolSupportDiskOffering(StoragePool destPool, DiskOffering diskOffering) { ++ String offeringTags = diskOffering != null ? diskOffering.getTags() : null; + return doesStoragePoolSupportDiskOfferingTags(destPool, offeringTags); } public static boolean doesNewDiskOfferingHasTagsAsOldDiskOffering(DiskOfferingVO oldDO, DiskOfferingVO newDO) { @@@ -3669,15 -3599,15 +3669,15 @@@ Pair<List<String>, Boolean> storagePoolTags = getStoragePoolTags(destPool); if ((storagePoolTags == null || !storagePoolTags.second()) && org.apache.commons.lang.StringUtils.isBlank(diskOfferingTags)) { if (storagePoolTags == null) { - logger.debug(String.format("Destination storage pool [%s] does not have any tags, and so does the disk offering. Therefore, they are compatible", destPool.getUuid())); - s_logger.debug(String.format("Storage pool [%s] does not have any tags, and so does the disk offering. Therefore, they are compatible", destPool.getUuid())); ++ logger.debug("Storage pool [{}] does not have any tags, and so does the disk offering. Therefore, they are compatible", destPool.getUuid()); } else { - logger.debug("Destination storage pool has tags [%s], and the disk offering has no tags. Therefore, they are compatible."); - s_logger.debug("Storage pool has tags [%s], and the disk offering has no tags. Therefore, they are compatible."); ++ logger.debug("Storage pool has tags [%s], and the disk offering has no tags. Therefore, they are compatible.", destPool.getUuid()); } return true; } if (storagePoolTags == null || CollectionUtils.isEmpty(storagePoolTags.first())) { - logger.debug(String.format("Destination storage pool [%s] has no tags, while disk offering has tags [%s]. Therefore, they are not compatible", destPool.getUuid(), - s_logger.debug(String.format("Storage pool [%s] has no tags, while disk offering has tags [%s]. Therefore, they are not compatible", destPool.getUuid(), -- diskOfferingTags)); ++ logger.debug("Destination storage pool [{}] has no tags, while disk offering has tags [{}]. Therefore, they are not compatible", destPool.getUuid(), ++ diskOfferingTags); return false; } List<String> storageTagsList = storagePoolTags.first(); @@@ -3689,7 -3619,7 +3689,7 @@@ } else { result = CollectionUtils.isSubCollection(Arrays.asList(newDiskOfferingTagsAsStringArray), storageTagsList); } - logger.debug(String.format("Destination storage pool [%s] accepts tags [%s]? %s", destPool.getUuid(), diskOfferingTags, result)); - s_logger.debug(String.format("Storage pool [%s] accepts tags [%s]? %s", destPool.getUuid(), diskOfferingTags, result)); ++ logger.debug(String.format("Destination storage pool [{}] accepts tags [{}]? {}", destPool.getUuid(), diskOfferingTags, result)); return result; } diff --cc server/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java index aac5d1277a6,e48a6d413ad..9e1fc46e02e --- a/server/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java @@@ -79,7 -78,7 +79,7 @@@ import org.apache.logging.log4j.Logger import javax.inject.Inject; import java.util.ArrayList; --import java.util.Arrays; ++import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Map; @@@ -206,8 -205,8 +206,8 @@@ public class VolumeImportUnmanageManage if (diskOffering.isCustomized()) { volumeApiService.validateCustomDiskOfferingSizeRange(volume.getVirtualSize() / ByteScaleUtils.GiB); } - if (!volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOffering.getTags())) { - if (!volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, diskOffering.getTags())) { - logFailureAndThrowException(String.format("Disk offering: %s storage tags are not compatible with selected storage pool: %s", diskOffering.getUuid(), pool.getUuid())); ++ if (!volumeApiService.doesStoragePoolSupportDiskOffering(pool, diskOffering)) { + logFailureAndThrowException(String.format("Disk offering: %s storage tags are not compatible with selected storage pool: %s", diskOffering, pool)); } // 7. create records @@@ -248,8 -247,8 +248,8 @@@ StorageFilerTO storageTO = new StorageFilerTO(pool); GetVolumesOnStorageCommand command = new GetVolumesOnStorageCommand(storageTO, volumePath, keyword); Answer answer = agentManager.easySend(host.getId(), command); -- if (answer == null || !(answer instanceof GetVolumesOnStorageAnswer)) { - logFailureAndThrowException("Cannot get volumes on storage pool via host " + host.getName()); ++ if (!(answer instanceof GetVolumesOnStorageAnswer)) { + logFailureAndThrowException(String.format("Cannot get volumes on storage pool via host %s", host)); } if (!answer.getResult()) { logFailureAndThrowException("Volume cannot be imported due to " + answer.getDetails()); @@@ -366,7 -365,7 +366,7 @@@ } private boolean checkIfVolumeForSnapshot(StoragePoolVO pool, String fullVolumePath) { -- List<String> absPathList = Arrays.asList(fullVolumePath); ++ List<String> absPathList = Collections.singletonList(fullVolumePath); return CollectionUtils.isNotEmpty(snapshotDataStoreDao.listByStoreAndInstallPaths(pool.getId(), DataStoreRole.Primary, absPathList)); } diff --cc server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index d8b6acccd03,abb0e6b63c5..8d5387b3ffd --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@@ -460,7 -456,7 +460,7 @@@ public class UnmanagedVMsManagerImpl im if (diskOffering == null) { return false; } - return volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOffering.getTags()); - return volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, diskOffering.getTags()); ++ return volumeApiService.doesStoragePoolSupportDiskOffering(pool, diskOffering); } private ServiceOfferingVO getUnmanagedInstanceServiceOffering(final UnmanagedInstanceTO instance, ServiceOfferingVO serviceOffering, final Account owner, final DataCenter zone, final Map<String, String> details, Hypervisor.HypervisorType hypervisorType) @@@ -538,7 -536,7 +538,7 @@@ return nicIpAddresses; } -- private StoragePool getStoragePool(final UnmanagedInstanceTO.Disk disk, final DataCenter zone, final Cluster cluster, String diskOfferingTags) { ++ private StoragePool getStoragePool(final UnmanagedInstanceTO.Disk disk, final DataCenter zone, final Cluster cluster, DiskOffering diskOffering) { StoragePool storagePool = null; final String dsHost = disk.getDatastoreHost(); final String dsPath = disk.getDatastorePath(); @@@ -549,7 -547,7 +549,7 @@@ for (StoragePool pool : pools) { if (pool.getDataCenterId() == zone.getId() && (pool.getClusterId() == null || pool.getClusterId().equals(cluster.getId())) && - volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOfferingTags)) { - volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, diskOfferingTags)) { ++ volumeApiService.doesStoragePoolSupportDiskOffering(pool, diskOffering)) { storagePool = pool; break; } @@@ -562,7 -560,7 +562,7 @@@ for (StoragePool pool : pools) { String searchPoolParam = StringUtils.isNotBlank(dsPath) ? dsPath : dsName; if (StringUtils.contains(pool.getPath(), searchPoolParam) && - volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOfferingTags)) { - volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, diskOfferingTags)) { ++ volumeApiService.doesStoragePoolSupportDiskOffering(pool, diskOffering)) { storagePool = pool; break; } @@@ -626,7 -624,7 +626,7 @@@ throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Size of disk offering(ID: %s) %dGB is found less than the size of disk(ID: %s) %dGB during VM import", diskOffering.getUuid(), (diskOffering.getDiskSize() / Resource.ResourceType.bytesToGiB), disk.getDiskId(), (disk.getCapacity() / (Resource.ResourceType.bytesToGiB)))); } diskOffering = diskOffering != null ? diskOffering : diskOfferingDao.findById(serviceOffering.getDiskOfferingId()); -- StoragePool storagePool = getStoragePool(disk, zone, cluster, diskOffering != null ? diskOffering.getTags() : null); ++ StoragePool storagePool = getStoragePool(disk, zone, cluster, diskOffering); if (diskOffering != null && !migrateAllowed && !storagePoolSupportsDiskOffering(storagePool, diskOffering)) { throw new InvalidParameterValueException(String.format("Disk offering: %s is not compatible with storage pool: %s of unmanaged disk: %s", diskOffering.getUuid(), storagePool.getUuid(), disk.getDiskId())); } @@@ -863,7 -861,7 +863,7 @@@ diskInfo.setDiskChain(new String[]{disk.getImagePath()}); chainInfo = gson.toJson(diskInfo); } -- StoragePool storagePool = getStoragePool(disk, zone, cluster, diskOffering != null ? diskOffering.getTags() : null); ++ StoragePool storagePool = getStoragePool(disk, zone, cluster, diskOffering); DiskProfile profile = volumeManager.importVolume(type, name, diskOffering, diskSize, minIops, maxIops, vm.getDataCenterId(), vm.getHypervisorType(), vm, template, owner, deviceId, storagePool.getId(), path, chainInfo); @@@ -1992,12 -1944,13 +1992,12 @@@ DataStoreTO temporaryConvertLocation, String vcenterHost, String vcenterUsername, String vcenterPassword, String datacenterName ) { - LOGGER.debug(String.format("Delegating the conversion of instance %s from VMware to KVM to the host %s (%s) after OVF export through ovftool", - sourceVM, convertHost.getId(), convertHost.getName())); + logger.debug("Delegating the conversion of instance {} from VMware to KVM to the host {} after OVF export through ovftool", sourceVM, convertHost); - RemoteInstanceTO remoteInstanceTO = new RemoteInstanceTO(sourceVMwareInstance.getName(), vcenterHost, vcenterUsername, vcenterPassword, datacenterName); + RemoteInstanceTO remoteInstanceTO = new RemoteInstanceTO(sourceVMwareInstance.getName(), sourceVMwareInstance.getPath(), vcenterHost, vcenterUsername, vcenterPassword, datacenterName); List<String> destinationStoragePools = selectInstanceConversionStoragePools(convertStoragePools, sourceVMwareInstance.getDisks(), serviceOffering, dataDiskOfferingMap); ConvertInstanceCommand cmd = new ConvertInstanceCommand(remoteInstanceTO, - Hypervisor.HypervisorType.KVM, temporaryConvertLocation, null, false, true); + Hypervisor.HypervisorType.KVM, destinationStoragePools, temporaryConvertLocation, null, false, true); int timeoutSeconds = UnmanagedVMsManager.ConvertVmwareInstanceToKvmTimeout.value() * 60 * 60; cmd.setWait(timeoutSeconds); int noOfThreads = UnmanagedVMsManager.ThreadsOnKVMHostToImportVMwareVMFiles.value(); @@@ -2065,48 -2018,35 +2065,67 @@@ List<StoragePoolVO> pools = new ArrayList<>(); pools.addAll(primaryDataStoreDao.findClusterWideStoragePoolsByHypervisorAndPoolType(destinationCluster.getId(), Hypervisor.HypervisorType.KVM, Storage.StoragePoolType.NetworkFilesystem)); pools.addAll(primaryDataStoreDao.findZoneWideStoragePoolsByHypervisorAndPoolType(destinationCluster.getDataCenterId(), Hypervisor.HypervisorType.KVM, Storage.StoragePoolType.NetworkFilesystem)); + List<String> diskOfferingTags = new ArrayList<>(); + if (pools.isEmpty()) { + String msg = String.format("Cannot find suitable storage pools in the cluster %s for the conversion", destinationCluster.getName()); - LOGGER.error(msg); ++ logger.error(msg); + throw new CloudRuntimeException(msg); + } + + if (serviceOffering.getDiskOfferingId() != null) { + DiskOfferingVO diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId()); + if (diskOffering == null) { + String msg = String.format("Cannot find disk offering with ID %s that belongs to the service offering %s", serviceOffering.getDiskOfferingId(), serviceOffering.getName()); - LOGGER.error(msg); ++ logger.error(msg); + throw new CloudRuntimeException(msg); + } + if (getStoragePoolWithTags(pools, diskOffering.getTags()) == null) { + String msg = String.format("Cannot find suitable storage pool for disk offering %s that belongs to the service offering %s", diskOffering.getName(), serviceOffering.getName()); - LOGGER.error(msg); ++ logger.error(msg); + throw new CloudRuntimeException(msg); + } + } for (Long diskOfferingId : dataDiskOfferingMap.values()) { DiskOfferingVO diskOffering = diskOfferingDao.findById(diskOfferingId); if (diskOffering == null) { String msg = String.format("Cannot find disk offering with ID %s", diskOfferingId); - LOGGER.error(msg); + logger.error(msg); throw new CloudRuntimeException(msg); } - if (getStoragePoolWithTags(pools, diskOffering.getTags()) == null) { - String msg = String.format("Cannot find suitable storage pool for disk offering %s", diskOffering.getName()); - LOGGER.error(msg); + diskOfferingTags.add(diskOffering.getTags()); + } + if (serviceOffering.getDiskOfferingId() != null) { + DiskOfferingVO diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId()); + if (diskOffering != null) { + diskOfferingTags.add(diskOffering.getTags()); + } + } + + pools = getPoolsWithMatchingTags(pools, diskOfferingTags); + if (pools.isEmpty()) { + String msg = String.format("Cannot find suitable storage pools in cluster %s for the conversion", destinationCluster); + logger.error(msg); + throw new CloudRuntimeException(msg); + } + return pools; + } + + private List<StoragePoolVO> getPoolsWithMatchingTags(List<StoragePoolVO> pools, List<String> diskOfferingTags) { + if (diskOfferingTags.isEmpty()) { + return pools; + } + List<StoragePoolVO> poolsSupportingTags = new ArrayList<>(pools); + for (String tags : diskOfferingTags) { + boolean tagsMatched = false; + for (StoragePoolVO pool : pools) { - if (volumeApiService.doesTargetStorageSupportDiskOffering(pool, tags)) { ++ if (volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, tags)) { + poolsSupportingTags.add(pool); + tagsMatched = true; + } + } + if (!tagsMatched) { + String msg = String.format("Cannot find suitable storage pools for the conversion with disk offering tags %s", tags); + logger.error(msg); throw new CloudRuntimeException(msg); } } diff --cc server/src/test/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImplTest.java index 8982034a8c5,aed8f6a291b..419acc0ca0b --- a/server/src/test/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImplTest.java @@@ -275,7 -275,7 +275,7 @@@ public class VolumeImportUnmanageManage when(diskOffering.isCustomized()).thenReturn(true); doReturn(diskOffering).when(volumeImportUnmanageManager).getOrCreateDiskOffering(account, diskOfferingId, zoneId, isLocal); doNothing().when(volumeApiService).validateCustomDiskOfferingSizeRange(anyLong()); - doReturn(true).when(volumeApiService).doesTargetStorageSupportDiskOffering(any(), isNull()); - doReturn(true).when(volumeApiService).doesStoragePoolSupportDiskOfferingTags(any(), isNull()); ++ doReturn(true).when(volumeApiService).doesStoragePoolSupportDiskOffering(any(), any()); doReturn(diskProfile).when(volumeManager).importVolume(any(), anyString(), any(), eq(virtualSize), isNull(), isNull(), anyLong(), any(), isNull(), isNull(), any(), isNull(), anyLong(), anyString(), isNull()); when(diskProfile.getVolumeId()).thenReturn(volumeId); diff --cc server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index 92681e058a2,9bdc05fe95c..8cfae6b9a6e --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@@ -17,58 -17,6 +17,61 @@@ package org.apache.cloudstack.vm; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.nullable; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.net.URI; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; + ++import com.cloud.offering.DiskOffering; +import org.apache.cloudstack.api.ResponseGenerator; +import org.apache.cloudstack.api.ResponseObject; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.admin.vm.ImportUnmanagedInstanceCmd; +import org.apache.cloudstack.api.command.admin.vm.ImportVmCmd; +import org.apache.cloudstack.api.command.admin.vm.ListUnmanagedInstancesCmd; +import org.apache.cloudstack.api.command.admin.vm.ListVmsForImportCmd; +import org.apache.cloudstack.api.response.ListResponse; ++import org.apache.cloudstack.api.response.UnmanagedInstanceResponse; +import org.apache.cloudstack.api.response.UserVmResponse; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; +import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; +import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; ++import org.jetbrains.annotations.NotNull; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.BDDMockito; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.mockito.junit.MockitoJUnitRunner; + import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; import com.cloud.agent.api.CheckConvertInstanceAnswer; @@@ -135,7 -82,7 +138,6 @@@ import com.cloud.storage.Volume import com.cloud.storage.VolumeApiService; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; --import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VMTemplatePoolDao; @@@ -154,7 -101,7 +156,6 @@@ import com.cloud.utils.db.EntityManager import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.DiskProfile; import com.cloud.vm.NicProfile; --import com.cloud.vm.NicVO; import com.cloud.vm.UserVmManager; import com.cloud.vm.UserVmVO; import com.cloud.vm.VMInstanceVO; @@@ -162,7 -109,58 +163,6 @@@ import com.cloud.vm.VirtualMachine import com.cloud.vm.dao.NicDao; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; --import com.cloud.vm.snapshot.dao.VMSnapshotDao; -import org.apache.cloudstack.api.ResponseGenerator; -import org.apache.cloudstack.api.ResponseObject; -import org.apache.cloudstack.api.ServerApiException; -import org.apache.cloudstack.api.command.admin.vm.ImportUnmanagedInstanceCmd; -import org.apache.cloudstack.api.command.admin.vm.ImportVmCmd; -import org.apache.cloudstack.api.command.admin.vm.ListUnmanagedInstancesCmd; -import org.apache.cloudstack.api.command.admin.vm.ListVmsForImportCmd; -import org.apache.cloudstack.api.response.ListResponse; -import org.apache.cloudstack.api.response.UserVmResponse; -import org.apache.cloudstack.context.CallContext; -import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; -import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService; -import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; -import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; -import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; -import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; -import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; -import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.junit.After; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.BDDMockito; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.MockedStatic; -import org.mockito.Mockito; -import org.mockito.MockitoAnnotations; -import org.mockito.junit.MockitoJUnitRunner; - -import java.net.URI; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.UUID; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.ArgumentMatchers.anyLong; -import static org.mockito.ArgumentMatchers.anyMap; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.nullable; -import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class UnmanagedVMsManagerImplTest { @@@ -175,10 -173,10 +175,6 @@@ @Mock private ClusterDao clusterDao; @Mock -- private ClusterVO clusterVO; -- @Mock -- private UserVmVO userVm; -- @Mock private ResourceManager resourceManager; @Mock private VMTemplatePoolDao templatePoolDao; @@@ -219,10 -217,10 +215,6 @@@ @Mock private ConfigurationDao configurationDao; @Mock -- private VMSnapshotDao vmSnapshotDao; -- @Mock -- private SnapshotDao snapshotDao; -- @Mock private UserVmDao userVmDao; @Mock private NicDao nicDao; @@@ -242,8 -240,8 +234,6 @@@ @Mock private VMInstanceVO virtualMachine; @Mock -- private NicVO nicVO; -- @Mock EntityManager entityMgr; @Mock DeploymentPlanningManager deploymentPlanningManager; @@@ -276,19 -274,19 +266,7 @@@ instance.setCpuSpeed(1000); instance.setMemory(1024); instance.setOperatingSystem("CentOS 7"); -- List<UnmanagedInstanceTO.Disk> instanceDisks = new ArrayList<>(); -- UnmanagedInstanceTO.Disk instanceDisk = new UnmanagedInstanceTO.Disk(); -- instanceDisk.setDiskId("1000-1"); -- instanceDisk.setPosition(0); -- instanceDisk.setLabel("DiskLabel"); -- instanceDisk.setController("scsi"); -- instanceDisk.setImagePath("[b6ccf44a1fa13e29b3667b4954fa10ee] TestInstance/ROOT-1.vmdk"); -- instanceDisk.setCapacity(5242880L); -- instanceDisk.setDatastoreName("Test"); -- instanceDisk.setDatastoreHost("Test"); -- instanceDisk.setDatastorePath("Test"); -- instanceDisk.setDatastoreType("NFS"); -- instanceDisks.add(instanceDisk); ++ List<UnmanagedInstanceTO.Disk> instanceDisks = getDisks(); instance.setDisks(instanceDisks); List<UnmanagedInstanceTO.Nic> instanceNics = new ArrayList<>(); UnmanagedInstanceTO.Nic instanceNic = new UnmanagedInstanceTO.Nic(); @@@ -388,6 -387,6 +366,24 @@@ when(virtualMachine.getState()).thenReturn(VirtualMachine.State.Running); } ++ @NotNull ++ private static List<UnmanagedInstanceTO.Disk> getDisks() { ++ List<UnmanagedInstanceTO.Disk> instanceDisks = new ArrayList<>(); ++ UnmanagedInstanceTO.Disk instanceDisk = new UnmanagedInstanceTO.Disk(); ++ instanceDisk.setDiskId("1000-1"); ++ instanceDisk.setPosition(0); ++ instanceDisk.setLabel("DiskLabel"); ++ instanceDisk.setController("scsi"); ++ instanceDisk.setImagePath("[b6ccf44a1fa13e29b3667b4954fa10ee] TestInstance/ROOT-1.vmdk"); ++ instanceDisk.setCapacity(5242880L); ++ instanceDisk.setDatastoreName("Test"); ++ instanceDisk.setDatastoreHost("Test"); ++ instanceDisk.setDatastorePath("Test"); ++ instanceDisk.setDatastoreType("NFS"); ++ instanceDisks.add(instanceDisk); ++ return instanceDisks; ++ } ++ @After public void tearDown() throws Exception { closeable.close(); @@@ -425,7 -424,7 +421,7 @@@ ImportUnmanagedInstanceCmd importUnmanageInstanceCmd = Mockito.mock(ImportUnmanagedInstanceCmd.class); when(importUnmanageInstanceCmd.getName()).thenReturn("TestInstance"); when(importUnmanageInstanceCmd.getDomainId()).thenReturn(null); - when(volumeApiService.doesTargetStorageSupportDiskOffering(any(StoragePool.class), any())).thenReturn(true); - when(volumeApiService.doesStoragePoolSupportDiskOfferingTags(any(StoragePool.class), any())).thenReturn(true); ++ when(volumeApiService.doesStoragePoolSupportDiskOffering(any(StoragePool.class), any())).thenReturn(true); try (MockedStatic<UsageEventUtils> ignored = Mockito.mockStatic(UsageEventUtils.class)) { unmanagedVMsManager.importUnmanagedInstance(importUnmanageInstanceCmd); } @@@ -489,7 -488,7 +485,7 @@@ when(cmd.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM.toString()); when(cmd.getUsername()).thenReturn("user"); when(cmd.getPassword()).thenReturn("pass"); -- ListResponse response = unmanagedVMsManager.listVmsForImport(cmd); ++ ListResponse<UnmanagedInstanceResponse> response = unmanagedVMsManager.listVmsForImport(cmd); Assert.assertEquals(1, response.getCount().intValue()); } @@@ -586,7 -585,7 +582,6 @@@ boolean selectTemporaryStorage) throws OperationTimedoutException, AgentUnavailableException { long clusterId = 1L; long zoneId = 1L; -- long podId = 1L; long existingDatacenterId = 1L; String vcenterHost = "192.168.1.2"; String datacenter = "Datacenter"; @@@ -704,7 -704,7 +699,8 @@@ when(agentManager.send(Mockito.eq(convertHostId), Mockito.any(CheckConvertInstanceCommand.class))).thenReturn(checkConvertInstanceAnswer); } - when(volumeApiService.doesTargetStorageSupportDiskOffering(any(StoragePool.class), any())).thenReturn(true); ++ when(volumeApiService.doesStoragePoolSupportDiskOffering(any(StoragePool.class), any(DiskOffering.class))).thenReturn(true); + when(volumeApiService.doesStoragePoolSupportDiskOfferingTags(any(StoragePool.class), any())).thenReturn(true); ConvertInstanceAnswer convertInstanceAnswer = mock(ConvertInstanceAnswer.class); ImportConvertedInstanceAnswer convertImportedInstanceAnswer = mock(ImportConvertedInstanceAnswer.class); @@@ -727,16 -726,16 +723,16 @@@ } @Test -- public void testImportFromLocalDisk() throws InsufficientServerCapacityException { -- testImportFromDisk("local"); ++ public void importFromLocalDisk() throws InsufficientServerCapacityException { ++ importFromDisk("local"); } @Test -- public void testImportFromsharedStorage() throws InsufficientServerCapacityException { -- testImportFromDisk("shared"); ++ public void importFromsharedStorage() throws InsufficientServerCapacityException { ++ importFromDisk("shared"); } -- private void testImportFromDisk(String source) throws InsufficientServerCapacityException { ++ private void importFromDisk(String source) throws InsufficientServerCapacityException { String vmname = "testVm"; ImportVmCmd cmd = Mockito.mock(ImportVmCmd.class); when(cmd.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM.toString()); @@@ -770,7 -769,7 +766,7 @@@ storagePools.add(storagePool); when(primaryDataStoreDao.findLocalStoragePoolsByHostAndTags(anyLong(), any())).thenReturn(storagePools); when(primaryDataStoreDao.findById(anyLong())).thenReturn(storagePool); - when(volumeApiService.doesTargetStorageSupportDiskOffering(any(StoragePool.class), any())).thenReturn(true); - when(volumeApiService.doesStoragePoolSupportDiskOfferingTags(any(StoragePool.class), any())).thenReturn(true); ++ when(volumeApiService.doesStoragePoolSupportDiskOffering(any(StoragePool.class), any())).thenReturn(true); StoragePoolHostVO storagePoolHost = Mockito.mock(StoragePoolHostVO.class); when(storagePoolHostDao.findByPoolHost(anyLong(), anyLong())).thenReturn(storagePoolHost); try (MockedStatic<UsageEventUtils> ignored = Mockito.mockStatic(UsageEventUtils.class)) { diff --cc vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/BaseMO.java index 888af714f31,27ca6396a47..c0e7f29375f --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/BaseMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/BaseMO.java @@@ -63,9 -63,12 +64,12 @@@ public class BaseMO } protected static Pair<String, List<ObjectContent>> createReturnObjectPair(RetrieveResult result) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("vmware result : " + ReflectionToStringBuilderUtils.reflectCollection(result)); + if (logger.isDebugEnabled()) { + logger.debug("vmware result : {} ", ReflectionToStringBuilderUtils.reflectCollection(result)); } + if (result == null) { + return new Pair<>(null, new ArrayList<>()); + } String tokenForRetrievingNewResults = result.getToken(); List<ObjectContent> listOfObjects = result.getObjects(); return new Pair<>(tokenForRetrievingNewResults, listOfObjects);
