From d0f3233fda33dbd3a7e2fd52dd3d42831cc1f1d4 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 10 Nov 2023 18:26:05 +0530 Subject: [PATCH 01/39] edge-zone,kvm,iso,cks: allow k8s deployment with direct-download iso (#8142) Signed-off-by: Abhishek Kumar --- .../api/command/user/iso/RegisterIsoCmd.java | 7 +++ .../image/TemplateDataFactoryImpl.java | 49 +++++++-------- .../kvm/storage/KVMStoragePoolManager.java | 6 +- .../kvm/storage/KVMStorageProcessor.java | 59 +++++++++++-------- .../cluster/KubernetesClusterManagerImpl.java | 15 ++++- .../version/KubernetesVersionManagerImpl.java | 20 +++++-- .../AddKubernetesSupportedVersionCmd.java | 10 +++- .../KubernetesSupportedVersionResponse.java | 8 +++ .../cloud/network/router/NetworkHelper.java | 3 + .../network/router/NetworkHelperImpl.java | 7 ++- .../views/compute/CreateKubernetesCluster.vue | 8 ++- .../image/AddKubernetesSupportedVersion.vue | 56 +++++++++++++----- 12 files changed, 174 insertions(+), 74 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/RegisterIsoCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/RegisterIsoCmd.java index 47018b3b38d..bdb51e849e6 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/RegisterIsoCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/RegisterIsoCmd.java @@ -177,6 +177,9 @@ public class RegisterIsoCmd extends BaseCmd implements UserCmd { } public Long getZoneId() { + if (zoneId == null || zoneId == -1) { + return null; + } return zoneId; } @@ -220,6 +223,10 @@ public class RegisterIsoCmd extends BaseCmd implements UserCmd { return directDownload == null ? false : directDownload; } + public void setDirectDownload(Boolean directDownload) { + this.directDownload = directDownload; + } + public boolean isPasswordEnabled() { return passwordEnabled == null ? false : passwordEnabled; } diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateDataFactoryImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateDataFactoryImpl.java index 492ec74382b..8951b9d7c24 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateDataFactoryImpl.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateDataFactoryImpl.java @@ -20,11 +20,10 @@ package org.apache.cloudstack.storage.image; import java.util.ArrayList; import java.util.List; +import java.util.stream.Collectors; import javax.inject.Inject; -import com.cloud.hypervisor.Hypervisor; -import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.direct.download.DirectDownloadManager; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; @@ -36,18 +35,21 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO; import org.apache.cloudstack.storage.image.store.TemplateObject; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; +import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.DataStoreRole; import com.cloud.storage.VMTemplateStoragePoolVO; import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VMTemplatePoolDao; +import com.cloud.utils.exception.CloudRuntimeException; @Component public class TemplateDataFactoryImpl implements TemplateDataFactory { @@ -203,12 +205,7 @@ public class TemplateDataFactoryImpl implements TemplateDataFactory { * Given existing spool refs, return one pool id existing on pools and refs */ private Long getOneMatchingPoolIdFromRefs(List existingRefs, List pools) { - if (pools.isEmpty()) { - throw new CloudRuntimeException("No storage pools found"); - } - if (existingRefs.isEmpty()) { - return pools.get(0).getId(); - } else { + if (!existingRefs.isEmpty()) { for (VMTemplateStoragePoolVO ref : existingRefs) { for (StoragePoolVO p : pools) { if (ref.getPoolId() == p.getId()) { @@ -217,45 +214,51 @@ public class TemplateDataFactoryImpl implements TemplateDataFactory { } } } - return null; + return pools.get(0).getId(); } /** - * Retrieve storage pools with scope = cluster or zone matching clusterId or dataCenterId depending on their scope + * Retrieve storage pools with scope = cluster or zone or local matching clusterId or dataCenterId or hostId depending on their scope */ - private List getStoragePoolsFromClusterOrZone(Long clusterId, long dataCenterId, Hypervisor.HypervisorType hypervisorType) { + private List getStoragePoolsForScope(long dataCenterId, Long clusterId, long hostId, Hypervisor.HypervisorType hypervisorType) { List pools = new ArrayList<>(); if (clusterId != null) { List clusterPools = primaryDataStoreDao.listPoolsByCluster(clusterId); + clusterPools = clusterPools.stream().filter(p -> !p.isLocal()).collect(Collectors.toList()); pools.addAll(clusterPools); } List zonePools = primaryDataStoreDao.findZoneWideStoragePoolsByHypervisor(dataCenterId, hypervisorType); pools.addAll(zonePools); + List localPools = primaryDataStoreDao.findLocalStoragePoolsByHostAndTags(hostId, null); + pools.addAll(localPools); return pools; } + protected Long getBypassedTemplateExistingOrNewPoolId(VMTemplateVO templateVO, Long hostId) { + HostVO host = hostDao.findById(hostId); + List pools = getStoragePoolsForScope(host.getDataCenterId(), host.getClusterId(), hostId, host.getHypervisorType()); + if (CollectionUtils.isEmpty(pools)) { + throw new CloudRuntimeException(String.format("No storage pool found to download template: %s", templateVO.getName())); + } + List existingRefs = templatePoolDao.listByTemplateId(templateVO.getId()); + return getOneMatchingPoolIdFromRefs(existingRefs, pools); + } + @Override public TemplateInfo getReadyBypassedTemplateOnPrimaryStore(long templateId, Long poolId, Long hostId) { VMTemplateVO templateVO = imageDataDao.findById(templateId); if (templateVO == null || !templateVO.isDirectDownload()) { return null; } - Long pool = poolId; + Long templatePoolId = poolId; if (poolId == null) { - //Get ISO from existing pool ref - HostVO host = hostDao.findById(hostId); - List pools = getStoragePoolsFromClusterOrZone(host.getClusterId(), host.getDataCenterId(), host.getHypervisorType()); - List existingRefs = templatePoolDao.listByTemplateId(templateId); - pool = getOneMatchingPoolIdFromRefs(existingRefs, pools); + templatePoolId = getBypassedTemplateExistingOrNewPoolId(templateVO, hostId); } - if (pool == null) { - throw new CloudRuntimeException("No storage pool found where to download template: " + templateId); - } - VMTemplateStoragePoolVO spoolRef = templatePoolDao.findByPoolTemplate(pool, templateId, null); + VMTemplateStoragePoolVO spoolRef = templatePoolDao.findByPoolTemplate(templatePoolId, templateId, null); if (spoolRef == null) { - directDownloadManager.downloadTemplate(templateId, pool, hostId); + directDownloadManager.downloadTemplate(templateId, templatePoolId, hostId); } - DataStore store = storeMgr.getDataStore(pool, DataStoreRole.Primary); + DataStore store = storeMgr.getDataStore(templatePoolId, DataStoreRole.Primary); return this.getTemplate(templateId, store); } 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 bfaa799e134..79c7e2a488a 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 @@ -294,12 +294,14 @@ public class KVMStoragePoolManager { String uuid = null; String sourceHost = ""; StoragePoolType protocol = null; - if (storageUri.getScheme().equalsIgnoreCase("nfs") || storageUri.getScheme().equalsIgnoreCase("NetworkFilesystem")) { + final String scheme = storageUri.getScheme().toLowerCase(); + List acceptedSchemes = List.of("nfs", "networkfilesystem", "filesystem"); + if (acceptedSchemes.contains(scheme)) { sourcePath = storageUri.getPath(); sourcePath = sourcePath.replace("//", "/"); sourceHost = storageUri.getHost(); uuid = UUID.nameUUIDFromBytes(new String(sourceHost + sourcePath).getBytes()).toString(); - protocol = StoragePoolType.NetworkFilesystem; + protocol = scheme.equals("filesystem") ? StoragePoolType.Filesystem: StoragePoolType.NetworkFilesystem; } // secondary storage registers itself through here diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index f7ec09ca50f..dd31025d35f 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -25,23 +25,26 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.Arrays; import java.util.Date; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; +import java.util.stream.Collectors; import javax.naming.ConfigurationException; -import org.apache.cloudstack.direct.download.DirectDownloadHelper; -import org.apache.cloudstack.direct.download.DirectTemplateDownloader; -import com.cloud.storage.ScopeType; -import com.cloud.storage.Volume; import org.apache.cloudstack.agent.directdownload.DirectDownloadAnswer; import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand; +import org.apache.cloudstack.direct.download.DirectDownloadHelper; +import org.apache.cloudstack.direct.download.DirectTemplateDownloader; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.storage.command.AttachAnswer; import org.apache.cloudstack.storage.command.AttachCommand; @@ -71,7 +74,10 @@ import org.apache.cloudstack.utils.qemu.QemuImgFile; import org.apache.cloudstack.utils.qemu.QemuObject; import org.apache.commons.collections.MapUtils; import org.apache.commons.io.FileUtils; - +import org.apache.commons.lang3.BooleanUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.builder.ToStringBuilder; +import org.apache.commons.lang3.builder.ToStringStyle; import org.apache.log4j.Logger; import org.libvirt.Connect; import org.libvirt.Domain; @@ -110,9 +116,11 @@ import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef.DiskProtocol; import com.cloud.hypervisor.kvm.resource.wrapper.LibvirtUtilitiesHelper; import com.cloud.storage.JavaStorageLayer; import com.cloud.storage.MigrationOptions; +import com.cloud.storage.ScopeType; import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StorageLayer; +import com.cloud.storage.Volume; import com.cloud.storage.resource.StorageProcessor; import com.cloud.storage.template.Processor; import com.cloud.storage.template.Processor.FormatInfo; @@ -126,16 +134,6 @@ import com.cloud.utils.script.Script; import com.cloud.utils.storage.S3.S3Utils; import com.cloud.vm.VmDetailConstants; -import java.nio.file.Files; -import java.nio.file.Paths; -import java.util.HashSet; -import java.util.Set; -import java.util.stream.Collectors; -import org.apache.commons.lang3.BooleanUtils; -import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.builder.ToStringBuilder; -import org.apache.commons.lang3.builder.ToStringStyle; - public class KVMStorageProcessor implements StorageProcessor { private static final Logger s_logger = Logger.getLogger(KVMStorageProcessor.class); private final KVMStoragePoolManager storagePoolMgr; @@ -1074,7 +1072,8 @@ public class KVMStorageProcessor implements StorageProcessor { s_logger.debug(String.format("This backup is temporary, not deleting snapshot [%s] on primary storage [%s]", snapshotPath, primaryPool.getUuid())); } } - protected synchronized void attachOrDetachISO(final Connect conn, final String vmName, String isoPath, final boolean isAttach, Map params) throws + + protected synchronized void attachOrDetachISO(final Connect conn, final String vmName, String isoPath, final boolean isAttach, Map params, DataStoreTO store) throws LibvirtException, InternalErrorException { DiskDef iso = new DiskDef(); boolean isUefiEnabled = MapUtils.isNotEmpty(params) && params.containsKey("UEFI"); @@ -1082,8 +1081,14 @@ public class KVMStorageProcessor implements StorageProcessor { final int index = isoPath.lastIndexOf("/"); final String path = isoPath.substring(0, index); final String name = isoPath.substring(index + 1); - final KVMStoragePool secondaryPool = storagePoolMgr.getStoragePoolByURI(path); - final KVMPhysicalDisk isoVol = secondaryPool.getPhysicalDisk(name); + KVMStoragePool storagePool; + if (store instanceof PrimaryDataStoreTO) { + PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO)store; + storagePool = storagePoolMgr.getStoragePool(primaryDataStoreTO.getPoolType(), store.getUuid()); + } else { + storagePool = storagePoolMgr.getStoragePoolByURI(path); + } + final KVMPhysicalDisk isoVol = storagePool.getPhysicalDisk(name); isoPath = isoVol.getPath(); iso.defISODisk(isoPath, isUefiEnabled); @@ -1112,7 +1117,7 @@ public class KVMStorageProcessor implements StorageProcessor { try { String dataStoreUrl = getDataStoreUrlFromStore(store); final Connect conn = LibvirtConnection.getConnectionByVmName(cmd.getVmName()); - attachOrDetachISO(conn, cmd.getVmName(), dataStoreUrl + File.separator + isoTO.getPath(), true, cmd.getControllerInfo()); + attachOrDetachISO(conn, cmd.getVmName(), dataStoreUrl + File.separator + isoTO.getPath(), true, cmd.getControllerInfo(), store); } catch (final LibvirtException e) { return new Answer(cmd, false, e.toString()); } catch (final InternalErrorException e) { @@ -1133,7 +1138,7 @@ public class KVMStorageProcessor implements StorageProcessor { try { String dataStoreUrl = getDataStoreUrlFromStore(store); final Connect conn = LibvirtConnection.getConnectionByVmName(cmd.getVmName()); - attachOrDetachISO(conn, cmd.getVmName(), dataStoreUrl + File.separator + isoTO.getPath(), false, cmd.getParams()); + attachOrDetachISO(conn, cmd.getVmName(), dataStoreUrl + File.separator + isoTO.getPath(), false, cmd.getParams(), store); } catch (final LibvirtException e) { return new Answer(cmd, false, e.toString()); } catch (final InternalErrorException e) { @@ -1149,19 +1154,25 @@ public class KVMStorageProcessor implements StorageProcessor { * Return data store URL from store */ private String getDataStoreUrlFromStore(DataStoreTO store) { - if (!(store instanceof NfsTO) && (!(store instanceof PrimaryDataStoreTO) || - store instanceof PrimaryDataStoreTO && !((PrimaryDataStoreTO) store).getPoolType().equals(StoragePoolType.NetworkFilesystem))) { + List supportedPoolType = List.of(StoragePoolType.NetworkFilesystem, StoragePoolType.Filesystem); + if (!(store instanceof NfsTO) && (!(store instanceof PrimaryDataStoreTO) || !supportedPoolType.contains(((PrimaryDataStoreTO) store).getPoolType()))) { + s_logger.error(String.format("Unsupported protocol, store: %s", store.getUuid())); throw new InvalidParameterValueException("unsupported protocol"); } if (store instanceof NfsTO) { NfsTO nfsStore = (NfsTO)store; return nfsStore.getUrl(); - } else if (store instanceof PrimaryDataStoreTO && ((PrimaryDataStoreTO) store).getPoolType().equals(StoragePoolType.NetworkFilesystem)) { + } else if (store instanceof PrimaryDataStoreTO) { //In order to support directly downloaded ISOs + StoragePoolType poolType = ((PrimaryDataStoreTO)store).getPoolType(); String psHost = ((PrimaryDataStoreTO) store).getHost(); String psPath = ((PrimaryDataStoreTO) store).getPath(); - return "nfs://" + psHost + File.separator + psPath; + if (StoragePoolType.NetworkFilesystem.equals(poolType)) { + return "nfs://" + psHost + File.separator + psPath; + } else if (StoragePoolType.Filesystem.equals(poolType)) { + return StoragePoolType.Filesystem.toString().toLowerCase() + "://" + psHost + File.separator + psPath; + } } return store.getUrl(); } diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java index f0fa335d22c..41ad7981e5d 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java @@ -118,6 +118,7 @@ import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; import com.cloud.network.dao.PhysicalNetworkDao; +import com.cloud.network.router.NetworkHelper; import com.cloud.network.rules.FirewallRule; import com.cloud.network.rules.FirewallRuleVO; import com.cloud.network.security.SecurityGroupManager; @@ -243,6 +244,8 @@ public class KubernetesClusterManagerImpl extends ManagerBase implements Kuberne private SecurityGroupManager securityGroupManager; @Inject public SecurityGroupService securityGroupService; + @Inject + public NetworkHelper networkHelper; private void logMessage(final Level logLevel, final String message, final Exception e) { if (logLevel == Level.WARN) { @@ -347,8 +350,12 @@ public class KubernetesClusterManagerImpl extends ManagerBase implements Kuberne public VMTemplateVO getKubernetesServiceTemplate(DataCenter dataCenter, Hypervisor.HypervisorType hypervisorType) { VMTemplateVO template = templateDao.findSystemVMReadyTemplate(dataCenter.getId(), hypervisorType); + if (DataCenter.Type.Edge.equals(dataCenter.getType()) && template != null && !template.isDirectDownload()) { + LOGGER.debug(String.format("Template %s can not be used for edge zone %s", template, dataCenter)); + template = templateDao.findRoutingTemplate(hypervisorType, networkHelper.getHypervisorRouterTemplateConfigMap().get(hypervisorType).valueIn(dataCenter.getId())); + } if (template == null) { - throw new CloudRuntimeException("Not able to find the System templates or not downloaded in zone " + dataCenter.getId()); + throw new CloudRuntimeException("Not able to find the System or Routing template in ready state for the zone " + dataCenter.getUuid()); } return template; } @@ -650,7 +657,11 @@ public class KubernetesClusterManagerImpl extends ManagerBase implements Kuberne } if (Grouping.AllocationState.Disabled == zone.getAllocationState()) { - throw new PermissionDeniedException(String.format("Cannot perform this operation, zone ID: %s is currently disabled", zone.getUuid())); + throw new PermissionDeniedException(String.format("Zone ID: %s is currently disabled", zone.getUuid())); + } + + if (DataCenter.Type.Edge.equals(zone.getType()) && networkId == null) { + throw new PermissionDeniedException("Kubernetes clusters cannot be created on an edge zone without an existing network"); } if (!isKubernetesServiceConfigured(zone)) { diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/version/KubernetesVersionManagerImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/version/KubernetesVersionManagerImpl.java index 643044f78d8..3ea30291f43 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/version/KubernetesVersionManagerImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/version/KubernetesVersionManagerImpl.java @@ -31,10 +31,12 @@ import org.apache.cloudstack.api.command.user.iso.RegisterIsoCmd; import org.apache.cloudstack.api.command.user.kubernetes.version.ListKubernetesSupportedVersionsCmd; import org.apache.cloudstack.api.response.KubernetesSupportedVersionResponse; import org.apache.cloudstack.api.response.ListResponse; +import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import com.cloud.api.query.dao.TemplateJoinDao; import com.cloud.api.query.vo.TemplateJoinVO; +import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.DataCenterDao; import com.cloud.event.ActionEvent; @@ -56,7 +58,6 @@ import com.cloud.utils.db.Filter; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.exception.CloudRuntimeException; -import org.apache.commons.lang3.StringUtils; public class KubernetesVersionManagerImpl extends ManagerBase implements KubernetesVersionService { public static final Logger LOGGER = Logger.getLogger(KubernetesVersionManagerImpl.class.getName()); @@ -104,6 +105,7 @@ public class KubernetesVersionManagerImpl extends ManagerBase implements Kuberne response.setIsoId(template.getUuid()); response.setIsoName(template.getName()); response.setIsoState(template.getState().toString()); + response.setDirectDownload(template.isDirectDownload()); } response.setCreated(kubernetesSupportedVersion.getCreated()); return response; @@ -147,7 +149,7 @@ public class KubernetesVersionManagerImpl extends ManagerBase implements Kuberne return versions; } - private VirtualMachineTemplate registerKubernetesVersionIso(final Long zoneId, final String versionName, final String isoUrl, final String isoChecksum)throws IllegalAccessException, NoSuchFieldException, + private VirtualMachineTemplate registerKubernetesVersionIso(final Long zoneId, final String versionName, final String isoUrl, final String isoChecksum, final boolean directDownload) throws IllegalAccessException, NoSuchFieldException, IllegalArgumentException, ResourceAllocationException { String isoName = String.format("%s-Kubernetes-Binaries-ISO", versionName); RegisterIsoCmd registerIsoCmd = new RegisterIsoCmd(); @@ -163,6 +165,7 @@ public class KubernetesVersionManagerImpl extends ManagerBase implements Kuberne if (StringUtils.isNotEmpty(isoChecksum)) { registerIsoCmd.setChecksum(isoChecksum); } + registerIsoCmd.setDirectDownload(directDownload); registerIsoCmd.setAccountName(accountManager.getSystemAccount().getAccountName()); registerIsoCmd.setDomainId(accountManager.getSystemAccount().getDomainId()); return templateService.registerIso(registerIsoCmd); @@ -288,6 +291,7 @@ public class KubernetesVersionManagerImpl extends ManagerBase implements Kuberne final String isoChecksum = cmd.getChecksum(); final Integer minimumCpu = cmd.getMinimumCpu(); final Integer minimumRamSize = cmd.getMinimumRamSize(); + final boolean isDirectDownload = cmd.isDirectDownload(); if (minimumCpu == null || minimumCpu < KubernetesClusterService.MIN_KUBERNETES_CLUSTER_NODE_CPU) { throw new InvalidParameterValueException(String.format("Invalid value for %s parameter. Minimum %d vCPUs required.", ApiConstants.MIN_CPU_NUMBER, KubernetesClusterService.MIN_KUBERNETES_CLUSTER_NODE_CPU)); } @@ -297,8 +301,14 @@ public class KubernetesVersionManagerImpl extends ManagerBase implements Kuberne if (compareSemanticVersions(semanticVersion, MIN_KUBERNETES_VERSION) < 0) { throw new InvalidParameterValueException(String.format("New supported Kubernetes version cannot be added as %s is minimum version supported by Kubernetes Service", MIN_KUBERNETES_VERSION)); } - if (zoneId != null && dataCenterDao.findById(zoneId) == null) { - throw new InvalidParameterValueException("Invalid zone specified"); + if (zoneId != null) { + DataCenter zone = dataCenterDao.findById(zoneId); + if (zone == null) { + throw new InvalidParameterValueException("Invalid zone specified"); + } + if (DataCenter.Type.Edge.equals(zone.getType()) && !isDirectDownload) { + throw new InvalidParameterValueException(String.format("Zone: %s supports only direct download Kubernetes versions", zone.getName())); + } } if (StringUtils.isEmpty(isoUrl)) { throw new InvalidParameterValueException(String.format("Invalid URL for ISO specified, %s", isoUrl)); @@ -312,7 +322,7 @@ public class KubernetesVersionManagerImpl extends ManagerBase implements Kuberne VMTemplateVO template = null; try { - VirtualMachineTemplate vmTemplate = registerKubernetesVersionIso(zoneId, name, isoUrl, isoChecksum); + VirtualMachineTemplate vmTemplate = registerKubernetesVersionIso(zoneId, name, isoUrl, isoChecksum, isDirectDownload); template = templateDao.findById(vmTemplate.getId()); } catch (IllegalAccessException | NoSuchFieldException | IllegalArgumentException | ResourceAllocationException ex) { LOGGER.error(String.format("Unable to register binaries ISO for supported kubernetes version, %s, with url: %s", name, isoUrl), ex); diff --git a/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/admin/kubernetes/version/AddKubernetesSupportedVersionCmd.java b/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/admin/kubernetes/version/AddKubernetesSupportedVersionCmd.java index 48bb26c8d44..380c93cca20 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/admin/kubernetes/version/AddKubernetesSupportedVersionCmd.java +++ b/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/command/admin/kubernetes/version/AddKubernetesSupportedVersionCmd.java @@ -31,6 +31,7 @@ import org.apache.cloudstack.api.command.admin.AdminCmd; import org.apache.cloudstack.api.response.KubernetesSupportedVersionResponse; import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.cloudstack.context.CallContext; +import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import com.cloud.exception.ConcurrentOperationException; @@ -38,7 +39,6 @@ import com.cloud.exception.InvalidParameterValueException; import com.cloud.kubernetes.version.KubernetesSupportedVersion; import com.cloud.kubernetes.version.KubernetesVersionService; import com.cloud.utils.exception.CloudRuntimeException; -import org.apache.commons.lang3.StringUtils; @APICommand(name = "addKubernetesSupportedVersion", description = "Add a supported Kubernetes version", @@ -84,6 +84,10 @@ public class AddKubernetesSupportedVersionCmd extends BaseCmd implements AdminCm description = "the minimum RAM size in MB to be set with the Kubernetes version") private Integer minimumRamSize; + @Parameter(name=ApiConstants.DIRECT_DOWNLOAD, type = CommandType.BOOLEAN, since="4.18.2", + description = "If set to true the Kubernetes supported version ISO will bypass Secondary Storage and be downloaded to Primary Storage on deployment. Default is false") + private Boolean directDownload; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -123,6 +127,10 @@ public class AddKubernetesSupportedVersionCmd extends BaseCmd implements AdminCm return minimumRamSize; } + public boolean isDirectDownload() { + return (directDownload != null) ? directDownload : false; + } + @Override public long getEntityOwnerId() { return CallContext.current().getCallingAccountId(); diff --git a/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/response/KubernetesSupportedVersionResponse.java b/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/response/KubernetesSupportedVersionResponse.java index 72a52682364..188328ac008 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/response/KubernetesSupportedVersionResponse.java +++ b/plugins/integrations/kubernetes-service/src/main/java/org/apache/cloudstack/api/response/KubernetesSupportedVersionResponse.java @@ -86,6 +86,10 @@ public class KubernetesSupportedVersionResponse extends BaseResponse { @Param(description = "the date when this Kubernetes supported version was created") private Date created; + @SerializedName(ApiConstants.DIRECT_DOWNLOAD) + @Param(description = "KVM Only: true if the ISO for the Kubernetes supported version is directly downloaded to Primary Storage bypassing Secondary Storage", since = "4.18.2") + private Boolean directDownload; + public String getId() { return id; } @@ -193,4 +197,8 @@ public class KubernetesSupportedVersionResponse extends BaseResponse { public void setCreated(Date created) { this.created = created; } + + public void setDirectDownload(Boolean directDownload) { + this.directDownload = directDownload; + } } diff --git a/server/src/main/java/com/cloud/network/router/NetworkHelper.java b/server/src/main/java/com/cloud/network/router/NetworkHelper.java index ea008e4c4ca..c9daa5eedb4 100644 --- a/server/src/main/java/com/cloud/network/router/NetworkHelper.java +++ b/server/src/main/java/com/cloud/network/router/NetworkHelper.java @@ -20,6 +20,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.network.router.deployment.RouterDeploymentDefinition; import com.cloud.agent.api.to.NicTO; @@ -93,4 +94,6 @@ public interface NetworkHelper { throws ConcurrentOperationException, InsufficientAddressCapacityException; public boolean validateHAProxyLBRule(final LoadBalancingRule rule); + + public Map> getHypervisorRouterTemplateConfigMap(); } diff --git a/server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java b/server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java index 56860499ae0..f0fa50bb913 100644 --- a/server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java +++ b/server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java @@ -28,7 +28,6 @@ import java.util.Map; import javax.annotation.PostConstruct; import javax.inject.Inject; -import com.cloud.utils.validation.ChecksumUtil; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; @@ -102,6 +101,7 @@ import com.cloud.user.dao.UserDao; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.NetUtils; +import com.cloud.utils.validation.ChecksumUtil; import com.cloud.vm.DomainRouterVO; import com.cloud.vm.Nic; import com.cloud.vm.NicProfile; @@ -968,4 +968,9 @@ public class NetworkHelperImpl implements NetworkHelper { public String acquireGuestIpAddressForVrouterRedundant(Network network) { return _ipAddrMgr.acquireGuestIpAddressByPlacement(network, null); } + + @Override + public Map> getHypervisorRouterTemplateConfigMap() { + return hypervisorsMap; + } } diff --git a/ui/src/views/compute/CreateKubernetesCluster.vue b/ui/src/views/compute/CreateKubernetesCluster.vue index 9ca7b0aa7c8..1c70dece6e1 100644 --- a/ui/src/views/compute/CreateKubernetesCluster.vue +++ b/ui/src/views/compute/CreateKubernetesCluster.vue @@ -343,7 +343,6 @@ export default { const listZones = json.listzonesresponse.zone if (listZones) { this.zones = this.zones.concat(listZones) - this.zones = this.zones.filter(zone => zone.type !== 'Edge') } }).finally(() => { this.zoneLoading = false @@ -356,6 +355,7 @@ export default { handleZoneChange (zone) { this.selectedZone = zone this.fetchKubernetesVersionData() + this.fetchNetworkData() }, fetchKubernetesVersionData () { this.kubernetesVersions = [] @@ -414,10 +414,14 @@ export default { }, fetchNetworkData () { const params = {} + if (!this.isObjectEmpty(this.selectedZone)) { + params.zoneid = this.selectedZone.id + } this.networkLoading = true api('listNetworks', params).then(json => { - const listNetworks = json.listnetworksresponse.network + var listNetworks = json.listnetworksresponse.network if (this.arrayHasItems(listNetworks)) { + listNetworks = listNetworks.filter(n => n.type !== 'L2') this.networks = this.networks.concat(listNetworks) } }).finally(() => { diff --git a/ui/src/views/image/AddKubernetesSupportedVersion.vue b/ui/src/views/image/AddKubernetesSupportedVersion.vue index ad4a9490a37..c88fc34695d 100644 --- a/ui/src/views/image/AddKubernetesSupportedVersion.vue +++ b/ui/src/views/image/AddKubernetesSupportedVersion.vue @@ -54,7 +54,8 @@ return option.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" :loading="zoneLoading" - :placeholder="apiParams.zoneid.description"> + :placeholder="apiParams.zoneid.description" + @change="handleZoneChange"> @@ -96,6 +97,15 @@ v-model:value="form.minmemory" :placeholder="apiParams.minmemory.description"/> + + + +
{{ $t('label.cancel') }} @@ -122,7 +132,10 @@ export default { return { zones: [], zoneLoading: false, - loading: false + loading: false, + selectedZone: {}, + directDownloadDisabled: false, + lastNonEdgeDirectDownloadUserSelection: false } }, beforeCreate () { @@ -143,7 +156,8 @@ export default { this.formRef = ref() this.form = reactive({ mincpunumber: 2, - minmemory: 2048 + minmemory: 2048, + directdownload: false }) this.rules = reactive({ semanticversion: [{ required: true, message: this.$t('message.error.kuberversion') }], @@ -198,7 +212,6 @@ export default { const listZones = json.listzonesresponse.zone if (listZones) { this.zones = this.zones.concat(listZones) - this.zones = this.zones.filter(zone => zone.type !== 'Edge') } }).finally(() => { this.zoneLoading = false @@ -207,23 +220,38 @@ export default { } }) }, + handleZoneChange (zoneIdx) { + const zone = this.zones[zoneIdx] + if (this.selectedZone.type === zone.type) { + return + } + var lastZoneType = this.selectedZone?.type || '' + if (lastZoneType !== 'Edge') { + this.nonEdgeDirectDownloadUserSelection = this.form.directdownload + } + this.selectedZone = zone + if (zone.type && zone.type === 'Edge') { + this.form.directdownload = true + this.directDownloadDisabled = true + return + } + this.directDownloadDisabled = false + this.form.directdownload = this.nonEdgeDirectDownloadUserSelection + }, handleSubmit (e) { e.preventDefault() if (this.loading) return this.formRef.value.validate().then(() => { const values = toRaw(this.form) this.loading = true - const params = { - semanticversion: values.semanticversion, - url: values.url + const params = {} + const customCheckParams = ['mincpunumber', 'minmemory', 'zoneid'] + for (const key in values) { + if (!customCheckParams.includes(key) && values[key]) { + params[key] = values[key] + } } - if (this.isValidValueForKey(values, 'name')) { - params.name = values.name - } - if (this.isValidValueForKey(values, 'checksum')) { - params.checksum = values.checksum - } - if (this.isValidValueForKey(values, 'zoneid')) { + if (this.isValidValueForKey(values, 'zoneid') && values.zoneid > 0) { params.zoneid = this.zones[values.zoneid].id } if (this.isValidValueForKey(values, 'mincpunumber') && values.mincpunumber > 0) { From b79e3937b46ffb4788d4139ee7b6f220f5d5614a Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 10 Nov 2023 14:15:42 +0100 Subject: [PATCH 02/39] UI: fix scale vm if first disk offering is dymamic (#8213) --- ui/src/views/compute/ScaleVM.vue | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/ui/src/views/compute/ScaleVM.vue b/ui/src/views/compute/ScaleVM.vue index 1a1d1309720..4ae02e7a80b 100644 --- a/ui/src/views/compute/ScaleVM.vue +++ b/ui/src/views/compute/ScaleVM.vue @@ -221,16 +221,19 @@ export default { this.params.serviceofferingid = id this.selectedOffering = this.offeringsMap[id] - api('listDiskOfferings', { - id: this.selectedOffering.diskofferingid - }).then(response => { - const diskOfferings = response.listdiskofferingsresponse.diskoffering || [] - if (this.offerings) { - this.selectedDiskOffering = diskOfferings[0] - } - }).catch(error => { - this.$notifyError(error) - }) + this.selectedDiskOffering = null + if (this.selectedOffering.diskofferingid) { + api('listDiskOfferings', { + id: this.selectedOffering.diskofferingid + }).then(response => { + const diskOfferings = response.listdiskofferingsresponse.diskoffering || [] + if (this.diskOfferings) { + this.selectedDiskOffering = diskOfferings[0] + } + }).catch(error => { + this.$notifyError(error) + }) + } this.params.automigrate = this.autoMigrate }, updateFieldValue (name, value) { From cfd6bffacf2383a35b6945dfdf26c9d98c48e2af Mon Sep 17 00:00:00 2001 From: John Bampton Date: Mon, 13 Nov 2023 18:33:19 +1000 Subject: [PATCH 03/39] PULL_REQUEST_TEMPLATE.md: fix spelling (#8220) --- PULL_REQUEST_TEMPLATE.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md index 7fe648a39fd..8293a22973a 100644 --- a/PULL_REQUEST_TEMPLATE.md +++ b/PULL_REQUEST_TEMPLATE.md @@ -10,10 +10,10 @@ This PR... - - + + - + ### Types of changes From df4cd2aae42f7fea114b091ee2830ff263e7aaf7 Mon Sep 17 00:00:00 2001 From: Fabricio Duarte Date: Tue, 14 Nov 2023 05:12:58 -0300 Subject: [PATCH 04/39] Inject hypervisor type and volume format on Quota tariffs (#8138) --- .../cloudstack/quota/QuotaManagerImpl.java | 2 +- .../presetvariables/PresetVariableHelper.java | 20 ++ .../activationrule/presetvariables/Value.java | 20 ++ .../quota/QuotaManagerImplTest.java | 4 +- .../PresetVariableHelperTest.java | 205 ++++++++++-------- .../presetvariables/ValueTest.java | 14 ++ .../utils/jsinterpreter/JsInterpreter.java | 17 +- .../jsinterpreter/JsInterpreterTest.java | 18 ++ 8 files changed, 204 insertions(+), 96 deletions(-) diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java index b17d27b0adf..d6499b91485 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java @@ -455,7 +455,7 @@ public class QuotaManagerImpl extends ManagerBase implements QuotaManager { } - jsInterpreter.injectVariable("resourceType", presetVariables.getResourceType()); + jsInterpreter.injectStringVariable("resourceType", presetVariables.getResourceType()); jsInterpreter.injectVariable("value", presetVariables.getValue().toString()); jsInterpreter.injectVariable("zone", presetVariables.getZone().toString()); } diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java index 1cf4f864ab9..6ce8bd889a6 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java @@ -27,6 +27,7 @@ import java.util.stream.Collectors; import javax.inject.Inject; +import com.cloud.hypervisor.Hypervisor; import org.apache.cloudstack.acl.RoleVO; import org.apache.cloudstack.acl.dao.RoleDao; import org.apache.cloudstack.backup.BackupOfferingVO; @@ -65,6 +66,7 @@ import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.GuestOSVO; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotVO; +import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; @@ -318,6 +320,10 @@ public class PresetVariableHelper { value.setTags(getPresetVariableValueResourceTags(vmId, ResourceObjectType.UserVm)); value.setTemplate(getPresetVariableValueTemplate(vmVo.getTemplateId())); + Hypervisor.HypervisorType hypervisorType = vmVo.getHypervisorType(); + if (hypervisorType != null) { + value.setHypervisorType(hypervisorType.name()); + } } protected void logNotLoadingMessageInTrace(String resource, int usageType) { @@ -470,6 +476,11 @@ public class PresetVariableHelper { value.setTags(getPresetVariableValueResourceTags(volumeId, ResourceObjectType.Volume)); value.setSize(ByteScaleUtils.bytesToMebibytes(volumeVo.getSize())); + + ImageFormat format = volumeVo.getFormat(); + if (format != null) { + value.setVolumeFormat(format.name()); + } } protected GenericPresetVariable getPresetVariableValueDiskOffering(Long diskOfferingId) { @@ -558,6 +569,10 @@ public class PresetVariableHelper { value.setSnapshotType(Snapshot.Type.values()[snapshotVo.getSnapshotType()]); value.setStorage(getPresetVariableValueStorage(getSnapshotDataStoreId(snapshotId, usageRecord.getZoneId()), usageType)); value.setTags(getPresetVariableValueResourceTags(snapshotId, ResourceObjectType.Snapshot)); + Hypervisor.HypervisorType hypervisorType = snapshotVo.getHypervisorType(); + if (hypervisorType != null) { + value.setHypervisorType(hypervisorType.name()); + } } protected SnapshotDataStoreVO getSnapshotImageStoreRef(long snapshotId, long zoneId) { @@ -621,6 +636,11 @@ public class PresetVariableHelper { value.setName(vmSnapshotVo.getName()); value.setTags(getPresetVariableValueResourceTags(vmSnapshotId, ResourceObjectType.VMSnapshot)); value.setVmSnapshotType(vmSnapshotVo.getType()); + + VMInstanceVO vmVo = vmInstanceDao.findByIdIncludingRemoved(vmSnapshotVo.getVmId()); + if (vmVo != null && vmVo.getHypervisorType() != null) { + value.setHypervisorType(vmVo.getHypervisorType().name()); + } } protected void loadPresetVariableValueForBackup(UsageVO usageRecord, Value value) { diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Value.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Value.java index 87fefe3a363..f3accd83be8 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Value.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Value.java @@ -41,6 +41,8 @@ public class Value extends GenericPresetVariable { private Storage storage; private ComputingResources computingResources; private BackupOffering backupOffering; + private String hypervisorType; + private String volumeFormat; public Host getHost() { return host; @@ -185,4 +187,22 @@ public class Value extends GenericPresetVariable { this.backupOffering = backupOffering; fieldNamesToIncludeInToString.add("backupOffering"); } + + public void setHypervisorType(String hypervisorType) { + this.hypervisorType = hypervisorType; + fieldNamesToIncludeInToString.add("hypervisorType"); + } + + public String getHypervisorType() { + return hypervisorType; + } + + public void setVolumeFormat(String volumeFormat) { + this.volumeFormat = volumeFormat; + fieldNamesToIncludeInToString.add("volumeFormat"); + } + + public String getVolumeFormat() { + return volumeFormat; + } } diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java index 5978dcf5fc4..e42e03dbe8e 100644 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java +++ b/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java @@ -267,7 +267,7 @@ public class QuotaManagerImplTest { Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.anyString()); Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.anyString()); Mockito.verify(jsInterpreterMock, Mockito.never()).injectVariable(Mockito.eq("project"), Mockito.anyString()); - Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.anyString()); + Mockito.verify(jsInterpreterMock).injectStringVariable(Mockito.eq("resourceType"), Mockito.anyString()); Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.anyString()); Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.anyString()); } @@ -288,7 +288,7 @@ public class QuotaManagerImplTest { Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.anyString()); Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.anyString()); Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("project"), Mockito.anyString()); - Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.anyString()); + Mockito.verify(jsInterpreterMock).injectStringVariable(Mockito.eq("resourceType"), Mockito.anyString()); Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.anyString()); Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.anyString()); } diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelperTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelperTest.java index cf1a680f2bb..ae15e573fa8 100644 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelperTest.java +++ b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelperTest.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import com.cloud.hypervisor.Hypervisor; import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.acl.RoleVO; import org.apache.cloudstack.acl.dao.RoleDao; @@ -70,6 +71,7 @@ import com.cloud.storage.GuestOSVO; import com.cloud.storage.ScopeType; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotVO; +import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.Storage.ProvisioningType; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.VolumeVO; @@ -485,38 +487,42 @@ public class PresetVariableHelperTest { @Test public void loadPresetVariableValueForRunningAndAllocatedVmTestRecordIsRunningOrAllocatedVmSetFields() { - Value expected = getValueForTests(); + for (Hypervisor.HypervisorType hypervisorType : Hypervisor.HypervisorType.values()) { + Value expected = getValueForTests(); - Mockito.doReturn(vmInstanceVoMock).when(vmInstanceDaoMock).findByIdIncludingRemoved(Mockito.anyLong()); + Mockito.doReturn(vmInstanceVoMock).when(vmInstanceDaoMock).findByIdIncludingRemoved(Mockito.anyLong()); - mockMethodValidateIfObjectIsNull(); + mockMethodValidateIfObjectIsNull(); - Mockito.doNothing().when(presetVariableHelperSpy).setPresetVariableHostInValueIfUsageTypeIsRunningVm(Mockito.any(Value.class), Mockito.anyInt(), - Mockito.any(VMInstanceVO.class)); + Mockito.doNothing().when(presetVariableHelperSpy).setPresetVariableHostInValueIfUsageTypeIsRunningVm(Mockito.any(Value.class), Mockito.anyInt(), + Mockito.any(VMInstanceVO.class)); - Mockito.doReturn(expected.getId()).when(vmInstanceVoMock).getUuid(); - Mockito.doReturn(expected.getName()).when(vmInstanceVoMock).getHostName(); - Mockito.doReturn(expected.getOsName()).when(presetVariableHelperSpy).getPresetVariableValueOsName(Mockito.anyLong()); - Mockito.doNothing().when(presetVariableHelperSpy).setPresetVariableValueServiceOfferingAndComputingResources(Mockito.any(), Mockito.anyInt(), Mockito.any()); - Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class)); - Mockito.doReturn(expected.getTemplate()).when(presetVariableHelperSpy).getPresetVariableValueTemplate(Mockito.anyLong()); + Mockito.doReturn(expected.getId()).when(vmInstanceVoMock).getUuid(); + Mockito.doReturn(expected.getName()).when(vmInstanceVoMock).getHostName(); + Mockito.doReturn(expected.getOsName()).when(presetVariableHelperSpy).getPresetVariableValueOsName(Mockito.anyLong()); + Mockito.doNothing().when(presetVariableHelperSpy).setPresetVariableValueServiceOfferingAndComputingResources(Mockito.any(), Mockito.anyInt(), Mockito.any()); + Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class)); + Mockito.doReturn(expected.getTemplate()).when(presetVariableHelperSpy).getPresetVariableValueTemplate(Mockito.anyLong()); + Mockito.doReturn(hypervisorType).when(vmInstanceVoMock).getHypervisorType(); - runningAndAllocatedVmUsageTypes.forEach(type -> { - Mockito.doReturn(type).when(usageVoMock).getUsageType(); + runningAndAllocatedVmUsageTypes.forEach(type -> { + Mockito.doReturn(type).when(usageVoMock).getUsageType(); - Value result = new Value(); - presetVariableHelperSpy.loadPresetVariableValueForRunningAndAllocatedVm(usageVoMock, result); + Value result = new Value(); + presetVariableHelperSpy.loadPresetVariableValueForRunningAndAllocatedVm(usageVoMock, result); - assertPresetVariableIdAndName(expected, result); - Assert.assertEquals(expected.getOsName(), result.getOsName()); - Assert.assertEquals(expected.getTags(), result.getTags()); - Assert.assertEquals(expected.getTemplate(), result.getTemplate()); + assertPresetVariableIdAndName(expected, result); + Assert.assertEquals(expected.getOsName(), result.getOsName()); + Assert.assertEquals(expected.getTags(), result.getTags()); + Assert.assertEquals(expected.getTemplate(), result.getTemplate()); + Assert.assertEquals(hypervisorType.name(), result.getHypervisorType()); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "osName", "tags", "template"), result); - }); + validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "osName", "tags", "template", "hypervisorType"), result); + }); + } - Mockito.verify(presetVariableHelperSpy, Mockito.times(runningAndAllocatedVmUsageTypes.size())).getPresetVariableValueResourceTags(Mockito.anyLong(), - Mockito.eq(ResourceObjectType.UserVm)); + Mockito.verify(presetVariableHelperSpy, Mockito.times(runningAndAllocatedVmUsageTypes.size() * Hypervisor.HypervisorType.values().length)) + .getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.eq(ResourceObjectType.UserVm)); } @Test @@ -636,75 +642,85 @@ public class PresetVariableHelperTest { @Test public void loadPresetVariableValueForVolumeTestRecordIsVolumeAndHasStorageSetFields() { - Value expected = getValueForTests(); + for (ImageFormat imageFormat : ImageFormat.values()) { + Value expected = getValueForTests(); - VolumeVO volumeVoMock = Mockito.mock(VolumeVO.class); - Mockito.doReturn(volumeVoMock).when(volumeDaoMock).findByIdIncludingRemoved(Mockito.anyLong()); - Mockito.doReturn(1l).when(volumeVoMock).getPoolId(); + VolumeVO volumeVoMock = Mockito.mock(VolumeVO.class); + Mockito.doReturn(volumeVoMock).when(volumeDaoMock).findByIdIncludingRemoved(Mockito.anyLong()); + Mockito.doReturn(1l).when(volumeVoMock).getPoolId(); - mockMethodValidateIfObjectIsNull(); + mockMethodValidateIfObjectIsNull(); - Mockito.doReturn(expected.getId()).when(volumeVoMock).getUuid(); - Mockito.doReturn(expected.getName()).when(volumeVoMock).getName(); - Mockito.doReturn(expected.getDiskOffering()).when(presetVariableHelperSpy).getPresetVariableValueDiskOffering(Mockito.anyLong()); - Mockito.doReturn(expected.getProvisioningType()).when(volumeVoMock).getProvisioningType(); - Mockito.doReturn(expected.getStorage()).when(presetVariableHelperSpy).getPresetVariableValueStorage(Mockito.anyLong(), Mockito.anyInt()); - Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class)); - Mockito.doReturn(expected.getSize()).when(volumeVoMock).getSize(); + Mockito.doReturn(expected.getId()).when(volumeVoMock).getUuid(); + Mockito.doReturn(expected.getName()).when(volumeVoMock).getName(); + Mockito.doReturn(expected.getDiskOffering()).when(presetVariableHelperSpy).getPresetVariableValueDiskOffering(Mockito.anyLong()); + Mockito.doReturn(expected.getProvisioningType()).when(volumeVoMock).getProvisioningType(); + Mockito.doReturn(expected.getStorage()).when(presetVariableHelperSpy).getPresetVariableValueStorage(Mockito.anyLong(), Mockito.anyInt()); + Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class)); + Mockito.doReturn(expected.getSize()).when(volumeVoMock).getSize(); + Mockito.doReturn(imageFormat).when(volumeVoMock).getFormat(); - Mockito.doReturn(UsageTypes.VOLUME).when(usageVoMock).getUsageType(); + Mockito.doReturn(UsageTypes.VOLUME).when(usageVoMock).getUsageType(); - Value result = new Value(); - presetVariableHelperSpy.loadPresetVariableValueForVolume(usageVoMock, result); + Value result = new Value(); + presetVariableHelperSpy.loadPresetVariableValueForVolume(usageVoMock, result); - Long expectedSize = ByteScaleUtils.bytesToMebibytes(expected.getSize()); + Long expectedSize = ByteScaleUtils.bytesToMebibytes(expected.getSize()); - assertPresetVariableIdAndName(expected, result); - Assert.assertEquals(expected.getDiskOffering(), result.getDiskOffering()); - Assert.assertEquals(expected.getProvisioningType(), result.getProvisioningType()); - Assert.assertEquals(expected.getStorage(), result.getStorage()); - Assert.assertEquals(expected.getTags(), result.getTags()); - Assert.assertEquals(expectedSize, result.getSize()); + assertPresetVariableIdAndName(expected, result); + Assert.assertEquals(expected.getDiskOffering(), result.getDiskOffering()); + Assert.assertEquals(expected.getProvisioningType(), result.getProvisioningType()); + Assert.assertEquals(expected.getStorage(), result.getStorage()); + Assert.assertEquals(expected.getTags(), result.getTags()); + Assert.assertEquals(expectedSize, result.getSize()); + Assert.assertEquals(imageFormat.name(), result.getVolumeFormat()); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "diskOffering", "provisioningType", "storage", "tags", "size"), result); + validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "diskOffering", "provisioningType", "storage", "tags", "size", "volumeFormat"), result); + } - Mockito.verify(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.eq(ResourceObjectType.Volume)); + Mockito.verify(presetVariableHelperSpy, Mockito.times(ImageFormat.values().length)).getPresetVariableValueResourceTags(Mockito.anyLong(), + Mockito.eq(ResourceObjectType.Volume)); } @Test public void loadPresetVariableValueForVolumeTestRecordIsVolumeAndDoesNotHaveStorageSetFields() { - Value expected = getValueForTests(); + for (ImageFormat imageFormat : ImageFormat.values()) { + Value expected = getValueForTests(); - VolumeVO volumeVoMock = Mockito.mock(VolumeVO.class); - Mockito.doReturn(volumeVoMock).when(volumeDaoMock).findByIdIncludingRemoved(Mockito.anyLong()); - Mockito.doReturn(null).when(volumeVoMock).getPoolId(); + VolumeVO volumeVoMock = Mockito.mock(VolumeVO.class); + Mockito.doReturn(volumeVoMock).when(volumeDaoMock).findByIdIncludingRemoved(Mockito.anyLong()); + Mockito.doReturn(null).when(volumeVoMock).getPoolId(); - mockMethodValidateIfObjectIsNull(); + mockMethodValidateIfObjectIsNull(); - Mockito.doReturn(expected.getId()).when(volumeVoMock).getUuid(); - Mockito.doReturn(expected.getName()).when(volumeVoMock).getName(); - Mockito.doReturn(expected.getDiskOffering()).when(presetVariableHelperSpy).getPresetVariableValueDiskOffering(Mockito.anyLong()); - Mockito.doReturn(expected.getProvisioningType()).when(volumeVoMock).getProvisioningType(); - Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class)); - Mockito.doReturn(expected.getSize()).when(volumeVoMock).getSize(); + Mockito.doReturn(expected.getId()).when(volumeVoMock).getUuid(); + Mockito.doReturn(expected.getName()).when(volumeVoMock).getName(); + Mockito.doReturn(expected.getDiskOffering()).when(presetVariableHelperSpy).getPresetVariableValueDiskOffering(Mockito.anyLong()); + Mockito.doReturn(expected.getProvisioningType()).when(volumeVoMock).getProvisioningType(); + Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class)); + Mockito.doReturn(expected.getSize()).when(volumeVoMock).getSize(); + Mockito.doReturn(imageFormat).when(volumeVoMock).getFormat(); - Mockito.doReturn(UsageTypes.VOLUME).when(usageVoMock).getUsageType(); + Mockito.doReturn(UsageTypes.VOLUME).when(usageVoMock).getUsageType(); - Value result = new Value(); - presetVariableHelperSpy.loadPresetVariableValueForVolume(usageVoMock, result); + Value result = new Value(); + presetVariableHelperSpy.loadPresetVariableValueForVolume(usageVoMock, result); - Long expectedSize = ByteScaleUtils.bytesToMebibytes(expected.getSize()); + Long expectedSize = ByteScaleUtils.bytesToMebibytes(expected.getSize()); - assertPresetVariableIdAndName(expected, result); - Assert.assertEquals(expected.getDiskOffering(), result.getDiskOffering()); - Assert.assertEquals(expected.getProvisioningType(), result.getProvisioningType()); - Assert.assertEquals(null, result.getStorage()); - Assert.assertEquals(expected.getTags(), result.getTags()); - Assert.assertEquals(expectedSize, result.getSize()); + assertPresetVariableIdAndName(expected, result); + Assert.assertEquals(expected.getDiskOffering(), result.getDiskOffering()); + Assert.assertEquals(expected.getProvisioningType(), result.getProvisioningType()); + Assert.assertNull(result.getStorage()); + Assert.assertEquals(expected.getTags(), result.getTags()); + Assert.assertEquals(expectedSize, result.getSize()); + Assert.assertEquals(imageFormat.name(), result.getVolumeFormat()); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "diskOffering", "provisioningType", "tags", "size"), result); + validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "diskOffering", "provisioningType", "tags", "size", "volumeFormat"), result); + } - Mockito.verify(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.eq(ResourceObjectType.Volume)); + Mockito.verify(presetVariableHelperSpy, Mockito.times(ImageFormat.values().length)).getPresetVariableValueResourceTags(Mockito.anyLong(), + Mockito.eq(ResourceObjectType.Volume)); } @Test @@ -852,37 +868,42 @@ public class PresetVariableHelperTest { @Test public void loadPresetVariableValueForSnapshotTestRecordIsSnapshotSetFields() { - Value expected = getValueForTests(); + for (Hypervisor.HypervisorType hypervisorType : Hypervisor.HypervisorType.values()) { + Value expected = getValueForTests(); - SnapshotVO snapshotVoMock = Mockito.mock(SnapshotVO.class); - Mockito.doReturn(snapshotVoMock).when(snapshotDaoMock).findByIdIncludingRemoved(Mockito.anyLong()); + SnapshotVO snapshotVoMock = Mockito.mock(SnapshotVO.class); + Mockito.doReturn(snapshotVoMock).when(snapshotDaoMock).findByIdIncludingRemoved(Mockito.anyLong()); - mockMethodValidateIfObjectIsNull(); + mockMethodValidateIfObjectIsNull(); - Mockito.doReturn(expected.getId()).when(snapshotVoMock).getUuid(); - Mockito.doReturn(expected.getName()).when(snapshotVoMock).getName(); - Mockito.doReturn(expected.getSize()).when(snapshotVoMock).getSize(); - Mockito.doReturn((short) 3).when(snapshotVoMock).getSnapshotType(); - Mockito.doReturn(1l).when(presetVariableHelperSpy).getSnapshotDataStoreId(Mockito.anyLong(), Mockito.anyLong()); - Mockito.doReturn(expected.getStorage()).when(presetVariableHelperSpy).getPresetVariableValueStorage(Mockito.anyLong(), Mockito.anyInt()); - Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class)); + Mockito.doReturn(expected.getId()).when(snapshotVoMock).getUuid(); + Mockito.doReturn(expected.getName()).when(snapshotVoMock).getName(); + Mockito.doReturn(expected.getSize()).when(snapshotVoMock).getSize(); + Mockito.doReturn((short) 3).when(snapshotVoMock).getSnapshotType(); + Mockito.doReturn(1l).when(presetVariableHelperSpy).getSnapshotDataStoreId(Mockito.anyLong(), Mockito.anyLong()); + Mockito.doReturn(expected.getStorage()).when(presetVariableHelperSpy).getPresetVariableValueStorage(Mockito.anyLong(), Mockito.anyInt()); + Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class)); + Mockito.doReturn(hypervisorType).when(snapshotVoMock).getHypervisorType(); - Mockito.doReturn(UsageTypes.SNAPSHOT).when(usageVoMock).getUsageType(); + Mockito.doReturn(UsageTypes.SNAPSHOT).when(usageVoMock).getUsageType(); - Value result = new Value(); - presetVariableHelperSpy.loadPresetVariableValueForSnapshot(usageVoMock, result); + Value result = new Value(); + presetVariableHelperSpy.loadPresetVariableValueForSnapshot(usageVoMock, result); - Long expectedSize = ByteScaleUtils.bytesToMebibytes(expected.getSize()); + Long expectedSize = ByteScaleUtils.bytesToMebibytes(expected.getSize()); - assertPresetVariableIdAndName(expected, result); - Assert.assertEquals(expected.getSnapshotType(), result.getSnapshotType()); - Assert.assertEquals(expected.getStorage(), result.getStorage()); - Assert.assertEquals(expected.getTags(), result.getTags()); - Assert.assertEquals(expectedSize, result.getSize()); + assertPresetVariableIdAndName(expected, result); + Assert.assertEquals(expected.getSnapshotType(), result.getSnapshotType()); + Assert.assertEquals(expected.getStorage(), result.getStorage()); + Assert.assertEquals(expected.getTags(), result.getTags()); + Assert.assertEquals(expectedSize, result.getSize()); + Assert.assertEquals(hypervisorType.name(), result.getHypervisorType()); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "snapshotType", "storage", "tags", "size"), result); + validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "snapshotType", "storage", "tags", "size", "hypervisorType"), result); + } - Mockito.verify(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.eq(ResourceObjectType.Snapshot)); + Mockito.verify(presetVariableHelperSpy, Mockito.times(Hypervisor.HypervisorType.values().length)).getPresetVariableValueResourceTags(Mockito.anyLong(), + Mockito.eq(ResourceObjectType.Snapshot)); } diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ValueTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ValueTest.java index c9d14401b3f..9e65de754c8 100644 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ValueTest.java +++ b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ValueTest.java @@ -136,4 +136,18 @@ public class ValueTest { variable.setBackupOffering(null); Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("backupOffering")); } + + @Test + public void setHypervisorTypeTestAddFieldHypervisorTypeToCollection() { + Value variable = new Value(); + variable.setHypervisorType(null); + Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("hypervisorType")); + } + + @Test + public void setVolumeFormatTestAddFieldVolumeFormatToCollection() { + Value variable = new Value(); + variable.setVolumeFormat(null); + Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("volumeFormat")); + } } diff --git a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java index 03ad3b401b8..b15bd31a15f 100644 --- a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java +++ b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java @@ -78,7 +78,7 @@ public class JsInterpreter implements Closeable { } /** - * Adds the parameters to a Map that will be converted to JS variables right before executing the scripts.. + * Adds the parameters to a Map that will be converted to JS variables right before executing the script. * @param key The name of the variable. * @param value The value of the variable. */ @@ -87,6 +87,21 @@ public class JsInterpreter implements Closeable { variables.put(key, value); } + /** + * Adds the parameter, surrounded by double quotes, to a Map that will be converted to a JS variable right before executing the script. + * @param key The name of the variable. + * @param value The value of the variable. + */ + public void injectStringVariable(String key, String value) { + if (value == null) { + logger.trace(String.format("Not injecting [%s] because its value is null.", key)); + return; + } + value = String.format("\"%s\"", value); + logger.trace(String.format(injectingLogMessage, key, value)); + variables.put(key, value); + } + /** * Injects the variables to the script and execute it. * @param script Code to be executed. diff --git a/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java b/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java index d1814625929..a8c2e6ec0d3 100644 --- a/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java +++ b/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java @@ -179,4 +179,22 @@ public class JsInterpreterTest { Assert.assertEquals(scriptEngineMock, jsInterpreterSpy.interpreter); Mockito.verify(nashornScriptEngineFactoryMock).getScriptEngine("--no-java"); } + + @Test + public void injectStringVariableTestNullValueDoNothing() { + jsInterpreterSpy.variables = new LinkedHashMap<>(); + + jsInterpreterSpy.injectStringVariable("a", null); + + Assert.assertTrue(jsInterpreterSpy.variables.isEmpty()); + } + + @Test + public void injectStringVariableTestNotNullValueSurroundWithDoubleQuotes() { + jsInterpreterSpy.variables = new LinkedHashMap<>(); + + jsInterpreterSpy.injectStringVariable("a", "b"); + + Assert.assertEquals(jsInterpreterSpy.variables.get("a"), "\"b\""); + } } From b7835d02d251a50e3cd3a45bc9bae3b14945944f Mon Sep 17 00:00:00 2001 From: Harikrishna Date: Tue, 14 Nov 2023 14:01:53 +0530 Subject: [PATCH 05/39] Fix deploy as is VM start after template deletion (#8115) --- .../template/HypervisorTemplateAdapter.java | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java index cdd58ce030e..1633cd8a360 100644 --- a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java +++ b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java @@ -29,6 +29,8 @@ import java.util.stream.Collectors; import javax.inject.Inject; import com.cloud.domain.Domain; +import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.dao.VMInstanceDao; import org.apache.cloudstack.agent.directdownload.CheckUrlAnswer; import org.apache.cloudstack.agent.directdownload.CheckUrlCommand; import org.apache.cloudstack.annotation.AnnotationService; @@ -142,6 +144,8 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { private TemplateDeployAsIsDetailsDao templateDeployAsIsDetailsDao; @Inject private AnnotationDao annotationDao; + @Inject + VMInstanceDao _vmInstanceDao; @Override public String getName() { @@ -662,11 +666,7 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { Pair, Long> tmplt = new Pair, Long>(VirtualMachineTemplate.class, template.getId()); _messageBus.publish(_name, EntityManager.MESSAGE_REMOVE_ENTITY_EVENT, PublishScope.LOCAL, tmplt); - // Remove template details - templateDetailsDao.removeDetails(template.getId()); - - // Remove deploy-as-is details (if any) - templateDeployAsIsDetailsDao.removeDetails(template.getId()); + checkAndRemoveTemplateDetails(template); // Remove comments (if any) AnnotationService.EntityType entityType = template.getFormat().equals(ImageFormat.ISO) ? @@ -677,6 +677,23 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { return success; } + /** + * removes details of the template and + * if the template is registered as deploy as is, + * then it also deletes the details related to deploy as is only if there are no VMs using the template + * @param template + */ + void checkAndRemoveTemplateDetails(VMTemplateVO template) { + templateDetailsDao.removeDetails(template.getId()); + + if (template.isDeployAsIs()) { + List vmInstanceVOList = _vmInstanceDao.listNonExpungedByTemplate(template.getId()); + if (CollectionUtils.isEmpty(vmInstanceVOList)) { + templateDeployAsIsDetailsDao.removeDetails(template.getId()); + } + } + } + @Override public TemplateProfile prepareDelete(DeleteTemplateCmd cmd) { TemplateProfile profile = super.prepareDelete(cmd); From 96b07d797b73c22b63fac7db3d4f00afa470f36f Mon Sep 17 00:00:00 2001 From: rRajivramachandran Date: Tue, 14 Nov 2023 03:17:32 -0600 Subject: [PATCH 06/39] Fix flaky tungsten test using comparator (#8232) --- .../cloudstack/network/tungsten/service/TungstenApiTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/network-elements/tungsten/src/test/java/org/apache/cloudstack/network/tungsten/service/TungstenApiTest.java b/plugins/network-elements/tungsten/src/test/java/org/apache/cloudstack/network/tungsten/service/TungstenApiTest.java index 3c12bfb5cba..580bea057d8 100644 --- a/plugins/network-elements/tungsten/src/test/java/org/apache/cloudstack/network/tungsten/service/TungstenApiTest.java +++ b/plugins/network-elements/tungsten/src/test/java/org/apache/cloudstack/network/tungsten/service/TungstenApiTest.java @@ -1360,6 +1360,7 @@ public class TungstenApiTest { s_logger.debug("Check if policy was listed all in Tungsten-Fabric"); List policyList3 = tungstenApi.listTungstenPolicy(projectUuid, null); + policyList3.sort(comparator); assertEquals(policyList1, policyList3); s_logger.debug("Check if policy was listed with uuid in Tungsten-Fabric"); @@ -1383,6 +1384,7 @@ public class TungstenApiTest { s_logger.debug("Check if network was listed all in Tungsten-Fabric"); List networkList3 = tungstenApi.listTungstenNetwork(projectUuid, null); + networkList3.sort(comparator); assertEquals(networkList1, networkList3); s_logger.debug("Check if network policy was listed with uuid in Tungsten-Fabric"); From 04061f12e5ae3e98e16fa49f689ef4e872bb8ed0 Mon Sep 17 00:00:00 2001 From: slavkap <51903378+slavkap@users.noreply.github.com> Date: Tue, 14 Nov 2023 15:03:24 +0200 Subject: [PATCH 07/39] storagetype API param in list service/disk offerings (#8215) --- .../user/offering/ListDiskOfferingsCmd.java | 11 +++++++++++ .../offering/ListServiceOfferingsCmd.java | 11 +++++++++++ .../com/cloud/api/query/QueryManagerImpl.java | 19 +++++++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java index 5fa24ec1630..c605b58d7c6 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java @@ -25,6 +25,7 @@ import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseListDomainResourcesCmd; import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.BaseCmd.CommandType; import org.apache.cloudstack.api.response.DiskOfferingResponse; import org.apache.cloudstack.api.response.ListResponse; @@ -60,6 +61,12 @@ public class ListDiskOfferingsCmd extends BaseListDomainResourcesCmd { @Parameter(name = ApiConstants.ENCRYPT, type = CommandType.BOOLEAN, description = "listed offerings support disk encryption", since = "4.18") private Boolean encrypt; + @Parameter(name = ApiConstants.STORAGE_TYPE, + type = CommandType.STRING, + description = "the storage type of the service offering. Values are local and shared.", + since = "4.19") + private String storageType; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -84,6 +91,10 @@ public class ListDiskOfferingsCmd extends BaseListDomainResourcesCmd { public Boolean getEncrypt() { return encrypt; } + public String getStorageType() { + return storageType; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java index 3208ef58a4f..f7c99459baa 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java @@ -23,6 +23,7 @@ import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseListDomainResourcesCmd; import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.BaseCmd.CommandType; import org.apache.cloudstack.api.response.ListResponse; import org.apache.cloudstack.api.response.ServiceOfferingResponse; import org.apache.cloudstack.api.response.UserVmResponse; @@ -88,6 +89,12 @@ public class ListServiceOfferingsCmd extends BaseListDomainResourcesCmd { since = "4.18") private Boolean encryptRoot; + @Parameter(name = ApiConstants.STORAGE_TYPE, + type = CommandType.STRING, + description = "the storage type of the service offering. Values are local and shared.", + since = "4.19") + private String storageType; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -130,6 +137,10 @@ public class ListServiceOfferingsCmd extends BaseListDomainResourcesCmd { public Boolean getEncryptRoot() { return encryptRoot; } + public String getStorageType() { + return storageType; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index c99aa293234..91b8d7eb988 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -237,6 +237,7 @@ import com.cloud.network.router.VirtualNetworkApplianceManager; import com.cloud.network.security.SecurityGroupVMMapVO; import com.cloud.network.security.dao.SecurityGroupVMMapDao; import com.cloud.offering.DiskOffering; +import com.cloud.offering.ServiceOffering; import com.cloud.org.Grouping; import com.cloud.projects.Project; import com.cloud.projects.Project.ListProjectResourcesCriteria; @@ -3130,6 +3131,8 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q Long volumeId = cmd.getVolumeId(); Long storagePoolId = cmd.getStoragePoolId(); Boolean encrypt = cmd.getEncrypt(); + String storageType = cmd.getStorageType(); + // Keeping this logic consistent with domain specific zones // if a domainId is provided, we just return the disk offering // associated with this domain @@ -3181,6 +3184,8 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q sc.addAnd("encrypt", SearchCriteria.Op.EQ, encrypt); } + useStorageType(sc, storageType); + if (zoneId != null) { SearchBuilder sb = _diskOfferingJoinDao.createSearchBuilder(); sb.and("zoneId", sb.entity().getZoneId(), Op.FIND_IN_SET); @@ -3260,6 +3265,17 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q return new Pair<>(result.first(), result.second()); } + private void useStorageType(SearchCriteria sc, String storageType) { + if (storageType != null) { + if (storageType.equalsIgnoreCase(ServiceOffering.StorageType.local.toString())) { + sc.addAnd("useLocalStorage", Op.EQ, true); + + } else if (storageType.equalsIgnoreCase(ServiceOffering.StorageType.shared.toString())) { + sc.addAnd("useLocalStorage", Op.EQ, false); + } + } + } + private List findRelatedDomainIds(Domain domain, boolean isRecursive) { List domainIds = _domainDao.getDomainParentIds(domain.getId()) .stream().collect(Collectors.toList()); @@ -3309,6 +3325,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q Integer memory = cmd.getMemory(); Integer cpuSpeed = cmd.getCpuSpeed(); Boolean encryptRoot = cmd.getEncryptRoot(); + String storageType = cmd.getStorageType(); SearchCriteria sc = _srvOfferingJoinDao.createSearchCriteria(); if (!accountMgr.isRootAdmin(caller.getId()) && isSystem) { @@ -3432,6 +3449,8 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q sc.addAnd("vmType", SearchCriteria.Op.EQ, vmTypeStr); } + useStorageType(sc, storageType); + if (zoneId != null) { SearchBuilder sb = _srvOfferingJoinDao.createSearchBuilder(); sb.and("zoneId", sb.entity().getZoneId(), Op.FIND_IN_SET); From 1a2dbebe4894f4db7e2d3529fd56befe9ef7dfd3 Mon Sep 17 00:00:00 2001 From: dahn Date: Wed, 15 Nov 2023 09:48:11 +0100 Subject: [PATCH 08/39] Let Prometheus exporter plugin support utf8 characters (#8228) --- .../metrics/PrometheusExporterServerImpl.java | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterServerImpl.java b/plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterServerImpl.java index b5ac137dadd..cc3b7d55653 100644 --- a/plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterServerImpl.java +++ b/plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterServerImpl.java @@ -28,6 +28,7 @@ import javax.inject.Inject; import java.io.IOException; import java.io.OutputStream; import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; import java.util.Arrays; public class PrometheusExporterServerImpl extends ManagerBase implements PrometheusExporterServer, Configurable { @@ -57,11 +58,21 @@ public class PrometheusExporterServerImpl extends ManagerBase implements Prometh response = prometheusExporter.getMetrics(); responseCode = 200; } - httpExchange.getResponseHeaders().set("content-type", "text/plain"); - httpExchange.sendResponseHeaders(responseCode, response.length()); + byte[] bytesToOutput = response.getBytes(StandardCharsets.UTF_8); + httpExchange.getResponseHeaders().set("content-type", "text/plain; charset=UTF-8"); + httpExchange.sendResponseHeaders(responseCode, bytesToOutput.length); final OutputStream os = httpExchange.getResponseBody(); - os.write(response.getBytes()); - os.close(); + try { + os.write(bytesToOutput); + } catch (IOException e) { + LOG.error(String.format("could not export Prometheus data due to %s", e.getLocalizedMessage())); + if (LOG.isDebugEnabled()) { + LOG.debug("Error during Prometheus export: ", e); + } + os.write("The system could not export Prometheus due to an internal error. Contact your operator to learn about the reason.".getBytes()); + } finally { + os.close(); + } } } From 1f29f6f04096d04a1284a4a8061262b3c3033de3 Mon Sep 17 00:00:00 2001 From: Bryan Lima <42067040+BryanMLima@users.noreply.github.com> Date: Wed, 15 Nov 2023 06:29:22 -0300 Subject: [PATCH 09/39] Public IP quarantine feature (#7378) --- .../com/cloud/network/NetworkService.java | 6 + .../com/cloud/network/PublicIpQuarantine.java | 36 ++ .../apache/cloudstack/api/ApiConstants.java | 4 + .../cloudstack/api/ResponseGenerator.java | 4 + .../user/address/ListQuarantinedIpsCmd.java | 51 +++ .../user/address/RemoveQuarantinedIpCmd.java | 72 ++++ .../user/address/UpdateQuarantinedIpCmd.java | 75 ++++ .../api/response/IpQuarantineResponse.java | 130 +++++++ .../apache/cloudstack/query/QueryService.java | 4 + .../com/cloud/network/IpAddressManager.java | 48 +++ .../com/cloud/network/dao/IPAddressDao.java | 3 + .../cloud/network/dao/IPAddressDaoImpl.java | 20 ++ .../com/cloud/network/dao/IPAddressVO.java | 9 - .../network/dao/PublicIpQuarantineDao.java | 27 ++ .../dao/PublicIpQuarantineDaoImpl.java | 71 ++++ .../network/vo/PublicIpQuarantineVO.java | 131 +++++++ ...spring-engine-schema-core-daos-context.xml | 1 + .../META-INF/db/schema-41810to41900.sql | 15 + .../main/java/com/cloud/utils/db/Filter.java | 4 + .../java/com/cloud/utils/db/GenericDao.java | 2 + .../com/cloud/utils/db/GenericDaoBase.java | 23 +- .../java/com/cloud/api/ApiResponseHelper.java | 21 ++ .../com/cloud/api/query/QueryManagerImpl.java | 51 +++ .../cloud/network/IpAddressManagerImpl.java | 129 ++++++- .../com/cloud/network/NetworkServiceImpl.java | 76 ++++ .../com/cloud/network/vpc/VpcManagerImpl.java | 8 +- .../cloud/server/ManagementServerImpl.java | 12 + .../cloud/network/IpAddressManagerTest.java | 213 ++++++++++-- .../cloud/network/NetworkServiceImplTest.java | 248 ++++++++++--- .../com/cloud/user/MockUsageEventDao.java | 5 + .../com/cloud/vpc/MockNetworkManagerImpl.java | 67 ++-- .../CreateNetworkOfferingTest.java | 50 +-- .../test/resources/createNetworkOffering.xml | 1 + .../integration/smoke/test_quarantined_ips.py | 329 ++++++++++++++++++ tools/apidoc/gen_toc.py | 3 + tools/marvin/marvin/lib/base.py | 5 +- ui/src/views/network/IpAddressesTab.vue | 4 +- 37 files changed, 1815 insertions(+), 143 deletions(-) create mode 100644 api/src/main/java/com/cloud/network/PublicIpQuarantine.java create mode 100644 api/src/main/java/org/apache/cloudstack/api/command/user/address/ListQuarantinedIpsCmd.java create mode 100644 api/src/main/java/org/apache/cloudstack/api/command/user/address/RemoveQuarantinedIpCmd.java create mode 100644 api/src/main/java/org/apache/cloudstack/api/command/user/address/UpdateQuarantinedIpCmd.java create mode 100644 api/src/main/java/org/apache/cloudstack/api/response/IpQuarantineResponse.java create mode 100644 engine/schema/src/main/java/com/cloud/network/dao/PublicIpQuarantineDao.java create mode 100644 engine/schema/src/main/java/com/cloud/network/dao/PublicIpQuarantineDaoImpl.java create mode 100644 engine/schema/src/main/java/com/cloud/network/vo/PublicIpQuarantineVO.java create mode 100644 test/integration/smoke/test_quarantined_ips.py diff --git a/api/src/main/java/com/cloud/network/NetworkService.java b/api/src/main/java/com/cloud/network/NetworkService.java index 099d73d5fcb..d959e3abce0 100644 --- a/api/src/main/java/com/cloud/network/NetworkService.java +++ b/api/src/main/java/com/cloud/network/NetworkService.java @@ -24,6 +24,8 @@ import org.apache.cloudstack.api.command.admin.network.DedicateGuestVlanRangeCmd import org.apache.cloudstack.api.command.admin.network.ListDedicatedGuestVlanRangesCmd; import org.apache.cloudstack.api.command.admin.network.ListGuestVlansCmd; import org.apache.cloudstack.api.command.admin.usage.ListTrafficTypeImplementorsCmd; +import org.apache.cloudstack.api.command.user.address.RemoveQuarantinedIpCmd; +import org.apache.cloudstack.api.command.user.address.UpdateQuarantinedIpCmd; import org.apache.cloudstack.api.command.user.network.CreateNetworkCmd; import org.apache.cloudstack.api.command.user.network.CreateNetworkPermissionsCmd; import org.apache.cloudstack.api.command.user.network.ListNetworkPermissionsCmd; @@ -246,4 +248,8 @@ public interface NetworkService { boolean resetNetworkPermissions(ResetNetworkPermissionsCmd resetNetworkPermissionsCmd); void validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(final Long serviceOfferingId) throws InvalidParameterValueException; + + PublicIpQuarantine updatePublicIpAddressInQuarantine(UpdateQuarantinedIpCmd cmd); + + void removePublicIpAddressFromQuarantine(RemoveQuarantinedIpCmd cmd); } diff --git a/api/src/main/java/com/cloud/network/PublicIpQuarantine.java b/api/src/main/java/com/cloud/network/PublicIpQuarantine.java new file mode 100644 index 00000000000..d1ec98afe46 --- /dev/null +++ b/api/src/main/java/com/cloud/network/PublicIpQuarantine.java @@ -0,0 +1,36 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.network; + +import org.apache.cloudstack.api.Identity; +import org.apache.cloudstack.api.InternalIdentity; + +import java.util.Date; + +public interface PublicIpQuarantine extends InternalIdentity, Identity { + Long getPublicIpAddressId(); + + Long getPreviousOwnerId(); + + Date getEndDate(); + + String getRemovalReason(); + + Date getRemoved(); + + Date getCreated(); +} diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index a6f27d1469a..5b6647991ef 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -224,6 +224,8 @@ public class ApiConstants { public static final String INSTANCES_STATS_USER_ONLY = "instancesstatsuseronly"; public static final String PREFIX = "prefix"; public static final String PREVIOUS_ACL_RULE_ID = "previousaclruleid"; + public static final String PREVIOUS_OWNER_ID = "previousownerid"; + public static final String PREVIOUS_OWNER_NAME = "previousownername"; public static final String NEXT_ACL_RULE_ID = "nextaclruleid"; public static final String MOVE_ACL_CONSISTENCY_HASH = "aclconsistencyhash"; public static final String IMAGE_PATH = "imagepath"; @@ -402,6 +404,7 @@ public class ApiConstants { public static final String SHOW_CAPACITIES = "showcapacities"; public static final String SHOW_REMOVED = "showremoved"; public static final String SHOW_RESOURCE_ICON = "showicon"; + public static final String SHOW_INACTIVE = "showinactive"; public static final String SHOW_UNIQUE = "showunique"; public static final String SIGNATURE = "signature"; public static final String SIGNATURE_VERSION = "signatureversion"; @@ -794,6 +797,7 @@ public class ApiConstants { public static final String IPSEC_PSK = "ipsecpsk"; public static final String GUEST_IP = "guestip"; public static final String REMOVED = "removed"; + public static final String REMOVAL_REASON = "removalreason"; public static final String COMPLETED = "completed"; public static final String IKE_VERSION = "ikeversion"; public static final String IKE_POLICY = "ikepolicy"; diff --git a/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java b/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java index 1a0df486298..030f70805d2 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java +++ b/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java @@ -64,6 +64,7 @@ import org.apache.cloudstack.api.response.HostResponse; import org.apache.cloudstack.api.response.HypervisorCapabilitiesResponse; import org.apache.cloudstack.api.response.HypervisorGuestOsNamesResponse; import org.apache.cloudstack.api.response.IPAddressResponse; +import org.apache.cloudstack.api.response.IpQuarantineResponse; import org.apache.cloudstack.api.response.ImageStoreResponse; import org.apache.cloudstack.api.response.InstanceGroupResponse; import org.apache.cloudstack.api.response.InternalLoadBalancerElementResponse; @@ -169,6 +170,7 @@ import com.cloud.network.OvsProvider; import com.cloud.network.PhysicalNetwork; import com.cloud.network.PhysicalNetworkServiceProvider; import com.cloud.network.PhysicalNetworkTrafficType; +import com.cloud.network.PublicIpQuarantine; import com.cloud.network.RemoteAccessVpn; import com.cloud.network.RouterHealthCheckResult; import com.cloud.network.Site2SiteCustomerGateway; @@ -529,4 +531,6 @@ public interface ResponseGenerator { DirectDownloadCertificateHostStatusResponse createDirectDownloadCertificateProvisionResponse(Long certificateId, Long hostId, Pair result); FirewallResponse createIpv6FirewallRuleResponse(FirewallRule acl); + + IpQuarantineResponse createQuarantinedIpsResponse(PublicIpQuarantine publicIp); } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListQuarantinedIpsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListQuarantinedIpsCmd.java new file mode 100644 index 00000000000..cc014702c81 --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListQuarantinedIpsCmd.java @@ -0,0 +1,51 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.user.address; + +import com.cloud.network.PublicIpQuarantine; +import org.apache.cloudstack.acl.RoleType; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.BaseListCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.response.IpQuarantineResponse; +import org.apache.cloudstack.api.response.ListResponse; + +@APICommand(name = "listQuarantinedIps", responseObject = IpQuarantineResponse.class, description = "List public IP addresses in quarantine.", since = "4.19", + entityType = {PublicIpQuarantine.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, authorized = {RoleType.Admin, RoleType.DomainAdmin}) +public class ListQuarantinedIpsCmd extends BaseListCmd { + + @Parameter(name = ApiConstants.SHOW_REMOVED, type = CommandType.BOOLEAN, description = "Show IPs removed from quarantine.") + private boolean showRemoved = false; + + @Parameter(name = ApiConstants.SHOW_INACTIVE, type = CommandType.BOOLEAN, description = "Show IPs that are no longer in quarantine.") + private boolean showInactive = false; + + public boolean isShowRemoved() { + return showRemoved; + } + public boolean isShowInactive() { + return showInactive; + } + + @Override + public void execute() { + ListResponse response = _queryService.listQuarantinedIps(this); + response.setResponseName(getCommandName()); + this.setResponseObject(response); + } +} diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/address/RemoveQuarantinedIpCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/address/RemoveQuarantinedIpCmd.java new file mode 100644 index 00000000000..82e8373d93a --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/address/RemoveQuarantinedIpCmd.java @@ -0,0 +1,72 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.user.address; + +import com.cloud.network.PublicIpQuarantine; +import com.cloud.user.Account; +import org.apache.cloudstack.acl.RoleType; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.response.IpQuarantineResponse; +import org.apache.cloudstack.api.response.SuccessResponse; + +@APICommand(name = "removeQuarantinedIp", responseObject = IpQuarantineResponse.class, description = "Removes a public IP address from quarantine. Only IPs in active " + + "quarantine can be removed.", + since = "4.19", entityType = {PublicIpQuarantine.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, + authorized = {RoleType.Admin, RoleType.DomainAdmin}) +public class RemoveQuarantinedIpCmd extends BaseCmd { + + @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = IpQuarantineResponse.class, description = "The ID of the public IP address in active quarantine. " + + "Either the IP address is informed, or the ID of the IP address in quarantine.") + private Long id; + + @Parameter(name = ApiConstants.IP_ADDRESS, type = CommandType.STRING, description = "The public IP address in active quarantine. Either the IP address is informed, or the ID" + + " of the IP address in quarantine.") + private String ipAddress; + + @Parameter(name = ApiConstants.REMOVAL_REASON, type = CommandType.STRING, required = true, description = "The reason for removing the public IP address from quarantine " + + "prematurely.") + private String removalReason; + + public Long getId() { + return id; + } + + public String getIpAddress() { + return ipAddress; + } + + public String getRemovalReason() { + return removalReason; + } + + @Override + public void execute() { + _networkService.removePublicIpAddressFromQuarantine(this); + final SuccessResponse response = new SuccessResponse(); + response.setResponseName(getCommandName()); + response.setSuccess(true); + setResponseObject(response); + } + + @Override + public long getEntityOwnerId() { + return Account.ACCOUNT_ID_SYSTEM; + } +} diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/address/UpdateQuarantinedIpCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/address/UpdateQuarantinedIpCmd.java new file mode 100644 index 00000000000..b3b71c33781 --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/address/UpdateQuarantinedIpCmd.java @@ -0,0 +1,75 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.user.address; + +import com.cloud.network.PublicIpQuarantine; +import com.cloud.user.Account; +import org.apache.cloudstack.acl.RoleType; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.response.IpQuarantineResponse; + +import java.util.Date; + +@APICommand(name = "updateQuarantinedIp", responseObject = IpQuarantineResponse.class, description = "Updates the quarantine end date for the given public IP address.", + since = "4.19", entityType = {PublicIpQuarantine.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, + authorized = {RoleType.Admin, RoleType.DomainAdmin}) +public class UpdateQuarantinedIpCmd extends BaseCmd { + + @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = IpQuarantineResponse.class, description = "The ID of the public IP address in " + + "active quarantine.") + private Long id; + + @Parameter(name = ApiConstants.IP_ADDRESS, type = CommandType.STRING, description = "The public IP address in active quarantine. Either the IP address is informed, or the ID" + + " of the IP address in quarantine.") + private String ipAddress; + + @Parameter(name = ApiConstants.END_DATE, type = BaseCmd.CommandType.DATE, required = true, description = "The date when the quarantine will no longer be active.") + private Date endDate; + + public Long getId() { + return id; + } + + public String getIpAddress() { + return ipAddress; + } + + public Date getEndDate() { + return endDate; + } + + @Override + public void execute() { + PublicIpQuarantine publicIpQuarantine = _networkService.updatePublicIpAddressInQuarantine(this); + if (publicIpQuarantine == null) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to update public IP quarantine."); + } + IpQuarantineResponse response = _responseGenerator.createQuarantinedIpsResponse(publicIpQuarantine); + response.setResponseName(getCommandName()); + this.setResponseObject(response); + } + + @Override + public long getEntityOwnerId() { + return Account.ACCOUNT_ID_SYSTEM; + } +} diff --git a/api/src/main/java/org/apache/cloudstack/api/response/IpQuarantineResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/IpQuarantineResponse.java new file mode 100644 index 00000000000..55720296315 --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/response/IpQuarantineResponse.java @@ -0,0 +1,130 @@ +//Licensed to the Apache Software Foundation (ASF) under one +//or more contributor license agreements. See the NOTICE file +//distributed with this work for additional information +//regarding copyright ownership. The ASF licenses this file +//to you under the Apache License, Version 2.0 (the +//"License"); you may not use this file except in compliance +//with the License. You may obtain a copy of the License at +// +//http://www.apache.org/licenses/LICENSE-2.0 +// +//Unless required by applicable law or agreed to in writing, +//software distributed under the License is distributed on an +//"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +//KIND, either express or implied. See the License for the +//specific language governing permissions and limitations +//under the License. +package org.apache.cloudstack.api.response; + +import com.cloud.network.PublicIpQuarantine; +import com.cloud.serializer.Param; +import com.google.gson.annotations.SerializedName; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.BaseResponse; +import org.apache.cloudstack.api.EntityReference; + +import java.util.Date; + +@EntityReference(value = {PublicIpQuarantine.class}) +public class IpQuarantineResponse extends BaseResponse { + + @SerializedName(ApiConstants.ID) + @Param(description = "ID of the quarantine process.") + private String id; + + @SerializedName(ApiConstants.IP_ADDRESS) + @Param(description = "The public IP address in quarantine.") + private String publicIpAddress; + + @SerializedName(ApiConstants.PREVIOUS_OWNER_ID) + @Param(description = "Account ID of the previous public IP address owner.") + private String previousOwnerId; + + @SerializedName(ApiConstants.PREVIOUS_OWNER_NAME) + @Param(description = "Account name of the previous public IP address owner.") + private String previousOwnerName; + + @SerializedName(ApiConstants.CREATED) + @Param(description = "When the quarantine was created.") + private Date created; + + @SerializedName(ApiConstants.REMOVED) + @Param(description = "When the quarantine was removed.") + private Date removed; + + @SerializedName(ApiConstants.END_DATE) + @Param(description = "End date for the quarantine.") + private Date endDate; + + @SerializedName(ApiConstants.REMOVAL_REASON) + @Param(description = "The reason for removing the IP from quarantine prematurely.") + private String removalReason; + + public IpQuarantineResponse() { + super("quarantinedips"); + } + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public String getPublicIpAddress() { + return publicIpAddress; + } + + public void setPublicIpAddress(String publicIpAddress) { + this.publicIpAddress = publicIpAddress; + } + + public String getPreviousOwnerId() { + return previousOwnerId; + } + + public void setPreviousOwnerId(String previousOwnerId) { + this.previousOwnerId = previousOwnerId; + } + + public String getPreviousOwnerName() { + return previousOwnerName; + } + + public void setPreviousOwnerName(String previousOwnerName) { + this.previousOwnerName = previousOwnerName; + } + + public Date getCreated() { + return created; + } + + public void setCreated(Date created) { + this.created = created; + } + + public Date getRemoved() { + return removed; + } + + public void setRemoved(Date removed) { + this.removed = removed; + } + + public Date getEndDate() { + return endDate; + } + + public void setEndDate(Date endDate) { + this.endDate = endDate; + } + + public String getRemovalReason() { + return removalReason; + } + + public void setRemovalReason(String removalReason) { + this.removalReason = removalReason; + } +} diff --git a/api/src/main/java/org/apache/cloudstack/query/QueryService.java b/api/src/main/java/org/apache/cloudstack/query/QueryService.java index 097a3c3f262..6f404452598 100644 --- a/api/src/main/java/org/apache/cloudstack/query/QueryService.java +++ b/api/src/main/java/org/apache/cloudstack/query/QueryService.java @@ -34,6 +34,7 @@ import org.apache.cloudstack.api.command.admin.storage.ListStorageTagsCmd; import org.apache.cloudstack.api.command.admin.user.ListUsersCmd; import org.apache.cloudstack.api.command.user.account.ListAccountsCmd; import org.apache.cloudstack.api.command.user.account.ListProjectAccountsCmd; +import org.apache.cloudstack.api.command.user.address.ListQuarantinedIpsCmd; import org.apache.cloudstack.api.command.user.affinitygroup.ListAffinityGroupsCmd; import org.apache.cloudstack.api.command.user.event.ListEventsCmd; import org.apache.cloudstack.api.command.user.iso.ListIsosCmd; @@ -64,6 +65,7 @@ import org.apache.cloudstack.api.response.HostResponse; import org.apache.cloudstack.api.response.HostTagResponse; import org.apache.cloudstack.api.response.ImageStoreResponse; import org.apache.cloudstack.api.response.InstanceGroupResponse; +import org.apache.cloudstack.api.response.IpQuarantineResponse; import org.apache.cloudstack.api.response.ListResponse; import org.apache.cloudstack.api.response.ManagementServerResponse; import org.apache.cloudstack.api.response.ProjectAccountResponse; @@ -183,6 +185,8 @@ public interface QueryService { List listRouterHealthChecks(GetRouterHealthCheckResultsCmd cmd); + ListResponse listQuarantinedIps(ListQuarantinedIpsCmd cmd); + ListResponse listSnapshots(ListSnapshotsCmd cmd); SnapshotResponse listSnapshot(CopySnapshotCmd cmd); diff --git a/engine/components-api/src/main/java/com/cloud/network/IpAddressManager.java b/engine/components-api/src/main/java/com/cloud/network/IpAddressManager.java index ab179d302d0..36937460b20 100644 --- a/engine/components-api/src/main/java/com/cloud/network/IpAddressManager.java +++ b/engine/components-api/src/main/java/com/cloud/network/IpAddressManager.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.network; +import java.util.Date; import java.util.List; import org.apache.cloudstack.api.response.AcquirePodIpCmdResponse; @@ -238,5 +239,52 @@ public interface IpAddressManager { public static final String MESSAGE_ASSIGN_IPADDR_EVENT = "Message.AssignIpAddr.Event"; public static final String MESSAGE_RELEASE_IPADDR_EVENT = "Message.ReleaseIpAddr.Event"; + + /** + * Checks if the given public IP address is not in active quarantine. + * It returns `true` if: + *
    + *
  • The IP was never in quarantine;
    • + *
  • The IP was in quarantine, but the quarantine expired;
    • + *
  • The IP is still in quarantine; however, the new owner is the same as the previous owner, therefore, the IP can be allocated.
    • + *
+ * + * It returns `false` if: + *
    + *
  • The IP is in active quarantine and the new owner is different from the previous owner.
    • + *
+ * + * @param ip used to check if it is in active quarantine. + * @param account used to identify the new owner of the public IP. + * @return true if the IP can be allocated, and false otherwise. + */ + boolean canPublicIpAddressBeAllocated(IpAddress ip, Account account); + + /** + * Adds the given public IP address to quarantine for the duration of the global configuration `public.ip.address.quarantine.duration` value. + * + * @param publicIpAddress to be quarantined. + * @param domainId used to retrieve the quarantine duration. + * @return the {@link PublicIpQuarantine} persisted in the database. + */ + PublicIpQuarantine addPublicIpAddressToQuarantine(IpAddress publicIpAddress, Long domainId); + + /** + * Prematurely removes a public IP address from quarantine. It is required to provide a reason for removing it. + * + * @param quarantineProcessId the ID of the active quarantine process. + * @param removalReason for prematurely removing the public IP address from quarantine. + */ + void removePublicIpAddressFromQuarantine(Long quarantineProcessId, String removalReason); + + /** + * Updates the end date of a public IP address in active quarantine. It can increase and decrease the duration of the quarantine. + * + * @param quarantineProcessId the ID of the quarantine process. + * @param endDate the new end date for the quarantine. + * @return the updated quarantine object. + */ + PublicIpQuarantine updatePublicIpAddressInQuarantine(Long quarantineProcessId, Date endDate); + void updateSourceNatIpAddress(IPAddressVO requestedIp, List userIps) throws Exception; } diff --git a/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDao.java b/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDao.java index bb4822ebb38..f75dc8a6661 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDao.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDao.java @@ -21,6 +21,7 @@ import java.util.List; import com.cloud.dc.Vlan.VlanType; import com.cloud.network.IpAddress.State; import com.cloud.utils.db.GenericDao; +import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.net.Ip; public interface IPAddressDao extends GenericDao { @@ -100,4 +101,6 @@ public interface IPAddressDao extends GenericDao { List listByDcIdAndAssociatedNetwork(long dcId); List listByNetworkId(long networkId); + + void buildQuarantineSearchCriteria(SearchCriteria sc); } diff --git a/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDaoImpl.java index d82ffbe2af0..9ffc4c9159c 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDaoImpl.java @@ -24,6 +24,7 @@ import java.util.List; import javax.annotation.PostConstruct; import javax.inject.Inject; +import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.resourcedetail.dao.UserIpAddressDetailsDao; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -32,6 +33,7 @@ import com.cloud.dc.Vlan.VlanType; import com.cloud.dc.VlanVO; import com.cloud.dc.dao.VlanDao; import com.cloud.network.IpAddress.State; +import com.cloud.network.vo.PublicIpQuarantineVO; import com.cloud.server.ResourceTag.ResourceObjectType; import com.cloud.tags.dao.ResourceTagDao; import com.cloud.utils.db.DB; @@ -69,6 +71,9 @@ public class IPAddressDaoImpl extends GenericDaoBase implemen @Inject UserIpAddressDetailsDao _detailsDao; + @Inject + PublicIpQuarantineDao publicIpQuarantineDao; + // make it public for JUnit test public IPAddressDaoImpl() { } @@ -534,4 +539,19 @@ public class IPAddressDaoImpl extends GenericDaoBase implemen sc.setParameters("state", State.Allocated); return listBy(sc); } + + @Override + public void buildQuarantineSearchCriteria(SearchCriteria sc) { + long accountId = CallContext.current().getCallingAccount().getAccountId(); + SearchBuilder listAllIpsInQuarantine = publicIpQuarantineDao.createSearchBuilder(); + listAllIpsInQuarantine.and("quarantineEndDate", listAllIpsInQuarantine.entity().getEndDate(), SearchCriteria.Op.GT); + listAllIpsInQuarantine.and("previousOwnerId", listAllIpsInQuarantine.entity().getPreviousOwnerId(), Op.NEQ); + + SearchCriteria searchCriteria = listAllIpsInQuarantine.create(); + searchCriteria.setParameters("quarantineEndDate", new Date()); + searchCriteria.setParameters("previousOwnerId", accountId); + Object[] quarantinedIpsIdsAllowedToUser = publicIpQuarantineDao.search(searchCriteria, null).stream().map(PublicIpQuarantineVO::getPublicIpAddressId).toArray(); + + sc.setParametersIfNotNull("quarantinedPublicIpsIdsNIN", quarantinedIpsIdsAllowedToUser); + } } diff --git a/engine/schema/src/main/java/com/cloud/network/dao/IPAddressVO.java b/engine/schema/src/main/java/com/cloud/network/dao/IPAddressVO.java index 7c4d56bd1ee..4c7569a55b9 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/IPAddressVO.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/IPAddressVO.java @@ -29,7 +29,6 @@ import javax.persistence.Id; import javax.persistence.Table; import javax.persistence.Temporal; import javax.persistence.TemporalType; -import javax.persistence.Transient; import com.cloud.network.IpAddress; import com.cloud.utils.db.GenericDao; @@ -97,14 +96,6 @@ public class IPAddressVO implements IpAddress { @Column(name = "is_system") private boolean system; - @Column(name = "account_id") - @Transient - private Long accountId = null; - - @Transient - @Column(name = "domain_id") - private Long domainId = null; - @Column(name = "vpc_id") private Long vpcId; diff --git a/engine/schema/src/main/java/com/cloud/network/dao/PublicIpQuarantineDao.java b/engine/schema/src/main/java/com/cloud/network/dao/PublicIpQuarantineDao.java new file mode 100644 index 00000000000..ccba6bb1889 --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/network/dao/PublicIpQuarantineDao.java @@ -0,0 +1,27 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.network.dao; + +import com.cloud.network.vo.PublicIpQuarantineVO; +import com.cloud.utils.db.GenericDao; + +public interface PublicIpQuarantineDao extends GenericDao { + + PublicIpQuarantineVO findByPublicIpAddressId(long publicIpAddressId); + + PublicIpQuarantineVO findByIpAddress(String publicIpAddress); +} diff --git a/engine/schema/src/main/java/com/cloud/network/dao/PublicIpQuarantineDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/dao/PublicIpQuarantineDaoImpl.java new file mode 100644 index 00000000000..a1b789b8a46 --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/network/dao/PublicIpQuarantineDaoImpl.java @@ -0,0 +1,71 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.network.dao; + +import com.cloud.network.vo.PublicIpQuarantineVO; +import com.cloud.utils.db.Filter; +import com.cloud.utils.db.GenericDaoBase; +import com.cloud.utils.db.JoinBuilder; +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; +import org.springframework.stereotype.Component; + +import javax.annotation.PostConstruct; +import javax.inject.Inject; + +@Component +public class PublicIpQuarantineDaoImpl extends GenericDaoBase implements PublicIpQuarantineDao { + private SearchBuilder publicIpAddressByIdSearch; + + private SearchBuilder ipAddressSearchBuilder; + + @Inject + IPAddressDao ipAddressDao; + + @PostConstruct + public void init() { + publicIpAddressByIdSearch = createSearchBuilder(); + publicIpAddressByIdSearch.and("publicIpAddressId", publicIpAddressByIdSearch.entity().getPublicIpAddressId(), SearchCriteria.Op.EQ); + + ipAddressSearchBuilder = ipAddressDao.createSearchBuilder(); + ipAddressSearchBuilder.and("ipAddress", ipAddressSearchBuilder.entity().getAddress(), SearchCriteria.Op.EQ); + ipAddressSearchBuilder.and("removed", ipAddressSearchBuilder.entity().getRemoved(), SearchCriteria.Op.NULL); + publicIpAddressByIdSearch.join("quarantineJoin", ipAddressSearchBuilder, ipAddressSearchBuilder.entity().getId(), + publicIpAddressByIdSearch.entity().getPublicIpAddressId(), JoinBuilder.JoinType.INNER); + + ipAddressSearchBuilder.done(); + publicIpAddressByIdSearch.done(); + } + + @Override + public PublicIpQuarantineVO findByPublicIpAddressId(long publicIpAddressId) { + SearchCriteria sc = publicIpAddressByIdSearch.create(); + sc.setParameters("publicIpAddressId", publicIpAddressId); + final Filter filter = new Filter(PublicIpQuarantineVO.class, "created", false); + + return findOneBy(sc, filter); + } + + @Override + public PublicIpQuarantineVO findByIpAddress(String publicIpAddress) { + SearchCriteria sc = publicIpAddressByIdSearch.create(); + sc.setJoinParameters("quarantineJoin", "ipAddress", publicIpAddress); + final Filter filter = new Filter(PublicIpQuarantineVO.class, "created", false); + + return findOneBy(sc, filter); + } +} diff --git a/engine/schema/src/main/java/com/cloud/network/vo/PublicIpQuarantineVO.java b/engine/schema/src/main/java/com/cloud/network/vo/PublicIpQuarantineVO.java new file mode 100644 index 00000000000..56d167a0060 --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/network/vo/PublicIpQuarantineVO.java @@ -0,0 +1,131 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.network.vo; + +import com.cloud.network.PublicIpQuarantine; +import com.cloud.utils.db.GenericDao; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import javax.persistence.Temporal; +import javax.persistence.TemporalType; +import java.util.Date; +import java.util.UUID; + +@Entity +@Table(name = "quarantined_ips") +public class PublicIpQuarantineVO implements PublicIpQuarantine { + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "id", nullable = false) + private Long id; + + @Column(name = "uuid", nullable = false) + private String uuid = UUID.randomUUID().toString(); + + @Column(name = "public_ip_address_id", nullable = false) + private Long publicIpAddressId; + + @Column(name = "previous_owner_id", nullable = false) + private Long previousOwnerId; + + @Column(name = GenericDao.CREATED_COLUMN, nullable = false) + @Temporal(value = TemporalType.TIMESTAMP) + private Date created; + + @Column(name = GenericDao.REMOVED_COLUMN) + @Temporal(value = TemporalType.TIMESTAMP) + private Date removed = null; + + @Column(name = "end_date", nullable = false) + @Temporal(value = TemporalType.TIMESTAMP) + private Date endDate; + + @Column(name = "removal_reason") + private String removalReason = null; + + public PublicIpQuarantineVO() { + } + + public PublicIpQuarantineVO(Long publicIpAddressId, Long previousOwnerId, Date created, Date endDate) { + this.publicIpAddressId = publicIpAddressId; + this.previousOwnerId = previousOwnerId; + this.created = created; + this.endDate = endDate; + } + + @Override + public long getId() { + return id; + } + + @Override + public Long getPublicIpAddressId() { + return publicIpAddressId; + } + + @Override + public Long getPreviousOwnerId() { + return previousOwnerId; + } + + @Override + public Date getEndDate() { + return endDate; + } + + @Override + public String getRemovalReason() { + return removalReason; + } + + @Override + public String getUuid() { + return uuid; + } + + public void setEndDate(Date endDate) { + this.endDate = endDate; + } + + public void setRemovalReason(String removalReason) { + this.removalReason = removalReason; + } + + @Override + public Date getRemoved() { + return removed; + } + + public void setRemoved(Date removed) { + this.removed = removed; + } + + @Override + public Date getCreated() { + return created; + } + + public void setCreated(Date created) { + this.created = created; + } +} diff --git a/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml b/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml index 2a0597ce9b5..f0c5e7630e1 100644 --- a/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml +++ b/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml @@ -276,6 +276,7 @@ + diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql b/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql index dd730058b7b..f2585deea1a 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql @@ -136,6 +136,21 @@ UPDATE `cloud`.`console_session` SET removed=now(); -- Modify acquired column in console_session to datetime type ALTER TABLE `cloud`.`console_session` DROP `acquired`, ADD `acquired` datetime COMMENT 'When the session was acquired' AFTER `host_id`; +-- IP quarantine PR#7378 +CREATE TABLE IF NOT EXISTS `cloud`.`quarantined_ips` ( + `id` bigint(20) unsigned NOT NULL auto_increment, + `uuid` varchar(255) UNIQUE, + `public_ip_address_id` bigint(20) unsigned NOT NULL COMMENT 'ID of the quarantined public IP address, foreign key to `user_ip_address` table', + `previous_owner_id` bigint(20) unsigned NOT NULL COMMENT 'ID of the previous owner of the public IP address, foreign key to `account` table', + `created` datetime NOT NULL, + `removed` datetime DEFAULT NULL, + `end_date` datetime NOT NULL, + `removal_reason` VARCHAR(255) DEFAULT NULL, + PRIMARY KEY (`id`), + CONSTRAINT `fk_quarantined_ips__public_ip_address_id` FOREIGN KEY(`public_ip_address_id`) REFERENCES `cloud`.`user_ip_address`(`id`), + CONSTRAINT `fk_quarantined_ips__previous_owner_id` FOREIGN KEY(`previous_owner_id`) REFERENCES `cloud`.`account`(`id`) +); + -- create_public_parameter_on_roles. #6960 ALTER TABLE `cloud`.`roles` ADD COLUMN `public_role` tinyint(1) NOT NULL DEFAULT '1' COMMENT 'Indicates whether the role will be visible to all users (public) or only to root admins (private). If this parameter is not specified during the creation of the role its value will be defaulted to true (public).'; diff --git a/framework/db/src/main/java/com/cloud/utils/db/Filter.java b/framework/db/src/main/java/com/cloud/utils/db/Filter.java index 59dc8c1477e..15161ab058f 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/Filter.java +++ b/framework/db/src/main/java/com/cloud/utils/db/Filter.java @@ -51,6 +51,10 @@ public class Filter { addOrderBy(clazz, field, ascending); } + public Filter(Class clazz, String field, boolean ascending) { + this(clazz, field, ascending, null, null); + } + public Filter(long limit) { _orderBy = " ORDER BY RAND() LIMIT " + limit; } diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java index 1eae0edd9c3..b2a9e6f733a 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java @@ -258,6 +258,8 @@ public interface GenericDao { public T findOneBy(final SearchCriteria sc); + T findOneBy(SearchCriteria sc, Filter filter); + /** * @return */ diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java index 5fd9580342c..6bf36df0941 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java @@ -422,7 +422,7 @@ public abstract class GenericDaoBase extends Compone return result; } catch (final SQLException e) { throw new CloudRuntimeException("DB Exception on: " + pstmt, e); - } catch (final Throwable e) { + } catch (final Exception e) { throw new CloudRuntimeException("Caught: " + pstmt, e); } } @@ -499,7 +499,7 @@ public abstract class GenericDaoBase extends Compone return results; } catch (final SQLException e) { throw new CloudRuntimeException("DB Exception on: " + pstmt, e); - } catch (final Throwable e) { + } catch (final Exception e) { throw new CloudRuntimeException("Caught: " + pstmt, e); } } @@ -907,6 +907,15 @@ public abstract class GenericDaoBase extends Compone return findOneIncludingRemovedBy(sc); } + @Override + @DB() + public T findOneBy(SearchCriteria sc, final Filter filter) { + sc = checkAndSetRemovedIsNull(sc); + filter.setLimit(1L); + List results = searchIncludingRemoved(sc, filter, null, false); + return results.isEmpty() ? null : results.get(0); + } + @DB() protected List listBy(SearchCriteria sc, final Filter filter) { sc = checkAndSetRemovedIsNull(sc); @@ -1145,7 +1154,7 @@ public abstract class GenericDaoBase extends Compone return result; } catch (final SQLException e) { throw new CloudRuntimeException("DB Exception on: " + pstmt, e); - } catch (final Throwable e) { + } catch (final Exception e) { throw new CloudRuntimeException("Caught: " + pstmt, e); } } @@ -1227,7 +1236,7 @@ public abstract class GenericDaoBase extends Compone return pstmt.executeUpdate(); } catch (final SQLException e) { throw new CloudRuntimeException("DB Exception on: " + pstmt, e); - } catch (final Throwable e) { + } catch (final Exception e) { throw new CloudRuntimeException("Caught: " + pstmt, e); } } @@ -2050,7 +2059,7 @@ public abstract class GenericDaoBase extends Compone return 0; } catch (final SQLException e) { throw new CloudRuntimeException("DB Exception on: " + pstmt, e); - } catch (final Throwable e) { + } catch (final Exception e) { throw new CloudRuntimeException("Caught: " + pstmt, e); } } @@ -2101,7 +2110,7 @@ public abstract class GenericDaoBase extends Compone return 0; } catch (final SQLException e) { throw new CloudRuntimeException("DB Exception in executing: " + sql, e); - } catch (final Throwable e) { + } catch (final Exception e) { throw new CloudRuntimeException("Caught exception in : " + sql, e); } } @@ -2158,7 +2167,7 @@ public abstract class GenericDaoBase extends Compone return 0; } catch (final SQLException e) { throw new CloudRuntimeException("DB Exception on: " + pstmt, e); - } catch (final Throwable e) { + } catch (final Exception e) { throw new CloudRuntimeException("Caught: " + pstmt, e); } } diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index 0d00fece4c3..3649197a218 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -100,6 +100,7 @@ import org.apache.cloudstack.api.response.ImageStoreResponse; import org.apache.cloudstack.api.response.InstanceGroupResponse; import org.apache.cloudstack.api.response.InternalLoadBalancerElementResponse; import org.apache.cloudstack.api.response.IpForwardingRuleResponse; +import org.apache.cloudstack.api.response.IpQuarantineResponse; import org.apache.cloudstack.api.response.IpRangeResponse; import org.apache.cloudstack.api.response.Ipv6RouteResponse; import org.apache.cloudstack.api.response.IsolationMethodResponse; @@ -282,6 +283,7 @@ import com.cloud.network.OvsProvider; import com.cloud.network.PhysicalNetwork; import com.cloud.network.PhysicalNetworkServiceProvider; import com.cloud.network.PhysicalNetworkTrafficType; +import com.cloud.network.PublicIpQuarantine; import com.cloud.network.RemoteAccessVpn; import com.cloud.network.RouterHealthCheckResult; import com.cloud.network.Site2SiteCustomerGateway; @@ -5088,4 +5090,23 @@ public class ApiResponseHelper implements ResponseGenerator { response.setObjectName("firewallrule"); return response; } + + @Override + public IpQuarantineResponse createQuarantinedIpsResponse(PublicIpQuarantine quarantinedIp) { + IpQuarantineResponse quarantinedIpsResponse = new IpQuarantineResponse(); + String ipAddress = userIpAddressDao.findById(quarantinedIp.getPublicIpAddressId()).getAddress().toString(); + Account previousOwner = _accountMgr.getAccount(quarantinedIp.getPreviousOwnerId()); + + quarantinedIpsResponse.setId(quarantinedIp.getUuid()); + quarantinedIpsResponse.setPublicIpAddress(ipAddress); + quarantinedIpsResponse.setPreviousOwnerId(previousOwner.getUuid()); + quarantinedIpsResponse.setPreviousOwnerName(previousOwner.getName()); + quarantinedIpsResponse.setCreated(quarantinedIp.getCreated()); + quarantinedIpsResponse.setRemoved(quarantinedIp.getRemoved()); + quarantinedIpsResponse.setEndDate(quarantinedIp.getEndDate()); + quarantinedIpsResponse.setRemovalReason(quarantinedIp.getRemovalReason()); + quarantinedIpsResponse.setResponseName("quarantinedip"); + + return quarantinedIpsResponse; + } } diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 91b8d7eb988..0056be5fce9 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -43,6 +43,9 @@ import com.cloud.network.as.dao.AutoScaleVmGroupDao; import com.cloud.network.as.dao.AutoScaleVmGroupVmMapDao; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; +import com.cloud.network.dao.PublicIpQuarantineDao; +import com.cloud.network.PublicIpQuarantine; +import com.cloud.network.vo.PublicIpQuarantineVO; import com.cloud.storage.dao.VolumeDao; import com.cloud.user.SSHKeyPairVO; import com.cloud.user.dao.SSHKeyPairDao; @@ -88,6 +91,7 @@ import org.apache.cloudstack.api.command.admin.user.ListUsersCmd; import org.apache.cloudstack.api.command.admin.zone.ListZonesCmdByAdmin; import org.apache.cloudstack.api.command.user.account.ListAccountsCmd; import org.apache.cloudstack.api.command.user.account.ListProjectAccountsCmd; +import org.apache.cloudstack.api.command.user.address.ListQuarantinedIpsCmd; import org.apache.cloudstack.api.command.user.affinitygroup.ListAffinityGroupsCmd; import org.apache.cloudstack.api.command.user.event.ListEventsCmd; import org.apache.cloudstack.api.command.user.iso.ListIsosCmd; @@ -119,6 +123,7 @@ import org.apache.cloudstack.api.response.HostResponse; import org.apache.cloudstack.api.response.HostTagResponse; import org.apache.cloudstack.api.response.ImageStoreResponse; import org.apache.cloudstack.api.response.InstanceGroupResponse; +import org.apache.cloudstack.api.response.IpQuarantineResponse; import org.apache.cloudstack.api.response.ListResponse; import org.apache.cloudstack.api.response.ManagementServerResponse; import org.apache.cloudstack.api.response.ProjectAccountResponse; @@ -541,6 +546,9 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q @Inject EntityManager entityManager; + @Inject + private PublicIpQuarantineDao publicIpQuarantineDao; + private SearchCriteria getMinimumCpuServiceOfferingJoinSearchCriteria(int cpu) { SearchCriteria sc = _srvOfferingJoinDao.createSearchCriteria(); SearchCriteria sc1 = _srvOfferingJoinDao.createSearchCriteria(); @@ -4754,6 +4762,49 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q } @Override + public ListResponse listQuarantinedIps(ListQuarantinedIpsCmd cmd) { + ListResponse response = new ListResponse<>(); + Pair, Integer> result = listQuarantinedIpsInternal(cmd.isShowRemoved(), cmd.isShowInactive()); + List ipsQuarantinedResponses = new ArrayList<>(); + + for (PublicIpQuarantine quarantinedIp : result.first()) { + IpQuarantineResponse ipsInQuarantineResponse = responseGenerator.createQuarantinedIpsResponse(quarantinedIp); + ipsQuarantinedResponses.add(ipsInQuarantineResponse); + } + + response.setResponses(ipsQuarantinedResponses); + return response; + } + + /** + * It lists the quarantine IPs that the caller account is allowed to see by filtering the domain path of the caller account. + * Furthermore, it lists inactive and removed quarantined IPs according to the command parameters. + */ + private Pair, Integer> listQuarantinedIpsInternal(boolean showRemoved, boolean showInactive) { + String callingAccountDomainPath = _domainDao.findById(CallContext.current().getCallingAccount().getDomainId()).getPath(); + + SearchBuilder filterAllowedOnly = _accountJoinDao.createSearchBuilder(); + filterAllowedOnly.and("path", filterAllowedOnly.entity().getDomainPath(), SearchCriteria.Op.LIKE); + + SearchBuilder listAllPublicIpsInQuarantineAllowedToTheCaller = publicIpQuarantineDao.createSearchBuilder(); + listAllPublicIpsInQuarantineAllowedToTheCaller.join("listQuarantinedJoin", filterAllowedOnly, + listAllPublicIpsInQuarantineAllowedToTheCaller.entity().getPreviousOwnerId(), + filterAllowedOnly.entity().getId(), JoinBuilder.JoinType.INNER); + + if (!showInactive) { + listAllPublicIpsInQuarantineAllowedToTheCaller.and("endDate", listAllPublicIpsInQuarantineAllowedToTheCaller.entity().getEndDate(), SearchCriteria.Op.GT); + } + + filterAllowedOnly.done(); + listAllPublicIpsInQuarantineAllowedToTheCaller.done(); + + SearchCriteria searchCriteria = listAllPublicIpsInQuarantineAllowedToTheCaller.create(); + searchCriteria.setJoinParameters("listQuarantinedJoin", "path", callingAccountDomainPath + "%"); + searchCriteria.setParametersIfNotNull("endDate", new Date()); + + return publicIpQuarantineDao.searchAndCount(searchCriteria, null, showRemoved); + } + public ListResponse listSnapshots(ListSnapshotsCmd cmd) { Account caller = CallContext.current().getCallingAccount(); Pair, Integer> result = searchForSnapshotsWithParams(cmd.getId(), cmd.getIds(), diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index c85328807df..75ea572491e 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -18,11 +18,13 @@ package com.cloud.network; import java.util.ArrayList; import java.util.Arrays; +import java.util.Calendar; import java.util.Date; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Random; import java.util.Set; import java.util.UUID; @@ -31,6 +33,8 @@ import java.util.stream.Collectors; import javax.inject.Inject; +import com.cloud.network.dao.PublicIpQuarantineDao; +import com.cloud.network.vo.PublicIpQuarantineVO; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.annotation.AnnotationService; @@ -308,6 +312,9 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage @Inject MessageBus messageBus; + @Inject + PublicIpQuarantineDao publicIpQuarantineDao; + SearchBuilder AssignIpAddressSearch; SearchBuilder AssignIpAddressFromPodVlanSearch; private static final Object allocatedLock = new Object(); @@ -318,6 +325,9 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage Boolean.class, "system.vm.public.ip.reservation.mode.strictness", "false", "If enabled, the use of System VMs public IP reservation is strict, preferred if not.", true, ConfigKey.Scope.Global); + public static final ConfigKey PUBLIC_IP_ADDRESS_QUARANTINE_DURATION = new ConfigKey<>("Network", Integer.class, "public.ip.address.quarantine.duration", + "0", "The duration (in minutes) for the public IP address to be quarantined when it is disassociated.", true, ConfigKey.Scope.Domain); + private Random rand = new Random(System.currentTimeMillis()); private List getIpv6SupportingVlanRangeIds(long dcId) throws InsufficientAddressCapacityException { @@ -523,8 +533,7 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage return true; } - private IpAddress allocateIP(Account ipOwner, boolean isSystem, long zoneId) throws ResourceAllocationException, InsufficientAddressCapacityException, - ConcurrentOperationException { + private IpAddress allocateIP(Account ipOwner, boolean isSystem, long zoneId) throws InsufficientAddressCapacityException, ConcurrentOperationException { Account caller = CallContext.current().getCallingAccount(); long callerUserId = CallContext.current().getCallingUserId(); // check permissions @@ -698,6 +707,9 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage public boolean disassociatePublicIpAddress(long addrId, long userId, Account caller) { boolean success = true; + IPAddressVO ipToBeDisassociated = _ipAddressDao.findById(addrId); + + PublicIpQuarantine publicIpQuarantine = null; // Cleanup all ip address resources - PF/LB/Static nat rules if (!cleanupIpResources(addrId, userId, caller)) { success = false; @@ -723,10 +735,9 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage } catch (ResourceUnavailableException e) { throw new CloudRuntimeException("We should never get to here because we used true when applyIpAssociations", e); } - } else { - if (ip.getState() == IpAddress.State.Releasing) { - _ipAddressDao.unassignIpAddress(ip.getId()); - } + } else if (ip.getState() == State.Releasing) { + publicIpQuarantine = addPublicIpAddressToQuarantine(ipToBeDisassociated, caller.getDomainId()); + _ipAddressDao.unassignIpAddress(ip.getId()); } annotationDao.removeByEntityType(AnnotationService.EntityType.PUBLIC_IP_ADDRESS.name(), ip.getUuid()); @@ -736,6 +747,8 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage releasePortableIpAddress(addrId); } s_logger.debug("Released a public ip id=" + addrId); + } else if (publicIpQuarantine != null) { + removePublicIpAddressFromQuarantine(publicIpQuarantine.getId(), "Public IP address removed from quarantine as there was an error while disassociating it."); } return success; @@ -972,6 +985,13 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage if (lockOneRow) { assert (addrs.size() == 1) : "Return size is incorrect: " + addrs.size(); + IpAddress ipAddress = addrs.get(0); + boolean ipCanBeAllocated = canPublicIpAddressBeAllocated(ipAddress, owner); + + if (!ipCanBeAllocated) { + throw new InsufficientAddressCapacityException(String.format("Failed to allocate public IP address [%s] as it is in quarantine.", ipAddress.getAddress()), + DataCenter.class, dcId); + } } if (assign && !fetchFromDedicatedRange && VlanType.VirtualNetwork.equals(vlanUse)) { @@ -1126,6 +1146,7 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage } else if (addr.getState() == IpAddress.State.Releasing) { // Cleanup all the resources for ip address if there are any, and only then un-assign ip in the system if (cleanupIpResources(addr.getId(), Account.ACCOUNT_ID_SYSTEM, _accountMgr.getSystemAccount())) { + addPublicIpAddressToQuarantine(addr, network.getDomainId()); _ipAddressDao.unassignIpAddress(addr.getId()); messageBus.publish(_name, MESSAGE_RELEASE_IPADDR_EVENT, PublishScope.LOCAL, addr); } else { @@ -1258,8 +1279,7 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage @DB @Override public IpAddress allocateIp(final Account ipOwner, final boolean isSystem, Account caller, long callerUserId, final DataCenter zone, final Boolean displayIp, final String ipaddress) - throws ConcurrentOperationException, - ResourceAllocationException, InsufficientAddressCapacityException { + throws ConcurrentOperationException, InsufficientAddressCapacityException, CloudRuntimeException { final VlanType vlanType = VlanType.VirtualNetwork; final boolean assign = false; @@ -2347,7 +2367,8 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {UseSystemPublicIps, RulesContinueOnError, SystemVmPublicIpReservationModeStrictness, VrouterRedundantTiersPlacement, AllowUserListAvailableIpsOnSharedNetwork}; + return new ConfigKey[] {UseSystemPublicIps, RulesContinueOnError, SystemVmPublicIpReservationModeStrictness, VrouterRedundantTiersPlacement, AllowUserListAvailableIpsOnSharedNetwork, + PUBLIC_IP_ADDRESS_QUARANTINE_DURATION}; } /** @@ -2381,6 +2402,96 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage return SystemVmPublicIpReservationModeStrictness; } + @Override + public boolean canPublicIpAddressBeAllocated(IpAddress ip, Account newOwner) { + PublicIpQuarantineVO publicIpQuarantineVO = publicIpQuarantineDao.findByPublicIpAddressId(ip.getId()); + + if (publicIpQuarantineVO == null) { + s_logger.debug(String.format("Public IP address [%s] is not in quarantine; therefore, it is allowed to be allocated.", ip)); + return true; + } + + if (!isPublicIpAddressStillInQuarantine(publicIpQuarantineVO, new Date())) { + s_logger.debug(String.format("Public IP address [%s] is no longer in quarantine; therefore, it is allowed to be allocated.", ip)); + return true; + } + + Account previousOwner = _accountMgr.getAccount(publicIpQuarantineVO.getPreviousOwnerId()); + + if (Objects.equals(previousOwner.getUuid(), newOwner.getUuid())) { + s_logger.debug(String.format("Public IP address [%s] is in quarantine; however, the Public IP previous owner [%s] is the same as the new owner [%s]; therefore the IP" + + " can be allocated. The public IP address will be removed from quarantine.", ip, previousOwner, newOwner)); + removePublicIpAddressFromQuarantine(publicIpQuarantineVO.getId(), "IP was removed from quarantine because it has been allocated by the previous owner"); + return true; + } + + s_logger.error(String.format("Public IP address [%s] is in quarantine and the previous owner [%s] is different than the new owner [%s]; therefore, the IP cannot be " + + "allocated.", ip, previousOwner, newOwner)); + return false; + } + + public boolean isPublicIpAddressStillInQuarantine(PublicIpQuarantineVO publicIpQuarantineVO, Date currentDate) { + Date quarantineEndDate = publicIpQuarantineVO.getEndDate(); + Date removedDate = publicIpQuarantineVO.getRemoved(); + boolean hasQuarantineEndedEarly = removedDate != null; + + return hasQuarantineEndedEarly && currentDate.before(removedDate) || + !hasQuarantineEndedEarly && currentDate.before(quarantineEndDate); + } + + @Override + public PublicIpQuarantine addPublicIpAddressToQuarantine(IpAddress publicIpAddress, Long domainId) { + Integer quarantineDuration = PUBLIC_IP_ADDRESS_QUARANTINE_DURATION.valueInDomain(domainId); + if (quarantineDuration <= 0) { + s_logger.debug(String.format("Not adding IP [%s] to quarantine because configuration [%s] has value equal or less to 0.", publicIpAddress.getAddress(), + PUBLIC_IP_ADDRESS_QUARANTINE_DURATION.key())); + return null; + } + + long ipId = publicIpAddress.getId(); + long accountId = publicIpAddress.getAccountId(); + + if (accountId == Account.ACCOUNT_ID_SYSTEM) { + s_logger.debug(String.format("Not adding IP [%s] to quarantine because it belongs to the system account.", publicIpAddress.getAddress())); + return null; + } + + Date currentDate = new Date(); + Calendar quarantineEndDate = Calendar.getInstance(); + quarantineEndDate.setTime(currentDate); + quarantineEndDate.add(Calendar.MINUTE, quarantineDuration); + + PublicIpQuarantineVO publicIpQuarantine = new PublicIpQuarantineVO(ipId, accountId, currentDate, quarantineEndDate.getTime()); + s_logger.debug(String.format("Adding public IP Address [%s] to quarantine for the duration of [%s] minute(s).", publicIpAddress.getAddress(), quarantineDuration)); + return publicIpQuarantineDao.persist(publicIpQuarantine); + } + + @Override + public void removePublicIpAddressFromQuarantine(Long quarantineProcessId, String removalReason) { + PublicIpQuarantineVO publicIpQuarantineVO = publicIpQuarantineDao.findById(quarantineProcessId); + Ip ipAddress = _ipAddressDao.findById(publicIpQuarantineVO.getPublicIpAddressId()).getAddress(); + Date removedDate = new Date(); + + publicIpQuarantineVO.setRemoved(removedDate); + publicIpQuarantineVO.setRemovalReason(removalReason); + + s_logger.debug(String.format("Removing public IP Address [%s] from quarantine by updating the removed date to [%s].", ipAddress, removedDate)); + publicIpQuarantineDao.persist(publicIpQuarantineVO); + } + + @Override + public PublicIpQuarantine updatePublicIpAddressInQuarantine(Long quarantineProcessId, Date newEndDate) { + PublicIpQuarantineVO publicIpQuarantineVO = publicIpQuarantineDao.findById(quarantineProcessId); + Ip ipAddress = _ipAddressDao.findById(publicIpQuarantineVO.getPublicIpAddressId()).getAddress(); + Date currentEndDate = publicIpQuarantineVO.getEndDate(); + + publicIpQuarantineVO.setEndDate(newEndDate); + + s_logger.debug(String.format("Updating the end date for the quarantine of the public IP Address [%s] from [%s] to [%s].", ipAddress, currentEndDate, newEndDate)); + publicIpQuarantineDao.persist(publicIpQuarantineVO); + return publicIpQuarantineVO; + } + @Override public void updateSourceNatIpAddress(IPAddressVO requestedIp, List userIps) throws Exception{ Transaction.execute((TransactionCallbackWithException) status -> { diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index 55373f0dd0f..0c17efaab36 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -41,6 +41,7 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.network.dao.PublicIpQuarantineDao; import com.cloud.offering.ServiceOffering; import com.cloud.service.dao.ServiceOfferingDao; import org.apache.cloudstack.acl.ControlledEntity.ACLType; @@ -55,6 +56,8 @@ import org.apache.cloudstack.api.command.admin.network.ListGuestVlansCmd; import org.apache.cloudstack.api.command.admin.network.ListNetworksCmdByAdmin; import org.apache.cloudstack.api.command.admin.network.UpdateNetworkCmdByAdmin; import org.apache.cloudstack.api.command.admin.usage.ListTrafficTypeImplementorsCmd; +import org.apache.cloudstack.api.command.user.address.RemoveQuarantinedIpCmd; +import org.apache.cloudstack.api.command.user.address.UpdateQuarantinedIpCmd; import org.apache.cloudstack.api.command.user.network.CreateNetworkCmd; import org.apache.cloudstack.api.command.user.network.CreateNetworkPermissionsCmd; import org.apache.cloudstack.api.command.user.network.ListNetworkPermissionsCmd; @@ -401,6 +404,8 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C CommandSetupHelper commandSetupHelper; @Inject ServiceOfferingDao serviceOfferingDao; + @Inject + PublicIpQuarantineDao publicIpQuarantineDao; @Autowired @Qualifier("networkHelper") @@ -5939,4 +5944,75 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C public ConfigKey[] getConfigKeys() { return new ConfigKey[] {AllowDuplicateNetworkName, AllowEmptyStartEndIpAddress, VRPrivateInterfaceMtu, VRPublicInterfaceMtu, AllowUsersToSpecifyVRMtu}; } + + @Override + public PublicIpQuarantine updatePublicIpAddressInQuarantine(UpdateQuarantinedIpCmd cmd) throws CloudRuntimeException { + Long ipId = cmd.getId(); + String ipAddress = cmd.getIpAddress(); + Date newEndDate = cmd.getEndDate(); + + if (new Date().after(newEndDate)) { + throw new InvalidParameterValueException(String.format("The given end date [%s] is invalid as it is before the current date.", newEndDate)); + } + + PublicIpQuarantine publicIpQuarantine = retrievePublicIpQuarantine(ipId, ipAddress); + checkCallerForPublicIpQuarantineAccess(publicIpQuarantine); + + String publicIpQuarantineAddress = _ipAddressDao.findById(publicIpQuarantine.getPublicIpAddressId()).getAddress().toString(); + Date currentEndDate = publicIpQuarantine.getEndDate(); + + if (new Date().after(currentEndDate)) { + throw new CloudRuntimeException(String.format("The quarantine for the public IP address [%s] is no longer active; thus, it cannot be updated.", publicIpQuarantineAddress)); + } + + return _ipAddrMgr.updatePublicIpAddressInQuarantine(publicIpQuarantine.getId(), newEndDate); + } + + @Override + public void removePublicIpAddressFromQuarantine(RemoveQuarantinedIpCmd cmd) throws CloudRuntimeException { + Long ipId = cmd.getId(); + String ipAddress = cmd.getIpAddress(); + PublicIpQuarantine publicIpQuarantine = retrievePublicIpQuarantine(ipId, ipAddress); + + String removalReason = cmd.getRemovalReason(); + if (StringUtils.isBlank(removalReason)) { + s_logger.error("The removalReason parameter cannot be blank."); + ipAddress = ObjectUtils.defaultIfNull(ipAddress, _ipAddressDao.findById(publicIpQuarantine.getPublicIpAddressId()).getAddress().toString()); + throw new CloudRuntimeException(String.format("The given reason for removing the public IP address [%s] from quarantine is blank.", ipAddress)); + } + + checkCallerForPublicIpQuarantineAccess(publicIpQuarantine); + + _ipAddrMgr.removePublicIpAddressFromQuarantine(publicIpQuarantine.getId(), removalReason); + } + + /** + * Retrieves the active quarantine for the given public IP address. It can find by the ID of the quarantine or the address of the public IP. + * @throws CloudRuntimeException if it does not find an active quarantine for the given public IP. + */ + protected PublicIpQuarantine retrievePublicIpQuarantine(Long ipId, String ipAddress) throws CloudRuntimeException { + PublicIpQuarantine publicIpQuarantine; + if (ipId != null) { + s_logger.debug("The ID of the IP in quarantine was informed; therefore, the `ipAddress` parameter will be ignored."); + publicIpQuarantine = publicIpQuarantineDao.findById(ipId); + } else if (ipAddress != null) { + s_logger.debug("The address of the IP in quarantine was informed, it will be used to fetch its metadata."); + publicIpQuarantine = publicIpQuarantineDao.findByIpAddress(ipAddress); + } else { + throw new CloudRuntimeException("Either the ID or the address of the IP in quarantine must be informed."); + } + + if (publicIpQuarantine == null) { + throw new CloudRuntimeException("There is no active quarantine for the specified IP address."); + } + + return publicIpQuarantine; + } + + protected void checkCallerForPublicIpQuarantineAccess(PublicIpQuarantine publicIpQuarantine) { + Account callingAccount = CallContext.current().getCallingAccount(); + DomainVO domainOfThePreviousOwner = _domainDao.findById(_accountDao.findById(publicIpQuarantine.getPreviousOwnerId()).getDomainId()); + + _accountMgr.checkAccess(callingAccount, domainOfThePreviousOwner); + } } diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index 4456f10b546..b4cb73f035d 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -1105,8 +1105,12 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis Vpc vpc = createVpc(cmd.getZoneId(), cmd.getVpcOffering(), cmd.getEntityOwnerId(), cmd.getVpcName(), cmd.getDisplayText(), cmd.getCidr(), cmd.getNetworkDomain(), cmd.getIp4Dns1(), cmd.getIp4Dns2(), cmd.getIp6Dns1(), cmd.getIp6Dns2(), cmd.isDisplay(), cmd.getPublicMtu()); - // associate cmd.getSourceNatIP() with this vpc - allocateSourceNatIp(vpc, cmd.getSourceNatIP()); + + String sourceNatIP = cmd.getSourceNatIP(); + if (sourceNatIP != null) { + s_logger.info(String.format("Trying to allocate the specified IP [%s] as the source NAT of VPC [%s].", sourceNatIP, vpc)); + allocateSourceNatIp(vpc, sourceNatIP); + } return vpc; } diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 5be85d051f4..7548b768327 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -43,6 +43,7 @@ import javax.crypto.spec.SecretKeySpec; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.network.dao.PublicIpQuarantineDao; import com.cloud.hypervisor.HypervisorGuru; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker; @@ -329,9 +330,12 @@ import org.apache.cloudstack.api.command.user.account.ListProjectAccountsCmd; import org.apache.cloudstack.api.command.user.address.AssociateIPAddrCmd; import org.apache.cloudstack.api.command.user.address.DisassociateIPAddrCmd; import org.apache.cloudstack.api.command.user.address.ListPublicIpAddressesCmd; +import org.apache.cloudstack.api.command.user.address.ListQuarantinedIpsCmd; import org.apache.cloudstack.api.command.user.address.ReleaseIPAddrCmd; +import org.apache.cloudstack.api.command.user.address.RemoveQuarantinedIpCmd; import org.apache.cloudstack.api.command.user.address.ReserveIPAddrCmd; import org.apache.cloudstack.api.command.user.address.UpdateIPAddrCmd; +import org.apache.cloudstack.api.command.user.address.UpdateQuarantinedIpCmd; import org.apache.cloudstack.api.command.user.affinitygroup.CreateAffinityGroupCmd; import org.apache.cloudstack.api.command.user.affinitygroup.DeleteAffinityGroupCmd; import org.apache.cloudstack.api.command.user.affinitygroup.ListAffinityGroupTypesCmd; @@ -979,6 +983,9 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe @Inject UserDataManager userDataManager; + @Inject + private PublicIpQuarantineDao publicIpQuarantineDao; + private LockControllerListener _lockControllerListener; private final ScheduledExecutorService _eventExecutor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("EventChecker")); private final ScheduledExecutorService _alertExecutor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("AlertChecker")); @@ -2508,10 +2515,12 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe final SearchBuilder sb2 = _publicIpAddressDao.createSearchBuilder(); buildParameters(sb2, cmd, false); sb2.and("ids", sb2.entity().getId(), SearchCriteria.Op.IN); + sb2.and("quarantinedPublicIpsIdsNIN", sb2.entity().getId(), SearchCriteria.Op.NIN); SearchCriteria sc2 = sb2.create(); setParameters(sc2, cmd, vlanType, isAllocated); sc2.setParameters("ids", freeAddrIds.toArray()); + _publicIpAddressDao.buildQuarantineSearchCriteria(sc2); addrs.addAll(_publicIpAddressDao.search(sc2, searchFilter)); // Allocated + Free } Collections.sort(addrs, Comparator.comparing(IPAddressVO::getAddress)); @@ -3798,6 +3807,9 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe cmdList.add(UpdateRemoteAccessVpnCmd.class); cmdList.add(UpdateVpnConnectionCmd.class); cmdList.add(UpdateVpnGatewayCmd.class); + cmdList.add(ListQuarantinedIpsCmd.class); + cmdList.add(UpdateQuarantinedIpCmd.class); + cmdList.add(RemoveQuarantinedIpCmd.class); // separated admin commands cmdList.add(ListAccountsCmdByAdmin.class); cmdList.add(ListZonesCmdByAdmin.class); diff --git a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java index d7863e438e6..935fb4e8c3b 100644 --- a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java +++ b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java @@ -17,19 +17,25 @@ package com.cloud.network; -import com.cloud.exception.ResourceUnavailableException; -import com.cloud.network.Network.Service; -import com.cloud.network.dao.IPAddressDao; -import com.cloud.network.dao.IPAddressVO; -import com.cloud.network.dao.NetworkDao; -import com.cloud.network.dao.NetworkVO; -import com.cloud.network.rules.StaticNat; -import com.cloud.network.rules.StaticNatImpl; -import com.cloud.offerings.NetworkOfferingVO; -import com.cloud.offerings.dao.NetworkOfferingDao; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Date; +import java.util.List; +import java.util.Vector; + +import com.cloud.network.dao.PublicIpQuarantineDao; +import com.cloud.network.vo.PublicIpQuarantineVO; import com.cloud.user.Account; -import com.cloud.user.AccountVO; -import com.cloud.utils.net.Ip; +import com.cloud.user.AccountManager; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -40,19 +46,18 @@ import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.runners.MockitoJUnitRunner; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Vector; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.anyLong; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.lenient; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import com.cloud.exception.ResourceUnavailableException; +import com.cloud.network.Network.Service; +import com.cloud.network.dao.IPAddressDao; +import com.cloud.network.dao.IPAddressVO; +import com.cloud.network.dao.NetworkDao; +import com.cloud.network.dao.NetworkVO; +import com.cloud.network.rules.StaticNat; +import com.cloud.network.rules.StaticNatImpl; +import com.cloud.offerings.NetworkOfferingVO; +import com.cloud.offerings.dao.NetworkOfferingDao; +import com.cloud.user.AccountVO; +import com.cloud.utils.net.Ip; @RunWith(MockitoJUnitRunner.class) public class IpAddressManagerTest { @@ -80,6 +85,34 @@ public class IpAddressManagerTest { AccountVO account; + @Mock + PublicIpQuarantineVO publicIpQuarantineVOMock; + + @Mock + PublicIpQuarantineDao publicIpQuarantineDaoMock; + + @Mock + IpAddress ipAddressMock; + + @Mock + AccountVO newOwnerMock; + + @Mock + AccountVO previousOwnerMock; + + @Mock + AccountManager accountManagerMock; + + final long dummyID = 1L; + + final String UUID = "uuid"; + + private static final Date currentDate = new Date(100L); + + private static final Date beforeCurrentDate = new Date(99L); + + private static final Date afterCurrentDate = new Date(101L); + @Before public void setup() throws ResourceUnavailableException { @@ -234,6 +267,136 @@ public class IpAddressManagerTest { return network; } + @Test + public void isPublicIpAddressStillInQuarantineTestRemovedDateIsNullAndCurrentDateIsEqualToEndDateShouldReturnFalse() { + Date endDate = currentDate; + + Mockito.when(publicIpQuarantineVOMock.getRemoved()).thenReturn(null); + Mockito.when(publicIpQuarantineVOMock.getEndDate()).thenReturn(endDate); + + boolean result = ipAddressManager.isPublicIpAddressStillInQuarantine(publicIpQuarantineVOMock, currentDate); + + Assert.assertFalse(result); + } + + @Test + public void isPublicIpAddressStillInQuarantineTestRemovedDateIsNullAndEndDateIsBeforeCurrentDateShouldReturnFalse() { + Date endDate = beforeCurrentDate; + + Mockito.when(publicIpQuarantineVOMock.getRemoved()).thenReturn(null); + Mockito.when(publicIpQuarantineVOMock.getEndDate()).thenReturn(endDate); + + boolean result = ipAddressManager.isPublicIpAddressStillInQuarantine(publicIpQuarantineVOMock, currentDate); + + Assert.assertFalse(result); + } + + @Test + public void isPublicIpAddressStillInQuarantineTestRemovedDateIsNullAndEndDateIsAfterCurrentDateShouldReturnTrue() { + Date endDate = afterCurrentDate; + + Mockito.when(publicIpQuarantineVOMock.getRemoved()).thenReturn(null); + Mockito.when(publicIpQuarantineVOMock.getEndDate()).thenReturn(endDate); + + boolean result = ipAddressManager.isPublicIpAddressStillInQuarantine(publicIpQuarantineVOMock, currentDate); + + Assert.assertTrue(result); + } + + @Test + public void isPublicIpAddressStillInQuarantineTestRemovedDateIsEqualCurrentDateShouldReturnFalse() { + Date removedDate = currentDate; + + Mockito.when(publicIpQuarantineVOMock.getEndDate()).thenReturn(currentDate); + Mockito.when(publicIpQuarantineVOMock.getRemoved()).thenReturn(removedDate); + + boolean result = ipAddressManager.isPublicIpAddressStillInQuarantine(publicIpQuarantineVOMock, currentDate); + + Assert.assertFalse(result); + } + + @Test + public void isPublicIpAddressStillInQuarantineTestRemovedDateIsBeforeCurrentDateShouldReturnFalse() { + Date removedDate = beforeCurrentDate; + + Mockito.when(publicIpQuarantineVOMock.getRemoved()).thenReturn(removedDate); + Mockito.when(publicIpQuarantineVOMock.getEndDate()).thenReturn(null); + + boolean result = ipAddressManager.isPublicIpAddressStillInQuarantine(publicIpQuarantineVOMock, currentDate); + + Assert.assertFalse(result); + } + + @Test + public void isPublicIpAddressStillInQuarantineTestRemovedDateIsAfterCurrentDateShouldReturnTrue() { + Date removedDate = afterCurrentDate; + + Mockito.when(publicIpQuarantineVOMock.getRemoved()).thenReturn(removedDate); + Mockito.when(publicIpQuarantineVOMock.getEndDate()).thenReturn(null); + + boolean result = ipAddressManager.isPublicIpAddressStillInQuarantine(publicIpQuarantineVOMock, currentDate); + + Assert.assertTrue(result); + } + + @Test + public void checkIfPublicIpAddressIsNotInQuarantineAndCanBeAllocatedTestIpIsNotInQuarantineShouldReturnTrue() { + Mockito.when(ipAddressMock.getId()).thenReturn(dummyID); + Mockito.when(publicIpQuarantineDaoMock.findByPublicIpAddressId(Mockito.anyLong())).thenReturn(null); + + boolean result = ipAddressManager.canPublicIpAddressBeAllocated(ipAddressMock, account); + + Assert.assertTrue(result); + } + + @Test + public void checkIfPublicIpAddressIsNotInQuarantineAndCanBeAllocatedTestIpIsNoLongerInQuarantineShouldReturnTrue() { + Mockito.when(ipAddressMock.getId()).thenReturn(dummyID); + Mockito.when(publicIpQuarantineDaoMock.findByPublicIpAddressId(Mockito.anyLong())).thenReturn(publicIpQuarantineVOMock); + Mockito.doReturn(false).when(ipAddressManager).isPublicIpAddressStillInQuarantine(Mockito.any(PublicIpQuarantineVO.class), Mockito.any(Date.class)); + + boolean result = ipAddressManager.canPublicIpAddressBeAllocated(ipAddressMock, newOwnerMock); + + Assert.assertTrue(result); + } + + @Test + public void checkIfPublicIpAddressIsNotInQuarantineAndCanBeAllocatedTestIpIsInQuarantineAndThePreviousOwnerIsTheSameAsTheNewOwnerShouldReturnTrue() { + Mockito.when(ipAddressMock.getId()).thenReturn(dummyID); + Mockito.when(publicIpQuarantineDaoMock.findByPublicIpAddressId(Mockito.anyLong())).thenReturn(publicIpQuarantineVOMock); + + Mockito.doReturn(true).when(ipAddressManager).isPublicIpAddressStillInQuarantine(Mockito.any(PublicIpQuarantineVO.class), Mockito.any(Date.class)); + Mockito.doNothing().when(ipAddressManager).removePublicIpAddressFromQuarantine(Mockito.anyLong(), Mockito.anyString()); + + Mockito.when(publicIpQuarantineVOMock.getPreviousOwnerId()).thenReturn(dummyID); + Mockito.when(accountManagerMock.getAccount(Mockito.anyLong())).thenReturn(previousOwnerMock); + Mockito.when(previousOwnerMock.getUuid()).thenReturn(UUID); + Mockito.when(newOwnerMock.getUuid()).thenReturn(UUID); + + boolean result = ipAddressManager.canPublicIpAddressBeAllocated(ipAddressMock, newOwnerMock); + + Assert.assertTrue(result); + } + + @Test + public void checkIfPublicIpAddressIsNotInQuarantineAndCanBeAllocatedTestIpIsInQuarantineAndThePreviousOwnerIsDifferentFromTheNewOwnerShouldReturnFalse() { + final String UUID_2 = "uuid_2"; + + Mockito.when(ipAddressMock.getId()).thenReturn(dummyID); + Mockito.when(publicIpQuarantineDaoMock.findByPublicIpAddressId(Mockito.anyLong())).thenReturn(publicIpQuarantineVOMock); + + Mockito.doReturn(true).when(ipAddressManager).isPublicIpAddressStillInQuarantine(Mockito.any(PublicIpQuarantineVO.class), Mockito.any(Date.class)); + + Mockito.when(publicIpQuarantineVOMock.getPreviousOwnerId()).thenReturn(dummyID); + Mockito.when(accountManagerMock.getAccount(Mockito.anyLong())).thenReturn(previousOwnerMock); + Mockito.when(previousOwnerMock.getUuid()).thenReturn(UUID); + Mockito.when(newOwnerMock.getUuid()).thenReturn(UUID_2); + + boolean result = ipAddressManager.canPublicIpAddressBeAllocated(ipAddressMock, newOwnerMock); + + Assert.assertFalse(result); + } + @Test public void updateSourceNatIpAddress() throws Exception { IPAddressVO requestedIp = Mockito.mock(IPAddressVO.class); diff --git a/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java b/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java index d6f5dbd9a7b..c993f7b7095 100644 --- a/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java +++ b/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java @@ -16,13 +16,60 @@ // under the License. package com.cloud.network; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.nullable; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.doReturn; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Calendar; +import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; + +import com.cloud.domain.Domain; +import com.cloud.domain.DomainVO; +import com.cloud.domain.dao.DomainDao; +import com.cloud.network.dao.PublicIpQuarantineDao; +import com.cloud.network.vo.PublicIpQuarantineVO; +import com.cloud.user.dao.AccountDao; +import com.cloud.utils.net.Ip; +import com.cloud.exception.InsufficientAddressCapacityException; +import org.apache.cloudstack.alert.AlertService; +import org.apache.cloudstack.api.command.user.address.UpdateQuarantinedIpCmd; +import org.apache.cloudstack.api.command.user.network.CreateNetworkCmd; +import org.apache.cloudstack.api.command.user.network.UpdateNetworkCmd; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentMatchers; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.springframework.test.util.ReflectionTestUtils; + import com.cloud.agent.api.to.IpAddressTO; import com.cloud.alert.AlertManager; import com.cloud.configuration.ConfigurationManager; import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.DataCenterDao; -import com.cloud.exception.InsufficientAddressCapacityException; import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ResourceAllocationException; @@ -60,44 +107,9 @@ import com.cloud.vm.NicVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.dao.DomainRouterDao; import com.cloud.vm.dao.NicDao; -import org.apache.cloudstack.alert.AlertService; -import org.apache.cloudstack.api.command.user.network.CreateNetworkCmd; -import org.apache.cloudstack.api.command.user.network.UpdateNetworkCmd; -import org.apache.cloudstack.context.CallContext; -import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; -import org.apache.cloudstack.framework.config.ConfigKey; import org.junit.After; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.ArgumentMatchers; -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 org.springframework.test.util.ReflectionTestUtils; - -import java.lang.reflect.Field; -import java.util.ArrayList; -import java.util.HashMap; -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.anyLong; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.nullable; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.lenient; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class NetworkServiceImplTest { @@ -157,7 +169,7 @@ public class NetworkServiceImplTest { ServiceOfferingVO serviceOfferingVoMock; @Mock - IpAddressManager ipAddressManager; + ConfigKey privateMtuKey; @Mock private CallContext callContextMock; @InjectMocks @@ -169,9 +181,46 @@ public class NetworkServiceImplTest { CommandSetupHelper commandSetupHelper; @Mock private Account accountMock; + + @Mock + private AccountVO accountVOMock; + @Mock + private DomainVO domainVOMock; @InjectMocks NetworkServiceImpl service = new NetworkServiceImpl(); + @Mock + DomainDao domainDaoMock; + + @Mock + AccountDao accountDaoMock; + + @Mock + UpdateQuarantinedIpCmd updateQuarantinedIpCmdMock; + + @Mock + PublicIpQuarantineDao publicIpQuarantineDaoMock; + + @Mock + private PublicIpQuarantineVO publicIpQuarantineVOMock; + + @Mock + private IPAddressVO ipAddressVOMock; + + @Mock + private IpAddressManager ipAddressManagerMock; + + @Mock + private Ip ipMock; + + private static Date beforeDate; + + private static Date afterDate; + + private final Long publicIpId = 1L; + + private final String dummyIpAddress = "192.168.0.1"; + private static final String VLAN_ID_900 = "900"; private static final String VLAN_ID_901 = "901"; private static final String VLAN_ID_902 = "902"; @@ -196,6 +245,20 @@ public class NetworkServiceImplTest { private AutoCloseable closeable; + @BeforeClass + public static void setUpBeforeClass() { + Date date = new Date(); + Calendar calendar = Calendar.getInstance(); + + calendar.setTime(date); + calendar.add(Calendar.DATE, -1); + beforeDate = calendar.getTime(); + + calendar.setTime(date); + calendar.add(Calendar.DATE, 1); + afterDate = calendar.getTime(); + } + private void registerCallContext() { account = new AccountVO("testaccount", 1L, "networkdomain", Account.Type.NORMAL, "uuid"); account.setId(ACCOUNT_ID); @@ -231,7 +294,7 @@ public class NetworkServiceImplTest { service.routerDao = routerDao; service.commandSetupHelper = commandSetupHelper; service.networkHelper = networkHelper; - service._ipAddrMgr = ipAddressManager; + service._ipAddrMgr = ipAddressManagerMock; callContextMocked = Mockito.mockStatic(CallContext.class); CallContext callContextMock = Mockito.mock(CallContext.class); callContextMocked.when(CallContext::current).thenReturn(callContextMock); @@ -744,6 +807,113 @@ public class NetworkServiceImplTest { networkServiceImplMock.validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(1l); } + @Test + public void updatePublicIpAddressInQuarantineTestQuarantineIsAlreadyExpiredShouldThrowCloudRuntimeException() { + Mockito.when(updateQuarantinedIpCmdMock.getId()).thenReturn(publicIpId); + Mockito.when(updateQuarantinedIpCmdMock.getEndDate()).thenReturn(afterDate); + Mockito.when(publicIpQuarantineDaoMock.findById(Mockito.anyLong())).thenReturn(publicIpQuarantineVOMock); + Mockito.when(accountDaoMock.findById(Mockito.anyLong())).thenReturn(accountVOMock); + Mockito.when(domainDaoMock.findById(Mockito.anyLong())).thenReturn(domainVOMock); + Mockito.doNothing().when(accountManager).checkAccess(Mockito.any(Account.class), Mockito.any(Domain.class)); + Mockito.when(ipAddressDao.findById(Mockito.anyLong())).thenReturn(ipAddressVOMock); + Mockito.when(ipAddressVOMock.getAddress()).thenReturn(ipMock); + Mockito.when(ipMock.toString()).thenReturn(dummyIpAddress); + Mockito.when(publicIpQuarantineVOMock.getEndDate()).thenReturn(beforeDate); + String expectedMessage = String.format("The quarantine for the public IP address [%s] is no longer active; thus, it cannot be updated.", dummyIpAddress); + CloudRuntimeException assertThrows = Assert.assertThrows(CloudRuntimeException.class, + () -> service.updatePublicIpAddressInQuarantine(updateQuarantinedIpCmdMock)); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void updatePublicIpAddressInQuarantineTestGivenEndDateIsBeforeCurrentDateShouldThrowInvalidParameterValueException() { + Mockito.when(updateQuarantinedIpCmdMock.getId()).thenReturn(publicIpId); + Mockito.when(updateQuarantinedIpCmdMock.getEndDate()).thenReturn(beforeDate); + + String expectedMessage = String.format("The given end date [%s] is invalid as it is before the current date.", beforeDate); + InvalidParameterValueException assertThrows = Assert.assertThrows(InvalidParameterValueException.class, + () -> service.updatePublicIpAddressInQuarantine(updateQuarantinedIpCmdMock)); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void updatePublicIpAddressInQuarantineTestQuarantineIsStillValidAndGivenEndDateIsAfterCurrentDateShouldWork() { + Calendar calendar = Calendar.getInstance(); + calendar.setTime(afterDate); + calendar.add(Calendar.DATE, 5); + Date expectedNewEndDate = calendar.getTime(); + + Mockito.when(updateQuarantinedIpCmdMock.getId()).thenReturn(publicIpId); + Mockito.when(updateQuarantinedIpCmdMock.getEndDate()).thenReturn(expectedNewEndDate); + Mockito.when(publicIpQuarantineDaoMock.findById(Mockito.anyLong())).thenReturn(publicIpQuarantineVOMock); + Mockito.when(accountDaoMock.findById(Mockito.anyLong())).thenReturn(accountVOMock); + Mockito.when(domainDaoMock.findById(Mockito.anyLong())).thenReturn(domainVOMock); + Mockito.doNothing().when(accountManager).checkAccess(Mockito.any(Account.class), Mockito.any(Domain.class)); + Mockito.when(ipAddressDao.findById(Mockito.anyLong())).thenReturn(ipAddressVOMock); + Mockito.when(ipAddressDao.findById(Mockito.anyLong())).thenReturn(ipAddressVOMock); + Mockito.when(ipAddressVOMock.getAddress()).thenReturn(ipMock); + Mockito.when(ipMock.toString()).thenReturn(dummyIpAddress); + Mockito.when(publicIpQuarantineVOMock.getEndDate()).thenReturn(afterDate); + Mockito.when(ipAddressManagerMock.updatePublicIpAddressInQuarantine(anyLong(), Mockito.any(Date.class))).thenReturn(publicIpQuarantineVOMock); + + PublicIpQuarantine actualPublicIpQuarantine = service.updatePublicIpAddressInQuarantine(updateQuarantinedIpCmdMock); + Mockito.when(actualPublicIpQuarantine.getEndDate()).thenReturn(expectedNewEndDate); + + Assert.assertEquals(expectedNewEndDate , actualPublicIpQuarantine.getEndDate()); + } + + @Test(expected = CloudRuntimeException.class) + public void retrievePublicIpQuarantineTestIpIdNullAndIpAddressNullShouldThrowException() { + service.retrievePublicIpQuarantine(null, null); + } + + @Test + public void retrievePublicIpQuarantineTestValidIpIdShouldReturnPublicQuarantine() { + Mockito.when(publicIpQuarantineDaoMock.findById(Mockito.anyLong())).thenReturn(publicIpQuarantineVOMock); + + service.retrievePublicIpQuarantine(1L, null); + Mockito.verify(publicIpQuarantineDaoMock, Mockito.times(1)).findById(Mockito.anyLong()); + Mockito.verify(publicIpQuarantineDaoMock, Mockito.times(0)).findByIpAddress(Mockito.anyString()); + } + + @Test(expected = CloudRuntimeException.class) + public void retrievePublicIpQuarantineTestInvalidIpIdShouldThrowException() { + Mockito.when(publicIpQuarantineDaoMock.findById(Mockito.anyLong())).thenReturn(null); + + service.retrievePublicIpQuarantine(1L, null); + Mockito.verify(publicIpQuarantineDaoMock, Mockito.times(1)).findById(Mockito.anyLong()); + Mockito.verify(publicIpQuarantineDaoMock, Mockito.times(0)).findByIpAddress(Mockito.anyString()); + } + + @Test + public void retrievePublicIpQuarantineTestValidIpAddressShouldReturnPublicQuarantine() { + Mockito.when(publicIpQuarantineDaoMock.findByIpAddress(Mockito.anyString())).thenReturn(publicIpQuarantineVOMock); + + service.retrievePublicIpQuarantine(null, "10.1.1.1"); + Mockito.verify(publicIpQuarantineDaoMock, Mockito.times(0)).findById(Mockito.anyLong()); + Mockito.verify(publicIpQuarantineDaoMock, Mockito.times(1)).findByIpAddress(Mockito.anyString()); + } + + @Test(expected = CloudRuntimeException.class) + public void retrievePublicIpQuarantineTestInvalidIpAddressShouldThrowException() { + Mockito.when(publicIpQuarantineDaoMock.findByIpAddress(Mockito.anyString())).thenReturn(null); + + service.retrievePublicIpQuarantine(null, "10.1.1.1"); + Mockito.verify(publicIpQuarantineDaoMock, Mockito.times(0)).findById(Mockito.anyLong()); + Mockito.verify(publicIpQuarantineDaoMock, Mockito.times(1)).findByIpAddress(Mockito.anyString()); + } + + @Test + public void retrievePublicIpQuarantineTestIpIdAndAddressInformedShouldUseId() { + Mockito.when(publicIpQuarantineDaoMock.findById(Mockito.anyLong())).thenReturn(publicIpQuarantineVOMock); + + service.retrievePublicIpQuarantine(1L, "10.1.1.1"); + Mockito.verify(publicIpQuarantineDaoMock, Mockito.times(1)).findById(Mockito.anyLong()); + Mockito.verify(publicIpQuarantineDaoMock, Mockito.times(0)).findByIpAddress(Mockito.anyString()); + } + @Test public void validateNotSharedNetworkRouterIPv4() { NetworkOffering ntwkOff = Mockito.mock(NetworkOffering.class); @@ -876,7 +1046,7 @@ public class NetworkServiceImplTest { when(networkVO.getId()).thenReturn(networkId); when(networkVO.getGuestType()).thenReturn(Network.GuestType.Isolated); try { - when(ipAddressManager.allocateIp(any(), anyBoolean(), any(), anyLong(), any(), any(), eq(srcNatIp))).thenReturn(ipAddress); + when(ipAddressManagerMock.allocateIp(any(), anyBoolean(), any(), anyLong(), any(), any(), eq(srcNatIp))).thenReturn(ipAddress); service.checkAndSetRouterSourceNatIp(account, createNetworkCmd, networkVO); } catch (InsufficientAddressCapacityException | ResourceAllocationException e) { Assert.fail(e.getMessage()); diff --git a/server/src/test/java/com/cloud/user/MockUsageEventDao.java b/server/src/test/java/com/cloud/user/MockUsageEventDao.java index 97792871b4d..4639c509249 100644 --- a/server/src/test/java/com/cloud/user/MockUsageEventDao.java +++ b/server/src/test/java/com/cloud/user/MockUsageEventDao.java @@ -261,6 +261,11 @@ public class MockUsageEventDao implements UsageEventDao{ return null; } + @Override + public UsageEventVO findOneBy(SearchCriteria sc, Filter filter) { + return null; + } + @Override public Class getEntityBeanType() { return null; diff --git a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java index da251cb502b..945448f7173 100644 --- a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java @@ -16,6 +16,37 @@ // under the License. package com.cloud.vpc; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import javax.inject.Inject; +import javax.naming.ConfigurationException; + +import com.cloud.network.PublicIpQuarantine; +import org.apache.cloudstack.acl.ControlledEntity.ACLType; +import org.apache.cloudstack.api.command.admin.address.ReleasePodIpCmdByAdmin; +import org.apache.cloudstack.api.command.admin.network.DedicateGuestVlanRangeCmd; +import org.apache.cloudstack.api.command.admin.network.ListDedicatedGuestVlanRangesCmd; +import org.apache.cloudstack.api.command.admin.network.ListGuestVlansCmd; +import org.apache.cloudstack.api.command.admin.usage.ListTrafficTypeImplementorsCmd; +import org.apache.cloudstack.api.command.user.address.RemoveQuarantinedIpCmd; +import org.apache.cloudstack.api.command.user.address.UpdateQuarantinedIpCmd; +import org.apache.cloudstack.api.command.user.network.CreateNetworkCmd; +import org.apache.cloudstack.api.command.user.network.CreateNetworkPermissionsCmd; +import org.apache.cloudstack.api.command.user.network.ListNetworkPermissionsCmd; +import org.apache.cloudstack.api.command.user.network.ListNetworksCmd; +import org.apache.cloudstack.api.command.user.network.RemoveNetworkPermissionsCmd; +import org.apache.cloudstack.api.command.user.network.ResetNetworkPermissionsCmd; +import org.apache.cloudstack.api.command.user.network.RestartNetworkCmd; +import org.apache.cloudstack.api.command.user.network.UpdateNetworkCmd; +import org.apache.cloudstack.api.command.user.vm.ListNicsCmd; +import org.apache.cloudstack.api.response.AcquirePodIpCmdResponse; +import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; +import org.apache.log4j.Logger; +import org.springframework.stereotype.Component; + import com.cloud.deploy.DataCenterDeployment; import com.cloud.deploy.DeployDestination; import com.cloud.deploy.DeploymentPlan; @@ -66,32 +97,6 @@ import com.cloud.vm.ReservationContext; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachine.Type; import com.cloud.vm.VirtualMachineProfile; -import org.apache.cloudstack.acl.ControlledEntity.ACLType; -import org.apache.cloudstack.api.command.admin.address.ReleasePodIpCmdByAdmin; -import org.apache.cloudstack.api.command.admin.network.DedicateGuestVlanRangeCmd; -import org.apache.cloudstack.api.command.admin.network.ListDedicatedGuestVlanRangesCmd; -import org.apache.cloudstack.api.command.admin.network.ListGuestVlansCmd; -import org.apache.cloudstack.api.command.admin.usage.ListTrafficTypeImplementorsCmd; -import org.apache.cloudstack.api.command.user.network.CreateNetworkCmd; -import org.apache.cloudstack.api.command.user.network.CreateNetworkPermissionsCmd; -import org.apache.cloudstack.api.command.user.network.ListNetworkPermissionsCmd; -import org.apache.cloudstack.api.command.user.network.ListNetworksCmd; -import org.apache.cloudstack.api.command.user.network.RemoveNetworkPermissionsCmd; -import org.apache.cloudstack.api.command.user.network.ResetNetworkPermissionsCmd; -import org.apache.cloudstack.api.command.user.network.RestartNetworkCmd; -import org.apache.cloudstack.api.command.user.network.UpdateNetworkCmd; -import org.apache.cloudstack.api.command.user.vm.ListNicsCmd; -import org.apache.cloudstack.api.response.AcquirePodIpCmdResponse; -import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; -import org.apache.log4j.Logger; -import org.springframework.stereotype.Component; - -import javax.inject.Inject; -import javax.naming.ConfigurationException; -import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; @Component public class MockNetworkManagerImpl extends ManagerBase implements NetworkOrchestrationService, NetworkService { @@ -1052,4 +1057,14 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkOrches @Override public void validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(final Long serviceOfferingId) { } + + @Override + public PublicIpQuarantine updatePublicIpAddressInQuarantine(UpdateQuarantinedIpCmd cmd) { + return null; + } + + @Override + public void removePublicIpAddressFromQuarantine(RemoveQuarantinedIpCmd cmd) { + + } } diff --git a/server/src/test/java/org/apache/cloudstack/networkoffering/CreateNetworkOfferingTest.java b/server/src/test/java/org/apache/cloudstack/networkoffering/CreateNetworkOfferingTest.java index adb974a2a06..d0b7ace4711 100644 --- a/server/src/test/java/org/apache/cloudstack/networkoffering/CreateNetworkOfferingTest.java +++ b/server/src/test/java/org/apache/cloudstack/networkoffering/CreateNetworkOfferingTest.java @@ -17,6 +17,31 @@ package org.apache.cloudstack.networkoffering; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.nullable; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import javax.inject.Inject; + +import com.cloud.network.dao.PublicIpQuarantineDao; +import org.apache.cloudstack.annotation.dao.AnnotationDao; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.framework.config.impl.ConfigurationVO; +import org.apache.cloudstack.resourcedetail.dao.UserIpAddressDetailsDao; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mockito; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + import com.cloud.configuration.ConfigurationManager; import com.cloud.event.dao.UsageEventDao; import com.cloud.event.dao.UsageEventDetailsDao; @@ -38,28 +63,6 @@ import com.cloud.user.UserVO; import com.cloud.utils.component.ComponentContext; import com.cloud.vm.dao.UserVmDetailsDao; import junit.framework.TestCase; -import org.apache.cloudstack.annotation.dao.AnnotationDao; -import org.apache.cloudstack.context.CallContext; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; -import org.apache.cloudstack.framework.config.impl.ConfigurationVO; -import org.apache.cloudstack.resourcedetail.dao.UserIpAddressDetailsDao; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mockito; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; - -import javax.inject.Inject; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.nullable; @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(locations = "classpath:/createNetworkOffering.xml") @@ -101,6 +104,9 @@ public class CreateNetworkOfferingTest extends TestCase { @Inject AnnotationDao annotationDao; + @Inject + PublicIpQuarantineDao publicIpQuarantineDao; + @Override @Before public void setUp() { diff --git a/server/src/test/resources/createNetworkOffering.xml b/server/src/test/resources/createNetworkOffering.xml index 0f558d11a7a..28e602720e8 100644 --- a/server/src/test/resources/createNetworkOffering.xml +++ b/server/src/test/resources/createNetworkOffering.xml @@ -72,4 +72,5 @@ + diff --git a/test/integration/smoke/test_quarantined_ips.py b/test/integration/smoke/test_quarantined_ips.py new file mode 100644 index 00000000000..42349fd2a53 --- /dev/null +++ b/test/integration/smoke/test_quarantined_ips.py @@ -0,0 +1,329 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import time + +from nose.plugins.attrib import attr + +from marvin.cloudstackAPI import updateConfiguration +from marvin.cloudstackException import CloudstackAPIException +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.base import Network, NetworkOffering, VpcOffering, VPC, PublicIPAddress +from marvin.lib.common import get_domain, get_zone + + +class Services: + """ Test Quarantine for public IPs + """ + + def __init__(self): + self.services = { + "root_domain": { + "name": "ROOT", + }, + "domain_admin": { + "username": "Domain admin", + "roletype": 2, + }, + "root_admin": { + "username": "Root admin", + "roletype": 1, + }, + "domain_vpc": { + "name": "domain-vpc", + "displaytext": "domain-vpc", + "cidr": "10.1.1.0/24", + }, + "domain_network": { + "name": "domain-network", + "displaytext": "domain-network", + }, + "root_vpc": { + "name": "root-vpc", + "displaytext": "root-vpc", + "cidr": "10.2.1.0/24", + }, + "root_network": { + "name": "root-network", + "displaytext": "root-network", + } + } + + +class TestQuarantineIPs(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + cls.testClient = super(TestQuarantineIPs, cls).getClsTestClient() + cls.apiclient = cls.testClient.getApiClient() + + cls.services = Services().services + cls.domain = get_domain(cls.apiclient) + cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests()) + return + + def setUp(self): + self.domain_admin_apiclient = self.testClient.getUserApiClient(self.services["domain_admin"]["username"], + self.services["root_domain"]["name"], + self.services["domain_admin"]["roletype"]) + + self.admin_apiclient = self.testClient.getUserApiClient(self.services["root_admin"]["username"], + self.services["root_domain"]["name"], + self.services["root_admin"]["roletype"]) + + """ + Set public.ip.address.quarantine.duration to 60 minutes + """ + update_configuration_cmd = updateConfiguration.updateConfigurationCmd() + update_configuration_cmd.name = "public.ip.address.quarantine.duration" + update_configuration_cmd.value = "1" + self.apiclient.updateConfiguration(update_configuration_cmd) + + self.cleanup = [] + return + + def tearDown(self): + """ + Reset public.ip.address.quarantine.duration to 0 minutes + """ + update_configuration_cmd = updateConfiguration.updateConfigurationCmd() + update_configuration_cmd.name = "public.ip.address.quarantine.duration" + update_configuration_cmd.value = "0" + self.apiclient.updateConfiguration(update_configuration_cmd) + + super(TestQuarantineIPs, self).tearDown() + + def create_vpc(self, api_client, services): + # Get network offering + network_offering = NetworkOffering.list(api_client, name="DefaultIsolatedNetworkOfferingForVpcNetworks") + self.assertTrue(network_offering is not None and len(network_offering) > 0, "No VPC network offering") + + # Getting VPC offering + vpc_offering = VpcOffering.list(api_client, name="Default VPC offering") + self.assertTrue(vpc_offering is not None and len(vpc_offering) > 0, "No VPC offerings found") + + # Creating VPC + vpc = VPC.create( + apiclient=api_client, + services=services, + networkDomain="vpc.networkacl", + vpcofferingid=vpc_offering[0].id, + zoneid=self.zone.id, + domainid=self.domain.id, + start=False + ) + + self.cleanup.append(vpc) + self.assertTrue(vpc is not None, "VPC creation failed") + return vpc + + @attr(tags=["advanced", "basic"], required_hardware="false") + def test_only_owner_can_allocate_ip_in_quarantine_vpc(self): + """ Test allocate IP in quarantine to VPC. + """ + # Creating Domain Admin VPC + domain_vpc = self.create_vpc(self.domain_admin_apiclient, self.services["domain_vpc"]) + + # Allocating source nat first + PublicIPAddress.create(self.domain_admin_apiclient, + zoneid=self.zone.id, + vpcid=domain_vpc.id) + + # Getting available public IP address + ip_address = PublicIPAddress.list(self.domain_admin_apiclient, state="Free", listall=True)[0].ipaddress + + self.debug( + f"creating public address with zone {self.zone.id} and vpc id {domain_vpc.id} and ip address {ip_address}.") + # Associating public IP address to Domain Admin account + public_ip = PublicIPAddress.create(self.domain_admin_apiclient, + zoneid=self.zone.id, + vpcid=domain_vpc.id, + ipaddress=ip_address) + self.assertIsNotNone(public_ip, "Failed to Associate IP Address") + self.assertEqual(public_ip.ipaddress.ipaddress, ip_address, "Associated IP is not same as specified") + + self.debug(f"Disassociating public IP {public_ip.ipaddress.ipaddress}.") + public_ip.delete(self.domain_admin_apiclient) + + # Creating Root Admin VPC + root_vpc = self.create_vpc(self.admin_apiclient, self.services["root_vpc"]) + + self.debug(f"Trying to allocate the same IP address {ip_address} that is still in quarantine.") + + with self.assertRaises(CloudstackAPIException) as exception: + PublicIPAddress.create(self.admin_apiclient, + zoneid=self.zone.id, + vpcid=root_vpc.id, + ipaddress=ip_address) + self.assertIn(f"Failed to allocate public IP address [{ip_address}] as it is in quarantine.", + exception.exception.errorMsg) + + # Owner should be able to allocate its IP in quarantine + public_ip = PublicIPAddress.create(self.domain_admin_apiclient, + zoneid=self.zone.id, + vpcid=domain_vpc.id, + ipaddress=ip_address) + self.assertIsNotNone(public_ip, "Failed to Associate IP Address") + self.assertEqual(public_ip.ipaddress.ipaddress, ip_address, "Associated IP is not same as specified") + + @attr(tags=["advanced", "basic"], required_hardware="false") + def test_another_user_can_allocate_ip_after_quarantined_has_ended_vpc(self): + """ Test allocate IP to VPC after quarantine has ended. + """ + # Creating Domain Admin VPC + domain_vpc = self.create_vpc(self.domain_admin_apiclient, self.services["domain_vpc"]) + + # Allocating source nat first + PublicIPAddress.create(self.domain_admin_apiclient, + zoneid=self.zone.id, + vpcid=domain_vpc.id) + + # Getting available public IP address + ip_address = PublicIPAddress.list(self.domain_admin_apiclient, state="Free", listall=True)[0].ipaddress + + self.debug( + f"creating public address with zone {self.zone.id} and vpc id {domain_vpc.id} and ip address {ip_address}.") + # Associating public IP address to Domain Admin account + public_ip = PublicIPAddress.create(self.domain_admin_apiclient, + zoneid=self.zone.id, + vpcid=domain_vpc.id, + ipaddress=ip_address) + self.assertIsNotNone(public_ip, "Failed to Associate IP Address") + self.assertEqual(public_ip.ipaddress.ipaddress, ip_address, "Associated IP is not same as specified") + + self.debug(f"Disassociating public IP {public_ip.ipaddress.ipaddress}.") + public_ip.delete(self.domain_admin_apiclient) + + # Creating Root Admin VPC + root_vpc = self.create_vpc(self.admin_apiclient, self.services["root_vpc"]) + + self.debug(f"Trying to allocate the same IP address {ip_address} after the quarantine duration.") + + time.sleep(60) + + public_ip_2 = PublicIPAddress.create(self.admin_apiclient, + zoneid=self.zone.id, + vpcid=root_vpc.id, + ipaddress=ip_address) + self.assertIsNotNone(public_ip_2, "Failed to Associate IP Address") + self.assertEqual(public_ip_2.ipaddress.ipaddress, ip_address, "Associated IP is not same as specified") + + @attr(tags=["advanced", "basic"], required_hardware="false") + def test_only_owner_can_allocate_ip_in_quarantine_network(self): + """ Test allocate IP in quarantine to network. + """ + network_offering = NetworkOffering.list(self.domain_admin_apiclient, + name="DefaultIsolatedNetworkOfferingWithSourceNatService") + domain_network = Network.create(self.domain_admin_apiclient, + zoneid=self.zone.id, + services=self.services["domain_network"], + networkofferingid=network_offering[0].id) + self.cleanup.append(domain_network) + + # Allocating source nat first + PublicIPAddress.create(self.domain_admin_apiclient, + zoneid=self.zone.id, + networkid=domain_network.id) + + # Getting available public IP address + ip_address = PublicIPAddress.list(self.domain_admin_apiclient, state="Free", listall=True)[0].ipaddress + + self.debug( + f"creating public address with zone {self.zone.id} and network id {domain_network.id} and ip address {ip_address}.") + # Associating public IP address to Domain Admin account + public_ip = PublicIPAddress.create(self.domain_admin_apiclient, + zoneid=self.zone.id, + networkid=domain_network.id, + ipaddress=ip_address) + self.assertIsNotNone(public_ip, "Failed to Associate IP Address") + self.assertEqual(public_ip.ipaddress.ipaddress, ip_address, "Associated IP is not same as specified") + + self.debug(f"Disassociating public IP {public_ip.ipaddress.ipaddress}.") + public_ip.delete(self.domain_admin_apiclient) + + # Creating Root Admin network + root_network = Network.create(self.admin_apiclient, + zoneid=self.zone.id, + services=self.services["root_network"], + networkofferingid=network_offering[0].id) + self.cleanup.append(root_network) + self.debug(f"Trying to allocate the same IP address {ip_address} that is still in quarantine.") + + with self.assertRaises(CloudstackAPIException) as exception: + PublicIPAddress.create(self.admin_apiclient, + zoneid=self.zone.id, + networkid=root_network.id, + ipaddress=ip_address) + self.assertIn(f"Failed to allocate public IP address [{ip_address}] as it is in quarantine.", + exception.exception.errorMsg) + + # Owner should be able to allocate its IP in quarantine + public_ip = PublicIPAddress.create(self.domain_admin_apiclient, + zoneid=self.zone.id, + networkid=domain_network.id, + ipaddress=ip_address) + self.assertIsNotNone(public_ip, "Failed to Associate IP Address") + self.assertEqual(public_ip.ipaddress.ipaddress, ip_address, "Associated IP is not same as specified") + + @attr(tags=["advanced", "basic"], required_hardware="false") + def test_another_user_can_allocate_ip_after_quarantined_has_ended_network(self): + """ Test allocate IP to network after quarantine has ended. + """ + network_offering = NetworkOffering.list(self.domain_admin_apiclient, + name="DefaultIsolatedNetworkOfferingWithSourceNatService") + domain_network = Network.create(self.domain_admin_apiclient, + zoneid=self.zone.id, + services=self.services["domain_network"], + networkofferingid=network_offering[0].id) + self.cleanup.append(domain_network) + # Allocating source nat first + PublicIPAddress.create(self.domain_admin_apiclient, + zoneid=self.zone.id, + networkid=domain_network.id) + + # Getting available public IP address + ip_address = PublicIPAddress.list(self.domain_admin_apiclient, state="Free", listall=True)[0].ipaddress + + self.debug( + f"creating public address with zone {self.zone.id} and network id {domain_network.id} and ip address {ip_address}.") + # Associating public IP address to Domain Admin account + public_ip = PublicIPAddress.create(self.domain_admin_apiclient, + zoneid=self.zone.id, + networkid=domain_network.id, + ipaddress=ip_address) + self.assertIsNotNone(public_ip, "Failed to Associate IP Address") + self.assertEqual(public_ip.ipaddress.ipaddress, ip_address, "Associated IP is not same as specified") + + self.debug(f"Disassociating public IP {public_ip.ipaddress.ipaddress}.") + public_ip.delete(self.domain_admin_apiclient) + + # Creating Root Admin VPC + root_network = Network.create(self.admin_apiclient, + zoneid=self.zone.id, + services=self.services["root_network"], + networkofferingid=network_offering[0].id) + self.cleanup.append(root_network) + + self.debug(f"Trying to allocate the same IP address {ip_address} after the quarantine duration.") + + time.sleep(60) + + public_ip_2 = PublicIPAddress.create(self.admin_apiclient, + zoneid=self.zone.id, + networkid=domain_network.id, + ipaddress=ip_address) + self.assertIsNotNone(public_ip_2, "Failed to Associate IP Address") + self.assertEqual(public_ip_2.ipaddress.ipaddress, ip_address, "Associated IP is not same as specified") diff --git a/tools/apidoc/gen_toc.py b/tools/apidoc/gen_toc.py index d46e544666c..c07d27ae06c 100644 --- a/tools/apidoc/gen_toc.py +++ b/tools/apidoc/gen_toc.py @@ -256,6 +256,9 @@ known_categories = { 'importVsphereStoragePolicies' : 'vSphere storage policies', 'listVsphereStoragePolicies' : 'vSphere storage policies', 'ConsoleEndpoint': 'Console Endpoint', + 'listQuarantinedIp': 'IP Quarantine', + 'updateQuarantinedIp': 'IP Quarantine', + 'removeQuarantinedIp': 'IP Quarantine', 'Shutdown': 'Shutdown' } diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index b0fd3198301..32b0e980407 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -1854,7 +1854,7 @@ class PublicIPAddress: if zoneid: cmd.zoneid = zoneid - elif "zoneid" in services: + elif services and "zoneid" in services: cmd.zoneid = services["zoneid"] if domainid: @@ -5105,7 +5105,7 @@ class VPC: @classmethod def create(cls, apiclient, services, vpcofferingid, zoneid, networkDomain=None, account=None, - domainid=None, **kwargs): + domainid=None, start=True, **kwargs): """Creates the virtual private connection (VPC)""" cmd = createVPC.createVPCCmd() @@ -5113,6 +5113,7 @@ class VPC: cmd.displaytext = "-".join([services["displaytext"], random_gen()]) cmd.vpcofferingid = vpcofferingid cmd.zoneid = zoneid + cmd.start = start if "cidr" in services: cmd.cidr = services["cidr"] if account: diff --git a/ui/src/views/network/IpAddressesTab.vue b/ui/src/views/network/IpAddressesTab.vue index 8b22ec66a33..37b5b210a96 100644 --- a/ui/src/views/network/IpAddressesTab.vue +++ b/ui/src/views/network/IpAddressesTab.vue @@ -453,8 +453,8 @@ export default { this.onCloseModal() }).catch(error => { this.$notification.error({ - message: `${this.$t('label.error')} ${error.response.status}`, - description: error.response.data.associateipaddressresponse.errortext || error.response.data.errorresponse.errortext, + message: this.$t('message.request.failed'), + description: (error.response && error.response.headers && error.response.headers['x-description']) || error.message, duration: 0 }) }).finally(() => { From be4a648f5ab708233267ced9eaa943c12f87697c Mon Sep 17 00:00:00 2001 From: GaOrtiga <49285692+GaOrtiga@users.noreply.github.com> Date: Wed, 15 Nov 2023 07:18:26 -0300 Subject: [PATCH 10/39] Create global configuration to allow changing the default nic adapter for user VMs in VMware (#7954) Co-authored-by: Gabriel --- .../com/cloud/hypervisor/guru/VmwareVmImplementer.java | 8 ++++---- .../cloud/hypervisor/vmware/manager/VmwareManager.java | 8 ++++++++ .../hypervisor/vmware/manager/VmwareManagerImpl.java | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java index 990a1875a57..d60dc99a429 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java @@ -129,15 +129,15 @@ class VmwareVmImplementer { } } } else { - // for user-VM, use E1000 as default if (nicDeviceType == null) { - details.put(VmDetailConstants.NIC_ADAPTER, VirtualEthernetCardType.E1000.toString()); + details.put(VmDetailConstants.NIC_ADAPTER, vmwareMgr.VmwareUserVmNicDeviceType.value()); } else { try { VirtualEthernetCardType.valueOf(nicDeviceType); } catch (Exception e) { - LOGGER.warn("Invalid NIC device type " + nicDeviceType + " is specified in VM details, switch to default E1000"); - details.put(VmDetailConstants.NIC_ADAPTER, VirtualEthernetCardType.E1000.toString()); + LOGGER.warn(String.format("Invalid NIC device type [%s] specified in VM details, switching to value [%s] of configuration [%s].", + nicDeviceType, vmwareMgr.VmwareUserVmNicDeviceType.value(), vmwareMgr.VmwareUserVmNicDeviceType.toString())); + details.put(VmDetailConstants.NIC_ADAPTER, vmwareMgr.VmwareUserVmNicDeviceType.value()); } } } 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 c2cdbccdae7..d64a2d38ece 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 @@ -53,6 +53,14 @@ public interface VmwareManager { "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); + static final ConfigKey VmwareUserVmNicDeviceType = new ConfigKey( + String.class, + "vmware.uservm.nic.device.type", + "Advanced", + "E1000", + "Specify the default network device type for user VMs, valid values are E1000, PCNet32, Vmxnet2, Vmxnet3", + true, ConfigKey.Scope.Global, null, null, null, null, null, ConfigKey.Kind.Select, "E1000,PCNet32,Vmxnet2,Vmxnet3"); + String composeWorkerName(); String getSystemVMIsoFileNameOnDatastore(); 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 199f96a8539..2074da7f082 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 @@ -296,7 +296,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, s_vmwareCleanupPortGroups, VMWARE_STATS_TIME_WINDOW}; + return new ConfigKey[] {s_vmwareNicHotplugWaitTimeout, s_vmwareCleanOldWorderVMs, templateCleanupInterval, s_vmwareSearchExcludeFolder, s_vmwareOVAPackageTimeout, s_vmwareCleanupPortGroups, VMWARE_STATS_TIME_WINDOW, VmwareUserVmNicDeviceType}; } @Override public boolean configure(String name, Map params) throws ConfigurationException { From 267a457efc55755583dd441c340a1ec1914615f2 Mon Sep 17 00:00:00 2001 From: Stephan Krug Date: Thu, 16 Nov 2023 05:17:17 -0300 Subject: [PATCH 11/39] Externalize KVM HA heartbeat frequency (#6892) Co-authored-by: Stephan Krug Co-authored-by: GaOrtiga <49285692+GaOrtiga@users.noreply.github.com> Co-authored-by: dahn --- agent/conf/agent.properties | 12 ++++++ .../agent/properties/AgentProperties.java | 38 +++++++++++++++++-- .../hypervisor/kvm/resource/KVMHABase.java | 10 +++-- .../hypervisor/kvm/resource/KVMHAMonitor.java | 1 - .../kvm/storage/KVMStoragePool.java | 12 +++--- 5 files changed, 60 insertions(+), 13 deletions(-) diff --git a/agent/conf/agent.properties b/agent/conf/agent.properties index b88da3621cd..3f07ba16237 100644 --- a/agent/conf/agent.properties +++ b/agent/conf/agent.properties @@ -407,3 +407,15 @@ iscsi.session.cleanup.enabled=false # The path of an executable file/script for host health check for CloudStack to Auto Disable/Enable the host # depending on the return value of the file/script # agent.health.check.script.path= + +# Time interval (in milliseconds) between KVM heartbeats. +# kvm.heartbeat.update.frequency=60000 + +# Number of maximum tries to KVM heartbeats. +# kvm.heartbeat.update.max.tries=5 + +# Time amount (in milliseconds) for the KVM heartbeat retry sleep. +# kvm.heartbeat.update.retry.sleep=10000 + +# Timeout (in milliseconds) of the KVM heartbeat checker. +# kvm.heartbeat.checker.timeout=360000 diff --git a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java index 610c5be759f..8f51d470f07 100644 --- a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java +++ b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java @@ -539,10 +539,10 @@ public class AgentProperties{ /** * Heartbeat update timeout (in ms).
* Depending on the use case, this timeout might need increasing/decreasing.
- * Data type: Integer.
- * Default value: 60000 + * Data type: Long.
+ * Default value: 60000L */ - public static final Property HEARTBEAT_UPDATE_TIMEOUT = new Property<>("heartbeat.update.timeout", 60000); + public static final Property HEARTBEAT_UPDATE_TIMEOUT = new Property<>("heartbeat.update.timeout", 60000L); /** * The timeout (in seconds) to retrieve the target's domain ID when migrating a VM with KVM.
@@ -740,6 +740,38 @@ public class AgentProperties{ */ public static final Property CONTROL_CIDR = new Property<>("control.cidr", "169.254.0.0/16"); + /** + * Time interval (in milliseconds) between KVM heartbeats.
+ * This property is for KVM only. + * Data type: Long.
+ * Default value: 60000l + */ + public static final Property KVM_HEARTBEAT_UPDATE_FREQUENCY = new Property<>("kvm.heartbeat.update.frequency", 60000L); + + /** + * Number of maximum tries to KVM heartbeats.
+ * This property is for KVM only. + * Data type: Long.
+ * Default value: 5l + */ + public static final Property KVM_HEARTBEAT_UPDATE_MAX_TRIES = new Property<>("kvm.heartbeat.update.max.tries", 5L); + + /** + * Time amount (in milliseconds) for the KVM heartbeat retry sleep.
+ * This property is for KVM only. + * Data type: Long.
+ * Default value: 10000l + */ + public static final Property KVM_HEARTBEAT_UPDATE_RETRY_SLEEP = new Property<>("kvm.heartbeat.update.retry.sleep", 10000L); + + /** + * Timeout (in milliseconds) of the KVM heartbeat checker.
+ * This property is for KVM only. + * Data type: Long.
+ * Default value: 360000l + */ + public static final Property KVM_HEARTBEAT_CHECKER_TIMEOUT = new Property<>("kvm.heartbeat.checker.timeout", 360000L); + public static class Property { private String name; private T defaultValue; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHABase.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHABase.java index 093070fddd6..b9abea4f0bc 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHABase.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHABase.java @@ -28,15 +28,17 @@ import com.cloud.hypervisor.kvm.storage.KVMStoragePool; import com.cloud.utils.script.OutputInterpreter; import com.cloud.utils.script.OutputInterpreter.AllLinesParser; import com.cloud.utils.script.Script; +import com.cloud.agent.properties.AgentProperties; +import com.cloud.agent.properties.AgentPropertiesFileHandler; public class KVMHABase { private static final Logger s_logger = Logger.getLogger(KVMHABase.class); private long _timeout = 60000; /* 1 minutes */ protected static String s_heartBeatPath; - protected long _heartBeatUpdateTimeout = 60000; - protected long _heartBeatUpdateFreq = 60000; - protected long _heartBeatUpdateMaxTries = 5; - protected long _heartBeatUpdateRetrySleep = 10000; + protected long _heartBeatUpdateTimeout = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HEARTBEAT_UPDATE_TIMEOUT); + protected long _heartBeatUpdateFreq = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_HEARTBEAT_UPDATE_FREQUENCY); + protected long _heartBeatUpdateMaxTries = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_HEARTBEAT_UPDATE_MAX_TRIES); + protected long _heartBeatUpdateRetrySleep = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_HEARTBEAT_UPDATE_RETRY_SLEEP); public static enum PoolType { PrimaryStorage, SecondaryStorage diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java index 72944b54e92..eb09408c14e 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java @@ -48,7 +48,6 @@ public class KVMHAMonitor extends KVMHABase implements Runnable { hostPrivateIp = host; configureHeartBeatPath(scriptPath); - _heartBeatUpdateTimeout = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HEARTBEAT_UPDATE_TIMEOUT); rebootHostAndAlertManagementOnHeartbeatTimeout = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.REBOOT_HOST_AND_ALERT_MANAGEMENT_ON_HEARTBEAT_TIMEOUT); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java index 98e779253cc..43a09ccf2bf 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java @@ -19,6 +19,8 @@ package com.cloud.hypervisor.kvm.storage; import java.util.List; import java.util.Map; +import com.cloud.agent.properties.AgentProperties; +import com.cloud.agent.properties.AgentPropertiesFileHandler; import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; import org.joda.time.Duration; @@ -29,11 +31,11 @@ import com.cloud.storage.Storage.StoragePoolType; public interface KVMStoragePool { - public static final long HeartBeatUpdateTimeout = 60000; - public static final long HeartBeatUpdateFreq = 60000; - public static final long HeartBeatUpdateMaxTries = 5; - public static final long HeartBeatUpdateRetrySleep = 10000; - public static final long HeartBeatCheckerTimeout = 360000; // 6 minutes + public static final long HeartBeatUpdateTimeout = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HEARTBEAT_UPDATE_TIMEOUT); + public static final long HeartBeatUpdateFreq = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_HEARTBEAT_UPDATE_FREQUENCY); + public static final long HeartBeatUpdateMaxTries = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_HEARTBEAT_UPDATE_MAX_TRIES); + public static final long HeartBeatUpdateRetrySleep = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_HEARTBEAT_UPDATE_RETRY_SLEEP); + public static final long HeartBeatCheckerTimeout = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_HEARTBEAT_CHECKER_TIMEOUT); public KVMPhysicalDisk createPhysicalDisk(String volumeUuid, PhysicalDiskFormat format, Storage.ProvisioningType provisioningType, long size, byte[] passphrase); From cc45bffdbd35f0d8021b917fd4233972cc481dc6 Mon Sep 17 00:00:00 2001 From: maheshnikam <55378196+nikam14@users.noreply.github.com> Date: Thu, 16 Nov 2023 16:07:53 +0530 Subject: [PATCH 12/39] Improved concatenation way in PropertiesStorage.java (#7486) Co-authored-by: dahn Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com> --- .../java/com/cloud/agent/dao/impl/PropertiesStorage.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java b/agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java index a1db88c86c4..87610c29f34 100644 --- a/agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java +++ b/agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java @@ -92,11 +92,14 @@ public class PropertiesStorage implements StorageComponent { file = new File(path); try { if (!file.createNewFile()) { - s_logger.error("Unable to create _file: " + file.getAbsolutePath()); + s_logger.error(String.format("Unable to create _file: %s", file.getAbsolutePath())); return false; } } catch (IOException e) { - s_logger.error("Unable to create _file: " + file.getAbsolutePath(), e); + s_logger.error(String.format("Unable to create file: %s", file.getAbsolutePath())); + if (s_logger.isDebugEnabled()) { + s_logger.debug(String.format("IOException while trying to create file: %s", file.getAbsolutePath()), e); + } return false; } } From 0735b91037dec0e006e6dbbd79e09d9db3ccae56 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Thu, 16 Nov 2023 17:39:38 +0530 Subject: [PATCH 13/39] api: introduce domainid and account parameter in createTemplate API (#8210) Introduces domainid and account as optional parameter for createTemplate API. It will allow admin to create templates for specified domain belonging to specific account. --- .../user/template/CreateTemplateCmd.java | 129 +++++++++++------- .../user/template/CreateTemplateCmdTest.java | 16 +++ 2 files changed, 94 insertions(+), 51 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/template/CreateTemplateCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/template/CreateTemplateCmd.java index 73a6155c8c5..6c39ab6d3c7 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/template/CreateTemplateCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/template/CreateTemplateCmd.java @@ -21,18 +21,6 @@ import java.util.List; import java.util.Map; import org.apache.cloudstack.acl.SecurityChecker; -import org.apache.cloudstack.api.command.user.UserCmd; -import org.apache.cloudstack.api.response.GuestOSResponse; -import org.apache.cloudstack.api.response.SnapshotResponse; -import org.apache.cloudstack.api.response.TemplateResponse; -import org.apache.cloudstack.api.response.UserVmResponse; -import org.apache.cloudstack.api.response.VolumeResponse; -import org.apache.cloudstack.api.response.ProjectResponse; - -import org.apache.cloudstack.api.response.ZoneResponse; -import org.apache.commons.lang3.StringUtils; -import org.apache.log4j.Logger; - import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiCommandResourceType; import org.apache.cloudstack.api.ApiConstants; @@ -41,13 +29,23 @@ import org.apache.cloudstack.api.BaseAsyncCreateCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ResponseObject.ResponseView; import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.user.UserCmd; +import org.apache.cloudstack.api.response.DomainResponse; +import org.apache.cloudstack.api.response.GuestOSResponse; +import org.apache.cloudstack.api.response.ProjectResponse; +import org.apache.cloudstack.api.response.SnapshotResponse; +import org.apache.cloudstack.api.response.TemplateResponse; +import org.apache.cloudstack.api.response.UserVmResponse; +import org.apache.cloudstack.api.response.VolumeResponse; +import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.cloudstack.context.CallContext; +import org.apache.commons.lang3.StringUtils; +import org.apache.log4j.Logger; import com.cloud.event.EventTypes; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceAllocationException; -import com.cloud.projects.Project; import com.cloud.storage.Snapshot; import com.cloud.storage.Volume; import com.cloud.template.VirtualMachineTemplate; @@ -139,6 +137,19 @@ public class CreateTemplateCmd extends BaseAsyncCreateCmd implements UserCmd { @Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, description = "the zone for the template. Can be specified with snapshot only", since = "4.19.0") private Long zoneId; + @Parameter(name = ApiConstants.DOMAIN_ID, + type = CommandType.UUID, + entityType = DomainResponse.class, + description = "an optional domainId. If the account parameter is used, domainId must also be used.", + since = "4.19.0") + private Long domainId; + + @Parameter(name = ApiConstants.ACCOUNT, + type = CommandType.STRING, + description = "an optional accountName. Must be used with domainId.", + since = "4.19.0") + private String accountName; + // /////////////////////////////////////////////////// // ///////////////// Accessors /////////////////////// // /////////////////////////////////////////////////// @@ -217,6 +228,14 @@ public class CreateTemplateCmd extends BaseAsyncCreateCmd implements UserCmd { return zoneId; } + public Long getDomainId() { + return domainId; + } + + public String getAccountName() { + return accountName; + } + // /////////////////////////////////////////////////// // ///////////// API Implementation/////////////////// // /////////////////////////////////////////////////// @@ -232,47 +251,12 @@ public class CreateTemplateCmd extends BaseAsyncCreateCmd implements UserCmd { @Override public long getEntityOwnerId() { - Long volumeId = getVolumeId(); - Long snapshotId = getSnapshotId(); Account callingAccount = CallContext.current().getCallingAccount(); - if (volumeId != null) { - Volume volume = _entityMgr.findById(Volume.class, volumeId); - if (volume != null) { - _accountService.checkAccess(callingAccount, SecurityChecker.AccessType.UseEntry, false, volume); - } else { - throw new InvalidParameterValueException("Unable to find volume by id=" + volumeId); - } - } else { - Snapshot snapshot = _entityMgr.findById(Snapshot.class, snapshotId); - if (snapshot != null) { - _accountService.checkAccess(callingAccount, SecurityChecker.AccessType.UseEntry, false, snapshot); - } else { - throw new InvalidParameterValueException("Unable to find snapshot by id=" + snapshotId); - } - } - - if(projectId != null){ - final Project project = _projectService.getProject(projectId); - if (project != null) { - if (project.getState() == Project.State.Active) { - Account projectAccount= _accountService.getAccount(project.getProjectAccountId()); - _accountService.checkAccess(callingAccount, SecurityChecker.AccessType.UseEntry, false, projectAccount); - return project.getProjectAccountId(); - } else { - final PermissionDeniedException ex = - new PermissionDeniedException("Can't add resources to the project with specified projectId in state=" + project.getState() + - " as it's no longer active"); - ex.addProxyObject(project.getUuid(), "projectId"); - throw ex; - } - } else { - throw new InvalidParameterValueException("Unable to find project by id"); - } - } - - return callingAccount.getId(); + ensureAccessCheck(callingAccount); + return findAccountIdToUse(callingAccount); } + @Override public String getEventType() { return EventTypes.EVENT_TEMPLATE_CREATE; @@ -330,4 +314,47 @@ public class CreateTemplateCmd extends BaseAsyncCreateCmd implements UserCmd { } } + + /*** + * Performs access check on volume and snapshot for given account + * @param account + */ + private void ensureAccessCheck(Account account) { + if (volumeId != null) { + Volume volume = _entityMgr.findById(Volume.class, volumeId); + if (volume != null) { + _accountService.checkAccess(account, SecurityChecker.AccessType.UseEntry, false, volume); + } else { + throw new InvalidParameterValueException("Unable to find volume by id=" + volumeId); + } + } else { + Snapshot snapshot = _entityMgr.findById(Snapshot.class, snapshotId); + if (snapshot != null) { + _accountService.checkAccess(account, SecurityChecker.AccessType.UseEntry, false, snapshot); + } else { + throw new InvalidParameterValueException("Unable to find snapshot by id=" + snapshotId); + } + } + } + + /*** + * Find accountId based on accountName and domainId or projectId + * if not found, return callingAccountId for further use + * @param callingAccount + * @return accountId + */ + private Long findAccountIdToUse(Account callingAccount) { + Long accountIdToUse = null; + try { + accountIdToUse = _accountService.finalyzeAccountId(accountName, domainId, projectId, true); + } catch (InvalidParameterValueException | PermissionDeniedException ex) { + if (s_logger.isDebugEnabled()) { + s_logger.debug(String.format("An exception occurred while finalizing account id with accountName, domainId and projectId" + + "using callingAccountId=%s", callingAccount.getUuid()), ex); + } + s_logger.warn("Unable to find accountId associated with accountName=" + accountName + " and domainId=" + + domainId + " or projectId=" + projectId + ", using callingAccountId=" + callingAccount.getUuid()); + } + return accountIdToUse != null ? accountIdToUse : callingAccount.getAccountId(); + } } diff --git a/api/src/test/java/org/apache/cloudstack/api/command/user/template/CreateTemplateCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/user/template/CreateTemplateCmdTest.java index d8af670a7b2..55e9c8dba1e 100644 --- a/api/src/test/java/org/apache/cloudstack/api/command/user/template/CreateTemplateCmdTest.java +++ b/api/src/test/java/org/apache/cloudstack/api/command/user/template/CreateTemplateCmdTest.java @@ -29,4 +29,20 @@ public class CreateTemplateCmdTest { ReflectionTestUtils.setField(cmd, "zoneId", id); Assert.assertEquals(id, cmd.getZoneId()); } + + @Test + public void testDomainId() { + final CreateTemplateCmd cmd = new CreateTemplateCmd(); + Long id = 2L; + ReflectionTestUtils.setField(cmd, "domainId", id); + Assert.assertEquals(id, cmd.getDomainId()); + } + + @Test + public void testGetAccountName() { + final CreateTemplateCmd cmd = new CreateTemplateCmd(); + String accountName = "user1"; + ReflectionTestUtils.setField(cmd, "accountName", accountName); + Assert.assertEquals(accountName, cmd.getAccountName()); + } } From 60017723357c5c5d3f81931d2b1be286aa986ad8 Mon Sep 17 00:00:00 2001 From: DK101010 <57522802+DK101010@users.noreply.github.com> Date: Thu, 16 Nov 2023 16:43:42 +0100 Subject: [PATCH 14/39] multi local storage handling for kvm (#6699) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: DK101010 Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com> --- .../com/cloud/agent/api/StoragePoolInfo.java | 7 ++ .../api/command/admin/host/ListHostsCmd.java | 4 + .../admin/storage/ListStoragePoolsCmd.java | 29 ++++- ...oudStackPrimaryDataStoreLifeCycleImpl.java | 83 ++++--------- .../com/cloud/api/query/QueryManagerImpl.java | 27 ++++- .../com/cloud/storage/StorageManagerImpl.java | 110 +++++++++++++++++- ui/src/views/infra/AddPrimaryStorage.vue | 32 ++++- 7 files changed, 220 insertions(+), 72 deletions(-) diff --git a/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java b/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java index d923694a854..eb7e8813ecd 100644 --- a/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java +++ b/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java @@ -52,6 +52,13 @@ public class StoragePoolInfo { this.details = details; } + public StoragePoolInfo(String uuid, String host, String hostPath, String localPath, StoragePoolType poolType, long capacityBytes, long availableBytes, + Map details, String name) { + this(uuid, host, hostPath, localPath, poolType, capacityBytes, availableBytes); + this.details = details; + this.name = name; + } + public long getCapacityBytes() { return capacityBytes; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/host/ListHostsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/host/ListHostsCmd.java index 75fe339b710..b8668f61ca4 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/host/ListHostsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/host/ListHostsCmd.java @@ -119,6 +119,10 @@ public class ListHostsCmd extends BaseListCmd { return id; } + public void setId(Long id) { + this.id = id; + } + public String getHostName() { return hostName; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ListStoragePoolsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ListStoragePoolsCmd.java index 209aaac279c..6923353b3bf 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ListStoragePoolsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ListStoragePoolsCmd.java @@ -24,6 +24,7 @@ import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseListCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.response.ClusterResponse; +import org.apache.cloudstack.api.response.HostResponse; import org.apache.cloudstack.api.response.ListResponse; import org.apache.cloudstack.api.response.PodResponse; import org.apache.cloudstack.api.response.StoragePoolResponse; @@ -63,16 +64,25 @@ public class ListStoragePoolsCmd extends BaseListCmd { @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = StoragePoolResponse.class, description = "the ID of the storage pool") private Long id; - @Parameter(name = ApiConstants.SCOPE, type = CommandType.STRING, entityType = StoragePoolResponse.class, description = "the ID of the storage pool") + @Parameter(name = ApiConstants.SCOPE, type = CommandType.STRING, entityType = StoragePoolResponse.class, description = "the scope of the storage pool") private String scope; + @Parameter(name = ApiConstants.STATUS, type = CommandType.STRING, description = "the status of the storage pool") private String status; + @Parameter(name = ApiConstants.HOST_ID, type = CommandType.UUID, entityType = HostResponse.class, description = "host ID of the storage pools") + private Long hostId; + + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// + public Long getHostId() { + return hostId; + } + public Long getClusterId() { return clusterId; } @@ -81,6 +91,10 @@ public class ListStoragePoolsCmd extends BaseListCmd { return ipAddress; } + public void setIpAddress(String ipAddress) { + this.ipAddress = ipAddress; + } + public String getStoragePoolName() { return storagePoolName; } @@ -108,6 +122,15 @@ public class ListStoragePoolsCmd extends BaseListCmd { public void setId(Long id) { this.id = id; } + + public String getScope() { + return scope; + } + + public void setScope(String scope) { + this.scope = scope; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// @@ -123,8 +146,4 @@ public class ListStoragePoolsCmd extends BaseListCmd { response.setResponseName(getCommandName()); this.setResponseObject(response); } - - public String getScope() { - return scope; - } } diff --git a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java index 213e5620553..6e178c119e3 100644 --- a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java +++ b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java @@ -27,6 +27,7 @@ import com.cloud.agent.api.ValidateVcenterDetailsCommand; import com.cloud.alert.AlertManager; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.StorageConflictException; +import com.cloud.exception.StorageUnavailableException; import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; @@ -44,7 +45,6 @@ import com.cloud.storage.dao.StoragePoolWorkDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.user.dao.UserDao; import com.cloud.utils.NumbersUtil; -import com.cloud.utils.UriUtils; import com.cloud.utils.db.DB; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VirtualMachineManager; @@ -67,10 +67,6 @@ import org.apache.cloudstack.storage.volume.datastore.PrimaryDataStoreHelper; import org.apache.log4j.Logger; import javax.inject.Inject; -import java.io.UnsupportedEncodingException; -import java.net.URI; -import java.net.URISyntaxException; -import java.net.URLDecoder; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -138,64 +134,26 @@ public class CloudStackPrimaryDataStoreLifeCycleImpl implements PrimaryDataStore PrimaryDataStoreParameters parameters = new PrimaryDataStoreParameters(); - UriUtils.UriInfo uriInfo = UriUtils.getUriInfo(url); - - String scheme = uriInfo.getScheme(); - String storageHost = uriInfo.getStorageHost(); - String storagePath = uriInfo.getStoragePath(); - try { - if (scheme == null) { - throw new InvalidParameterValueException("scheme is null " + url + ", add nfs:// (or cifs://) as a prefix"); - } else if (scheme.equalsIgnoreCase("nfs")) { - if (storageHost == null || storagePath == null || storageHost.trim().isEmpty() || storagePath.trim().isEmpty()) { - throw new InvalidParameterValueException("host or path is null, should be nfs://hostname/path"); - } - } else if (scheme.equalsIgnoreCase("cifs")) { - // Don't validate against a URI encoded URI. - URI cifsUri = new URI(url); - String warnMsg = UriUtils.getCifsUriParametersProblems(cifsUri); - if (warnMsg != null) { - throw new InvalidParameterValueException(warnMsg); - } - } else if (scheme.equalsIgnoreCase("sharedMountPoint")) { - if (storagePath == null) { - throw new InvalidParameterValueException("host or path is null, should be sharedmountpoint://localhost/path"); - } - } else if (scheme.equalsIgnoreCase("rbd")) { - if (storagePath == null) { - throw new InvalidParameterValueException("host or path is null, should be rbd://hostname/pool"); - } - } else if (scheme.equalsIgnoreCase("gluster")) { - if (storageHost == null || storagePath == null || storageHost.trim().isEmpty() || storagePath.trim().isEmpty()) { - throw new InvalidParameterValueException("host or path is null, should be gluster://hostname/volume"); - } - } - } catch (URISyntaxException e) { - throw new InvalidParameterValueException(url + " is not a valid uri"); - } - String tags = (String)dsInfos.get("tags"); Map details = (Map)dsInfos.get("details"); parameters.setTags(tags); parameters.setDetails(details); - String hostPath = null; - try { - hostPath = URLDecoder.decode(storagePath, "UTF-8"); - } catch (UnsupportedEncodingException e) { - s_logger.error("[ignored] we are on a platform not supporting \"UTF-8\"!?!", e); - } - if (hostPath == null) { // if decoding fails, use getPath() anyway - hostPath = storagePath; - } + String scheme = dsInfos.get("scheme").toString(); + String storageHost = dsInfos.get("host").toString(); + String hostPath = dsInfos.get("hostPath").toString(); + String uri = String.format("%s://%s%s", scheme, storageHost, hostPath); + Object localStorage = dsInfos.get("localStorage"); - if (localStorage != null) { - hostPath = hostPath.replaceFirst("/", ""); + if (localStorage != null) { + hostPath = hostPath.contains("//") ? hostPath.replaceFirst("/", "") : hostPath; hostPath = hostPath.replace("+", " "); } - String userInfo = uriInfo.getUserInfo(); - int port = uriInfo.getPort(); + + String userInfo = dsInfos.get("userInfo") != null ? dsInfos.get("userInfo").toString() : null; + int port = dsInfos.get("port") != null ? Integer.parseInt(dsInfos.get("port").toString()) : -1; + if (s_logger.isDebugEnabled()) { s_logger.debug("createPool Params @ scheme - " + scheme + " storageHost - " + storageHost + " hostPath - " + hostPath + " port - " + port); } @@ -312,8 +270,8 @@ public class CloudStackPrimaryDataStoreLifeCycleImpl implements PrimaryDataStore parameters.setPort(0); parameters.setPath(hostPath); } else { - s_logger.warn("Unable to figure out the scheme for URI: " + uriInfo); - throw new IllegalArgumentException("Unable to figure out the scheme for URI: " + uriInfo); + s_logger.warn("Unable to figure out the scheme for URI: " + scheme); + throw new IllegalArgumentException("Unable to figure out the scheme for URI: " + scheme); } } @@ -321,7 +279,7 @@ public class CloudStackPrimaryDataStoreLifeCycleImpl implements PrimaryDataStore List pools = primaryDataStoreDao.listPoolByHostPath(storageHost, hostPath); if (!pools.isEmpty() && !scheme.equalsIgnoreCase("sharedmountpoint")) { Long oldPodId = pools.get(0).getPodId(); - throw new CloudRuntimeException("Storage pool " + uriInfo + " already in use by another pod (id=" + oldPodId + ")"); + throw new CloudRuntimeException("Storage pool " + hostPath + " already in use by another pod (id=" + oldPodId + ")"); } } @@ -550,7 +508,16 @@ public class CloudStackPrimaryDataStoreLifeCycleImpl implements PrimaryDataStore @Override public boolean attachHost(DataStore store, HostScope scope, StoragePoolInfo existingInfo) { - dataStoreHelper.attachHost(store, scope, existingInfo); + DataStore dataStore = dataStoreHelper.attachHost(store, scope, existingInfo); + if(existingInfo.getCapacityBytes() == 0){ + try { + storageMgr.connectHostToSharedPool(scope.getScopeId(), dataStore.getId()); + } catch (StorageUnavailableException ex) { + s_logger.error("Storage unavailable ",ex); + } catch (StorageConflictException ex) { + s_logger.error("Storage already exists ",ex); + } + } return true; } diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 0056be5fce9..7a7e88ef449 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -2793,10 +2793,29 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q @Override public ListResponse searchForStoragePools(ListStoragePoolsCmd cmd) { - Pair, Integer> result = searchForStoragePoolsInternal(cmd); - ListResponse response = new ListResponse(); + Pair, Integer> result = cmd.getHostId() != null ? searchForLocalStorages(cmd) : searchForStoragePoolsInternal(cmd); + return createStoragesPoolResponse(result); + } - List poolResponses = ViewResponseHelper.createStoragePoolResponse(result.first().toArray(new StoragePoolJoinVO[result.first().size()])); + private Pair, Integer> searchForLocalStorages(ListStoragePoolsCmd cmd) { + long id = cmd.getHostId(); + String scope = ScopeType.HOST.toString(); + Pair, Integer> localStorages; + + ListHostsCmd listHostsCmd = new ListHostsCmd(); + listHostsCmd.setId(id); + Pair, Integer> hosts = searchForServersInternal(listHostsCmd); + + cmd.setScope(scope); + localStorages = searchForStoragePoolsInternal(cmd); + + return localStorages; + } + + private ListResponse createStoragesPoolResponse(Pair, Integer> storagePools) { + ListResponse response = new ListResponse<>(); + + List poolResponses = ViewResponseHelper.createStoragePoolResponse(storagePools.first().toArray(new StoragePoolJoinVO[storagePools.first().size()])); for (StoragePoolResponse poolResponse : poolResponses) { DataStore store = dataStoreManager.getPrimaryDataStore(poolResponse.getId()); if (store != null) { @@ -2816,7 +2835,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q } } - response.setResponses(poolResponses, result.second()); + response.setResponses(poolResponses, storagePools.second()); return response; } diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 82a6c159565..494855b253d 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -237,6 +237,9 @@ import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.dao.VMInstanceDao; import com.google.common.collect.Sets; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; + @Component public class StorageManagerImpl extends ManagerBase implements StorageManager, ClusterManagerListener, Configurable { @@ -653,6 +656,41 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C return true; } + private DataStore createLocalStorage(Map poolInfos) throws ConnectionException{ + Object existingUuid = poolInfos.get("uuid"); + if( existingUuid == null ){ + poolInfos.put("uuid", UUID.randomUUID().toString()); + } + String hostAddress = poolInfos.get("host").toString(); + Host host = _hostDao.findByName(hostAddress); + + if( host == null ) { + host = _hostDao.findByIp(hostAddress); + + if( host == null ) { + host = _hostDao.findByPublicIp(hostAddress); + + if( host == null ) { + throw new InvalidParameterValueException(String.format("host %s not found",hostAddress)); + } + } + } + + long capacityBytes = poolInfos.get("capacityBytes") != null ? Long.parseLong(poolInfos.get("capacityBytes").toString()) : 0; + + StoragePoolInfo pInfo = new StoragePoolInfo(poolInfos.get("uuid").toString(), + host.getPrivateIpAddress(), + poolInfos.get("hostPath").toString(), + poolInfos.get("hostPath").toString(), + StoragePoolType.Filesystem, + capacityBytes, + 0, + (Map)poolInfos.get("details"), + poolInfos.get("name").toString()); + + return createLocalStorage(host, pInfo); + } + @DB @Override public DataStore createLocalStorage(Host host, StoragePoolInfo pInfo) throws ConnectionException { @@ -698,17 +736,19 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C DataStoreLifeCycle lifeCycle = provider.getDataStoreLifeCycle(); if (pool == null) { Map params = new HashMap(); - String name = createLocalStoragePoolName(host, pInfo); + String name = pInfo.getName() != null ? pInfo.getName() : createLocalStoragePoolName(host, pInfo); params.put("zoneId", host.getDataCenterId()); params.put("clusterId", host.getClusterId()); params.put("podId", host.getPodId()); params.put("hypervisorType", host.getHypervisorType()); - params.put("url", pInfo.getPoolType().toString() + "://" + pInfo.getHost() + "/" + pInfo.getHostPath()); params.put("name", name); params.put("localStorage", true); params.put("details", pInfo.getDetails()); params.put("uuid", pInfo.getUuid()); params.put("providerName", provider.getName()); + params.put("scheme", pInfo.getPoolType().toString()); + params.put("host", pInfo.getHost()); + params.put("hostPath", pInfo.getHostPath()); store = lifeCycle.initialize(params); } else { @@ -740,6 +780,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Override public PrimaryDataStoreInfo createPool(CreateStoragePoolCmd cmd) throws ResourceInUseException, IllegalArgumentException, UnknownHostException, ResourceUnavailableException { String providerName = cmd.getStorageProviderName(); + Map uriParams = extractUriParamsAsMap(cmd.getUrl()); DataStoreProvider storeProvider = _dataStoreProviderMgr.getDataStoreProvider(providerName); if (storeProvider == null) { @@ -753,7 +794,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C Long podId = cmd.getPodId(); Long zoneId = cmd.getZoneId(); - ScopeType scopeType = ScopeType.CLUSTER; + ScopeType scopeType = uriParams.get("scheme").toString().equals("file") ? ScopeType.HOST : ScopeType.CLUSTER; String scope = cmd.getScope(); if (scope != null) { try { @@ -819,11 +860,16 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C params.put("managed", cmd.isManaged()); params.put("capacityBytes", cmd.getCapacityBytes()); params.put("capacityIops", cmd.getCapacityIops()); + params.putAll(uriParams); DataStoreLifeCycle lifeCycle = storeProvider.getDataStoreLifeCycle(); DataStore store = null; try { - store = lifeCycle.initialize(params); + if (params.get("scheme").toString().equals("file")) { + store = createLocalStorage(params); + } else { + store = lifeCycle.initialize(params); + } if (scopeType == ScopeType.CLUSTER) { ClusterScope clusterScope = new ClusterScope(clusterId, podId, zoneId); lifeCycle.attachCluster(store, clusterScope); @@ -848,6 +894,62 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(store.getId(), DataStoreRole.Primary); } + private Map extractUriParamsAsMap(String url){ + Map uriParams = new HashMap<>(); + UriUtils.UriInfo uriInfo = UriUtils.getUriInfo(url); + + String scheme = uriInfo.getScheme(); + String storageHost = uriInfo.getStorageHost(); + String storagePath = uriInfo.getStoragePath(); + try { + if (scheme == null) { + throw new InvalidParameterValueException("scheme is null " + url + ", add nfs:// (or cifs://) as a prefix"); + } else if (scheme.equalsIgnoreCase("nfs")) { + if (storageHost == null || storagePath == null || storageHost.trim().isEmpty() || storagePath.trim().isEmpty()) { + throw new InvalidParameterValueException("host or path is null, should be nfs://hostname/path"); + } + } else if (scheme.equalsIgnoreCase("cifs")) { + // Don't validate against a URI encoded URI. + URI cifsUri = new URI(url); + String warnMsg = UriUtils.getCifsUriParametersProblems(cifsUri); + if (warnMsg != null) { + throw new InvalidParameterValueException(warnMsg); + } + } else if (scheme.equalsIgnoreCase("sharedMountPoint")) { + if (storagePath == null) { + throw new InvalidParameterValueException("host or path is null, should be sharedmountpoint://localhost/path"); + } + } else if (scheme.equalsIgnoreCase("rbd")) { + if (storagePath == null) { + throw new InvalidParameterValueException("host or path is null, should be rbd://hostname/pool"); + } + } else if (scheme.equalsIgnoreCase("gluster")) { + if (storageHost == null || storagePath == null || storageHost.trim().isEmpty() || storagePath.trim().isEmpty()) { + throw new InvalidParameterValueException("host or path is null, should be gluster://hostname/volume"); + } + } + } catch (URISyntaxException e) { + throw new InvalidParameterValueException(url + " is not a valid uri"); + } + + String hostPath = null; + try { + hostPath = URLDecoder.decode(storagePath, "UTF-8"); + } catch (UnsupportedEncodingException e) { + s_logger.error("[ignored] we are on a platform not supporting \"UTF-8\"!?!", e); + } + if (hostPath == null) { // if decoding fails, use getPath() anyway + hostPath = storagePath; + } + + uriParams.put("scheme", scheme); + uriParams.put("host", storageHost); + uriParams.put("hostPath", hostPath); + uriParams.put("userInfo", uriInfo.getUserInfo()); + uriParams.put("port", uriInfo.getPort() + ""); + return uriParams; + } + private Map extractApiParamAsMap(Map ds) { Map details = new HashMap(); if (ds != null) { diff --git a/ui/src/views/infra/AddPrimaryStorage.vue b/ui/src/views/infra/AddPrimaryStorage.vue index b30b0341ca4..91c0dcbf42e 100644 --- a/ui/src/views/infra/AddPrimaryStorage.vue +++ b/ui/src/views/infra/AddPrimaryStorage.vue @@ -32,6 +32,7 @@ {{ $t('label.clusterid') }} {{ $t('label.zoneid') }} + {{ $t('label.hostid') }}
@@ -168,7 +170,7 @@
-
+
diff --git a/ui/src/components/view/DetailsTab.vue b/ui/src/components/view/DetailsTab.vue index ebdc459c439..b04830b3c3c 100644 --- a/ui/src/components/view/DetailsTab.vue +++ b/ui/src/components/view/DetailsTab.vue @@ -207,9 +207,11 @@ export default { } const managementDeviceIds = [] - for (const vnfnic of this.resource.vnfnics) { - if (vnfnic.management) { - managementDeviceIds.push(vnfnic.deviceid) + if (this.resource.vnfnics) { + for (const vnfnic of this.resource.vnfnics) { + if (vnfnic.management) { + managementDeviceIds.push(vnfnic.deviceid) + } } } const managementIps = [] diff --git a/ui/src/config/section/network.js b/ui/src/config/section/network.js index 5c78a4d0e68..0362b424d1d 100644 --- a/ui/src/config/section/network.js +++ b/ui/src/config/section/network.js @@ -325,6 +325,7 @@ export default { title: 'label.vnf.appliances', icon: 'gateway-outlined', permission: ['listVirtualMachinesMetrics'], + resourceType: 'UserVm', params: () => { return { details: 'servoff,tmpl,nics', isvnf: true } }, diff --git a/ui/src/views/compute/DeployVnfAppliance.vue b/ui/src/views/compute/DeployVnfAppliance.vue index e571f4e23c4..5e1baac3d8f 100644 --- a/ui/src/views/compute/DeployVnfAppliance.vue +++ b/ui/src/views/compute/DeployVnfAppliance.vue @@ -365,7 +365,7 @@