Skip to content

Commit 3c75d93

Browse files
DaanHooglandDaanHoogland
committed
Merge branch '4.20'
2 parents 55c8115 + d776534 commit 3c75d93

File tree

42 files changed

+821
-255
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+821
-255
lines changed

Diff for: ‎api/src/main/java/com/cloud/storage/VMTemplateStorageResourceAssoc.java

+3
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();

Diff for: ‎api/src/main/java/org/apache/cloudstack/api/command/user/iso/ExtractIsoCmd.java

+10-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.apache.cloudstack.api.command.user.iso;
1818

1919

20+
import com.cloud.dc.DataCenter;
2021
import org.apache.cloudstack.api.APICommand;
2122
import org.apache.cloudstack.api.ApiCommandResourceType;
2223
import org.apache.cloudstack.api.ApiConstants;
@@ -101,7 +102,15 @@ public long getEntityOwnerId() {
101102

102103
@Override
103104
public String getEventDescription() {
104-
return "extracting ISO: " + getId() + " from zone: " + getZoneId();
105+
String isoId = this._uuidMgr.getUuid(VirtualMachineTemplate.class, getId());
106+
String baseDescription = String.format("Extracting ISO: %s", isoId);
107+
108+
Long zoneId = getZoneId();
109+
if (zoneId == null) {
110+
return baseDescription;
111+
}
112+
113+
return String.format("%s from zone: %s", baseDescription, this._uuidMgr.getUuid(DataCenter.class, zoneId));
105114
}
106115

107116
@Override

Diff for: ‎api/src/main/java/org/apache/cloudstack/api/command/user/template/ExtractTemplateCmd.java

+9-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,15 @@ public String getEventType() {
101101

102102
@Override
103103
public String getEventDescription() {
104-
return "extracting template: " + this._uuidMgr.getUuid(VirtualMachineTemplate.class, getId()) + ((getZoneId() != null) ? " from zone: " + this._uuidMgr.getUuid(DataCenter.class, getZoneId()) : "");
104+
String templateId = this._uuidMgr.getUuid(VirtualMachineTemplate.class, getId());
105+
String baseDescription = String.format("Extracting template: %s", templateId);
106+
107+
Long zoneId = getZoneId();
108+
if (zoneId == null) {
109+
return baseDescription;
110+
}
111+
112+
return String.format("%s from zone: %s", baseDescription, this._uuidMgr.getUuid(DataCenter.class, zoneId));
105113
}
106114

107115
@Override

Diff for: ‎api/src/main/java/org/apache/cloudstack/api/command/user/volume/ExtractVolumeCmd.java

+6-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.apache.cloudstack.api.command.user.volume;
1818

1919

20+
import com.cloud.dc.DataCenter;
2021
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
2122
import org.apache.cloudstack.api.ACL;
2223
import org.apache.cloudstack.api.APICommand;
@@ -114,12 +115,15 @@ public String getEventType() {
114115

115116
@Override
116117
public String getEventDescription() {
117-
return "Extraction job";
118+
String volumeId = this._uuidMgr.getUuid(Volume.class, getId());
119+
String zoneId = this._uuidMgr.getUuid(DataCenter.class, getZoneId());
120+
121+
return String.format("Extracting volume: %s from zone: %s", volumeId, zoneId);
118122
}
119123

120124
@Override
121125
public void execute() {
122-
CallContext.current().setEventDetails("Volume Id: " + this._uuidMgr.getUuid(Volume.class, getId()));
126+
CallContext.current().setEventDetails(getEventDescription());
123127
String uploadUrl = _volumeService.extractVolume(this);
124128
if (uploadUrl != null) {
125129
ExtractResponse response = _responseGenerator.createVolumeExtractResponse(id, zoneId, getEntityOwnerId(), mode, uploadUrl);

Diff for: ‎engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -5240,10 +5240,9 @@ public Outcome<VirtualMachine> migrateVmAwayThroughJobQueue(final String vmUuid,
52405240
workJob = newVmWorkJobAndInfo.first();
52415241
VmWorkMigrateAway workInfo = new VmWorkMigrateAway(newVmWorkJobAndInfo.second(), srcHostId);
52425242

5243-
workJob.setCmdInfo(VmWorkSerializer.serialize(workInfo));
5243+
setCmdInfoAndSubmitAsyncJob(workJob, workInfo, vmId);
52445244
}
52455245

5246-
_jobMgr.submitAsyncJob(workJob, VmWorkConstants.VM_WORK_QUEUE, vmId);
52475246

52485247
AsyncJobExecutionContext.getCurrentExecutionContext().joinJob(workJob.getId());
52495248

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

+29-3
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

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

+88
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+
}

Diff for: ‎engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ public void setRemoved(Date removed) {
503503

504504
@Override
505505
public String toString() {
506-
return String.format("VM instance %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "id", "instanceName", "uuid", "type"));
506+
return String.format("VM instance %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "id", "instanceName", "uuid", "type", "state"));
507507
}
508508

509509
@Override

Diff for: ‎engine/schema/src/main/resources/META-INF/db/schema-42000to42010.sql

+22
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,25 @@ CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.storage_pool', 'used_iops', 'bigint
4747

4848
-- Add reason column for op_ha_work
4949
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.op_ha_work', 'reason', 'varchar(32) DEFAULT NULL COMMENT "Reason for the HA work"');
50+
51+
-- Grant access to 2FA APIs for the "Read-Only User - Default" role
52+
53+
CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only User - Default', 'setupUserTwoFactorAuthentication', 'ALLOW');
54+
CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only User - Default', 'validateUserTwoFactorAuthenticationCode', 'ALLOW');
55+
CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only User - Default', 'listUserTwoFactorAuthenticatorProviders', 'ALLOW');
56+
57+
-- Grant access to 2FA APIs for the "Support User - Default" role
58+
59+
CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support User - Default', 'setupUserTwoFactorAuthentication', 'ALLOW');
60+
CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support User - Default', 'validateUserTwoFactorAuthenticationCode', 'ALLOW');
61+
CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support User - Default', 'listUserTwoFactorAuthenticatorProviders', 'ALLOW');
62+
63+
-- Grant access to 2FA APIs for the "Read-Only Admin - Default" role
64+
65+
CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only Admin - Default', 'setupUserTwoFactorAuthentication', 'ALLOW');
66+
CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only Admin - Default', 'validateUserTwoFactorAuthenticationCode', 'ALLOW');
67+
68+
-- Grant access to 2FA APIs for the "Support Admin - Default" role
69+
70+
CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support Admin - Default', 'setupUserTwoFactorAuthentication', 'ALLOW');
71+
CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support Admin - Default', 'validateUserTwoFactorAuthenticationCode', 'ALLOW');

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

+76-15
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)