diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java index c605b58d7c6..e7284d515a2 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.api.command.user.offering; +import org.apache.cloudstack.api.BaseListProjectAndAccountResourcesCmd; import org.apache.cloudstack.api.response.StoragePoolResponse; import org.apache.cloudstack.api.response.VolumeResponse; import org.apache.cloudstack.api.response.ZoneResponse; @@ -23,7 +24,6 @@ import org.apache.log4j.Logger; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; -import org.apache.cloudstack.api.BaseListDomainResourcesCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.BaseCmd.CommandType; import org.apache.cloudstack.api.response.DiskOfferingResponse; @@ -31,7 +31,7 @@ import org.apache.cloudstack.api.response.ListResponse; @APICommand(name = "listDiskOfferings", description = "Lists all available disk offerings.", responseObject = DiskOfferingResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) -public class ListDiskOfferingsCmd extends BaseListDomainResourcesCmd { +public class ListDiskOfferingsCmd extends BaseListProjectAndAccountResourcesCmd { public static final Logger s_logger = Logger.getLogger(ListDiskOfferingsCmd.class.getName()); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java index f7c99459baa..a9a699ed3ef 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java @@ -16,12 +16,12 @@ // under the License. package org.apache.cloudstack.api.command.user.offering; +import org.apache.cloudstack.api.BaseListProjectAndAccountResourcesCmd; import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.log4j.Logger; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; -import org.apache.cloudstack.api.BaseListDomainResourcesCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.BaseCmd.CommandType; import org.apache.cloudstack.api.response.ListResponse; @@ -30,7 +30,7 @@ import org.apache.cloudstack.api.response.UserVmResponse; @APICommand(name = "listServiceOfferings", description = "Lists all available service offerings.", responseObject = ServiceOfferingResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) -public class ListServiceOfferingsCmd extends BaseListDomainResourcesCmd { +public class ListServiceOfferingsCmd extends BaseListProjectAndAccountResourcesCmd { public static final Logger s_logger = Logger.getLogger(ListServiceOfferingsCmd.class.getName()); diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 7a7e88ef449..98f162697d2 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -3153,6 +3153,8 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q Object keyword = cmd.getKeyword(); Long domainId = cmd.getDomainId(); Boolean isRootAdmin = accountMgr.isRootAdmin(account.getAccountId()); + Long projectId = cmd.getProjectId(); + String accountName = cmd.getAccountName(); Boolean isRecursive = cmd.isRecursive(); Long zoneId = cmd.getZoneId(); Long volumeId = cmd.getVolumeId(); @@ -3246,9 +3248,9 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q // Filter offerings that are not associated with caller's domain // Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet! - Account caller = CallContext.current().getCallingAccount(); - if (caller.getType() != Account.Type.ADMIN) { - Domain callerDomain = _domainDao.findById(caller.getDomainId()); + account = accountMgr.finalizeOwner(account, accountName, domainId, projectId); + if (!Account.Type.ADMIN.equals(account.getType())) { + Domain callerDomain = _domainDao.findById(account.getDomainId()); List domainIds = findRelatedDomainIds(callerDomain, isRecursive); List ids = _diskOfferingDetailsDao.findOfferingIdsByDomainIds(domainIds); @@ -3338,6 +3340,8 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q searchFilter.addOrderBy(ServiceOfferingJoinVO.class, "id", true); Account caller = CallContext.current().getCallingAccount(); + Long projectId = cmd.getProjectId(); + String accountName = cmd.getAccountName(); Object name = cmd.getServiceOfferingName(); Object id = cmd.getId(); Object keyword = cmd.getKeyword(); @@ -3354,6 +3358,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q Boolean encryptRoot = cmd.getEncryptRoot(); String storageType = cmd.getStorageType(); + final Account owner = accountMgr.finalizeOwner(caller, accountName, domainId, projectId); SearchCriteria sc = _srvOfferingJoinDao.createSearchCriteria(); if (!accountMgr.isRootAdmin(caller.getId()) && isSystem) { throw new InvalidParameterValueException("Only ROOT admins can access system's offering"); @@ -3365,8 +3370,8 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q if (domainId != null && !accountMgr.isRootAdmin(caller.getId())) { // check if the user's domain == so's domain || user's domain is a // child of so's domain - if (!isPermissible(caller.getDomainId(), domainId)) { - throw new PermissionDeniedException("The account:" + caller.getAccountName() + " does not fall in the same domain hierarchy as the service offering"); + if (!isPermissible(owner.getDomainId(), domainId)) { + throw new PermissionDeniedException("The account:" + owner.getAccountName() + " does not fall in the same domain hierarchy as the service offering"); } } @@ -3432,16 +3437,16 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q throw new InvalidParameterValueException("Only root admins can access system's offering"); } if (isRecursive) { // domain + all sub-domains - if (caller.getType() == Account.Type.NORMAL) { + if (owner.getType() == Account.Type.NORMAL) { throw new InvalidParameterValueException("Only ROOT admins and Domain admins can list service offerings with isrecursive=true"); } } } else { // for root users - if (caller.getDomainId() != 1 && isSystem) { // NON ROOT admin - throw new InvalidParameterValueException("Non ROOT admins cannot access system's offering"); + if (owner.getDomainId() != 1 && isSystem) { // NON ROOT admin + throw new InvalidParameterValueException("Non ROOT admins cannot access system's offering."); } - if (domainId != null) { + if (domainId != null && accountName == null) { sc.addAnd("domainId", Op.FIND_IN_SET, String.valueOf(domainId)); } } @@ -3527,8 +3532,8 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q // Filter offerings that are not associated with caller's domain // Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet! - if (caller.getType() != Account.Type.ADMIN) { - Domain callerDomain = _domainDao.findById(caller.getDomainId()); + if (owner.getType() != Account.Type.ADMIN) { + Domain callerDomain = _domainDao.findById(owner.getDomainId()); List domainIds = findRelatedDomainIds(callerDomain, isRecursive); List ids = _srvOfferingDetailsDao.findOfferingIdsByDomainIds(domainIds); diff --git a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java index f71f46fdc03..9cb7f4e8aaf 100644 --- a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java +++ b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java @@ -1686,6 +1686,16 @@ StateListener, Configurable { for (VolumeVO toBeCreated : volumesTobeCreated) { s_logger.debug("Checking suitable pools for volume (Id, Type): (" + toBeCreated.getId() + "," + toBeCreated.getVolumeType().name() + ")"); + if (toBeCreated.getState() == Volume.State.Allocated && toBeCreated.getPoolId() != null) { + toBeCreated.setPoolId(null); + if (!_volsDao.update(toBeCreated.getId(), toBeCreated)) { + throw new CloudRuntimeException(String.format("Error updating volume [%s] to clear pool Id.", toBeCreated.getId())); + } + if (s_logger.isDebugEnabled()) { + String msg = String.format("Setting pool_id to NULL for volume id=%s as it is in Allocated state", toBeCreated.getId()); + s_logger.debug(msg); + } + } // If the plan specifies a poolId, it means that this VM's ROOT // volume is ready and the pool should be reused. // In this case, also check if rest of the volumes are ready and can diff --git a/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java b/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java index 2dfd2829ed9..6bfc8fb629d 100644 --- a/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java +++ b/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.deploy; + import com.cloud.agent.AgentManager; import com.cloud.capacity.CapacityManager; import com.cloud.capacity.dao.CapacityDao; @@ -25,6 +26,7 @@ import com.cloud.dc.ClusterDetailsVO; import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; +import com.cloud.dc.HostPodVO; import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; import com.cloud.dc.dao.DedicatedResourceDao; @@ -52,6 +54,7 @@ import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; +import com.cloud.storage.StoragePoolStatus; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; @@ -61,11 +64,14 @@ import com.cloud.storage.dao.GuestOSDao; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDao; +import com.cloud.template.VirtualMachineTemplate; +import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountVO; import com.cloud.user.dao.AccountDao; import com.cloud.utils.Pair; import com.cloud.utils.component.ComponentContext; +import com.cloud.vm.DiskProfile; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachine.Type; @@ -80,7 +86,10 @@ import org.apache.cloudstack.affinity.dao.AffinityGroupDao; import org.apache.cloudstack.affinity.dao.AffinityGroupDomainMapDao; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.engine.cloud.entity.api.db.dao.VMReservationDao; +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.PrimaryDataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.messagebus.MessageBus; @@ -127,6 +136,8 @@ import java.util.Set; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(loader = AnnotationConfigContextLoader.class) @@ -202,6 +213,18 @@ public class DeploymentPlanningManagerImplTest { @Mock ConfigurationDao configDao; + @Mock + AccountManager _accountMgr; + + @Inject + DiskOfferingDao _diskOfferingDao; + + @Mock + DataStoreManager _dataStoreManager; + + @Inject + HostPodDao _podDao; + private static final long dataCenterId = 1L; private static final long instanceId = 123L; private static final long hostId = 0L; @@ -247,6 +270,8 @@ public class DeploymentPlanningManagerImplTest { List planners = new ArrayList(); planners.add(_planner); _dpm.setPlanners(planners); + StoragePoolAllocator allocator = Mockito.mock(StoragePoolAllocator.class); + _dpm.setStoragePoolAllocators(Arrays.asList(allocator)); Mockito.when(host.getId()).thenReturn(hostId); Mockito.doNothing().when(_dpm).avoidDisabledResources(vmProfile, dc, avoids); @@ -255,13 +280,13 @@ public class DeploymentPlanningManagerImplTest { @Test public void dataCenterAvoidTest() throws InsufficientServerCapacityException, AffinityConflictException { ServiceOfferingVO svcOffering = - new ServiceOfferingVO("testOffering", 1, 512, 500, 1, 1, false, false, false, "test dpm", - false, VirtualMachine.Type.User, null, "FirstFitPlanner", true, false); + new ServiceOfferingVO("testOffering", 1, 512, 500, 1, 1, false, false, false, "test dpm", + false, VirtualMachine.Type.User, null, "FirstFitPlanner", true, false); Mockito.when(vmProfile.getServiceOffering()).thenReturn(svcOffering); DataCenterDeployment plan = new DataCenterDeployment(dataCenterId); - Mockito.when(avoids.shouldAvoid((DataCenterVO)Matchers.anyObject())).thenReturn(true); + Mockito.when(avoids.shouldAvoid((DataCenterVO) Matchers.anyObject())).thenReturn(true); DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null); assertNull("DataCenter is in avoid set, destination should be null! ", dest); } @@ -269,12 +294,12 @@ public class DeploymentPlanningManagerImplTest { @Test public void plannerCannotHandleTest() throws InsufficientServerCapacityException, AffinityConflictException { ServiceOfferingVO svcOffering = - new ServiceOfferingVO("testOffering", 1, 512, 500, 1, 1, false, false, false, "test dpm", - false, VirtualMachine.Type.User, null, "UserDispersingPlanner", true, false); + new ServiceOfferingVO("testOffering", 1, 512, 500, 1, 1, false, false, false, "test dpm", + false, VirtualMachine.Type.User, null, "UserDispersingPlanner", true, false); Mockito.when(vmProfile.getServiceOffering()).thenReturn(svcOffering); DataCenterDeployment plan = new DataCenterDeployment(dataCenterId); - Mockito.when(avoids.shouldAvoid((DataCenterVO)Matchers.anyObject())).thenReturn(false); + Mockito.when(avoids.shouldAvoid((DataCenterVO) Matchers.anyObject())).thenReturn(false); Mockito.when(_planner.canHandle(vmProfile, plan, avoids)).thenReturn(false); DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null); @@ -284,15 +309,15 @@ public class DeploymentPlanningManagerImplTest { @Test public void emptyClusterListTest() throws InsufficientServerCapacityException, AffinityConflictException { ServiceOfferingVO svcOffering = - new ServiceOfferingVO("testOffering", 1, 512, 500, 1, 1, false, false, false, "test dpm", - false, VirtualMachine.Type.User, null, "FirstFitPlanner", true, false); + new ServiceOfferingVO("testOffering", 1, 512, 500, 1, 1, false, false, false, "test dpm", + false, VirtualMachine.Type.User, null, "FirstFitPlanner", true, false); Mockito.when(vmProfile.getServiceOffering()).thenReturn(svcOffering); DataCenterDeployment plan = new DataCenterDeployment(dataCenterId); - Mockito.when(avoids.shouldAvoid((DataCenterVO)Matchers.anyObject())).thenReturn(false); + Mockito.when(avoids.shouldAvoid((DataCenterVO) Matchers.anyObject())).thenReturn(false); Mockito.when(_planner.canHandle(vmProfile, plan, avoids)).thenReturn(true); - Mockito.when(((DeploymentClusterPlanner)_planner).orderClusters(vmProfile, plan, avoids)).thenReturn(null); + Mockito.when(((DeploymentClusterPlanner) _planner).orderClusters(vmProfile, plan, avoids)).thenReturn(null); DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null); assertNull("Planner cannot handle, destination should be null! ", dest); } @@ -368,7 +393,8 @@ public class DeploymentPlanningManagerImplTest { } } - private void prepareAndVerifyAvoidDisabledResourcesTest(int timesRouter, int timesAdminVm, int timesDisabledResource, long roleId, Type vmType, boolean isSystemDepolyable, + private void prepareAndVerifyAvoidDisabledResourcesTest(int timesRouter, int timesAdminVm, + int timesDisabledResource, long roleId, Type vmType, boolean isSystemDepolyable, boolean isAdminVmDeployable) { Mockito.doReturn(isSystemDepolyable).when(_dpm).isRouterDeployableInDisabledResources(); Mockito.doReturn(isAdminVmDeployable).when(_dpm).isAdminVmDeployableInDisabledResources(); @@ -481,9 +507,9 @@ public class DeploymentPlanningManagerImplTest { @Test public void volumesRequireEncryptionTest() { - VolumeVO vol1 = new VolumeVO("vol1", dataCenterId,podId,1L,1L, instanceId,"folder","path", Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.ROOT); - VolumeVO vol2 = new VolumeVO("vol2", dataCenterId,podId,1L,1L, instanceId,"folder","path",Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.DATADISK); - VolumeVO vol3 = new VolumeVO("vol3", dataCenterId,podId,1L,1L, instanceId,"folder","path",Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.DATADISK); + VolumeVO vol1 = new VolumeVO("vol1", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.ROOT); + VolumeVO vol2 = new VolumeVO("vol2", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.DATADISK); + VolumeVO vol3 = new VolumeVO("vol3", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.DATADISK); vol2.setPassphraseId(1L); List volumes = List.of(vol1, vol2, vol3); @@ -492,9 +518,9 @@ public class DeploymentPlanningManagerImplTest { @Test public void volumesDoNotRequireEncryptionTest() { - VolumeVO vol1 = new VolumeVO("vol1", dataCenterId,podId,1L,1L, instanceId,"folder","path",Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.ROOT); - VolumeVO vol2 = new VolumeVO("vol2", dataCenterId,podId,1L,1L, instanceId,"folder","path",Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.DATADISK); - VolumeVO vol3 = new VolumeVO("vol3", dataCenterId,podId,1L,1L, instanceId,"folder","path",Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.DATADISK); + VolumeVO vol1 = new VolumeVO("vol1", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.ROOT); + VolumeVO vol2 = new VolumeVO("vol2", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.DATADISK); + VolumeVO vol3 = new VolumeVO("vol3", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.DATADISK); List volumes = List.of(vol1, vol2, vol3); Assert.assertFalse("Volumes do not require encryption, but reporting they do", _dpm.anyVolumeRequiresEncryption(volumes)); @@ -511,7 +537,7 @@ public class DeploymentPlanningManagerImplTest { }}; host.setDetails(hostDetails); - VolumeVO vol1 = new VolumeVO("vol1", dataCenterId,podId,1L,1L, instanceId,"folder","path",Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.ROOT); + VolumeVO vol1 = new VolumeVO("vol1", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.ROOT); vol1.setPassphraseId(1L); setupMocksForPlanDeploymentHostTests(host, vol1); @@ -536,7 +562,7 @@ public class DeploymentPlanningManagerImplTest { }}; host.setDetails(hostDetails); - VolumeVO vol1 = new VolumeVO("vol1", dataCenterId,podId,1L,1L, instanceId,"folder","path",Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.ROOT); + VolumeVO vol1 = new VolumeVO("vol1", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.ROOT); vol1.setPassphraseId(1L); setupMocksForPlanDeploymentHostTests(host, vol1); @@ -561,7 +587,7 @@ public class DeploymentPlanningManagerImplTest { }}; host.setDetails(hostDetails); - VolumeVO vol1 = new VolumeVO("vol1", dataCenterId,podId,1L,1L, instanceId,"folder","path",Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.ROOT); + VolumeVO vol1 = new VolumeVO("vol1", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.ROOT); setupMocksForPlanDeploymentHostTests(host, vol1); @@ -585,7 +611,7 @@ public class DeploymentPlanningManagerImplTest { }}; host.setDetails(hostDetails); - VolumeVO vol1 = new VolumeVO("vol1", dataCenterId,podId,1L,1L, instanceId,"folder","path",Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.ROOT); + VolumeVO vol1 = new VolumeVO("vol1", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.ROOT); setupMocksForPlanDeploymentHostTests(host, vol1); @@ -610,7 +636,7 @@ public class DeploymentPlanningManagerImplTest { host.setDetails(hostDetails); Mockito.when(host.getStatus()).thenReturn(Status.Up); - VolumeVO vol1 = new VolumeVO("vol1", dataCenterId,podId,1L,1L, instanceId,"folder","path",Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.ROOT); + VolumeVO vol1 = new VolumeVO("vol1", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.ROOT); vol1.setPassphraseId(1L); setupMocksForPlanDeploymentHostTests(host, vol1); @@ -640,7 +666,7 @@ public class DeploymentPlanningManagerImplTest { host.setDetails(hostDetails); Mockito.when(host.getStatus()).thenReturn(Status.Up); - VolumeVO vol1 = new VolumeVO("vol1", dataCenterId,podId,1L,1L, instanceId,"folder","path",Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.ROOT); + VolumeVO vol1 = new VolumeVO("vol1", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.ROOT); vol1.setPassphraseId(1L); setupMocksForPlanDeploymentHostTests(host, vol1); @@ -666,7 +692,7 @@ public class DeploymentPlanningManagerImplTest { host.setDetails(hostDetails); Mockito.when(host.getStatus()).thenReturn(Status.Up); - VolumeVO vol1 = new VolumeVO("vol1", dataCenterId,podId,1L,1L, instanceId,"folder","path",Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.ROOT); + VolumeVO vol1 = new VolumeVO("vol1", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.ROOT); vol1.setPassphraseId(1L); DeploymentClusterPlanner planner = setupMocksForPlanDeploymentHostTests(host, vol1); @@ -691,7 +717,7 @@ public class DeploymentPlanningManagerImplTest { host.setDetails(hostDetails); Mockito.when(host.getStatus()).thenReturn(Status.Up); - VolumeVO vol1 = new VolumeVO("vol1", dataCenterId,podId,1L,1L, instanceId,"folder","path",Storage.ProvisioningType.THIN, (long)10<<30, Volume.Type.ROOT); + VolumeVO vol1 = new VolumeVO("vol1", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.ROOT); vol1.setPassphraseId(1L); DeploymentClusterPlanner planner = setupMocksForPlanDeploymentHostTests(host, vol1); @@ -707,6 +733,84 @@ public class DeploymentPlanningManagerImplTest { } } + @Test + public void findSuitablePoolsForVolumesTest() throws Exception { + Long diskOfferingId = 1L; + HostVO host = Mockito.spy(new HostVO("host")); + Map hostDetails = new HashMap<>() { + { + put(Host.HOST_VOLUME_ENCRYPTION, "true"); + } + }; + host.setDetails(hostDetails); + Mockito.when(host.getStatus()).thenReturn(Status.Up); + + VolumeVO vol1 = Mockito.spy(new VolumeVO("vol1", dataCenterId, podId, 1L, 1L, instanceId, "folder", "path", + Storage.ProvisioningType.THIN, (long) 10 << 30, Volume.Type.ROOT)); + Mockito.when(vol1.getId()).thenReturn(1L); + vol1.setState(Volume.State.Allocated); + vol1.setPassphraseId(1L); + vol1.setPoolId(1L); + vol1.setDiskOfferingId(diskOfferingId); + + StoragePoolVO storagePool = new StoragePoolVO(); + storagePool.setStatus(StoragePoolStatus.Maintenance); + storagePool.setId(vol1.getPoolId()); + storagePool.setDataCenterId(dataCenterId); + storagePool.setPodId(podId); + storagePool.setClusterId(clusterId); + + DiskProfile diskProfile = Mockito.mock(DiskProfile.class); + + StoragePoolAllocator allocator = Mockito.mock(StoragePoolAllocator.class); + + DataCenterDeployment plan = new DataCenterDeployment(dataCenterId, podId, clusterId, null, null, null); + + Account account = Mockito.mock(Account.class); + Mockito.when(account.getId()).thenReturn(1L); + Mockito.when(vmProfile.getOwner()).thenReturn(account); + Mockito.when(_accountMgr.isRootAdmin(account.getId())).thenReturn(Boolean.FALSE); + + Mockito.when(_dcDao.findById(dataCenterId)).thenReturn(dc); + Mockito.when(dc.getAllocationState()).thenReturn(AllocationState.Enabled); + + HostPodVO podVo = Mockito.mock(HostPodVO.class); + Mockito.when(podVo.getAllocationState()).thenReturn(AllocationState.Enabled); + Mockito.doReturn(podVo).when(_podDao).findById(podId); + + ClusterVO cluster = Mockito.mock(ClusterVO.class); + Mockito.when(cluster.getAllocationState()).thenReturn(AllocationState.Enabled); + Mockito.when(_clusterDao.findById(clusterId)).thenReturn(cluster); + + DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class); + + Mockito.when(_diskOfferingDao.findById(vol1.getDiskOfferingId())).thenReturn(diskOffering); + VirtualMachineTemplate vmt = Mockito.mock(VirtualMachineTemplate.class); + + ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(vmProfile.getServiceOffering()).thenReturn(serviceOffering); + + PrimaryDataStore primaryDataStore = Mockito.mock(PrimaryDataStore.class); + + Mockito.when(vmt.getFormat()).thenReturn(Storage.ImageFormat.ISO); + Mockito.when(vmProfile.getTemplate()).thenReturn(vmt); + + Mockito.when(vmProfile.getId()).thenReturn(1L); + Mockito.when(vmProfile.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(volDao.findUsableVolumesForInstance(1L)).thenReturn(Arrays.asList(vol1)); + Mockito.when(volDao.findByInstanceAndType(1L, Volume.Type.ROOT)).thenReturn(Arrays.asList(vol1)); + Mockito.when(_dataStoreManager.getPrimaryDataStore(vol1.getPoolId())).thenReturn((DataStore) primaryDataStore); + Mockito.when(avoids.shouldAvoid(storagePool)).thenReturn(Boolean.FALSE); + + Mockito.doReturn(Arrays.asList(storagePool)).when(allocator).allocateToPool(diskProfile, vmProfile, plan, + avoids, 10); + Mockito.when(volDao.update(vol1.getId(), vol1)).thenReturn(true); + _dpm.findSuitablePoolsForVolumes(vmProfile, plan, avoids, 10); + verify(vol1, times(1)).setPoolId(null); + assertTrue(vol1.getPoolId() == null); + + } + // This is so ugly but everything is so intertwined... private DeploymentClusterPlanner setupMocksForPlanDeploymentHostTests(HostVO host, VolumeVO vol1) { long diskOfferingId = 345L; @@ -739,10 +843,10 @@ public class DeploymentPlanningManagerImplTest { Mockito.doNothing().when(hostDao).loadDetails(host); Mockito.doReturn(volumeVOs).when(volDao).findByInstance(ArgumentMatchers.anyLong()); Mockito.doReturn(suitable).when(_dpm).findSuitablePoolsForVolumes( - ArgumentMatchers.any(VirtualMachineProfile.class), - ArgumentMatchers.any(DataCenterDeployment.class), - ArgumentMatchers.any(ExcludeList.class), - ArgumentMatchers.anyInt() + ArgumentMatchers.any(VirtualMachineProfile.class), + ArgumentMatchers.any(DataCenterDeployment.class), + ArgumentMatchers.any(ExcludeList.class), + ArgumentMatchers.anyInt() ); ClusterVO clusterVO = new ClusterVO(); @@ -750,10 +854,10 @@ public class DeploymentPlanningManagerImplTest { Mockito.when(_clusterDao.findById(ArgumentMatchers.anyLong())).thenReturn(clusterVO); Mockito.doReturn(List.of(host)).when(_dpm).findSuitableHosts( - ArgumentMatchers.any(VirtualMachineProfile.class), - ArgumentMatchers.any(DeploymentPlan.class), - ArgumentMatchers.any(ExcludeList.class), - ArgumentMatchers.anyInt() + ArgumentMatchers.any(VirtualMachineProfile.class), + ArgumentMatchers.any(DeploymentPlan.class), + ArgumentMatchers.any(ExcludeList.class), + ArgumentMatchers.anyInt() ); Map suitableVolumeStoragePoolMap = new HashMap<>() {{ @@ -766,13 +870,13 @@ public class DeploymentPlanningManagerImplTest { Mockito.when(capacityMgr.checkIfHostReachMaxGuestLimit(host)).thenReturn(false); Mockito.when(capacityMgr.checkIfHostHasCpuCapability(ArgumentMatchers.anyLong(), ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt())).thenReturn(true); Mockito.when(capacityMgr.checkIfHostHasCapacity( - ArgumentMatchers.anyLong(), - ArgumentMatchers.anyInt(), - ArgumentMatchers.anyLong(), - ArgumentMatchers.anyBoolean(), - ArgumentMatchers.anyFloat(), - ArgumentMatchers.anyFloat(), - ArgumentMatchers.anyBoolean() + ArgumentMatchers.anyLong(), + ArgumentMatchers.anyInt(), + ArgumentMatchers.anyLong(), + ArgumentMatchers.anyBoolean(), + ArgumentMatchers.anyFloat(), + ArgumentMatchers.anyFloat(), + ArgumentMatchers.anyBoolean() )).thenReturn(true); Mockito.when(serviceOfferingDetailsDao.findDetail(vmProfile.getServiceOfferingId(), GPU.Keys.vgpuType.toString())).thenReturn(null); @@ -783,9 +887,9 @@ public class DeploymentPlanningManagerImplTest { DeploymentClusterPlanner planner = Mockito.spy(new FirstFitPlanner()); try { Mockito.doReturn(List.of(clusterId), List.of()).when(planner).orderClusters( - ArgumentMatchers.any(VirtualMachineProfile.class), - ArgumentMatchers.any(DeploymentPlan.class), - ArgumentMatchers.any(ExcludeList.class) + ArgumentMatchers.any(VirtualMachineProfile.class), + ArgumentMatchers.any(DeploymentPlan.class), + ArgumentMatchers.any(ExcludeList.class) ); } catch (Exception ex) { ex.printStackTrace(); @@ -803,7 +907,8 @@ public class DeploymentPlanningManagerImplTest { return dc; } - private void assertAvoidIsEmpty(ExcludeList avoids, boolean isDcEmpty, boolean isPodsEmpty, boolean isClustersEmpty, boolean isHostsEmpty) { + private void assertAvoidIsEmpty(ExcludeList avoids, boolean isDcEmpty, boolean isPodsEmpty, boolean isClustersEmpty, + boolean isHostsEmpty) { Assert.assertEquals(isDcEmpty, CollectionUtils.isEmpty(avoids.getDataCentersToAvoid())); Assert.assertEquals(isPodsEmpty, CollectionUtils.isEmpty(avoids.getPodsToAvoid())); Assert.assertEquals(isClustersEmpty, CollectionUtils.isEmpty(avoids.getClustersToAvoid())); @@ -811,8 +916,9 @@ public class DeploymentPlanningManagerImplTest { } @Configuration - @ComponentScan(basePackageClasses = {DeploymentPlanningManagerImpl.class}, includeFilters = {@Filter(value = TestConfiguration.Library.class, - type = FilterType.CUSTOM)}, useDefaultFilters = false) + @ComponentScan(basePackageClasses = {DeploymentPlanningManagerImpl.class}, + includeFilters = {@Filter(value = TestConfiguration.Library.class, + type = FilterType.CUSTOM)}, useDefaultFilters = false) public static class TestConfiguration extends SpringUtils.CloudStackTestConfiguration { @Bean @@ -1081,7 +1187,8 @@ public class DeploymentPlanningManagerImplTest { Assert.assertEquals(2, hosts.get(7).getId()); } - private List prepareMockForAvoidOtherClustersForDeploymentIfMigrationDisabled(boolean configValue, boolean mockVolumes, boolean mockClusterStoreVolume) { + private List prepareMockForAvoidOtherClustersForDeploymentIfMigrationDisabled(boolean configValue, + boolean mockVolumes, boolean mockClusterStoreVolume) { try { Field f = ConfigKey.class.getDeclaredField("_defaultValue"); f.setAccessible(true); @@ -1136,28 +1243,28 @@ public class DeploymentPlanningManagerImplTest { @Test public void avoidOtherClustersForDeploymentIfMigrationDisabledConfigAllows() { - prepareMockForAvoidOtherClustersForDeploymentIfMigrationDisabled(true,false, false); + prepareMockForAvoidOtherClustersForDeploymentIfMigrationDisabled(true, false, false); Assert.assertTrue(CollectionUtils.isEmpty(runAvoidOtherClustersForDeploymentIfMigrationDisabledTest())); } @Test public void avoidOtherClustersForDeploymentIfMigrationDisabledNoVmVolumes() { - prepareMockForAvoidOtherClustersForDeploymentIfMigrationDisabled(false,false, false); + prepareMockForAvoidOtherClustersForDeploymentIfMigrationDisabled(false, false, false); Assert.assertTrue(CollectionUtils.isEmpty(runAvoidOtherClustersForDeploymentIfMigrationDisabledTest())); } @Test public void avoidOtherClustersForDeploymentIfMigrationDisabledVmVolumesNonValidScope() { - prepareMockForAvoidOtherClustersForDeploymentIfMigrationDisabled(false,true, false); + prepareMockForAvoidOtherClustersForDeploymentIfMigrationDisabled(false, true, false); Assert.assertTrue(CollectionUtils.isEmpty(runAvoidOtherClustersForDeploymentIfMigrationDisabledTest())); } @Test public void avoidOtherClustersForDeploymentIfMigrationDisabledValid() { - List allClusters = prepareMockForAvoidOtherClustersForDeploymentIfMigrationDisabled(false,true, true); + List allClusters = prepareMockForAvoidOtherClustersForDeploymentIfMigrationDisabled(false, true, true); Set avoidedClusters = runAvoidOtherClustersForDeploymentIfMigrationDisabledTest(); Assert.assertTrue(CollectionUtils.isNotEmpty(avoidedClusters)); - Assert.assertEquals(allClusters.size()-1, avoidedClusters.size()); + Assert.assertEquals(allClusters.size() - 1, avoidedClusters.size()); Assert.assertFalse(avoidedClusters.contains(allClusters.get(0))); } } diff --git a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java index 8df824502cd..478547bff1e 100644 --- a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java @@ -21,6 +21,8 @@ import com.cloud.host.Host; import com.cloud.storage.dao.VolumeDao; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.VMInstanceDao; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -127,4 +129,32 @@ public class StorageManagerImplTest { Assert.assertTrue(storageManagerImpl.isVolumeSuspectedDestroyDuplicateOfVmVolume(volume)); } + @Test + public void storagePoolCompatibleWithVolumePoolTestVolumeWithPoolIdInAllocatedState() { + StoragePoolVO storagePool = new StoragePoolVO(); + storagePool.setPoolType(Storage.StoragePoolType.PowerFlex); + storagePool.setId(1L); + VolumeVO volume = new VolumeVO(); + volume.setState(Volume.State.Allocated); + volume.setPoolId(1L); + PrimaryDataStoreDao storagePoolDao = Mockito.mock(PrimaryDataStoreDao.class); + storageManagerImpl._storagePoolDao = storagePoolDao; + Mockito.doReturn(storagePool).when(storagePoolDao).findById(volume.getPoolId()); + Assert.assertFalse(storageManagerImpl.storagePoolCompatibleWithVolumePool(storagePool, volume)); + + } + + @Test + public void storagePoolCompatibleWithVolumePoolTestVolumeWithoutPoolIdInAllocatedState() { + StoragePoolVO storagePool = new StoragePoolVO(); + storagePool.setPoolType(Storage.StoragePoolType.PowerFlex); + storagePool.setId(1L); + VolumeVO volume = new VolumeVO(); + volume.setState(Volume.State.Allocated); + PrimaryDataStoreDao storagePoolDao = Mockito.mock(PrimaryDataStoreDao.class); + storageManagerImpl._storagePoolDao = storagePoolDao; + Assert.assertTrue(storageManagerImpl.storagePoolCompatibleWithVolumePool(storagePool, volume)); + + } + } diff --git a/systemvm/agent/scripts/ssvm-check.sh b/systemvm/agent/scripts/ssvm-check.sh index b2721a93b3f..f5d69cb4548 100644 --- a/systemvm/agent/scripts/ssvm-check.sh +++ b/systemvm/agent/scripts/ssvm-check.sh @@ -103,10 +103,10 @@ else echo "Verifying if we can at least ping the storage" STORAGE_ADDRESSES=`grep "secondaryStorageServerAddress" $CMDLINE | sed -E 's/.*secondaryStorageServerAddress=([^ ]*).*/\1/g'` - if [[ -z "$STORAGE_ADDRESS" ]] + if [[ -z "$STORAGE_ADDRESSES" ]] then STORAGE_NETWORK_GATEWAY=`grep "storagegateway" $CMDLINE | sed -E 's/.*storagegateway=([^ ]*).*/\1/g'` - echo "Storage address is empty, trying to ping storage network gateway instead ($STORAGE_NETWORK_GATEWAY)" + echo "Storage address list is empty, trying to ping storage network gateway instead ($STORAGE_NETWORK_GATEWAY)" ping -c 2 $STORAGE_NETWORK_GATEWAY if [ $? -eq 0 ] then @@ -118,7 +118,7 @@ else fi else echo "Storage address(s): $STORAGE_ADDRESSES, trying to ping" - STORAGE_ADDRESS_LIST=$(echo $STORAGE_ADDRESSES | tr ",") + STORAGE_ADDRESS_LIST=$(echo $STORAGE_ADDRESSES | tr "," "\n") for STORAGE_ADDRESS in $STORAGE_ADDRESS_LIST do echo "Pinging storage address: $STORAGE_ADDRESS"