Skip to content

Commit e53159f

Browse files
shwstpprshwstppr
authored
server: fix volume migration pool tag check (#48)
From 4.9.3.0 to 4.10.0.0 storing of tags for storage pools was changed from cloud.storage_pool_details to cloud.storage_pool_tags. Up-port in #47 missed this and was using tag retrieval from the old table. https://github.com/apache/cloudstack/blob/4.10.0.0/setup/db/db/schema-4930to41000.sql#L139-L151 Unit tests replaced from https://github.com/apache/cloudstack/blob/4.12.0.0/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
1 parent 62da865 commit e53159f

File tree

2 files changed

+14
-57
lines changed

2 files changed

+14
-57
lines changed

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@
7171
import org.apache.cloudstack.storage.command.DettachCommand;
7272
import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
7373
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
74-
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
7574
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
7675
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
7776
import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
@@ -120,6 +119,7 @@
120119
import com.cloud.storage.Storage.ImageFormat;
121120
import com.cloud.storage.dao.DiskOfferingDao;
122121
import com.cloud.storage.dao.SnapshotDao;
122+
import com.cloud.storage.dao.StoragePoolTagsDao;
123123
import com.cloud.storage.dao.VMTemplateDao;
124124
import com.cloud.storage.dao.VolumeDao;
125125
import com.cloud.storage.dao.VolumeDetailsDao;
@@ -258,6 +258,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
258258
ClusterDetailsDao _clusterDetailsDao;
259259
@Inject
260260
StorageManager storageMgr;
261+
@Inject
262+
StoragePoolTagsDao storagePoolTagsDao;
261263

262264
protected Gson _gson;
263265

@@ -2079,7 +2081,7 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) {
20792081
if(diskOffering.equals(null)) {
20802082
throw new CloudRuntimeException("volume '" + vol.getUuid() +"', has no diskoffering. Migration target cannot be checked.");
20812083
}
2082-
if(! doesTargetStorageSupportDiskOffering(destPool, diskOffering)) {
2084+
if(!doesTargetStorageSupportDiskOffering(destPool, diskOffering)) {
20832085
throw new CloudRuntimeException("Migration target has no matching tags for volume '" +vol.getName() + "(" + vol.getUuid() + ")'");
20842086
}
20852087

@@ -2227,15 +2229,11 @@ public boolean doesTargetStorageSupportDiskOffering(StoragePool destPool, String
22272229
* Retrieves the storage pool tags as a {@link String}. If the storage pool does not have tags we return a null value.
22282230
*/
22292231
protected String getStoragePoolTags(StoragePool destPool) {
2230-
List<StoragePoolDetailVO> storagePoolDetails = storagePoolDetailsDao.listDetails(destPool.getId());
2231-
if (CollectionUtils.isEmpty(storagePoolDetails)) {
2232+
List<String> destPoolTags = storagePoolTagsDao.getStoragePoolTags(destPool.getId());
2233+
if (CollectionUtils.isEmpty(destPoolTags)) {
22322234
return null;
22332235
}
2234-
StringBuilder tagsBuilder = new StringBuilder();
2235-
for (StoragePoolDetailVO storagePoolDetailVO : storagePoolDetails) {
2236-
tagsBuilder.append(storagePoolDetailVO.getName() + ",");
2237-
}
2238-
return tagsBuilder.substring(0, tagsBuilder.length() -1);
2236+
return StringUtils.join(destPoolTags, ",");
22392237
}
22402238

22412239
private Volume orchestrateMigrateVolume(long volumeId, long destPoolId, boolean liveMigrateVolume) {

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

Lines changed: 7 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,6 @@
5151
import org.apache.cloudstack.framework.jobs.dao.AsyncJobJoinMapDao;
5252
import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO;
5353
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
54-
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
55-
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
5654
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
5755
import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
5856
import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
@@ -113,8 +111,6 @@ public class VolumeApiServiceImplTest {
113111
@Mock
114112
private AccountManager accountManagerMock;
115113
@Mock
116-
StoragePoolDetailsDao storagePoolDetailsDao;
117-
@Mock
118114
VMSnapshotDao _vmSnapshotDao;
119115
@Mock
120116
private VolumeService volumeServiceMock;
@@ -887,23 +883,13 @@ public void deleteVolumeTestVolumeStateReadyThrowingRuntimeException() throws In
887883
volumeApiServiceImpl.deleteVolume(volumeMockId, accountMock);
888884
}
889885

890-
StoragePoolDetailVO a = new StoragePoolDetailVO(1l, "A", "true", false);
891-
StoragePoolDetailVO b = new StoragePoolDetailVO(2l, "B", "true", false);
892-
StoragePoolDetailVO c = new StoragePoolDetailVO(3l, "C", "true", false);
893-
StoragePoolDetailVO d = new StoragePoolDetailVO(4l, "D", "true", false);
894-
StoragePoolDetailVO x = new StoragePoolDetailVO(24l, "X", "true", false);
895-
StoragePoolDetailVO y = new StoragePoolDetailVO(25l, "Y", "true", false);
896-
897886
@Test
898887
public void doesTargetStorageSupportDiskOfferingTestDiskOfferingMoreTagsThanStorageTags() {
899888
DiskOfferingVO diskOfferingVoMock = Mockito.mock(DiskOfferingVO.class);
900889
Mockito.doReturn("A,B,C").when(diskOfferingVoMock).getTags();
901890

902-
List<StoragePoolDetailVO> details = new ArrayList<>();
903-
details.add(a);
904891
StoragePool storagePoolMock = Mockito.mock(StoragePool.class);
905-
when(storagePoolMock.getId()).thenReturn(1L);
906-
when(storagePoolDetailsDao.listDetails(1L)).thenReturn(details);
892+
Mockito.doReturn("A").when(volumeApiServiceImpl).getStoragePoolTags(storagePoolMock);
907893

908894
boolean result = volumeApiServiceImpl.doesTargetStorageSupportDiskOffering(storagePoolMock, diskOfferingVoMock);
909895

@@ -915,16 +901,8 @@ public void doesTargetStorageSupportDiskOfferingTestDiskOfferingTagsIsSubSetOfSt
915901
DiskOfferingVO diskOfferingVoMock = Mockito.mock(DiskOfferingVO.class);
916902
Mockito.doReturn("A,B,C").when(diskOfferingVoMock).getTags();
917903

918-
List<StoragePoolDetailVO> details = new ArrayList<>();
919-
details.add(a);
920-
details.add(b);
921-
details.add(c);
922-
details.add(d);
923-
details.add(x);
924-
details.add(y);
925904
StoragePool storagePoolMock = Mockito.mock(StoragePool.class);
926-
when(storagePoolMock.getId()).thenReturn(1L);
927-
when(storagePoolDetailsDao.listDetails(1L)).thenReturn(details);
905+
Mockito.doReturn("A,B,C,D,X,Y").when(volumeApiServiceImpl).getStoragePoolTags(storagePoolMock);
928906

929907
boolean result = volumeApiServiceImpl.doesTargetStorageSupportDiskOffering(storagePoolMock, diskOfferingVoMock);
930908

@@ -936,16 +914,8 @@ public void doesTargetStorageSupportDiskOfferingTestDiskOfferingTagsEmptyAndStor
936914
DiskOfferingVO diskOfferingVoMock = Mockito.mock(DiskOfferingVO.class);
937915
Mockito.doReturn("").when(diskOfferingVoMock).getTags();
938916

939-
List<StoragePoolDetailVO> details = new ArrayList<>();
940-
details.add(a);
941-
details.add(b);
942-
details.add(c);
943-
details.add(d);
944-
details.add(x);
945-
details.add(y);
946917
StoragePool storagePoolMock = Mockito.mock(StoragePool.class);
947-
when(storagePoolMock.getId()).thenReturn(1L);
948-
when(storagePoolDetailsDao.listDetails(1L)).thenReturn(details);
918+
Mockito.doReturn("A,B,C,D,X,Y").when(volumeApiServiceImpl).getStoragePoolTags(storagePoolMock);
949919

950920
boolean result = volumeApiServiceImpl.doesTargetStorageSupportDiskOffering(storagePoolMock, diskOfferingVoMock);
951921

@@ -957,10 +927,8 @@ public void doesTargetStorageSupportDiskOfferingTestDiskOfferingTagsNotEmptyAndS
957927
DiskOfferingVO diskOfferingVoMock = Mockito.mock(DiskOfferingVO.class);
958928
Mockito.doReturn("A").when(diskOfferingVoMock).getTags();
959929

960-
List<StoragePoolDetailVO> details = new ArrayList<>();
961930
StoragePool storagePoolMock = Mockito.mock(StoragePool.class);
962-
when(storagePoolMock.getId()).thenReturn(1L);
963-
when(storagePoolDetailsDao.listDetails(1L)).thenReturn(details);
931+
Mockito.doReturn("").when(volumeApiServiceImpl).getStoragePoolTags(storagePoolMock);
964932

965933
boolean result = volumeApiServiceImpl.doesTargetStorageSupportDiskOffering(storagePoolMock, diskOfferingVoMock);
966934

@@ -972,10 +940,8 @@ public void doesTargetStorageSupportDiskOfferingTestDiskOfferingTagsEmptyAndStor
972940
DiskOfferingVO diskOfferingVoMock = Mockito.mock(DiskOfferingVO.class);
973941
Mockito.doReturn("").when(diskOfferingVoMock).getTags();
974942

975-
List<StoragePoolDetailVO> details = new ArrayList<>();
976943
StoragePool storagePoolMock = Mockito.mock(StoragePool.class);
977-
when(storagePoolMock.getId()).thenReturn(1L);
978-
when(storagePoolDetailsDao.listDetails(1L)).thenReturn(details);
944+
Mockito.doReturn("").when(volumeApiServiceImpl).getStoragePoolTags(storagePoolMock);
979945

980946
boolean result = volumeApiServiceImpl.doesTargetStorageSupportDiskOffering(storagePoolMock, diskOfferingVoMock);
981947

@@ -987,12 +953,8 @@ public void doesTargetStorageSupportDiskOfferingTestDiskOfferingTagsDifferentFro
987953
DiskOfferingVO diskOfferingVoMock = Mockito.mock(DiskOfferingVO.class);
988954
Mockito.doReturn("A,B").when(diskOfferingVoMock).getTags();
989955

990-
List<StoragePoolDetailVO> details = new ArrayList<>();
991-
details.add(c);
992-
details.add(d);
993956
StoragePool storagePoolMock = Mockito.mock(StoragePool.class);
994-
when(storagePoolMock.getId()).thenReturn(1L);
995-
when(storagePoolDetailsDao.listDetails(1L)).thenReturn(details);
957+
Mockito.doReturn("C,D").when(volumeApiServiceImpl).getStoragePoolTags(storagePoolMock);
996958

997959
boolean result = volumeApiServiceImpl.doesTargetStorageSupportDiskOffering(storagePoolMock, diskOfferingVoMock);
998960

@@ -1004,11 +966,8 @@ public void doesTargetStorageSupportDiskOfferingTestDiskOfferingTagsEqualsStorag
1004966
DiskOfferingVO diskOfferingVoMock = Mockito.mock(DiskOfferingVO.class);
1005967
Mockito.doReturn("A").when(diskOfferingVoMock).getTags();
1006968

1007-
List<StoragePoolDetailVO> details = new ArrayList<>();
1008-
details.add(a);
1009969
StoragePool storagePoolMock = Mockito.mock(StoragePool.class);
1010-
when(storagePoolMock.getId()).thenReturn(1L);
1011-
when(storagePoolDetailsDao.listDetails(1L)).thenReturn(details);
970+
Mockito.doReturn("A").when(volumeApiServiceImpl).getStoragePoolTags(storagePoolMock);
1012971

1013972
boolean result = volumeApiServiceImpl.doesTargetStorageSupportDiskOffering(storagePoolMock, diskOfferingVoMock);
1014973

0 commit comments

Comments
 (0)