Skip to content

Commit dc7068a

Browse files
winterhazelDaan Hoogland
winterhazel
authored and
Daan Hoogland
committed
Address public IP limit validations
1 parent e8f8aca commit dc7068a

File tree

8 files changed

+81
-39
lines changed

8 files changed

+81
-39
lines changed

‎engine/schema/src/main/java/com/cloud/dc/dao/AccountVlanMapDao.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,6 @@ public interface AccountVlanMapDao extends GenericDao<AccountVlanMapVO, Long> {
2727

2828
public List<AccountVlanMapVO> listAccountVlanMapsByVlan(long vlanDbId);
2929

30-
public AccountVlanMapVO findAccountVlanMap(long accountId, long vlanDbId);
30+
public AccountVlanMapVO findAccountVlanMap(Long accountId, long vlanDbId);
3131

3232
}

‎engine/schema/src/main/java/com/cloud/dc/dao/AccountVlanMapDaoImpl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ public List<AccountVlanMapVO> listAccountVlanMapsByVlan(long vlanDbId) {
4848
}
4949

5050
@Override
51-
public AccountVlanMapVO findAccountVlanMap(long accountId, long vlanDbId) {
51+
public AccountVlanMapVO findAccountVlanMap(Long accountId, long vlanDbId) {
5252
SearchCriteria<AccountVlanMapVO> sc = AccountVlanSearch.create();
53-
sc.setParameters("accountId", accountId);
53+
sc.setParametersIfNotNull("accountId", accountId);
5454
sc.setParameters("vlanDbId", vlanDbId);
5555
return findOneIncludingRemovedBy(sc);
5656
}

‎engine/schema/src/main/java/com/cloud/dc/dao/DomainVlanMapDao.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,5 @@
2424
public interface DomainVlanMapDao extends GenericDao<DomainVlanMapVO, Long> {
2525
public List<DomainVlanMapVO> listDomainVlanMapsByDomain(long domainId);
2626
public List<DomainVlanMapVO> listDomainVlanMapsByVlan(long vlanDbId);
27-
public DomainVlanMapVO findDomainVlanMap(long domainId, long vlanDbId);
27+
public DomainVlanMapVO findDomainVlanMap(Long domainId, long vlanDbId);
2828
}

‎engine/schema/src/main/java/com/cloud/dc/dao/DomainVlanMapDaoImpl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ public List<DomainVlanMapVO> listDomainVlanMapsByVlan(long vlanDbId) {
4646
}
4747

4848
@Override
49-
public DomainVlanMapVO findDomainVlanMap(long domainId, long vlanDbId) {
49+
public DomainVlanMapVO findDomainVlanMap(Long domainId, long vlanDbId) {
5050
SearchCriteria<DomainVlanMapVO> sc = DomainVlanSearch.create();
51-
sc.setParameters("domainId", domainId);
51+
sc.setParametersIfNotNull("domainId", domainId);
5252
sc.setParameters("vlanDbId", vlanDbId);
5353
return findOneIncludingRemovedBy(sc);
5454
}

‎server/src/main/java/com/cloud/api/ApiDBUtils.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2244,6 +2244,10 @@ public static boolean isAdmin(Account account) {
22442244
return s_accountService.isAdmin(account.getId());
22452245
}
22462246

2247+
public static Account getSystemAccount() {
2248+
return s_accountService.getSystemAccount();
2249+
}
2250+
22472251
public static List<ResourceTagJoinVO> listResourceTagViewByResourceUUID(String resourceUUID, ResourceObjectType resourceType) {
22482252
return s_tagJoinDao.listBy(resourceUUID, resourceType);
22492253
}

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

Lines changed: 51 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252

5353
import com.cloud.exception.UnsupportedServiceException;
5454
import com.cloud.network.as.AutoScaleManager;
55+
import com.cloud.resourcelimit.CheckedReservation;
5556
import com.cloud.user.AccountManagerImpl;
5657
import org.apache.cloudstack.acl.RoleType;
5758
import org.apache.cloudstack.acl.SecurityChecker;
@@ -128,6 +129,7 @@
128129
import org.apache.cloudstack.region.Region;
129130
import org.apache.cloudstack.region.RegionVO;
130131
import org.apache.cloudstack.region.dao.RegionDao;
132+
import org.apache.cloudstack.reservation.dao.ReservationDao;
131133
import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO;
132134
import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao;
133135
import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
@@ -395,6 +397,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
395397
@Inject
396398
ResourceLimitService _resourceLimitMgr;
397399
@Inject
400+
ReservationDao reservationDao;
401+
@Inject
398402
ProjectManager _projectMgr;
399403
@Inject
400404
DataStoreManager _dataStoreMgr;
@@ -4833,22 +4837,20 @@ public Vlan createVlanAndPublicIpRange(final CreateVlanIpRangeCmd cmd) throws In
48334837
throw new InvalidParameterValueException("Gateway, netmask and zoneId have to be passed in for virtual and direct untagged networks");
48344838
}
48354839

4836-
if (forVirtualNetwork) {
4837-
if (vlanOwner != null) {
4838-
4839-
final long accountIpRange = NetUtils.ip2Long(endIP) - NetUtils.ip2Long(startIP) + 1;
4840-
4841-
// check resource limits
4842-
_resourceLimitMgr.checkResourceLimit(vlanOwner, ResourceType.public_ip, accountIpRange);
4843-
}
4844-
}
48454840
// Check if the IP range overlaps with the private ip
48464841
if (ipv4) {
48474842
checkOverlapPrivateIpRange(zoneId, startIP, endIP);
48484843
}
48494844

4850-
return commitVlan(zoneId, podId, startIP, endIP, newVlanGateway, newVlanNetmask, vlanId, forVirtualNetwork, forSystemVms, networkId, physicalNetworkId, startIPv6, endIPv6, ip6Gateway,
4851-
ip6Cidr, domain, vlanOwner, network, sameSubnet, cmd.isForNsx());
4845+
long reservedIpAddressesAmount = 0L;
4846+
if (forVirtualNetwork && vlanOwner != null) {
4847+
reservedIpAddressesAmount = NetUtils.ip2Long(endIP) - NetUtils.ip2Long(startIP) + 1;
4848+
}
4849+
4850+
try (CheckedReservation publicIpReservation = new CheckedReservation(vlanOwner, ResourceType.public_ip, null, null, null, reservedIpAddressesAmount, null, reservationDao, _resourceLimitMgr)) {
4851+
return commitVlan(zoneId, podId, startIP, endIP, newVlanGateway, newVlanNetmask, vlanId, forVirtualNetwork, forSystemVms, networkId, physicalNetworkId, startIPv6, endIPv6, ip6Gateway,
4852+
ip6Cidr, domain, vlanOwner, network, sameSubnet, cmd.isForNsx());
4853+
}
48524854
}
48534855

