Skip to content

Commit ac6b1b3

Browse files
winterhazelFabricio Duarte
winterhazel
and
Fabricio Duarte
authored
Migrate public templates that have URLs on data migration across secondary storages (apache#10364)
Co-authored-by: Fabricio Duarte <[email protected]>
1 parent 6de084c commit ac6b1b3

File tree

8 files changed

+336
-26
lines changed

8 files changed

+336
-26
lines changed

‎api/src/main/java/com/cloud/storage/VMTemplateStorageResourceAssoc.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.cloud.storage;
1818

1919
import java.util.Date;
20+
import java.util.List;
2021

2122
import org.apache.cloudstack.api.InternalIdentity;
2223

@@ -25,6 +26,8 @@ public static enum Status {
2526
UNKNOWN, DOWNLOAD_ERROR, NOT_DOWNLOADED, DOWNLOAD_IN_PROGRESS, DOWNLOADED, ABANDONED, UPLOADED, NOT_UPLOADED, UPLOAD_ERROR, UPLOAD_IN_PROGRESS, CREATING, CREATED, BYPASSED
2627
}
2728

29+
List<Status> PENDING_DOWNLOAD_STATES = List.of(Status.NOT_DOWNLOADED, Status.DOWNLOAD_IN_PROGRESS);
30+
2831
String getInstallPath();
2932

3033
long getTemplateId();

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,7 @@ protected List<DataObject> getAllReadyTemplates(DataStore srcDataStore, Map<Data
208208
List<TemplateInfo> files = new LinkedList<>();
209209
for (TemplateDataStoreVO template : templates) {
210210
VMTemplateVO templateVO = templateDao.findById(template.getTemplateId());
211-
if (template.getState() == ObjectInDataStoreStateMachine.State.Ready && templateVO != null &&
212-
(!templateVO.isPublicTemplate() || (templateVO.isPublicTemplate() && templateVO.getUrl() == null)) &&
213-
templateVO.getHypervisorType() != Hypervisor.HypervisorType.Simulator && templateVO.getParentTemplateId() == null) {
211+
if (shouldMigrateTemplate(template, templateVO)) {
214212
files.add(templateFactory.getTemplate(template.getTemplateId(), srcDataStore));
215213
}
216214
}
@@ -231,6 +229,34 @@ protected List<DataObject> getAllReadyTemplates(DataStore srcDataStore, Map<Data
231229
return getAllReadyTemplates(srcDataStore, childTemplates, templates);
232230
}
233231

232+
/**
233+
* Returns whether a template should be migrated. A template should be migrated if:
234+
* <ol>
235+
* <li>its state is ready, and</li>
236+
* <li>its hypervisor type is not simulator, and</li>
237+
* <li>it is not a child template.</li>
238+
* </ol>
239+
*/
240+
protected boolean shouldMigrateTemplate(TemplateDataStoreVO template, VMTemplateVO templateVO) {
241+
if (template.getState() != State.Ready) {
242+
logger.debug("Template [{}] should not be migrated as it is not ready.", template);
243+
return false;
244+
}
245+
246+
if (templateVO.getHypervisorType() == Hypervisor.HypervisorType.Simulator) {
247+
logger.debug("Template [{}] should not be migrated as its hypervisor type is simulator.", template);
248+
return false;
249+
}
250+
251+
if (templateVO.getParentTemplateId() != null) {
252+
logger.debug("Template [{}] should not be migrated as it has a parent template.", template);
253+
return false;
254+
}
255+
256+
logger.debug("Template [{}] should be migrated.", template);
257+
return true;
258+
}
259+
234260
/** Returns parent snapshots and snapshots that do not have any children; snapshotChains comprises of the snapshot chain info
235261
* for each parent snapshot and the cumulative size of the chain - this is done to ensure that all the snapshots in a chain
236262
* are migrated to the same datastore

‎engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtilityTest.java‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package org.apache.cloudstack.engine.orchestration;
18+
19+
import com.cloud.hypervisor.Hypervisor;
20+
import com.cloud.storage.VMTemplateVO;
21+
import junit.framework.TestCase;
22+
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
23+
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
24+
import org.junit.Assert;
25+
import org.junit.Test;
26+
import org.junit.runner.RunWith;
27+
import org.mockito.Mock;
28+
import org.mockito.Mockito;
29+
import org.mockito.Spy;
30+
import org.mockito.junit.MockitoJUnitRunner;
31+
32+
@RunWith(MockitoJUnitRunner.class)
33+
public class DataMigrationUtilityTest extends TestCase {
34+
35+
@Spy
36+
private DataMigrationUtility dataMigrationUtility;
37+
38+
@Mock
39+
private VMTemplateVO templateVoMock;
40+
41+
@Mock
42+
private TemplateDataStoreVO templateDataStoreVoMock;
43+
44+
private void prepareForShouldMigrateTemplateTests() {
45+
Mockito.when(templateDataStoreVoMock.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Ready);
46+
Mockito.when(templateVoMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM);
47+
Mockito.when(templateVoMock.getParentTemplateId()).thenReturn(null);
48+
}
49+
50+
@Test
51+
public void shouldMigrateTemplateTestReturnsFalseWhenTemplateIsNotReady() {
52+
prepareForShouldMigrateTemplateTests();
53+
Mockito.when(templateDataStoreVoMock.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Migrating);
54+
55+
boolean result = dataMigrationUtility.shouldMigrateTemplate(templateDataStoreVoMock, templateVoMock);
56+
57+
Assert.assertFalse(result);
58+
}
59+
60+
@Test
61+
public void shouldMigrateTemplateTestReturnsFalseWhenHypervisorTypeIsSimulator() {
62+
prepareForShouldMigrateTemplateTests();
63+
Mockito.when(templateVoMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.Simulator);
64+
65+
boolean result = dataMigrationUtility.shouldMigrateTemplate(templateDataStoreVoMock, templateVoMock);
66+
67+
Assert.assertFalse(result);
68+
}
69+
70+
@Test
71+
public void shouldMigrateTemplateTestReturnsFalseWhenTemplateHasParentTemplate() {
72+
prepareForShouldMigrateTemplateTests();
73+
Mockito.when(templateVoMock.getParentTemplateId()).thenReturn(1L);
74+
75+
boolean result = dataMigrationUtility.shouldMigrateTemplate(templateDataStoreVoMock, templateVoMock);
76+
77+
Assert.assertFalse(result);
78+
}
79+
80+
@Test
81+
public void shouldMigrateTemplateTestReturnsTrueWhenTemplateIsReadyAndDoesNotHaveParentTemplateAndHypervisorTypeIsNotSimulator() {
82+
prepareForShouldMigrateTemplateTests();
83+
84+
boolean result = dataMigrationUtility.shouldMigrateTemplate(templateDataStoreVoMock, templateVoMock);
85+
86+
Assert.assertTrue(result);
87+
}
88+
}

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

Lines changed: 76 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424

2525
import javax.inject.Inject;
2626

27+
import com.cloud.storage.VMTemplateStorageResourceAssoc;
28+
import com.cloud.storage.download.DownloadListener;
29+
import com.cloud.utils.exception.CloudRuntimeException;
2730
import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
2831
import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionService;
2932
import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
@@ -118,26 +121,21 @@ public AsyncCallFuture<DataObjectResult> migrateData(DataObject srcDataObject, D
118121
}
119122
} else if (srcDataObject instanceof TemplateInfo && templateChain != null && templateChain.containsKey(srcDataObject)) {
120123
for (TemplateInfo templateInfo : templateChain.get(srcDataObject).first()) {
124+
if (templateIsOnDestination(templateInfo, destDatastore)) {
125+
res.setResult("Template already exists on destination.");
126+
res.setSuccess(true);
127+
logger.debug("Deleting template {} from source data store [{}].", srcDataObject.getTO().toString(),
128+
srcDataObject.getDataStore().getTO().toString());
129+
srcDataObject.getDataStore().delete(srcDataObject);
130+
future.complete(res);
131+
continue;
132+
}
121133
destDataObject = destDatastore.create(templateInfo);
122134
templateInfo.processEvent(ObjectInDataStoreStateMachine.Event.MigrateDataRequested);
123135
destDataObject.processEvent(ObjectInDataStoreStateMachine.Event.MigrateDataRequested);
124136
migrateJob(future, templateInfo, destDataObject, destDatastore);
125137
}
126-
}
127-
else {
128-
// Check if template in destination store, if yes, do not proceed
129-
if (srcDataObject instanceof TemplateInfo) {
130-
logger.debug("Checking if template present at destination");
131-
TemplateDataStoreVO templateStoreVO = templateStoreDao.findByStoreTemplate(destDatastore.getId(), srcDataObject.getId());
132-
if (templateStoreVO != null) {
133-
String msg = "Template already exists in destination store";
134-
logger.debug(msg);
135-
res.setResult(msg);
136-
res.setSuccess(true);
137-
future.complete(res);
138-
return future;
139-
}
140-
}
138+
} else {
141139
destDataObject = destDatastore.create(srcDataObject);
142140
srcDataObject.processEvent(ObjectInDataStoreStateMachine.Event.MigrateDataRequested);
143141
destDataObject.processEvent(ObjectInDataStoreStateMachine.Event.MigrateDataRequested);
@@ -160,6 +158,69 @@ public AsyncCallFuture<DataObjectResult> migrateData(DataObject srcDataObject, D
160158
return future;
161159
}
162160

161+
/**
162+
* Returns a boolean indicating whether a template is ready on the provided data store. If the template is being downloaded,
163+
* waits until the download finishes.
164+
* @param srcDataObject the template.
165+
* @param destDatastore the data store.
166+
*/
167+
protected boolean templateIsOnDestination(DataObject srcDataObject, DataStore destDatastore) {
168+
if (!(srcDataObject instanceof TemplateInfo)) {
169+
return false;
170+
}
171+
172+
String templateAsString = srcDataObject.getTO().toString();
173+
String destDatastoreAsString = destDatastore.getTO().toString();
174+
TemplateDataStoreVO templateStoreVO;
175+
176+
long timer = getTemplateDownloadTimeout();
177+
long msToSleep = 10000L;
178+
int previousDownloadPercentage = -1;
179+
180+
while (true) {
181+
templateStoreVO = templateStoreDao.findByStoreTemplate(destDatastore.getId(), srcDataObject.getId());
182+
if (templateStoreVO == null) {
183+
logger.debug("{} is not present at destination [{}].", templateAsString, destDatastoreAsString);
184+
return false;
185+
}
186+
VMTemplateStorageResourceAssoc.Status downloadState = templateStoreVO.getDownloadState();
187+
if (downloadState == null || !VMTemplateStorageResourceAssoc.PENDING_DOWNLOAD_STATES.contains(downloadState)) {
188+
break;
189+
}
190+
if (previousDownloadPercentage == templateStoreVO.getDownloadPercent()) {
191+
timer -= msToSleep;
192+
} else {
193+
timer = getTemplateDownloadTimeout();
194+
}
195+
if (timer <= 0) {
196+
throw new CloudRuntimeException(String.format("Timeout while waiting for %s to be downloaded to image store [%s]. " +
197+
"The download percentage has not changed for %d milliseconds.", templateAsString, destDatastoreAsString, getTemplateDownloadTimeout()));
198+
}
199+
waitForTemplateDownload(msToSleep, templateAsString, destDatastoreAsString);
200+
}
201+
202+
if (templateStoreVO.getState() == ObjectInDataStoreStateMachine.State.Ready) {
203+
logger.debug("{} already exists on destination [{}].", templateAsString, destDatastoreAsString);
204+
return true;
205+
}
206+
return false;
207+
}
208+
209+
protected long getTemplateDownloadTimeout() {
210+
return DownloadListener.DOWNLOAD_TIMEOUT;
211+
}
212+
213+
protected void waitForTemplateDownload(long msToSleep, String templateAsString, String destDatastoreAsString) {
214+
logger.debug("{} is being downloaded to destination [{}]; we will verify in {} milliseconds if the download has finished.",
215+
templateAsString, destDatastoreAsString, msToSleep);
216+
try {
217+
Thread.sleep(msToSleep);
218+
} catch (InterruptedException e) {
219+
logger.warn("[ignored] interrupted while waiting for template {} download to finish before trying to migrate it to data store [{}].",
220+
templateAsString, destDatastoreAsString);
221+
}
222+
}
223+
163224
protected void migrateJob(AsyncCallFuture<DataObjectResult> future, DataObject srcDataObject, DataObject destDataObject, DataStore destDatastore) throws ExecutionException, InterruptedException {
164225
MigrateDataContext<DataObjectResult> context = new MigrateDataContext<DataObjectResult>(null, future, srcDataObject, destDataObject, destDatastore);
165226
AsyncCallbackDispatcher<SecondaryStorageServiceImpl, CopyCommandResult> caller = AsyncCallbackDispatcher.create(this);

0 commit comments

Comments
 (0)