Skip to content

Remove the validation of the amount of acquired public IPs when enabling static NAT, adding PF and LB rules on VPC public IPs #10568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Pearl1594 merged 2 commits into from
Apr 22, 2025
Merged

Remove the validation of the amount of acquired public IPs when enabling static NAT, adding PF and LB rules on VPC public IPs #10568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d12df39
fix inconsistency on public IP limits
bernardodemarco Mar 13, 2025
c6b55bd
fix log message
bernardodemarco Mar 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 55 additions & 41 deletions server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1044,36 +1044,48 @@
@Override
public void markPublicIpAsAllocated(final IPAddressVO addr) {
synchronized (allocatedLock) {
Transaction.execute(new TransactionCallbackNoReturn() {
Transaction.execute(new TransactionCallbackWithExceptionNoReturn<CloudRuntimeException>() {

Check warning on line 1047 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1047 

Added line #L1047 was not covered by tests
@Override
public void doInTransactionWithoutResult(TransactionStatus status) {
Account owner = _accountMgr.getAccount(addr.getAllocatedToAccountId());
if (_ipAddressDao.lockRow(addr.getId(), true) != null) {
final IPAddressVO userIp = _ipAddressDao.findById(addr.getId());
if (userIp.getState() == IpAddress.State.Allocating || addr.getState() == IpAddress.State.Free || addr.getState() == IpAddress.State.Reserved) {
boolean shouldUpdateIpResourceCount = checkIfIpResourceCountShouldBeUpdated(addr);
addr.setState(IpAddress.State.Allocated);
if (_ipAddressDao.update(addr.getId(), addr)) {
// Save usage event
if (owner.getAccountId() != Account.ACCOUNT_ID_SYSTEM) {
VlanVO vlan = _vlanDao.findById(addr.getVlanId());
String guestType = vlan.getVlanType().toString();
if (!isIpDedicated(addr)) {
final boolean usageHidden = isUsageHidden(addr);
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_IP_ASSIGN, owner.getId(), addr.getDataCenterId(), addr.getId(),
addr.getAddress().toString(), addr.isSourceNat(), guestType, addr.getSystem(), usageHidden,
addr.getClass().getName(), addr.getUuid());
}
if (shouldUpdateIpResourceCount) {
_resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.public_ip);
}
}
} else {
s_logger.error("Failed to mark public IP as allocated with id=" + addr.getId() + " address=" + addr.getAddress());
final IPAddressVO userIp = _ipAddressDao.lockRow(addr.getId(), true);

Check warning on line 1051 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1051 

Added line #L1051 was not covered by tests
if (userIp == null) {
s_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()));
return;

Check warning on line 1054 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1053-L1054 

Added lines #L1053 - L1054 were not covered by tests
}

List<IpAddress.State> expectedIpAddressStates = List.of(IpAddress.State.Allocating, IpAddress.State.Free, IpAddress.State.Reserved);

Check warning on line 1057 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1057 

Added line #L1057 was not covered by tests
if (!expectedIpAddressStates.contains(userIp.getState())) {
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()));
return;

Check warning on line 1060 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1059-L1060 

Added lines #L1059 - L1060 were not covered by tests
}

boolean shouldUpdateIpResourceCount = checkIfIpResourceCountShouldBeUpdated(addr);
addr.setState(IpAddress.State.Allocated);
boolean updatedIpAddress = _ipAddressDao.update(addr.getId(), addr);

Check warning on line 1065 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1063-L1065 

Added lines #L1063 - L1065 were not covered by tests
if (!updatedIpAddress) {
s_logger.error(String.format("Failed to mark public IP as allocated with ID [%s] and address [%s]", addr.getId(), addr.getAddress()));
return;

Check warning on line 1068 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1067-L1068 

Added lines #L1067 - L1068 were not covered by tests
}

if (owner.getAccountId() != Account.ACCOUNT_ID_SYSTEM) {
if (shouldUpdateIpResourceCount) {
try (CheckedReservation publicIpReservation = new CheckedReservation(owner, ResourceType.public_ip, 1L, reservationDao, _resourceLimitMgr)) {
_resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.public_ip);
} catch (Exception e) {
_ipAddressDao.unassignIpAddress(addr.getId());
throw new CloudRuntimeException(e);

Check warning on line 1077 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1073-L1077 

Added lines #L1073 - L1077 were not covered by tests
}
}
} else {
s_logger.error("Failed to acquire row lock to mark public IP as allocated with id=" + addr.getId() + " address=" + addr.getAddress());

VlanVO vlan = _vlanDao.findById(addr.getVlanId());
String guestType = vlan.getVlanType().toString();

Check warning on line 1082 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1081-L1082 

Added lines #L1081 - L1082 were not covered by tests
if (!isIpDedicated(addr)) {
final boolean usageHidden = isUsageHidden(addr);
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_IP_ASSIGN, owner.getId(), addr.getDataCenterId(), addr.getId(),
addr.getAddress().toString(), addr.isSourceNat(), guestType, addr.getSystem(), usageHidden,
addr.getClass().getName(), addr.getUuid());

Check warning on line 1087 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1084-L1087 

Added lines #L1084 - L1087 were not covered by tests
}
}
}
});
Expand Down Expand Up @@ -1557,28 +1569,30 @@
}

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

s_logger.debug("Associating ip " + ipToAssoc + " to network " + network);

s_logger.debug(String.format("Associating IP [%s] to network [%s].", ipToAssoc, network));

Check warning on line 1572 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1572 

