Skip to content

Commit 8adb8df

Browse files
shwstpprshwstppr
authored
server: find suitable disk offering for volume upload (apache#5852)
* server: find suitable disk offering for volume upload Fixes apache#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 <abhishek.mrt22@gmail.com>
1 parent ddd311c commit 8adb8df

5 files changed

Lines changed: 121 additions & 26 deletions

File tree

‎engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDao.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,6 @@ public interface DiskOfferingDao extends GenericDao<DiskOfferingVO, Long> {
3434

3535
List<DiskOfferingVO> listAllBySizeAndProvisioningType(long size, Storage.ProvisioningType provisioningType);
3636

37+
List<DiskOfferingVO> findCustomDiskOfferings();
38+
3739
}

‎engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao;
3030
import org.springframework.stereotype.Component;
3131

32+
import com.cloud.offering.DiskOffering;
3233
import com.cloud.offering.DiskOffering.Type;
3334
import com.cloud.storage.DiskOfferingVO;
3435
import com.cloud.storage.Storage;
@@ -173,6 +174,18 @@ public List<DiskOfferingVO> listAllBySizeAndProvisioningType(long size, Storage.
173174
}
174175
}
175176

177+
@Override
178+
public List<DiskOfferingVO> findCustomDiskOfferings() {
179+
SearchBuilder<DiskOfferingVO> sb = createSearchBuilder();
180+
sb.and("type", sb.entity().getType(), SearchCriteria.Op.EQ);
181+
sb.and("customized", sb.entity().isCustomized(), SearchCriteria.Op.EQ);
182+
sb.done();
183+
SearchCriteria<DiskOfferingVO> sc = sb.create();
184+
sc.setParameters("customized", true);
185+
sc.setParameters("type", DiskOffering.Type.Disk.toString());
186+
return listBy(sc);
187+
}
188+
176189
@Override
177190
public boolean remove(Long id) {
178191
DiskOfferingVO diskOffering = createForUpdate();

‎server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,8 @@
1919
import java.util.List;
2020
import java.util.Map;
2121

22-
import com.cloud.api.ApiDBUtils;
23-
import com.cloud.dc.VsphereStoragePolicyVO;
24-
import com.cloud.dc.dao.VsphereStoragePolicyDao;
25-
import com.cloud.server.ResourceTag;
26-
import com.cloud.user.AccountManager;
22+
import javax.inject.Inject;
23+
2724
import org.apache.cloudstack.annotation.AnnotationService;
2825
import org.apache.cloudstack.annotation.dao.AnnotationDao;
2926
import org.apache.cloudstack.api.ApiConstants;
@@ -32,16 +29,19 @@
3229
import org.apache.log4j.Logger;
3330
import org.springframework.stereotype.Component;
3431

32+
import com.cloud.api.ApiDBUtils;
3533
import com.cloud.api.query.vo.DiskOfferingJoinVO;
34+
import com.cloud.dc.VsphereStoragePolicyVO;
35+
import com.cloud.dc.dao.VsphereStoragePolicyDao;
3636
import com.cloud.offering.DiskOffering;
3737
import com.cloud.offering.ServiceOffering;
38+
import com.cloud.server.ResourceTag;
39+
import com.cloud.user.AccountManager;
3840
import com.cloud.utils.db.Attribute;
3941
import com.cloud.utils.db.GenericDaoBase;
4042
import com.cloud.utils.db.SearchBuilder;
4143
import com.cloud.utils.db.SearchCriteria;
4244

43-
import javax.inject.Inject;
44-
4545
@Component
4646
public class DiskOfferingJoinDaoImpl extends GenericDaoBase<DiskOfferingJoinVO, Long> implements DiskOfferingJoinDao {
4747
public static final Logger s_logger = Logger.getLogger(DiskOfferingJoinDaoImpl.class);

‎server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java‎

Lines changed: 45 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@
125125
import com.cloud.hypervisor.Hypervisor.HypervisorType;
126126
import com.cloud.hypervisor.HypervisorCapabilitiesVO;
127127
import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao;
128+
import com.cloud.offering.DiskOffering;
128129
import com.cloud.org.Grouping;
129130
import com.cloud.resource.ResourceState;
130131
import com.cloud.serializer.GsonHelper;
@@ -315,6 +316,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
315316

316317
private static final Set<Volume.State> STATES_VOLUME_CANNOT_BE_DESTROYED = new HashSet<>(Arrays.asList(Volume.State.Destroy, Volume.State.Expunging, Volume.State.Expunged, Volume.State.Allocated));
317318

319+
private static final String CUSTOM_DISK_OFFERING_UNIQUE_NAME = "Cloud.com-Custom";
320+
318321
protected VolumeApiServiceImpl() {
319322
_volStateMachine = Volume.State.getStateMachine();
320323
_gson = GsonHelper.getGsonLogger();
@@ -478,7 +481,6 @@ private boolean validateVolume(Account caller, long ownerId, Long zoneId, String
478481
if (!diskOffering.isCustomized()) {
479482
throw new InvalidParameterValueException("Please specify a custom sized disk offering.");
480483
}
481-
482484
_configMgr.checkDiskOfferingAccess(volumeOwner, diskOffering, zone);
483485
}
484486

@@ -489,12 +491,41 @@ public String getRandomVolumeName() {
489491
return UUID.randomUUID().toString();
490492
}
491493

494+
private Long getDefaultCustomOfferingId(Account owner, DataCenter zone) {
495+
DiskOfferingVO diskOfferingVO = _diskOfferingDao.findByUniqueName(CUSTOM_DISK_OFFERING_UNIQUE_NAME);
496+
if (diskOfferingVO == null || !DiskOffering.State.Active.equals(diskOfferingVO.getState())) {
497+
return null;
498+
}
499+
try {
500+
_configMgr.checkDiskOfferingAccess(owner, diskOfferingVO, zone);
501+
return diskOfferingVO.getId();
502+
} catch (PermissionDeniedException ignored) {
503+
}
504+
return null;
505+
}
506+
507+
private Long getCustomDiskOfferingIdForVolumeUpload(Account owner, DataCenter zone) {
508+
Long offeringId = getDefaultCustomOfferingId(owner, zone);
509+
if (offeringId != null) {
510+
return offeringId;
511+
}
512+
List<DiskOfferingVO> offerings = _diskOfferingDao.findCustomDiskOfferings();
513+
for (DiskOfferingVO offering : offerings) {
514+
try {
515+
_configMgr.checkDiskOfferingAccess(owner, offering, zone);
516+
return offering.getId();
517+
} catch (PermissionDeniedException ignored) {}
518+
}
519+
return null;
520+
}
521+
492522
@DB
493523
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) {
494-
return Transaction.execute(new TransactionCallback<VolumeVO>() {
524+
return Transaction.execute(new TransactionCallbackWithException<VolumeVO, CloudRuntimeException>() {
495525
@Override
496526
public VolumeVO doInTransaction(TransactionStatus status) {
497527
VolumeVO volume = new VolumeVO(volumeName, zoneId, -1, -1, -1, new Long(-1), null, null, Storage.ProvisioningType.THIN, 0, Volume.Type.DATADISK);
528+
DataCenter zone = _dcDao.findById(zoneId);
498529
volume.setPoolId(null);
499530
volume.setDataCenterId(zoneId);
500531
volume.setPodId(null);
@@ -504,23 +535,22 @@ public VolumeVO doInTransaction(TransactionStatus status) {
504535
volume.setAccountId((owner == null) ? Account.ACCOUNT_ID_SYSTEM : owner.getAccountId());
505536
volume.setDomainId((owner == null) ? Domain.ROOT_DOMAIN : owner.getDomainId());
506537

507-
if (diskOfferingId == null) {
508-
DiskOfferingVO diskOfferingVO = _diskOfferingDao.findByUniqueName("Cloud.com-Custom");
509-
if (diskOfferingVO != null) {
510-
long defaultDiskOfferingId = diskOfferingVO.getId();
511-
volume.setDiskOfferingId(defaultDiskOfferingId);
538+
Long volumeDiskOfferingId = diskOfferingId;
539+
if (volumeDiskOfferingId == null) {
540+
volumeDiskOfferingId = getCustomDiskOfferingIdForVolumeUpload(owner, zone);
541+
if (volumeDiskOfferingId == null) {
542+
throw new CloudRuntimeException(String.format("Unable to find custom disk offering in zone: %s for volume upload", zone.getUuid()));
512543
}
513-
} else {
514-
volume.setDiskOfferingId(diskOfferingId);
544+
}
515545

516-
DiskOfferingVO diskOfferingVO = _diskOfferingDao.findById(diskOfferingId);
546+
volume.setDiskOfferingId(volumeDiskOfferingId);
547+
DiskOfferingVO diskOfferingVO = _diskOfferingDao.findById(volumeDiskOfferingId);
517548

518-
Boolean isCustomizedIops = diskOfferingVO != null && diskOfferingVO.isCustomizedIops() != null ? diskOfferingVO.isCustomizedIops() : false;
549+
Boolean isCustomizedIops = diskOfferingVO != null && diskOfferingVO.isCustomizedIops() != null ? diskOfferingVO.isCustomizedIops() : false;
519550

520-
if (isCustomizedIops == null || !isCustomizedIops) {
521-
volume.setMinIops(diskOfferingVO.getMinIops());
522-
volume.setMaxIops(diskOfferingVO.getMaxIops());
523-
}
551+
if (isCustomizedIops == null || !isCustomizedIops) {
552+
volume.setMinIops(diskOfferingVO.getMinIops());
553+
volume.setMaxIops(diskOfferingVO.getMaxIops());
524554
}
525555

526556
// volume.setSize(size);

‎ui/src/views/storage/UploadLocalVolume.vue‎

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@
6969
optionFilterProp="children"
7070
:filterOption="(input, option) => {
7171
return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0
72-
}" >
72+
}"
73+
@change="onZoneChange" >
7374
<a-select-option :value="zone.id" v-for="zone in zones" :key="zone.id" :label="zone.name || zone.description">
7475
<span>
7576
<resource-icon v-if="zone.icon" :image="zone.icon.base64image" size="1x" style="margin-right: 5px"/>
@@ -79,6 +80,22 @@
7980
</a-select-option>
8081
</a-select>
8182
</a-form-item>
83+
<a-form-item>
84+
<tooltip-label slot="label" :title="$t('label.diskofferingid')" :tooltip="apiParams.diskofferingid.description"/>
85+
<a-select
86+
v-decorator="['diskofferingid', {}]"
87+
:loading="offeringLoading"
88+
:placeholder="apiParams.diskofferingid.description"
89+
showSearch
90+
optionFilterProp="children"
91+
:filterOption="(input, option) => {
92+
return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0
93+
}" >
94+
<a-select-option v-for="opt in offerings" :key="opt.id">
95+
{{ opt.name || opt.displaytext }}
96+
</a-select-option>
97+
</a-select>
98+
</a-form-item>
8299
<a-form-item>
83100
<tooltip-label slot="label" :title="$t('label.format')" :tooltip="apiParams.format.description"/>
84101
<a-select
@@ -133,6 +150,8 @@ export default {
133150
return {
134151
fileList: [],
135152
zones: [],
153+
offerings: [],
154+
offeringLoading: false,
136155
formats: ['RAW', 'VHD', 'VHDX', 'OVA', 'QCOW2'],
137156
zoneSelected: '',
138157
uploadParams: null,
@@ -153,11 +172,34 @@ export default {
153172
if (json && json.listzonesresponse && json.listzonesresponse.zone) {
154173
this.zones = json.listzonesresponse.zone
155174
if (this.zones.length > 0) {
156-
this.zoneSelected = this.zones[0].id
175+
this.onZoneChange(this.zones[0].id)
157176
}
158177
}
159178
})
160179
},
180+
onZoneChange (zoneId) {
181+
this.zoneSelected = this.zones[0].id
182+
this.fetchDiskOfferings(zoneId)
183+
},
184+
fetchDiskOfferings (zoneId) {
185+
this.offeringLoading = true
186+
this.offerings = [{ id: -1, name: '' }]
187+
this.form.setFieldsValue({
188+
diskofferingid: undefined
189+
})
190+
api('listDiskOfferings', {
191+
zoneid: zoneId,
192+
listall: true
193+
}).then(json => {
194+
for (var offering of json.listdiskofferingsresponse.diskoffering) {
195+
if (offering.iscustomized) {
196+
this.offerings.push(offering)
197+
}
198+
}
199+
}).finally(() => {
200+
this.offeringLoading = false
201+
})
202+
},
161203
handleRemove (file) {
162204
const index = this.fileList.indexOf(file)
163205
const newFileList = this.fileList.slice()
@@ -188,7 +230,7 @@ export default {
188230
}
189231
this.loading = true
190232
api('getUploadParamsForVolume', params).then(json => {
191-
this.uploadParams = (json.postuploadvolumeresponse && json.postuploadvolumeresponse.getuploadparams) ? json.postuploadvolumeresponse.getuploadparams : ''
233+
this.uploadParams = json.postuploadvolumeresponse?.getuploadparams || ''
192234
const { fileList } = this
193235
if (this.fileList.length > 1) {
194236
this.$notification.error({
@@ -224,12 +266,20 @@ export default {
224266
}).catch(e => {
225267
this.$notification.error({
226268
message: this.$t('message.upload.failed'),
227-
description: `${this.$t('message.upload.iso.failed.description')} - ${e}`,
269+
description: `${this.$t('message.upload.volume.failed')} - ${e}`,
228270
duration: 0
229271
})
230272
}).finally(() => {
231273
this.loading = false
232274
})
275+
}).catch(e => {
276+
this.$notification.error({
277+
message: this.$t('message.upload.failed'),
278+
description: `${this.$t('message.upload.volume.failed')} - ${e?.response?.data?.postuploadvolumeresponse?.errortext || e}`,
279+
duration: 0
280+
})
281+
}).finally(() => {
282+
this.loading = false
233283
})
234284
})
235285
},

0 commit comments

Comments
 (0)