Skip to content

Commit dd9a443

Browse files
winterhazelwinterhazel
committed
Match download timer on migration with actual download timeout, add tests
1 parent 0620b7f commit dd9a443

File tree

5 files changed

+241
-13
lines changed

5 files changed

+241
-13
lines changed

‎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: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import javax.inject.Inject;
2626

2727
import com.cloud.storage.VMTemplateStorageResourceAssoc;
28+
import com.cloud.storage.download.DownloadListener;
2829
import com.cloud.utils.exception.CloudRuntimeException;
2930
import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
3031
import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionService;
@@ -172,8 +173,8 @@ protected boolean templateIsOnDestination(DataObject srcDataObject, DataStore de
172173
String destDatastoreAsString = destDatastore.getTO().toString();
173174
TemplateDataStoreVO templateStoreVO;
174175

175-
long timer = getTemplateDownloadTimeoutInSeconds();
176-
int secondsToSleep = 10;
176+
long timer = getTemplateDownloadTimeout();
177+
long msToSleep = 10000L;
177178
int previousDownloadPercentage = -1;
178179

179180
while (true) {
@@ -187,15 +188,15 @@ protected boolean templateIsOnDestination(DataObject srcDataObject, DataStore de
187188
break;
188189
}
189190
if (previousDownloadPercentage == templateStoreVO.getDownloadPercent()) {
190-
timer -= secondsToSleep;
191+
timer -= msToSleep;
191192
} else {
192-
timer = getTemplateDownloadTimeoutInSeconds();
193+
timer = getTemplateDownloadTimeout();
193194
}
194195
if (timer <= 0) {
195196
throw new CloudRuntimeException(String.format("Timeout while waiting for %s to be downloaded to image store [%s]. " +
196-
"The download percentage has not changed for %d seconds.", templateAsString, destDatastoreAsString, getTemplateDownloadTimeoutInSeconds()));
197+
"The download percentage has not changed for %d milliseconds.", templateAsString, destDatastoreAsString, getTemplateDownloadTimeout()));
197198
}
198-
waitForTemplateDownload(secondsToSleep, templateAsString, destDatastoreAsString);
199+
waitForTemplateDownload(msToSleep, templateAsString, destDatastoreAsString);
199200
}
200201

