Skip to content

Commit fbb9dec

Browse files
DaanHooglandDaanHoogland
committed
Merge release branch 4.18 to main
* 4.18: server: do not check zone imagestores for directdownload template delete (#7607) UI: Zone wizard fix (#7588)
2 parents 70d5470 + 5e5d194 commit fbb9dec

7 files changed

Lines changed: 101 additions & 37 deletions

File tree

‎api/src/main/java/com/cloud/configuration/ConfigurationService.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -274,16 +274,15 @@ public interface ConfigurationService {
274274
/**
275275
* Edits a zone in the database. Will not allow you to edit DNS values if there are VMs in the specified zone.
276276
*
277-
* @param UpdateZoneCmd
277+
* @param cmd command object containing the id of the zone to update and relevant attributes
278278
* @return Updated zone
279279
*/
280280
DataCenter editZone(UpdateZoneCmd cmd);
281281

282282
/**
283283
* Deletes a zone from the database. Will not allow you to delete zones that are being used anywhere in the system.
284284
*
285-
* @param userId
286-
* @param zoneId
285+
* @param cmd command object containg the zoneid
287286
*/
288287
boolean deleteZone(DeleteZoneCmd cmd);
289288

@@ -319,13 +318,12 @@ Vlan createVlanAndPublicIpRange(CreateVlanIpRangeCmd cmd) throws InsufficientCap
319318
Vlan updateVlanAndPublicIpRange(UpdateVlanIpRangeCmd cmd) throws ConcurrentOperationException,
320319
ResourceUnavailableException, ResourceAllocationException;
321320
/**
322-
* Marks the the account with the default zone-id.
321+
* Marks the account with the default zone-id.
323322
*
324323
* @param accountName
325324
* @param domainId
326-
* @param zoneId
325+
* @param defaultZoneId
327326
* @return The new account object
328-
* @throws ,
329327
*/
330328
Account markDefaultZone(String accountName, long domainId, long defaultZoneId);
331329

‎plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141

4242
import javax.inject.Inject;
4343
import javax.naming.ConfigurationException;
44+
import javax.persistence.EntityExistsException;
4445

4546
import org.apache.cloudstack.api.command.admin.zone.AddVmwareDcCmd;
4647
import org.apache.cloudstack.api.command.admin.zone.ImportVsphereStoragePoliciesCmd;
@@ -1171,12 +1172,7 @@ public VmwareDatacenterVO addVmwareDatacenter(AddVmwareDcCmd cmd) throws Resourc
11711172
// Association of VMware DC to zone is not allowed if zone already has resources added.
11721173
validateZoneWithResources(zoneId, "add VMware datacenter to zone");
11731174

1174-
// Check if DC is already part of zone
1175-
// In that case vmware_data_center table should have the DC
1176-
vmwareDc = vmwareDcDao.getVmwareDatacenterByGuid(vmwareDcName + "@" + vCenterHost);
1177-
if (vmwareDc != null) {
1178-
throw new ResourceInUseException("This DC is already part of other CloudStack zone(s). Cannot add this DC to more zones.");
1179-
}
1175+
checkIfDcIsUsed(vCenterHost, vmwareDcName, zoneId);
11801176

11811177
VmwareContext context = null;
11821178
DatacenterMO dcMo = null;
@@ -1210,11 +1206,9 @@ public VmwareDatacenterVO addVmwareDatacenter(AddVmwareDcCmd cmd) throws Resourc
12101206
throw new ResourceInUseException("This DC is being managed by other CloudStack deployment. Cannot add this DC to zone.");
12111207
}
12121208

1213-
// Add DC to database into vmware_data_center table
1214-
vmwareDc = new VmwareDatacenterVO(guid, vmwareDcName, vCenterHost, userName, password);
1215-
vmwareDc = vmwareDcDao.persist(vmwareDc);
1209+
vmwareDc = createOrUpdateDc(guid, vmwareDcName, vCenterHost, userName, password);
12161210

1217-
// Map zone with vmware datacenter
1211+
// Map zone with vmware datacenter
12181212
vmwareDcZoneMap = new VmwareDatacenterZoneMapVO(zoneId, vmwareDc.getId());
12191213

12201214
vmwareDcZoneMap = vmwareDatacenterZoneMapDao.persist(vmwareDcZoneMap);
@@ -1243,6 +1237,41 @@ public VmwareDatacenterVO addVmwareDatacenter(AddVmwareDcCmd cmd) throws Resourc
12431237
return vmwareDc;
12441238
}
12451239

