From 6323aac01b7772c201ae02dac911cb1afed9ac70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Beims=20Br=C3=A4scher?= Date: Fri, 7 Jun 2019 03:14:00 -0300 Subject: [PATCH] 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. --- .../cloud/storage/VolumeApiServiceImpl.java | 24 ++++++++----------- .../storage/VolumeApiServiceImplTest.java | 18 +++++++------- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index b2a395b4ef8..ae9af51a71d 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -72,8 +72,6 @@ import org.apache.cloudstack.storage.command.AttachCommand; 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.service.dao.ServiceOfferingDetailsDao; 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 class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic // 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 class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic * 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 storagePoolDetails = storagePoolDetailsDao.listDetails(destPool.getId()); - if (CollectionUtils.isEmpty(storagePoolDetails)) { + List 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) { diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 693b437079b..bb5599f0a99 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -49,8 +49,6 @@ import org.apache.cloudstack.framework.jobs.AsyncJobManager; 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.hypervisor.Hypervisor.HypervisorType; 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 class VolumeApiServiceImplTest { @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 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 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);