Skip to content

Commit 06428dd

Browse files
slavkapslavkap
committed
Add StorPool tags on the new ROOT volume
Add the StorPool's tags when volume is created from a snapshot or a volume is attached as a ROOT to a VM Addressed reviews
1 parent be5b5bd commit 06428dd

File tree

6 files changed

+78
-28
lines changed

6 files changed

+78
-28
lines changed

‎api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
import com.cloud.vm.VirtualMachine;
3737
import com.cloud.vm.VmDetailConstants;
3838

39+
import java.util.Objects;
40+
import java.util.stream.Stream;
3941
import org.apache.cloudstack.acl.RoleType;
4042
import org.apache.cloudstack.affinity.AffinityGroupResponse;
4143
import org.apache.cloudstack.api.ACL;
@@ -861,9 +863,10 @@ public void execute() {
861863

862864
@Override
863865
public void create() throws ResourceAllocationException {
864-
if (!isVolumeOrSnapshotProvided() && templateId == null) {
865-
throw new CloudRuntimeException("There isn't a ROOT volume, snapshot of ROOT volume or template provided to deploy a Virtual machine");
866+
if (Stream.of(templateId, snapshotId, volumeId).filter(Objects::nonNull).count() != 1) {
867+
throw new CloudRuntimeException(String.format("Only one of the parameters - template ID [%s], volume ID [%s] or snapshot ID [%s] - should be provided to deploy a Virtual machine", templateId, volumeId, snapshotId));
866868
}
869+
867870
try {
868871
UserVm vm = _userVmService.createVirtualMachine(this);
869872

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/CloudOrchestrator.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import com.cloud.storage.Snapshot;
2222
import com.cloud.storage.Volume;
23+
import com.cloud.template.VirtualMachineTemplate;
2324
import java.net.URL;
2425
import java.util.ArrayList;
2526
import java.util.LinkedHashMap;
@@ -256,8 +257,12 @@ public VirtualMachineEntity createVirtualMachine(String id, String owner, String
256257
}
257258
}
258259
}
259-
260-
_itMgr.allocate(vm.getInstanceName(), _templateDao.findByIdIncludingRemoved(new Long(templateId)), computeOffering, rootDiskOfferingInfo, dataDiskOfferings, networkIpMap, plan,
260+
VirtualMachineTemplate template = null;
261+
if (volume != null || snapshot != null) {
262+
template = _templateDao.findByIdIncludingRemoved(new Long(templateId));
263+
} else
264+
template = _templateDao.findById(new Long(templateId));
265+
_itMgr.allocate(vm.getInstanceName(), template, computeOffering, rootDiskOfferingInfo, dataDiskOfferings, networkIpMap, plan,
261266
hypervisorType, extraDhcpOptionMap, dataDiskTemplateToDiskOfferingMap, volume, snapshot);
262267

263268
return vmEntity;

‎plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java‎

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver;
3838
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
3939
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
40+
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
4041
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
4142
import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
4243
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
@@ -171,6 +172,8 @@ public class StorPoolPrimaryDataStoreDriver implements PrimaryDataStoreDriver {
171172
private ServiceOfferingDetailsDao serviceOfferingDetailDao;
172173
@Inject
173174
private ServiceOfferingDao serviceOfferingDao;
175+
@Inject
176+
private VolumeDataFactory volumeDataFactory;
174177

175178
private SnapshotDataStoreVO getSnapshotImageStoreRef(long snapshotId, long zoneId) {
176179
List<SnapshotDataStoreVO> snaps = snapshotDataStoreDao.listReadyBySnapshot(snapshotId, DataStoreRole.Image);
@@ -987,6 +990,12 @@ public void copyAsync(DataObject srcData, DataObject dstData, AsyncCompletionCal
987990
}
988991

989992
private StorPoolVolumeDef createVolumeWithTags(SnapshotInfo sinfo, String snapshotName, VolumeInfo vinfo, String volumeName, Long size, SpConnectionDesc conn) {
993+
Pair<String, String> templateAndTier = getTemplateAndTier(vinfo, conn);
994+
Map<String, String> tags = StorPoolHelper.addStorPoolTags(volumeName, getVMInstanceUUID(vinfo.getInstanceId()), "volume", getVcPolicyTag(vinfo.getInstanceId()), templateAndTier.first());
995+
return new StorPoolVolumeDef(null, size, tags, snapshotName, sinfo.getBaseVolume().getMaxIops(), templateAndTier.second(), null, null, null);
996+
}
997+
998+
private Pair<String, String> getTemplateAndTier(VolumeInfo vinfo, SpConnectionDesc conn) {
990999
String tier = null;
9911000
String template = null;
9921001
if (vinfo.getDiskOfferingId() != null) {
@@ -999,10 +1008,8 @@ private StorPoolVolumeDef createVolumeWithTags(SnapshotInfo sinfo, String snapsh
9991008
if (template == null) {
10001009
template = conn.getTemplateName();
10011010
}
1002-
Map<String, String> tags = StorPoolHelper.addStorPoolTags(volumeName, getVMInstanceUUID(vinfo.getInstanceId()), "volume", getVcPolicyTag(vinfo.getInstanceId()), tier);
1003-
return new StorPoolVolumeDef(null, size, tags, snapshotName, sinfo.getBaseVolume().getMaxIops(), template, null, null, null);
1011+
return new Pair<>(tier, template);
10041012
}
1005-
10061013
private Answer createVolumeSnapshot(StorageSubSystemCommand cmd, Long size, SpConnectionDesc conn,
10071014
String volName, TemplateObjectTO dstTO) {
10081015
Answer answer;
@@ -1323,24 +1330,33 @@ public void provideVmInfo(long vmId, long volumeId) {
13231330
return;
13241331
}
13251332
StoragePoolVO poolVO = primaryStoreDao.findById(volume.getPoolId());
1326-
if (poolVO != null) {
1327-
try {
1328-
SpConnectionDesc conn = StorPoolUtil.getSpConnection(poolVO.getUuid(), poolVO.getId(), storagePoolDetailsDao, primaryStoreDao);
1329-
String volName = StorPoolStorageAdaptor.getVolumeNameFromPath(volume.getPath(), true);
1330-
VMInstanceVO userVM = vmInstanceDao.findById(vmId);
1331-
Map<String, String> tags = StorPoolHelper.addStorPoolTags(null, userVM.getUuid(), null, getVcPolicyTag(vmId), null);
1332-
if (volume.getDeviceId() != null) {
1333-
tags.put("disk", volume.getDeviceId().toString());
1334-
}
1335-
StorPoolVolumeDef spVolume = new StorPoolVolumeDef(volName, null, tags, null, null, null, null, null, null);
1333+
if (poolVO != null && StoragePoolType.StorPool.equals(poolVO.getPoolType())) {
1334+
VolumeInfo vInfo = volumeDataFactory.getVolume(volumeId);
1335+
if (vInfo == null) {
1336+
StorPoolUtil.spLog("Could not find volume with volume ID [%s] to set tags", volumeId);
1337+
return;
1338+
}
1339+
updateVolumeWithTags(poolVO, vInfo);
1340+
}
1341+
}
13361342

1337-
SpApiResponse resp = StorPoolUtil.volumeUpdate(spVolume, conn);
1338-
if (resp.getError() != null) {
1339-
logger.warn(String.format("Could not update VC policy tags of a volume with id [%s]", volume.getUuid()));
1340-
}
1341-
} catch (Exception e) {
1342-
logger.warn(String.format("Could not update Virtual machine tags due to %s", e.getMessage()));
1343+
private void updateVolumeWithTags(StoragePoolVO poolVO, VolumeInfo vInfo) {
1344+
try {
1345+
SpConnectionDesc conn = StorPoolUtil.getSpConnection(poolVO.getUuid(), poolVO.getId(), storagePoolDetailsDao, primaryStoreDao);
1346+
String volName = StorPoolStorageAdaptor.getVolumeNameFromPath(vInfo.getPath(), true);
1347+
Pair<String, String> templateAndTier = getTemplateAndTier(vInfo, conn);
1348+
Map<String, String> tags = StorPoolHelper.addStorPoolTags(volName, getVMInstanceUUID(vInfo.getInstanceId()), "volume", getVcPolicyTag(vInfo.getInstanceId()), templateAndTier.first());
1349+
if (vInfo.getDeviceId() != null) {
1350+
tags.put("disk", vInfo.getDeviceId().toString());
13431351
}
1352+
StorPoolVolumeDef spVolume = new StorPoolVolumeDef(volName, null, tags, null, null, templateAndTier.second(), null, null, null);
1353+
StorPoolUtil.spLog("Updating volume's tags [%s] with template [%s]", tags, templateAndTier.second());
1354+
SpApiResponse resp = StorPoolUtil.volumeUpdate(spVolume, conn);
1355+
if (resp.getError() != null) {
1356+
logger.warn(String.format("Could not update VC policy tags of a volume with id [%s]", vInfo.getUuid()));
1357+
}
1358+
} catch (Exception e) {
1359+
logger.warn(String.format("Could not update Virtual machine tags due to %s", e.getMessage()));
13441360
}
13451361
}
13461362

‎test/integration/plugins/storpool/test_storpool_tiers.py‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,12 +171,15 @@ def setUpCloudStack(cls):
171171
cls.random_data = "random.data"
172172
cls.virtual_machine = VirtualMachine.create(
173173
cls.apiclient,
174-
cls.services["small"],
174+
{"name": "StorPool-%s" % uuid.uuid4()},
175+
zoneid=cls.zone.id,
176+
templateid=cls.template.id,
175177
accountid=cls.account.name,
176178
domainid=cls.account.domainid,
177-
templateid=cls.template.id,
178179
serviceofferingid=cls.service_offering.id,
179180
overridediskofferingid=cls.disk_offerings_tier1_tags.id,
181+
hypervisor=cls.hypervisor,
182+
rootdisksize=10
180183
)
181184

182185
volume = list_volumes(
@@ -222,6 +225,7 @@ def tearDown(self):
222225
def test_01_check_tags_on_deployed_vm_and_datadisk(self):
223226
virtual_machine_tier1_tag = self.deploy_vm_and_check_tier_tag()
224227
virtual_machine_tier1_tag.stop(self.apiclient, forced=True)
228+
virtual_machine_tier1_tag.delete(self.apiclient, expunge=True)
225229

226230
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
227231
def test_02_change_offering_on_attached_root_disk(self):
@@ -235,6 +239,7 @@ def test_02_change_offering_on_attached_root_disk(self):
235239
self.vc_policy_tags(volumes=root_volume, vm=virtual_machine_tier1_tag, qos_or_template=self.qos,
236240
disk_offering_id=self.disk_offerings_tier2_tags.id, attached=True)
237241
virtual_machine_tier1_tag.stop(self.apiclient, forced=True)
242+
virtual_machine_tier1_tag.delete(self.apiclient, expunge=True)
238243

239244
def test_03_change_offering_on_attached_data_disk(self):
240245
virtual_machine_tier1_tag = self.deploy_vm_and_check_tier_tag()
@@ -247,6 +252,7 @@ def test_03_change_offering_on_attached_data_disk(self):
247252
self.vc_policy_tags(volumes=root_volume, vm=virtual_machine_tier1_tag, qos_or_template=self.qos,
248253
disk_offering_id=self.disk_offerings_tier2_tags.id, attached=True)
249254
virtual_machine_tier1_tag.stop(self.apiclient, forced=True)
255+
virtual_machine_tier1_tag.delete(self.apiclient, expunge=True)
250256

251257
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
252258
def test_04_check_templates_on_deployed_vm_and_datadisk(self):
@@ -268,6 +274,7 @@ def test_04_check_templates_on_deployed_vm_and_datadisk(self):
268274
for v in volumes:
269275
self.check_storpool_template(v, self.disk_offerings_tier1_template.id, self.spTemplate)
270276
virtual_machine_template_tier1.stop(self.apiclient, forced=True)
277+
virtual_machine_template_tier1.delete(self.apiclient, expunge=True)
271278

272279
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
273280
def test_05_check_templates_on_deployed_vm_and_datadisk_tier2(self):
@@ -289,6 +296,7 @@ def test_05_check_templates_on_deployed_vm_and_datadisk_tier2(self):
289296
for v in volumes:
290297
self.check_storpool_template(v, self.disk_offerings_tier2_template.id, self.spTemplate)
291298
virtual_machine_template_tier2.stop(self.apiclient, forced=True)
299+
virtual_machine_template_tier2.delete(self.apiclient, expunge=True)
292300

293301
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
294302
def test_06_change_offerings_with_tags_detached_volume(self):
@@ -322,6 +330,7 @@ def test_06_change_offerings_with_tags_detached_volume(self):
322330
self.changeOfferingForVolume(volumes[0].id, self.disk_offerings_tier1_tags.id, volumes[0].size)
323331
self.vc_policy_tags(volumes=volumes, vm=virtual_machine_tier2_tag, qos_or_template=self.qos,
324332
disk_offering_id=self.disk_offerings_tier1_tags.id, attached=True)
333+
virtual_machine_tier2_tag.delete(self.apiclient, expunge=True)
325334

326335
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
327336
def test_07_change_offerings_with_template_detached_volume(self):
@@ -354,6 +363,7 @@ def test_07_change_offerings_with_template_detached_volume(self):
354363
self.changeOfferingForVolume(volumes[0].id, self.disk_offerings_tier1_template.id, volumes[0].size)
355364
self.check_storpool_template(volume=volumes[0], disk_offering_id=self.disk_offerings_tier1_template.id,
356365
qos_or_template=self.spTemplate)
366+
virtual_machine_tier2_template.delete(self.apiclient, expunge=True)
357367

358368
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
359369
def test_08_deploy_vm_with_tags_and_template_in_offerings(self):
@@ -392,6 +402,7 @@ def test_08_deploy_vm_with_tags_and_template_in_offerings(self):
392402
self.changeOfferingForVolume(volumes[0].id, self.disk_offerings_tier1_tags.id, volumes[0].size)
393403
self.vc_policy_tags(volumes=volumes, vm=virtual_machine_tier2_template, qos_or_template=self.qos,
394404
disk_offering_id=self.disk_offerings_tier1_tags.id, attached=True)
405+
virtual_machine_tier2_template.delete(self.apiclient, expunge=True)
395406

396407
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
397408
def test_09_resize_root_volume(self):
@@ -408,6 +419,7 @@ def test_09_resize_root_volume(self):
408419
self.vc_policy_tags(volumes=root_volume, vm=virtual_machine_tier1_tag, qos_or_template=self.qos,
409420
disk_offering_id=self.disk_offerings_tier2_tags.id, attached=True)
410421
virtual_machine_tier1_tag.stop(self.apiclient, forced=True)
422+
virtual_machine_tier1_tag.delete(self.apiclient, expunge=True)
411423

412424
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
413425
def test_10_shrink_root_volume(self):
@@ -425,6 +437,7 @@ def test_10_shrink_root_volume(self):
425437
listall=True)
426438
self.vc_policy_tags(volumes=root_volume, vm=virtual_machine_tier1_tag, qos_or_template=self.qos,
427439
disk_offering_id=self.disk_offerings_tier2_tags.id, attached=True)
440+
virtual_machine_tier1_tag.delete(self.apiclient, expunge=True)
428441

429442
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
430443
def test_11_resize_data_volume(self):
@@ -441,6 +454,7 @@ def test_11_resize_data_volume(self):
441454
self.vc_policy_tags(volumes=root_volume, vm=virtual_machine_tier1_tag, qos_or_template=self.qos,
442455
disk_offering_id=self.disk_offerings_tier2_tags.id, attached=True)
443456
virtual_machine_tier1_tag.stop(self.apiclient, forced=True)
457+
virtual_machine_tier1_tag.delete(self.apiclient, expunge=True)
444458

445459
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
446460
def test_12_shrink_data_volume(self):
@@ -458,6 +472,7 @@ def test_12_shrink_data_volume(self):
458472
self.vc_policy_tags(volumes=root_volume, vm=virtual_machine_tier1_tag, qos_or_template=self.qos,
459473
disk_offering_id=self.disk_offerings_tier2_tags.id, attached=True)
460474
virtual_machine_tier1_tag.stop(self.apiclient, forced=True)
475+
virtual_machine_tier1_tag.delete(self.apiclient, expunge=True)
461476

462477
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
463478
def test_13_deploy_vm_from_volume_check_tags(self):
@@ -466,6 +481,7 @@ def test_13_deploy_vm_from_volume_check_tags(self):
466481
listall=True)
467482
self.vc_policy_tags(volumes=root_volume, vm=vm, qos_or_template=self.qos,
468483
disk_offering_id=self.disk_offerings_tier1_tags.id, attached=True)
484+
vm.delete(self.apiclient, expunge=True)
469485

470486
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
471487
def test_14_deploy_vm_from_snapshot_check_tags(self):
@@ -474,6 +490,7 @@ def test_14_deploy_vm_from_snapshot_check_tags(self):
474490
listall=True)
475491
self.vc_policy_tags(volumes=root_volume, vm=vm, qos_or_template=self.qos,
476492
disk_offering_id=self.disk_offerings_tier1_tags.id, attached=True)
493+
vm.delete(self.apiclient, expunge=True)
477494

478495
def deploy_vm_and_check_tier_tag(self):
479496
virtual_machine_tier1_tag = VirtualMachine.create(
@@ -602,8 +619,11 @@ def deploy_vm_from_snapshot_or_template(self, snapshotid, is_snapshot=False):
602619
self.apiclient,
603620
snapshot_id=snapshotid,
604621
services=self.services,
605-
disk_offering=self.disk_offering.id,
622+
account=self.account.name,
623+
domainid=self.account.domainid,
624+
disk_offering=self.disk_offerings_tier1_tags.id,
606625
zoneid=self.zone.id,
626+
size=10
607627
)
608628
virtual_machine = VirtualMachine.create(self.apiclient,
609629
{"name": "StorPool-%s" % uuid.uuid4()},
@@ -617,4 +637,5 @@ def deploy_vm_from_snapshot_or_template(self, snapshotid, is_snapshot=False):
617637
ssh_client = virtual_machine.get_ssh_client()
618638
except Exception as e:
619639
self.fail("SSH failed for virtual machine: %s - %s" %
620-
(virtual_machine.ipaddress, e))
640+
(virtual_machine.ipaddress, e))
641+
return virtual_machine

‎test/integration/smoke/test_vm_lifecycle_with_snapshot_or_volume.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,8 @@ def create_volume_from_snapshot_deploy_vm(self, snapshotid):
298298
snapshot_id=snapshotid,
299299
services=self.services,
300300
disk_offering=self.disk_offering.id,
301+
account=self.account.name,
302+
domainid=self.account.domainid,
301303
zoneid=self.zone.id,
302304
)
303305
virtual_machine = VirtualMachine.create(self.apiclient,

‎tools/marvin/marvin/lib/base.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1222,7 +1222,7 @@ def create_custom_disk(cls, apiclient, services, account=None,
12221222

12231223
@classmethod
12241224
def create_from_snapshot(cls, apiclient, snapshot_id, services,
1225-
account=None, domainid=None, projectid=None, zoneid=None, disk_offering=None):
1225+
account=None, domainid=None, projectid=None, zoneid=None, disk_offering=None, size=None):
12261226
"""Create Volume from snapshot"""
12271227
cmd = createVolume.createVolumeCmd()
12281228
cmd.name = "-".join([services["diskname"], random_gen()])
@@ -1252,6 +1252,9 @@ def create_from_snapshot(cls, apiclient, snapshot_id, services,
12521252
if disk_offering:
12531253
cmd.diskofferingid = disk_offering
12541254

1255+
if size:
1256+
cmd.size = size
1257+
12551258
return Volume(apiclient.createVolume(cmd).__dict__)
12561259

12571260
@classmethod

0 commit comments

Comments
 (0)