Added line #L1572 was not covered by tests
boolean success = false;
IPAddressVO ip = null;
try (CheckedReservation publicIpReservation = new CheckedReservation(owner, ResourceType.public_ip, 1l, reservationDao, _resourceLimitMgr)) {
ip = _ipAddressDao.findById(ipId);
//update ip address with networkId
ip.setAssociatedWithNetworkId(networkId);
ip.setSourceNat(isSourceNat);
_ipAddressDao.update(ipId, ip);

success = applyIpAssociations(network, false);
try {
Pair<IPAddressVO, Boolean> updatedIpAddress = Transaction.execute((TransactionCallbackWithException<Pair<IPAddressVO, Boolean>, Exception>) status -> {
IPAddressVO ipAddress = _ipAddressDao.findById(ipId);
ipAddress.setAssociatedWithNetworkId(networkId);
ipAddress.setSourceNat(isSourceNat);
_ipAddressDao.update(ipId, ipAddress);
return new Pair<>(_ipAddressDao.findById(ipId), applyIpAssociations(network, false));

Check warning on line 1581 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1575-L1581 

Added lines #L1575 - L1581 were not covered by tests
});

ip = updatedIpAddress.first();
success = updatedIpAddress.second();

Check warning on line 1585 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1584-L1585 

Added lines #L1584 - L1585 were not covered by tests
if (success) {
s_logger.debug("Successfully associated ip address " + ip.getAddress().addr() + " to network " + network);
s_logger.debug(String.format("Successfully associated IP address [%s] to network [%s]", ip.getAddress().addr(), network));

Check warning on line 1587 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1587 

Added line #L1587 was not covered by tests
} else {
s_logger.warn("Failed to associate ip address " + ip.getAddress().addr() + " to network " + network);
s_logger.warn(String.format("Failed to associate IP address [%s] to network [%s]", ip.getAddress().addr(), network));

Check warning on line 1589 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1589 

Added line #L1589 was not covered by tests
}
return _ipAddressDao.findById(ipId);
return ip;

Check warning on line 1591 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1591 

Added line #L1591 was not covered by tests
} catch (Exception e) {
s_logger.error(String.format("Failed to associate ip address %s to network %s", ipToAssoc, network), e);
throw new CloudRuntimeException(String.format("Failed to associate ip address %s to network %s", ipToAssoc, network), e);
String errorMessage = String.format("Failed to associate IP address [%s] to network [%s]", ipToAssoc, network);
s_logger.error(errorMessage, e);
throw new CloudRuntimeException(errorMessage, e);

Check warning on line 1595 in server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java#L1593-L1595 

Added lines #L1593 - L1595 were not covered by tests
} finally {
if (!success && releaseOnFailure) {
if (ip != null) {
Expand Down
28 changes: 11 additions & 17 deletions server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;

import com.cloud.resourcelimit.CheckedReservation;
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
import org.apache.cloudstack.alert.AlertService;
import org.apache.cloudstack.annotation.AnnotationService;
Expand Down Expand Up @@ -2928,32 +2927,27 @@
// check permissions
_accountMgr.checkAccess(caller, null, false, owner, vpc);

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

Check warning on line 2930 in server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java#L2930 

Added line #L2930 was not covered by tests

final boolean isSourceNatFinal = isSrcNatIpRequired(vpc.getVpcOfferingId()) && getExistingSourceNatInVpc(vpc.getAccountId(), vpcId) == null;
try (CheckedReservation publicIpReservation = new CheckedReservation(owner, ResourceType.public_ip, 1l, reservationDao, _resourceLimitMgr)) {
Transaction.execute(new TransactionCallbackNoReturn() {
@Override
public void doInTransactionWithoutResult(final TransactionStatus status) {
try {
IPAddressVO updatedIpAddress = Transaction.execute((TransactionCallbackWithException<IPAddressVO, CloudRuntimeException>) status -> {

Check warning on line 2934 in server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java#L2933-L2934 

Added lines #L2933 - L2934 were not covered by tests
final IPAddressVO ip = _ipAddressDao.findById(ipId);
// update ip address with networkId
ip.setVpcId(vpcId);
ip.setSourceNat(isSourceNatFinal);

_ipAddressDao.update(ipId, ip);

// mark ip as allocated
_ipAddrMgr.markPublicIpAsAllocated(ip);
}
return _ipAddressDao.findById(ipId);

Check warning on line 2940 in server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java#L2940 

Added line #L2940 was not covered by tests
});

s_logger.debug(String.format("Successfully assigned IP [%s] to VPC [%s]", ipToAssoc, vpc));
CallContext.current().putContextParameter(IpAddress.class, ipToAssoc.getUuid());
return updatedIpAddress;

Check warning on line 2945 in server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java#L2943-L2945 

Added lines #L2943 - L2945 were not covered by tests
} catch (Exception e) {
s_logger.error("Failed to associate ip " + ipToAssoc + " to vpc " + vpc, e);
throw new CloudRuntimeException("Failed to associate ip " + ipToAssoc + " to vpc " + vpc, e);
String errorMessage = String.format("Failed to associate IP address [%s] to VPC [%s]", ipToAssoc, vpc);
s_logger.error(errorMessage, e);
throw new CloudRuntimeException(errorMessage, e);

Check warning on line 2949 in server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java#L2947-L2949 

Added lines #L2947 - L2949 were not covered by tests
}

s_logger.debug("Successfully assigned ip " + ipToAssoc + " to vpc " + vpc);
CallContext.current().putContextParameter(IpAddress.class, ipToAssoc.getUuid());
return _ipAddressDao.findById(ipId);
}

@Override
Expand Down
Loading