From 1275db4081aa8b09148909d6e6b65bc377d4992f Mon Sep 17 00:00:00 2001 From: dahn Date: Mon, 3 Jul 2023 11:56:52 +0200 Subject: [PATCH 1/2] UI: Zone wizard fix (#7588) Co-authored-by: Abhishek Kumar --- .../configuration/ConfigurationService.java | 10 ++-- .../vmware/manager/VmwareManagerImpl.java | 49 +++++++++++++++---- .../ConfigurationManagerImpl.java | 8 +-- .../cloud/resource/ResourceManagerImpl.java | 2 +- .../views/infra/zone/ZoneWizardLaunchZone.vue | 14 ++++-- 5 files changed, 58 insertions(+), 25 deletions(-) diff --git a/api/src/main/java/com/cloud/configuration/ConfigurationService.java b/api/src/main/java/com/cloud/configuration/ConfigurationService.java index 1b94e94ed52..97d4b42974b 100644 --- a/api/src/main/java/com/cloud/configuration/ConfigurationService.java +++ b/api/src/main/java/com/cloud/configuration/ConfigurationService.java @@ -274,7 +274,7 @@ public interface ConfigurationService { /** * Edits a zone in the database. Will not allow you to edit DNS values if there are VMs in the specified zone. * - * @param UpdateZoneCmd + * @param cmd command object containing the id of the zone to update and relevant attributes * @return Updated zone */ DataCenter editZone(UpdateZoneCmd cmd); @@ -282,8 +282,7 @@ public interface ConfigurationService { /** * Deletes a zone from the database. Will not allow you to delete zones that are being used anywhere in the system. * - * @param userId - * @param zoneId + * @param cmd command object containg the zoneid */ boolean deleteZone(DeleteZoneCmd cmd); @@ -319,13 +318,12 @@ public interface ConfigurationService { Vlan updateVlanAndPublicIpRange(UpdateVlanIpRangeCmd cmd) throws ConcurrentOperationException, ResourceUnavailableException, ResourceAllocationException; /** - * Marks the the account with the default zone-id. + * Marks the account with the default zone-id. * * @param accountName * @param domainId - * @param zoneId + * @param defaultZoneId * @return The new account object - * @throws , */ Account markDefaultZone(String accountName, long domainId, long defaultZoneId); diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java index d9a01203a03..199f96a8539 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java @@ -41,6 +41,7 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; +import javax.persistence.EntityExistsException; import org.apache.cloudstack.api.command.admin.zone.AddVmwareDcCmd; import org.apache.cloudstack.api.command.admin.zone.ImportVsphereStoragePoliciesCmd; @@ -1171,12 +1172,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw // Association of VMware DC to zone is not allowed if zone already has resources added. validateZoneWithResources(zoneId, "add VMware datacenter to zone"); - // Check if DC is already part of zone - // In that case vmware_data_center table should have the DC - vmwareDc = vmwareDcDao.getVmwareDatacenterByGuid(vmwareDcName + "@" + vCenterHost); - if (vmwareDc != null) { - throw new ResourceInUseException("This DC is already part of other CloudStack zone(s). Cannot add this DC to more zones."); - } + checkIfDcIsUsed(vCenterHost, vmwareDcName, zoneId); VmwareContext context = null; DatacenterMO dcMo = null; @@ -1210,11 +1206,9 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw throw new ResourceInUseException("This DC is being managed by other CloudStack deployment. Cannot add this DC to zone."); } - // Add DC to database into vmware_data_center table - vmwareDc = new VmwareDatacenterVO(guid, vmwareDcName, vCenterHost, userName, password); - vmwareDc = vmwareDcDao.persist(vmwareDc); + vmwareDc = createOrUpdateDc(guid, vmwareDcName, vCenterHost, userName, password); - // Map zone with vmware datacenter + // Map zone with vmware datacenter vmwareDcZoneMap = new VmwareDatacenterZoneMapVO(zoneId, vmwareDc.getId()); vmwareDcZoneMap = vmwareDatacenterZoneMapDao.persist(vmwareDcZoneMap); @@ -1243,6 +1237,41 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw return vmwareDc; } + VmwareDatacenterVO createOrUpdateDc(String guid, String name, String host, String user, String password) { + VmwareDatacenterVO vmwareDc = new VmwareDatacenterVO(guid, name, host, user, password); + // Add DC to database into vmware_data_center table + try { + vmwareDc = vmwareDcDao.persist(vmwareDc); + } catch (EntityExistsException e) { + // if that fails just get the record as is + vmwareDc = vmwareDcDao.getVmwareDatacenterByGuid(guid); + // we could now update the `vmwareDC` with the user supplied `password`, `user`, `name` and `host`, + // but let's assume user error for now + } + + return vmwareDc; + } + + /** + * Check if DC is already part of zone + * In that case vmware_data_center table should have the DC and a dc zone mapping should exist + * + * @param vCenterHost + * @param vmwareDcName + * @param zoneId + * @throws ResourceInUseException if the DC can not be used. + */ + private void checkIfDcIsUsed(String vCenterHost, String vmwareDcName, Long zoneId) throws ResourceInUseException { + VmwareDatacenterVO vmwareDc; + vmwareDc = vmwareDcDao.getVmwareDatacenterByGuid(vmwareDcName + "@" + vCenterHost); + if (vmwareDc != null) { + VmwareDatacenterZoneMapVO mapping = vmwareDatacenterZoneMapDao.findByVmwareDcId(vmwareDc.getId()); + if (mapping != null && Long.compare(zoneId, mapping.getZoneId()) == 0) { + 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)); + } + } + } + @Override @ActionEvent(eventType = EventTypes.EVENT_ZONE_EDIT, eventDescription = "updating VMware datacenter") public VmwareDatacenter updateVmwareDatacenter(UpdateVmwareDcCmd cmd) { diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 9255f9275e9..737cdc769f6 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -2367,10 +2367,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati throw new CloudRuntimeException(errorMsg + "there are Secondary storages in this zone"); } - // Check if there are any non-removed VMware datacenters in the zone. - //if (_vmwareDatacenterZoneMapDao.findByZoneId(zoneId) != null) { - // throw new CloudRuntimeException(errorMsg + "there are VMware datacenters in this zone."); - //} + // We could check if there are any non-removed VMware datacenters in the zone. EWe don´t care. + // These can continu to exist as long as the mapping will be gone (see line deleteZone } private void checkZoneParameters(final String zoneName, final String dns1, final String dns2, final String internalDns1, final String internalDns2, final boolean checkForDuplicates, final Long domainId, @@ -2510,6 +2508,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati for (final VlanVO vlan : vlans) { _vlanDao.remove(vlan.getId()); } + // we should actually find the mapping and remove if it exists + // but we don't know about vmware/plugin/hypervisors at this point final boolean success = _zoneDao.remove(zoneId); diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index d552c12e1a7..1909063dfe3 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -546,7 +546,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, } // save cluster details for later cluster/host cross-checking - final Map details = new HashMap(); + final Map details = new HashMap<>(); details.put("url", url); details.put("username", StringUtils.defaultString(username)); details.put("password", StringUtils.defaultString(password)); diff --git a/ui/src/views/infra/zone/ZoneWizardLaunchZone.vue b/ui/src/views/infra/zone/ZoneWizardLaunchZone.vue index 5b53abd3b27..bf43e4bd5c4 100644 --- a/ui/src/views/infra/zone/ZoneWizardLaunchZone.vue +++ b/ui/src/views/infra/zone/ZoneWizardLaunchZone.vue @@ -345,7 +345,7 @@ export default { params.internaldns1 = this.prefillContent?.internalDns1 || null params.internaldns2 = this.prefillContent?.internalDns2 || null params.domain = this.prefillContent?.networkDomain || null - params.isedge = this.prefillContent?.zoneSuperType === 'Edge' || false + params.isedge = this.isEdgeZone try { if (!this.stepData.stepMove.includes('createZone')) { @@ -835,7 +835,10 @@ export default { const params = {} params.zoneId = this.stepData.zoneReturned.id - params.name = this.prefillContent?.podName || this.stepData.zoneReturned.type === 'Edge' ? 'Pod-' + this.stepData.zoneReturned.name : null + params.name = this.prefillContent?.podName || null + if (this.isEdgeZone) { + params.name = 'Pod-' + this.stepData.zoneReturned.name + } params.gateway = this.prefillContent?.podReservedGateway || null params.netmask = this.prefillContent?.podReservedNetmask || null params.startIp = this.prefillContent?.podReservedStartIp || null @@ -1218,7 +1221,10 @@ export default { } params.clustertype = clusterType params.podId = this.stepData.podReturned.id - let clusterName = this.prefillContent.clusterName || this.stepData.zoneReturned.type === 'Edge' ? 'Cluster-' + this.stepData.zoneReturned.name : null + let clusterName = this.prefillContent?.clusterName || null + if (this.isEdgeZone) { + clusterName = 'Cluster-' + this.stepData.zoneReturned.name + } if (hypervisor === 'VMware') { params.username = this.prefillContent?.vCenterUsername || null @@ -2051,7 +2057,7 @@ export default { return new Promise((resolve, reject) => { let message = '' - api('addCluster', args).then(json => { + api('addCluster', args, 'POST').then(json => { const result = json.addclusterresponse.cluster[0] resolve(result) }).catch(error => { From 5e5d194d77ba38fe23475cebd7b38855293e691c Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 4 Jul 2023 13:07:06 +0530 Subject: [PATCH 2/2] server: do not check zone imagestores for directdownload template delete (#7607) Signed-off-by: Abhishek Kumar --- .../template/HypervisorTemplateAdapter.java | 9 ++-- .../HypervisorTemplateAdapterTest.java | 46 +++++++++++++++---- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java index 1ca9ca73036..2204e5a78b0 100644 --- a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java +++ b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java @@ -177,7 +177,10 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { return ans.getTemplateSize(); } - private void checkZoneImageStores(final List zoneIdList) { + protected void checkZoneImageStores(final VMTemplateVO template, final List zoneIdList) { + if (template.isDirectDownload()) { + return; + } if (zoneIdList != null && CollectionUtils.isEmpty(storeMgr.getImageStoresByScope(new ZoneScope(zoneIdList.get(0))))) { throw new InvalidParameterValueException("Failed to find a secondary storage in the specified zone."); } @@ -677,14 +680,14 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { if (template.getTemplateType() == TemplateType.SYSTEM) { throw new InvalidParameterValueException("The DomR template cannot be deleted."); } - checkZoneImageStores(profile.getZoneIdList()); + checkZoneImageStores(profile.getTemplate(), profile.getZoneIdList()); return profile; } @Override public TemplateProfile prepareDelete(DeleteIsoCmd cmd) { TemplateProfile profile = super.prepareDelete(cmd); - checkZoneImageStores(profile.getZoneIdList()); + checkZoneImageStores(profile.getTemplate(), profile.getZoneIdList()); return profile; } } diff --git a/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java b/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java index d8ff3bc354e..c50ca655d37 100644 --- a/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java +++ b/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java @@ -18,6 +18,13 @@ package com.cloud.template; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyLong; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -29,6 +36,7 @@ import java.util.Map; import java.util.concurrent.ExecutionException; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService; @@ -44,7 +52,7 @@ import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO; import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity; import org.junit.Assert; import org.junit.Before; -//import org.junit.Test; +import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; @@ -61,23 +69,17 @@ import com.cloud.event.EventTypes; import com.cloud.event.UsageEventUtils; import com.cloud.event.UsageEventVO; import com.cloud.event.dao.UsageEventDao; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.TemplateProfile; import com.cloud.storage.VMTemplateStorageResourceAssoc.Status; -import com.cloud.storage.dao.VMTemplateZoneDao; import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.dao.VMTemplateZoneDao; import com.cloud.user.AccountVO; import com.cloud.user.ResourceLimitService; import com.cloud.user.dao.AccountDao; import com.cloud.utils.component.ComponentContext; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.anyLong; -import static org.mockito.Mockito.when; -import static org.mockito.Mockito.eq; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.mock; - @RunWith(PowerMockRunner.class) @PrepareForTest(ComponentContext.class) public class HypervisorTemplateAdapterTest { @@ -118,6 +120,9 @@ public class HypervisorTemplateAdapterTest { @Mock ConfigurationDao _configDao; + @Mock + DataStoreManager storeMgr; + @InjectMocks HypervisorTemplateAdapter _adapter; @@ -282,4 +287,27 @@ public class HypervisorTemplateAdapterTest { cleanupUsageUtils(); } + + @Test + public void testCheckZoneImageStoresDirectDownloadTemplate() { + VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class); + Mockito.when(templateVO.isDirectDownload()).thenReturn(true); + _adapter.checkZoneImageStores(templateVO, List.of(1L)); + } + + @Test + public void testCheckZoneImageStoresRegularTemplateWithStore() { + VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class); + Mockito.when(templateVO.isDirectDownload()).thenReturn(false); + Mockito.when(storeMgr.getImageStoresByScope(Mockito.any())).thenReturn(List.of(Mockito.mock(DataStore.class))); + _adapter.checkZoneImageStores(templateVO, List.of(1L)); + } + + @Test(expected = InvalidParameterValueException.class) + public void testCheckZoneImageStoresRegularTemplateNoStore() { + VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class); + Mockito.when(templateVO.isDirectDownload()).thenReturn(false); + Mockito.when(storeMgr.getImageStoresByScope(Mockito.any())).thenReturn(new ArrayList<>()); + _adapter.checkZoneImageStores(templateVO, List.of(1L)); + } }