Skip to content

Commit d12df39

Browse files
bernardodemarcobernardodemarco
committed
fix inconsistency on public IP limits
1 parent 95c2481 commit d12df39

File tree

2 files changed

+66
-58
lines changed

2 files changed

+66
-58
lines changed

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

Lines changed: 55 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,36 +1044,48 @@ public List<IPAddressVO> listAvailablePublicIps(final long dcId, final Long podI
10441044
@Override
10451045
public void markPublicIpAsAllocated(final IPAddressVO addr) {
10461046
synchronized (allocatedLock) {
1047-
Transaction.execute(new TransactionCallbackNoReturn() {
1047+
Transaction.execute(new TransactionCallbackWithExceptionNoReturn<CloudRuntimeException>() {
10481048
@Override
10491049
public void doInTransactionWithoutResult(TransactionStatus status) {
10501050
Account owner = _accountMgr.getAccount(addr.getAllocatedToAccountId());
1051-
if (_ipAddressDao.lockRow(addr.getId(), true) != null) {
1052-
final IPAddressVO userIp = _ipAddressDao.findById(addr.getId());
1053-
if (userIp.getState() == IpAddress.State.Allocating || addr.getState() == IpAddress.State.Free || addr.getState() == IpAddress.State.Reserved) {
1054-
boolean shouldUpdateIpResourceCount = checkIfIpResourceCountShouldBeUpdated(addr);
1055-
addr.setState(IpAddress.State.Allocated);
1056-
if (_ipAddressDao.update(addr.getId(), addr)) {
1057-
// Save usage event
1058-
if (owner.getAccountId() != Account.ACCOUNT_ID_SYSTEM) {
1059-
VlanVO vlan = _vlanDao.findById(addr.getVlanId());
1060-
String guestType = vlan.getVlanType().toString();
1061-
if (!isIpDedicated(addr)) {
1062-
final boolean usageHidden = isUsageHidden(addr);
1063-
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_IP_ASSIGN, owner.getId(), addr.getDataCenterId(), addr.getId(),
1064-
addr.getAddress().toString(), addr.isSourceNat(), guestType, addr.getSystem(), usageHidden,
1065-
addr.getClass().getName(), addr.getUuid());
1066-
}
1067-
if (shouldUpdateIpResourceCount) {
1068-
_resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.public_ip);
1069-
}
1070-
}
1071-
} else {
1072-
s_logger.error("Failed to mark public IP as allocated with id=" + addr.getId() + " address=" + addr.getAddress());
1051+
final IPAddressVO userIp = _ipAddressDao.lockRow(addr.getId(), true);
1052+
if (userIp == null) {
1053+
s_logger.error(String.format("Failed to acquire row lock to mark public IP as allocated with ID [%s] address [%s]", addr.getId(), addr.getAddress()));
1054+
return;
1055+
}
1056+
1057+
List<IpAddress.State> expectedIpAddressStates = List.of(IpAddress.State.Allocating, IpAddress.State.Free, IpAddress.State.Reserved);
1058+
if (!expectedIpAddressStates.contains(userIp.getState())) {
1059+
s_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()));
1060+
return;
1061+
}
1062+
1063+
boolean shouldUpdateIpResourceCount = checkIfIpResourceCountShouldBeUpdated(addr);
1064+
addr.setState(IpAddress.State.Allocated);
1065+
boolean updatedIpAddress = _ipAddressDao.update(addr.getId(), addr);
1066+
if (!updatedIpAddress) {
1067+
s_logger.error(String.format("Failed to mark public IP as allocated with ID [%s] and address [%s]", addr.getId(), addr.getAddress()));
1068+
return;
1069+
}
1070+
1071+
if (owner.getAccountId() != Account.ACCOUNT_ID_SYSTEM) {
1072+
if (shouldUpdateIpResourceCount) {
1073+
try (CheckedReservation publicIpReservation = new CheckedReservation(owner, ResourceType.public_ip, 1L, reservationDao, _resourceLimitMgr)) {
1074+
_resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.public_ip);
1075+
} catch (Exception e) {
1076+
_ipAddressDao.unassignIpAddress(addr.getId());
1077+
throw new CloudRuntimeException(e);
10731078
}
10741079
}
1075-
} else {
1076-
s_logger.error("Failed to acquire row lock to mark public IP as allocated with id=" + addr.getId() + " address=" + addr.getAddress());
1080+
1081+
VlanVO vlan = _vlanDao.findById(addr.getVlanId());
1082+
String guestType = vlan.getVlanType().toString();
1083+
if (!isIpDedicated(addr)) {
1084+
final boolean usageHidden = isUsageHidden(addr);
1085+
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_IP_ASSIGN, owner.getId(), addr.getDataCenterId(), addr.getId(),
1086+
addr.getAddress().toString(), addr.isSourceNat(), guestType, addr.getSystem(), usageHidden,
1087+
addr.getClass().getName(), addr.getUuid());
1088+
}
10771089
}
10781090
}
10791091
});
@@ -1557,28 +1569,30 @@ public IPAddressVO associateIPToGuestNetwork(long ipId, long networkId, boolean
15571569
}
15581570

15591571
boolean isSourceNat = isSourceNatAvailableForNetwork(owner, ipToAssoc, network);
1560-
1561-
s_logger.debug("Associating ip " + ipToAssoc + " to network " + network);
1562-
1572+
s_logger.debug(String.format("Associating IP [%s] to network [%s].", ipToAssoc, network));
15631573
boolean success = false;
15641574
IPAddressVO ip = null;
1565-
try (CheckedReservation publicIpReservation = new CheckedReservation(owner, ResourceType.public_ip, 1l, reservationDao, _resourceLimitMgr)) {
1566-
ip = _ipAddressDao.findById(ipId);
1567-
//update ip address with networkId
1568-
ip.setAssociatedWithNetworkId(networkId);
1569-
ip.setSourceNat(isSourceNat);
1570-
_ipAddressDao.update(ipId, ip);
1571-
1572-
success = applyIpAssociations(network, false);
1575+
try {
1576+
Pair<IPAddressVO, Boolean> updatedIpAddress = Transaction.execute((TransactionCallbackWithException<Pair<IPAddressVO, Boolean>, Exception>) status -> {
1577+
IPAddressVO ipAddress = _ipAddressDao.findById(ipId);
1578+
ipAddress.setAssociatedWithNetworkId(networkId);
1579+
ipAddress.setSourceNat(isSourceNat);
1580+
_ipAddressDao.update(ipId, ipAddress);
1581+
return new Pair<>(_ipAddressDao.findById(ipId), applyIpAssociations(network, false));
1582+
});
1583+
1584+
ip = updatedIpAddress.first();
1585+
success = updatedIpAddress.second();
15731586
if (success) {
1574-
s_logger.debug("Successfully associated ip address " + ip.getAddress().addr() + " to network " + network);
1587+
s_logger.debug(String.format("Successfully associated IP address [%s] to network [%s]", ip.getAddress().addr(), network));
15751588
} else {
1576-
s_logger.warn("Failed to associate ip address " + ip.getAddress().addr() + " to network " + network);
1589+
s_logger.warn(String.format("Failed to associate IP address [%s] to network [%s]", ip.getAddress().addr(), network));
15771590
}
1578-
return _ipAddressDao.findById(ipId);
1591+
return ip;
15791592
} catch (Exception e) {
1580-
s_logger.error(String.format("Failed to associate ip address %s to network %s", ipToAssoc, network), e);
1581-
throw new CloudRuntimeException(String.format("Failed to associate ip address %s to network %s", ipToAssoc, network), e);
1593+
String errorMessage = String.format("Failed to associate IP address [%s] to network [%s]", ipToAssoc, network);
1594+
s_logger.error(errorMessage, e);
1595+
throw new CloudRuntimeException(errorMessage, e);
15821596
} finally {
15831597
if (!success && releaseOnFailure) {
15841598
if (ip != null) {

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

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
import javax.inject.Inject;
4343
import javax.naming.ConfigurationException;
4444

45-
import com.cloud.resourcelimit.CheckedReservation;
4645
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
4746
import org.apache.cloudstack.alert.AlertService;
4847
import org.apache.cloudstack.annotation.AnnotationService;
@@ -2928,32 +2927,27 @@ public IpAddress associateIPToVpc(final long ipId, final long vpcId) throws Reso
29282927
// check permissions
29292928
_accountMgr.checkAccess(caller, null, false, owner, vpc);
29302929

2931-
s_logger.debug("Associating ip " + ipToAssoc + " to vpc " + vpc);
2930+
s_logger.debug(String.format("Associating IP [%s] to VPC [%s]", ipToAssoc, vpc));
29322931

29332932
final boolean isSourceNatFinal = isSrcNatIpRequired(vpc.getVpcOfferingId()) && getExistingSourceNatInVpc(vpc.getAccountId(), vpcId) == null;
2934-
try (CheckedReservation publicIpReservation = new CheckedReservation(owner, ResourceType.public_ip, 1l, reservationDao, _resourceLimitMgr)) {
2935-
Transaction.execute(new TransactionCallbackNoReturn() {
2936-
@Override
2937-
public void doInTransactionWithoutResult(final TransactionStatus status) {
2933+
try {
2934+
IPAddressVO updatedIpAddress = Transaction.execute((TransactionCallbackWithException<IPAddressVO, CloudRuntimeException>) status -> {
29382935
final IPAddressVO ip = _ipAddressDao.findById(ipId);
2939-
// update ip address with networkId
29402936
ip.setVpcId(vpcId);
29412937
ip.setSourceNat(isSourceNatFinal);
2942-
29432938
_ipAddressDao.update(ipId, ip);
2944-
2945-
// mark ip as allocated
29462939
_ipAddrMgr.markPublicIpAsAllocated(ip);
2947-
}
2940+
return _ipAddressDao.findById(ipId);
29482941
});
2942+
2943+
s_logger.debug(String.format("Successfully assigned IP [%s] to VPC [%s]", ipToAssoc, vpc));
2944+
CallContext.current().putContextParameter(IpAddress.class, ipToAssoc.getUuid());
2945+
return updatedIpAddress;
29492946
} catch (Exception e) {
2950-
s_logger.error("Failed to associate ip " + ipToAssoc + " to vpc " + vpc, e);
2951-
throw new CloudRuntimeException("Failed to associate ip " + ipToAssoc + " to vpc " + vpc, e);
2947+
String errorMessage = String.format("Failed to associate IP address [%s] to VPC [%s]", ipToAssoc, vpc);
2948+
s_logger.error(errorMessage, e);
2949+
throw new CloudRuntimeException(errorMessage, e);
29522950
}
2953-
2954-
s_logger.debug("Successfully assigned ip " + ipToAssoc + " to vpc " + vpc);
2955-
CallContext.current().putContextParameter(IpAddress.class, ipToAssoc.getUuid());
2956-
return _ipAddressDao.findById(ipId);
29572951
}
29582952

29592953
@Override

0 commit comments

Comments
 (0)