This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 7be7ef66fb3 Improve error message on storage tags update (#6269)
7be7ef66fb3 is described below
commit 7be7ef66fb34523938664544c3fb4aab430c494b
Author: Bryan Lima <[email protected]>
AuthorDate: Wed Sep 14 04:06:20 2022 -0300
Improve error message on storage tags update (#6269)
Co-authored-by: Daniel Augusto Veronezi Salvador
<[email protected]>
Co-authored-by: dahn <[email protected]>
---
.../main/java/com/cloud/storage/dao/VolumeDao.java | 8 +++
.../java/com/cloud/storage/dao/VolumeDaoImpl.java | 13 +++++
.../datastore/db/PrimaryDataStoreDaoImpl.java | 8 +--
.../configuration/ConfigurationManagerImpl.java | 12 ++++-
.../configuration/ConfigurationManagerTest.java | 60 ++++++++++++++++++++++
5 files changed, 96 insertions(+), 5 deletions(-)
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<VolumeVO,
Long>, StateDao<Volume.S
* Updates the disk offering for the given volume.
*/
void updateDiskOffering(long volumeId, long diskOfferingId);
+
+ /**
+ * Retrieves volumes that use the disk offering passed as parameter.
+ *
+ * @param diskOfferingId the disk offering ID.
+ * @return the list of volumes that uses that disk offering.
+ */
+ List<VolumeVO> 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<VolumeVO,
Long> implements Vol
protected final GenericSearchBuilder<VolumeVO, Long> ActiveTemplateSearch;
protected final SearchBuilder<VolumeVO> InstanceStatesSearch;
protected final SearchBuilder<VolumeVO> AllFieldsSearch;
+ protected final SearchBuilder<VolumeVO> diskOfferingSearch;
protected final SearchBuilder<VolumeVO> RootDiskStateSearch;
protected GenericSearchBuilder<VolumeVO, Long> CountByAccount;
protected GenericSearchBuilder<VolumeVO, SumCount> primaryStorageSearch;
@@ -262,6 +263,14 @@ public class VolumeDaoImpl extends
GenericDaoBase<VolumeVO, Long> implements Vol
return listIncludingRemovedBy(sc);
}
+ @Override
+ public List<VolumeVO> findByDiskOfferingId(long diskOfferingId) {
+ SearchCriteria<VolumeVO> sc = diskOfferingSearch.create();
+ sc.setParameters("diskOfferingId", diskOfferingId);
+
+ return listBy(sc);
+ }
+
@Override
public boolean isAnyVolumeActivelyUsingTemplateOnPool(long templateId,
long poolId) {
SearchCriteria<Long> sc = ActiveTemplateSearch.create();
@@ -392,6 +401,10 @@ public class VolumeDaoImpl extends
GenericDaoBase<VolumeVO, Long> 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<StoragePoolVO, Long>
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<String> 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<VolumeVO> 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<String> storageTagsNull = new ArrayList<>();
+ List<StoragePoolVO> pools = new
ArrayList<>(Arrays.asList(storagePoolVO));
+ List<VolumeVO> 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<String> storageTagsWithMissingTag = new
ArrayList<>(Arrays.asList("tag1"));
+ List<StoragePoolVO> pools = new
ArrayList<>(Arrays.asList(storagePoolVO));
+ List<VolumeVO> 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<String> storageTagsWithCorrectTags = new
ArrayList<>(Arrays.asList("tag1","tag2"));
+ List<StoragePoolVO> pools = new
ArrayList<>(Arrays.asList(storagePoolVO));
+ List<VolumeVO> 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);
