diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java index 9eb623a7bd6..71c291e900b 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java @@ -131,4 +131,12 @@ public interface VolumeDao extends GenericDao, StateDao findByDiskOfferingId(long diskOfferingId); } diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java index d934f80dc4e..9a46e923f88 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java @@ -61,6 +61,7 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol protected final GenericSearchBuilder ActiveTemplateSearch; protected final SearchBuilder InstanceStatesSearch; protected final SearchBuilder AllFieldsSearch; + protected final SearchBuilder diskOfferingSearch; protected final SearchBuilder RootDiskStateSearch; protected GenericSearchBuilder CountByAccount; protected GenericSearchBuilder primaryStorageSearch; @@ -262,6 +263,14 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol return listIncludingRemovedBy(sc); } + @Override + public List findByDiskOfferingId(long diskOfferingId) { + SearchCriteria sc = diskOfferingSearch.create(); + sc.setParameters("diskOfferingId", diskOfferingId); + + return listBy(sc); + } + @Override public boolean isAnyVolumeActivelyUsingTemplateOnPool(long templateId, long poolId) { SearchCriteria sc = ActiveTemplateSearch.create(); @@ -392,6 +401,10 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol TemplateZoneSearch.and("zone", TemplateZoneSearch.entity().getDataCenterId(), Op.EQ); TemplateZoneSearch.done(); + diskOfferingSearch = createSearchBuilder(); + diskOfferingSearch.and("diskOfferingId", diskOfferingSearch.entity().getDiskOfferingId(), Op.EQ); + diskOfferingSearch.done(); + TotalSizeByPoolSearch = createSearchBuilder(SumCount.class); TotalSizeByPoolSearch.select("sum", Func.SUM, TotalSizeByPoolSearch.entity().getSize()); TotalSizeByPoolSearch.select("count", Func.COUNT, (Object[])null); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java index 2ab95bb8cfc..2fc52b3fb28 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java @@ -74,10 +74,10 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase protected final String TagsSqlPrefix = "SELECT storage_pool.* from storage_pool LEFT JOIN storage_pool_tags ON storage_pool.id = storage_pool_tags.pool_id WHERE storage_pool.removed is null and storage_pool.status = 'Up' and storage_pool.data_center_id = ? and (storage_pool.pod_id = ? or storage_pool.pod_id is null) and storage_pool.scope = ? and ("; protected final String TagsSqlSuffix = ") GROUP BY storage_pool_tags.pool_id HAVING COUNT(storage_pool_tags.tag) >= ?"; - private static final String GET_STORAGE_POOLS_OF_VOLUMES_WITHOUT_OR_NOT_HAVING_TAGS = "select s.id " + - "from volumes vol " + - "join storage_pool s on vol.pool_id=s.id " + - "where vol.disk_offering_id= ? and vol.state not in (\"Destroy\", \"Error\", \"Expunging\") group by s.id"; + private static final String GET_STORAGE_POOLS_OF_VOLUMES_WITHOUT_OR_NOT_HAVING_TAGS = "SELECT s.* " + + "FROM volumes vol " + + "JOIN storage_pool s ON vol.pool_id = s.id " + + "WHERE vol.disk_offering_id = ? AND vol.state NOT IN (\"Destroy\", \"Error\", \"Expunging\", \"Expunged\") GROUP BY s.id"; /** * Used in method findPoolsByDetailsOrTagsInternal diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index ff45b73509a..d0a5eec68fa 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -44,6 +44,7 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; + import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.affinity.AffinityGroup; import org.apache.cloudstack.affinity.AffinityGroupService; @@ -118,6 +119,7 @@ import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.ObjectUtils; @@ -239,6 +241,7 @@ import com.cloud.storage.Storage; import com.cloud.storage.Storage.ProvisioningType; import com.cloud.storage.StorageManager; import com.cloud.storage.Volume; +import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.StoragePoolTagsDao; import com.cloud.storage.dao.VMTemplateZoneDao; @@ -3900,7 +3903,14 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati for (StoragePoolVO storagePoolVO : pools) { List tagsOnPool = storagePoolTagDao.getStoragePoolTags(storagePoolVO.getId()); if (CollectionUtils.isEmpty(tagsOnPool) || !tagsOnPool.containsAll(listOfTags)) { - throw new InvalidParameterValueException(String.format("There are active volumes using offering [%s], and the pools [%s] don't have the new tags", diskOffering.getId(), pools)); + DiskOfferingVO offeringToRetrieveInfo = _diskOfferingDao.findById(diskOffering.getId()); + List volumes = _volumeDao.findByDiskOfferingId(diskOffering.getId()); + String listOfVolumesNamesAndUuid = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(volumes, "name", "uuid"); + String diskOfferingInfo = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(offeringToRetrieveInfo, "name", "uuid"); + String poolInfo = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(storagePoolVO, "name", "uuid"); + throw new InvalidParameterValueException(String.format("There are active volumes using the disk offering %s, and the pool %s doesn't have the new tags. " + + "The following volumes are using the mentioned disk offering %s. Please first add the new tags to the mentioned storage pools before adding them" + + " to the disk offering.", diskOfferingInfo, poolInfo, listOfVolumesNamesAndUuid)); } } } diff --git a/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java b/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java index d0943ad5281..7515f125972 100644 --- a/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java +++ b/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java @@ -53,6 +53,8 @@ import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.log4j.Logger; import org.junit.After; import org.junit.Assert; @@ -104,6 +106,8 @@ import com.cloud.network.dao.Ipv6GuestPrefixSubnetNetworkMapDao; import com.cloud.network.dao.PhysicalNetworkDao; import com.cloud.network.dao.PhysicalNetworkVO; import com.cloud.projects.ProjectManager; +import com.cloud.storage.dao.DiskOfferingDao; +import com.cloud.storage.dao.StoragePoolTagsDao; import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.VolumeDao; @@ -186,6 +190,16 @@ public class ConfigurationManagerTest { @Mock DiskOfferingVO diskOfferingVOMock; @Mock + PrimaryDataStoreDao primaryDataStoreDao; + @Mock + StoragePoolTagsDao storagePoolTagsDao; + @Mock + DiskOfferingDao diskOfferingDao; + @Mock + VolumeVO volumeVO; + @Mock + StoragePoolVO storagePoolVO; + @Mock DataCenterGuestIpv6PrefixDao dataCenterGuestIpv6PrefixDao; @Mock Ipv6GuestPrefixSubnetNetworkMapDao ipv6GuestPrefixSubnetNetworkMapDao; @@ -1023,6 +1037,52 @@ public class ConfigurationManagerTest { Mockito.verify(configurationMgr, Mockito.times(1)).updateOfferingTagsIfIsNotNull(tags, diskOfferingVOMock); } + @Test (expected = InvalidParameterValueException.class) + public void updateDiskOfferingTagsWithPrimaryStorageTagsEqualNullTestThrowException(){ + String tags = "tags"; + List storageTagsNull = new ArrayList<>(); + List pools = new ArrayList<>(Arrays.asList(storagePoolVO)); + List volumes = new ArrayList<>(Arrays.asList(volumeVO)); + + Mockito.when(primaryDataStoreDao.listStoragePoolsWithActiveVolumesByOfferingId(anyLong())).thenReturn(pools); + Mockito.when(storagePoolTagsDao.getStoragePoolTags(anyLong())).thenReturn(storageTagsNull); + Mockito.when(diskOfferingDao.findById(anyLong())).thenReturn(diskOfferingVOMock); + Mockito.when(_volumeDao.findByDiskOfferingId(anyLong())).thenReturn(volumes); + + this.configurationMgr.updateOfferingTagsIfIsNotNull(tags, diskOfferingVOMock); + } + + @Test (expected = InvalidParameterValueException.class) + public void updateDiskOfferingTagsWithPrimaryStorageMissingTagsTestThrowException(){ + String tags = "tag1,tag2"; + List storageTagsWithMissingTag = new ArrayList<>(Arrays.asList("tag1")); + List pools = new ArrayList<>(Arrays.asList(storagePoolVO)); + List volumes = new ArrayList<>(Arrays.asList(volumeVO)); + + Mockito.when(primaryDataStoreDao.listStoragePoolsWithActiveVolumesByOfferingId(anyLong())).thenReturn(pools); + Mockito.when(storagePoolTagsDao.getStoragePoolTags(anyLong())).thenReturn(storageTagsWithMissingTag); + Mockito.when(diskOfferingDao.findById(anyLong())).thenReturn(diskOfferingVOMock); + Mockito.when(_volumeDao.findByDiskOfferingId(anyLong())).thenReturn(volumes); + + this.configurationMgr.updateOfferingTagsIfIsNotNull(tags, diskOfferingVOMock); + } + + @Test + public void updateDiskOfferingTagsWithPrimaryStorageWithCorrectTagsTestSuccess(){ + String tags = "tag1,tag2"; + List storageTagsWithCorrectTags = new ArrayList<>(Arrays.asList("tag1","tag2")); + List pools = new ArrayList<>(Arrays.asList(storagePoolVO)); + List volumes = new ArrayList<>(Arrays.asList(volumeVO)); + + Mockito.when(primaryDataStoreDao.listStoragePoolsWithActiveVolumesByOfferingId(anyLong())).thenReturn(pools); + Mockito.when(storagePoolTagsDao.getStoragePoolTags(anyLong())).thenReturn(storageTagsWithCorrectTags); + Mockito.when(diskOfferingDao.findById(anyLong())).thenReturn(diskOfferingVOMock); + Mockito.when(_volumeDao.findByDiskOfferingId(anyLong())).thenReturn(volumes); + + this.configurationMgr.updateOfferingTagsIfIsNotNull(tags, diskOfferingVOMock); + Mockito.verify(diskOfferingVOMock, Mockito.times(1)).setTags(tags); + } + @Test(expected = IllegalArgumentException.class) public void testInvalidCreateDataCenterGuestIpv6Prefix() { CreateGuestNetworkIpv6PrefixCmd cmd = Mockito.mock(CreateGuestNetworkIpv6PrefixCmd.class);