48544856
private Network getNetwork(Long networkId) {
@@ -5377,7 +5379,7 @@ public Vlan updateVlanAndPublicIpRange(final long id, String startIp,
53775379
String endIpv6,
53785380
String ip6Gateway,
53795381
String ip6Cidr,
5380-
Boolean forSystemVms) throws ConcurrentOperationException {
5382+
Boolean forSystemVms) throws ConcurrentOperationException, ResourceAllocationException {
53815383

53825384
VlanVO vlanRange = _vlanDao.findById(id);
53835385
if (vlanRange == null) {
@@ -5397,24 +5399,50 @@ public Vlan updateVlanAndPublicIpRange(final long id, String startIp,
53975399
}
53985400
}
53995401

5402+
AccountVlanMapVO accountMap = _accountVlanMapDao.findAccountVlanMap(null, id);
5403+
Account account = accountMap != null ? _accountDao.findById(accountMap.getAccountId()) : null;
5404+
5405+
DomainVlanMapVO domainMap = _domainVlanMapDao.findDomainVlanMap(null, id);
5406+
Long domainId = domainMap != null ? domainMap.getDomainId() : null;
5407+
54005408
final Boolean isRangeForSystemVM = checkIfVlanRangeIsForSystemVM(id);
54015409
if (forSystemVms != null && isRangeForSystemVM != forSystemVms) {
54025410
if (VlanType.DirectAttached.equals(vlanRange.getVlanType())) {
54035411
throw new InvalidParameterValueException("forSystemVms is not available for this IP range with vlan type: " + VlanType.DirectAttached);
54045412
}
54055413
// Check if range has already been dedicated
5406-
final List<AccountVlanMapVO> maps = _accountVlanMapDao.listAccountVlanMapsByVlan(id);
5407-
if (maps != null && !maps.isEmpty()) {
5414+
if (account != null) {
54085415
throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to an account");
54095416
}
5410-
5411-
List<DomainVlanMapVO> domainmaps = _domainVlanMapDao.listDomainVlanMapsByVlan(id);
5412-
if (domainmaps != null && !domainmaps.isEmpty()) {
5417+
if (domainId != null) {
54135418
throw new InvalidParameterValueException("Specified Public IP range has already been dedicated to a domain");
54145419
}
54155420
}
54165421
if (ipv4) {
5422+
long existingIpAddressAmount = 0L;
5423+
long newIpAddressAmount = 0L;
5424+
5425+
if (account != null) {
5426+
// IPv4 public range is dedicated to an account (IPv6 cannot be dedicated at the moment).
5427+
// We need to update the resource count.
5428+
existingIpAddressAmount = _publicIpAddressDao.countIPs(vlanRange.getDataCenterId(), id, false);
5429+
newIpAddressAmount = NetUtils.ip2Long(endIp) - NetUtils.ip2Long(startIp) + 1;
5430+
}
5431+
5432+
try (CheckedReservation publicIpReservation = new CheckedReservation(account, ResourceType.public_ip, null, null, null, newIpAddressAmount, existingIpAddressAmount, reservationDao, _resourceLimitMgr)) {
5433+
54175434
updateVlanAndIpv4Range(id, vlanRange, startIp, endIp, gateway, netmask, isRangeForSystemVM, forSystemVms);
5435+
5436+
if (account != null) {
5437+
long countDiff = newIpAddressAmount - existingIpAddressAmount;
5438+
if (countDiff > 0) {
5439+
_resourceLimitMgr.incrementResourceCount(account.getId(), ResourceType.public_ip, countDiff);
5440+
} else if (countDiff < 0) {
5441+
_resourceLimitMgr.decrementResourceCount(account.getId(), ResourceType.public_ip, Math.abs(countDiff));
5442+
}
5443+
}
5444+
5445+
}
54185446
}
54195447
if (ipv6) {
54205448
updateVlanAndIpv6Range(id, vlanRange, startIpv6, endIpv6, ip6Gateway, ip6Cidr, isRangeForSystemVM, forSystemVms);
@@ -5801,12 +5829,6 @@ public Vlan dedicatePublicIpRange(final DedicatePublicIpRangeCmd cmd) throws Res
58015829
throw new InvalidParameterValueException("Public IP range can be dedicated to an account only in the zone of type " + NetworkType.Advanced);
58025830
}
58035831

5804-
// Check Public IP resource limits
5805-
if (vlanOwner != null) {
5806-
final int accountPublicIpRange = _publicIpAddressDao.countIPs(zoneId, vlanDbId, false);
5807-
_resourceLimitMgr.checkResourceLimit(vlanOwner, ResourceType.public_ip, accountPublicIpRange);
5808-
}
5809-
58105832
// Check if any of the Public IP addresses is allocated to another
58115833
// account
58125834
final List<IPAddressVO> ips = _publicIpAddressDao.listByVlanId(vlanDbId);
@@ -5827,6 +5849,10 @@ public Vlan dedicatePublicIpRange(final DedicatePublicIpRangeCmd cmd) throws Res
58275849
}
58285850
}
58295851

5852+
// Check Public IP resource limits
5853+
long reservedIpAddressesAmount = vlanOwner != null ? _publicIpAddressDao.countIPs(zoneId, vlanDbId, false) : 0L;
5854+
try (CheckedReservation publicIpReservation = new CheckedReservation(vlanOwner, ResourceType.public_ip, null, null, null, reservedIpAddressesAmount, null, reservationDao, _resourceLimitMgr)) {
5855+
58305856
if (vlanOwner != null) {
58315857
// Create an AccountVlanMapVO entry
58325858
final AccountVlanMapVO accountVlanMapVO = new AccountVlanMapVO(vlanOwner.getId(), vlan.getId());
@@ -5850,6 +5876,8 @@ public Vlan dedicatePublicIpRange(final DedicatePublicIpRangeCmd cmd) throws Res
58505876
}
58515877

58525878
return vlan;
5879+
5880+
}
58535881
}
58545882

58555883
@Override

‎server/src/main/java/com/cloud/network/NetworkServiceImpl.java‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import javax.inject.Inject;
4141
import javax.naming.ConfigurationException;
4242

43+
import com.cloud.resourcelimit.CheckedReservation;
4344
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
4445
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
4546
import org.apache.cloudstack.alert.AlertService;
@@ -75,6 +76,7 @@
7576
import org.apache.cloudstack.network.RoutedIpv4Manager;
7677
import org.apache.cloudstack.network.dao.NetworkPermissionDao;
7778
import org.apache.cloudstack.network.element.InternalLoadBalancerElementService;
79+
import org.apache.cloudstack.reservation.dao.ReservationDao;
7880
import org.apache.commons.collections.CollectionUtils;
7981
import org.apache.commons.collections.MapUtils;
8082
import org.apache.commons.lang3.BooleanUtils;
@@ -328,6 +330,8 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C
328330
@Inject
329331
ResourceLimitService _resourceLimitMgr;
330332
@Inject
333+
ReservationDao reservationDao;
334+
@Inject
331335
DomainManager _domainMgr;
332336
@Inject
333337
ProjectManager _projectMgr;
@@ -1143,15 +1147,10 @@ public IpAddress reserveIpAddress(Account account, Boolean displayIp, Long ipAdd
11431147
if (ipDedicatedAccountId != null && !ipDedicatedAccountId.equals(account.getAccountId())) {
11441148
throw new InvalidParameterValueException("Unable to reserve a IP because it is dedicated to another Account.");
11451149
}
1146-
if (ipDedicatedAccountId == null) {
1147-
// Check that the maximum number of public IPs for the given accountId will not be exceeded
1148-
try {
1149-
_resourceLimitMgr.checkResourceLimit(account, Resource.ResourceType.public_ip);
1150-
} catch (ResourceAllocationException ex) {
1151-
logger.warn("Failed to allocate resource of type " + ex.getResourceType() + " for account " + account);
1152-
throw new AccountLimitException("Maximum number of public IP addresses for account: " + account.getAccountName() + " has been exceeded.");
1153-
}
1154-
}
1150+
1151+
long reservedIpAddressesAmount = ipDedicatedAccountId == null ? 1L : 0L;
1152+
try (CheckedReservation publicIpAddressReservation = new CheckedReservation(account, Resource.ResourceType.public_ip, reservedIpAddressesAmount, reservationDao, _resourceLimitMgr)) {
1153+
11551154
List<AccountVlanMapVO> maps = _accountVlanMapDao.listAccountVlanMapsByVlan(ipVO.getVlanId());
11561155
ipVO.setAllocatedTime(new Date());
11571156
ipVO.setAllocatedToAccountId(account.getAccountId());
@@ -1161,10 +1160,15 @@ public IpAddress reserveIpAddress(Account account, Boolean displayIp, Long ipAdd
11611160
ipVO.setDisplay(displayIp);
11621161
}
11631162
ipVO = _ipAddressDao.persist(ipVO);
1164-
if (ipDedicatedAccountId == null) {
1163+
if (reservedIpAddressesAmount > 0) {
11651164
_resourceLimitMgr.incrementResourceCount(account.getId(), Resource.ResourceType.public_ip);
11661165
}
11671166
return ipVO;
1167+
1168+
} catch (ResourceAllocationException ex) {
1169+
logger.warn("Failed to allocate resource of type " + ex.getResourceType() + " for account " + account);
1170+
throw new AccountLimitException("Maximum number of public IP addresses for account: " + account.getAccountName() + " has been exceeded.");
1171+
}
11681172
}
11691173

11701174
@Override

‎server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.Objects;
2424
import java.util.stream.Collectors;
2525

26+
import com.cloud.api.ApiDBUtils;
2627
import org.apache.cloudstack.context.CallContext;
2728
import org.apache.cloudstack.reservation.ReservationVO;
2829
import org.apache.cloudstack.reservation.dao.ReservationDao;
@@ -146,6 +147,11 @@ public CheckedReservation(Account account, Long domainId, ResourceType resourceT
146147

147148
this.reservationDao = reservationDao;
148149
this.resourceLimitService = resourceLimitService;
150+
151+
// When allocating to a domain instead of a specific account, consider the system account as the owner for the validations here.
152+
if (account == null) {
153+
account = ApiDBUtils.getSystemAccount();
154+
}
149155
this.account = account;
150156

151157
if (domainId == null) {

0 commit comments

Comments
 (0)