201202
if (templateStoreVO.getState() == ObjectInDataStoreStateMachine.State.Ready) {
@@ -205,15 +206,15 @@ protected boolean templateIsOnDestination(DataObject srcDataObject, DataStore de
205206
return false;
206207
}
207208

208-
protected long getTemplateDownloadTimeoutInSeconds() {
209-
return 1800L;
209+
protected long getTemplateDownloadTimeout() {
210+
return DownloadListener.DOWNLOAD_TIMEOUT;
210211
}
211212

212-
protected void waitForTemplateDownload(int secondsToSleep, String templateAsString, String destDatastoreAsString) {
213-
logger.debug("{} is being downloaded to destination [{}]; we will verify in {} seconds if the download has finished.",
214-
templateAsString, destDatastoreAsString, secondsToSleep);
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);
215216
try {
216-
Thread.sleep(secondsToSleep * 1000L);
217+
Thread.sleep(msToSleep);
217218
} catch (InterruptedException e) {
218219
logger.warn("[ignored] interrupted while waiting for template {} download to finish before trying to migrate it to data store [{}].",
219220
templateAsString, destDatastoreAsString);

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

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
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.storage.image;
18+
19+
import com.cloud.agent.api.to.DataStoreTO;
20+
import com.cloud.storage.VMTemplateStorageResourceAssoc;
21+
import com.cloud.utils.exception.CloudRuntimeException;
22+
import junit.framework.TestCase;
23+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
24+
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
25+
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
26+
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
27+
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
28+
import org.apache.cloudstack.storage.to.TemplateObjectTO;
29+
import org.junit.Assert;
30+
import org.junit.Test;
31+
import org.junit.runner.RunWith;
32+
import org.mockito.InjectMocks;
33+
import org.mockito.Mock;
34+
import org.mockito.Mockito;
35+
import org.mockito.Spy;
36+
import org.mockito.junit.MockitoJUnitRunner;
37+
38+
@RunWith(MockitoJUnitRunner.class)
39+
public class SecondaryStorageServiceImplTest extends TestCase {
40+
41+
@Spy
42+
@InjectMocks
43+
private SecondaryStorageServiceImpl secondaryStorageService;
44+
45+
@Mock
46+
TemplateDataStoreDao templateDataStoreDaoMock;
47+
48+
@Mock
49+
TemplateDataStoreVO templateDataStoreVoMock;
50+
51+
@Mock
52+
TemplateInfo templateInfoMock;
53+
54+
@Mock
55+
TemplateObjectTO templateObjectToMock;
56+
57+
@Mock
58+
DataStore dataStoreMock;
59+
60+
@Mock
61+
DataStoreTO dataStoreToMock;
62+
63+
private void prepareForTemplateIsOnDestinationTests() {
64+
long dataStoreId = 1;
65+
long templateId = 2;
66+
67+
Mockito.when(dataStoreMock.getId()).thenReturn(dataStoreId);
68+
Mockito.when(dataStoreMock.getTO()).thenReturn(dataStoreToMock);
69+
Mockito.when(templateInfoMock.getId()).thenReturn(templateId);
70+
Mockito.when(templateInfoMock.getTO()).thenReturn(templateObjectToMock);
71+
Mockito.doReturn(templateDataStoreVoMock).when(templateDataStoreDaoMock).findByStoreTemplate(dataStoreId, templateId);
72+
Mockito.when(templateDataStoreVoMock.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Ready);
73+
}
74+
75+
@Test
76+
public void templateIsOnDestinationTestReturnsFalseWhenTemplateStoreRefDoesNotExist() {
77+
prepareForTemplateIsOnDestinationTests();
78+
Mockito.doReturn(null).when(templateDataStoreDaoMock).findByStoreTemplate(Mockito.anyLong(), Mockito.anyLong());
79+
80+
boolean result = secondaryStorageService.templateIsOnDestination(templateInfoMock, dataStoreMock);
81+
82+
Assert.assertFalse(result);
83+
}
84+
85+
@Test
86+
public void templateIsOnDestinationTestReturnsTrueWhenTemplateIsReady() {
87+
prepareForTemplateIsOnDestinationTests();
88+
89+
boolean result = secondaryStorageService.templateIsOnDestination(templateInfoMock, dataStoreMock);
90+
91+
Assert.assertTrue(result);
92+
}
93+
94+
@Test
95+
public void templateIsOnDestinationTestReturnsFalseWhenTemplateIsNotReady() {
96+
prepareForTemplateIsOnDestinationTests();
97+
Mockito.when(templateDataStoreVoMock.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Creating);
98+
99+
boolean result = secondaryStorageService.templateIsOnDestination(templateInfoMock, dataStoreMock);
100+
101+
Assert.assertFalse(result);
102+
}
103+
104+
@Test
105+
public void templateIsOnDestinationTestReturnsTrueIfTemplateIsDownloadedSuccessfully() {
106+
prepareForTemplateIsOnDestinationTests();
107+
Mockito.when(templateDataStoreVoMock.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.DOWNLOAD_IN_PROGRESS);
108+
Mockito.doAnswer(I -> Mockito.when(templateDataStoreVoMock.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.DOWNLOADED)).when(secondaryStorageService).waitForTemplateDownload(Mockito.anyInt(), Mockito.anyString(), Mockito.anyString());
109+
110+
boolean result = secondaryStorageService.templateIsOnDestination(templateInfoMock, dataStoreMock);
111+
112+
Assert.assertTrue(result);
113+
}
114+
115+
@Test
116+
public void templateIsOnDestinationTestReturnsFalseIfTemplateIsNotDownloadedSuccessfully() {
117+
prepareForTemplateIsOnDestinationTests();
118+
Mockito.when(templateDataStoreVoMock.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.DOWNLOAD_IN_PROGRESS);
119+
Mockito.doAnswer(I -> {
120+
Mockito.when(templateDataStoreVoMock.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.DOWNLOAD_ERROR);
121+
Mockito.when(templateDataStoreVoMock.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Failed);
122+
return "mocked download fail";
123+
}).when(secondaryStorageService).waitForTemplateDownload(Mockito.anyInt(), Mockito.anyString(), Mockito.anyString());
124+
125+
boolean result = secondaryStorageService.templateIsOnDestination(templateInfoMock, dataStoreMock);
126+
127+
Assert.assertFalse(result);
128+
}
129+
130+
@Test(expected = CloudRuntimeException.class)
131+
public void templateIsOnDestinationTestThrowsExceptionIfDownloadTimesOut() {
132+
prepareForTemplateIsOnDestinationTests();
133+
Mockito.when(templateDataStoreVoMock.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.DOWNLOAD_IN_PROGRESS);
134+
Mockito.doReturn(0L).when(secondaryStorageService).getTemplateDownloadTimeout();
135+
136+
secondaryStorageService.templateIsOnDestination(templateInfoMock, dataStoreMock);
137+
}
138+
}

‎server/src/main/java/com/cloud/storage/download/DownloadActiveState.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public String handleTimeout(long updateMs) {
7676
getDownloadListener().log("handleTimeout, updateMs=" + updateMs + ", curr state= " + getName(), Level.TRACE);
7777
}
7878
String newState = getName();
79-
if (updateMs > 5 * DownloadListener.STATUS_POLL_INTERVAL) {
79+
if (updateMs > DownloadListener.DOWNLOAD_TIMEOUT) {
8080
newState = Status.DOWNLOAD_ERROR.toString();
8181
getDownloadListener().log("timeout: transitioning to download error state, currstate=" + getName(), Level.DEBUG);
8282
} else if (updateMs > 3 * DownloadListener.STATUS_POLL_INTERVAL) {

‎server/src/main/java/com/cloud/storage/download/DownloadListener.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ protected void runInContext() {
100100
protected Logger logger = LogManager.getLogger(getClass());
101101
public static final int SMALL_DELAY = 100;
102102
public static final long STATUS_POLL_INTERVAL = 10000L;
103+
public static final long DOWNLOAD_TIMEOUT = 5 * STATUS_POLL_INTERVAL;
103104

104105
public static final String DOWNLOADED = Status.DOWNLOADED.toString();
105106
public static final String NOT_DOWNLOADED = Status.NOT_DOWNLOADED.toString();

0 commit comments

Comments
 (0)