Skip to content

Commit 1d1b332

Browse files
slavkapslavkap
authored
remove StorPool tags from detached volumes (#8377)
* remove tags from detached volumes * Adress comments * address comments * Address comments
1 parent 7394af3 commit 1d1b332

5 files changed

Lines changed: 86 additions & 22 deletions

File tree

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,15 @@ public boolean grantAccess(DataObject data, Host host, DataStore dataStore) {
190190

191191
@Override
192192
public void revokeAccess(DataObject data, Host host, DataStore dataStore) {
193+
if (DataObjectType.VOLUME == data.getType()) {
194+
final VolumeVO volume = volumeDao.findById(data.getId());
195+
if (volume.getInstanceId() == null) {
196+
StorPoolUtil.spLog("Removing tags from detached volume=%s", volume.toString());
197+
SpConnectionDesc conn = StorPoolUtil.getSpConnection(dataStore.getUuid(), dataStore.getId(), storagePoolDetailsDao, primaryStoreDao);
198+
StorPoolUtil.volumeRemoveTags(StorPoolStorageAdaptor.getVolumeNameFromPath(volume.getPath(), true), conn);
199+
}
200+
}
201+
193202
}
194203

195204
private void updateStoragePool(final long poolId, final long deltaUsedBytes) {
@@ -1038,7 +1047,7 @@ public void revertSnapshot(final SnapshotInfo snapshot, final SnapshotInfo snaps
10381047
}
10391048

10401049
if (vinfo.getMaxIops() != null) {
1041-
response = StorPoolUtil.volumeUpdateTags(volumeName, null, vinfo.getMaxIops(), conn, null);
1050+
response = StorPoolUtil.volumeUpdateIopsAndTags(volumeName, null, vinfo.getMaxIops(), conn, null);
10421051
if (response.getError() != null) {
10431052
StorPoolUtil.spLog("Volume was reverted successfully but max iops could not be set due to %s", response.getError().getDescr());
10441053
}
@@ -1127,13 +1136,16 @@ public boolean isVmInfoNeeded() {
11271136
@Override
11281137
public void provideVmInfo(long vmId, long volumeId) {
11291138
VolumeVO volume = volumeDao.findById(volumeId);
1139+
if (volume.getInstanceId() == null) {
1140+
return;
1141+
}
11301142
StoragePoolVO poolVO = primaryStoreDao.findById(volume.getPoolId());
11311143
if (poolVO != null) {
11321144
try {
11331145
SpConnectionDesc conn = StorPoolUtil.getSpConnection(poolVO.getUuid(), poolVO.getId(), storagePoolDetailsDao, primaryStoreDao);
11341146
String volName = StorPoolStorageAdaptor.getVolumeNameFromPath(volume.getPath(), true);
11351147
VMInstanceVO userVM = vmInstanceDao.findById(vmId);
1136-
SpApiResponse resp = StorPoolUtil.volumeUpdateTags(volName, volume.getInstanceId() != null ? userVM.getUuid() : "", null, conn, getVcPolicyTag(vmId));
1148+
SpApiResponse resp = StorPoolUtil.volumeUpdateIopsAndTags(volName, volume.getInstanceId() != null ? userVM.getUuid() : "", null, conn, getVcPolicyTag(vmId));
11371149
if (resp.getError() != null) {
11381150
log.warn(String.format("Could not update VC policy tags of a volume with id [%s]", volume.getUuid()));
11391151
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,14 @@ public static SpApiResponse volumeUpdate(final String name, final Long newSize,
533533
return POST("MultiCluster/VolumeUpdate/" + name, json, conn);
534534
}
535535

536-
public static SpApiResponse volumeUpdateTags(final String name, final String uuid, Long iops,
536+
public static SpApiResponse volumeRemoveTags(String name, SpConnectionDesc conn) {
537+
Map<String, Object> json = new HashMap<>();
538+
Map<String, String> tags = StorPoolHelper.addStorPoolTags(null, "", null, "");
539+
json.put("tags", tags);
540+
return POST("MultiCluster/VolumeUpdate/" + name, json, conn);
541+
}
542+
543+
public static SpApiResponse volumeUpdateIopsAndTags(final String name, final String uuid, Long iops,
537544
SpConnectionDesc conn, String vcPolicy) {
538545
Map<String, Object> json = new HashMap<>();
539546
Map<String, String> tags = StorPoolHelper.addStorPoolTags(null, uuid, null, vcPolicy);

‎plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolVMSnapshotStrategy.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) {
335335
}
336336
VolumeInfo vinfo = volFactory.getVolume(volumeObjectTO.getId());
337337
if (vinfo.getMaxIops() != null) {
338-
resp = StorPoolUtil.volumeUpdateTags(volumeName, null, vinfo.getMaxIops(), conn, null);
338+
resp = StorPoolUtil.volumeUpdateIopsAndTags(volumeName, null, vinfo.getMaxIops(), conn, null);
339339

340340
if (resp.getError() != null) {
341341
StorPoolUtil.spLog("Volume was reverted successfully but max iops could not be set due to %s",

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

Lines changed: 59 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,19 @@ def setUpCloudStack(cls):
218218
hypervisor=cls.hypervisor,
219219
rootdisksize=10
220220
)
221+
cls.virtual_machine3 = VirtualMachine.create(
222+
cls.apiclient,
223+
{"name":"StorPool-%s" % uuid.uuid4() },
224+
zoneid=cls.zone.id,
225+
templateid=template.id,
226+
accountid=cls.account.name,
227+
domainid=cls.account.domainid,
228+
serviceofferingid=cls.service_offering.id,
229+
hypervisor=cls.hypervisor,
230+
diskofferingid=cls.disk_offerings.id,
231+
size=2,
232+
rootdisksize=10
233+
)
221234
cls.template = template
222235
cls.random_data_0 = random_gen(size=100)
223236
cls.test_dir = "/tmp"
@@ -270,7 +283,7 @@ def test_01_set_vcpolicy_tag_to_vm_with_attached_disks(self):
270283
virtualmachineid = self.virtual_machine.id, listall=True
271284
)
272285

273-
self.vc_policy_tags(volumes, vm_tags, vm)
286+
self.vc_policy_tags(volumes, vm_tags, vm, True)
274287

275288

276289
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
@@ -310,7 +323,7 @@ def test_03_create_vm_snapshot_vc_policy_tag(self):
310323
vm = list_virtual_machines(self.apiclient,id = self.virtual_machine.id, listall=True)
311324
vm_tags = vm[0].tags
312325

313-
self.vc_policy_tags(volumes, vm_tags, vm)
326+
self.vc_policy_tags(volumes, vm_tags, vm, True)
314327

315328

316329
self.assertEqual(volume_attached.id, self.volume.id, "Is not the same volume ")
@@ -442,7 +455,7 @@ def test_04_revert_vm_snapshots_vc_policy_tag(self):
442455
vm = list_virtual_machines(self.apiclient,id = self.virtual_machine.id, listall=True)
443456
vm_tags = vm[0].tags
444457

445-
self.vc_policy_tags(volumes, vm_tags, vm)
458+
self.vc_policy_tags(volumes, vm_tags, vm, True)
446459

447460
self.assertEqual(
448461
self.random_data_0,
@@ -490,18 +503,17 @@ def test_05_delete_vm_snapshots(self):
490503
def test_06_remove_vcpolicy_tag_when_disk_detached(self):
491504
""" Test remove vc-policy tag to disk detached from VM"""
492505
time.sleep(60)
493-
volume_detached = self.virtual_machine.detach_volume(
494-
self.apiclient,
495-
self.volume_2
496-
)
497506
vm = list_virtual_machines(self.apiclient,id = self.virtual_machine.id, listall=True)
498507
vm_tags = vm[0].tags
499508
volumes = list_volumes(
500509
self.apiclient,
501-
virtualmachineid = self.virtual_machine.id, listall=True
510+
id= self.volume_2.id, listall=True,
502511
)
503-
504-
self.vc_policy_tags( volumes, vm_tags, vm)
512+
volume_detached = self.virtual_machine.detach_volume(
513+
self.apiclient,
514+
self.volume_2
515+
)
516+
self.vc_policy_tags( volumes, vm_tags, vm, False)
505517

506518
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
507519
def test_07_delete_vcpolicy_tag(self):
@@ -538,7 +550,7 @@ def test_08_vcpolicy_tag_to_reverted_disk(self):
538550
virtualmachineid = self.virtual_machine2.id, listall=True,
539551
type = "ROOT"
540552
)
541-
self.vc_policy_tags(volume, vm_tags, vm)
553+
self.vc_policy_tags(volume, vm_tags, vm, True)
542554

543555
snapshot = Snapshot.create(
544556
self.apiclient,
@@ -560,21 +572,52 @@ def test_08_vcpolicy_tag_to_reverted_disk(self):
560572
vm_tags = vm[0].tags
561573

562574
vol = list_volumes(self.apiclient, id = snapshot.volumeid, listall=True)
563-
self.vc_policy_tags(vol, vm_tags, vm)
575+
self.vc_policy_tags(vol, vm_tags, vm, True)
576+
577+
@attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
578+
def test_09_remove_vm_tags_on_datadisks_attached_to_destroyed_vm(self):
579+
tag = Tag.create(
580+
self.apiclient,
581+
resourceIds=self.virtual_machine3.id,
582+
resourceType='UserVm',
583+
tags={'vc-policy': 'testing_vc-policy'}
584+
)
585+
vm = list_virtual_machines(self.apiclient,id = self.virtual_machine3.id, listall=True)
586+
vm_tags = vm[0].tags
587+
volumes = list_volumes(
588+
self.apiclient,
589+
virtualmachineid = self.virtual_machine3.id, listall=True
590+
)
564591

592+
self.vc_policy_tags(volumes, vm_tags, vm, True)
565593

566-
def vc_policy_tags(self, volumes, vm_tags, vm):
567-
flag = False
594+
volumes = list_volumes(
595+
self.apiclient,
596+
virtualmachineid = self.virtual_machine3.id, listall=True, type="DATADISK"
597+
)
598+
self.virtual_machine3.delete(self.apiclient, expunge=True)
599+
600+
self.vc_policy_tags(volumes, vm_tags, vm, False)
601+
602+
def vc_policy_tags(self, volumes, vm_tags, vm, should_tags_exists=None):
603+
vcPolicyTag = False
604+
cvmTag = False
568605
for v in volumes:
569606
name = v.path.split("/")[3]
570607
spvolume = self.spapi.volumeList(volumeName="~" + name)
571608
tags = spvolume[0].tags
572609
for t in tags:
573610
for vm_tag in vm_tags:
574611
if t == vm_tag.key:
575-
flag = True
612+
vcPolicyTag = True
576613
self.assertEqual(tags[t], vm_tag.value, "Tags are not equal")
577614
if t == 'cvm':
615+
cvmTag = True
578616
self.assertEqual(tags[t], vm[0].id, "CVM tag is not the same as vm UUID")
579617
#self.assertEqual(tag.tags., second, msg)
580-
self.assertTrue(flag, "There aren't volumes with vm tags")
618+
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")
621+
else:
622+
self.assertFalse(vcPolicyTag, "The tags should be removed")
623+
self.assertFalse(cvmTag, "The tags should be removed")

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,7 @@ def create(cls, apiclient, services, templateid=None, accountid=None,
527527
customcpuspeed=None, custommemory=None, rootdisksize=None,
528528
rootdiskcontroller=None, vpcid=None, macaddress=None, datadisktemplate_diskoffering_list={},
529529
properties=None, nicnetworklist=None, bootmode=None, boottype=None, dynamicscalingenabled=None,
530-
userdataid=None, userdatadetails=None, extraconfig=None):
530+
userdataid=None, userdatadetails=None, extraconfig=None, size=None):
531531
"""Create the instance"""
532532

533533
cmd = deployVirtualMachine.deployVirtualMachineCmd()
@@ -649,7 +649,9 @@ def create(cls, apiclient, services, templateid=None, accountid=None,
649649
if rootdiskcontroller:
650650
cmd.details[0]["rootDiskController"] = rootdiskcontroller
651651

652-
if "size" in services:
652+
if size:
653+
cmd.size = size
654+
elif "size" in services:
653655
cmd.size = services["size"]
654656

655657
if group:

0 commit comments

Comments
 (0)