diff --git a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java index 7739be83845..b54713ede5f 100644 --- a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java +++ b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java @@ -276,27 +276,35 @@ public class LinstorStorageAdaptor implements StorageAdaptor { } final DevelopersApi api = getLinstorAPI(pool); + String rscName; try { - final String rscName = getLinstorRscName(volumePath); + rscName = getLinstorRscName(volumePath); ResourceMakeAvailable rma = new ResourceMakeAvailable(); ApiCallRcList answers = api.resourceMakeAvailableOnNode(rscName, localNodeName, rma); checkLinstorAnswersThrow(answers); + } catch (ApiException apiEx) { + s_logger.error(apiEx); + throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx); + } + + try + { // allow 2 primaries for live migration, should be removed by disconnect on the other end ResourceDefinitionModify rdm = new ResourceDefinitionModify(); Properties props = new Properties(); props.put("DrbdOptions/Net/allow-two-primaries", "yes"); rdm.setOverrideProps(props); - answers = api.resourceDefinitionModify(rscName, rdm); + ApiCallRcList answers = api.resourceDefinitionModify(rscName, rdm); if (answers.hasError()) { logger.error("Unable to set 'allow-two-primaries' on " + rscName); - throw new CloudRuntimeException(answers.get(0).getMessage()); + // do not fail here as adding allow-two-primaries property is only a problem while live migrating } } catch (ApiException apiEx) { logger.error(apiEx); - throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx); + // do not fail here as adding allow-two-primaries property is only a problem while live migrating } return true; } @@ -358,21 +366,22 @@ public class LinstorStorageAdaptor implements StorageAdaptor { ResourceDefinitionModify rdm = new ResourceDefinitionModify(); rdm.deleteProps(Collections.singletonList("DrbdOptions/Net/allow-two-primaries")); ApiCallRcList answers = api.resourceDefinitionModify(rsc.get().getName(), rdm); - if (answers.hasError()) - { - logger.error("Failed to remove 'allow-two-primaries' on " + rsc.get().getName()); - throw new CloudRuntimeException(answers.get(0).getMessage()); + if (answers.hasError()) { + logger.error( + String.format("Failed to remove 'allow-two-primaries' on %s: %s", + rsc.get().getName(), LinstorUtil.getBestErrorMessage(answers))); + // do not fail here as removing allow-two-primaries property isn't fatal } return true; } logger.warn("Linstor: Couldn't find resource for this path: " + localPath); } catch (ApiException apiEx) { - logger.error(apiEx); - throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx); + logger.error(apiEx.getBestMessage()); + // do not fail here as removing allow-two-primaries property isn't fatal } } - return false; + return true; } @Override 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 32f1fa7bd11..f7e643ca62b 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 @@ -191,6 +191,15 @@ public class StorPoolPrimaryDataStoreDriver implements PrimaryDataStoreDriver { @Override public void revokeAccess(DataObject data, Host host, DataStore dataStore) { + if (DataObjectType.VOLUME == data.getType()) { + final VolumeVO volume = volumeDao.findById(data.getId()); + if (volume.getInstanceId() == null) { + StorPoolUtil.spLog("Removing tags from detached volume=%s", volume.toString()); + SpConnectionDesc conn = StorPoolUtil.getSpConnection(dataStore.getUuid(), dataStore.getId(), storagePoolDetailsDao, primaryStoreDao); + StorPoolUtil.volumeRemoveTags(StorPoolStorageAdaptor.getVolumeNameFromPath(volume.getPath(), true), conn); + } + } + } private void updateStoragePool(final long poolId, final long deltaUsedBytes) { @@ -1039,7 +1048,7 @@ public class StorPoolPrimaryDataStoreDriver implements PrimaryDataStoreDriver { } if (vinfo.getMaxIops() != null) { - response = StorPoolUtil.volumeUpdateTags(volumeName, null, vinfo.getMaxIops(), conn, null); + response = StorPoolUtil.volumeUpdateIopsAndTags(volumeName, null, vinfo.getMaxIops(), conn, null); if (response.getError() != null) { StorPoolUtil.spLog("Volume was reverted successfully but max iops could not be set due to %s", response.getError().getDescr()); } @@ -1128,13 +1137,16 @@ public class StorPoolPrimaryDataStoreDriver implements PrimaryDataStoreDriver { @Override public void provideVmInfo(long vmId, long volumeId) { VolumeVO volume = volumeDao.findById(volumeId); + if (volume.getInstanceId() == null) { + return; + } StoragePoolVO poolVO = primaryStoreDao.findById(volume.getPoolId()); if (poolVO != null) { try { 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.volumeUpdateTags(volName, volume.getInstanceId() != null ? userVM.getUuid() : "", null, conn, getVcPolicyTag(vmId)); + SpApiResponse resp = StorPoolUtil.volumeUpdateIopsAndTags(volName, volume.getInstanceId() != null ? userVM.getUuid() : "", null, conn, getVcPolicyTag(vmId)); 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 8db93d968d6..214f93f735f 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 @@ -534,7 +534,14 @@ public class StorPoolUtil { return POST("MultiCluster/VolumeUpdate/" + name, json, conn); } - public static SpApiResponse volumeUpdateTags(final String name, final String uuid, Long iops, + public static SpApiResponse volumeRemoveTags(String name, SpConnectionDesc conn) { + Map json = new HashMap<>(); + Map tags = StorPoolHelper.addStorPoolTags(null, "", null, ""); + json.put("tags", tags); + return POST("MultiCluster/VolumeUpdate/" + name, json, conn); + } + + public static SpApiResponse volumeUpdateIopsAndTags(final String name, final String uuid, Long iops, SpConnectionDesc conn, String vcPolicy) { Map json = new HashMap<>(); Map tags = StorPoolHelper.addStorPoolTags(null, uuid, null, vcPolicy); diff --git a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolVMSnapshotStrategy.java b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolVMSnapshotStrategy.java index d3c4b456f62..2596b6a5bde 100644 --- a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolVMSnapshotStrategy.java +++ b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolVMSnapshotStrategy.java @@ -333,7 +333,7 @@ public class StorPoolVMSnapshotStrategy extends DefaultVMSnapshotStrategy { } VolumeInfo vinfo = volFactory.getVolume(volumeObjectTO.getId()); if (vinfo.getMaxIops() != null) { - resp = StorPoolUtil.volumeUpdateTags(volumeName, null, vinfo.getMaxIops(), conn, null); + resp = StorPoolUtil.volumeUpdateIopsAndTags(volumeName, null, vinfo.getMaxIops(), conn, null); if (resp.getError() != null) { StorPoolUtil.spLog("Volume was reverted successfully but max iops could not be set due to %s", diff --git a/test/integration/plugins/storpool/TestTagsOnStorPool.py b/test/integration/plugins/storpool/TestTagsOnStorPool.py index 6d13e2081d9..a66fb3570f4 100644 --- a/test/integration/plugins/storpool/TestTagsOnStorPool.py +++ b/test/integration/plugins/storpool/TestTagsOnStorPool.py @@ -218,6 +218,19 @@ class TestStoragePool(cloudstackTestCase): hypervisor=cls.hypervisor, rootdisksize=10 ) + cls.virtual_machine3 = VirtualMachine.create( + cls.apiclient, + {"name":"StorPool-%s" % uuid.uuid4() }, + zoneid=cls.zone.id, + templateid=template.id, + accountid=cls.account.name, + domainid=cls.account.domainid, + serviceofferingid=cls.service_offering.id, + hypervisor=cls.hypervisor, + diskofferingid=cls.disk_offerings.id, + size=2, + rootdisksize=10 + ) cls.template = template cls.random_data_0 = random_gen(size=100) cls.test_dir = "/tmp" @@ -270,7 +283,7 @@ class TestStoragePool(cloudstackTestCase): virtualmachineid = self.virtual_machine.id, listall=True ) - self.vc_policy_tags(volumes, vm_tags, vm) + self.vc_policy_tags(volumes, vm_tags, vm, True) @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") @@ -310,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) + self.vc_policy_tags(volumes, vm_tags, vm, True) self.assertEqual(volume_attached.id, self.volume.id, "Is not the same volume ") @@ -442,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) + self.vc_policy_tags(volumes, vm_tags, vm, True) self.assertEqual( self.random_data_0, @@ -490,18 +503,17 @@ class TestStoragePool(cloudstackTestCase): def test_06_remove_vcpolicy_tag_when_disk_detached(self): """ Test remove vc-policy tag to disk detached from VM""" time.sleep(60) - volume_detached = self.virtual_machine.detach_volume( - self.apiclient, - self.volume_2 - ) vm = list_virtual_machines(self.apiclient,id = self.virtual_machine.id, listall=True) vm_tags = vm[0].tags volumes = list_volumes( self.apiclient, - virtualmachineid = self.virtual_machine.id, listall=True + id= self.volume_2.id, listall=True, ) - - self.vc_policy_tags( volumes, vm_tags, vm) + volume_detached = self.virtual_machine.detach_volume( + self.apiclient, + self.volume_2 + ) + self.vc_policy_tags( volumes, vm_tags, vm, False) @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") def test_07_delete_vcpolicy_tag(self): @@ -538,7 +550,7 @@ class TestStoragePool(cloudstackTestCase): virtualmachineid = self.virtual_machine2.id, listall=True, type = "ROOT" ) - self.vc_policy_tags(volume, vm_tags, vm) + self.vc_policy_tags(volume, vm_tags, vm, True) snapshot = Snapshot.create( self.apiclient, @@ -560,11 +572,36 @@ class TestStoragePool(cloudstackTestCase): vm_tags = vm[0].tags vol = list_volumes(self.apiclient, id = snapshot.volumeid, listall=True) - self.vc_policy_tags(vol, vm_tags, vm) + self.vc_policy_tags(vol, vm_tags, vm, True) + @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") + def test_09_remove_vm_tags_on_datadisks_attached_to_destroyed_vm(self): + tag = Tag.create( + self.apiclient, + resourceIds=self.virtual_machine3.id, + resourceType='UserVm', + tags={'vc-policy': 'testing_vc-policy'} + ) + vm = list_virtual_machines(self.apiclient,id = self.virtual_machine3.id, listall=True) + vm_tags = vm[0].tags + volumes = list_volumes( + self.apiclient, + virtualmachineid = self.virtual_machine3.id, listall=True + ) - def vc_policy_tags(self, volumes, vm_tags, vm): - flag = False + self.vc_policy_tags(volumes, vm_tags, vm, True) + + volumes = list_volumes( + self.apiclient, + 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) + + def vc_policy_tags(self, volumes, vm_tags, vm, should_tags_exists=None): + vcPolicyTag = False + cvmTag = False for v in volumes: name = v.path.split("/")[3] spvolume = self.spapi.volumeList(volumeName="~" + name) @@ -572,9 +609,15 @@ class TestStoragePool(cloudstackTestCase): for t in tags: for vm_tag in vm_tags: if t == vm_tag.key: - flag = True + vcPolicyTag = 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) - self.assertTrue(flag, "There aren't volumes with vm tags") + if should_tags_exists: + self.assertTrue(vcPolicyTag, "There aren't volumes with vm tags") + self.assertTrue(cvmTag, "There aren't volumes with vm tags") + else: + self.assertFalse(vcPolicyTag, "The tags should be removed") + self.assertFalse(cvmTag, "The tags should be removed") diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index f9b6d53626f..98923775fe6 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -527,7 +527,7 @@ class VirtualMachine: customcpuspeed=None, custommemory=None, rootdisksize=None, rootdiskcontroller=None, vpcid=None, macaddress=None, datadisktemplate_diskoffering_list={}, properties=None, nicnetworklist=None, bootmode=None, boottype=None, dynamicscalingenabled=None, - userdataid=None, userdatadetails=None, extraconfig=None): + userdataid=None, userdatadetails=None, extraconfig=None, size=None): """Create the instance""" cmd = deployVirtualMachine.deployVirtualMachineCmd() @@ -649,7 +649,9 @@ class VirtualMachine: if rootdiskcontroller: cmd.details[0]["rootDiskController"] = rootdiskcontroller - if "size" in services: + if size: + cmd.size = size + elif "size" in services: cmd.size = services["size"] if group: