Skip to content

Commit 1c448d3

Browse files
harikrishna-patnalaharikrishna-patnala
committed
Allow copy of templates from secondary storages of other zone when adding a new secondary storage
1 parent e8200a0 commit 1c448d3

File tree

4 files changed

+200
-12
lines changed

4 files changed

+200
-12
lines changed

‎engine/components-api/src/main/java/com/cloud/storage/StorageManager.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,9 @@ public interface StorageManager extends StorageService {
220220
"storage.pool.host.connect.workers", "1",
221221
"Number of worker threads to be used to connect hosts to a primary storage", true);
222222

223-
ConfigKey<Boolean> COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES = new ConfigKey<>(Boolean.class, "copy.public.templates.from.other.storages",
224-
"Storage", "true", "Allow SSVMs to try copying public templates from one secondary storage to another instead of downloading them from the source.",
223+
ConfigKey<Boolean> COPY_TEMPLATES_FROM_OTHER_SECONDARY_STORAGES = new ConfigKey<>(Boolean.class, "copy.templates.from.other.secondary.storages",
224+
"Storage", "true", "Allow templates to be copied from existing Secondary Storage servers (within the same zone or across zones) " +
225+
"when adding a new Secondary Storage, instead of downloading them from the source URL.",
225226
true, ConfigKey.Scope.Zone, null);
226227

227228
/**

‎engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java‎

Lines changed: 92 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131

3232
import javax.inject.Inject;
3333

34+
import org.apache.cloudstack.context.CallContext;
3435
import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
3536
import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
3637
import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
@@ -615,28 +616,110 @@ protected void tryDownloadingTemplateToImageStore(VMTemplateVO tmplt, DataStore
615616
}
616617

617618
protected boolean tryCopyingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) {
618-
Long zoneId = destStore.getScope().getScopeId();
619-
List<DataStore> storesInZone = _storeMgr.getImageStoresByZoneIds(zoneId);
620-
for (DataStore sourceStore : storesInZone) {
619+
Long destZoneId = destStore.getScope().getScopeId();
620+
621+
List<DataStore> storesInSameZone = _storeMgr.getImageStoresByZoneIds(destZoneId);
622+
if (searchAndCopyWithinZone(tmplt, destStore, storesInSameZone)) {
623+
return true;
624+
}
625+
626+
logger.debug("Template [{}] not found in any image store of zone [{}]. Checking other zones",
627+
tmplt.getUniqueName(), destZoneId);
628+
629+
return searchAndCopyAcrossZones(tmplt, destStore, destZoneId);
630+
}
631+
632+
private boolean searchAndCopyAcrossZones(VMTemplateVO tmplt, DataStore destStore, Long destZoneId) {
633+
List<Long> allZoneIds = _dcDao.listAllIds();
634+
for (Long otherZoneId : allZoneIds) {
635+
if (otherZoneId.equals(destZoneId)) {
636+
continue;
637+
}
638+
639+
List<DataStore> storesInOtherZone = _storeMgr.getImageStoresByZoneIds(otherZoneId);
640+
logger.debug("Checking zone [{}] for template [{}]...", otherZoneId, tmplt.getUniqueName());
641+
642+
if (storesInOtherZone == null || storesInOtherZone.isEmpty()) {
643+
logger.debug("Zone [{}] has no image stores. Skipping.", otherZoneId);
644+
continue;
645+
}
646+
647+
DataStore sourceStore = findTemplateInStores(tmplt, storesInOtherZone);
648+
if (sourceStore == null) {
649+
logger.debug("Template [{}] not found in any image store of zone [{}].",
650+
tmplt.getUniqueName(), otherZoneId);
651+
continue;
652+
}
653+
654+
logger.info("Template [{}] found in zone [{}]. Initiating cross-zone copy to zone [{}].",
655+
tmplt.getUniqueName(), otherZoneId, destZoneId);
656+
657+
return copyTemplateAcrossZones(sourceStore, destStore, tmplt);
658+
}
659+
660+
logger.debug("Template [{}] was not found in any zone. Cannot perform zone-to-zone copy.",
661+
tmplt.getUniqueName());
662+
return false;
663+
}
664+
665+
private boolean searchAndCopyWithinZone(VMTemplateVO tmplt, DataStore destStore, List<DataStore> stores) {
666+
for (DataStore sourceStore : stores) {
621667
Map<String, TemplateProp> existingTemplatesInSourceStore = listTemplate(sourceStore);
622-
if (existingTemplatesInSourceStore == null || !existingTemplatesInSourceStore.containsKey(tmplt.getUniqueName())) {
623-
logger.debug("Template [{}] does not exist on image store [{}]; searching on another one.",
668+
if (existingTemplatesInSourceStore == null ||
669+
!existingTemplatesInSourceStore.containsKey(tmplt.getUniqueName())) {
670+
logger.debug("Template [{}] does not exist on image store [{}]; searching another.",
624671
tmplt.getUniqueName(), sourceStore.getName());
625672
continue;
626673
}
674+
627675
TemplateObject sourceTmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), sourceStore);
628676
if (sourceTmpl.getInstallPath() == null) {
629-
logger.warn("Can not copy template [{}] from image store [{}], as it returned a null install path.", tmplt.getUniqueName(),
630-
sourceStore.getName());
677+
logger.warn("Cannot copy template [{}] from image store [{}]; install path is null.",
678+
tmplt.getUniqueName(), sourceStore.getName());
631679
continue;
632680
}
681+
633682
storageOrchestrator.orchestrateTemplateCopyToImageStore(sourceTmpl, destStore);
634683
return true;
635684
}
636-
logger.debug("Can't copy template [{}] from another image store.", tmplt.getUniqueName());
637685
return false;
638686
}
639687

688+
private DataStore findTemplateInStores(VMTemplateVO tmplt, List<DataStore> stores) {
689+
for (DataStore store : stores) {
690+
Map<String, TemplateProp> templates = listTemplate(store);
691+
if (templates != null && templates.containsKey(tmplt.getUniqueName())) {
692+
return store;
693+
}
694+
}
695+
return null;
696+
}
697+
698+
private boolean copyTemplateAcrossZones(DataStore sourceStore,
699+
DataStore destStore,
700+
VMTemplateVO tmplt) {
701+
Long dstZoneId = destStore.getScope().getScopeId();
702+
DataCenterVO dstZone = _dcDao.findById(dstZoneId);
703+
704+
if (dstZone == null) {
705+
logger.warn("Destination zone [{}] not found for template [{}]",
706+
dstZoneId, tmplt.getUniqueName());
707+
return false;
708+
}
709+
710+
try {
711+
Long userId = CallContext.current().getCallingUserId();
712+
return _tmpltMgr.copy(userId, tmplt, sourceStore, dstZone);
713+
} catch (Exception e) {
714+
logger.error("Failed to copy template [{}] from zone [{}] to zone [{}]",
715+
tmplt.getUniqueName(),
716+
sourceStore.getScope().getScopeId(),
717+
dstZoneId,
718+
e);
719+
return false;
720+
}
721+
}
722+
640723
@Override
641724
public AsyncCallFuture<TemplateApiResult> copyTemplateToImageStore(DataObject source, DataStore destStore) {
642725
TemplateObject sourceTmpl = (TemplateObject) source;
@@ -681,7 +764,7 @@ protected Void copyTemplateToImageStoreCallback(AsyncCallbackDispatcher<Template
681764
}
682765

683766
protected boolean isCopyFromOtherStoragesEnabled(Long zoneId) {
684-
return StorageManager.COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES.valueIn(zoneId);
767+
return StorageManager.COPY_TEMPLATES_FROM_OTHER_SECONDARY_STORAGES.valueIn(zoneId);
685768
}
686769

687770
protected void publishTemplateCreation(TemplateInfo tmplt) {

‎engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,12 @@
1818
*/
1919
package org.apache.cloudstack.storage.image;
2020

21+
import com.cloud.dc.DataCenterVO;
22+
import com.cloud.dc.dao.DataCenterDao;
23+
import com.cloud.exception.ResourceAllocationException;
24+
import com.cloud.exception.StorageUnavailableException;
2125
import com.cloud.storage.template.TemplateProp;
26+
import com.cloud.template.TemplateManager;
2227
import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
2328
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
2429
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
@@ -82,6 +87,12 @@ public class TemplateServiceImplTest {
8287
@Mock
8388
StorageOrchestrationService storageOrchestrator;
8489

90+
@Mock
91+
DataCenterDao _dcDao;
92+
93+
@Mock
94+
TemplateManager _tmpltMgr;
95+
8596
Map<String, TemplateProp> templatesInSourceStore = new HashMap<>();
8697

8798
@Before
@@ -167,6 +178,11 @@ public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenInstallPathIsNull(
167178
templatesInSourceStore.put(tmpltMock.getUniqueName(), tmpltPropMock);
168179
Mockito.doReturn(null).when(templateInfoMock).getInstallPath();
169180

181+
Scope scopeMock = Mockito.mock(Scope.class);
182+
Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
183+
Mockito.doReturn(1L).when(scopeMock).getScopeId();
184+
Mockito.doReturn(List.of(1L)).when(_dcDao).listAllIds();
185+
170186
boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
171187

172188
Assert.assertFalse(result);
@@ -183,4 +199,92 @@ public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAno
183199
Assert.assertTrue(result);
184200
Mockito.verify(storageOrchestrator).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any());
185201
}
202+
203+
@Test
204+
public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherZone() throws StorageUnavailableException, ResourceAllocationException {
205+
Scope scopeMock = Mockito.mock(Scope.class);
206+
Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
207+
Mockito.doReturn(1L).when(scopeMock).getScopeId();
208+
Mockito.doReturn(List.of(sourceStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(1L);
209+
Mockito.doReturn(null).when(templateService).listTemplate(sourceStoreMock);
210+
Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds();
211+
212+
DataStore otherZoneStoreMock = Mockito.mock(DataStore.class);
213+
Mockito.doReturn(List.of(otherZoneStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(2L);
214+
215+
Map<String, TemplateProp> templatesInOtherZone = new HashMap<>();
216+
templatesInOtherZone.put(tmpltMock.getUniqueName(), tmpltPropMock);
217+
Mockito.doReturn(templatesInOtherZone).when(templateService).listTemplate(otherZoneStoreMock);
218+
219+
DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class);
220+
Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L);
221+
Mockito.doReturn(true).when(_tmpltMgr).copy(Mockito.anyLong(), Mockito.eq(tmpltMock), Mockito.eq(otherZoneStoreMock), Mockito.eq(dstZoneMock));
222+
223+
boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
224+
225+
Assert.assertTrue(result);
226+
}
227+
228+
@Test
229+
public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenDestinationZoneIsMissing() {
230+
Scope scopeMock = Mockito.mock(Scope.class);
231+
Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
232+
Mockito.doReturn(1L).when(scopeMock).getScopeId();
233+
Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds();
234+
Mockito.doReturn(List.of()).when(dataStoreManagerMock).getImageStoresByZoneIds(1L);
235+
236+
DataStore otherZoneStoreMock = Mockito.mock(DataStore.class);
237+
Mockito.doReturn(List.of(otherZoneStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(2L);
238+
239+
Map<String, TemplateProp> templates = new HashMap<>();
240+
templates.put(tmpltMock.getUniqueName(), tmpltPropMock);
241+
Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock);
242+
Mockito.doReturn(null).when(_dcDao).findById(1L);
243+
244+
boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
245+
246+
Assert.assertFalse(result);
247+
}
248+
249+
@Test
250+
public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenCrossZoneCopyThrowsException() throws StorageUnavailableException, ResourceAllocationException {
251+
Scope scopeMock = Mockito.mock(Scope.class);
252+
Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
253+
Mockito.doReturn(1L).when(scopeMock).getScopeId();
254+
Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds();
255+
Mockito.doReturn(List.of()).when(dataStoreManagerMock).getImageStoresByZoneIds(1L);
256+
257+
DataStore otherZoneStoreMock = Mockito.mock(DataStore.class);
258+
Mockito.doReturn(List.of(otherZoneStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(2L);
259+
260+
Map<String, TemplateProp> templates = new HashMap<>();
261+
templates.put(tmpltMock.getUniqueName(), tmpltPropMock);
262+
Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock);
263+
264+
DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class);
265+
Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L);
266+
Mockito.doThrow(new RuntimeException("copy failed")).when(_tmpltMgr).copy(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
267+
268+
Scope sourceScopeMock = Mockito.mock(Scope.class);
269+
Mockito.doReturn(sourceScopeMock).when(otherZoneStoreMock).getScope();
270+
Mockito.doReturn(2L).when(sourceScopeMock).getScopeId();
271+
272+
boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
273+
274+
Assert.assertFalse(result);
275+
}
276+
277+
@Test
278+
public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenTemplateNotFoundInAnyZone() {
279+
Scope scopeMock = Mockito.mock(Scope.class);
280+
Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
281+
Mockito.doReturn(1L).when(scopeMock).getScopeId();
282+
Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds();
283+
Mockito.doReturn(List.of(sourceStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(Mockito.anyLong());
284+
Mockito.doReturn(null).when(templateService).listTemplate(Mockito.any());
285+
286+
boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
287+
288+
Assert.assertFalse(result);
289+
}
186290
}

‎server/src/main/java/com/cloud/storage/StorageManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4195,7 +4195,7 @@ public ConfigKey<?>[] getConfigKeys() {
41954195
DataStoreDownloadFollowRedirects,
41964196
AllowVolumeReSizeBeyondAllocation,
41974197
StoragePoolHostConnectWorkers,
4198-
COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES
4198+
COPY_TEMPLATES_FROM_OTHER_SECONDARY_STORAGES
41994199
};
42004200
}
42014201

0 commit comments

Comments
 (0)