From 37e3657770745d8c69c1177d63645850c17a6b26 Mon Sep 17 00:00:00 2001 From: Daniel Augusto Veronezi Salvador Date: Fri, 20 Feb 2026 16:20:14 +0100 Subject: [PATCH 01/41] [22.0] resource allocation --- .../com/cloud/user/ResourceLimitService.java | 2 + .../service/NetworkOrchestrationService.java | 2 +- .../orchestration/NetworkOrchestrator.java | 49 +++++++++++++------ .../com/cloud/network/NetworkServiceImpl.java | 2 +- .../resourcelimit/CheckedReservation.java | 32 ++++++++---- .../ResourceLimitManagerImpl.java | 42 ++++++++++------ .../resourcelimit/CheckedReservationTest.java | 17 +------ .../ResourceLimitManagerImplTest.java | 6 ++- .../com/cloud/vpc/MockNetworkManagerImpl.java | 2 +- .../vpc/MockResourceLimitManagerImpl.java | 10 ++++ 10 files changed, 104 insertions(+), 60 deletions(-) diff --git a/api/src/main/java/com/cloud/user/ResourceLimitService.java b/api/src/main/java/com/cloud/user/ResourceLimitService.java index 49b20fe2fef..ddb367b26e3 100644 --- a/api/src/main/java/com/cloud/user/ResourceLimitService.java +++ b/api/src/main/java/com/cloud/user/ResourceLimitService.java @@ -191,6 +191,7 @@ public interface ResourceLimitService { */ public void checkResourceLimit(Account account, ResourceCount.ResourceType type, long... count) throws ResourceAllocationException; public void checkResourceLimitWithTag(Account account, ResourceCount.ResourceType type, String tag, long... count) throws ResourceAllocationException; + public void checkResourceLimitWithTag(Account account, Long domainId, boolean considerSystemAccount, ResourceCount.ResourceType type, String tag, long... count) throws ResourceAllocationException; /** * Gets the count of resources for a resource type and account @@ -294,4 +295,5 @@ public interface ResourceLimitService { void incrementVmGpuResourceCount(long accountId, Boolean display, ServiceOffering serviceOffering, VirtualMachineTemplate template, Long gpu); void decrementVmGpuResourceCount(long accountId, Boolean display, ServiceOffering serviceOffering, VirtualMachineTemplate template, Long gpu); + long recalculateDomainResourceCount(final long domainId, final ResourceType type, String tag); } diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java index 31b08429cc4..d05a0bab32b 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java @@ -310,7 +310,7 @@ public interface NetworkOrchestrationService { void removeDhcpServiceInSubnet(Nic nic); - boolean resourceCountNeedsUpdate(NetworkOffering ntwkOff, ACLType aclType); + boolean isResourceCountUpdateNeeded(NetworkOffering networkOffering); void prepareAllNicsForMigration(VirtualMachineProfile vm, DeployDestination dest); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index ae07c4c7bc4..d4c8186e04a 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -58,6 +58,7 @@ import org.apache.cloudstack.framework.messagebus.PublishScope; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.network.RoutedIpv4Manager; import org.apache.cloudstack.network.dao.NetworkPermissionDao; +import org.apache.cloudstack.reservation.dao.ReservationDao; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.ObjectUtils; @@ -86,6 +87,7 @@ import com.cloud.api.query.dao.DomainRouterJoinDao; import com.cloud.api.query.vo.DomainRouterJoinVO; import com.cloud.bgp.BGPService; import com.cloud.configuration.ConfigurationManager; +import com.cloud.configuration.Resource; import com.cloud.configuration.Resource.ResourceType; import com.cloud.dc.ASNumberVO; import com.cloud.dc.ClusterVO; @@ -214,6 +216,7 @@ import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.offerings.dao.NetworkOfferingDetailsDao; import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; import com.cloud.resource.ResourceManager; +import com.cloud.resourcelimit.CheckedReservation; import com.cloud.server.ManagementServer; import com.cloud.user.Account; import com.cloud.user.ResourceLimitService; @@ -447,6 +450,8 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra ClusterDao clusterDao; @Inject RoutedIpv4Manager routedIpv4Manager; + @Inject + private ReservationDao reservationDao; protected StateMachine2 _stateMachine; ScheduledExecutorService _executor; @@ -2747,12 +2752,6 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra return null; } - final boolean updateResourceCount = resourceCountNeedsUpdate(ntwkOff, aclType); - //check resource limits - if (updateResourceCount) { - _resourceLimitMgr.checkResourceLimit(owner, ResourceType.network, isDisplayNetworkEnabled); - } - // Validate network offering if (ntwkOff.getState() != NetworkOffering.State.Enabled) { // see NetworkOfferingVO @@ -2771,6 +2770,8 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra boolean ipv6 = false; + try (CheckedReservation networkReservation = new CheckedReservation(owner, domainId, Resource.ResourceType.network, null, null, 1L, reservationDao, _resourceLimitMgr)) { + if (StringUtils.isNoneBlank(ip6Gateway, ip6Cidr)) { ipv6 = true; } @@ -3110,8 +3111,8 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra } } - if (updateResourceCount) { - _resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.network, isDisplayNetworkEnabled); + if (isResourceCountUpdateNeeded(ntwkOff)) { + changeAccountResourceCountOrRecalculateDomainResourceCount(owner.getAccountId(), domainId, isDisplayNetworkEnabled, true); } UsageEventUtils.publishNetworkCreation(network); @@ -3122,6 +3123,10 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra CallContext.current().setEventDetails("Network Id: " + network.getId()); CallContext.current().putContextParameter(Network.class, network.getUuid()); return network; + } catch (Exception e) { + logger.error(e); + throw new RuntimeException(e); + } } @Override @@ -3487,9 +3492,8 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra } final NetworkOffering ntwkOff = _entityMgr.findById(NetworkOffering.class, networkFinal.getNetworkOfferingId()); - final boolean updateResourceCount = resourceCountNeedsUpdate(ntwkOff, networkFinal.getAclType()); - if (updateResourceCount) { - _resourceLimitMgr.decrementResourceCount(networkFinal.getAccountId(), ResourceType.network, networkFinal.getDisplayNetwork()); + if (isResourceCountUpdateNeeded(ntwkOff)) { + changeAccountResourceCountOrRecalculateDomainResourceCount(networkFinal.getAccountId(), networkFinal.getDomainId(), networkFinal.getDisplayNetwork(), false); } } return deletedVlans.second(); @@ -3512,6 +3516,23 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra return success; } + /** + * If it is a shared network with {@link ACLType#Domain}, it will belong to account {@link Account#ACCOUNT_ID_SYSTEM} and the resources will be not incremented for the + * domain. Therefore, we force the recalculation of the domain's resource count in this case. Otherwise, it will change the count for the account owner. + * @param incrementAccountResourceCount If true, the account resource count will be incremented by 1; otherwise, it will decremented by 1. + */ + private void changeAccountResourceCountOrRecalculateDomainResourceCount(Long accountId, Long domainId, boolean displayNetwork, boolean incrementAccountResourceCount) { + if (Account.ACCOUNT_ID_SYSTEM == accountId && ObjectUtils.isNotEmpty(domainId)) { + _resourceLimitMgr.recalculateDomainResourceCount(domainId, ResourceType.network, null); + } else { + if (incrementAccountResourceCount) { + _resourceLimitMgr.incrementResourceCount(accountId, ResourceType.network, displayNetwork); + } else { + _resourceLimitMgr.decrementResourceCount(accountId, ResourceType.network, displayNetwork); + } + } + } + private void publishDeletedVlanRanges(List deletedVlanRangeToPublish) { if (CollectionUtils.isNotEmpty(deletedVlanRangeToPublish)) { for (VlanVO vlan : deletedVlanRangeToPublish) { @@ -3521,10 +3542,8 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra } @Override - public boolean resourceCountNeedsUpdate(final NetworkOffering ntwkOff, final ACLType aclType) { - //Update resource count only for Isolated account specific non-system networks - final boolean updateResourceCount = ntwkOff.getGuestType() == GuestType.Isolated && !ntwkOff.isSystemOnly() && aclType == ACLType.Account; - return updateResourceCount; + public boolean isResourceCountUpdateNeeded(NetworkOffering networkOffering) { + return !networkOffering.isSystemOnly(); } protected Pair> deleteVlansInNetwork(final NetworkVO network, final long userId, final Account callerAccount) { diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index 58c56134560..8b5e2c5a5ef 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -3197,7 +3197,7 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C if (displayNetwork != null && displayNetwork != network.getDisplayNetwork()) { // Update resource count if it needs to be updated NetworkOffering networkOffering = _networkOfferingDao.findById(network.getNetworkOfferingId()); - if (_networkMgr.resourceCountNeedsUpdate(networkOffering, network.getAclType())) { + if (_networkMgr.isResourceCountUpdateNeeded(networkOffering)) { _resourceLimitMgr.changeResourceCount(network.getAccountId(), Resource.ResourceType.network, displayNetwork); } diff --git a/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java b/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java index d66e1eb912a..211ca65a9d8 100644 --- a/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java +++ b/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java @@ -49,6 +49,7 @@ public class CheckedReservation implements AutoCloseable { ResourceLimitService resourceLimitService; private final Account account; + private Long domainId; private final ResourceType resourceType; private Long amount; private List reservations; @@ -73,12 +74,12 @@ public class CheckedReservation implements AutoCloseable { this.reservations = null; } - protected void checkLimitAndPersistReservations(Account account, ResourceType resourceType, Long resourceId, List resourceLimitTags, Long amount) throws ResourceAllocationException { + protected void checkLimitAndPersistReservations(Account account, Long domainId, ResourceType resourceType, Long resourceId, List resourceLimitTags, Long amount) throws ResourceAllocationException { try { - checkLimitAndPersistReservation(account, resourceType, resourceId, null, amount); + checkLimitAndPersistReservation(account, domainId, resourceType, resourceId, null, amount); if (CollectionUtils.isNotEmpty(resourceLimitTags)) { for (String tag : resourceLimitTags) { - checkLimitAndPersistReservation(account, resourceType, resourceId, tag, amount); + checkLimitAndPersistReservation(account, domainId, resourceType, resourceId, tag, amount); } } } catch (ResourceAllocationException rae) { @@ -87,11 +88,11 @@ public class CheckedReservation implements AutoCloseable { } } - protected void checkLimitAndPersistReservation(Account account, ResourceType resourceType, Long resourceId, String tag, Long amount) throws ResourceAllocationException { + protected void checkLimitAndPersistReservation(Account account, Long domainId, ResourceType resourceType, Long resourceId, String tag, Long amount) throws ResourceAllocationException { if (amount > 0) { - resourceLimitService.checkResourceLimitWithTag(account, resourceType, tag, amount); + resourceLimitService.checkResourceLimitWithTag(account, domainId, true, resourceType, tag, amount); } - ReservationVO reservationVO = new ReservationVO(account.getAccountId(), account.getDomainId(), resourceType, tag, amount); + ReservationVO reservationVO = new ReservationVO(account.getAccountId(), domainId, resourceType, tag, amount); if (resourceId != null) { reservationVO.setResourceId(resourceId); } @@ -114,9 +115,20 @@ public class CheckedReservation implements AutoCloseable { */ public CheckedReservation(Account account, ResourceType resourceType, Long resourceId, List resourceLimitTags, Long amount, ReservationDao reservationDao, ResourceLimitService resourceLimitService) throws ResourceAllocationException { + this(account, account.getDomainId(), resourceType, resourceId, resourceLimitTags, amount, reservationDao, resourceLimitService); + } + + public CheckedReservation(Account account, Long domainId, ResourceType resourceType, Long resourceId, List resourceLimitTags, Long amount, + ReservationDao reservationDao, ResourceLimitService resourceLimitService) throws ResourceAllocationException { this.reservationDao = reservationDao; this.resourceLimitService = resourceLimitService; this.account = account; + + this.domainId = domainId; + if (domainId == null) { + this.domainId = account.getDomainId(); + } + this.resourceType = resourceType; this.amount = amount; this.reservations = new ArrayList<>(); @@ -127,7 +139,7 @@ public class CheckedReservation implements AutoCloseable { setGlobalLock(); if (quotaLimitLock.lock(TRY_TO_GET_LOCK_TIME)) { try { - checkLimitAndPersistReservations(account, resourceType, resourceId, resourceLimitTags, amount); + checkLimitAndPersistReservations(account, this.domainId, resourceType, resourceId, resourceLimitTags, amount); CallContext.current().putContextParameter(getContextParameterKey(), getIds()); } catch (NullPointerException npe) { throw new CloudRuntimeException("not enough means to check limits", npe); @@ -138,11 +150,11 @@ public class CheckedReservation implements AutoCloseable { throw new ResourceAllocationException(String.format("unable to acquire resource reservation \"%s\"", quotaLimitLock.getName()), resourceType); } } else { - checkLimitAndPersistReservations(account, resourceType, resourceId, resourceLimitTags, amount); + checkLimitAndPersistReservations(account, this.domainId, resourceType, resourceId, resourceLimitTags, amount); } } else { logger.debug("not reserving any amount of resources for {} in domain {}, type: {}, tag: {}", - account.getAccountName(), account.getDomainId(), resourceType, getResourceLimitTagsAsString()); + account.getAccountName(), this.domainId, resourceType, getResourceLimitTagsAsString()); } } @@ -153,7 +165,7 @@ public class CheckedReservation implements AutoCloseable { @NotNull private void setGlobalLock() { - String lockName = String.format("CheckedReservation-%s/%d", account.getDomainId(), resourceType.getOrdinal()); + String lockName = String.format("CheckedReservation-%s/%d", this.domainId, resourceType.getOrdinal()); setQuotaLimitLock(GlobalLock.getInternLock(lockName)); } diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 9a6c8a85f18..c2974dedd5c 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -36,9 +36,6 @@ import java.util.stream.Stream; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.event.ActionEventUtils; -import com.cloud.event.EventTypes; -import com.cloud.utils.Ternary; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.ApiCommandResourceType; import org.apache.cloudstack.api.response.AccountResponse; @@ -86,12 +83,15 @@ import com.cloud.dc.dao.VlanDao; import com.cloud.domain.Domain; import com.cloud.domain.DomainVO; import com.cloud.domain.dao.DomainDao; +import com.cloud.event.ActionEventUtils; +import com.cloud.event.EventTypes; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceAllocationException; import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.NetworkDao; +import com.cloud.network.dao.NetworkDomainDao; import com.cloud.network.vpc.dao.VpcDao; import com.cloud.offering.DiskOffering; import com.cloud.offering.ServiceOffering; @@ -118,6 +118,7 @@ import com.cloud.user.AccountVO; import com.cloud.user.ResourceLimitService; import com.cloud.user.dao.AccountDao; import com.cloud.utils.Pair; +import com.cloud.utils.Ternary; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.concurrency.NamedThreadFactory; import com.cloud.utils.db.DB; @@ -203,6 +204,8 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim DiskOfferingDao diskOfferingDao; @Inject BucketDao bucketDao; + @Inject + private NetworkDomainDao networkDomainDao; protected GenericSearchBuilder templateSizeSearch; protected GenericSearchBuilder snapshotSizeSearch; @@ -514,15 +517,7 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim return max; } - protected void checkDomainResourceLimit(final Account account, final Project project, final ResourceType type, String tag, long numResources) throws ResourceAllocationException { - // check all domains in the account's domain hierarchy - Long domainId; - if (project != null) { - domainId = project.getDomainId(); - } else { - domainId = account.getDomainId(); - } - + protected void checkDomainResourceLimit(Long domainId, final ResourceType type, String tag, long numResources) throws ResourceAllocationException { while (domainId != null) { DomainVO domain = _domainDao.findById(domainId); // no limit check if it is ROOT domain @@ -644,11 +639,16 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim @Override public void checkResourceLimitWithTag(final Account account, final ResourceType type, String tag, long... count) throws ResourceAllocationException { + checkResourceLimitWithTag(account, null, false, type, tag, count); + } + + @Override + public void checkResourceLimitWithTag(final Account account, Long domainId, boolean considerSystemAccount, final ResourceType type, String tag, long... count) throws ResourceAllocationException { final long numResources = ((count.length == 0) ? 1 : count[0]); Project project = null; // Don't place any limits on system or root admin accounts - if (_accountMgr.isRootAdmin(account.getId())) { + if (_accountMgr.isRootAdmin(account.getId()) && !(considerSystemAccount && Account.ACCOUNT_ID_SYSTEM == account.getId())) { return; } @@ -656,6 +656,14 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim project = _projectDao.findByProjectAccountId(account.getId()); } + if (domainId == null) { + if (project != null) { + domainId = project.getDomainId(); + } else { + domainId = account.getDomainId(); + } + } + Long domainIdFinal = domainId; final Project projectFinal = project; Transaction.execute(new TransactionCallbackWithExceptionNoReturn() { @Override @@ -665,7 +673,7 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim // Check account limits checkAccountResourceLimit(account, projectFinal, type, tag, numResources); // check all domains in the account's domain hierarchy - checkDomainResourceLimit(account, projectFinal, type, tag, numResources); + checkDomainResourceLimit(domainIdFinal, type, tag, numResources); } }); } @@ -1201,7 +1209,7 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim * @param type the resource type to do the recalculation for * @return the resulting new resource count */ - protected long recalculateDomainResourceCount(final long domainId, final ResourceType type, String tag) { + public long recalculateDomainResourceCount(final long domainId, final ResourceType type, String tag) { List accounts = _accountDao.findActiveAccountsForDomain(domainId); List childDomains = _domainDao.findImmediateChildrenForParent(domainId); @@ -1242,6 +1250,10 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim newResourceCount += _projectDao.countProjectsForDomain(domainId); } + if (type == ResourceType.network) { + newResourceCount += networkDomainDao.listDomainNetworkMapByDomain(domainId).size(); + } + // TODO make sure that the resource counts are not null for (ResourceCountVO resourceCount : resourceCounts) { if (resourceCount.getResourceOwnerType() == ResourceOwnerType.Domain && resourceCount.getDomainId() == domainId) { diff --git a/server/src/test/java/com/cloud/resourcelimit/CheckedReservationTest.java b/server/src/test/java/com/cloud/resourcelimit/CheckedReservationTest.java index 247647dd010..5e72f204493 100644 --- a/server/src/test/java/com/cloud/resourcelimit/CheckedReservationTest.java +++ b/server/src/test/java/com/cloud/resourcelimit/CheckedReservationTest.java @@ -149,23 +149,10 @@ public class CheckedReservationTest { @Test public void testMultipleReservationsWithOneFailing() { List tags = List.of("abc", "xyz"); - when(account.getAccountId()).thenReturn(1L); - when(account.getDomainId()).thenReturn(4L); Map persistedReservations = new HashMap<>(); - Mockito.when(reservationDao.persist(Mockito.any(ReservationVO.class))).thenAnswer((Answer) invocation -> { - ReservationVO reservationVO = (ReservationVO) invocation.getArguments()[0]; - Long id = (long) (persistedReservations.size() + 1); - ReflectionTestUtils.setField(reservationVO, "id", id); - persistedReservations.put(id, reservationVO); - return reservationVO; - }); - Mockito.when(reservationDao.remove(Mockito.anyLong())).thenAnswer((Answer) invocation -> { - Long id = (Long) invocation.getArguments()[0]; - persistedReservations.remove(id); - return true; - }); + try { - Mockito.doThrow(ResourceAllocationException.class).when(resourceLimitService).checkResourceLimitWithTag(account, Resource.ResourceType.cpu, "xyz", 1L); + Mockito.doThrow(ResourceAllocationException.class).when(resourceLimitService).checkResourceLimitWithTag(account, account.getDomainId(), true, Resource.ResourceType.cpu, "xyz", 1L); try (CheckedReservation vmReservation = new CheckedReservation(account, Resource.ResourceType.user_vm, tags, 1L, reservationDao, resourceLimitService); CheckedReservation cpuReservation = new CheckedReservation(account, Resource.ResourceType.cpu, tags, 1L, reservationDao, resourceLimitService); CheckedReservation memReservation = new CheckedReservation(account, Resource.ResourceType.memory, tags, 256L, reservationDao, resourceLimitService); diff --git a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java index a968a2da0b7..40ba3b477c2 100644 --- a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java +++ b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java @@ -623,10 +623,11 @@ public class ResourceLimitManagerImplTest { public void testCheckResourceLimitWithTagNonAdmin() throws ResourceAllocationException { AccountVO account = Mockito.mock(AccountVO.class); Mockito.when(account.getId()).thenReturn(1L); + Mockito.when(account.getDomainId()).thenReturn(1L); Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(false); Mockito.doReturn(new ArrayList()).when(resourceLimitManager).lockAccountAndOwnerDomainRows(Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyString()); Mockito.doNothing().when(resourceLimitManager).checkAccountResourceLimit(account, null, Resource.ResourceType.cpu, hostTags.get(0), 1); - Mockito.doNothing().when(resourceLimitManager).checkDomainResourceLimit(account, null, Resource.ResourceType.cpu, hostTags.get(0), 1); + Mockito.doNothing().when(resourceLimitManager).checkDomainResourceLimit(1L, Resource.ResourceType.cpu, hostTags.get(0), 1); try { resourceLimitManager.checkResourceLimitWithTag(account, Resource.ResourceType.cpu, hostTags.get(0), 1); } catch (ResourceAllocationException e) { @@ -642,9 +643,10 @@ public class ResourceLimitManagerImplTest { Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(false); ProjectVO projectVO = Mockito.mock(ProjectVO.class); Mockito.when(projectDao.findByProjectAccountId(Mockito.anyLong())).thenReturn(projectVO); + Mockito.when(projectVO.getDomainId()).thenReturn(1L); Mockito.doReturn(new ArrayList()).when(resourceLimitManager).lockAccountAndOwnerDomainRows(Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyString()); Mockito.doNothing().when(resourceLimitManager).checkAccountResourceLimit(account, projectVO, Resource.ResourceType.cpu, hostTags.get(0), 1); - Mockito.doNothing().when(resourceLimitManager).checkDomainResourceLimit(account, projectVO, Resource.ResourceType.cpu, hostTags.get(0), 1); + Mockito.doNothing().when(resourceLimitManager).checkDomainResourceLimit(1L, Resource.ResourceType.cpu, hostTags.get(0), 1); try { resourceLimitManager.checkResourceLimitWithTag(account, Resource.ResourceType.cpu, hostTags.get(0), 1); } catch (ResourceAllocationException e) { diff --git a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java index 93120673720..c429cb1438f 100644 --- a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java @@ -945,7 +945,7 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkOrches } @Override - public boolean resourceCountNeedsUpdate(NetworkOffering ntwkOff, ACLType aclType) { + public boolean isResourceCountUpdateNeeded(NetworkOffering ntwkOff) { return false; //To change body of implemented methods use File | Settings | File Templates. } diff --git a/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java index 151c7ff8908..7ace8a9049e 100644 --- a/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java @@ -237,6 +237,11 @@ public class MockResourceLimitManagerImpl extends ManagerBase implements Resourc } + @Override + public void checkResourceLimitWithTag(Account account, Long domainId, boolean considerSystemAccount, ResourceType type, String tag, long... count) throws ResourceAllocationException { + + } + @Override public List getResourceLimitHostTags() { return null; @@ -396,4 +401,9 @@ public class MockResourceLimitManagerImpl extends ManagerBase implements Resourc public void decrementVmGpuResourceCount(long accountId, Boolean display, ServiceOffering serviceOffering, VirtualMachineTemplate template, Long gpu) { } + + @Override + public long recalculateDomainResourceCount(long domainId, ResourceType type, String tag) { + return 0; + } } From 003c8408179b04af8cc05243e49bc6712729b2fa Mon Sep 17 00:00:00 2001 From: Daniel Augusto Veronezi Salvador Date: Fri, 20 Feb 2026 16:37:24 +0100 Subject: [PATCH 02/41] [22.0] resource instance limits --- .../java/com/cloud/vm/UserVmManagerImpl.java | 281 +++++++++--------- 1 file changed, 142 insertions(+), 139 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 96c87c5376d..f1ea5bc2576 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -60,8 +60,6 @@ import javax.naming.ConfigurationException; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.ParserConfigurationException; -import com.cloud.storage.SnapshotPolicyVO; -import com.cloud.storage.dao.SnapshotPolicyDao; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; @@ -266,12 +264,12 @@ import com.cloud.hypervisor.kvm.dpdk.DpdkHelper; import com.cloud.kubernetes.cluster.KubernetesServiceHelper; import com.cloud.network.IpAddressManager; import com.cloud.network.Network; +import com.cloud.network.NetworkService; import com.cloud.network.Network.GuestType; import com.cloud.network.Network.IpAddresses; import com.cloud.network.Network.Provider; import com.cloud.network.Network.Service; import com.cloud.network.NetworkModel; -import com.cloud.network.NetworkService; import com.cloud.network.Networks.TrafficType; import com.cloud.network.PhysicalNetwork; import com.cloud.network.as.AutoScaleManager; @@ -325,6 +323,7 @@ import com.cloud.storage.GuestOSCategoryVO; import com.cloud.storage.GuestOSVO; import com.cloud.storage.ScopeType; import com.cloud.storage.Snapshot; +import com.cloud.storage.SnapshotPolicyVO; import com.cloud.storage.SnapshotVO; import com.cloud.storage.Storage; import com.cloud.storage.Storage.ImageFormat; @@ -343,6 +342,7 @@ import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.GuestOSCategoryDao; import com.cloud.storage.dao.GuestOSDao; import com.cloud.storage.dao.SnapshotDao; +import com.cloud.storage.dao.SnapshotPolicyDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VMTemplateZoneDao; import com.cloud.storage.dao.VolumeDao; @@ -682,19 +682,19 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir @Inject VnfTemplateManager vnfTemplateManager; - private static final ConfigKey VmIpFetchWaitInterval = new ConfigKey("Advanced", Integer.class, "externaldhcp.vmip.retrieval.interval", "180", + private static final ConfigKey VmIpFetchWaitInterval = new ConfigKey<>("Advanced", Integer.class, "externaldhcp.vmip.retrieval.interval", "180", "Wait Interval (in seconds) for shared network vm dhcp ip addr fetch for next iteration ", true); - private static final ConfigKey VmIpFetchTrialMax = new ConfigKey("Advanced", Integer.class, "externaldhcp.vmip.max.retry", "10", + private static final ConfigKey VmIpFetchTrialMax = new ConfigKey<>("Advanced", Integer.class, "externaldhcp.vmip.max.retry", "10", "The max number of retrieval times for shared network vm dhcp ip fetch, in case of failures", true); - private static final ConfigKey VmIpFetchThreadPoolMax = new ConfigKey("Advanced", Integer.class, "externaldhcp.vmipFetch.threadPool.max", "10", + private static final ConfigKey VmIpFetchThreadPoolMax = new ConfigKey<>("Advanced", Integer.class, "externaldhcp.vmipFetch.threadPool.max", "10", "number of threads for fetching vms ip address", true); - private static final ConfigKey VmIpFetchTaskWorkers = new ConfigKey("Advanced", Integer.class, "externaldhcp.vmipfetchtask.workers", "10", + private static final ConfigKey VmIpFetchTaskWorkers = new ConfigKey<>("Advanced", Integer.class, "externaldhcp.vmipfetchtask.workers", "10", "number of worker threads for vm ip fetch task ", true); - private static final ConfigKey AllowDeployVmIfGivenHostFails = new ConfigKey("Advanced", Boolean.class, "allow.deploy.vm.if.deploy.on.given.host.fails", "false", + private static final ConfigKey AllowDeployVmIfGivenHostFails = new ConfigKey<>("Advanced", Boolean.class, "allow.deploy.vm.if.deploy.on.given.host.fails", "false", "allow vm to deploy on different host if vm fails to deploy on the given host ", true); private static final ConfigKey KvmAdditionalConfigAllowList = new ConfigKey<>(String.class, @@ -706,7 +706,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir private static final ConfigKey VmwareAdditionalConfigAllowList = new ConfigKey<>(String.class, "allow.additional.vm.configuration.list.vmware", "Advanced", "", "Comma separated list of allowed additional configuration options.", true, ConfigKey.Scope.Global, null, null, EnableAdditionalVmConfig.key(), null, null, ConfigKey.Kind.CSV, null); - private static final ConfigKey VmDestroyForcestop = new ConfigKey("Advanced", Boolean.class, "vm.destroy.forcestop", "false", + private static final ConfigKey VmDestroyForcestop = new ConfigKey<>("Advanced", Boolean.class, "vm.destroy.forcestop", "false", "On destroy, force-stop takes this value ", true); @Override @@ -1192,7 +1192,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (dc.getNetworkType() == DataCenter.NetworkType.Advanced) { //List all networks of vm List vmNetworks = _vmNetworkMapDao.getNetworks(vmId); - List routers = new ArrayList(); + List routers = new ArrayList<>(); //List the stopped routers for(long vmNetworkId : vmNetworks) { List router = _routerDao.listStopped(vmNetworkId); @@ -3202,7 +3202,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // Verify that vm's hostName is unique - List vmNtwks = new ArrayList(nics.size()); + List vmNtwks = new ArrayList<>(nics.size()); for (Nic nic : nics) { vmNtwks.add(_networkDao.findById(nic.getNetworkId())); } @@ -3772,7 +3772,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir StorageUnavailableException, ResourceAllocationException { Account caller = CallContext.current().getCallingAccount(); - List networkList = new ArrayList(); + List networkList = new ArrayList<>(); // Verify that caller can perform actions in behalf of vm owner _accountMgr.checkAccess(caller, null, true, owner); @@ -3798,7 +3798,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir //add the default securityGroup only if no security group is specified if (securityGroupIdList == null || securityGroupIdList.isEmpty()) { if (securityGroupIdList == null) { - securityGroupIdList = new ArrayList(); + securityGroupIdList = new ArrayList<>(); } SecurityGroup defaultGroup = _securityGroupMgr.getDefaultSecurityGroup(owner.getId()); if (defaultGroup != null) { @@ -3830,7 +3830,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir Map dataDiskTemplateToDiskOfferingMap, Map userVmOVFProperties, boolean dynamicScalingEnabled, Long overrideDiskOfferingId, String vmType, Volume volume, Snapshot snapshot) throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException, StorageUnavailableException, ResourceAllocationException { Account caller = CallContext.current().getCallingAccount(); - List networkList = new ArrayList(); + List networkList = new ArrayList<>(); boolean isSecurityGroupEnabledNetworkUsed = false; boolean isVmWare = (template.getHypervisorType() == HypervisorType.VMware || (hypervisor != null && hypervisor == HypervisorType.VMware)); @@ -3908,7 +3908,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir //add the default securityGroup only if no security group is specified if (securityGroupIdList == null || securityGroupIdList.isEmpty()) { if (securityGroupIdList == null) { - securityGroupIdList = new ArrayList(); + securityGroupIdList = new ArrayList<>(); } SecurityGroup defaultGroup = _securityGroupMgr.getDefaultSecurityGroup(owner.getId()); @@ -3943,7 +3943,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir StorageUnavailableException, ResourceAllocationException { Account caller = CallContext.current().getCallingAccount(); - List networkList = new ArrayList(); + List networkList = new ArrayList<>(); // Verify that caller can perform actions in behalf of vm owner _accountMgr.checkAccess(caller, null, true, owner); @@ -4898,10 +4898,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir logger.debug("Allocating in the DB for vm"); DataCenterDeployment plan = new DataCenterDeployment(zone.getId()); - List computeTags = new ArrayList(); + List computeTags = new ArrayList<>(); computeTags.add(offering.getHostTag()); - List rootDiskTags = new ArrayList(); + List rootDiskTags = new ArrayList<>(); DiskOfferingVO rootDiskOfferingVO = _diskOfferingDao.findById(rootDiskOfferingId); rootDiskTags.add(rootDiskOfferingVO.getTags()); @@ -5115,7 +5115,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir VirtualMachine.class.getName(), vm.getUuid(), isDisplay); } else { - Map customParameters = new HashMap(); + Map customParameters = new HashMap<>(); customParameters.put(UsageEventVO.DynamicParameters.cpuNumber.name(), serviceOffering.getCpu().toString()); customParameters.put(UsageEventVO.DynamicParameters.cpuSpeed.name(), serviceOffering.getSpeed().toString()); customParameters.put(UsageEventVO.DynamicParameters.memory.name(), serviceOffering.getRamSize().toString()); @@ -5132,7 +5132,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } logger.debug("Collect vm network statistics from host before stopping Vm"); long hostId = userVm.getHostId(); - List vmNames = new ArrayList(); + List vmNames = new ArrayList<>(); vmNames.add(userVm.getInstanceName()); final HostVO host = _hostDao.findById(hostId); Account account = _accountMgr.getAccount(userVm.getAccountId()); @@ -5708,132 +5708,137 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (owner.getState() == Account.State.DISABLED) { throw new PermissionDeniedException(String.format("The owner of %s is disabled: %s", vm, owner)); } - VMTemplateVO template = _templateDao.findByIdIncludingRemoved(vm.getTemplateId()); - if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) { - // check if account/domain is with in resource limits to start a new vm - ServiceOfferingVO offering = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); - resourceLimitService.checkVmResourceLimit(owner, vm.isDisplayVm(), offering, template); - } - // check if vm is security group enabled - if (_securityGroupMgr.isVmSecurityGroupEnabled(vmId) && _securityGroupMgr.getSecurityGroupsForVm(vmId).isEmpty() - && !_securityGroupMgr.isVmMappedToDefaultSecurityGroup(vmId) && _networkModel.canAddDefaultSecurityGroup()) { - // if vm is not mapped to security group, create a mapping - if (logger.isDebugEnabled()) { - logger.debug("Vm " + vm + " is security group enabled, but not mapped to default security group; creating the mapping automatically"); + Pair> vmParamPair; + try (CheckedReservation vmReservation = new CheckedReservation(owner, ResourceType.user_vm, vm.getId(), null, 1L, reservationDao, _resourceLimitMgr)) { + VMTemplateVO template = _templateDao.findByIdIncludingRemoved(vm.getTemplateId()); + if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) { + // check if account/domain is with in resource limits to start a new vm + ServiceOfferingVO offering = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); + resourceLimitService.checkVmResourceLimit(owner, vm.isDisplayVm(), offering, template); } - - SecurityGroup defaultSecurityGroup = _securityGroupMgr.getDefaultSecurityGroup(vm.getAccountId()); - if (defaultSecurityGroup != null) { - List groupList = new ArrayList(); - groupList.add(defaultSecurityGroup.getId()); - _securityGroupMgr.addInstanceToGroups(vm, groupList); - } - } - // Choose deployment planner - // Host takes 1st preference, Cluster takes 2nd preference and Pod takes 3rd - // Default behaviour is invoked when host, cluster or pod are not specified - boolean isRootAdmin = _accountService.isRootAdmin(callerAccount.getId()); - Pod destinationPod = getDestinationPod(podId, isRootAdmin); - Cluster destinationCluster = getDestinationCluster(clusterId, isRootAdmin); - HostVO destinationHost = getDestinationHost(hostId, isRootAdmin, isExplicitHost); - DataCenterDeployment plan = null; - boolean deployOnGivenHost = false; - if (destinationHost != null) { - logger.debug("Destination Host to deploy the VM is specified, specifying a deployment plan to deploy the VM"); - _hostDao.loadHostTags(destinationHost); - validateStrictHostTagCheck(vm, destinationHost); - - final ServiceOfferingVO offering = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); - Pair cpuCapabilityAndCapacity = _capacityMgr.checkIfHostHasCpuCapabilityAndCapacity(destinationHost, offering, false); - if (!cpuCapabilityAndCapacity.first() || !cpuCapabilityAndCapacity.second()) { - String errorMsg; - if (!cpuCapabilityAndCapacity.first()) { - errorMsg = String.format("Cannot deploy the VM to specified host %s, requested CPU and speed is more than the host capability", destinationHost); - } else { - errorMsg = String.format("Cannot deploy the VM to specified host %s, host does not have enough free CPU or RAM, please check the logs", destinationHost); + // check if vm is security group enabled + if (_securityGroupMgr.isVmSecurityGroupEnabled(vmId) && _securityGroupMgr.getSecurityGroupsForVm(vmId).isEmpty() + && !_securityGroupMgr.isVmMappedToDefaultSecurityGroup(vmId) && _networkModel.canAddDefaultSecurityGroup()) { + // if vm is not mapped to security group, create a mapping + if (logger.isDebugEnabled()) { + logger.debug("Vm " + vm + " is security group enabled, but not mapped to default security group; creating the mapping automatically"); } - logger.info(errorMsg); + + SecurityGroup defaultSecurityGroup = _securityGroupMgr.getDefaultSecurityGroup(vm.getAccountId()); + if (defaultSecurityGroup != null) { + List groupList = new ArrayList<>(); + groupList.add(defaultSecurityGroup.getId()); + _securityGroupMgr.addInstanceToGroups(vm, groupList); + } + } + // Choose deployment planner + // Host takes 1st preference, Cluster takes 2nd preference and Pod takes 3rd + // Default behaviour is invoked when host, cluster or pod are not specified + boolean isRootAdmin = _accountService.isRootAdmin(callerAccount.getId()); + Pod destinationPod = getDestinationPod(podId, isRootAdmin); + Cluster destinationCluster = getDestinationCluster(clusterId, isRootAdmin); + HostVO destinationHost = getDestinationHost(hostId, isRootAdmin, isExplicitHost); + DataCenterDeployment plan = null; + boolean deployOnGivenHost = false; + if (destinationHost != null) { + logger.debug("Destination Host to deploy the VM is specified, specifying a deployment plan to deploy the VM"); + _hostDao.loadHostTags(destinationHost); + validateStrictHostTagCheck(vm, destinationHost); + + final ServiceOfferingVO offering = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); + Pair cpuCapabilityAndCapacity = _capacityMgr.checkIfHostHasCpuCapabilityAndCapacity(destinationHost, offering, false); + if (!cpuCapabilityAndCapacity.first() || !cpuCapabilityAndCapacity.second()) { + String errorMsg; + if (!cpuCapabilityAndCapacity.first()) { + errorMsg = String.format("Cannot deploy the VM to specified host %s, requested CPU and speed is more than the host capability", destinationHost); + } else { + errorMsg = String.format("Cannot deploy the VM to specified host %s, host does not have enough free CPU or RAM, please check the logs", destinationHost); + } + logger.info(errorMsg); + if (!AllowDeployVmIfGivenHostFails.value()) { + throw new InvalidParameterValueException(errorMsg); + } + } else { + plan = new DataCenterDeployment(vm.getDataCenterId(), destinationHost.getPodId(), destinationHost.getClusterId(), destinationHost.getId(), null, null); + if (!AllowDeployVmIfGivenHostFails.value()) { + deployOnGivenHost = true; + } + } + } else if (destinationCluster != null) { + logger.debug("Destination Cluster to deploy the VM is specified, specifying a deployment plan to deploy the VM"); + plan = new DataCenterDeployment(vm.getDataCenterId(), destinationCluster.getPodId(), destinationCluster.getId(), null, null, null); if (!AllowDeployVmIfGivenHostFails.value()) { - throw new InvalidParameterValueException(errorMsg); - }; - } else { - plan = new DataCenterDeployment(vm.getDataCenterId(), destinationHost.getPodId(), destinationHost.getClusterId(), destinationHost.getId(), null, null); + deployOnGivenHost = true; + } + } else if (destinationPod != null) { + logger.debug("Destination Pod to deploy the VM is specified, specifying a deployment plan to deploy the VM"); + plan = new DataCenterDeployment(vm.getDataCenterId(), destinationPod.getId(), null, null, null, null); if (!AllowDeployVmIfGivenHostFails.value()) { deployOnGivenHost = true; } } - } else if (destinationCluster != null) { - logger.debug("Destination Cluster to deploy the VM is specified, specifying a deployment plan to deploy the VM"); - plan = new DataCenterDeployment(vm.getDataCenterId(), destinationCluster.getPodId(), destinationCluster.getId(), null, null, null); - if (!AllowDeployVmIfGivenHostFails.value()) { - deployOnGivenHost = true; - } - } else if (destinationPod != null) { - logger.debug("Destination Pod to deploy the VM is specified, specifying a deployment plan to deploy the VM"); - plan = new DataCenterDeployment(vm.getDataCenterId(), destinationPod.getId(), null, null, null, null); - if (!AllowDeployVmIfGivenHostFails.value()) { - deployOnGivenHost = true; - } - } - // Set parameters - Map params = null; - if (vm.isUpdateParameters()) { - _vmDao.loadDetails(vm); + // Set parameters + Map params = null; + if (vm.isUpdateParameters()) { + _vmDao.loadDetails(vm); - String password = getCurrentVmPasswordOrDefineNewPassword(String.valueOf(additionalParams.getOrDefault(VirtualMachineProfile.Param.VmPassword, "")), vm, template); + String password = getCurrentVmPasswordOrDefineNewPassword(String.valueOf(additionalParams.getOrDefault(VirtualMachineProfile.Param.VmPassword, "")), vm, template); - if (!validPassword(password)) { - throw new InvalidParameterValueException("A valid password for this virtual machine was not provided."); - } - - // Check if an SSH key pair was selected for the instance and if so - // use it to encrypt & save the vm password - encryptAndStorePassword(vm, password); - - params = createParameterInParameterMap(params, additionalParams, VirtualMachineProfile.Param.VmPassword, password); - } - - if(additionalParams.containsKey(VirtualMachineProfile.Param.BootIntoSetup)) { - if (! HypervisorType.VMware.equals(vm.getHypervisorType())) { - throw new InvalidParameterValueException(ApiConstants.BOOT_INTO_SETUP + " makes no sense for " + vm.getHypervisorType()); - } - Object paramValue = additionalParams.get(VirtualMachineProfile.Param.BootIntoSetup); - if (logger.isTraceEnabled()) { - logger.trace("It was specified whether to enter setup mode: " + paramValue.toString()); - } - params = createParameterInParameterMap(params, additionalParams, VirtualMachineProfile.Param.BootIntoSetup, paramValue); - } - - VirtualMachineEntity vmEntity = _orchSrvc.getVirtualMachine(vm.getUuid()); - - DeploymentPlanner planner = null; - if (deploymentPlannerToUse != null) { - // if set to null, the deployment planner would be later figured out either from global config var, or from - // the service offering - planner = _planningMgr.getDeploymentPlannerByName(deploymentPlannerToUse); - if (planner == null) { - throw new InvalidParameterValueException("Can't find a planner by name " + deploymentPlannerToUse); - } - } - vmEntity.setParamsToEntity(additionalParams); - - String reservationId = vmEntity.reserve(planner, plan, new ExcludeList(), Long.toString(callerUser.getId())); - vmEntity.deploy(reservationId, Long.toString(callerUser.getId()), params, deployOnGivenHost); - - Pair> vmParamPair = new Pair(vm, params); - if (vm != null && vm.isUpdateParameters()) { - // this value is not being sent to the backend; need only for api - // display purposes - if (template.isEnablePassword()) { - if (vm.getDetail(VmDetailConstants.PASSWORD) != null) { - vmInstanceDetailsDao.removeDetail(vm.getId(), VmDetailConstants.PASSWORD); + if (!validPassword(password)) { + throw new InvalidParameterValueException("A valid password for this virtual machine was not provided."); } - vm.setUpdateParameters(false); - _vmDao.update(vm.getId(), vm); - } - } + // Check if an SSH key pair was selected for the instance and if so + // use it to encrypt & save the vm password + encryptAndStorePassword(vm, password); + + params = createParameterInParameterMap(params, additionalParams, VirtualMachineProfile.Param.VmPassword, password); + } + + if (additionalParams.containsKey(VirtualMachineProfile.Param.BootIntoSetup)) { + if (!HypervisorType.VMware.equals(vm.getHypervisorType())) { + throw new InvalidParameterValueException(ApiConstants.BOOT_INTO_SETUP + " makes no sense for " + vm.getHypervisorType()); + } + Object paramValue = additionalParams.get(VirtualMachineProfile.Param.BootIntoSetup); + if (logger.isTraceEnabled()) { + logger.trace("It was specified whether to enter setup mode: " + paramValue.toString()); + } + params = createParameterInParameterMap(params, additionalParams, VirtualMachineProfile.Param.BootIntoSetup, paramValue); + } + + VirtualMachineEntity vmEntity = _orchSrvc.getVirtualMachine(vm.getUuid()); + + DeploymentPlanner planner = null; + if (deploymentPlannerToUse != null) { + // if set to null, the deployment planner would be later figured out either from global config var, or from + // the service offering + planner = _planningMgr.getDeploymentPlannerByName(deploymentPlannerToUse); + if (planner == null) { + throw new InvalidParameterValueException("Can't find a planner by name " + deploymentPlannerToUse); + } + } + vmEntity.setParamsToEntity(additionalParams); + + String reservationId = vmEntity.reserve(planner, plan, new ExcludeList(), Long.toString(callerUser.getId())); + vmEntity.deploy(reservationId, Long.toString(callerUser.getId()), params, deployOnGivenHost); + + vmParamPair = new Pair(vm, params); + if (vm != null && vm.isUpdateParameters()) { + // this value is not being sent to the backend; need only for api + // display purposes + if (template.isEnablePassword()) { + if (vm.getDetail(VmDetailConstants.PASSWORD) != null) { + vmInstanceDetailsDao.removeDetail(vm.getId(), VmDetailConstants.PASSWORD); + } + vm.setUpdateParameters(false); + _vmDao.update(vm.getId(), vm); + } + } + } catch (Exception e) { + logger.error("Failed to start VM {}", vm, e); + throw new CloudRuntimeException("Failed to start VM " + vm, e); + } return vmParamPair; } @@ -6013,7 +6018,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return; } long hostId = userVm.getHostId(); - List vmNames = new ArrayList(); + List vmNames = new ArrayList<>(); vmNames.add(userVm.getInstanceName()); final HostVO host = _hostDao.findById(hostId); Account account = _accountMgr.getAccount(userVm.getAccountId()); @@ -6878,7 +6883,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir //transform group names to ids here if (cmd.getSecurityGroupNameList() != null) { - List securityGroupIds = new ArrayList(); + List securityGroupIds = new ArrayList<>(); for (String groupName : cmd.getSecurityGroupNameList()) { SecurityGroup sg = _securityGroupMgr.getSecurityGroup(groupName, cmd.getEntityOwnerId()); if (sg == null) { @@ -7661,7 +7666,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } private Map getVolumePoolMappingForMigrateVmWithStorage(VMInstanceVO vm, Map volumeToPool) { - Map volToPoolObjectMap = new HashMap(); + Map volToPoolObjectMap = new HashMap<>(); List vmVolumes = getVmVolumesForMigrateVmWithStorage(vm); @@ -8615,10 +8620,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir /** * Attempts to create a network suitable for the creation of a VM ({@link NetworkOrchestrationService#createGuestNetwork}). * If no physical network is found, throws a {@link InvalidParameterValueException}. - * @param caller The account which calls for the network creation. * @param newAccount The account to which the network will be created. * @param zone The zone where the network will be created. - * @param requiredOffering The network offering required to create the network. * @return The NetworkVO for the network created. * @throws InsufficientCapacityException * @throws ResourceAllocationException From 8d269cf5bef1faf069264ff2e66e0124823023ad Mon Sep 17 00:00:00 2001 From: Daniel Augusto Veronezi Salvador Date: Fri, 20 Feb 2026 17:19:58 +0100 Subject: [PATCH 03/41] [22.0] Implement/fix limit validation for projects --- .../cloud/projects/ProjectManagerImpl.java | 51 +++++++++++++++---- .../resourcelimit/CheckedReservation.java | 4 +- .../com/cloud/resourcelimit/Reserver.java | 24 +++++++++ .../ResourceLimitManagerImpl.java | 5 -- 4 files changed, 68 insertions(+), 16 deletions(-) create mode 100644 server/src/main/java/com/cloud/resourcelimit/Reserver.java diff --git a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java index 7a743e3ce76..8302f0ddf15 100644 --- a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java +++ b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java @@ -36,6 +36,7 @@ import javax.inject.Inject; import javax.mail.MessagingException; import javax.naming.ConfigurationException; +import com.cloud.resourcelimit.CheckedReservation; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.ProjectRole; import org.apache.cloudstack.acl.SecurityChecker.AccessType; @@ -47,6 +48,7 @@ import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.messagebus.MessageBus; import org.apache.cloudstack.framework.messagebus.PublishScope; import org.apache.cloudstack.managed.context.ManagedContextRunnable; +import org.apache.cloudstack.reservation.dao.ReservationDao; import org.apache.cloudstack.utils.mailing.MailAddress; import org.apache.cloudstack.utils.mailing.SMTPMailProperties; import org.apache.cloudstack.utils.mailing.SMTPMailSender; @@ -159,6 +161,8 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C private VpcManager _vpcMgr; @Inject MessageBus messageBus; + @Inject + private ReservationDao reservationDao; protected boolean _invitationRequired = false; protected long _invitationTimeOut = 86400000; @@ -272,8 +276,7 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C owner = _accountDao.findById(user.getAccountId()); } - //do resource limit check - _resourceLimitMgr.checkResourceLimit(owner, ResourceType.project); + try (CheckedReservation projectReservation = new CheckedReservation(owner, ResourceType.project, null, null, 1L, reservationDao, _resourceLimitMgr)) { final Account ownerFinal = owner; User finalUser = user; @@ -308,6 +311,7 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C messageBus.publish(_name, ProjectManager.MESSAGE_CREATE_TUNGSTEN_PROJECT_EVENT, PublishScope.LOCAL, project); return project; + } } @Override @@ -491,6 +495,9 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C //remove account ProjectAccountVO projectAccount = _projectAccountDao.findByProjectIdAccountId(projectId, account.getId()); success = _projectAccountDao.remove(projectAccount.getId()); + if (projectAccount.getAccountRole() == Role.Admin) { + _resourceLimitMgr.decrementResourceCount(account.getId(), ResourceType.project); + } //remove all invitations for account if (success) { @@ -594,12 +601,22 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C if (username == null) { throw new InvalidParameterValueException("User information (ID) is required to add user to the project"); } + + boolean shouldIncrementResourceCount = projectRole != null && Role.Admin == projectRole; + try (CheckedReservation cr = new CheckedReservation(userAccount, ResourceType.project, shouldIncrementResourceCount ? 1L : 0L, reservationDao, _resourceLimitMgr)) { if (assignUserToProject(project, user.getId(), user.getAccountId(), projectRole, Optional.ofNullable(role).map(ProjectRole::getId).orElse(null)) != null) { + if (shouldIncrementResourceCount) { + _resourceLimitMgr.incrementResourceCount(userAccount.getId(), ResourceType.project); + } return true; + } else { + logger.warn("Failed to add user to project: {}", project); + return false; + } + } catch (ResourceAllocationException e) { + throw new RuntimeException(e); } - logger.warn("Failed to add user to project: {}", project); - return false; } } @@ -652,14 +669,18 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C } private void updateProjectAccount(ProjectAccountVO futureOwner, Role newAccRole, Long accountId) throws ResourceAllocationException { - _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(accountId), ResourceType.project); + Account account = _accountMgr.getAccount(accountId); + boolean shouldIncrementResourceCount = Role.Admin == newAccRole; + + try (CheckedReservation checkedReservation = new CheckedReservation(account, ResourceType.project, shouldIncrementResourceCount ? 1L : 0L, reservationDao, _resourceLimitMgr)) { futureOwner.setAccountRole(newAccRole); _projectAccountDao.update(futureOwner.getId(), futureOwner); - if (newAccRole != null && Role.Admin == newAccRole) { + if (shouldIncrementResourceCount) { _resourceLimitMgr.incrementResourceCount(accountId, ResourceType.project); } else { _resourceLimitMgr.decrementResourceCount(accountId, ResourceType.project); } + } } @Override @@ -701,7 +722,8 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C " doesn't belong to the project. Add it to the project first and then change the project's ownership"); } - //do resource limit check + try (CheckedReservation checkedReservation = new CheckedReservation(futureOwnerAccount, ResourceType.project, null, null, 1L, reservationDao, _resourceLimitMgr)) { + _resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(futureOwnerAccount.getId()), ResourceType.project); //unset the role for the old owner @@ -714,7 +736,7 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C futureOwner.setAccountRole(Role.Admin); _projectAccountDao.update(futureOwner.getId(), futureOwner); _resourceLimitMgr.incrementResourceCount(futureOwnerAccount.getId(), ResourceType.project); - + } } else { logger.trace("Future owner {}is already the owner of the project {}", newOwnerName, project); } @@ -857,13 +879,22 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C if (account == null) { throw new InvalidParameterValueException("Account information is required for assigning account to the project"); } + + boolean shouldIncrementResourceCount = projectRoleType != null && Role.Admin == projectRoleType; + try (CheckedReservation cr = new CheckedReservation(account, ResourceType.project, shouldIncrementResourceCount ? 1L : 0L, reservationDao, _resourceLimitMgr)) { if (assignAccountToProject(project, account.getId(), projectRoleType, null, Optional.ofNullable(projectRole).map(ProjectRole::getId).orElse(null)) != null) { + if (shouldIncrementResourceCount) { + _resourceLimitMgr.incrementResourceCount(account.getId(), ResourceType.project); + } return true; } else { logger.warn("Failed to add account {} to project {}", accountName, project); return false; } + } catch (ResourceAllocationException e) { + throw new RuntimeException(e); + } } } @@ -1042,7 +1073,9 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C boolean success = true; ProjectAccountVO projectAccount = _projectAccountDao.findByProjectIdUserId(projectId, user.getAccountId(), user.getId()); success = _projectAccountDao.remove(projectAccount.getId()); - + if (projectAccount.getAccountRole() == Role.Admin) { + _resourceLimitMgr.decrementResourceCount(user.getAccountId(), ResourceType.project); + } if (success) { logger.debug("Removed user {} from project. Removing any invite sent to the user", user); ProjectInvitation invite = _projectInvitationDao.findByUserIdProjectId(user.getId(), user.getAccountId(), projectId); diff --git a/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java b/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java index 211ca65a9d8..8d2e19d475e 100644 --- a/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java +++ b/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java @@ -40,7 +40,7 @@ import com.cloud.utils.db.GlobalLock; import com.cloud.utils.exception.CloudRuntimeException; -public class CheckedReservation implements AutoCloseable { +public class CheckedReservation implements Reserver { protected Logger logger = LogManager.getLogger(getClass()); private static final int TRY_TO_GET_LOCK_TIME = 120; @@ -174,7 +174,7 @@ public class CheckedReservation implements AutoCloseable { } @Override - public void close() throws Exception { + public void close() { removeAllReservations(); } diff --git a/server/src/main/java/com/cloud/resourcelimit/Reserver.java b/server/src/main/java/com/cloud/resourcelimit/Reserver.java new file mode 100644 index 00000000000..4d5e3ac30c5 --- /dev/null +++ b/server/src/main/java/com/cloud/resourcelimit/Reserver.java @@ -0,0 +1,24 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package com.cloud.resourcelimit; + +public interface Reserver extends AutoCloseable { + + void close(); + +} diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index c2974dedd5c..90e941cad98 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -1245,11 +1245,6 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim long oldResourceCount = 0L; ResourceCountVO domainRC = null; - // calculate project count here - if (type == ResourceType.project) { - newResourceCount += _projectDao.countProjectsForDomain(domainId); - } - if (type == ResourceType.network) { newResourceCount += networkDomainDao.listDomainNetworkMapByDomain(domainId).size(); } From 831ef82ff9b691c52c34d339b445ce32b7b1070a Mon Sep 17 00:00:00 2001 From: Daniel Augusto Veronezi Salvador Date: Fri, 20 Feb 2026 17:26:25 +0100 Subject: [PATCH 04/41] [22.0] resource allocation vpc --- .../com/cloud/network/vpc/VpcManagerImpl.java | 48 ++++++++++--------- .../cloud/network/vpc/VpcManagerImplTest.java | 8 ++-- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index e4219c858da..336772e7480 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -42,29 +42,8 @@ import javax.annotation.PostConstruct; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.configuration.ConfigurationManager; -import com.cloud.configuration.ConfigurationManagerImpl; -import com.cloud.bgp.BGPService; -import com.cloud.dc.ASNumberVO; -import com.cloud.dc.dao.ASNumberDao; -import com.cloud.dc.Vlan; -import com.cloud.network.RemoteAccessVpn; -import com.cloud.network.Site2SiteVpnConnection; -import com.cloud.network.dao.NetrisProviderDao; -import com.cloud.network.dao.NsxProviderDao; -import com.cloud.network.dao.RemoteAccessVpnDao; -import com.cloud.network.dao.RemoteAccessVpnVO; -import com.cloud.network.dao.Site2SiteCustomerGatewayDao; -import com.cloud.network.dao.Site2SiteCustomerGatewayVO; -import com.cloud.network.dao.Site2SiteVpnConnectionDao; -import com.cloud.network.dao.Site2SiteVpnConnectionVO; -import com.cloud.network.element.NetrisProviderVO; -import com.cloud.network.element.NetworkACLServiceProvider; -import com.cloud.network.element.NsxProviderVO; -import com.cloud.network.rules.RulesManager; -import com.cloud.network.vpn.RemoteAccessVpnService; -import com.cloud.vm.dao.VMInstanceDao; import com.google.common.collect.Sets; + import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.alert.AlertService; import org.apache.cloudstack.annotation.AnnotationService; @@ -106,12 +85,18 @@ import com.cloud.agent.manager.Commands; import com.cloud.alert.AlertManager; import com.cloud.api.query.dao.VpcOfferingJoinDao; import com.cloud.api.query.vo.VpcOfferingJoinVO; +import com.cloud.bgp.BGPService; import com.cloud.configuration.Config; +import com.cloud.configuration.ConfigurationManager; +import com.cloud.configuration.ConfigurationManagerImpl; import com.cloud.configuration.Resource.ResourceType; +import com.cloud.dc.ASNumberVO; import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; import com.cloud.dc.Vlan.VlanType; +import com.cloud.dc.Vlan; import com.cloud.dc.VlanVO; +import com.cloud.dc.dao.ASNumberDao; import com.cloud.dc.dao.DataCenterDao; import com.cloud.dc.dao.VlanDao; import com.cloud.deploy.DeployDestination; @@ -141,18 +126,33 @@ import com.cloud.network.NetworkService; import com.cloud.network.Networks.BroadcastDomainType; import com.cloud.network.Networks.TrafficType; import com.cloud.network.PhysicalNetwork; +import com.cloud.network.RemoteAccessVpn; +import com.cloud.network.Site2SiteVpnConnection; import com.cloud.network.addr.PublicIp; import com.cloud.network.dao.FirewallRulesDao; import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; +import com.cloud.network.dao.NetrisProviderDao; +import com.cloud.network.dao.NsxProviderDao; +import com.cloud.network.dao.RemoteAccessVpnDao; +import com.cloud.network.dao.RemoteAccessVpnVO; +import com.cloud.network.dao.Site2SiteCustomerGatewayDao; +import com.cloud.network.dao.Site2SiteCustomerGatewayVO; +import com.cloud.network.dao.Site2SiteVpnConnectionDao; +import com.cloud.network.dao.Site2SiteVpnConnectionVO; +import com.cloud.network.element.NetrisProviderVO; +import com.cloud.network.element.NetworkACLServiceProvider; import com.cloud.network.element.NetworkElement; +import com.cloud.network.element.NsxProviderVO; import com.cloud.network.element.StaticNatServiceProvider; import com.cloud.network.element.VpcProvider; import com.cloud.network.router.CommandSetupHelper; import com.cloud.network.router.NetworkHelper; import com.cloud.network.router.VpcVirtualNetworkApplianceManager; +import com.cloud.network.rules.RulesManager; +import com.cloud.network.vpn.RemoteAccessVpnService; import com.cloud.network.vpc.VpcOffering.State; import com.cloud.network.vpc.dao.NetworkACLDao; import com.cloud.network.vpc.dao.PrivateIpDao; @@ -171,6 +171,7 @@ import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; import com.cloud.org.Grouping; import com.cloud.projects.Project.ListProjectResourcesCriteria; +import com.cloud.resourcelimit.CheckedReservation; import com.cloud.server.ResourceTag.ResourceObjectType; import com.cloud.tags.ResourceTagVO; import com.cloud.tags.dao.ResourceTagDao; @@ -207,6 +208,7 @@ import com.cloud.vm.ReservationContextImpl; import com.cloud.vm.VirtualMachine; import com.cloud.vm.dao.DomainRouterDao; import com.cloud.vm.dao.NicDao; +import com.cloud.vm.dao.VMInstanceDao; import static com.cloud.offering.NetworkOffering.RoutingMode.Dynamic; @@ -1317,6 +1319,7 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis vpc.setDisplay(Boolean.TRUE.equals(displayVpc)); vpc.setUseRouterIpResolver(Boolean.TRUE.equals(useVrIpResolver)); + try (CheckedReservation vpcReservation = new CheckedReservation(owner, ResourceType.vpc, null, null, 1L, reservationDao, _resourceLimitMgr)) { if (vpc.getCidr() == null && cidrSize != null) { // Allocate a CIDR for VPC Ipv4GuestSubnetNetworkMap subnet = routedIpv4Manager.getOrCreateIpv4SubnetForVpc(vpc, cidrSize); @@ -1336,6 +1339,7 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis routedIpv4Manager.persistBgpPeersForVpc(newVpc.getId(), bgpPeerIds); } return newVpc; + } } private void validateVpcCidrSize(Account caller, long accountId, VpcOffering vpcOffering, String cidr, Integer cidrSize, long zoneId) { diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index 4f92c60e25a..cd07cdbe2e1 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -54,6 +54,7 @@ import com.cloud.network.vpc.dao.VpcOfferingServiceMapDao; import com.cloud.offering.NetworkOffering; import com.cloud.offerings.NetworkOfferingServiceMapVO; import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; +import com.cloud.resourcelimit.CheckedReservation; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountVO; @@ -81,6 +82,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.MockedConstruction; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.mockito.junit.MockitoJUnitRunner; @@ -516,7 +518,7 @@ public class VpcManagerImplTest { VpcVO vpc = Mockito.mock(VpcVO.class); Mockito.when(vpcDao.persist(any(), anyMap())).thenReturn(vpc); Mockito.when(vpc.getUuid()).thenReturn("uuid"); - try { + try (MockedConstruction mockCheckedReservation = Mockito.mockConstruction(CheckedReservation.class)) { doNothing().when(resourceLimitService).checkResourceLimit(account, Resource.ResourceType.vpc); manager.createVpc(zoneId, vpcOfferingId, vpcOwnerId, vpcName, vpcName, ip4Cidr, vpcDomain, ip4Dns[0], ip4Dns[1], null, null, true, 1500, null, null, null, false); @@ -533,7 +535,7 @@ public class VpcManagerImplTest { Mockito.when(vpc.getUuid()).thenReturn("uuid"); doReturn(true).when(routedIpv4Manager).isRoutedVpc(any()); doNothing().when(routedIpv4Manager).getOrCreateIpv4SubnetForVpc(any(), anyString()); - try { + try (MockedConstruction mockCheckedReservation = Mockito.mockConstruction(CheckedReservation.class)) { doNothing().when(resourceLimitService).checkResourceLimit(account, Resource.ResourceType.vpc); manager.createVpc(zoneId, vpcOfferingId, vpcOwnerId, vpcName, vpcName, ip4Cidr, vpcDomain, ip4Dns[0], ip4Dns[1], null, null, true, 1500, null, null, null, false); @@ -556,7 +558,7 @@ public class VpcManagerImplTest { Ipv4GuestSubnetNetworkMap ipv4GuestSubnetNetworkMap = Mockito.mock(Ipv4GuestSubnetNetworkMap.class); doReturn(ipv4GuestSubnetNetworkMap).when(routedIpv4Manager).getOrCreateIpv4SubnetForVpc(any(), anyInt()); List bgpPeerIds = Arrays.asList(11L, 12L); - try { + try (MockedConstruction mockCheckedReservation = Mockito.mockConstruction(CheckedReservation.class)) { doNothing().when(resourceLimitService).checkResourceLimit(account, Resource.ResourceType.vpc); manager.createVpc(zoneId, vpcOfferingId, vpcOwnerId, vpcName, vpcName, null, vpcDomain, ip4Dns[0], ip4Dns[1], null, null, true, 1500, 24, null, bgpPeerIds, false); From 1f849caa0be5e4ae6941942d236fdd580d3068e9 Mon Sep 17 00:00:00 2001 From: Daniel Augusto Veronezi Salvador Date: Fri, 20 Feb 2026 17:27:15 +0100 Subject: [PATCH 05/41] [22.0] resource reservation on volume creation --- .../com/cloud/user/ResourceLimitService.java | 2 +- .../ResourceLimitManagerImpl.java | 3 +- .../cloud/storage/VolumeApiServiceImpl.java | 37 +++++++++++++------ .../vpc/MockResourceLimitManagerImpl.java | 5 +++ 4 files changed, 33 insertions(+), 14 deletions(-) diff --git a/api/src/main/java/com/cloud/user/ResourceLimitService.java b/api/src/main/java/com/cloud/user/ResourceLimitService.java index ddb367b26e3..1f0661b8426 100644 --- a/api/src/main/java/com/cloud/user/ResourceLimitService.java +++ b/api/src/main/java/com/cloud/user/ResourceLimitService.java @@ -253,7 +253,7 @@ public interface ResourceLimitService { void updateTaggedResourceLimitsAndCountsForAccounts(List responses, String tag); void updateTaggedResourceLimitsAndCountsForDomains(List responses, String tag); void checkVolumeResourceLimit(Account owner, Boolean display, Long size, DiskOffering diskOffering) throws ResourceAllocationException; - + List getResourceLimitStorageTagsForResourceCountOperation(Boolean display, DiskOffering diskOffering); void checkVolumeResourceLimitForDiskOfferingChange(Account owner, Boolean display, Long currentSize, Long newSize, DiskOffering currentOffering, DiskOffering newOffering) throws ResourceAllocationException; diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 90e941cad98..c5faca365f2 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -1729,7 +1729,8 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim return tags; } - protected List getResourceLimitStorageTagsForResourceCountOperation(Boolean display, DiskOffering diskOffering) { + @Override + public List getResourceLimitStorageTagsForResourceCountOperation(Boolean display, DiskOffering diskOffering) { if (Boolean.FALSE.equals(display)) { return new ArrayList<>(); } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index de6dd4ec67f..5023686b248 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -35,16 +35,11 @@ import java.util.stream.Collectors; import javax.inject.Inject; -import com.cloud.projects.Project; -import com.cloud.projects.ProjectManager; -import com.cloud.vm.snapshot.VMSnapshot; -import com.cloud.vm.snapshot.VMSnapshotDetailsVO; -import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao; -import org.apache.cloudstack.api.command.user.volume.AssignVolumeCmd; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.InternalIdentity; import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.user.volume.AssignVolumeCmd; import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd; import org.apache.cloudstack.api.command.user.volume.ChangeOfferingForVolumeCmd; import org.apache.cloudstack.api.command.user.volume.CheckAndRepairVolumeCmd; @@ -94,6 +89,7 @@ import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; import org.apache.cloudstack.framework.jobs.impl.OutcomeImpl; import org.apache.cloudstack.framework.jobs.impl.VmWorkJobVO; import org.apache.cloudstack.jobs.JobInfo; +import org.apache.cloudstack.reservation.dao.ReservationDao; import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO; import org.apache.cloudstack.resourcedetail.SnapshotPolicyDetailVO; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; @@ -166,8 +162,11 @@ import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao; import com.cloud.offering.DiskOffering; import com.cloud.org.Cluster; import com.cloud.org.Grouping; +import com.cloud.projects.Project; +import com.cloud.projects.ProjectManager; import com.cloud.resource.ResourceManager; import com.cloud.resource.ResourceState; +import com.cloud.resourcelimit.CheckedReservation; import com.cloud.serializer.GsonHelper; import com.cloud.server.ManagementService; import com.cloud.server.ResourceTag; @@ -242,8 +241,12 @@ import com.cloud.vm.VmWorkTakeVolumeSnapshot; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDetailsDao; import com.cloud.vm.dao.VMInstanceDao; +import com.cloud.vm.snapshot.VMSnapshot; +import com.cloud.vm.snapshot.VMSnapshotDetailsVO; import com.cloud.vm.snapshot.VMSnapshotVO; import com.cloud.vm.snapshot.dao.VMSnapshotDao; +import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao; + import com.google.gson.Gson; import com.google.gson.GsonBuilder; import com.google.gson.JsonParseException; @@ -367,6 +370,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic HostPodDao podDao; @Inject EndPointSelector _epSelector; + @Inject + private ReservationDao reservationDao; @Inject private VMSnapshotDetailsDao vmSnapshotDetailsDao; @@ -937,8 +942,12 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic Storage.ProvisioningType provisioningType = diskOffering.getProvisioningType(); - // Check that the resource limit for volume & primary storage won't be exceeded - _resourceLimitMgr.checkVolumeResourceLimit(owner,displayVolume, size, diskOffering); + List tags = _resourceLimitMgr.getResourceLimitStorageTagsForResourceCountOperation(displayVolume, diskOffering); + if (tags.size() == 1 && tags.get(0) == null) { + tags = new ArrayList<>(); + } + try (CheckedReservation volumeReservation = new CheckedReservation(owner, ResourceType.volume, null, tags, 1L, reservationDao, _resourceLimitMgr); + CheckedReservation primaryStorageReservation = new CheckedReservation(owner, ResourceType.primary_storage, null, tags, size, reservationDao, _resourceLimitMgr)) { // Verify that zone exists DataCenterVO zone = _dcDao.findById(zoneId); @@ -961,6 +970,10 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic return commitVolume(cmd.getSnapshotId(), caller, owner, displayVolume, zoneId, diskOfferingId, provisioningType, size, minIops, maxIops, parentVolume, userSpecifiedName, _uuidMgr.generateUuid(Volume.class, cmd.getCustomId()), details); + } catch (Exception e) { + logger.error(e); + throw new RuntimeException(e); + } } @Override @@ -978,7 +991,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic return Transaction.execute(new TransactionCallback() { @Override public VolumeVO doInTransaction(TransactionStatus status) { - VolumeVO volume = new VolumeVO(userSpecifiedName, -1, -1, -1, -1, new Long(-1), null, null, provisioningType, 0, Volume.Type.DATADISK); + VolumeVO volume = new VolumeVO(userSpecifiedName, -1, -1, -1, -1, -1L, null, null, provisioningType, 0, Volume.Type.DATADISK); volume.setPoolId(null); volume.setUuid(uuid); volume.setDataCenterId(zoneId); @@ -5276,20 +5289,20 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic private Pair orchestrateAttachVolumeToVM(VmWorkAttachVolume work) throws Exception { Volume vol = orchestrateAttachVolumeToVM(work.getVmId(), work.getVolumeId(), work.getDeviceId()); - return new Pair(JobInfo.Status.SUCCEEDED, _jobMgr.marshallResultObject(new Long(vol.getId()))); + return new Pair(JobInfo.Status.SUCCEEDED, _jobMgr.marshallResultObject(vol.getId())); } @ReflectionUse private Pair orchestrateDetachVolumeFromVM(VmWorkDetachVolume work) throws Exception { Volume vol = orchestrateDetachVolumeFromVM(work.getVmId(), work.getVolumeId()); - return new Pair(JobInfo.Status.SUCCEEDED, _jobMgr.marshallResultObject(new Long(vol.getId()))); + return new Pair(JobInfo.Status.SUCCEEDED, _jobMgr.marshallResultObject(vol.getId())); } @ReflectionUse private Pair orchestrateResizeVolume(VmWorkResizeVolume work) throws Exception { Volume vol = orchestrateResizeVolume(work.getVolumeId(), work.getCurrentSize(), work.getNewSize(), work.getNewMinIops(), work.getNewMaxIops(), work.getNewHypervisorSnapshotReserve(), work.getNewServiceOfferingId(), work.isShrinkOk()); - return new Pair(JobInfo.Status.SUCCEEDED, _jobMgr.marshallResultObject(new Long(vol.getId()))); + return new Pair(JobInfo.Status.SUCCEEDED, _jobMgr.marshallResultObject(vol.getId())); } @ReflectionUse diff --git a/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java index 7ace8a9049e..fa030d22cc6 100644 --- a/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java @@ -277,6 +277,11 @@ public class MockResourceLimitManagerImpl extends ManagerBase implements Resourc } + @Override + public List getResourceLimitStorageTagsForResourceCountOperation(Boolean display, DiskOffering diskOffering) { + return null; + } + @Override public void checkVolumeResourceLimitForDiskOfferingChange(Account owner, Boolean display, Long currentSize, Long newSize, DiskOffering currentOffering, DiskOffering newOffering) throws ResourceAllocationException { From 46a6bbad270e58ac068803764f6d78a3c5f98e36 Mon Sep 17 00:00:00 2001 From: dahn Date: Mon, 9 Feb 2026 14:23:18 +0100 Subject: [PATCH 06/41] Fix: KVM Direct Download URL injection (#60) Co-authored-by: nvazquez --- .../direct/download/DirectTemplateDownloaderImpl.java | 11 ++++++----- .../download/MetalinkDirectTemplateDownloader.java | 2 +- .../direct/download/NfsDirectTemplateDownloader.java | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/apache/cloudstack/direct/download/DirectTemplateDownloaderImpl.java b/core/src/main/java/org/apache/cloudstack/direct/download/DirectTemplateDownloaderImpl.java index d22c803818b..3f336f3801b 100644 --- a/core/src/main/java/org/apache/cloudstack/direct/download/DirectTemplateDownloaderImpl.java +++ b/core/src/main/java/org/apache/cloudstack/direct/download/DirectTemplateDownloaderImpl.java @@ -21,6 +21,7 @@ package org.apache.cloudstack.direct.download; import com.cloud.utils.UriUtils; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.utils.security.DigestHelper; +import org.apache.commons.io.FilenameUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; @@ -33,6 +34,7 @@ import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.UUID; public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDownloader { @@ -128,15 +130,14 @@ public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDown */ protected File createTemporaryDirectoryAndFile(String downloadDir) { createFolder(downloadDir); - return new File(downloadDir + File.separator + getFileNameFromUrl()); + return new File(downloadDir + File.separator + getTemporaryFileName()); } /** - * Return filename from url + * Return filename from the temporary download file */ - public String getFileNameFromUrl() { - String[] urlParts = url.split("/"); - return urlParts[urlParts.length - 1]; + public String getTemporaryFileName() { + return String.format("%s.%s", UUID.randomUUID(), FilenameUtils.getExtension(url)); } @Override diff --git a/core/src/main/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloader.java b/core/src/main/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloader.java index 5335da99150..ed7c2095949 100644 --- a/core/src/main/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloader.java +++ b/core/src/main/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloader.java @@ -97,7 +97,7 @@ public class MetalinkDirectTemplateDownloader extends DirectTemplateDownloaderIm DirectTemplateDownloader urlDownloader = createDownloaderForMetalinks(getUrl(), getTemplateId(), getDestPoolPath(), getChecksum(), headers, connectTimeout, soTimeout, null, temporaryDownloadPath); try { - setDownloadedFilePath(downloadDir + File.separator + getFileNameFromUrl()); + setDownloadedFilePath(downloadDir + File.separator + getTemporaryFileName()); File f = new File(getDownloadedFilePath()); if (f.exists()) { f.delete(); diff --git a/core/src/main/java/org/apache/cloudstack/direct/download/NfsDirectTemplateDownloader.java b/core/src/main/java/org/apache/cloudstack/direct/download/NfsDirectTemplateDownloader.java index 21184ef07fe..6b0959b78ff 100644 --- a/core/src/main/java/org/apache/cloudstack/direct/download/NfsDirectTemplateDownloader.java +++ b/core/src/main/java/org/apache/cloudstack/direct/download/NfsDirectTemplateDownloader.java @@ -69,7 +69,7 @@ public class NfsDirectTemplateDownloader extends DirectTemplateDownloaderImpl { String mount = String.format(mountCommand, srcHost + ":" + srcPath, "/mnt/" + mountSrcUuid); Script.runSimpleBashScript(mount); String downloadDir = getDestPoolPath() + File.separator + getDirectDownloadTempPath(getTemplateId()); - setDownloadedFilePath(downloadDir + File.separator + getFileNameFromUrl()); + setDownloadedFilePath(downloadDir + File.separator + getTemporaryFileName()); Script.runSimpleBashScript("cp /mnt/" + mountSrcUuid + srcPath + " " + getDownloadedFilePath()); Script.runSimpleBashScript("umount /mnt/" + mountSrcUuid); return new Pair<>(true, getDownloadedFilePath()); From 7703fdacab4503526208bc019740a8df81552f48 Mon Sep 17 00:00:00 2001 From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com> Date: Tue, 2 Dec 2025 19:11:55 +0530 Subject: [PATCH 07/41] [minio] Handle user's canned policy when a bucket is deleted --- .../java/com/cloud/agent/api/to/BucketTO.java | 7 ++ .../driver/MinIOObjectStoreDriverImpl.java | 79 +++++++++++++------ .../MinIOObjectStoreDriverImplTest.java | 7 +- 3 files changed, 66 insertions(+), 27 deletions(-) diff --git a/api/src/main/java/com/cloud/agent/api/to/BucketTO.java b/api/src/main/java/com/cloud/agent/api/to/BucketTO.java index f7e4bfea80f..fd8237998a7 100644 --- a/api/src/main/java/com/cloud/agent/api/to/BucketTO.java +++ b/api/src/main/java/com/cloud/agent/api/to/BucketTO.java @@ -26,10 +26,13 @@ public final class BucketTO { private String secretKey; + private long accountId; + public BucketTO(Bucket bucket) { this.name = bucket.getName(); this.accessKey = bucket.getAccessKey(); this.secretKey = bucket.getSecretKey(); + this.accountId = bucket.getAccountId(); } public BucketTO(String name) { @@ -47,4 +50,8 @@ public final class BucketTO { public String getSecretKey() { return this.secretKey; } + + public long getAccountId() { + return this.accountId; + } } diff --git a/plugins/storage/object/minio/src/main/java/org/apache/cloudstack/storage/datastore/driver/MinIOObjectStoreDriverImpl.java b/plugins/storage/object/minio/src/main/java/org/apache/cloudstack/storage/datastore/driver/MinIOObjectStoreDriverImpl.java index 9dc4b30414e..28e3b85e1a5 100644 --- a/plugins/storage/object/minio/src/main/java/org/apache/cloudstack/storage/datastore/driver/MinIOObjectStoreDriverImpl.java +++ b/plugins/storage/object/minio/src/main/java/org/apache/cloudstack/storage/datastore/driver/MinIOObjectStoreDriverImpl.java @@ -24,6 +24,8 @@ import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; import javax.crypto.KeyGenerator; import javax.crypto.SecretKey; @@ -98,6 +100,51 @@ public class MinIOObjectStoreDriverImpl extends BaseObjectStoreDriverImpl { return String.format("%s-%s", ACS_PREFIX, account.getUuid()); } + private void updateCannedPolicy(long storeId, Account account, String excludeBucket) { + List buckets = _bucketDao.listByObjectStoreIdAndAccountId(storeId, account.getId()); + + String resources = buckets.stream() + .map(BucketVO::getName) + .filter(name -> !Objects.equals(name, excludeBucket)) + .map(name -> "\"arn:aws:s3:::" + name + "/*\"") + .collect(Collectors.joining(",\n")); + String policy; + if (resources.isEmpty()) { + // Resource cannot be empty in a canned Policy so deny access to all resources if the user has no buckets + policy = " {\n" + + " \"Statement\": [\n" + + " {\n" + + " \"Action\": \"s3:*\",\n" + + " \"Effect\": \"Deny\",\n" + + " \"Resource\": [\"arn:aws:s3:::*\", \"arn:aws:s3:::*/*\"]\n" + + " }\n" + + " ],\n" + + " \"Version\": \"2012-10-17\"\n" + + " }"; + } else { + policy = " {\n" + + " \"Statement\": [\n" + + " {\n" + + " \"Action\": \"s3:*\",\n" + + " \"Effect\": \"Allow\",\n" + + " \"Resource\": [" + resources + "]\n" + + " }\n" + + " ],\n" + + " \"Version\": \"2012-10-17\"\n" + + " }"; + } + + MinioAdminClient minioAdminClient = getMinIOAdminClient(storeId); + String policyName = getUserOrAccessKeyForAccount(account) + "-policy"; + String userName = getUserOrAccessKeyForAccount(account); + try { + minioAdminClient.addCannedPolicy(policyName, policy); + minioAdminClient.setPolicy(userName, false, policyName); + } catch (NoSuchAlgorithmException | IOException | InvalidKeyException e) { + throw new CloudRuntimeException(e); + } + } + @Override public Bucket createBucket(Bucket bucket, boolean objectLock) { //ToDo Client pool mgmt @@ -125,33 +172,8 @@ public class MinIOObjectStoreDriverImpl extends BaseObjectStoreDriverImpl { throw new CloudRuntimeException(e); } - List buckets = _bucketDao.listByObjectStoreIdAndAccountId(storeId, accountId); - StringBuilder resources_builder = new StringBuilder(); - for(BucketVO exitingBucket : buckets) { - resources_builder.append("\"arn:aws:s3:::"+exitingBucket.getName()+"/*\",\n"); - } - resources_builder.append("\"arn:aws:s3:::"+bucketName+"/*\"\n"); + updateCannedPolicy(storeId, account,null); - String policy = " {\n" + - " \"Statement\": [\n" + - " {\n" + - " \"Action\": \"s3:*\",\n" + - " \"Effect\": \"Allow\",\n" + - " \"Principal\": \"*\",\n" + - " \"Resource\": ["+resources_builder+"]" + - " }\n" + - " ],\n" + - " \"Version\": \"2012-10-17\"\n" + - " }"; - MinioAdminClient minioAdminClient = getMinIOAdminClient(storeId); - String policyName = getUserOrAccessKeyForAccount(account) + "-policy"; - String userName = getUserOrAccessKeyForAccount(account); - try { - minioAdminClient.addCannedPolicy(policyName, policy); - minioAdminClient.setPolicy(userName, false, policyName); - } catch (Exception e) { - throw new CloudRuntimeException(e); - } String accessKey = _accountDetailsDao.findDetail(accountId, MINIO_ACCESS_KEY).getValue(); String secretKey = _accountDetailsDao.findDetail(accountId, MINIO_SECRET_KEY).getValue(); ObjectStoreVO store = _storeDao.findById(storeId); @@ -183,6 +205,8 @@ public class MinIOObjectStoreDriverImpl extends BaseObjectStoreDriverImpl { @Override public boolean deleteBucket(BucketTO bucket, long storeId) { String bucketName = bucket.getName(); + long accountId = bucket.getAccountId(); + Account account = _accountDao.findById(accountId); MinioClient minioClient = getMinIOClient(storeId); try { if(!minioClient.bucketExists(BucketExistsArgs.builder().bucket(bucketName).build())) { @@ -197,6 +221,9 @@ public class MinIOObjectStoreDriverImpl extends BaseObjectStoreDriverImpl { } catch (Exception e) { throw new CloudRuntimeException(e); } + + updateCannedPolicy(storeId, account, bucketName); + return true; } diff --git a/plugins/storage/object/minio/src/test/java/org/apache/cloudstack/storage/datastore/driver/MinIOObjectStoreDriverImplTest.java b/plugins/storage/object/minio/src/test/java/org/apache/cloudstack/storage/datastore/driver/MinIOObjectStoreDriverImplTest.java index 1a8b3d9663a..d3298a235ca 100644 --- a/plugins/storage/object/minio/src/test/java/org/apache/cloudstack/storage/datastore/driver/MinIOObjectStoreDriverImplTest.java +++ b/plugins/storage/object/minio/src/test/java/org/apache/cloudstack/storage/datastore/driver/MinIOObjectStoreDriverImplTest.java @@ -129,10 +129,15 @@ public class MinIOObjectStoreDriverImplTest { @Test public void testDeleteBucket() throws Exception { String bucketName = "test-bucket"; - BucketTO bucket = new BucketTO(bucketName); + BucketVO bucketVO = new BucketVO(1L, 1L, 1L, bucketName, 1, false, false, false, null); + BucketTO bucket = new BucketTO(bucketVO); + when(accountDao.findById(1L)).thenReturn(account); + when(account.getUuid()).thenReturn(UUID.randomUUID().toString()); + when(bucketDao.listByObjectStoreIdAndAccountId(anyLong(), anyLong())).thenReturn(new ArrayList()); doReturn(minioClient).when(minioObjectStoreDriverImpl).getMinIOClient(anyLong()); when(minioClient.bucketExists(BucketExistsArgs.builder().bucket(bucketName).build())).thenReturn(true); doNothing().when(minioClient).removeBucket(RemoveBucketArgs.builder().bucket(bucketName).build()); + doReturn(minioAdminClient).when(minioObjectStoreDriverImpl).getMinIOAdminClient(anyLong()); boolean success = minioObjectStoreDriverImpl.deleteBucket(bucket, 1L); assertTrue(success); verify(minioClient, times(1)).bucketExists(any()); From 3d678e726ad3336fcd6bf4d991f34fa5de0e4fb8 Mon Sep 17 00:00:00 2001 From: Daniel Augusto Veronezi Salvador Date: Wed, 18 Feb 2026 15:16:50 +0100 Subject: [PATCH 08/41] [22.0] resource reservation on volume snapshot creation --- .../storage/snapshot/SnapshotManagerImpl.java | 155 +++++++++--------- .../storage/snapshot/SnapshotManagerTest.java | 55 ++++--- 2 files changed, 103 insertions(+), 107 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java index 4f91a66552e..fdc661d72b6 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -16,8 +16,6 @@ // under the License. package com.cloud.storage.snapshot; - -import com.cloud.storage.StoragePoolStatus; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -36,9 +34,6 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.host.dao.HostDao; -import com.cloud.storage.Upload; -import com.cloud.storage.dao.SnapshotDetailsDao; import org.apache.cloudstack.acl.SecurityChecker; import com.cloud.api.ApiDBUtils; import org.apache.cloudstack.annotation.AnnotationService; @@ -77,6 +72,7 @@ import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.jobs.AsyncJob; import org.apache.cloudstack.managed.context.ManagedContextRunnable; +import org.apache.cloudstack.reservation.dao.ReservationDao; import org.apache.cloudstack.resourcedetail.SnapshotPolicyDetailVO; import org.apache.cloudstack.resourcedetail.dao.SnapshotPolicyDetailsDao; import org.apache.cloudstack.snapshot.SnapshotHelper; @@ -119,10 +115,12 @@ import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.exception.StorageUnavailableException; import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.org.Grouping; import com.cloud.projects.Project.ListProjectResourcesCriteria; import com.cloud.resource.ResourceManager; +import com.cloud.resourcelimit.CheckedReservation; import com.cloud.server.ResourceTag.ResourceObjectType; import com.cloud.server.TaggedResourceService; import com.cloud.storage.CreateSnapshotPayload; @@ -135,15 +133,18 @@ import com.cloud.storage.SnapshotPolicyVO; import com.cloud.storage.SnapshotScheduleVO; import com.cloud.storage.SnapshotVO; import com.cloud.storage.Storage; +import com.cloud.storage.StoragePoolStatus; import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; +import com.cloud.storage.Upload; import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; +import com.cloud.storage.dao.SnapshotDetailsDao; import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.SnapshotPolicyDao; import com.cloud.storage.dao.SnapshotScheduleDao; @@ -251,7 +252,8 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement public TaggedResourceService taggedResourceService; @Inject private AnnotationDao annotationDao; - + @Inject + private ReservationDao reservationDao; @Inject protected SnapshotHelper snapshotHelper; @Inject @@ -312,7 +314,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement hostIdsToTryFirst = new long[] {vmHostId}; } - List hostIdsToAvoid = new ArrayList(); + List hostIdsToAvoid = new ArrayList<>(); for (int retry = _totalRetries; retry >= 0; retry--) { try { Pair result = _storageMgr.sendToPool(pool, hostIdsToTryFirst, hostIdsToAvoid, cmd); @@ -377,12 +379,12 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement if (instanceId != null) { UserVmVO vm = _vmDao.findById(instanceId); if (vm.getState() != State.Stopped && vm.getState() != State.Shutdown) { - throw new InvalidParameterValueException("The VM the specified disk is attached to is not in the shutdown state."); + throw new InvalidParameterValueException("The Instance the specified disk is attached to is not in the shutdown state."); } // If target VM has associated VM snapshots then don't allow to revert from snapshot List vmSnapshots = _vmSnapshotDao.findByVm(instanceId); if (vmSnapshots.size() > 0 && !Type.GROUP.name().equals(snapshot.getTypeDescription())) { - throw new InvalidParameterValueException("Unable to revert snapshot for VM, please remove VM snapshots before reverting VM from snapshot"); + throw new InvalidParameterValueException("Unable to revert Snapshot for Instance, please remove Instance Snapshots before reverting Instance from Snapshot"); } } @@ -412,7 +414,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement } public void updateVolumeSizeAndPrimaryStorageCount(VolumeVO volume, SnapshotVO snapshot) { - Long differenceBetweenVolumeAndSnapshotSize = new Long(volume.getSize() - snapshot.getSize()); + Long differenceBetweenVolumeAndSnapshotSize = volume.getSize() - snapshot.getSize(); if (differenceBetweenVolumeAndSnapshotSize != 0) { if (differenceBetweenVolumeAndSnapshotSize > 0) { _resourceLimitMgr.decrementResourceCount(snapshot.getAccountId(), ResourceType.primary_storage, differenceBetweenVolumeAndSnapshotSize); @@ -661,7 +663,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement //Double check the snapshot is removed or not SnapshotVO parentSnap = _snapshotDao.findById(parentSnapshotDataStoreVO.getSnapshotId()); if (parentSnap != null && parentSnapshotDataStoreVO.getInstallPath() != null && parentSnapshotDataStoreVO.getInstallPath().equals(vmSnapshot.getName())) { - throw new InvalidParameterValueException("Creating snapshot failed due to snapshot : " + parentSnap.getUuid() + " is created from the same vm snapshot"); + throw new InvalidParameterValueException("Creating Snapshot failed due to Snapshot : " + parentSnap.getUuid() + " is created from the same Instance Snapshot"); } } SnapshotInfo snapshotInfo = this.snapshotFactory.getSnapshot(snapshotId, store); @@ -675,20 +677,19 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement _snapshotDao.update(snapshot.getId(), snapshot); snapshotInfo = this.snapshotFactory.getSnapshot(snapshotId, store); - Long snapshotOwnerId = vm.getAccountId(); + long snapshotOwnerId = vm.getAccountId(); try { SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.BACKUP); if (snapshotStrategy == null) { - throw new CloudRuntimeException(String.format("Unable to find snapshot strategy to handle snapshot [%s]", snapshot)); + throw new CloudRuntimeException(String.format("Unable to find Snapshot strategy to handle Snapshot [%s]", snapshot)); } snapshotInfo = snapshotStrategy.backupSnapshot(snapshotInfo); - } catch (Exception e) { - logger.debug("Failed to backup snapshot from vm snapshot", e); + logger.debug("Failed to backup Snapshot from Instance Snapshot", e); _resourceLimitMgr.decrementResourceCount(snapshotOwnerId, ResourceType.snapshot); - _resourceLimitMgr.decrementResourceCount(snapshotOwnerId, ResourceType.secondary_storage, new Long(volume.getSize())); - throw new CloudRuntimeException("Failed to backup snapshot from vm snapshot", e); + _resourceLimitMgr.decrementResourceCount(snapshotOwnerId, ResourceType.secondary_storage, volume.getSize()); + throw new CloudRuntimeException("Failed to backup Snapshot from Instance Snapshot", e); } finally { if (snapshotOnPrimaryStore != null) { _snapshotStoreDao.remove(snapshotOnPrimaryStore.getId()); @@ -893,53 +894,44 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement _accountMgr.checkAccess(caller, null, true, snapshotCheck); SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshotCheck, zoneId, SnapshotOperation.DELETE); - if (snapshotStrategy == null) { logger.error("Unable to find snapshot strategy to handle snapshot [{}]", snapshotCheck); - return false; } Pair, List> storeRefAndZones = getStoreRefsAndZonesForSnapshotDelete(snapshotId, zoneId); List snapshotStoreRefs = storeRefAndZones.first(); List zoneIds = storeRefAndZones.second(); - boolean result = snapshotStrategy.deleteSnapshot(snapshotId, zoneId); - if (result) { - for (Long zId : zoneIds) { - if (snapshotCheck.getState() == Snapshot.State.BackedUp) { - UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_DELETE, snapshotCheck.getAccountId(), zId, snapshotId, - snapshotCheck.getName(), null, null, 0L, snapshotCheck.getClass().getName(), snapshotCheck.getUuid()); + try { + boolean result = snapshotStrategy.deleteSnapshot(snapshotId, zoneId); + if (result) { + for (Long zId : zoneIds) { + if (snapshotCheck.getState() == Snapshot.State.BackedUp) { + UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_DELETE, snapshotCheck.getAccountId(), zId, snapshotId, + snapshotCheck.getName(), null, null, 0L, snapshotCheck.getClass().getName(), snapshotCheck.getUuid()); + } + } + final SnapshotVO postDeleteSnapshotEntry = _snapshotDao.findById(snapshotId); + if (postDeleteSnapshotEntry == null || Snapshot.State.Destroyed.equals(postDeleteSnapshotEntry.getState())) { + annotationDao.removeByEntityType(AnnotationService.EntityType.SNAPSHOT.name(), snapshotCheck.getUuid()); + + if (snapshotCheck.getState() != Snapshot.State.Error && snapshotCheck.getState() != Snapshot.State.Destroyed) { + _resourceLimitMgr.decrementResourceCount(snapshotCheck.getAccountId(), ResourceType.snapshot); + } + } + for (SnapshotDataStoreVO snapshotStoreRef : snapshotStoreRefs) { + if (ObjectInDataStoreStateMachine.State.Ready.equals(snapshotStoreRef.getState()) && !DataStoreRole.Primary.equals(snapshotStoreRef.getRole())) { + _resourceLimitMgr.decrementResourceCount(snapshotCheck.getAccountId(), ResourceType.secondary_storage, snapshotStoreRef.getPhysicalSize()); + } } } - final SnapshotVO postDeleteSnapshotEntry = _snapshotDao.findById(snapshotId); - if (postDeleteSnapshotEntry == null || Snapshot.State.Destroyed.equals(postDeleteSnapshotEntry.getState())) { - annotationDao.removeByEntityType(AnnotationService.EntityType.SNAPSHOT.name(), snapshotCheck.getUuid()); - if (snapshotCheck.getState() != Snapshot.State.Error && snapshotCheck.getState() != Snapshot.State.Destroyed) { - _resourceLimitMgr.decrementResourceCount(snapshotCheck.getAccountId(), ResourceType.snapshot); - } - } - for (SnapshotDataStoreVO snapshotStoreRef : snapshotStoreRefs) { - if (ObjectInDataStoreStateMachine.State.Ready.equals(snapshotStoreRef.getState()) && !DataStoreRole.Primary.equals(snapshotStoreRef.getRole())) { - _resourceLimitMgr.decrementResourceCount(snapshotCheck.getAccountId(), ResourceType.secondary_storage, new Long(snapshotStoreRef.getPhysicalSize())); - } - } - } - final SnapshotVO postDeleteSnapshotEntry = _snapshotDao.findById(snapshotId); - if (postDeleteSnapshotEntry == null || Snapshot.State.Destroyed.equals(postDeleteSnapshotEntry.getState())) { - annotationDao.removeByEntityType(AnnotationService.EntityType.SNAPSHOT.name(), snapshotCheck.getUuid()); + return result; + } catch (Exception e) { + logger.debug("Failed to delete snapshot {}:{}", snapshotCheck, e.toString()); - if (snapshotCheck.getState() != Snapshot.State.Error && snapshotCheck.getState() != Snapshot.State.Destroyed) { - _resourceLimitMgr.decrementResourceCount(snapshotCheck.getAccountId(), ResourceType.snapshot); - } + throw new CloudRuntimeException("Failed to delete snapshot:" + e.toString()); } - for (SnapshotDataStoreVO snapshotStoreRef : snapshotStoreRefs) { - if (ObjectInDataStoreStateMachine.State.Ready.equals(snapshotStoreRef.getState()) && !DataStoreRole.Primary.equals(snapshotStoreRef.getRole())) { - _resourceLimitMgr.decrementResourceCount(snapshotCheck.getAccountId(), ResourceType.secondary_storage, new Long(snapshotStoreRef.getPhysicalSize())); - } - } - - return result; } @Override @@ -953,7 +945,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement Map tags = cmd.getTags(); Long zoneId = cmd.getZoneId(); Account caller = CallContext.current().getCallingAccount(); - List permittedAccounts = new ArrayList(); + List permittedAccounts = new ArrayList<>(); // Verify parameters if (volumeId != null) { @@ -965,7 +957,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement List ids = getIdsListFromCmd(cmd.getId(), cmd.getIds()); - Ternary domainIdRecursiveListProject = new Ternary(cmd.getDomainId(), + Ternary domainIdRecursiveListProject = new Ternary<>(cmd.getDomainId(), cmd.isRecursive(), null); _accountMgr.buildACLSearchParameters(caller, id, cmd.getAccountName(), cmd.getProjectId(), permittedAccounts, domainIdRecursiveListProject, cmd.listAll(), false); Long domainId = domainIdRecursiveListProject.first(); @@ -1058,7 +1050,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement } Pair, Integer> result = _snapshotDao.searchAndCount(sc, searchFilter); - return new Pair, Integer>(result.first(), result.second()); + return new Pair<>(result.first(), result.second()); } @Override @@ -1118,7 +1110,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement if (Type.MANUAL == snapshot.getRecurringType()) { _resourceLimitMgr.decrementResourceCount(accountId, ResourceType.snapshot); for (SnapshotDataStoreVO snapshotStoreRef : snapshotStoreRefs) { - _resourceLimitMgr.decrementResourceCount(accountId, ResourceType.secondary_storage, new Long(snapshotStoreRef.getPhysicalSize())); + _resourceLimitMgr.decrementResourceCount(accountId, ResourceType.secondary_storage, snapshotStoreRef.getPhysicalSize()); } } @@ -1147,7 +1139,6 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement throw new InvalidParameterValueException("Backing up of snapshot is not supported by the zone of the volume. Snapshots can not be taken for multiple zones"); } boolean isRootAdminCaller = _accountMgr.isRootAdmin(caller.getId()); - if (hasZones) { for (Long zoneId : zoneIds) { getCheckedDestinationZoneForSnapshotCopy(zoneId, isRootAdminCaller); @@ -1283,7 +1274,6 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement logger.debug("Acquired lock for creating snapshot policy [{}] for volume {}.", intervalType, volume); - try { SnapshotPolicyVO policy = _snapshotPolicyDao.findOneByVolumeInterval(volumeId, intervalType); @@ -1490,7 +1480,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement } // List only future schedules, not past ones. - List snapshotSchedules = new ArrayList(); + List snapshotSchedules = new ArrayList<>(); if (policyId == null) { List policyInstances = listPoliciesforVolume(volumeId); for (SnapshotPolicyVO policyInstance : policyInstances) { @@ -1609,7 +1599,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement List activeVMSnapshots = _vmSnapshotDao.listByInstanceId(userVm.getId(), VMSnapshot.State.Creating, VMSnapshot.State.Reverting, VMSnapshot.State.Expunging); if (activeVMSnapshots.size() > 0) { - throw new CloudRuntimeException("There is other active vm snapshot tasks on the instance to which the volume is attached, please try again later"); + throw new CloudRuntimeException("There is other active Instance Snapshot tasks on the Instance to which the volume is attached, please try again later"); } } } @@ -1624,7 +1614,6 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement Long snapshotId = payload.getSnapshotId(); Account snapshotOwner = payload.getAccount(); - SnapshotInfo snapshot = snapshotFactory.getSnapshot(snapshotId, volume.getDataStore()); StoragePool storagePool = _storagePoolDao.findById(volume.getPoolId()); @@ -1689,7 +1678,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement snapshotStoreRef.getPhysicalSize(), volume.getSize(), snapshot.getClass().getName(), snapshot.getUuid()); // Correct the resource count of snapshot in case of delta snapshots. - _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, new Long(volume.getSize() - snapshotStoreRef.getPhysicalSize())); + _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, volume.getSize() - snapshotStoreRef.getPhysicalSize()); if (!payload.getAsyncBackup()) { if (backupSnapToSecondary) { @@ -1707,14 +1696,14 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement logger.debug("Failed to create snapshot" + cre.getLocalizedMessage()); } _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.snapshot); - _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, new Long(volume.getSize())); + _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, volume.getSize()); throw cre; } catch (Exception e) { if (logger.isDebugEnabled()) { logger.debug("Failed to create snapshot", e); } _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.snapshot); - _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, new Long(volume.getSize())); + _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, volume.getSize()); throw new CloudRuntimeException("Failed to create snapshot", e); } return snapshot; @@ -1909,7 +1898,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement } if (policyIds == null) { - policyIds = new ArrayList(); + policyIds = new ArrayList<>(); policyIds.add(policyId); } else if (policyIds.size() <= 0) { // Not even sure how this is even possible @@ -1992,24 +1981,6 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement Type snapshotType = getSnapshotType(policyId); Account owner = _accountMgr.getAccount(volume.getAccountId()); - ResourceType storeResourceType = ResourceType.secondary_storage; - if (!isBackupSnapshotToSecondaryForZone(volume.getDataCenterId()) || - Snapshot.LocationType.PRIMARY.equals(locationType)) { - storeResourceType = ResourceType.primary_storage; - } - try { - _resourceLimitMgr.checkResourceLimit(owner, ResourceType.snapshot); - _resourceLimitMgr.checkResourceLimit(owner, storeResourceType, volume.getSize()); - } catch (ResourceAllocationException e) { - if (snapshotType != Type.MANUAL) { - String msg = String.format("Snapshot resource limit exceeded for account %s. Failed to create recurring snapshots", owner); - logger.warn(msg); - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_UPDATE_RESOURCE_COUNT, 0L, 0L, msg, "Snapshot resource limit exceeded for account id : " + owner.getId() - + ". Failed to create recurring snapshots; please use updateResourceLimit to increase the limit"); - } - throw e; - } - // Determine the name for this snapshot // Snapshot Name: VMInstancename + volumeName + timeString String timeString = DateUtil.getDateDisplayString(DateUtil.GMT_TIMEZONE, new Date(), DateUtil.YYYYMMDD_FORMAT); @@ -2041,6 +2012,14 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement hypervisorType = volume.getHypervisorType(); } + ResourceType storeResourceType = ResourceType.secondary_storage; + if (!isBackupSnapshotToSecondaryForZone(volume.getDataCenterId()) || + Snapshot.LocationType.PRIMARY.equals(locationType)) { + storeResourceType = ResourceType.primary_storage; + } + + try (CheckedReservation volumeSnapshotReservation = new CheckedReservation(owner, ResourceType.snapshot, null, null, 1L, reservationDao, _resourceLimitMgr); + CheckedReservation storageReservation = new CheckedReservation(owner, storeResourceType, null, null, volume.getSize(), reservationDao, _resourceLimitMgr)) { SnapshotVO snapshotVO = new SnapshotVO(volume.getDataCenterId(), volume.getAccountId(), volume.getDomainId(), volume.getId(), volume.getDiskOfferingId(), snapshotName, (short)snapshotType.ordinal(), snapshotType.name(), volume.getSize(), volume.getMinIops(), volume.getMaxIops(), hypervisorType, locationType); @@ -2052,6 +2031,17 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement _resourceLimitMgr.incrementResourceCount(volume.getAccountId(), ResourceType.snapshot); _resourceLimitMgr.incrementResourceCount(volume.getAccountId(), storeResourceType, volume.getSize()); return snapshot; + } catch (Exception e) { + if (e instanceof ResourceAllocationException) { + if (snapshotType != Type.MANUAL) { + String msg = String.format("Snapshot resource limit exceeded for account id : %s. Failed to create recurring snapshots", owner.getId()); + logger.warn(msg); + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_UPDATE_RESOURCE_COUNT, 0L, 0L, msg, msg + ". Please, use updateResourceLimit to increase the limit"); + } + throw (ResourceAllocationException) e; + } + throw new CloudRuntimeException(e); + } } @Override @@ -2211,6 +2201,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement } return true; } + @DB private List copySnapshotToZones(SnapshotVO snapshotVO, DataStore srcSecStore, List dstZones) throws StorageUnavailableException, ResourceAllocationException { AccountVO account = _accountDao.findById(snapshotVO.getAccountId()); diff --git a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java index 3b5d92103e7..8eee0ed6626 100755 --- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java +++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java @@ -16,6 +16,27 @@ // under the License. package com.cloud.storage.snapshot; +import org.apache.cloudstack.api.command.user.snapshot.ExtractSnapshotCmd; +import org.apache.cloudstack.context.CallContext; +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.ObjectInDataStoreStateMachine; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation; +import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.snapshot.SnapshotHelper; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity; + import com.cloud.api.ApiDBUtils; import com.cloud.configuration.Resource.ResourceType; import com.cloud.dc.DataCenter; @@ -27,6 +48,7 @@ import com.cloud.exception.ResourceAllocationException; import com.cloud.hypervisor.Hypervisor; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.resource.ResourceManager; +import com.cloud.resourcelimit.CheckedReservation; import com.cloud.server.ResourceTag; import com.cloud.server.TaggedResourceService; import com.cloud.storage.DataStoreRole; @@ -57,27 +79,6 @@ import com.cloud.vm.snapshot.VMSnapshot; import com.cloud.vm.snapshot.VMSnapshotVO; import com.cloud.vm.snapshot.dao.VMSnapshotDao; -import org.apache.cloudstack.api.command.user.snapshot.ExtractSnapshotCmd; -import org.apache.cloudstack.context.CallContext; -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.ObjectInDataStoreStateMachine; -import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; -import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; -import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService; -import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy; -import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation; -import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory; -import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; -import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; -import org.apache.cloudstack.framework.config.ConfigKey; -import org.apache.cloudstack.snapshot.SnapshotHelper; -import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; -import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; -import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; -import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity; - import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -87,6 +88,7 @@ import org.junit.runner.RunWith; import org.mockito.BDDMockito; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedConstruction; import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; @@ -237,8 +239,6 @@ public class SnapshotManagerTest { when(_storageStrategyFactory.getSnapshotStrategy(Mockito.any(SnapshotVO.class), Mockito.eq(SnapshotOperation.BACKUP))).thenReturn(snapshotStrategy); when(_storageStrategyFactory.getSnapshotStrategy(Mockito.any(SnapshotVO.class), Mockito.eq(SnapshotOperation.REVERT))).thenReturn(snapshotStrategy); - doNothing().when(_resourceLimitMgr).checkResourceLimit(any(Account.class), any(ResourceType.class)); - doNothing().when(_resourceLimitMgr).checkResourceLimit(any(Account.class), any(ResourceType.class), anyLong()); doNothing().when(_resourceLimitMgr).incrementResourceCount(anyLong(), any(ResourceType.class)); doNothing().when(_resourceLimitMgr).incrementResourceCount(anyLong(), any(ResourceType.class), anyLong()); @@ -320,7 +320,12 @@ public class SnapshotManagerTest { when(mockList2.size()).thenReturn(0); when(_vmSnapshotDao.listByInstanceId(TEST_VM_ID, VMSnapshot.State.Creating, VMSnapshot.State.Reverting, VMSnapshot.State.Expunging)).thenReturn(mockList2); when(_snapshotDao.persist(any(SnapshotVO.class))).thenReturn(snapshotMock); - _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null, null); + + try (MockedConstruction mockCheckedReservation = Mockito.mockConstruction(CheckedReservation.class)) { + _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null, null); + } catch (ResourceAllocationException e) { + Assert.fail(String.format("Failure with exception: %s", e.getMessage())); + } } @Test(expected = InvalidParameterValueException.class) @@ -468,7 +473,7 @@ public class SnapshotManagerTest { public void validateCreateTagsForSnapshotPolicyWithValidTags(){ Mockito.doReturn(null).when(taggedResourceServiceMock).createTags(any(), any(), any(), any()); - Map map = new HashMap<>(); + Map map = new HashMap<>(); map.put("test", "test"); _snapshotMgr.createTagsForSnapshotPolicy(map, snapshotPolicyVoMock); From d11d182c715585fc61da0c39e87e1e455e28da49 Mon Sep 17 00:00:00 2001 From: Daniel Augusto Veronezi Salvador Date: Fri, 20 Feb 2026 10:54:08 +0530 Subject: [PATCH 09/41] [22.0] Fix resource limit reservation and check during StartVirtualMachine --- .../java/com/cloud/vm/UserVmManagerImpl.java | 504 +++++++++--------- 1 file changed, 254 insertions(+), 250 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index f1ea5bc2576..24baf538394 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -615,26 +615,20 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir SnapshotPolicyDao snapshotPolicyDao; @Inject BackupScheduleDao backupScheduleDao; - @Inject private StatsCollector statsCollector; @Inject private UserDataDao userDataDao; - @Inject protected SnapshotHelper snapshotHelper; - @Inject private AutoScaleManager autoScaleManager; - @Inject VMScheduleManager vmScheduleManager; @Inject NsxProviderDao nsxProviderDao; - @Inject NetworkService networkService; - @Inject SnapshotDataFactory snapshotDataFactory; @@ -798,7 +792,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir NicVO nic = _nicDao.findById(nicId); try { - logger.debug("Trying IP retrieval for VM [id: {}, uuid: {}, name: {}], nic {}", vmId, vmUuid, vmName, nic); + logger.debug("Trying IP retrieval for Instance [ID: {}, UUID: {}, name: {}], NIC {}", vmId, vmUuid, vmName, nic); Answer answer = _agentMgr.send(hostId, cmd); if (answer.getResult()) { String vmIp = answer.getDetails(); @@ -813,12 +807,12 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (nic != null) { nic.setIPv4Address(vmIp); _nicDao.update(nicId, nic); - logger.debug("Vm [id: {}, uuid: {}, name: {}] - IP {} retrieved successfully", vmId, vmUuid, vmName, vmIp); + logger.debug("Instance [ID: {}, UUID: {}, name: {}] - IP {} retrieved successfully", vmId, vmUuid, vmName, vmIp); vmIdCountMap.remove(nicId); decrementCount = false; ActionEventUtils.onActionEvent(User.UID_SYSTEM, Account.ACCOUNT_ID_SYSTEM, Domain.ROOT_DOMAIN, EventTypes.EVENT_NETWORK_EXTERNAL_DHCP_VM_IPFETCH, - String.format("VM [id: %d, uuid: %s, name: %s], nic %s, IP address %s got fetched successfully", + String.format("Instance [ID: %d, UUID: %s, name: %s], NIC %s, IP address %s got fetched successfully", vmId, vmUuid, vmName, nic, vmIp), vmId, ApiCommandResourceType.VirtualMachine.toString()); } } @@ -826,7 +820,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // since no changes are being done, we should not decrement IP usage decrementCount = false; if (answer.getDetails() != null) { - logger.debug("Failed to get vm ip for Vm [id: {}, uuid: {}, name: {}], details: {}", + logger.debug("Failed to get Instance IP for Instance [ID: {}, UUID: {}, name: {}], details: {}", vmId, vmUuid, vmName, answer.getDetails()); } } @@ -838,7 +832,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (decrementCount) { VmAndCountDetails vmAndCount = vmIdCountMap.get(nicId); vmAndCount.decrementCount(); - logger.debug("Ip is not retrieved for VM [id: {}, uuid: {}, name: {}] nic {} ... decremented count to {}", + logger.debug("IP is not retrieved for Instance [ID: {}, UUID: {}, name: {}] NIC {} ... decremented count to {}", vmId, vmUuid, vmName, nic, vmAndCount.getRetrievalCount()); vmIdCountMap.put(nicId, vmAndCount); } @@ -848,7 +842,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir private void addVmUefiBootOptionsToParams(Map params, String bootType, String bootMode) { if (logger.isTraceEnabled()) { - logger.trace(String.format("Adding boot options (%s, %s, %s) into the param map for VM start as UEFI detail(%s=%s) found for the VM", + logger.trace(String.format("Adding boot options (%s, %s, %s) into the param map for Instance start as UEFI detail(%s=%s) found for the Instance", VirtualMachineProfile.Param.UefiFlag.getName(), VirtualMachineProfile.Param.BootType.getName(), VirtualMachineProfile.Param.BootMode.getName(), @@ -869,14 +863,14 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // Do parameters input validation if (userVm == null) { - throw new InvalidParameterValueException("unable to find a virtual machine with id " + cmd.getId()); + throw new InvalidParameterValueException("unable to find an Instance with id " + cmd.getId()); } _vmDao.loadDetails(userVm); VMTemplateVO template = _templateDao.findByIdIncludingRemoved(userVm.getTemplateId()); if (template == null || !template.isEnablePassword()) { - throw new InvalidParameterValueException("Fail to reset password for the virtual machine, the template is not password enabled"); + throw new InvalidParameterValueException("Fail to reset password for the Instance, the Template is not password enabled"); } if (userVm.getState() == State.Error || userVm.getState() == State.Expunging) { @@ -896,7 +890,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (result) { userVm.setPassword(password); } else { - throw new CloudRuntimeException("Failed to reset password for the virtual machine "); + throw new CloudRuntimeException("Failed to reset password for the Instance "); } return userVm; @@ -914,7 +908,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (template.isEnablePassword()) { Nic defaultNic = _networkModel.getDefaultNic(vmId); if (defaultNic == null) { - logger.error("Unable to reset password for vm " + vmInstance + " as the instance doesn't have default nic"); + logger.error("Unable to reset password for Instance " + vmInstance + " as the Instance doesn't have default NIC"); return false; } @@ -934,7 +928,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // Need to reboot the virtual machine so that the password gets // redownloaded from the DomR, and reset on the VM if (!result) { - logger.debug("Failed to reset password for the virtual machine; no need to reboot the vm"); + logger.debug("Failed to reset password for the Instance; no need to reboot the Instance"); return false; } else { final UserVmVO userVm = _vmDao.findById(vmId); @@ -950,16 +944,16 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } if (rebootVirtualMachine(userId, vmId, false, false) == null) { - logger.warn("Failed to reboot the vm " + vmInstance); + logger.warn("Failed to reboot the Instance " + vmInstance); return false; } else { - logger.debug("Vm " + vmInstance + " is rebooted successfully as a part of password reset"); + logger.debug("Instance " + vmInstance + " is rebooted successfully as a part of password reset"); return true; } } } else { if (logger.isDebugEnabled()) { - logger.debug("Reset password called for a vm that is not using a password enabled template"); + logger.debug("Reset password called for a Instance that is not using a password enabled Template"); } return false; } @@ -974,7 +968,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir UserVmVO userVm = _vmDao.findById(cmd.getId()); if (userVm == null) { - throw new InvalidParameterValueException("unable to find a virtual machine by id" + cmd.getId()); + throw new InvalidParameterValueException("unable to find an Instance by id" + cmd.getId()); } if (UserVmManager.SHAREDFSVM.equals(userVm.getUserVmType())) { throw new InvalidParameterValueException("Operation not supported on Shared FileSystem Instance"); @@ -1025,7 +1019,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir UserVmVO userVm = _vmDao.findById(cmd.getId()); if (userVm == null) { - throw new InvalidParameterValueException("unable to find a virtual machine by id " + cmd.getId()); + throw new InvalidParameterValueException("unable to find an Instance by id" + cmd.getId()); } if (UserVmManager.SHAREDFSVM.equals(userVm.getUserVmType())) { throw new InvalidParameterValueException("Operation not supported on Shared FileSystem Instance"); @@ -1077,7 +1071,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir UserVmVO vm = _vmDao.findById(userVm.getId()); _vmDao.loadDetails(vm); if (!result) { - throw new CloudRuntimeException("Failed to reset SSH Key for the virtual machine "); + throw new CloudRuntimeException("Failed to reset SSH Key for the Instance "); } removeEncryptedPasswordFromUserVmVoDetails(userVm.getId()); @@ -1097,7 +1091,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir VMTemplateVO template = _templateDao.findByIdIncludingRemoved(vmInstance.getTemplateId()); Nic defaultNic = _networkModel.getDefaultNic(vmId); if (defaultNic == null) { - logger.error("Unable to reset SSH Key for vm " + vmInstance + " as the instance doesn't have default nic"); + logger.error("Unable to reset SSH Key for Instance " + vmInstance + " as the Instance doesn't have default NIC"); return false; } @@ -1112,9 +1106,9 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir throw new CloudRuntimeException("Can't find network element for " + Service.UserData.getName() + " provider needed for SSH Key reset"); } boolean result = element.saveSSHKey(defaultNetwork, defaultNicProfile, vmProfile, sshPublicKeys); - // Need to reboot the virtual machine so that the password gets redownloaded from the DomR, and reset on the VM + // Need to reboot the Instance so that the password gets redownloaded from the DomR, and reset on the VM if (!result) { - logger.debug("Failed to reset SSH Key for the virtual machine; no need to reboot the vm"); + logger.debug("Failed to reset SSH Key for the Instance; no need to reboot the Instance"); return false; } else { final UserVmVO userVm = _vmDao.findById(vmId); @@ -1159,7 +1153,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir logger.debug("Unable to stop due to ", e); status = false; } catch (CloudException e) { - throw new CloudRuntimeException("Unable to contact the agent to stop the virtual machine " + vm, e); + throw new CloudRuntimeException("Unable to contact the agent to stop the Instance " + vm, e); } return status; } @@ -1261,10 +1255,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir //UserVmVO vmInstance = _vmDao.findById(vmId); VMInstanceVO vmInstance = _vmInstanceDao.findById(vmId); if (vmInstance == null) { - throw new InvalidParameterValueException("unable to find a virtual machine with id " + vmId); + throw new InvalidParameterValueException("unable to find an Instance with id " + vmId); } else if (!(vmInstance.getState().equals(State.Stopped))) { - throw new InvalidParameterValueException("Unable to upgrade virtual machine " + vmInstance.toString() + " " + " in state " + vmInstance.getState() - + "; make sure the virtual machine is stopped"); + throw new InvalidParameterValueException("Unable to upgrade Instance " + vmInstance.toString() + " " + " in state " + vmInstance.getState() + + "; make sure the Instance is stopped"); } _accountMgr.checkAccess(caller, null, true, vmInstance); @@ -1334,21 +1328,21 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir int cpuNumber = NumbersUtil.parseInt(customParameters.get(UsageEventVO.DynamicParameters.cpuNumber.name()), -1); Integer maxCPUCores = ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value() == 0 ? Integer.MAX_VALUE: ConfigurationManagerImpl.VM_SERVICE_OFFERING_MAX_CPU_CORES.value(); if (cpuNumber < minCPU || cpuNumber > maxCPU || cpuNumber > maxCPUCores) { - throw new InvalidParameterValueException(String.format("Invalid cpu cores value, specify a value between %d and %d", minCPU, Math.min(maxCPUCores, maxCPU))); + throw new InvalidParameterValueException(String.format("Invalid CPU cores value, specify a value between %d and %d", minCPU, Math.min(maxCPUCores, maxCPU))); } } else if (customParameters.containsKey(UsageEventVO.DynamicParameters.cpuNumber.name())) { - throw new InvalidParameterValueException("The cpu cores of this offering id:" + serviceOffering.getUuid() - + " is not customizable. This is predefined in the template."); + throw new InvalidParameterValueException("The CPU cores of this offering id:" + serviceOffering.getUuid() + + " is not customizable. This is predefined in the Template."); } if (serviceOffering.getSpeed() == null) { String cpuSpeed = customParameters.get(UsageEventVO.DynamicParameters.cpuSpeed.name()); if ((cpuSpeed == null) || (NumbersUtil.parseInt(cpuSpeed, -1) <= 0)) { - throw new InvalidParameterValueException("Invalid cpu speed value, specify a value between 1 and " + Integer.MAX_VALUE); + throw new InvalidParameterValueException("Invalid CPU speed value, specify a value between 1 and " + Integer.MAX_VALUE); } } else if (!serviceOffering.isCustomCpuSpeedSupported() && customParameters.containsKey(UsageEventVO.DynamicParameters.cpuSpeed.name())) { - throw new InvalidParameterValueException("The cpu speed of this offering id:" + serviceOffering.getUuid() - + " is not customizable. This is predefined in the template."); + throw new InvalidParameterValueException("The CPU speed of this offering id:" + serviceOffering.getUuid() + + " is not customizable. This is predefined in the Template."); } if (serviceOffering.getRamSize() == null) { @@ -1360,7 +1354,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir throw new InvalidParameterValueException(String.format("Invalid memory value, specify a value between %d and %d", minMemory, Math.min(maxRAMSize, maxMemory))); } } else if (customParameters.containsKey(UsageEventVO.DynamicParameters.memory.name())) { - throw new InvalidParameterValueException("The memory of this offering id:" + serviceOffering.getUuid() + " is not customizable. This is predefined in the template."); + throw new InvalidParameterValueException("The memory of this offering id:" + serviceOffering.getUuid() + " is not customizable. This is predefined in the Template."); } } else { throw new InvalidParameterValueException("Need to specify custom parameter values cpu, cpu speed and memory when using custom offering"); @@ -1373,7 +1367,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // Check resource limits for CPU and Memory. ServiceOfferingVO newServiceOffering = serviceOfferingDao.findById(svcOffId); if (newServiceOffering.getState() == ServiceOffering.State.Inactive) { - throw new InvalidParameterValueException(String.format("Unable to upgrade virtual machine %s with an inactive service offering %s", vmInstance.getUuid(), newServiceOffering.getUuid())); + throw new InvalidParameterValueException(String.format("Unable to upgrade Instance %s with an inactive service offering %s", vmInstance.getUuid(), newServiceOffering.getUuid())); } if (newServiceOffering.isDynamic()) { newServiceOffering.setDynamicFlag(true); @@ -1455,7 +1449,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } @Override - @ActionEvent(eventType = EventTypes.EVENT_NIC_CREATE, eventDescription = "Creating Nic", async = true) + @ActionEvent(eventType = EventTypes.EVENT_NIC_CREATE, eventDescription = "Creating NIC", async = true) public UserVm addNicToVirtualMachine(AddNicToVMCmd cmd) throws InvalidParameterValueException, PermissionDeniedException, CloudRuntimeException { Long vmId = cmd.getVmId(); Long networkId = cmd.getNetworkId(); @@ -1465,17 +1459,17 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir UserVmVO vmInstance = _vmDao.findById(vmId); if (vmInstance == null) { - throw new InvalidParameterValueException("unable to find a virtual machine with id " + vmId); + throw new InvalidParameterValueException("Unable to find an Instance with ID " + vmId); } // Check that Vm does not have VM Snapshots if (_vmSnapshotDao.findByVm(vmId).size() > 0) { - throw new InvalidParameterValueException("NIC cannot be added to VM with VM Snapshots"); + throw new InvalidParameterValueException("NIC cannot be added to Instance with Instance Snapshots"); } NetworkVO network = _networkDao.findById(networkId); if (network == null) { - throw new InvalidParameterValueException("unable to find a network with id " + networkId); + throw new InvalidParameterValueException("Unable to find a Network with ID " + networkId); } if (UserVmManager.SHAREDFSVM.equals(vmInstance.getUserVmType()) && network.getGuestType() == Network.GuestType.Shared) { @@ -1510,7 +1504,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // Verify that zone is not Basic DataCenterVO dc = _dcDao.findById(vmInstance.getDataCenterId()); if (dc.getNetworkType() == DataCenter.NetworkType.Basic) { - throw new CloudRuntimeException(String.format("Zone %s, has a NetworkType of Basic. Can't add a new NIC to a VM on a Basic Network", dc)); + throw new CloudRuntimeException(String.format("Zone %s, has a NetworkType of Basic. Can't add a new NIC to a Instance on a Basic Network", dc)); } //ensure network belongs in zone @@ -1520,13 +1514,13 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } if(_networkModel.getNicInNetwork(vmInstance.getId(),network.getId()) != null){ - logger.debug("VM {} already in network {} going to add another NIC", vmInstance, network); + logger.debug("Instance {} already in network {} going to add another NIC", vmInstance, network); } else { //* get all vms hostNames in the network List hostNames = _vmInstanceDao.listDistinctHostNames(network.getId()); //* verify that there are no duplicates if (hostNames.contains(vmInstance.getHostName())) { - throw new CloudRuntimeException("Network " + network.getName() + " already has a vm with host name: " + vmInstance.getHostName()); + throw new CloudRuntimeException("Network " + network.getName() + " already has an Instance with host name: " + vmInstance.getHostName()); } } @@ -1567,7 +1561,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir */ public void setNicAsDefaultIfNeeded(UserVmVO vmInstance, NicProfile nicProfile) { if (_networkModel.getDefaultNic(vmInstance.getId()) == null) { - logger.debug(String.format("Setting NIC %s as default as VM %s has no default NIC.", nicProfile.getName(), vmInstance.getName())); + logger.debug(String.format("Setting NIC %s as default as Instance %s has no default NIC.", nicProfile.getName(), vmInstance.getName())); nicProfile.setDefaultNic(true); } } @@ -1609,7 +1603,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } @Override - @ActionEvent(eventType = EventTypes.EVENT_NIC_DELETE, eventDescription = "Removing Nic", async = true) + @ActionEvent(eventType = EventTypes.EVENT_NIC_DELETE, eventDescription = "Removing NIC", async = true) public UserVm removeNicFromVirtualMachine(RemoveNicFromVMCmd cmd) throws InvalidParameterValueException, PermissionDeniedException, CloudRuntimeException { Long vmId = cmd.getVmId(); Long nicId = cmd.getNicId(); @@ -1617,22 +1611,22 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir UserVmVO vmInstance = _vmDao.findById(vmId); if (vmInstance == null) { - throw new InvalidParameterValueException("Unable to find a virtual machine with id " + vmId); + throw new InvalidParameterValueException("Unable to find an Instance with ID " + vmId); } // Check that Vm does not have VM Snapshots if (_vmSnapshotDao.findByVm(vmId).size() > 0) { - throw new InvalidParameterValueException("NIC cannot be removed from VM with VM Snapshots"); + throw new InvalidParameterValueException("NIC cannot be removed from Instance with Instance Snapshots"); } NicVO nic = _nicDao.findById(nicId); if (nic == null) { - throw new InvalidParameterValueException("Unable to find a nic with id " + nicId); + throw new InvalidParameterValueException("Unable to find a NIC with ID " + nicId); } NetworkVO network = _networkDao.findById(nic.getNetworkId()); if (network == null) { - throw new InvalidParameterValueException("Unable to find a network with id " + nic.getNetworkId()); + throw new InvalidParameterValueException("Unable to find a Network with ID " + nic.getNetworkId()); } // Perform permission check on VM @@ -1646,17 +1640,17 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // check to see if nic is attached to VM if (nic.getInstanceId() != vmId) { - throw new InvalidParameterValueException(nic + " is not a nic on " + vmInstance); + throw new InvalidParameterValueException(nic + " is not a NIC on " + vmInstance); } // don't delete default NIC on a user VM if (nic.isDefaultNic() && vmInstance.getType() == VirtualMachine.Type.User) { - throw new InvalidParameterValueException("Unable to remove nic from " + vmInstance + " in " + network + ", nic is default."); + throw new InvalidParameterValueException("Unable to remove NIC from " + vmInstance + " in " + network + ", NIC is default."); } // if specified nic is associated with PF/LB/Static NAT if (_rulesMgr.listAssociatedRulesForGuestNic(nic).size() > 0) { - throw new InvalidParameterValueException("Unable to remove nic from " + vmInstance + " in " + network + ", nic has associated Port forwarding or Load balancer or Static NAT rules."); + throw new InvalidParameterValueException("Unable to remove NIC from " + vmInstance + " in " + network + ", NIC has associated Port forwarding or Load balancer or Static NAT rules."); } boolean nicremoved = false; @@ -1678,7 +1672,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } @Override - @ActionEvent(eventType = EventTypes.EVENT_NIC_UPDATE, eventDescription = "Creating Nic", async = true) + @ActionEvent(eventType = EventTypes.EVENT_NIC_UPDATE, eventDescription = "Creating NIC", async = true) public UserVm updateDefaultNicForVirtualMachine(UpdateDefaultNicForVMCmd cmd) throws InvalidParameterValueException, CloudRuntimeException { Long vmId = cmd.getVmId(); Long nicId = cmd.getNicId(); @@ -1686,21 +1680,21 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir UserVmVO vmInstance = _vmDao.findById(vmId); if (vmInstance == null) { - throw new InvalidParameterValueException("unable to find a virtual machine with id " + vmId); + throw new InvalidParameterValueException("Unable to find an Instance with ID " + vmId); } // Check that Vm does not have VM Snapshots if (_vmSnapshotDao.findByVm(vmId).size() > 0) { - throw new InvalidParameterValueException("NIC cannot be updated for VM with VM Snapshots"); + throw new InvalidParameterValueException("NIC cannot be updated for Instance with Instance Snapshots"); } NicVO nic = _nicDao.findById(nicId); if (nic == null) { - throw new InvalidParameterValueException("unable to find a nic with id " + nicId); + throw new InvalidParameterValueException("Unable to find a NIC with ID " + nicId); } NetworkVO network = _networkDao.findById(nic.getNetworkId()); if (network == null) { - throw new InvalidParameterValueException("unable to find a network with id " + nic.getNetworkId()); + throw new InvalidParameterValueException("Unable to find a Network with ID " + nic.getNetworkId()); } // Perform permission check on VM @@ -1717,11 +1711,11 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir //check to see if nic is attached to VM if (nic.getInstanceId() != vmId) { - throw new InvalidParameterValueException(nic + " is not a nic on " + vmInstance); + throw new InvalidParameterValueException(nic + " is not a NIC on " + vmInstance); } // if current default equals chosen new default, Throw an exception if (nic.isDefaultNic()) { - throw new CloudRuntimeException("refusing to set default nic because chosen nic is already the default"); + throw new CloudRuntimeException("refusing to set default NIC because chosen NIC is already the default"); } //make sure the VM is Running or Stopped @@ -1738,8 +1732,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } if (existing == null) { - logger.warn("Failed to update default nic, no nic profile found for existing default network"); - throw new CloudRuntimeException("Failed to find a nic profile for the existing default network. This is bad and probably means some sort of configuration corruption"); + logger.warn("Failed to update default NIC, no NIC profile found for existing default Network"); + throw new CloudRuntimeException("Failed to find a NIC profile for the existing default Network. This is bad and probably means some sort of configuration corruption"); } Network oldDefaultNetwork = null; @@ -1851,19 +1845,19 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir Account ipOwner = _accountDao.findByIdIncludingRemoved(vm.getAccountId()); // verify ip address - logger.debug("Calling the ip allocation ..."); + logger.debug("Calling the IP allocation ..."); DataCenter dc = _dcDao.findById(network.getDataCenterId()); if (dc == null) { - throw new InvalidParameterValueException("There is no dc with the nic"); + throw new InvalidParameterValueException("There is no dc with the NIC"); } if (dc.getNetworkType() == NetworkType.Advanced && network.getGuestType() == Network.GuestType.Isolated) { try { ipaddr = _ipAddrMgr.allocateGuestIP(network, ipaddr); } catch (InsufficientAddressCapacityException e) { - throw new InvalidParameterValueException(String.format("Allocating ip to guest nic %s failed, for insufficient address capacity", nicVO)); + throw new InvalidParameterValueException(String.format("Allocating IP to guest NIC %s failed, for insufficient address capacity", nicVO)); } if (ipaddr == null) { - throw new InvalidParameterValueException(String.format("Allocating ip to guest nic %s failed, please choose another ip", nicVO)); + throw new InvalidParameterValueException(String.format("Allocating IP to guest NIC %s failed, please choose another IP", nicVO)); } if (nicVO.getIPv4Address() != null) { @@ -1879,14 +1873,14 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (dc.getNetworkType() == NetworkType.Basic) { podId = vm.getPodIdToDeployIn(); if (podId == null) { - throw new InvalidParameterValueException("vm pod id is null in Basic zone; can't decide the range for ip allocation"); + throw new InvalidParameterValueException("Instance Pod ID is null in Basic zone; can't decide the range for IP allocation"); } } try { ipaddr = _ipAddrMgr.allocatePublicIpForGuestNic(network, podId, ipOwner, ipaddr); if (ipaddr == null) { - throw new InvalidParameterValueException("Allocating ip to guest nic " + nicVO.getUuid() + " failed, please choose another ip"); + throw new InvalidParameterValueException("Allocating IP to guest NIC " + nicVO.getUuid() + " failed, please choose another IP"); } final IPAddressVO newIp = _ipAddressDao.findByIpAndSourceNetworkId(network.getId(), ipaddr); @@ -1905,7 +1899,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir }); } } catch (InsufficientAddressCapacityException e) { - logger.error("Allocating ip to guest nic {} failed, for insufficient address capacity", nicVO); + logger.error("Allocating IP to guest NIC {} failed, for insufficient address capacity", nicVO); return null; } } else { @@ -2124,7 +2118,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // Check zone wide flag boolean enableDynamicallyScaleVm = EnableDynamicallyScaleVm.valueIn(vmInstance.getDataCenterId()); if (!enableDynamicallyScaleVm) { - throw new PermissionDeniedException("Dynamically scaling virtual machines is disabled for this zone, please contact your admin."); + throw new PermissionDeniedException("Dynamically scaling Instances is disabled for this zone, please contact your admin."); } // Check vm flag @@ -2327,7 +2321,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir final UserVmVO vm = _vmDao.findById(vmId); if (vm == null) { - throw new InvalidParameterValueException("unable to find a virtual machine with id " + vmId); + throw new InvalidParameterValueException("Unable to find an Instance with id " + vmId); } if (UserVmManager.SHAREDFSVM.equals(vm.getUserVmType())) { throw new InvalidParameterValueException("Operation not supported on Shared FileSystem Instance"); @@ -2363,7 +2357,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // if the account is deleted, throw error if (account.getRemoved() != null) { - throw new CloudRuntimeException("Unable to recover VM as the account is deleted"); + throw new CloudRuntimeException("Unable to recover Instance as the Account is deleted"); } // Get serviceOffering for Virtual Machine @@ -2637,14 +2631,14 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir for (IPAddressVO ip : ips) { try { if (_rulesMgr.disableStaticNat(ip.getId(), _accountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM), User.UID_SYSTEM, true)) { - logger.debug("Disabled 1-1 nat for ip address {} as a part of vm {} expunge", ip, vm); + logger.debug("Disabled 1-1 NAT for IP address {} as a part of Instance {} expunge", ip, vm); } else { - logger.warn("Failed to disable static nat for ip address {} as a part of vm {} expunge", ip, vm); + logger.warn("Failed to disable static NAT for IP address {} as a part of Instance {} expunge", ip, vm); success = false; } } catch (ResourceUnavailableException e) { success = false; - logger.warn("Failed to disable static nat for ip address {} as a part of vm {} expunge because resource is unavailable", ip, vm, e); + logger.warn("Failed to disable static NAT for IP address {} as a part of Instance {} expunge because resource is unavailable", ip, vm, e); } } @@ -2711,11 +2705,11 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (vmIdAndCount.getRetrievalCount() <= 0) { vmIdCountMap.remove(nicId); - logger.debug("Vm {} nic {} count is zero .. removing vm nic from map ", vmId, nicId); + logger.debug("Instance {} NIC {} count is zero .. removing Instance NIC from map ", vmId, nicId); ActionEventUtils.onActionEvent(User.UID_SYSTEM, Account.ACCOUNT_ID_SYSTEM, Domain.ROOT_DOMAIN, EventTypes.EVENT_NETWORK_EXTERNAL_DHCP_VM_IPFETCH, - "VM " + vmId + " nic id "+ nicId + " ip addr fetch failed ", vmId, ApiCommandResourceType.VirtualMachine.toString()); + "Instance " + vmId + " NIC id "+ nicId + " IP addr fetch failed ", vmId, ApiCommandResourceType.VirtualMachine.toString()); continue; } @@ -2764,9 +2758,9 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir List vms = _vmDao.findDestroyedVms(new Date(System.currentTimeMillis() - ((long)_expungeDelay << 10))); if (logger.isInfoEnabled()) { if (vms.size() == 0) { - logger.trace("Found " + vms.size() + " vms to expunge."); + logger.trace("Found " + vms.size() + " Instances to expunge."); } else { - logger.info("Found " + vms.size() + " vms to expunge."); + logger.info("Found " + vms.size() + " Instances to expunge."); } } for (UserVmVO vm : vms) { @@ -3178,10 +3172,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (isDynamicallyScalable == true) { VMTemplateVO template = _templateDao.findByIdIncludingRemoved(vm.getTemplateId()); if (!template.isDynamicallyScalable()) { - throw new InvalidParameterValueException("Dynamic Scaling cannot be enabled for the VM since its template does not have dynamic scaling enabled"); + throw new InvalidParameterValueException("Dynamic Scaling cannot be enabled for the Instance since its Template does not have dynamic scaling enabled"); } if (!offering.isDynamicScalingEnabled()) { - throw new InvalidParameterValueException("Dynamic Scaling cannot be enabled for the VM since its service offering does not have dynamic scaling enabled"); + throw new InvalidParameterValueException("Dynamic Scaling cannot be enabled for the Instance since its service offering does not have dynamic scaling enabled"); } if (!UserVmManager.EnableDynamicallyScaleVm.valueIn(vm.getDataCenterId())) { logger.debug("Dynamic Scaling cannot be enabled for the VM {} since the global setting enable.dynamic.scale.vm is set to false", vm); @@ -3403,11 +3397,11 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // Verify input parameters UserVmVO vmInstance = _vmDao.findById(vmId); if (vmInstance == null) { - throw new InvalidParameterValueException("Unable to find a virtual machine with id " + vmId); + throw new InvalidParameterValueException("Unable to find a Instance with ID " + vmId); } if (vmInstance.getState() != State.Running) { - throw new InvalidParameterValueException(String.format("The VM %s (%s) is not running, unable to reboot it", + throw new InvalidParameterValueException(String.format("The Instance %s (%s) is not running, unable to reboot it", vmInstance.getUuid(), vmInstance.getDisplayNameOrHostName())); } @@ -3423,7 +3417,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return restoreVMInternal(caller, vmInstance); } } else { - throw new InvalidParameterValueException("Unable to find service offering: " + serviceOfferingId + " corresponding to the vm"); + throw new InvalidParameterValueException("Unable to find service offering: " + serviceOfferingId + " corresponding to the Instance"); } Boolean enterSetup = cmd.getBootIntoSetup(); @@ -3438,8 +3432,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir for (NicVO nic : nics) { Network network = _networkModel.getNetwork(nic.getNetworkId()); if (GuestType.L2.equals(network.getGuestType()) || _networkModel.isSharedNetworkWithoutServices(network.getId())) { - logger.debug("Adding vm " +vmId +" nic id "+ nic.getId() +" into vmIdCountMap as part of vm " + - "reboot for vm ip fetch "); + logger.debug("Adding Instance " +vmId +" NIC ID "+ nic.getId() +" into vmIdCountMap as part of Instance " + + "reboot for Instance IP fetch "); vmIdCountMap.put(nic.getId(), new VmAndCountDetails(nic.getInstanceId(), VmIpFetchTrialMax.value())); } } @@ -3518,11 +3512,11 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir checkPluginsIfVmCanBeDestroyed(vm); // check if there are active volume snapshots tasks - logger.debug("Checking if there are any ongoing snapshots on the ROOT volumes associated with VM {}", vm); + logger.debug("Checking if there are any ongoing Snapshots on the ROOT volumes associated with Instance {}", vm); if (checkStatusOfVolumeSnapshots(vm, Volume.Type.ROOT)) { - throw new CloudRuntimeException("There is/are unbacked up snapshot(s) on ROOT volume, vm destroy is not permitted, please try again later."); + throw new CloudRuntimeException("There is/are unbacked up Snapshot(s) on ROOT volume, Instance destroy is not permitted, please try again later."); } - logger.debug("Found no ongoing snapshots on volume of type ROOT, for the vm {}", vm); + logger.debug("Found no ongoing Snapshots on volume of type ROOT, for the Instance {}", vm); List volumesToBeDeleted = getVolumesFromIds(cmd); @@ -3594,7 +3588,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir boolean isNameInUse = _vmGroupDao.isNameInUse(accountId, groupName); if (isNameInUse) { - throw new InvalidParameterValueException(String.format("Unable to create vm group, a group with name %s already exists for account %s", groupName, owner)); + throw new InvalidParameterValueException(String.format("Unable to create Instance group, a group with name %s already exists for Account %s", groupName, owner)); } return createVmGroup(groupName, accountId); @@ -3611,7 +3605,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // not // created. if (account == null) { - logger.warn("Failed to acquire lock on account"); + logger.warn("Failed to acquire lock on Account"); return null; } InstanceGroupVO group = _vmGroupDao.findByAccountAndName(accountId, groupName); @@ -3687,8 +3681,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // it. InstanceGroupVO ngrpLock = _vmGroupDao.lockRow(groupFinal.getId(), false); if (ngrpLock == null) { - logger.warn("Failed to acquire lock on vm group {}", groupFinal); - throw new CloudRuntimeException(String.format("Failed to acquire lock on vm group %s", groupFinal)); + logger.warn("Failed to acquire lock on Instance group {}", groupFinal); + throw new CloudRuntimeException(String.format("Failed to acquire lock on Instance group %s", groupFinal)); } // Currently don't allow to assign a vm to more than one group @@ -3806,7 +3800,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } else { // create default security group for the account if (logger.isDebugEnabled()) { - logger.debug("Couldn't find default security group for the account " + owner + " so creating a new one"); + logger.debug("Couldn't find default security group for the Account " + owner + " so creating a new one"); } defaultGroup = _securityGroupMgr.createSecurityGroup(SecurityGroupManager.DEFAULT_GROUP_NAME, SecurityGroupManager.DEFAULT_GROUP_DESCRIPTION, owner.getDomainId(), owner.getId(), owner.getAccountName()); @@ -3917,7 +3911,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } else { // create default security group for the account if (logger.isDebugEnabled()) { - logger.debug("Couldn't find default security group for the account " + owner + " so creating a new one"); + logger.debug("Couldn't find default security group for the Account " + owner + " so creating a new one"); } defaultGroup = _securityGroupMgr.createSecurityGroup(SecurityGroupManager.DEFAULT_GROUP_NAME, SecurityGroupManager.DEFAULT_GROUP_DESCRIPTION, owner.getDomainId(), owner.getId(), owner.getAccountName()); @@ -3970,11 +3964,11 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // Only ISOs, XenServer, KVM, and VmWare template types are // supported for vpc networks if (template.getFormat() != ImageFormat.ISO && !vpcSupportedHTypes.contains(template.getHypervisorType())) { - throw new InvalidParameterValueException("Can't create vm from template with hypervisor " + template.getHypervisorType() + " in vpc network " + network); + throw new InvalidParameterValueException("Can't create Instance from Template with hypervisor " + template.getHypervisorType() + " in VPC Network " + network); } else if (template.getFormat() == ImageFormat.ISO && !vpcSupportedHTypes.contains(hypervisor)) { // Only XenServer, KVM, and VMware hypervisors are supported // for vpc networks - throw new InvalidParameterValueException("Can't create vm of hypervisor type " + hypervisor + " in vpc network"); + throw new InvalidParameterValueException("Can't create Instance of hypervisor type " + hypervisor + " in VPC Network"); } } @@ -4018,11 +4012,11 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // Only ISOs, XenServer, KVM, and VmWare template types are // supported for vpc networks if (template.getFormat() != ImageFormat.ISO && !vpcSupportedHTypes.contains(template.getHypervisorType())) { - throw new InvalidParameterValueException("Can't create vm from template with hypervisor " + template.getHypervisorType() + " in vpc network " + network); + throw new InvalidParameterValueException("Can't create Instance from Template with hypervisor " + template.getHypervisorType() + " in VPC Network " + network); } else if (template.getFormat() == ImageFormat.ISO && !vpcSupportedHTypes.contains(hypervisor)) { // Only XenServer, KVM, and VMware hypervisors are supported // for vpc networks - throw new InvalidParameterValueException("Can't create vm of hypervisor type " + hypervisor + " in vpc network"); + throw new InvalidParameterValueException("Can't create Instance of hypervisor type " + hypervisor + " in VPC Network"); } } @@ -4057,12 +4051,12 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // get Virtual networks List virtualNetworks = _networkModel.listNetworksForAccount(owner.getId(), zone.getId(), Network.GuestType.Isolated); if (virtualNetworks == null) { - throw new InvalidParameterValueException("No (virtual) networks are found for account " + owner); + throw new InvalidParameterValueException("No (virtual) networks are found for Account " + owner); } if (virtualNetworks.isEmpty()) { defaultNetwork = createDefaultNetworkForAccount(zone, owner, requiredOfferings); } else if (virtualNetworks.size() > 1 && !selectAny) { - throw new InvalidParameterValueException("More than 1 default Isolated networks are found for account " + owner + "; please specify networkIds"); + throw new InvalidParameterValueException("More than 1 default Isolated networks are found for Account " + owner + "; please specify networkIds"); } else { defaultNetwork = _networkDao.findById(virtualNetworks.get(0).getId()); } @@ -4083,7 +4077,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir throw new InvalidParameterValueException("Unable to find physical network with id: " + physicalNetworkId + " and tag: " + requiredOfferings.get(0).getTags()); } - logger.debug("Creating network for account {} from the network offering {} as a part of deployVM process", owner, requiredOfferings.get(0)); + logger.debug("Creating Network for Account {} from the network offering {} as a part of deployVM process", owner, requiredOfferings.get(0)); Network newNetwork = _networkMgr.createGuestNetwork(requiredOfferings.get(0).getId(), owner.getAccountName() + "-network", owner.getAccountName() + "-network", null, null, null, false, null, owner, null, physicalNetwork, zone.getId(), ACLType.Account, null, null, null, null, true, null, null, null, null, null, null, null, null, null, null, null); @@ -4140,12 +4134,12 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir HypervisorType hypervisorType = null; if (template.getHypervisorType() == null || template.getHypervisorType() == HypervisorType.None) { if (hypervisor == null || hypervisor == HypervisorType.None) { - throw new InvalidParameterValueException("hypervisor parameter is needed to deploy VM or the hypervisor parameter value passed is invalid"); + throw new InvalidParameterValueException("Hypervisor parameter is needed to deploy VM or the hypervisor parameter value passed is invalid"); } hypervisorType = hypervisor; } else { if (hypervisor != null && hypervisor != HypervisorType.None && hypervisor != template.getHypervisorType()) { - throw new InvalidParameterValueException("Hypervisor passed to the deployVm call, is different from the hypervisor type of the template"); + throw new InvalidParameterValueException("Hypervisor passed to the deployVm call, is different from the hypervisor type of the Template"); } hypervisorType = template.getHypervisorType(); } @@ -4326,20 +4320,20 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (dataDiskTemplate == null || (!dataDiskTemplate.getTemplateType().equals(TemplateType.DATADISK)) && (dataDiskTemplate.getState().equals(VirtualMachineTemplate.State.Active))) { - throw new InvalidParameterValueException("Invalid template id specified for Datadisk template" + datadiskTemplateToDiskOffering.getKey()); + throw new InvalidParameterValueException("Invalid Template ID specified for Datadisk Template" + datadiskTemplateToDiskOffering.getKey()); } long dataDiskTemplateId = datadiskTemplateToDiskOffering.getKey(); if (!dataDiskTemplate.getParentTemplateId().equals(template.getId())) { - throw new InvalidParameterValueException(String.format("Invalid Datadisk template. Specified Datadisk template %s doesn't belong to template %s", dataDiskTemplate, template)); + throw new InvalidParameterValueException(String.format("Invalid Datadisk Template. Specified Datadisk Template %s doesn't belong to Template %s", dataDiskTemplate, template)); } if (dataDiskOffering == null) { - throw new InvalidParameterValueException(String.format("Invalid disk offering %s specified for datadisk template %s", datadiskTemplateToDiskOffering.getValue(), dataDiskTemplate)); + throw new InvalidParameterValueException(String.format("Invalid disk offering %s specified for datadisk Template %s", datadiskTemplateToDiskOffering.getValue(), dataDiskTemplate)); } if (dataDiskOffering.isCustomized()) { - throw new InvalidParameterValueException(String.format("Invalid disk offering %s specified for datadisk template %s. Custom Disk offerings are not supported for Datadisk templates", dataDiskOffering, dataDiskTemplate)); + throw new InvalidParameterValueException(String.format("Invalid disk offering %s specified for datadisk Template %s. Custom Disk offerings are not supported for Datadisk Templates", dataDiskOffering, dataDiskTemplate)); } if (dataDiskOffering.getDiskSize() < dataDiskTemplate.getSize()) { - throw new InvalidParameterValueException(String.format("Invalid disk offering %s specified for datadisk template %s. Disk offering size should be greater than or equal to the template size", dataDiskOffering, dataDiskTemplate)); + throw new InvalidParameterValueException(String.format("Invalid disk offering %s specified for datadisk Template %s. Disk offering size should be greater than or equal to the Template size", dataDiskOffering, dataDiskTemplate)); } _templateDao.loadDetails(dataDiskTemplate); resourceLimitService.checkVolumeResourceLimit(owner, true, dataDiskOffering.getDiskSize(), dataDiskOffering); @@ -5545,7 +5539,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (answer != null && answer instanceof RestoreVMSnapshotAnswer) { RestoreVMSnapshotAnswer restoreVMSnapshotAnswer = (RestoreVMSnapshotAnswer) answer; if (restoreVMSnapshotAnswer == null || !restoreVMSnapshotAnswer.getResult()) { - logger.warn("Unable to restore the vm snapshot from image file to the VM: " + restoreVMSnapshotAnswer.getDetails()); + logger.warn("Unable to restore the Instance Snapshot from image file to the Instance: " + restoreVMSnapshotAnswer.getDetails()); } } @@ -5674,13 +5668,135 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return startVirtualMachine(vmId, podId, clusterId, hostId, additionalParams, deploymentPlannerToUse, true); } + private Pair> startVirtualMachineUnchecked(UserVmVO vm, VMTemplateVO template, Long podId, + Long clusterId, Long hostId, @NotNull Map additionalParams, String deploymentPlannerToUse, + boolean isExplicitHost, boolean isRootAdmin) throws ResourceUnavailableException, InsufficientCapacityException { + + // check if vm is security group enabled + if (_securityGroupMgr.isVmSecurityGroupEnabled(vm.getId()) && _securityGroupMgr.getSecurityGroupsForVm(vm.getId()).isEmpty() + && !_securityGroupMgr.isVmMappedToDefaultSecurityGroup(vm.getId()) && _networkModel.canAddDefaultSecurityGroup()) { + // if vm is not mapped to security group, create a mapping + if (logger.isDebugEnabled()) { + logger.debug("Vm " + vm + " is security group enabled, but not mapped to default security group; creating the mapping automatically"); + } + + SecurityGroup defaultSecurityGroup = _securityGroupMgr.getDefaultSecurityGroup(vm.getAccountId()); + if (defaultSecurityGroup != null) { + List groupList = new ArrayList<>(); + groupList.add(defaultSecurityGroup.getId()); + _securityGroupMgr.addInstanceToGroups(vm, groupList); + } + } + + // Choose deployment planner + // Host takes 1st preference, Cluster takes 2nd preference and Pod takes 3rd + // Default behaviour is invoked when host, cluster or pod are not specified + Pod destinationPod = getDestinationPod(podId, isRootAdmin); + Cluster destinationCluster = getDestinationCluster(clusterId, isRootAdmin); + HostVO destinationHost = getDestinationHost(hostId, isRootAdmin, isExplicitHost); + DataCenterDeployment plan = null; + boolean deployOnGivenHost = false; + if (destinationHost != null) { + logger.debug("Destination Host to deploy the VM is specified, specifying a deployment plan to deploy the VM"); + _hostDao.loadHostTags(destinationHost); + validateStrictHostTagCheck(vm, destinationHost); + + final ServiceOfferingVO offering = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); + Pair cpuCapabilityAndCapacity = _capacityMgr.checkIfHostHasCpuCapabilityAndCapacity(destinationHost, offering, false); + if (!cpuCapabilityAndCapacity.first() || !cpuCapabilityAndCapacity.second()) { + String errorMsg; + if (!cpuCapabilityAndCapacity.first()) { + errorMsg = String.format("Cannot deploy the VM to specified host %s, requested CPU and speed is more than the host capability", destinationHost); + } else { + errorMsg = String.format("Cannot deploy the VM to specified host %s, host does not have enough free CPU or RAM, please check the logs", destinationHost); + } + logger.info(errorMsg); + if (!AllowDeployVmIfGivenHostFails.value()) { + throw new InvalidParameterValueException(errorMsg); + } + } else { + plan = new DataCenterDeployment(vm.getDataCenterId(), destinationHost.getPodId(), destinationHost.getClusterId(), destinationHost.getId(), null, null); + if (!AllowDeployVmIfGivenHostFails.value()) { + deployOnGivenHost = true; + } + } + } else if (destinationCluster != null) { + logger.debug("Destination Cluster to deploy the VM is specified, specifying a deployment plan to deploy the VM"); + plan = new DataCenterDeployment(vm.getDataCenterId(), destinationCluster.getPodId(), destinationCluster.getId(), null, null, null); + if (!AllowDeployVmIfGivenHostFails.value()) { + deployOnGivenHost = true; + } + } else if (destinationPod != null) { + logger.debug("Destination Pod to deploy the VM is specified, specifying a deployment plan to deploy the VM"); + plan = new DataCenterDeployment(vm.getDataCenterId(), destinationPod.getId(), null, null, null, null); + if (!AllowDeployVmIfGivenHostFails.value()) { + deployOnGivenHost = true; + } + } + + // Set parameters + Map params = null; + if (vm.isUpdateParameters()) { + _vmDao.loadDetails(vm); + String password = getCurrentVmPasswordOrDefineNewPassword(String.valueOf(additionalParams.getOrDefault(VirtualMachineProfile.Param.VmPassword, "")), vm, template); + if (!validPassword(password)) { + throw new InvalidParameterValueException("A valid password for this virtual machine was not provided."); + } + // Check if an SSH key pair was selected for the instance and if so + // use it to encrypt & save the vm password + encryptAndStorePassword(vm, password); + params = createParameterInParameterMap(params, additionalParams, VirtualMachineProfile.Param.VmPassword, password); + } + + if (additionalParams.containsKey(VirtualMachineProfile.Param.BootIntoSetup)) { + if (!HypervisorType.VMware.equals(vm.getHypervisorType())) { + throw new InvalidParameterValueException(ApiConstants.BOOT_INTO_SETUP + " makes no sense for " + vm.getHypervisorType()); + } + Object paramValue = additionalParams.get(VirtualMachineProfile.Param.BootIntoSetup); + if (logger.isTraceEnabled()) { + logger.trace("It was specified whether to enter setup mode: " + paramValue.toString()); + } + params = createParameterInParameterMap(params, additionalParams, VirtualMachineProfile.Param.BootIntoSetup, paramValue); + } + + VirtualMachineEntity vmEntity = _orchSrvc.getVirtualMachine(vm.getUuid()); + + DeploymentPlanner planner = null; + if (deploymentPlannerToUse != null) { + // if set to null, the deployment planner would be later figured out either from global config var, or from + // the service offering + planner = _planningMgr.getDeploymentPlannerByName(deploymentPlannerToUse); + if (planner == null) { + throw new InvalidParameterValueException("Can't find a planner by name " + deploymentPlannerToUse); + } + } + vmEntity.setParamsToEntity(additionalParams); + + UserVO callerUser = _userDao.findById(CallContext.current().getCallingUserId()); + String reservationId = vmEntity.reserve(planner, plan, new ExcludeList(), Long.toString(callerUser.getId())); + vmEntity.deploy(reservationId, Long.toString(callerUser.getId()), params, deployOnGivenHost); + + Pair> vmParamPair = new Pair(vm, params); + if (vm.isUpdateParameters()) { + // this value is not being sent to the backend; need only for api + // display purposes + if (template.isEnablePassword()) { + if (vm.getDetail(VmDetailConstants.PASSWORD) != null) { + vmInstanceDetailsDao.removeDetail(vm.getId(), VmDetailConstants.PASSWORD); + } + vm.setUpdateParameters(false); + _vmDao.update(vm.getId(), vm); + } + } + return vmParamPair; + } + @Override public Pair> startVirtualMachine(long vmId, Long podId, Long clusterId, Long hostId, @NotNull Map additionalParams, String deploymentPlannerToUse, boolean isExplicitHost) throws ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { // Input validation final Account callerAccount = CallContext.current().getCallingAccount(); - UserVO callerUser = _userDao.findById(CallContext.current().getCallingUserId()); // if account is removed, return error if (callerAccount == null || callerAccount.getRemoved() != null) { @@ -5708,138 +5824,26 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (owner.getState() == Account.State.DISABLED) { throw new PermissionDeniedException(String.format("The owner of %s is disabled: %s", vm, owner)); } - Pair> vmParamPair; - try (CheckedReservation vmReservation = new CheckedReservation(owner, ResourceType.user_vm, vm.getId(), null, 1L, reservationDao, _resourceLimitMgr)) { - VMTemplateVO template = _templateDao.findByIdIncludingRemoved(vm.getTemplateId()); - if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) { - // check if account/domain is with in resource limits to start a new vm - ServiceOfferingVO offering = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); - resourceLimitService.checkVmResourceLimit(owner, vm.isDisplayVm(), offering, template); + boolean isRootAdmin = _accountService.isRootAdmin(callerAccount.getId()); + + VMTemplateVO template = _templateDao.findByIdIncludingRemoved(vm.getTemplateId()); + if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) { + ServiceOfferingVO offering = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); + List resourceLimitHostTags = resourceLimitService.getResourceLimitHostTags(offering, template); + try (CheckedReservation vmReservation = new CheckedReservation(owner, ResourceType.user_vm, resourceLimitHostTags, 1l, reservationDao, resourceLimitService); + CheckedReservation cpuReservation = new CheckedReservation(owner, ResourceType.cpu, resourceLimitHostTags, Long.valueOf(offering.getCpu()), reservationDao, resourceLimitService); + CheckedReservation memReservation = new CheckedReservation(owner, ResourceType.memory, resourceLimitHostTags, Long.valueOf(offering.getRamSize()), reservationDao, resourceLimitService); + ) { + return startVirtualMachineUnchecked(vm, template, podId, clusterId, hostId, additionalParams, deploymentPlannerToUse, isExplicitHost, isRootAdmin); + } catch (ResourceAllocationException | CloudRuntimeException e) { + throw e; + } catch (Exception e) { + logger.error("Failed to start VM {} : error during resource reservation and allocation", e); + throw new CloudRuntimeException(e); } - // check if vm is security group enabled - if (_securityGroupMgr.isVmSecurityGroupEnabled(vmId) && _securityGroupMgr.getSecurityGroupsForVm(vmId).isEmpty() - && !_securityGroupMgr.isVmMappedToDefaultSecurityGroup(vmId) && _networkModel.canAddDefaultSecurityGroup()) { - // if vm is not mapped to security group, create a mapping - if (logger.isDebugEnabled()) { - logger.debug("Vm " + vm + " is security group enabled, but not mapped to default security group; creating the mapping automatically"); - } - - SecurityGroup defaultSecurityGroup = _securityGroupMgr.getDefaultSecurityGroup(vm.getAccountId()); - if (defaultSecurityGroup != null) { - List groupList = new ArrayList<>(); - groupList.add(defaultSecurityGroup.getId()); - _securityGroupMgr.addInstanceToGroups(vm, groupList); - } - } - // Choose deployment planner - // Host takes 1st preference, Cluster takes 2nd preference and Pod takes 3rd - // Default behaviour is invoked when host, cluster or pod are not specified - boolean isRootAdmin = _accountService.isRootAdmin(callerAccount.getId()); - Pod destinationPod = getDestinationPod(podId, isRootAdmin); - Cluster destinationCluster = getDestinationCluster(clusterId, isRootAdmin); - HostVO destinationHost = getDestinationHost(hostId, isRootAdmin, isExplicitHost); - DataCenterDeployment plan = null; - boolean deployOnGivenHost = false; - if (destinationHost != null) { - logger.debug("Destination Host to deploy the VM is specified, specifying a deployment plan to deploy the VM"); - _hostDao.loadHostTags(destinationHost); - validateStrictHostTagCheck(vm, destinationHost); - - final ServiceOfferingVO offering = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); - Pair cpuCapabilityAndCapacity = _capacityMgr.checkIfHostHasCpuCapabilityAndCapacity(destinationHost, offering, false); - if (!cpuCapabilityAndCapacity.first() || !cpuCapabilityAndCapacity.second()) { - String errorMsg; - if (!cpuCapabilityAndCapacity.first()) { - errorMsg = String.format("Cannot deploy the VM to specified host %s, requested CPU and speed is more than the host capability", destinationHost); - } else { - errorMsg = String.format("Cannot deploy the VM to specified host %s, host does not have enough free CPU or RAM, please check the logs", destinationHost); - } - logger.info(errorMsg); - if (!AllowDeployVmIfGivenHostFails.value()) { - throw new InvalidParameterValueException(errorMsg); - } - } else { - plan = new DataCenterDeployment(vm.getDataCenterId(), destinationHost.getPodId(), destinationHost.getClusterId(), destinationHost.getId(), null, null); - if (!AllowDeployVmIfGivenHostFails.value()) { - deployOnGivenHost = true; - } - } - } else if (destinationCluster != null) { - logger.debug("Destination Cluster to deploy the VM is specified, specifying a deployment plan to deploy the VM"); - plan = new DataCenterDeployment(vm.getDataCenterId(), destinationCluster.getPodId(), destinationCluster.getId(), null, null, null); - if (!AllowDeployVmIfGivenHostFails.value()) { - deployOnGivenHost = true; - } - } else if (destinationPod != null) { - logger.debug("Destination Pod to deploy the VM is specified, specifying a deployment plan to deploy the VM"); - plan = new DataCenterDeployment(vm.getDataCenterId(), destinationPod.getId(), null, null, null, null); - if (!AllowDeployVmIfGivenHostFails.value()) { - deployOnGivenHost = true; - } - } - - // Set parameters - Map params = null; - if (vm.isUpdateParameters()) { - _vmDao.loadDetails(vm); - - String password = getCurrentVmPasswordOrDefineNewPassword(String.valueOf(additionalParams.getOrDefault(VirtualMachineProfile.Param.VmPassword, "")), vm, template); - - if (!validPassword(password)) { - throw new InvalidParameterValueException("A valid password for this virtual machine was not provided."); - } - - // Check if an SSH key pair was selected for the instance and if so - // use it to encrypt & save the vm password - encryptAndStorePassword(vm, password); - - params = createParameterInParameterMap(params, additionalParams, VirtualMachineProfile.Param.VmPassword, password); - } - - if (additionalParams.containsKey(VirtualMachineProfile.Param.BootIntoSetup)) { - if (!HypervisorType.VMware.equals(vm.getHypervisorType())) { - throw new InvalidParameterValueException(ApiConstants.BOOT_INTO_SETUP + " makes no sense for " + vm.getHypervisorType()); - } - Object paramValue = additionalParams.get(VirtualMachineProfile.Param.BootIntoSetup); - if (logger.isTraceEnabled()) { - logger.trace("It was specified whether to enter setup mode: " + paramValue.toString()); - } - params = createParameterInParameterMap(params, additionalParams, VirtualMachineProfile.Param.BootIntoSetup, paramValue); - } - - VirtualMachineEntity vmEntity = _orchSrvc.getVirtualMachine(vm.getUuid()); - - DeploymentPlanner planner = null; - if (deploymentPlannerToUse != null) { - // if set to null, the deployment planner would be later figured out either from global config var, or from - // the service offering - planner = _planningMgr.getDeploymentPlannerByName(deploymentPlannerToUse); - if (planner == null) { - throw new InvalidParameterValueException("Can't find a planner by name " + deploymentPlannerToUse); - } - } - vmEntity.setParamsToEntity(additionalParams); - - String reservationId = vmEntity.reserve(planner, plan, new ExcludeList(), Long.toString(callerUser.getId())); - vmEntity.deploy(reservationId, Long.toString(callerUser.getId()), params, deployOnGivenHost); - - vmParamPair = new Pair(vm, params); - if (vm != null && vm.isUpdateParameters()) { - // this value is not being sent to the backend; need only for api - // display purposes - if (template.isEnablePassword()) { - if (vm.getDetail(VmDetailConstants.PASSWORD) != null) { - vmInstanceDetailsDao.removeDetail(vm.getId(), VmDetailConstants.PASSWORD); - } - vm.setUpdateParameters(false); - _vmDao.update(vm.getId(), vm); - } - } - } catch (Exception e) { - logger.error("Failed to start VM {}", vm, e); - throw new CloudRuntimeException("Failed to start VM " + vm, e); + } else { + return startVirtualMachineUnchecked(vm, template, podId, clusterId, hostId, additionalParams, deploymentPlannerToUse, isExplicitHost, isRootAdmin); } - return vmParamPair; } /** @@ -7025,7 +7029,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // Check that Vm does not have VM Snapshots if (_vmSnapshotDao.findByVm(vmId).size() > 0) { - throw new InvalidParameterValueException("VM's disk cannot be migrated, please remove all the VM Snapshots for this VM"); + throw new InvalidParameterValueException("Instance's disk cannot be migrated, please remove all the Instance Snapshots for this Instance"); } return vm; @@ -7439,7 +7443,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (!isServiceOfferingUsingPlannerInPreferredMode(vm.getServiceOfferingId())) { //Check if all vms on destination host are created using strict implicit mode if (!checkIfAllVmsCreatedInStrictMode(accountOfVm, vmsOnDest)) { - msg = String.format("VM of account %d with strict implicit deployment planner being migrated to host %s not having all vms strict implicitly dedicated to account %d", accountOfVm, destHost, accountOfVm); + msg = String.format("Instance of Account %d with strict implicit deployment planner being migrated to host %s not having all Instances strict implicitly dedicated to Account %d", accountOfVm, destHost, accountOfVm); } } else { //If vm is deployed using preferred implicit planner, check if all vms on destination host must be @@ -7447,7 +7451,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir for (VMInstanceVO vmsDest : vmsOnDest) { ServiceOfferingVO destPlanner = serviceOfferingDao.findById(vm.getId(), vmsDest.getServiceOfferingId()); if (!((destPlanner.getDeploymentPlanner() != null && destPlanner.getDeploymentPlanner().equals("ImplicitDedicationPlanner")) && vmsDest.getAccountId() == accountOfVm)) { - msg = String.format("VM of account %d with preffered implicit deployment planner being migrated to host %s not having all vms implicitly dedicated to account %d", accountOfVm, destHost, accountOfVm); + msg = String.format("Instance of Account %d with preferred implicit deployment planner being migrated to host %s not having all Instances implicitly dedicated to Account %d", accountOfVm, destHost, accountOfVm); } } } @@ -7522,12 +7526,12 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } for (VMInstanceVO vm : allVmsOnHost) { if (!isImplicitPlannerUsedByOffering(vm.getServiceOfferingId()) || vm.getAccountId() != accountId) { - logger.info("Host {} for VM {} found to be running a vm created by a planner other than implicit, or running vms of other account", + logger.info("Host {} for Instance {} found to be running an Instance created by a planner other than implicit, or running Instances of other Account", _hostDao.findById(vm.getHostId()), vm); createdByImplicitStrict = false; break; } else if (isServiceOfferingUsingPlannerInPreferredMode(vm.getServiceOfferingId()) || vm.getAccountId() != accountId) { - logger.info("Host {} for VM {} found to be running a vm created by an implicit planner in preferred mode, or running vms of other account", + logger.info("Host {} for Instance {} found to be running an Instance created by an implicit planner in preferred mode, or running Instances of other Account", _hostDao.findById(vm.getHostId()), vm); createdByImplicitStrict = false; break; @@ -7646,7 +7650,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // Check max guest vm limit for the destinationHost. if (_capacityMgr.checkIfHostReachMaxGuestLimit(destinationHost)) { - throw new VirtualMachineMigrationException(String.format("Cannot migrate VM as destination host %s (ID: %s) already has max running vms (count includes system VMs)", + throw new VirtualMachineMigrationException(String.format("Cannot migrate Instance as destination host %s (ID: %s) already has max running Instances (count includes system VMs)", destinationHost.getName(), destinationHost.getUuid())); } @@ -7781,7 +7785,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (!vm.getHypervisorType().isFunctionalitySupported(Functionality.VmStorageMigrationWithSnapshots) && CollectionUtils.isNotEmpty(_vmSnapshotDao.findByVm(vmId))) { - throw new InvalidParameterValueException("VM with VM Snapshots cannot be migrated with storage, please remove all VM snapshots"); + throw new InvalidParameterValueException("Instance with Instance Snapshots cannot be migrated with storage, please remove all Instance Snapshots"); } Pair sourceDestinationHosts = getHostsForMigrateVmWithStorage(vm, destinationHost); @@ -8876,7 +8880,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // If target VM has associated VM snapshots then don't allow restore of VM List vmSnapshots = _vmSnapshotDao.findByVm(vmId); if (vmSnapshots.size() > 0) { - throw new InvalidParameterValueException("Unable to restore VM, please remove VM snapshots before restoring VM"); + throw new InvalidParameterValueException("Unable to restore Instance, please remove Instance Snapshots before restoring Instance"); } VMTemplateVO template = getRestoreVirtualMachineTemplate(caller, newTemplateId, rootVols, vm); From 95816b44e93341e63688c66da7d0673acb59c953 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 15 Jan 2026 10:22:28 +0530 Subject: [PATCH 10/41] extensions: allow reserved resource details Adds a new request parameter for create/updateExtension API to allow operator to provide detail names for the extension resources which will be reserved to be used by the extension. The end user won't be able to view or add details with these details names for the resource. Signed-off-by: Abhishek Kumar --- .../apache/cloudstack/api/ApiConstants.java | 1 + .../api/response/ExtensionResponse.java | 10 ++ .../cloudstack/extension/ExtensionHelper.java | 3 + .../META-INF/db/views/cloud.user_vm_view.sql | 1 + .../extensions/api/CreateExtensionCmd.java | 10 ++ .../extensions/api/UpdateExtensionCmd.java | 10 ++ .../manager/ExtensionsManagerImpl.java | 98 ++++++++-- .../api/CreateExtensionCmdTest.java | 14 ++ .../api/UpdateExtensionCmdTest.java | 15 ++ .../manager/ExtensionsManagerImplTest.java | 167 +++++++++++++++++- .../api/query/dao/UserVmJoinDaoImpl.java | 25 ++- .../com/cloud/api/query/vo/UserVmJoinVO.java | 7 + .../java/com/cloud/vm/UserVmManagerImpl.java | 9 + .../api/query/dao/UserVmJoinDaoImplTest.java | 4 + ui/public/locales/en.json | 1 + ui/src/config/section/extension.js | 2 +- ui/src/views/extension/CreateExtension.vue | 11 ++ ui/src/views/extension/UpdateExtension.vue | 17 +- 18 files changed, 371 insertions(+), 34 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 8fca652518f..4fa5f595308 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -502,6 +502,7 @@ public class ApiConstants { public static final String RECOVER = "recover"; public static final String REPAIR = "repair"; public static final String REQUIRES_HVM = "requireshvm"; + public static final String RESERVED_RESOURCE_DETAILS = "reservedresourcedetails"; public static final String RESOURCES = "resources"; public static final String RESOURCE_COUNT = "resourcecount"; public static final String RESOURCE_NAME = "resourcename"; diff --git a/api/src/main/java/org/apache/cloudstack/api/response/ExtensionResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/ExtensionResponse.java index fdf1e87df50..911839da405 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/ExtensionResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/ExtensionResponse.java @@ -85,6 +85,12 @@ public class ExtensionResponse extends BaseResponse { @Param(description = "Removal timestamp of the extension, if applicable") private Date removed; + @SerializedName(ApiConstants.RESERVED_RESOURCE_DETAILS) + @Param(description = "Resource detail names as comma separated string that should be reserved and not visible " + + "to end users", + since = "4.22.1") + protected String reservedResourceDetails; + public ExtensionResponse(String id, String name, String description, String type) { this.id = id; this.name = name; @@ -179,4 +185,8 @@ public class ExtensionResponse extends BaseResponse { public void setRemoved(Date removed) { this.removed = removed; } + + public void setReservedResourceDetails(String reservedResourceDetails) { + this.reservedResourceDetails = reservedResourceDetails; + } } diff --git a/api/src/main/java/org/apache/cloudstack/extension/ExtensionHelper.java b/api/src/main/java/org/apache/cloudstack/extension/ExtensionHelper.java index f50f841ed74..a01131278a7 100644 --- a/api/src/main/java/org/apache/cloudstack/extension/ExtensionHelper.java +++ b/api/src/main/java/org/apache/cloudstack/extension/ExtensionHelper.java @@ -17,8 +17,11 @@ package org.apache.cloudstack.extension; +import java.util.List; + public interface ExtensionHelper { Long getExtensionIdForCluster(long clusterId); Extension getExtension(long id); Extension getExtensionForCluster(long clusterId); + List getExtensionReservedResourceDetails(long extensionId); } diff --git a/engine/schema/src/main/resources/META-INF/db/views/cloud.user_vm_view.sql b/engine/schema/src/main/resources/META-INF/db/views/cloud.user_vm_view.sql index 94bc8640fd5..34aa59075ac 100644 --- a/engine/schema/src/main/resources/META-INF/db/views/cloud.user_vm_view.sql +++ b/engine/schema/src/main/resources/META-INF/db/views/cloud.user_vm_view.sql @@ -79,6 +79,7 @@ SELECT `vm_template`.`format` AS `template_format`, `vm_template`.`display_text` AS `template_display_text`, `vm_template`.`enable_password` AS `password_enabled`, + `vm_template`.`extension_id` AS `template_extension_id`, `iso`.`id` AS `iso_id`, `iso`.`uuid` AS `iso_uuid`, `iso`.`name` AS `iso_name`, diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/CreateExtensionCmd.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/CreateExtensionCmd.java index 5ab54149645..9d76a7e6ec2 100644 --- a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/CreateExtensionCmd.java +++ b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/CreateExtensionCmd.java @@ -83,6 +83,12 @@ public class CreateExtensionCmd extends BaseCmd { description = "Details in key/value pairs using format details[i].keyname=keyvalue. Example: details[0].endpoint.url=urlvalue") protected Map details; + @Parameter(name = ApiConstants.RESERVED_RESOURCE_DETAILS, type = CommandType.STRING, + description = "Resource detail names as comma separated string that should be reserved and not visible " + + "to end users", + since = "4.22.1") + protected String reservedResourceDetails; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -115,6 +121,10 @@ public class CreateExtensionCmd extends BaseCmd { return convertDetailsToMap(details); } + public String getReservedResourceDetails() { + return reservedResourceDetails; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmd.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmd.java index ded07d2dd32..5baaea1709d 100644 --- a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmd.java +++ b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmd.java @@ -78,6 +78,12 @@ public class UpdateExtensionCmd extends BaseCmd { "if false or not set, no action)") private Boolean cleanupDetails; + @Parameter(name = ApiConstants.RESERVED_RESOURCE_DETAILS, type = CommandType.STRING, + description = "Resource detail names as comma separated string that should be reserved and not visible " + + "to end users", + since = "4.22.1") + protected String reservedResourceDetails; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -106,6 +112,10 @@ public class UpdateExtensionCmd extends BaseCmd { return cleanupDetails; } + public String getReservedResourceDetails() { + return reservedResourceDetails; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java index 4171b9615fe..1422338ddc9 100644 --- a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java +++ b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java @@ -216,6 +216,11 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana @Inject AccountService accountService; + // Map of in-built extension names and their reserved resource details that shouldn't be accessible to end-users + protected static final Map> INBUILT_RESERVED_RESOURCE_DETAILS = Map.of( + "proxmox", List.of("proxmox_vmid") + ); + private ScheduledExecutorService extensionPathStateCheckExecutor; protected String getDefaultExtensionRelativePath(String name) { @@ -563,6 +568,25 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana updateExtensionPathReady(extension, true); } + protected void addInbuiltExtensionReservedResourceDetails(long extensionId, List reservedResourceDetails) { + ExtensionVO vo = extensionDao.findById(extensionId); + if (vo == null || vo.isUserDefined()) { + return; + } + String lowerName = StringUtils.defaultString(vo.getName()).toLowerCase(); + Optional>> match = INBUILT_RESERVED_RESOURCE_DETAILS.entrySet().stream() + .filter(e -> lowerName.contains(e.getKey().toLowerCase())) + .findFirst(); + if (match.isPresent()) { + Set existing = new HashSet<>(reservedResourceDetails); + for (String detailKey : match.get().getValue()) { + if (existing.add(detailKey)) { + reservedResourceDetails.add(detailKey); + } + } + } + } + @Override public String getExtensionsPath() { return externalProvisioner.getExtensionsPath(); @@ -577,6 +601,7 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana String relativePath = cmd.getPath(); final Boolean orchestratorRequiresPrepareVm = cmd.isOrchestratorRequiresPrepareVm(); final String stateStr = cmd.getState(); + final String reservedResourceDetails = cmd.getReservedResourceDetails(); ExtensionVO extensionByName = extensionDao.findByName(name); if (extensionByName != null) { throw new CloudRuntimeException("Extension by name already exists"); @@ -624,6 +649,10 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana ApiConstants.ORCHESTRATOR_REQUIRES_PREPARE_VM, String.valueOf(orchestratorRequiresPrepareVm), false)); } + if (StringUtils.isNotBlank(reservedResourceDetails)) { + detailsVOList.add(new ExtensionDetailsVO(extension.getId(), + ApiConstants.RESERVED_RESOURCE_DETAILS, reservedResourceDetails, false)); + } if (CollectionUtils.isNotEmpty(detailsVOList)) { extensionDetailsDao.saveDetails(detailsVOList); } @@ -704,6 +733,7 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana final String stateStr = cmd.getState(); final Map details = cmd.getDetails(); final Boolean cleanupDetails = cmd.isCleanupDetails(); + final String reservedResourceDetails = cmd.getReservedResourceDetails(); final ExtensionVO extensionVO = extensionDao.findById(id); if (extensionVO == null) { throw new InvalidParameterValueException("Failed to find the extension"); @@ -732,7 +762,8 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana throw new CloudRuntimeException(String.format("Failed to updated the extension: %s", extensionVO.getName())); } - updateExtensionsDetails(cleanupDetails, details, orchestratorRequiresPrepareVm, id); + updateExtensionsDetails(cleanupDetails, details, orchestratorRequiresPrepareVm, reservedResourceDetails, + id); return extensionVO; }); if (StringUtils.isNotBlank(stateStr)) { @@ -748,9 +779,11 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana return result; } - protected void updateExtensionsDetails(Boolean cleanupDetails, Map details, Boolean orchestratorRequiresPrepareVm, long id) { + protected void updateExtensionsDetails(Boolean cleanupDetails, Map details, + Boolean orchestratorRequiresPrepareVm, String reservedResourceDetails, long id) { final boolean needToUpdateAllDetails = Boolean.TRUE.equals(cleanupDetails) || MapUtils.isNotEmpty(details); - if (!needToUpdateAllDetails && orchestratorRequiresPrepareVm == null) { + if (!needToUpdateAllDetails && orchestratorRequiresPrepareVm == null && + StringUtils.isBlank(reservedResourceDetails)) { return; } if (needToUpdateAllDetails) { @@ -761,6 +794,9 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana hiddenDetails.put(ApiConstants.ORCHESTRATOR_REQUIRES_PREPARE_VM, String.valueOf(orchestratorRequiresPrepareVm)); } + if (StringUtils.isNotBlank(reservedResourceDetails)) { + hiddenDetails.put(ApiConstants.RESERVED_RESOURCE_DETAILS, reservedResourceDetails); + } if (MapUtils.isNotEmpty(hiddenDetails)) { hiddenDetails.forEach((key, value) -> detailsVOList.add( new ExtensionDetailsVO(id, key, value, false))); @@ -775,15 +811,29 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana extensionDetailsDao.removeDetails(id); } } else { - ExtensionDetailsVO detailsVO = extensionDetailsDao.findDetail(id, - ApiConstants.ORCHESTRATOR_REQUIRES_PREPARE_VM); - if (detailsVO == null) { - extensionDetailsDao.persist(new ExtensionDetailsVO(id, - ApiConstants.ORCHESTRATOR_REQUIRES_PREPARE_VM, - String.valueOf(orchestratorRequiresPrepareVm), false)); - } else if (Boolean.parseBoolean(detailsVO.getValue()) != orchestratorRequiresPrepareVm) { - detailsVO.setValue(String.valueOf(orchestratorRequiresPrepareVm)); - extensionDetailsDao.update(detailsVO.getId(), detailsVO); + if (orchestratorRequiresPrepareVm != null) { + ExtensionDetailsVO detailsVO = extensionDetailsDao.findDetail(id, + ApiConstants.ORCHESTRATOR_REQUIRES_PREPARE_VM); + if (detailsVO == null) { + extensionDetailsDao.persist(new ExtensionDetailsVO(id, + ApiConstants.ORCHESTRATOR_REQUIRES_PREPARE_VM, + String.valueOf(orchestratorRequiresPrepareVm), false)); + } else if (Boolean.parseBoolean(detailsVO.getValue()) != orchestratorRequiresPrepareVm) { + detailsVO.setValue(String.valueOf(orchestratorRequiresPrepareVm)); + extensionDetailsDao.update(detailsVO.getId(), detailsVO); + } + } + if (StringUtils.isNotBlank(reservedResourceDetails)) { + ExtensionDetailsVO detailsVO = extensionDetailsDao.findDetail(id, + ApiConstants.RESERVED_RESOURCE_DETAILS); + if (detailsVO == null) { + extensionDetailsDao.persist(new ExtensionDetailsVO(id, + ApiConstants.RESERVED_RESOURCE_DETAILS, + reservedResourceDetails, false)); + } else if (!reservedResourceDetails.equals(detailsVO.getValue())) { + detailsVO.setValue(reservedResourceDetails); + extensionDetailsDao.update(detailsVO.getId(), detailsVO); + } } } } @@ -961,12 +1011,16 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana hiddenDetails = extensionDetails.second(); } else { hiddenDetails = extensionDetailsDao.listDetailsKeyPairs(extension.getId(), - List.of(ApiConstants.ORCHESTRATOR_REQUIRES_PREPARE_VM)); + List.of(ApiConstants.ORCHESTRATOR_REQUIRES_PREPARE_VM, + ApiConstants.RESERVED_RESOURCE_DETAILS)); } if (hiddenDetails.containsKey(ApiConstants.ORCHESTRATOR_REQUIRES_PREPARE_VM)) { response.setOrchestratorRequiresPrepareVm(Boolean.parseBoolean( hiddenDetails.get(ApiConstants.ORCHESTRATOR_REQUIRES_PREPARE_VM))); } + if (hiddenDetails.containsKey(ApiConstants.RESERVED_RESOURCE_DETAILS)) { + response.setReservedResourceDetails(hiddenDetails.get(ApiConstants.RESERVED_RESOURCE_DETAILS)); + } response.setObjectName(Extension.class.getSimpleName().toLowerCase()); return response; } @@ -1605,6 +1659,24 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana return extensionDao.findById(extensionId); } + @Override + public List getExtensionReservedResourceDetails(long extensionId) { + ExtensionDetailsVO detailsVO = extensionDetailsDao.findDetail(extensionId, + ApiConstants.RESERVED_RESOURCE_DETAILS); + if (detailsVO == null || !StringUtils.isNotBlank(detailsVO.getValue())) { + return Collections.emptyList(); + } + List reservedDetails = new ArrayList<>(); + String[] parts = detailsVO.getValue().split(","); + for (String part : parts) { + if (StringUtils.isNotBlank(part)) { + reservedDetails.add(part.trim()); + } + } + addInbuiltExtensionReservedResourceDetails(extensionId, reservedDetails); + return reservedDetails; + } + @Override public boolean start() { long pathStateCheckInterval = PathStateCheckInterval.value(); diff --git a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/api/CreateExtensionCmdTest.java b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/api/CreateExtensionCmdTest.java index 2edb6ea48e3..2f630966056 100644 --- a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/api/CreateExtensionCmdTest.java +++ b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/api/CreateExtensionCmdTest.java @@ -94,4 +94,18 @@ public class CreateExtensionCmdTest { setField(cmd, "details", details); assertTrue(MapUtils.isNotEmpty(cmd.getDetails())); } + + @Test + public void getReservedResourceDetailsReturnsValueWhenSet() { + setField(cmd, "reservedResourceDetails", "detail1,detail2,detail3"); + String result = cmd.getReservedResourceDetails(); + assertEquals("detail1,detail2,detail3", result); + } + + @Test + public void getReservedResourceDetailsReturnsNullWhenNotSet() { + setField(cmd, "reservedResourceDetails", null); + String result = cmd.getReservedResourceDetails(); + assertNull(result); + } } diff --git a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmdTest.java b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmdTest.java index f0a3a6fcf21..5c5c2014a52 100644 --- a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmdTest.java +++ b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmdTest.java @@ -26,6 +26,7 @@ import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.springframework.test.util.ReflectionTestUtils.setField; import java.util.EnumSet; import java.util.HashMap; @@ -134,6 +135,20 @@ public class UpdateExtensionCmdTest { assertTrue(cmd.isCleanupDetails()); } + @Test + public void getReservedResourceDetailsReturnsValueWhenSet() { + setField(cmd, "reservedResourceDetails", "detail1,detail2,detail3"); + String result = cmd.getReservedResourceDetails(); + assertEquals("detail1,detail2,detail3", result); + } + + @Test + public void getReservedResourceDetailsReturnsNullWhenNotSet() { + setField(cmd, "reservedResourceDetails", null); + String result = cmd.getReservedResourceDetails(); + assertNull(result); + } + @Test public void executeSetsExtensionResponseWhenManagerSucceeds() { Extension extension = mock(Extension.class); diff --git a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java index 085ae212b28..ff3fce06b00 100644 --- a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java +++ b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java @@ -23,11 +23,13 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.anyLong; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; @@ -40,6 +42,7 @@ import static org.mockito.Mockito.when; import java.io.File; import java.security.InvalidParameterException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Date; @@ -49,8 +52,6 @@ import java.util.List; import java.util.Map; import java.util.UUID; -import com.cloud.exception.PermissionDeniedException; -import com.cloud.user.AccountService; import org.apache.cloudstack.acl.Role; import org.apache.cloudstack.acl.RoleService; import org.apache.cloudstack.acl.RoleType; @@ -85,9 +86,11 @@ import org.apache.cloudstack.framework.extensions.dao.ExtensionResourceMapDao; import org.apache.cloudstack.framework.extensions.dao.ExtensionResourceMapDetailsDao; import org.apache.cloudstack.framework.extensions.vo.ExtensionCustomActionDetailsVO; import org.apache.cloudstack.framework.extensions.vo.ExtensionCustomActionVO; +import org.apache.cloudstack.framework.extensions.vo.ExtensionDetailsVO; import org.apache.cloudstack.framework.extensions.vo.ExtensionResourceMapVO; import org.apache.cloudstack.framework.extensions.vo.ExtensionVO; import org.apache.cloudstack.utils.identity.ManagementServerNode; +import org.apache.commons.collections.CollectionUtils; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -113,6 +116,7 @@ import com.cloud.dc.dao.ClusterDao; import com.cloud.exception.AgentUnavailableException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.OperationTimedoutException; +import com.cloud.exception.PermissionDeniedException; import com.cloud.host.Host; import com.cloud.host.dao.HostDao; import com.cloud.host.dao.HostDetailsDao; @@ -122,6 +126,7 @@ import com.cloud.org.Cluster; import com.cloud.serializer.GsonHelper; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.user.Account; +import com.cloud.user.AccountService; import com.cloud.utils.Pair; import com.cloud.utils.UuidUtils; import com.cloud.utils.db.EntityManager; @@ -664,6 +669,8 @@ public class ExtensionsManagerImplTest { when(cmd.getPath()).thenReturn(null); when(cmd.isOrchestratorRequiresPrepareVm()).thenReturn(null); when(cmd.getState()).thenReturn(null); + String reservedResourceDetails = "abc,xyz"; + when(cmd.getReservedResourceDetails()).thenReturn(reservedResourceDetails); when(extensionDao.findByName("ext1")).thenReturn(null); when(extensionDao.persist(any())).thenAnswer(inv -> { ExtensionVO extensionVO = inv.getArgument(0); @@ -671,11 +678,20 @@ public class ExtensionsManagerImplTest { return extensionVO; }); when(managementServerHostDao.listBy(any())).thenReturn(Collections.emptyList()); - + List detailsList = new ArrayList<>(); + doAnswer(inv -> { + List detailsVO = inv.getArgument(0); + detailsList.addAll(detailsVO); + return null; + }).when(extensionDetailsDao).saveDetails(anyList()); Extension ext = extensionsManager.createExtension(cmd); assertEquals("ext1", ext.getName()); verify(extensionDao).persist(any()); + assertTrue(CollectionUtils.isNotEmpty(detailsList)); + assertTrue(detailsList.stream() + .anyMatch(detail -> ApiConstants.RESERVED_RESOURCE_DETAILS.equals(detail.getName()) + && reservedResourceDetails.equals(detail.getValue()))); } @Test @@ -938,14 +954,32 @@ public class ExtensionsManagerImplTest { public void updateExtensionsDetails_SavesDetails_WhenDetailsProvided() { long extensionId = 10L; Map details = Map.of("foo", "bar", "baz", "qux"); - extensionsManager.updateExtensionsDetails(false, details, null, extensionId); + extensionsManager.updateExtensionsDetails(false, details, null, null, extensionId); verify(extensionDetailsDao).saveDetails(any()); } + @Test + public void updateExtensionsDetails_PersistReservedDetail_WhenProvided() { + long extensionId = 10L; + when(extensionDetailsDao.persist(any())).thenReturn(mock(ExtensionDetailsVO.class)); + extensionsManager.updateExtensionsDetails(false, null, null, "abc,xyz", extensionId); + verify(extensionDetailsDao).persist(any()); + } + + @Test + public void updateExtensionsDetails_UpdateReservedDetail_WhenProvided() { + long extensionId = 10L; + when(extensionDetailsDao.findDetail(anyLong(), eq(ApiConstants.RESERVED_RESOURCE_DETAILS))) + .thenReturn(mock(ExtensionDetailsVO.class)); + when(extensionDetailsDao.update(anyLong(), any())).thenReturn(true); + extensionsManager.updateExtensionsDetails(false, null, null, "abc,xyz", extensionId); + verify(extensionDetailsDao).update(anyLong(), any()); + } + @Test public void updateExtensionsDetails_DoesNothing_WhenDetailsAndCleanupAreNull() { long extensionId = 11L; - extensionsManager.updateExtensionsDetails(null, null, null, extensionId); + extensionsManager.updateExtensionsDetails(null, null, null, null, extensionId); verify(extensionDetailsDao, never()).removeDetails(anyLong()); verify(extensionDetailsDao, never()).saveDetails(any()); } @@ -953,7 +987,7 @@ public class ExtensionsManagerImplTest { @Test public void updateExtensionsDetails_RemovesDetailsOnly_WhenCleanupIsTrue() { long extensionId = 12L; - extensionsManager.updateExtensionsDetails(true, null, null, extensionId); + extensionsManager.updateExtensionsDetails(true, null, null, null, extensionId); verify(extensionDetailsDao).removeDetails(extensionId); verify(extensionDetailsDao, never()).saveDetails(any()); } @@ -961,7 +995,7 @@ public class ExtensionsManagerImplTest { @Test public void updateExtensionsDetails_PersistsOrchestratorFlag_WhenFlagIsNotNull() { long extensionId = 13L; - extensionsManager.updateExtensionsDetails(false, null, true, extensionId); + extensionsManager.updateExtensionsDetails(false, null, true, null, extensionId); verify(extensionDetailsDao).persist(any()); } @@ -970,7 +1004,7 @@ public class ExtensionsManagerImplTest { long extensionId = 14L; Map details = Map.of("foo", "bar"); doThrow(CloudRuntimeException.class).when(extensionDetailsDao).saveDetails(any()); - extensionsManager.updateExtensionsDetails(false, details, null, extensionId); + extensionsManager.updateExtensionsDetails(false, details, null, null, extensionId); } @Test @@ -1161,7 +1195,8 @@ public class ExtensionsManagerImplTest { when(externalProvisioner.getExtensionPath("entry2.sh")).thenReturn("/some/path/entry2.sh"); Map hiddenDetails = Map.of(ApiConstants.ORCHESTRATOR_REQUIRES_PREPARE_VM, "false"); - when(extensionDetailsDao.listDetailsKeyPairs(2L, List.of(ApiConstants.ORCHESTRATOR_REQUIRES_PREPARE_VM))) + when(extensionDetailsDao.listDetailsKeyPairs(2L, List.of( + ApiConstants.ORCHESTRATOR_REQUIRES_PREPARE_VM, ApiConstants.RESERVED_RESOURCE_DETAILS))) .thenReturn(hiddenDetails); EnumSet viewDetails = EnumSet.noneOf(ApiConstants.ExtensionDetails.class); @@ -2069,4 +2104,118 @@ public class ExtensionsManagerImplTest { } } + @Test + public void getExtensionReservedResourceDetailsReturnsEmptyListWhenDetailsNotFound() { + long extensionId = 1L; + when(extensionDetailsDao.findDetail(extensionId, ApiConstants.RESERVED_RESOURCE_DETAILS)).thenReturn(null); + + List result = extensionsManager.getExtensionReservedResourceDetails(extensionId); + + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + @Test + public void getExtensionReservedResourceDetailsReturnsEmptyListWhenValueIsBlank() { + long extensionId = 2L; + ExtensionDetailsVO detailsVO = mock(ExtensionDetailsVO.class); + when(detailsVO.getValue()).thenReturn(" "); + when(extensionDetailsDao.findDetail(extensionId, ApiConstants.RESERVED_RESOURCE_DETAILS)).thenReturn(detailsVO); + + List result = extensionsManager.getExtensionReservedResourceDetails(extensionId); + + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + @Test + public void getExtensionReservedResourceDetailsReturnsListOfTrimmedDetails() { + long extensionId = 3L; + ExtensionDetailsVO detailsVO = mock(ExtensionDetailsVO.class); + when(detailsVO.getValue()).thenReturn(" detail1 , detail2,detail3 "); + when(extensionDetailsDao.findDetail(extensionId, ApiConstants.RESERVED_RESOURCE_DETAILS)).thenReturn(detailsVO); + + List result = extensionsManager.getExtensionReservedResourceDetails(extensionId); + + assertNotNull(result); + assertEquals(3, result.size()); + assertEquals("detail1", result.get(0)); + assertEquals("detail2", result.get(1)); + assertEquals("detail3", result.get(2)); + } + + @Test + public void getExtensionReservedResourceDetailsHandlesEmptyPartsGracefully() { + long extensionId = 4L; + ExtensionDetailsVO detailsVO = mock(ExtensionDetailsVO.class); + when(detailsVO.getValue()).thenReturn("detail1,,detail2, ,detail3"); + when(extensionDetailsDao.findDetail(extensionId, ApiConstants.RESERVED_RESOURCE_DETAILS)).thenReturn(detailsVO); + + List result = extensionsManager.getExtensionReservedResourceDetails(extensionId); + + assertNotNull(result); + assertEquals(3, result.size()); + assertEquals("detail1", result.get(0)); + assertEquals("detail2", result.get(1)); + assertEquals("detail3", result.get(2)); + } + + @Test + public void getExtensionReservedResourceDetailsReturnsEmptyListWhenSplitResultsInNoParts() { + long extensionId = 5L; + ExtensionDetailsVO detailsVO = mock(ExtensionDetailsVO.class); + when(detailsVO.getValue()).thenReturn(","); + when(extensionDetailsDao.findDetail(extensionId, ApiConstants.RESERVED_RESOURCE_DETAILS)).thenReturn(detailsVO); + + List result = extensionsManager.getExtensionReservedResourceDetails(extensionId); + + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + @Test + public void addInbuiltExtensionReservedResourceDetailsDoesNothingWhenExtensionNotFound() { + when(extensionDao.findById(1L)).thenReturn(null); + List reservedResourceDetails = new ArrayList<>(); + extensionsManager.addInbuiltExtensionReservedResourceDetails(1L, reservedResourceDetails); + assertTrue(reservedResourceDetails.isEmpty()); + } + + @Test + public void addInbuiltExtensionReservedResourceDetailsDoesNothingForUserDefinedExtension() { + ExtensionVO extension = mock(ExtensionVO.class); + when(extension.isUserDefined()).thenReturn(true); + when(extensionDao.findById(2L)).thenReturn(extension); + List reservedResourceDetails = new ArrayList<>(); + reservedResourceDetails.add("existing-detail"); + extensionsManager.addInbuiltExtensionReservedResourceDetails(2L, reservedResourceDetails); + assertEquals(1, reservedResourceDetails.size()); + assertTrue(reservedResourceDetails.contains("existing-detail")); + } + + @Test + public void addInbuiltExtensionReservedResourceDetailsDoesNothingWhenNoMatchFound() { + ExtensionVO extension = mock(ExtensionVO.class); + when(extension.isUserDefined()).thenReturn(false); + when(extension.getName()).thenReturn("no-such-inbuilt-key-expected"); + when(extensionDao.findById(3L)).thenReturn(extension); + List reservedResourceDetails = new ArrayList<>(); + extensionsManager.addInbuiltExtensionReservedResourceDetails(3L, reservedResourceDetails); + assertTrue(reservedResourceDetails.isEmpty()); + } + + @Test + public void addInbuiltExtensionReservedResourceDetailsAddedDetails() { + ExtensionVO extension = mock(ExtensionVO.class); + when(extension.isUserDefined()).thenReturn(false); + Map.Entry> entry = + ExtensionsManagerImpl.INBUILT_RESERVED_RESOURCE_DETAILS.entrySet().iterator().next(); + when(extension.getName()).thenReturn(entry.getKey()); + when(extensionDao.findById(3L)).thenReturn(extension); + List reservedResourceDetails = new ArrayList<>(); + extensionsManager.addInbuiltExtensionReservedResourceDetails(3L, reservedResourceDetails); + assertFalse(reservedResourceDetails.isEmpty()); + assertEquals(reservedResourceDetails.size(), entry.getValue().size()); + assertTrue(reservedResourceDetails.containsAll(entry.getValue())); + } } diff --git a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java index a2f9544de39..84b259a89a0 100644 --- a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java @@ -17,14 +17,13 @@ package com.cloud.api.query.dao; import java.text.DecimalFormat; -import java.util.ArrayList; -import java.util.Collections; import java.time.LocalDate; import java.time.ZoneId; import java.time.temporal.ChronoUnit; +import java.util.ArrayList; import java.util.Calendar; +import java.util.Collections; import java.util.Date; - import java.util.HashMap; import java.util.Hashtable; import java.util.List; @@ -34,8 +33,6 @@ import java.util.stream.Collectors; import javax.inject.Inject; -import com.cloud.gpu.dao.VgpuProfileDao; -import com.cloud.service.dao.ServiceOfferingDao; import org.apache.cloudstack.affinity.AffinityGroupResponse; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; @@ -49,6 +46,7 @@ import org.apache.cloudstack.api.response.SecurityGroupResponse; import org.apache.cloudstack.api.response.UserVmResponse; import org.apache.cloudstack.api.response.VnfNicResponse; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.extension.ExtensionHelper; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.query.QueryService; import org.apache.cloudstack.vm.lease.VMLeaseManager; @@ -61,11 +59,13 @@ import com.cloud.api.ApiDBUtils; import com.cloud.api.ApiResponseHelper; import com.cloud.api.query.vo.UserVmJoinVO; import com.cloud.gpu.GPU; +import com.cloud.gpu.dao.VgpuProfileDao; import com.cloud.host.ControlState; import com.cloud.network.IpAddress; import com.cloud.network.vpc.VpcVO; import com.cloud.network.vpc.dao.VpcDao; import com.cloud.service.ServiceOfferingDetailsVO; +import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.GuestOS; import com.cloud.storage.Storage.TemplateType; @@ -92,7 +92,6 @@ import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.VmStats; import com.cloud.vm.dao.NicExtraDhcpOptionDao; import com.cloud.vm.dao.NicSecondaryIpVO; - import com.cloud.vm.dao.VMInstanceDetailsDao; @Component @@ -124,6 +123,8 @@ public class UserVmJoinDaoImpl extends GenericDaoBaseWithTagInformation VmDetailSearch; private final SearchBuilder activeVmByIsoSearch; @@ -456,7 +457,16 @@ public class UserVmJoinDaoImpl extends GenericDaoBaseWithTagInformation userVmSettingsToHide = new ArrayList<>(); + String[] parts = QueryService.UserVMDeniedDetails.value().split(","); + if (parts.length > 0) { + Collections.addAll(userVmSettingsToHide, parts); + } + if (userVm.getTemplateExtensionId() != null) { + userVmSettingsToHide.addAll(extensionHelper.getExtensionReservedResourceDetails( + userVm.getTemplateExtensionId())); + } + for (String key : userVmSettingsToHide) { resourceDetails.remove(key.trim()); } @@ -512,7 +522,6 @@ public class UserVmJoinDaoImpl extends GenericDaoBaseWithTagInformation (item).trim()) .collect(Collectors.toList()); userDenyListedSettings.addAll(QueryService.RootAdminOnlyVmSettings); + if (template != null && template.getExtensionId() != null) { + userDenyListedSettings.addAll(extensionHelper.getExtensionReservedResourceDetails( + template.getExtensionId())); + } + final List userReadOnlySettings = Stream.of(QueryService.UserVMReadOnlyDetails.value().split(",")) .map(item -> (item).trim()) .collect(Collectors.toList()); diff --git a/server/src/test/java/com/cloud/api/query/dao/UserVmJoinDaoImplTest.java b/server/src/test/java/com/cloud/api/query/dao/UserVmJoinDaoImplTest.java index 14074add021..7dc3a486779 100755 --- a/server/src/test/java/com/cloud/api/query/dao/UserVmJoinDaoImplTest.java +++ b/server/src/test/java/com/cloud/api/query/dao/UserVmJoinDaoImplTest.java @@ -26,6 +26,7 @@ import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ResponseObject; import org.apache.cloudstack.api.response.UserVmResponse; +import org.apache.cloudstack.extension.ExtensionHelper; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -78,6 +79,9 @@ public class UserVmJoinDaoImplTest extends GenericDaoBaseWithTagInformationBaseT @Mock private VnfTemplateDetailsDao vnfTemplateDetailsDao; + @Mock + ExtensionHelper extensionHelper; + private UserVmJoinVO userVm = new UserVmJoinVO(); private UserVmResponse userVmResponse = new UserVmResponse(); diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 4f450e940fc..00078c0a2b5 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -2087,6 +2087,7 @@ "label.requireshvm": "HVM", "label.requiresupgrade": "Requires upgrade", "label.reserved": "Reserved", +"label.reservedresourcedetails": "Reserved resource details", "label.reserved.system.gateway": "Reserved system gateway", "label.reserved.system.ip": "Reserved system IP", "label.reserved.system.netmask": "Reserved system netmask", diff --git a/ui/src/config/section/extension.js b/ui/src/config/section/extension.js index 4c6d9ebf076..5904abae30b 100644 --- a/ui/src/config/section/extension.js +++ b/ui/src/config/section/extension.js @@ -44,7 +44,7 @@ export default { }, 'created'] return fields }, - details: ['name', 'description', 'id', 'type', 'details', 'path', 'pathready', 'isuserdefined', 'orchestratorrequirespreparevm', 'created'], + details: ['name', 'description', 'id', 'type', 'details', 'path', 'pathready', 'isuserdefined', 'orchestratorrequirespreparevm', 'reservedresourcedetails', 'created'], filters: ['orchestrator'], tabs: [{ name: 'details', diff --git a/ui/src/views/extension/CreateExtension.vue b/ui/src/views/extension/CreateExtension.vue index 9a7e0c8ae76..6cbd934a1f2 100644 --- a/ui/src/views/extension/CreateExtension.vue +++ b/ui/src/views/extension/CreateExtension.vue @@ -89,6 +89,14 @@ + + + +