From 8adb8df2fe8a3b29f19dc571b65ac33987f9be59 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 2 Feb 2022 16:35:47 +0530 Subject: [PATCH] server: find suitable disk offering for volume upload (#5852) * server: find suitable disk offering for volume upload Fixes #5696 * fix npe check * fixes, refactor, rename method and handle custom iops * ui: allow offering selection * list only disk offerings * show name * revert error check * use checkaccess Signed-off-by: Abhishek Kumar --- .../cloud/storage/dao/DiskOfferingDao.java | 2 + .../storage/dao/DiskOfferingDaoImpl.java | 13 ++++ .../query/dao/DiskOfferingJoinDaoImpl.java | 14 ++--- .../cloud/storage/VolumeApiServiceImpl.java | 60 ++++++++++++++----- ui/src/views/storage/UploadLocalVolume.vue | 58 ++++++++++++++++-- 5 files changed, 121 insertions(+), 26 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDao.java index 3305752459d..2425e584122 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDao.java @@ -34,4 +34,6 @@ public interface DiskOfferingDao extends GenericDao { List listAllBySizeAndProvisioningType(long size, Storage.ProvisioningType provisioningType); + List findCustomDiskOfferings(); + } diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java index b9fa10c1236..ce52b0ed3dc 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java @@ -29,6 +29,7 @@ import javax.persistence.EntityExistsException; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.springframework.stereotype.Component; +import com.cloud.offering.DiskOffering; import com.cloud.offering.DiskOffering.Type; import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.Storage; @@ -173,6 +174,18 @@ public class DiskOfferingDaoImpl extends GenericDaoBase im } } + @Override + public List findCustomDiskOfferings() { + SearchBuilder sb = createSearchBuilder(); + sb.and("type", sb.entity().getType(), SearchCriteria.Op.EQ); + sb.and("customized", sb.entity().isCustomized(), SearchCriteria.Op.EQ); + sb.done(); + SearchCriteria sc = sb.create(); + sc.setParameters("customized", true); + sc.setParameters("type", DiskOffering.Type.Disk.toString()); + return listBy(sc); + } + @Override public boolean remove(Long id) { DiskOfferingVO diskOffering = createForUpdate(); diff --git a/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java index 685f30169b4..a738aed3020 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java @@ -19,11 +19,8 @@ package com.cloud.api.query.dao; import java.util.List; import java.util.Map; -import com.cloud.api.ApiDBUtils; -import com.cloud.dc.VsphereStoragePolicyVO; -import com.cloud.dc.dao.VsphereStoragePolicyDao; -import com.cloud.server.ResourceTag; -import com.cloud.user.AccountManager; +import javax.inject.Inject; + import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.api.ApiConstants; @@ -32,16 +29,19 @@ import org.apache.cloudstack.context.CallContext; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; +import com.cloud.api.ApiDBUtils; import com.cloud.api.query.vo.DiskOfferingJoinVO; +import com.cloud.dc.VsphereStoragePolicyVO; +import com.cloud.dc.dao.VsphereStoragePolicyDao; import com.cloud.offering.DiskOffering; import com.cloud.offering.ServiceOffering; +import com.cloud.server.ResourceTag; +import com.cloud.user.AccountManager; import com.cloud.utils.db.Attribute; import com.cloud.utils.db.GenericDaoBase; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; -import javax.inject.Inject; - @Component public class DiskOfferingJoinDaoImpl extends GenericDaoBase implements DiskOfferingJoinDao { public static final Logger s_logger = Logger.getLogger(DiskOfferingJoinDaoImpl.class); diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index d092ad11135..35de68b43e0 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -125,6 +125,7 @@ import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.hypervisor.HypervisorCapabilitiesVO; import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao; +import com.cloud.offering.DiskOffering; import com.cloud.org.Grouping; import com.cloud.resource.ResourceState; import com.cloud.serializer.GsonHelper; @@ -315,6 +316,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic private static final Set STATES_VOLUME_CANNOT_BE_DESTROYED = new HashSet<>(Arrays.asList(Volume.State.Destroy, Volume.State.Expunging, Volume.State.Expunged, Volume.State.Allocated)); + private static final String CUSTOM_DISK_OFFERING_UNIQUE_NAME = "Cloud.com-Custom"; + protected VolumeApiServiceImpl() { _volStateMachine = Volume.State.getStateMachine(); _gson = GsonHelper.getGsonLogger(); @@ -478,7 +481,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic if (!diskOffering.isCustomized()) { throw new InvalidParameterValueException("Please specify a custom sized disk offering."); } - _configMgr.checkDiskOfferingAccess(volumeOwner, diskOffering, zone); } @@ -489,12 +491,41 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic return UUID.randomUUID().toString(); } + private Long getDefaultCustomOfferingId(Account owner, DataCenter zone) { + DiskOfferingVO diskOfferingVO = _diskOfferingDao.findByUniqueName(CUSTOM_DISK_OFFERING_UNIQUE_NAME); + if (diskOfferingVO == null || !DiskOffering.State.Active.equals(diskOfferingVO.getState())) { + return null; + } + try { + _configMgr.checkDiskOfferingAccess(owner, diskOfferingVO, zone); + return diskOfferingVO.getId(); + } catch (PermissionDeniedException ignored) { + } + return null; + } + + private Long getCustomDiskOfferingIdForVolumeUpload(Account owner, DataCenter zone) { + Long offeringId = getDefaultCustomOfferingId(owner, zone); + if (offeringId != null) { + return offeringId; + } + List offerings = _diskOfferingDao.findCustomDiskOfferings(); + for (DiskOfferingVO offering : offerings) { + try { + _configMgr.checkDiskOfferingAccess(owner, offering, zone); + return offering.getId(); + } catch (PermissionDeniedException ignored) {} + } + return null; + } + @DB protected VolumeVO persistVolume(final Account owner, final Long zoneId, final String volumeName, final String url, final String format, final Long diskOfferingId, final Volume.State state) { - return Transaction.execute(new TransactionCallback() { + return Transaction.execute(new TransactionCallbackWithException() { @Override public VolumeVO doInTransaction(TransactionStatus status) { VolumeVO volume = new VolumeVO(volumeName, zoneId, -1, -1, -1, new Long(-1), null, null, Storage.ProvisioningType.THIN, 0, Volume.Type.DATADISK); + DataCenter zone = _dcDao.findById(zoneId); volume.setPoolId(null); volume.setDataCenterId(zoneId); volume.setPodId(null); @@ -504,23 +535,22 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic volume.setAccountId((owner == null) ? Account.ACCOUNT_ID_SYSTEM : owner.getAccountId()); volume.setDomainId((owner == null) ? Domain.ROOT_DOMAIN : owner.getDomainId()); - if (diskOfferingId == null) { - DiskOfferingVO diskOfferingVO = _diskOfferingDao.findByUniqueName("Cloud.com-Custom"); - if (diskOfferingVO != null) { - long defaultDiskOfferingId = diskOfferingVO.getId(); - volume.setDiskOfferingId(defaultDiskOfferingId); + Long volumeDiskOfferingId = diskOfferingId; + if (volumeDiskOfferingId == null) { + volumeDiskOfferingId = getCustomDiskOfferingIdForVolumeUpload(owner, zone); + if (volumeDiskOfferingId == null) { + throw new CloudRuntimeException(String.format("Unable to find custom disk offering in zone: %s for volume upload", zone.getUuid())); } - } else { - volume.setDiskOfferingId(diskOfferingId); + } - DiskOfferingVO diskOfferingVO = _diskOfferingDao.findById(diskOfferingId); + volume.setDiskOfferingId(volumeDiskOfferingId); + DiskOfferingVO diskOfferingVO = _diskOfferingDao.findById(volumeDiskOfferingId); - Boolean isCustomizedIops = diskOfferingVO != null && diskOfferingVO.isCustomizedIops() != null ? diskOfferingVO.isCustomizedIops() : false; + Boolean isCustomizedIops = diskOfferingVO != null && diskOfferingVO.isCustomizedIops() != null ? diskOfferingVO.isCustomizedIops() : false; - if (isCustomizedIops == null || !isCustomizedIops) { - volume.setMinIops(diskOfferingVO.getMinIops()); - volume.setMaxIops(diskOfferingVO.getMaxIops()); - } + if (isCustomizedIops == null || !isCustomizedIops) { + volume.setMinIops(diskOfferingVO.getMinIops()); + volume.setMaxIops(diskOfferingVO.getMaxIops()); } // volume.setSize(size); diff --git a/ui/src/views/storage/UploadLocalVolume.vue b/ui/src/views/storage/UploadLocalVolume.vue index c4f1d569014..0e5df836fcc 100644 --- a/ui/src/views/storage/UploadLocalVolume.vue +++ b/ui/src/views/storage/UploadLocalVolume.vue @@ -69,7 +69,8 @@ optionFilterProp="children" :filterOption="(input, option) => { return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 - }" > + }" + @change="onZoneChange" > @@ -79,6 +80,22 @@ + + + + + {{ opt.name || opt.displaytext }} + + + 0) { - this.zoneSelected = this.zones[0].id + this.onZoneChange(this.zones[0].id) } } }) }, + onZoneChange (zoneId) { + this.zoneSelected = this.zones[0].id + this.fetchDiskOfferings(zoneId) + }, + fetchDiskOfferings (zoneId) { + this.offeringLoading = true + this.offerings = [{ id: -1, name: '' }] + this.form.setFieldsValue({ + diskofferingid: undefined + }) + api('listDiskOfferings', { + zoneid: zoneId, + listall: true + }).then(json => { + for (var offering of json.listdiskofferingsresponse.diskoffering) { + if (offering.iscustomized) { + this.offerings.push(offering) + } + } + }).finally(() => { + this.offeringLoading = false + }) + }, handleRemove (file) { const index = this.fileList.indexOf(file) const newFileList = this.fileList.slice() @@ -188,7 +230,7 @@ export default { } this.loading = true api('getUploadParamsForVolume', params).then(json => { - this.uploadParams = (json.postuploadvolumeresponse && json.postuploadvolumeresponse.getuploadparams) ? json.postuploadvolumeresponse.getuploadparams : '' + this.uploadParams = json.postuploadvolumeresponse?.getuploadparams || '' const { fileList } = this if (this.fileList.length > 1) { this.$notification.error({ @@ -224,12 +266,20 @@ export default { }).catch(e => { this.$notification.error({ message: this.$t('message.upload.failed'), - description: `${this.$t('message.upload.iso.failed.description')} - ${e}`, + description: `${this.$t('message.upload.volume.failed')} - ${e}`, duration: 0 }) }).finally(() => { this.loading = false }) + }).catch(e => { + this.$notification.error({ + message: this.$t('message.upload.failed'), + description: `${this.$t('message.upload.volume.failed')} - ${e?.response?.data?.postuploadvolumeresponse?.errortext || e}`, + duration: 0 + }) + }).finally(() => { + this.loading = false }) }) },