diff --git a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java index 6ca67cb5923..d8aa9d48e7d 100644 --- a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java +++ b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java @@ -222,7 +222,6 @@ public class StorPoolPrimaryDataStoreDriver implements PrimaryDataStoreDriver { StorPoolUtil.volumeRemoveTags(StorPoolStorageAdaptor.getVolumeNameFromPath(volume.getPath(), true), conn); } } - } private void updateStoragePool(final long poolId, final long deltaUsedBytes) { @@ -327,19 +326,14 @@ public class StorPoolPrimaryDataStoreDriver implements PrimaryDataStoreDriver { Long vmId, SpConnectionDesc conn) { SpApiResponse resp = new SpApiResponse(); Map tags = StorPoolHelper.addStorPoolTags(name, getVMInstanceUUID(vmId), "volume", getVcPolicyTag(vmId), tier); - if (tier != null || template != null) { - StorPoolUtil.spLog( - "Creating volume [%s] with template [%s] or tier tags [%s] described in disk/service offerings details", - vinfo.getUuid(), template, tier); - resp = StorPoolUtil.volumeCreate(size, null, template, tags, conn); - } else { - StorPoolUtil.spLog( - "StorpoolPrimaryDataStoreDriver.createAsync volume: name=%s, uuid=%s, isAttached=%s vm=%s, payload=%s, template: %s", - vinfo.getName(), vinfo.getUuid(), vinfo.isAttachedVM(), vinfo.getAttachedVmName(), - vinfo.getpayload(), conn.getTemplateName()); - resp = StorPoolUtil.volumeCreate(name, null, size, getVMInstanceUUID(vinfo.getInstanceId()), null, - "volume", vinfo.getMaxIops(), conn); + if (vinfo.getDeviceId() != null) { + tags.put("disk", vinfo.getDeviceId().toString()); } + if (template == null) { + template = conn.getTemplateName(); + } + StorPoolVolumeDef volume = new StorPoolVolumeDef(null, size, tags, null, vinfo.getMaxIops(), template, null, null, null); + resp = StorPoolUtil.volumeCreate(volume, conn); return resp; } @@ -827,20 +821,24 @@ public class StorPoolPrimaryDataStoreDriver implements PrimaryDataStoreDriver { if (tier == null) { template = getTemplateFromOfferingDetail(vinfo.getDiskOfferingId()); } - } - - if (tier != null || template != null) { - Map tags = StorPoolHelper.addStorPoolTags(name, getVMInstanceUUID(vmId), "volume", getVcPolicyTag(vmId), tier); - StorPoolUtil.spLog( "Creating volume [%s] with template [%s] or tier tags [%s] described in disk/service offerings details", vinfo.getUuid(), template, tier); - resp = StorPoolUtil.volumeCreate(size, parentName, template, tags, conn); - } else { - resp = StorPoolUtil.volumeCreate(name, parentName, size, getVMInstanceUUID(vmId), - getVcPolicyTag(vmId), "volume", vinfo.getMaxIops(), conn); } + Map tags = StorPoolHelper.addStorPoolTags(name, getVMInstanceUUID(vmId), "volume", getVcPolicyTag(vmId), tier); + + if (vinfo.getDeviceId() != null) { + tags.put("disk", vinfo.getDeviceId().toString()); + } + + if (template == null) { + template = conn.getTemplateName(); + } + + StorPoolVolumeDef volumeDef = new StorPoolVolumeDef(null, size, tags, parentName, null, template, null, null, null); + resp = StorPoolUtil.volumeCreate(volumeDef, conn); + if (resp.getError() == null) { updateStoragePool(dstData.getDataStore().getId(), vinfo.getSize()); updateVolumePoolType(vinfo); @@ -1309,7 +1307,13 @@ public class StorPoolPrimaryDataStoreDriver implements PrimaryDataStoreDriver { SpConnectionDesc conn = StorPoolUtil.getSpConnection(poolVO.getUuid(), poolVO.getId(), storagePoolDetailsDao, primaryStoreDao); String volName = StorPoolStorageAdaptor.getVolumeNameFromPath(volume.getPath(), true); VMInstanceVO userVM = vmInstanceDao.findById(vmId); - SpApiResponse resp = StorPoolUtil.volumeUpdateIopsAndTags(volName, volume.getInstanceId() != null ? userVM.getUuid() : "", null, conn, getVcPolicyTag(vmId)); + Map tags = StorPoolHelper.addStorPoolTags(null, userVM.getUuid(), null, getVcPolicyTag(vmId), null); + if (volume.getDeviceId() != null) { + tags.put("disk", volume.getDeviceId().toString()); + } + StorPoolVolumeDef spVolume = new StorPoolVolumeDef(volName, null, tags, null, null, null, null, null, null); + + SpApiResponse resp = StorPoolUtil.volumeUpdate(spVolume, conn); if (resp.getError() != null) { logger.warn(String.format("Could not update VC policy tags of a volume with id [%s]", volume.getUuid())); } diff --git a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolUtil.java b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolUtil.java index fd62157f136..fa9248033bf 100644 --- a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolUtil.java +++ b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolUtil.java @@ -520,6 +520,10 @@ public class StorPoolUtil { return POST("MultiCluster/VolumeCreate", json, conn); } + public static SpApiResponse volumeCreate(StorPoolVolumeDef volume, SpConnectionDesc conn) { + return POST("MultiCluster/VolumeCreate", volume, conn); + } + public static SpApiResponse volumeCreate(SpConnectionDesc conn) { Map json = new LinkedHashMap<>(); json.put("name", ""); @@ -568,6 +572,7 @@ public class StorPoolUtil { public static SpApiResponse volumeRemoveTags(String name, SpConnectionDesc conn) { Map json = new HashMap<>(); Map tags = StorPoolHelper.addStorPoolTags(null, "", null, "", null); + tags.put("disk", ""); json.put("tags", tags); return POST("MultiCluster/VolumeUpdate/" + name, json, conn); } diff --git a/test/integration/plugins/storpool/TestTagsOnStorPool.py b/test/integration/plugins/storpool/TestTagsOnStorPool.py index a66fb3570f4..ea5c2a4cc78 100644 --- a/test/integration/plugins/storpool/TestTagsOnStorPool.py +++ b/test/integration/plugins/storpool/TestTagsOnStorPool.py @@ -283,7 +283,7 @@ class TestStoragePool(cloudstackTestCase): virtualmachineid = self.virtual_machine.id, listall=True ) - self.vc_policy_tags(volumes, vm_tags, vm, True) + self.vc_policy_tags(volumes, vm_tags, vm, should_tags_exists=True) @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") @@ -323,7 +323,7 @@ class TestStoragePool(cloudstackTestCase): vm = list_virtual_machines(self.apiclient,id = self.virtual_machine.id, listall=True) vm_tags = vm[0].tags - self.vc_policy_tags(volumes, vm_tags, vm, True) + self.vc_policy_tags(volumes, vm_tags, vm, should_tags_exists=True) self.assertEqual(volume_attached.id, self.volume.id, "Is not the same volume ") @@ -455,7 +455,7 @@ class TestStoragePool(cloudstackTestCase): vm = list_virtual_machines(self.apiclient,id = self.virtual_machine.id, listall=True) vm_tags = vm[0].tags - self.vc_policy_tags(volumes, vm_tags, vm, True) + self.vc_policy_tags(volumes, vm_tags, vm, should_tags_exists=True) self.assertEqual( self.random_data_0, @@ -491,14 +491,12 @@ class TestStoragePool(cloudstackTestCase): list_snapshot_response = VmSnapshot.list( self.apiclient, - #vmid=self.virtual_machine.id, virtualmachineid=self.virtual_machine.id, listall=False) self.debug('list_snapshot_response -------------------- %s' % list_snapshot_response) self.assertIsNone(list_snapshot_response, "snapshot is already deleted") - @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") def test_06_remove_vcpolicy_tag_when_disk_detached(self): """ Test remove vc-policy tag to disk detached from VM""" @@ -513,7 +511,7 @@ class TestStoragePool(cloudstackTestCase): self.apiclient, self.volume_2 ) - self.vc_policy_tags( volumes, vm_tags, vm, False) + self.vc_policy_tags( volumes, vm_tags, vm, should_tags_exists=False) @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") def test_07_delete_vcpolicy_tag(self): @@ -550,7 +548,7 @@ class TestStoragePool(cloudstackTestCase): virtualmachineid = self.virtual_machine2.id, listall=True, type = "ROOT" ) - self.vc_policy_tags(volume, vm_tags, vm, True) + self.vc_policy_tags(volume, vm_tags, vm, should_tags_exists=True) snapshot = Snapshot.create( self.apiclient, @@ -571,8 +569,8 @@ class TestStoragePool(cloudstackTestCase): vm = list_virtual_machines(self.apiclient,id = self.virtual_machine2.id) vm_tags = vm[0].tags - vol = list_volumes(self.apiclient, id = snapshot.volumeid, listall=True) - self.vc_policy_tags(vol, vm_tags, vm, True) + vol = list_volumes(self.apiclient, id=snapshot.volumeid, listall=True) + self.vc_policy_tags(vol, vm_tags, vm, should_tags_exists=True) @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") def test_09_remove_vm_tags_on_datadisks_attached_to_destroyed_vm(self): @@ -586,38 +584,82 @@ class TestStoragePool(cloudstackTestCase): vm_tags = vm[0].tags volumes = list_volumes( self.apiclient, - virtualmachineid = self.virtual_machine3.id, listall=True + virtualmachineid=self.virtual_machine3.id, listall=True ) - self.vc_policy_tags(volumes, vm_tags, vm, True) + self.vc_policy_tags(volumes, vm_tags, vm, should_tags_exists=True) volumes = list_volumes( self.apiclient, - virtualmachineid = self.virtual_machine3.id, listall=True, type="DATADISK" + virtualmachineid=self.virtual_machine3.id, listall=True, type="DATADISK" ) self.virtual_machine3.delete(self.apiclient, expunge=True) - self.vc_policy_tags(volumes, vm_tags, vm, False) + self.vc_policy_tags(volumes, vm_tags, vm, should_tags_exists=False) - def vc_policy_tags(self, volumes, vm_tags, vm, should_tags_exists=None): - vcPolicyTag = False - cvmTag = False + @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") + def test_10_check_tags_on_deployed_vm_with_data_disk(self): + """ + Check disk and cvm tags are set on all volumes when VM is deployed with additional DATA disk + Detach the DATA disk + """ + vm = VirtualMachine.create( + self.apiclient, + {"name":"StorPool-%s" % uuid.uuid4() }, + zoneid=self.zone.id, + templateid=self.template.id, + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.service_offering.id, + hypervisor=self.hypervisor, + diskofferingid=self.disk_offerings.id, + size=2, + rootdisksize=10 + ) + volumes = list_volumes( + self.apiclient, + virtualmachineid=vm.id, listall=True + ) + vm1 = list_virtual_machines(self.apiclient,id=vm.id, listall=True) + vm_tags = vm1[0].tags + self.vc_policy_tags(volumes, vm_tags, vm1, False, True) + vm.stop(self.apiclient, forced=True) + volumes = list_volumes( + self.apiclient, + virtualmachineid=vm.id, listall=True, type="DATADISK" + ) + + self.debug("detaching volume %s" % volumes) + VirtualMachine.detach_volume(vm, self.apiclient, volumes[0]) + self.vc_policy_tags(volumes, vm_tags, vm1, False, False) + + def vc_policy_tags(self, volumes, vm_tags, vm, tag_check=True, should_tags_exists=None,): + vc_policy_tag = False + cvm_tag = False + disk_id_tag = False for v in volumes: name = v.path.split("/")[3] - spvolume = self.spapi.volumeList(volumeName="~" + name) - tags = spvolume[0].tags + volume = self.spapi.volumeList(volumeName="~" + name) + tags = volume[0].tags + self.debug("Tags %s" % tags) for t in tags: for vm_tag in vm_tags: if t == vm_tag.key: - vcPolicyTag = True + vc_policy_tag = True self.assertEqual(tags[t], vm_tag.value, "Tags are not equal") - if t == 'cvm': - cvmTag = True - self.assertEqual(tags[t], vm[0].id, "CVM tag is not the same as vm UUID") - #self.assertEqual(tag.tags., second, msg) + if t == 'cvm': + cvm_tag = True + self.assertEqual(tags[t], vm[0].id, "CVM tag is not the same as vm UUID") + if t == 'disk': + disk_id_tag = True + self.assertEqual(tags[t], str(v.deviceid), "Disk tag is not equal to the device ID") if should_tags_exists: - self.assertTrue(vcPolicyTag, "There aren't volumes with vm tags") - self.assertTrue(cvmTag, "There aren't volumes with vm tags") + if tag_check: + self.assertTrue(vc_policy_tag, "There aren't volumes with vc policy tags") + self.assertTrue(cvm_tag, "There aren't volumes with vm UUID tags") + self.assertTrue(disk_id_tag, "There aren't volumes with vm disk tag") else: - self.assertFalse(vcPolicyTag, "The tags should be removed") - self.assertFalse(cvmTag, "The tags should be removed") + if tag_check: + self.assertFalse(vc_policy_tag, "The vc policy tag should be removed") + self.assertFalse(cvm_tag, "The cvm tag should be removed") + self.assertFalse(disk_id_tag, "The disk tag should be removed")