server: Fix migration target has no matching tags (#3329)

The code prior to this commit was looking into storage tags at the
storage_pool_details. However, it gets null (table is empty). It should
select from storage_pool_tags, which would result on the storage pool
tags. and then reflect on the code that matched the volume tags (e.g.
'aTag') with the storage pool tags (empty).

The code prior to this commit was looking for the storage tags at the table
storage_pool_details, which is empty. It should select from storage_pool_tags,
which contains the tags from each tagged storage.

Author: Gabriel Beims Bräscher

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

@@ -72,8 +72,6 @@
 import org.apache.cloudstack.storage.command.DettachCommand;
 import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
@@ -120,6 +118,7 @@
 import com.cloud.storage.Storage.ImageFormat;
 import com.cloud.storage.dao.DiskOfferingDao;
 import com.cloud.storage.dao.SnapshotDao;
+import com.cloud.storage.dao.StoragePoolTagsDao;
 import com.cloud.storage.dao.VMTemplateDao;
 import com.cloud.storage.dao.VolumeDao;
 import com.cloud.storage.snapshot.SnapshotApiService;
@@ -254,7 +253,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
     @Inject
     private StorageManager storageMgr;
     @Inject
-    private StoragePoolDetailsDao storagePoolDetailsDao;
+    private StoragePoolTagsDao storagePoolTagsDao;
     @Inject
     private StorageUtil storageUtil;
 
@@ -2084,11 +2083,12 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) {
         // OfflineVmwareMigration: check storage tags on disk(offering)s in comparison to destination storage pool
         // OfflineVmwareMigration: if no match return a proper error now
         DiskOfferingVO diskOffering = _diskOfferingDao.findById(vol.getDiskOfferingId());
-        if(diskOffering.equals(null)) {
-            throw new CloudRuntimeException("volume '" + vol.getUuid() +"', has no diskoffering. Migration target cannot be checked.");
+        if (diskOffering.equals(null)) {
+            throw new CloudRuntimeException("volume '" + vol.getUuid() + "', has no diskoffering. Migration target cannot be checked.");
         }
-        if(! doesTargetStorageSupportDiskOffering(destPool, diskOffering)) {
-            throw new CloudRuntimeException("Migration target has no matching tags for volume '" +vol.getName() + "(" + vol.getUuid() + ")'");
+        if (!doesTargetStorageSupportDiskOffering(destPool, diskOffering)) {
+            throw new CloudRuntimeException(String.format("Migration target pool [%s, tags:%s] has no matching tags for volume [%s, uuid:%s, tags:%s]", destPool.getName(),
+                    getStoragePoolTags(destPool), vol.getName(), vol.getUuid(), diskOffering.getTags()));
         }
 
         if (liveMigrateVolume && destPool.getClusterId() != null && srcClusterId != null) {
@@ -2278,15 +2278,11 @@ public boolean doesTargetStorageSupportDiskOffering(StoragePool destPool, String
      *  Retrieves the storage pool tags as a {@link String}. If the storage pool does not have tags we return a null value.
      */
     protected String getStoragePoolTags(StoragePool destPool) {
-        List<StoragePoolDetailVO> storagePoolDetails = storagePoolDetailsDao.listDetails(destPool.getId());
-        if (CollectionUtils.isEmpty(storagePoolDetails)) {
+        List<String> destPoolTags = storagePoolTagsDao.getStoragePoolTags(destPool.getId());
+        if (CollectionUtils.isEmpty(destPoolTags)) {
             return null;
         }
-        String storageTags = "";
-        for (StoragePoolDetailVO storagePoolDetailVO : storagePoolDetails) {
-            storageTags = storageTags + storagePoolDetailVO.getName() + ",";
-        }
-        return storageTags.substring(0, storageTags.length() - 1);
+        return StringUtils.join(destPoolTags, ",");
     }
 
     private Volume orchestrateMigrateVolume(VolumeVO volume, StoragePool destPool, boolean liveMigrateVolume, DiskOfferingVO newDiskOffering) {

server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java

@@ -49,8 +49,6 @@
 import org.apache.cloudstack.framework.jobs.dao.AsyncJobJoinMapDao;
 import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
@@ -79,6 +77,7 @@
 import com.cloud.org.Grouping;
 import com.cloud.serializer.GsonHelper;
 import com.cloud.storage.Volume.Type;
+import com.cloud.storage.dao.StoragePoolTagsDao;
 import com.cloud.storage.dao.VolumeDao;
 import com.cloud.storage.snapshot.SnapshotManager;
 import com.cloud.user.Account;
@@ -146,7 +145,7 @@ public class VolumeApiServiceImplTest {
     @Mock
     private HostDao _hostDao;
     @Mock
-    private StoragePoolDetailsDao storagePoolDetailsDao;
+    private StoragePoolTagsDao storagePoolTagsDao;
 
     private DetachVolumeCmd detachCmd = new DetachVolumeCmd();
     private Class<?> _detachCmdClass = detachCmd.getClass();
@@ -516,26 +515,25 @@ public void tearDown() {
 
     @Test
     public void getStoragePoolTagsTestStorageWithoutTags() {
-        Mockito.when(storagePoolDetailsDao.listDetails(storagePoolMockId)).thenReturn(new ArrayList<>());
+        Mockito.when(storagePoolTagsDao.getStoragePoolTags(storagePoolMockId)).thenReturn(new ArrayList<>());
 
         String returnedStoragePoolTags = volumeApiServiceImpl.getStoragePoolTags(storagePoolMock);
 
         Assert.assertNull(returnedStoragePoolTags);
-
     }
 
     @Test
     public void getStoragePoolTagsTestStorageWithTags() {
-        ArrayList<StoragePoolDetailVO> tags = new ArrayList<>();
-        StoragePoolDetailVO tag1 = new StoragePoolDetailVO(1l, "tag1", "value", true);
-        StoragePoolDetailVO tag2 = new StoragePoolDetailVO(1l, "tag2", "value", true);
-        StoragePoolDetailVO tag3 = new StoragePoolDetailVO(1l, "tag3", "value", true);
+        ArrayList<String> tags = new ArrayList<>();
+        String tag1 = "tag1";
+        String tag2 = "tag2";
+        String tag3 = "tag3";
 
         tags.add(tag1);
         tags.add(tag2);
         tags.add(tag3);
 
-        Mockito.when(storagePoolDetailsDao.listDetails(storagePoolMockId)).thenReturn(tags);
+        Mockito.when(storagePoolTagsDao.getStoragePoolTags(storagePoolMockId)).thenReturn(tags);
 
         String returnedStoragePoolTags = volumeApiServiceImpl.getStoragePoolTags(storagePoolMock);