Skip to content

Commit 5c0346e

Browse files
slavkapslavkap
authored
Adding device ID to a StorPool volume (#10587)
1 parent a137283 commit 5c0346e

File tree

3 files changed

+101
-50
lines changed

3 files changed

+101
-50
lines changed

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

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,6 @@ public void revokeAccess(DataObject data, Host host, DataStore dataStore) {
222222
StorPoolUtil.volumeRemoveTags(StorPoolStorageAdaptor.getVolumeNameFromPath(volume.getPath(), true), conn);
223223
}
224224
}
225-
226225
}
227226

228227
private void updateStoragePool(final long poolId, final long deltaUsedBytes) {
@@ -327,19 +326,14 @@ private SpApiResponse createStorPoolVolume(String template, String tier, VolumeI
327326
Long vmId, SpConnectionDesc conn) {
328327
SpApiResponse resp = new SpApiResponse();
329328
Map<String, String> tags = StorPoolHelper.addStorPoolTags(name, getVMInstanceUUID(vmId), "volume", getVcPolicyTag(vmId), tier);
330-
if (tier != null || template != null) {
331-
StorPoolUtil.spLog(
332-
"Creating volume [%s] with template [%s] or tier tags [%s] described in disk/service offerings details",
333-
vinfo.getUuid(), template, tier);
334-
resp = StorPoolUtil.volumeCreate(size, null, template, tags, conn);
335-
} else {
336-
StorPoolUtil.spLog(
337-
"StorpoolPrimaryDataStoreDriver.createAsync volume: name=%s, uuid=%s, isAttached=%s vm=%s, payload=%s, template: %s",
338-
vinfo.getName(), vinfo.getUuid(), vinfo.isAttachedVM(), vinfo.getAttachedVmName(),
339-
vinfo.getpayload(), conn.getTemplateName());
340-
resp = StorPoolUtil.volumeCreate(name, null, size, getVMInstanceUUID(vinfo.getInstanceId()), null,
341-
"volume", vinfo.getMaxIops(), conn);
329+
if (vinfo.getDeviceId() != null) {
330+
tags.put("disk", vinfo.getDeviceId().toString());
331+
}
332+
if (template == null) {
333+
template = conn.getTemplateName();
342334
}
335+
StorPoolVolumeDef volume = new StorPoolVolumeDef(null, size, tags, null, vinfo.getMaxIops(), template, null, null, null);
336+
resp = StorPoolUtil.volumeCreate(volume, conn);
343337
return resp;
344338
}
345339

@@ -827,20 +821,24 @@ public void copyAsync(DataObject srcData, DataObject dstData, AsyncCompletionCal
827821
if (tier == null) {
828822
template = getTemplateFromOfferingDetail(vinfo.getDiskOfferingId());
829823
}
830-
}
831-
832-
if (tier != null || template != null) {
833-
Map<String, String> tags = StorPoolHelper.addStorPoolTags(name, getVMInstanceUUID(vmId), "volume", getVcPolicyTag(vmId), tier);
834-
835824
StorPoolUtil.spLog(
836825
"Creating volume [%s] with template [%s] or tier tags [%s] described in disk/service offerings details",
837826
vinfo.getUuid(), template, tier);
838-
resp = StorPoolUtil.volumeCreate(size, parentName, template, tags, conn);
839-
} else {
840-
resp = StorPoolUtil.volumeCreate(name, parentName, size, getVMInstanceUUID(vmId),
841-
getVcPolicyTag(vmId), "volume", vinfo.getMaxIops(), conn);
842827
}
843828

829+
Map<String, String> tags = StorPoolHelper.addStorPoolTags(name, getVMInstanceUUID(vmId), "volume", getVcPolicyTag(vmId), tier);
830+
831+
if (vinfo.getDeviceId() != null) {
832+
tags.put("disk", vinfo.getDeviceId().toString());
833+
}
834+
835+
if (template == null) {
836+
template = conn.getTemplateName();
837+
}
838+
839+
StorPoolVolumeDef volumeDef = new StorPoolVolumeDef(null, size, tags, parentName, null, template, null, null, null);
840+
resp = StorPoolUtil.volumeCreate(volumeDef, conn);
841+
844842
if (resp.getError() == null) {
845843
updateStoragePool(dstData.getDataStore().getId(), vinfo.getSize());
846844
updateVolumePoolType(vinfo);
@@ -1309,7 +1307,13 @@ public void provideVmInfo(long vmId, long volumeId) {
13091307
SpConnectionDesc conn = StorPoolUtil.getSpConnection(poolVO.getUuid(), poolVO.getId(), storagePoolDetailsDao, primaryStoreDao);
13101308
String volName = StorPoolStorageAdaptor.getVolumeNameFromPath(volume.getPath(), true);
13111309
VMInstanceVO userVM = vmInstanceDao.findById(vmId);
1312-
SpApiResponse resp = StorPoolUtil.volumeUpdateIopsAndTags(volName, volume.getInstanceId() != null ? userVM.getUuid() : "", null, conn, getVcPolicyTag(vmId));
1310+
Map<String, String> tags = StorPoolHelper.addStorPoolTags(null, userVM.getUuid(), null, getVcPolicyTag(vmId), null);
1311+
if (volume.getDeviceId() != null) {
1312+
tags.put("disk", volume.getDeviceId().toString());
1313+
}
1314+
StorPoolVolumeDef spVolume = new StorPoolVolumeDef(volName, null, tags, null, null, null, null, null, null);
1315+
1316+
SpApiResponse resp = StorPoolUtil.volumeUpdate(spVolume, conn);
13131317
if (resp.getError() != null) {
13141318
logger.warn(String.format("Could not update VC policy tags of a volume with id [%s]", volume.getUuid()));
13151319
}

‎plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolUtil.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,10 @@ public static SpApiResponse volumeCreate(Long size, String parentName, String te
520520
return POST("MultiCluster/VolumeCreate", json, conn);
521521
}
522522

523+
public static SpApiResponse volumeCreate(StorPoolVolumeDef volume, SpConnectionDesc conn) {
524+
return POST("MultiCluster/VolumeCreate", volume, conn);
525+
}
526+
523527
public static SpApiResponse volumeCreate(SpConnectionDesc conn) {
524528
Map<String, Object> json = new LinkedHashMap<>();
525529
json.put("name", "");
@@ -568,6 +572,7 @@ public static SpApiResponse volumeUpdate(final String name, final Long newSize,
568572
public static SpApiResponse volumeRemoveTags(String name, SpConnectionDesc conn) {
569573
Map<String, Object> json = new HashMap<>();
570574
Map<String, String> tags = StorPoolHelper.addStorPoolTags(null, "", null, "", null);
575+
tags.put("disk", "");
571576
json.put("tags", tags);
572577
return POST("MultiCluster/VolumeUpdate/" + name, json, conn);
573578
}

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

Lines changed: 69 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ def test_01_set_vcpolicy_tag_to_vm_with_attached_disks(self):
283283
virtualmachineid = self.virtual_machine.id, listall=True
284284
)
285285

286-
self.vc_policy_tags(volumes, vm_tags, vm, True)
286+
self.vc_policy_tags(volumes, vm_tags, vm, should_tags_exists=True)
287287

288288

289289
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
@@ -323,7 +323,7 @@ def test_03_create_vm_snapshot_vc_policy_tag(self):
323323
vm = list_virtual_machines(self.apiclient,id = self.virtual_machine.id, listall=True)
324324
vm_tags = vm[0].tags
325325

326-
self.vc_policy_tags(volumes, vm_tags, vm, True)
326+
self.vc_policy_tags(volumes, vm_tags, vm, should_tags_exists=True)
327327

328328

329329
self.assertEqual(volume_attached.id, self.volume.id, "Is not the same volume ")
@@ -455,7 +455,7 @@ def test_04_revert_vm_snapshots_vc_policy_tag(self):
455455
vm = list_virtual_machines(self.apiclient,id = self.virtual_machine.id, listall=True)
456456
vm_tags = vm[0].tags
457457

458-
self.vc_policy_tags(volumes, vm_tags, vm, True)
458+
self.vc_policy_tags(volumes, vm_tags, vm, should_tags_exists=True)
459459

460460
self.assertEqual(
461461
self.random_data_0,
@@ -491,14 +491,12 @@ def test_05_delete_vm_snapshots(self):
491491

492492
list_snapshot_response = VmSnapshot.list(
493493
self.apiclient,
494-
#vmid=self.virtual_machine.id,
495494
virtualmachineid=self.virtual_machine.id,
496495
listall=False)
497496
self.debug('list_snapshot_response -------------------- %s' % list_snapshot_response)
498497

499498
self.assertIsNone(list_snapshot_response, "snapshot is already deleted")
500499

501-
502500
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
503501
def test_06_remove_vcpolicy_tag_when_disk_detached(self):
504502
""" Test remove vc-policy tag to disk detached from VM"""
@@ -513,7 +511,7 @@ def test_06_remove_vcpolicy_tag_when_disk_detached(self):
513511
self.apiclient,
514512
self.volume_2
515513
)
516-
self.vc_policy_tags( volumes, vm_tags, vm, False)
514+
self.vc_policy_tags( volumes, vm_tags, vm, should_tags_exists=False)
517515

518516
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
519517
def test_07_delete_vcpolicy_tag(self):
@@ -550,7 +548,7 @@ def test_08_vcpolicy_tag_to_reverted_disk(self):
550548
virtualmachineid = self.virtual_machine2.id, listall=True,
551549
type = "ROOT"
552550
)
553-
self.vc_policy_tags(volume, vm_tags, vm, True)
551+
self.vc_policy_tags(volume, vm_tags, vm, should_tags_exists=True)
554552

555553
snapshot = Snapshot.create(
556554
self.apiclient,
@@ -571,8 +569,8 @@ def test_08_vcpolicy_tag_to_reverted_disk(self):
571569
vm = list_virtual_machines(self.apiclient,id = self.virtual_machine2.id)
572570
vm_tags = vm[0].tags
573571

574-
vol = list_volumes(self.apiclient, id = snapshot.volumeid, listall=True)
575-
self.vc_policy_tags(vol, vm_tags, vm, True)
572+
vol = list_volumes(self.apiclient, id=snapshot.volumeid, listall=True)
573+
self.vc_policy_tags(vol, vm_tags, vm, should_tags_exists=True)
576574

577575
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
578576
def test_09_remove_vm_tags_on_datadisks_attached_to_destroyed_vm(self):
@@ -586,38 +584,82 @@ def test_09_remove_vm_tags_on_datadisks_attached_to_destroyed_vm(self):
586584
vm_tags = vm[0].tags
587585
volumes = list_volumes(
588586
self.apiclient,
589-
virtualmachineid = self.virtual_machine3.id, listall=True
587+
virtualmachineid=self.virtual_machine3.id, listall=True
590588
)
591589

592-
self.vc_policy_tags(volumes, vm_tags, vm, True)
590+
self.vc_policy_tags(volumes, vm_tags, vm, should_tags_exists=True)
593591

594592
volumes = list_volumes(
595593
self.apiclient,
596-
virtualmachineid = self.virtual_machine3.id, listall=True, type="DATADISK"
594+
virtualmachineid=self.virtual_machine3.id, listall=True, type="DATADISK"
597595
)
598596
self.virtual_machine3.delete(self.apiclient, expunge=True)
599597

600-
self.vc_policy_tags(volumes, vm_tags, vm, False)
598+
self.vc_policy_tags(volumes, vm_tags, vm, should_tags_exists=False)
601599

602-
def vc_policy_tags(self, volumes, vm_tags, vm, should_tags_exists=None):
603-
vcPolicyTag = False
604-
cvmTag = False
600+
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
601+
def test_10_check_tags_on_deployed_vm_with_data_disk(self):
602+
"""
603+
Check disk and cvm tags are set on all volumes when VM is deployed with additional DATA disk
604+
Detach the DATA disk
605+
"""
606+
vm = VirtualMachine.create(
607+
self.apiclient,
608+
{"name":"StorPool-%s" % uuid.uuid4() },
609+
zoneid=self.zone.id,
610+
templateid=self.template.id,
611+
accountid=self.account.name,
612+
domainid=self.account.domainid,
613+
serviceofferingid=self.service_offering.id,
614+
hypervisor=self.hypervisor,
615+
diskofferingid=self.disk_offerings.id,
616+
size=2,
617+
rootdisksize=10
618+
)
619+
volumes = list_volumes(
620+
self.apiclient,
621+
virtualmachineid=vm.id, listall=True
622+
)
623+
vm1 = list_virtual_machines(self.apiclient,id=vm.id, listall=True)
624+
vm_tags = vm1[0].tags
625+
self.vc_policy_tags(volumes, vm_tags, vm1, False, True)
626+
vm.stop(self.apiclient, forced=True)
627+
volumes = list_volumes(
628+
self.apiclient,
629+
virtualmachineid=vm.id, listall=True, type="DATADISK"
630+
)
631+
632+
self.debug("detaching volume %s" % volumes)
633+
VirtualMachine.detach_volume(vm, self.apiclient, volumes[0])
634+
self.vc_policy_tags(volumes, vm_tags, vm1, False, False)
635+
636+
def vc_policy_tags(self, volumes, vm_tags, vm, tag_check=True, should_tags_exists=None,):
637+
vc_policy_tag = False
638+
cvm_tag = False
639+
disk_id_tag = False
605640
for v in volumes:
606641
name = v.path.split("/")[3]
607-
spvolume = self.spapi.volumeList(volumeName="~" + name)
608-
tags = spvolume[0].tags
642+
volume = self.spapi.volumeList(volumeName="~" + name)
643+
tags = volume[0].tags
644+
self.debug("Tags %s" % tags)
609645
for t in tags:
610646
for vm_tag in vm_tags:
611647
if t == vm_tag.key:
612-
vcPolicyTag = True
648+
vc_policy_tag = True
613649
self.assertEqual(tags[t], vm_tag.value, "Tags are not equal")
614-
if t == 'cvm':
615-
cvmTag = True
616-
self.assertEqual(tags[t], vm[0].id, "CVM tag is not the same as vm UUID")
617-
#self.assertEqual(tag.tags., second, msg)
650+
if t == 'cvm':
651+
cvm_tag = True
652+
self.assertEqual(tags[t], vm[0].id, "CVM tag is not the same as vm UUID")
653+
if t == 'disk':
654+
disk_id_tag = True
655+
self.assertEqual(tags[t], str(v.deviceid), "Disk tag is not equal to the device ID")
618656
if should_tags_exists:
619-
self.assertTrue(vcPolicyTag, "There aren't volumes with vm tags")
620-
self.assertTrue(cvmTag, "There aren't volumes with vm tags")
657+
if tag_check:
658+
self.assertTrue(vc_policy_tag, "There aren't volumes with vc policy tags")
659+
self.assertTrue(cvm_tag, "There aren't volumes with vm UUID tags")
660+
self.assertTrue(disk_id_tag, "There aren't volumes with vm disk tag")
621661
else:
622-
self.assertFalse(vcPolicyTag, "The tags should be removed")
623-
self.assertFalse(cvmTag, "The tags should be removed")
662+
if tag_check:
663+
self.assertFalse(vc_policy_tag, "The vc policy tag should be removed")
664+
self.assertFalse(cvm_tag, "The cvm tag should be removed")
665+
self.assertFalse(disk_id_tag, "The disk tag should be removed")

0 commit comments

Comments
 (0)