Skip to content

Commit 3d02f09

Browse files
bernardodemarcodhslove
bernardodemarco
authored and
dhslove
committed
Remove the validation of the amount of acquired public IPs when enabling static NAT, adding PF and LB rules on VPC public IPs (apache#10568)
* fix inconsistency on public IP limits * fix log message
1 parent 78af30a commit 3d02f09

File tree

2 files changed

+68
-57
lines changed

2 files changed

+68
-57
lines changed

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

Lines changed: 56 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,36 +1038,48 @@ public List<IPAddressVO> listAvailablePublicIps(final long dcId, final Long podI
10381038
@Override
10391039
public void markPublicIpAsAllocated(final IPAddressVO addr) {
10401040
synchronized (allocatedLock) {
1041-
Transaction.execute(new TransactionCallbackNoReturn() {
1041+
Transaction.execute(new TransactionCallbackWithExceptionNoReturn<CloudRuntimeException>() {
10421042
@Override
10431043
public void doInTransactionWithoutResult(TransactionStatus status) {
10441044
Account owner = _accountMgr.getAccount(addr.getAllocatedToAccountId());
1045-
if (_ipAddressDao.lockRow(addr.getId(), true) != null) {
1046-
final IPAddressVO userIp = _ipAddressDao.findById(addr.getId());
1047-
if (userIp.getState() == IpAddress.State.Allocating || addr.getState() == IpAddress.State.Free || addr.getState() == IpAddress.State.Reserved) {
1048-
boolean shouldUpdateIpResourceCount = checkIfIpResourceCountShouldBeUpdated(addr);
1049-
addr.setState(IpAddress.State.Allocated);
1050-
if (_ipAddressDao.update(addr.getId(), addr)) {
1051-
// Save usage event
1052-
if (owner.getAccountId() != Account.ACCOUNT_ID_SYSTEM) {
1053-
VlanVO vlan = _vlanDao.findById(addr.getVlanId());
1054-
String guestType = vlan.getVlanType().toString();
1055-
if (!isIpDedicated(addr)) {
1056-
final boolean usageHidden = isUsageHidden(addr);
1057-
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_IP_ASSIGN, owner.getId(), addr.getDataCenterId(), addr.getId(),
1058-
addr.getAddress().toString(), addr.isSourceNat(), guestType, addr.getSystem(), usageHidden,
1059-
addr.getClass().getName(), addr.getUuid());
1060-
}
1061-
if (shouldUpdateIpResourceCount) {
1062-
_resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.public_ip);
1063-
}
1064-
}
1065-
} else {
1066-
logger.error("Failed to mark public IP as allocated: {}", addr);
1045+
final IPAddressVO userIp = _ipAddressDao.lockRow(addr.getId(), true);
1046+
if (userIp == null) {
1047+
logger.error(String.format("Failed to acquire row lock to mark public IP as allocated with ID [%s] and address [%s]", addr.getId(), addr.getAddress()));
1048+
return;
1049+
}
1050+
1051+
List<IpAddress.State> expectedIpAddressStates = List.of(IpAddress.State.Allocating, IpAddress.State.Free, IpAddress.State.Reserved);
1052+
if (!expectedIpAddressStates.contains(userIp.getState())) {
1053+
logger.debug(String.format("Not marking public IP with ID [%s] and address [%s] as allocated, since it is in the [%s] state.", addr.getId(), addr.getAddress(), userIp.getState()));
1054+
return;
1055+
}
1056+
1057+
boolean shouldUpdateIpResourceCount = checkIfIpResourceCountShouldBeUpdated(addr);
1058+
addr.setState(IpAddress.State.Allocated);
1059+
boolean updatedIpAddress = _ipAddressDao.update(addr.getId(), addr);
1060+
if (!updatedIpAddress) {
1061+
logger.error(String.format("Failed to mark public IP as allocated with ID [%s] and address [%s]", addr.getId(), addr.getAddress()));
1062+
return;
1063+
}
1064+
1065+
if (owner.getAccountId() != Account.ACCOUNT_ID_SYSTEM) {
1066+
if (shouldUpdateIpResourceCount) {
1067+
try (CheckedReservation publicIpReservation = new CheckedReservation(owner, ResourceType.public_ip, 1L, reservationDao, _resourceLimitMgr)) {
1068+
_resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.public_ip);
1069+
} catch (Exception e) {
1070+
_ipAddressDao.unassignIpAddress(addr.getId());
1071+
throw new CloudRuntimeException(e);
10671072
}
10681073
}
1069-
} else {
1070-
logger.error("Failed to acquire row lock to mark public IP as allocated: {}", addr);
1074+
1075+
VlanVO vlan = _vlanDao.findById(addr.getVlanId());
1076+
String guestType = vlan.getVlanType().toString();
1077+
if (!isIpDedicated(addr)) {
1078+
final boolean usageHidden = isUsageHidden(addr);
1079+
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_IP_ASSIGN, owner.getId(), addr.getDataCenterId(), addr.getId(),
1080+
addr.getAddress().toString(), addr.isSourceNat(), guestType, addr.getSystem(), usageHidden,
1081+
addr.getClass().getName(), addr.getUuid());
1082+
}
10711083
}
10721084
}
10731085
});
@@ -1553,27 +1565,31 @@ public IPAddressVO associateIPToGuestNetwork(long ipId, long networkId, boolean
15531565

15541566
boolean isSourceNat = isSourceNatAvailableForNetwork(owner, ipToAssoc, network);
15551567

1556-
logger.debug("Associating ip " + ipToAssoc + " to network " + network);
1557-
1568+
logger.debug(String.format("Associating IP [%s] to network [%s].", ipToAssoc, network));
1569+
15581570
boolean success = false;
15591571
IPAddressVO ip = null;
1560-
try (CheckedReservation publicIpReservation = new CheckedReservation(owner, ResourceType.public_ip, 1l, reservationDao, _resourceLimitMgr)) {
1561-
ip = _ipAddressDao.findById(ipId);
1562-
//update ip address with networkId
1563-
ip.setAssociatedWithNetworkId(networkId);
1564-
ip.setSourceNat(isSourceNat);
1565-
_ipAddressDao.update(ipId, ip);
1566-
1567-
success = applyIpAssociations(network, false);
1572+
try {
1573+
Pair<IPAddressVO, Boolean> updatedIpAddress = Transaction.execute((TransactionCallbackWithException<Pair<IPAddressVO, Boolean>, Exception>) status -> {
1574+
IPAddressVO ipAddress = _ipAddressDao.findById(ipId);
1575+
ipAddress.setAssociatedWithNetworkId(networkId);
1576+
ipAddress.setSourceNat(isSourceNat);
1577+
_ipAddressDao.update(ipId, ipAddress);
1578+
return new Pair<>(_ipAddressDao.findById(ipId), applyIpAssociations(network, false));
1579+
});
1580+
1581+
ip = updatedIpAddress.first();
1582+
success = updatedIpAddress.second();
15681583
if (success) {
1569-
logger.debug("Successfully associated ip address " + ip.getAddress().addr() + " to network " + network);
1584+
logger.debug(String.format("Successfully associated IP address [%s] to network [%s]", ip.getAddress().addr(), network));
15701585
} else {
1571-
logger.warn("Failed to associate ip address " + ip.getAddress().addr() + " to network " + network);
1586+
logger.warn(String.format("Failed to associate IP address [%s] to network [%s]", ip.getAddress().addr(), network));
15721587
}
1573-
return _ipAddressDao.findById(ipId);
1588+
return ip;
15741589
} catch (Exception e) {
1575-
logger.error(String.format("Failed to associate ip address %s to network %s", ipToAssoc, network), e);
1576-
throw new CloudRuntimeException(String.format("Failed to associate ip address %s to network %s", ipToAssoc, network), e);
1590+
String errorMessage = String.format("Failed to associate IP address [%s] to network [%s]", ipToAssoc, network);
1591+
logger.error(errorMessage, e);
1592+
throw new CloudRuntimeException(errorMessage, e);
15771593
} finally {
15781594
if (!success && releaseOnFailure) {
15791595
if (ip != null) {

‎server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3207,32 +3207,27 @@ public IpAddress associateIPToVpc(final long ipId, final long vpcId) throws Reso
32073207
// check permissions
32083208
_accountMgr.checkAccess(caller, null, false, owner, vpc);
32093209

3210-
logger.debug("Associating ip " + ipToAssoc + " to vpc " + vpc);
3210+
logger.debug(String.format("Associating IP [%s] to VPC [%s]", ipToAssoc, vpc));
32113211

3212-
final boolean isSourceNatFinal = isSrcNatIpRequired(vpc.getVpcOfferingId()) && getExistingSourceNatInVpc(vpc.getAccountId(), vpcId, false) == null;
3213-
try (CheckedReservation publicIpReservation = new CheckedReservation(owner, ResourceType.public_ip, 1l, reservationDao, _resourceLimitMgr)) {
3214-
Transaction.execute(new TransactionCallbackNoReturn() {
3215-
@Override
3216-
public void doInTransactionWithoutResult(final TransactionStatus status) {
3212+
final boolean isSourceNatFinal = isSrcNatIpRequired(vpc.getVpcOfferingId()) && getExistingSourceNatInVpc(vpc.getAccountId(), vpcId) == null;
3213+
try {
3214+
IPAddressVO updatedIpAddress = Transaction.execute((TransactionCallbackWithException<IPAddressVO, CloudRuntimeException>) status -> {
32173215
final IPAddressVO ip = _ipAddressDao.findById(ipId);
3218-
// update ip address with networkId
32193216
ip.setVpcId(vpcId);
32203217
ip.setSourceNat(isSourceNatFinal);
3221-
32223218
_ipAddressDao.update(ipId, ip);
3223-
3224-
// mark ip as allocated
32253219
_ipAddrMgr.markPublicIpAsAllocated(ip);
3226-
}
3220+
return _ipAddressDao.findById(ipId);
32273221
});
3222+
3223+
logger.debug(String.format("Successfully assigned IP [%s] to VPC [%s]", ipToAssoc, vpc));
3224+
CallContext.current().putContextParameter(IpAddress.class, ipToAssoc.getUuid());
3225+
return updatedIpAddress;
32283226
} catch (Exception e) {
3229-
logger.error("Failed to associate ip " + ipToAssoc + " to vpc " + vpc, e);
3230-
throw new CloudRuntimeException("Failed to associate ip " + ipToAssoc + " to vpc " + vpc, e);
3227+
String errorMessage = String.format("Failed to associate IP address [%s] to VPC [%s]", ipToAssoc, vpc);
3228+
logger.error(errorMessage, e);
3229+
throw new CloudRuntimeException(errorMessage, e);
32313230
}
3232-
3233-
logger.debug("Successfully assigned ip " + ipToAssoc + " to vpc " + vpc);
3234-
CallContext.current().putContextParameter(IpAddress.class, ipToAssoc.getUuid());
3235-
return _ipAddressDao.findById(ipId);
32363231
}
32373232

32383233
@Override

0 commit comments

Comments
 (0)