1240+
VmwareDatacenterVO createOrUpdateDc(String guid, String name, String host, String user, String password) {
1241+
VmwareDatacenterVO vmwareDc = new VmwareDatacenterVO(guid, name, host, user, password);
1242+
// Add DC to database into vmware_data_center table
1243+
try {
1244+
vmwareDc = vmwareDcDao.persist(vmwareDc);
1245+
} catch (EntityExistsException e) {
1246+
// if that fails just get the record as is
1247+
vmwareDc = vmwareDcDao.getVmwareDatacenterByGuid(guid);
1248+
// we could now update the `vmwareDC` with the user supplied `password`, `user`, `name` and `host`,
1249+
// but let's assume user error for now
1250+
}
1251+
1252+
return vmwareDc;
1253+
}
1254+
1255+
/**
1256+
* Check if DC is already part of zone
1257+
* In that case vmware_data_center table should have the DC and a dc zone mapping should exist
1258+
*
1259+
* @param vCenterHost
1260+
* @param vmwareDcName
1261+
* @param zoneId
1262+
* @throws ResourceInUseException if the DC can not be used.
1263+
*/
1264+
private void checkIfDcIsUsed(String vCenterHost, String vmwareDcName, Long zoneId) throws ResourceInUseException {
1265+
VmwareDatacenterVO vmwareDc;
1266+
vmwareDc = vmwareDcDao.getVmwareDatacenterByGuid(vmwareDcName + "@" + vCenterHost);
1267+
if (vmwareDc != null) {
1268+
VmwareDatacenterZoneMapVO mapping = vmwareDatacenterZoneMapDao.findByVmwareDcId(vmwareDc.getId());
1269+
if (mapping != null && Long.compare(zoneId, mapping.getZoneId()) == 0) {
1270+
throw new ResourceInUseException(String.format("This DC (%s) is already part of other CloudStack zone (%d). Cannot add this DC to more zones.", vmwareDc.getUuid(), zoneId));
1271+
}
1272+
}
1273+
}
1274+
12461275
@Override
12471276
@ActionEvent(eventType = EventTypes.EVENT_ZONE_EDIT, eventDescription = "updating VMware datacenter")
12481277
public VmwareDatacenter updateVmwareDatacenter(UpdateVmwareDcCmd cmd) {

‎server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2369,10 +2369,8 @@ protected void checkIfZoneIsDeletable(final long zoneId) {
23692369
throw new CloudRuntimeException(errorMsg + "there are Secondary storages in this zone");
23702370
}
23712371

2372-
// Check if there are any non-removed VMware datacenters in the zone.
2373-
//if (_vmwareDatacenterZoneMapDao.findByZoneId(zoneId) != null) {
2374-
// throw new CloudRuntimeException(errorMsg + "there are VMware datacenters in this zone.");
2375-
//}
2372+
// We could check if there are any non-removed VMware datacenters in the zone. EWe don´t care.
2373+
// These can continu to exist as long as the mapping will be gone (see line deleteZone
23762374
}
23772375

