Skip to content

Commit 48c08a2

Browse files
author
GabrielBrascher
committed
Address reviewer and add method that updates the DB template reference
1 parent 28af880 commit 48c08a2

3 files changed

Lines changed: 52 additions & 14 deletions

File tree

‎engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.apache.cloudstack.storage.motion;
2020

2121
import java.io.File;
22+
import java.util.Date;
2223
import java.util.Map;
2324
import java.util.Set;
2425

@@ -29,6 +30,7 @@
2930
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
3031
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
3132
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
33+
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
3234
import org.apache.cloudstack.storage.command.CopyCommand;
3335
import org.apache.cloudstack.storage.datastore.DataStoreManagerImpl;
3436
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
@@ -52,6 +54,7 @@
5254
import com.cloud.storage.StorageManager;
5355
import com.cloud.storage.StoragePool;
5456
import com.cloud.storage.VMTemplateStoragePoolVO;
57+
import com.cloud.storage.VMTemplateStorageResourceAssoc;
5558
import com.cloud.storage.VolumeVO;
5659
import com.cloud.storage.dao.VMTemplatePoolDao;
5760
import com.cloud.utils.exception.CloudRuntimeException;
@@ -165,7 +168,7 @@ protected boolean shouldMigrateVolume(StoragePoolVO sourceStoragePool, Host dest
165168
* If the template is not on the target primary storage then it migrates the template.
166169
*/
167170
@Override
168-
protected void migrateTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVolumeInfo, DataStore destDataStore, StoragePool destStoragePool,
171+
protected void copyTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVolumeInfo, StoragePool srcStoragePool, DataStore destDataStore, StoragePool destStoragePool,
169172
Host destHost) {
170173
VMTemplateStoragePoolVO sourceVolumeTemplateStoragePoolVO = vmTemplatePoolDao.findByPoolTemplate(destStoragePool.getId(), srcVolumeInfo.getTemplateId());
171174
if (sourceVolumeTemplateStoragePoolVO == null && destStoragePool.getPoolType() == StoragePoolType.Filesystem) {
@@ -178,10 +181,29 @@ protected void migrateTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVo
178181

179182
TemplateInfo destTemplateInfo = templateDataFactory.getTemplate(srcVolumeInfo.getTemplateId(), destDataStore);
180183
final TemplateObjectTO destTemplate = new TemplateObjectTO(destTemplateInfo);
181-
sendCopyCommand(destHost, sourceTemplate, destTemplate, destDataStore);
184+
Answer copyCommandAnswer = sendCopyCommand(destHost, sourceTemplate, destTemplate, destDataStore);
185+
186+
if (copyCommandAnswer != null && copyCommandAnswer.getResult()) {
187+
updateTemplateReferenceIfSuccessfulCopy(srcVolumeInfo, srcStoragePool, destTemplateInfo, destDataStore);
188+
}
182189
}
183190
}
184191

192+
/**
193+
* Update the template reference on table "template_spool_ref" (VMTemplateStoragePoolVO) if the template copy was successful.
194+
*/
195+
protected void updateTemplateReferenceIfSuccessfulCopy(VolumeInfo srcVolumeInfo, StoragePool srcStoragePool, TemplateInfo destTemplateInfo, DataStore destDataStore) {
196+
VMTemplateStoragePoolVO srcVolumeTemplateStoragePoolVO = vmTemplatePoolDao.findByPoolTemplate(srcStoragePool.getId(), srcVolumeInfo.getTemplateId());
197+
VMTemplateStoragePoolVO destVolumeTemplateStoragePoolVO = new VMTemplateStoragePoolVO(destDataStore.getId(), srcVolumeInfo.getTemplateId());
198+
destVolumeTemplateStoragePoolVO.setDownloadPercent(100);
199+
destVolumeTemplateStoragePoolVO.setDownloadState(VMTemplateStorageResourceAssoc.Status.DOWNLOADED);
200+
destVolumeTemplateStoragePoolVO.setState(ObjectInDataStoreStateMachine.State.Ready);
201+
destVolumeTemplateStoragePoolVO.setTemplateSize(srcVolumeTemplateStoragePoolVO.getTemplateSize());
202+
destVolumeTemplateStoragePoolVO.setLocalDownloadPath(destTemplateInfo.getUuid());
203+
destVolumeTemplateStoragePoolVO.setInstallPath(destTemplateInfo.getUuid());
204+
vmTemplatePoolDao.persist(destVolumeTemplateStoragePoolVO);
205+
}
206+
185207
/**
186208
* Sends the CopyCommand to migrate the template to the dest host.
187209
*/
@@ -211,7 +233,7 @@ protected void logInCaseOfTemplateCopyFailure(Answer copyCommandAnswer, Template
211233
if (copyCommandAnswer.getDetails() != null) {
212234
failureDetails = " Details: " + copyCommandAnswer.getDetails();
213235
}
214-
LOGGER.debug(generateFailToCopyTemplateMessage(sourceTemplate, destDataStore) + failureDetails);
236+
LOGGER.error(generateFailToCopyTemplateMessage(sourceTemplate, destDataStore) + failureDetails);
215237
}
216238
}
217239
}

‎engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import com.cloud.storage.dao.SnapshotDetailsDao;
6464
import com.cloud.storage.dao.SnapshotDetailsVO;
6565
import com.cloud.storage.dao.VMTemplateDao;
66+
import com.cloud.storage.dao.VMTemplatePoolDao;
6667
import com.cloud.storage.dao.VolumeDao;
6768
import com.cloud.storage.dao.VolumeDetailsDao;
6869
import com.cloud.utils.NumbersUtil;
@@ -160,6 +161,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
160161
@Inject private VolumeService _volumeService;
161162
@Inject private StorageCacheManager cacheMgr;
162163
@Inject private EndPointSelector selector;
164+
@Inject
165+
private VMTemplatePoolDao vmTemplatePoolDao;
163166

164167
@Override
165168
public StrategyPriority canHandle(DataObject srcData, DataObject destData) {
@@ -1724,7 +1727,7 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
17241727
continue;
17251728
}
17261729

1727-
migrateTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, destDataStore, destStoragePool, destHost);
1730+
copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, sourceStoragePool, destDataStore, destStoragePool, destHost);
17281731

17291732
VolumeVO destVolume = duplicateVolumeOnAnotherStorage(srcVolume, destStoragePool);
17301733
VolumeInfo destVolumeInfo = _volumeDataFactory.getVolume(destVolume.getId(), destDataStore);
@@ -1862,9 +1865,11 @@ protected void setVolumePath(VolumeVO volume) {
18621865
}
18631866

18641867
/**
1865-
* For this strategy it is not necessary to check and migrate the template; however, classes that extend this one may need to check if the template is on the target storage pool (and if not then migrate) before migrating the VM.
1868+
* For this strategy it is not necessary to check and migrate the template;
1869+
* however, classes that extend this one may need to copy the template to the target storage pool before migrating the VM.
18661870
*/
1867-
protected void migrateTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVolumeInfo, DataStore destDataStore, StoragePool destStoragePool, Host destHost) {
1871+
protected void copyTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVolumeInfo, StoragePool srcStoragePool, DataStore destDataStore, StoragePool destStoragePool,
1872+
Host destHost) {
18681873
// This method is used by classes that extend this one
18691874
}
18701875

‎engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
2626
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
2727
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
28+
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
2829
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
2930
import org.apache.cloudstack.storage.command.CopyCommand;
3031
import org.apache.cloudstack.storage.datastore.DataStoreManagerImpl;
@@ -91,6 +92,8 @@ public class KvmNonManagedStorageSystemDataMotionTest {
9192
private VMTemplatePoolDao vmTemplatePoolDao;
9293
@Mock
9394
private DataStoreManagerImpl dataStoreManagerImpl;
95+
@Mock
96+
private VolumeDataFactory volumeDataFactory;
9497

9598
@Spy
9699
@InjectMocks
@@ -323,13 +326,15 @@ private void configureAndTestSendCommandTest(Class<? extends CloudException> exc
323326
}
324327

325328
@Test
326-
public void migrateTemplateToTargetStorageIfNeededTestTemplateAlreadyOnTargetHost() throws AgentUnavailableException, OperationTimedoutException {
327-
configureAndTestmigrateTemplateToTargetStorageIfNeeded(new VMTemplateStoragePoolVO(0l, 0l), StoragePoolType.Filesystem, 0);
329+
public void copyTemplateToTargetStorageIfNeededTestTemplateAlreadyOnTargetHost() throws AgentUnavailableException, OperationTimedoutException {
330+
Answer copyCommandAnswer = Mockito.mock(Answer.class);
331+
Mockito.when(copyCommandAnswer.getResult()).thenReturn(true);
332+
configureAndTestcopyTemplateToTargetStorageIfNeeded(new VMTemplateStoragePoolVO(0l, 0l), StoragePoolType.Filesystem, 0);
328333
}
329334

330335
@Test
331336
public void migrateTemplateToTargetStorageIfNeededTestTemplateNotOnTargetHost() throws AgentUnavailableException, OperationTimedoutException {
332-
configureAndTestmigrateTemplateToTargetStorageIfNeeded(null, StoragePoolType.Filesystem, 1);
337+
configureAndTestcopyTemplateToTargetStorageIfNeeded(null, StoragePoolType.Filesystem, 1);
333338
}
334339

335340
@Test
@@ -339,20 +344,25 @@ public void migrateTemplateToTargetStorageIfNeededTestNonDesiredStoragePoolType(
339344
if (storagePoolTypeArray[i] == StoragePoolType.Filesystem) {
340345
continue;
341346
}
342-
configureAndTestmigrateTemplateToTargetStorageIfNeeded(new VMTemplateStoragePoolVO(0l, 0l), storagePoolTypeArray[i], 0);
347+
configureAndTestcopyTemplateToTargetStorageIfNeeded(new VMTemplateStoragePoolVO(0l, 0l), storagePoolTypeArray[i], 0);
343348
}
344349
}
345350

346-
private void configureAndTestmigrateTemplateToTargetStorageIfNeeded(VMTemplateStoragePoolVO vmTemplateStoragePoolVO, StoragePoolType storagePoolType, int times) {
351+
private void configureAndTestcopyTemplateToTargetStorageIfNeeded(VMTemplateStoragePoolVO vmTemplateStoragePoolVO, StoragePoolType storagePoolType, int times) {
347352
DataStore destDataStore = Mockito.mock(DataStore.class);
348353
Host destHost = Mockito.mock(Host.class);
349354

350355
VolumeInfo srcVolumeInfo = Mockito.mock(VolumeInfo.class);
351356
Mockito.when(srcVolumeInfo.getTemplateId()).thenReturn(0l);
352357

358+
StoragePool srcStoragePool = Mockito.mock(StoragePool.class);
359+
360+
VolumeInfo destVolumeInfo = Mockito.mock(VolumeInfo.class);
361+
Mockito.when(volumeDataFactory.getVolume(Mockito.anyLong(), Mockito.any(DataStore.class))).thenReturn(destVolumeInfo);
362+
353363
StoragePool destStoragePool = Mockito.mock(StoragePool.class);
354364
Mockito.when(destStoragePool.getId()).thenReturn(0l);
355-
Mockito.when(destStoragePool.getPoolType()).thenReturn(storagePoolType); //TODO parameter for generic test method
365+
Mockito.when(destStoragePool.getPoolType()).thenReturn(storagePoolType);
356366

357367
DataStore sourceTemplateDataStore = Mockito.mock(DataStore.class);
358368
Mockito.when(sourceTemplateDataStore.getName()).thenReturn("sourceTemplateName");
@@ -375,8 +385,9 @@ private void configureAndTestmigrateTemplateToTargetStorageIfNeeded(VMTemplateSt
375385
Mockito.when(dataStoreManagerImpl.getImageStore(Mockito.anyLong())).thenReturn(sourceTemplateDataStore);
376386
Mockito.when(templateDataFactory.getTemplate(Mockito.anyLong(), Mockito.eq(sourceTemplateDataStore))).thenReturn(sourceTemplateInfo);
377387
Mockito.when(templateDataFactory.getTemplate(Mockito.anyLong(), Mockito.eq(destDataStore))).thenReturn(sourceTemplateInfo);
378-
379-
kvmNonManagedStorageDataMotionStrategy.migrateTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, destDataStore, destStoragePool, destHost);
388+
kvmNonManagedStorageDataMotionStrategy.copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, srcStoragePool, destDataStore, destStoragePool, destHost);
389+
Mockito.doNothing().when(kvmNonManagedStorageDataMotionStrategy).updateTemplateReferenceIfSuccessfulCopy(Mockito.any(VolumeInfo.class), Mockito.any(StoragePool.class),
390+
Mockito.any(TemplateInfo.class), Mockito.any(DataStore.class));
380391

381392
InOrder verifyInOrder = Mockito.inOrder(vmTemplatePoolDao, dataStoreManagerImpl, templateDataFactory, kvmNonManagedStorageDataMotionStrategy);
382393
verifyInOrder.verify(vmTemplatePoolDao, Mockito.times(1)).findByPoolTemplate(Mockito.anyLong(), Mockito.anyLong());

0 commit comments

Comments
 (0)