23782376
private void checkZoneParameters(final String zoneName, final String dns1, final String dns2, final String internalDns1, final String internalDns2, final boolean checkForDuplicates, final Long domainId,
@@ -2512,6 +2510,8 @@ public Boolean doInTransaction(final TransactionStatus status) {
25122510
for (final VlanVO vlan : vlans) {
25132511
_vlanDao.remove(vlan.getId());
25142512
}
2513+
// we should actually find the mapping and remove if it exists
2514+
// but we don't know about vmware/plugin/hypervisors at this point
25152515

25162516
final boolean success = _zoneDao.remove(zoneId);
25172517

‎server/src/main/java/com/cloud/resource/ResourceManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,7 @@ public List<? extends Cluster> discoverCluster(final AddClusterCmd cmd) throws I
552552
}
553553

554554
// save cluster details for later cluster/host cross-checking
555-
final Map<String, String> details = new HashMap<String, String>();
555+
final Map<String, String> details = new HashMap<>();
556556
details.put("url", url);
557557
details.put("username", StringUtils.defaultString(username));
558558
details.put("password", StringUtils.defaultString(password));

‎server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,10 @@ private Long performDirectDownloadUrlValidation(final String format, final Strin
177177
return ans.getTemplateSize();
178178
}
179179

180-
private void checkZoneImageStores(final List<Long> zoneIdList) {
180+
protected void checkZoneImageStores(final VMTemplateVO template, final List<Long> zoneIdList) {
181+
if (template.isDirectDownload()) {
182+
return;
183+
}
181184
if (zoneIdList != null && CollectionUtils.isEmpty(storeMgr.getImageStoresByScope(new ZoneScope(zoneIdList.get(0))))) {
182185
throw new InvalidParameterValueException("Failed to find a secondary storage in the specified zone.");
183186
}
@@ -677,14 +680,14 @@ public TemplateProfile prepareDelete(DeleteTemplateCmd cmd) {
677680
if (template.getTemplateType() == TemplateType.SYSTEM) {
678681
throw new InvalidParameterValueException("The DomR template cannot be deleted.");
679682
}
680-
checkZoneImageStores(profile.getZoneIdList());
683+
checkZoneImageStores(profile.getTemplate(), profile.getZoneIdList());
681684
return profile;
682685
}
683686

684687
@Override
685688
public TemplateProfile prepareDelete(DeleteIsoCmd cmd) {
686689
TemplateProfile profile = super.prepareDelete(cmd);
687-
checkZoneImageStores(profile.getZoneIdList());
690+
checkZoneImageStores(profile.getTemplate(), profile.getZoneIdList());
688691
return profile;
689692
}
690693
}

‎server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@
1818

1919
package com.cloud.template;
2020

21+
import static org.mockito.Mockito.any;
22+
import static org.mockito.Mockito.anyLong;
23+
import static org.mockito.Mockito.doAnswer;
24+
import static org.mockito.Mockito.eq;
25+
import static org.mockito.Mockito.mock;
26+
import static org.mockito.Mockito.when;
27+
2128
import java.lang.reflect.Field;
2229
import java.lang.reflect.InvocationTargetException;
2330
import java.lang.reflect.Method;
@@ -29,6 +36,7 @@
2936
import java.util.concurrent.ExecutionException;
3037

3138
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
39+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
3240
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
3341
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
3442
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService;
@@ -44,7 +52,7 @@
4452
import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
4553
import org.junit.Assert;
4654
import org.junit.Before;
47-
//import org.junit.Test;
55+
import org.junit.Test;
4856
import org.junit.runner.RunWith;
4957
import org.mockito.InjectMocks;
5058
import org.mockito.Mock;
@@ -61,23 +69,17 @@
6169
import com.cloud.event.UsageEventUtils;
6270
import com.cloud.event.UsageEventVO;
6371
import com.cloud.event.dao.UsageEventDao;
72+
import com.cloud.exception.InvalidParameterValueException;
6473
import com.cloud.storage.Storage.ImageFormat;
6574
import com.cloud.storage.TemplateProfile;
6675
import com.cloud.storage.VMTemplateStorageResourceAssoc.Status;
67-
import com.cloud.storage.dao.VMTemplateZoneDao;
6876
import com.cloud.storage.VMTemplateVO;
77+
import com.cloud.storage.dao.VMTemplateZoneDao;
6978
import com.cloud.user.AccountVO;
7079
import com.cloud.user.ResourceLimitService;
7180
import com.cloud.user.dao.AccountDao;
7281
import com.cloud.utils.component.ComponentContext;
7382

74-
import static org.mockito.Mockito.any;
75-
import static org.mockito.Mockito.anyLong;
76-
import static org.mockito.Mockito.when;
77-
import static org.mockito.Mockito.eq;
78-
import static org.mockito.Mockito.doAnswer;
79-
import static org.mockito.Mockito.mock;
80-
8183
@RunWith(PowerMockRunner.class)
8284
@PrepareForTest(ComponentContext.class)
8385
public class HypervisorTemplateAdapterTest {
@@ -118,6 +120,9 @@ public class HypervisorTemplateAdapterTest {
118120
@Mock
119121
ConfigurationDao _configDao;
120122

123+
@Mock
124+
DataStoreManager storeMgr;
125+
121126
@InjectMocks
122127
HypervisorTemplateAdapter _adapter;
123128

@@ -282,4 +287,27 @@ public void testEmitDeleteEventUuid() throws InterruptedException, ExecutionExce
282287

283288
cleanupUsageUtils();
284289
}
290+
291+
@Test
292+
public void testCheckZoneImageStoresDirectDownloadTemplate() {
293+
VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
294+
Mockito.when(templateVO.isDirectDownload()).thenReturn(true);
295+
_adapter.checkZoneImageStores(templateVO, List.of(1L));
296+
}
297+
298+
@Test
299+
public void testCheckZoneImageStoresRegularTemplateWithStore() {
300+
VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
301+
Mockito.when(templateVO.isDirectDownload()).thenReturn(false);
302+
Mockito.when(storeMgr.getImageStoresByScope(Mockito.any())).thenReturn(List.of(Mockito.mock(DataStore.class)));
303+
_adapter.checkZoneImageStores(templateVO, List.of(1L));
304+
}
305+
306+
@Test(expected = InvalidParameterValueException.class)
307+
public void testCheckZoneImageStoresRegularTemplateNoStore() {
308+
VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
309+
Mockito.when(templateVO.isDirectDownload()).thenReturn(false);
310+
Mockito.when(storeMgr.getImageStoresByScope(Mockito.any())).thenReturn(new ArrayList<>());
311+
_adapter.checkZoneImageStores(templateVO, List.of(1L));
312+
}
285313
}

‎ui/src/views/infra/zone/ZoneWizardLaunchZone.vue‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ export default {
345345
params.internaldns1 = this.prefillContent?.internalDns1 || null
346346
params.internaldns2 = this.prefillContent?.internalDns2 || null
347347
params.domain = this.prefillContent?.networkDomain || null
348-
params.isedge = this.prefillContent?.zoneSuperType === 'Edge' || false
348+
params.isedge = this.isEdgeZone
349349
350350
try {
351351
if (!this.stepData.stepMove.includes('createZone')) {
@@ -835,7 +835,10 @@ export default {
835835
836836
const params = {}
837837
params.zoneId = this.stepData.zoneReturned.id
838-
params.name = this.prefillContent?.podName || this.stepData.zoneReturned.type === 'Edge' ? 'Pod-' + this.stepData.zoneReturned.name : null
838+
params.name = this.prefillContent?.podName || null
839+
if (this.isEdgeZone) {
840+
params.name = 'Pod-' + this.stepData.zoneReturned.name
841+
}
839842
params.gateway = this.prefillContent?.podReservedGateway || null
840843
params.netmask = this.prefillContent?.podReservedNetmask || null
841844
params.startIp = this.prefillContent?.podReservedStartIp || null
@@ -1218,7 +1221,10 @@ export default {
12181221
}
12191222
params.clustertype = clusterType
12201223
params.podId = this.stepData.podReturned.id
1221-
let clusterName = this.prefillContent.clusterName || this.stepData.zoneReturned.type === 'Edge' ? 'Cluster-' + this.stepData.zoneReturned.name : null
1224+
let clusterName = this.prefillContent?.clusterName || null
1225+
if (this.isEdgeZone) {
1226+
clusterName = 'Cluster-' + this.stepData.zoneReturned.name
1227+
}
12221228
12231229
if (hypervisor === 'VMware') {
12241230
params.username = this.prefillContent?.vCenterUsername || null
@@ -2051,7 +2057,7 @@ export default {
20512057
return new Promise((resolve, reject) => {
20522058
let message = ''
20532059
2054-
api('addCluster', args).then(json => {
2060+
api('addCluster', args, 'POST').then(json => {
20552061
const result = json.addclusterresponse.cluster[0]
20562062
resolve(result)
20572063
}).catch(error => {

0 commit comments

Comments
 (0)