etan39
/
cloudstack
mirror of https://github.com/apache/cloudstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Flexibilize public IP selection (#11076)

This commit is contained in:
Erik Böck 2026-04-21 07:53:11 -03:00 committed by GitHub
parent 1fc4cb90bf
commit 9f96c9d5eb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 92 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
engine/schema/src/main/java/com/cloud/network/dao/PublicIpQuarantineDao.java
View File

@ -19,9 +19,21 @@ package com.cloud.network.dao;
import com.cloud.network.vo.PublicIpQuarantineVO;
import com.cloud.utils.db.GenericDao;
import java.util.Date;
import java.util.List;
public interface PublicIpQuarantineDao extends GenericDao<PublicIpQuarantineVO, Long> {
PublicIpQuarantineVO findByPublicIpAddressId(long publicIpAddressId);
PublicIpQuarantineVO findByIpAddress(String publicIpAddress);
/**
* Returns a list of public IP addresses that are actively quarantined at the specified date and the previous owner differs from the specified user.
*
* @param userId used to check against the IP's previous owner;
* @param date used to check if the quarantine is active;
* @return a list of PublicIpQuarantineVOs.
*/
List<PublicIpQuarantineVO> listQuarantinedIpAddressesToUser(Long userId, Date date);
}

22
engine/schema/src/main/java/com/cloud/network/dao/PublicIpQuarantineDaoImpl.java
View File

@ -26,6 +26,8 @@ import org.springframework.stereotype.Component;
import javax.annotation.PostConstruct;
import javax.inject.Inject;
import java.util.Date;
import java.util.List;
@Component
public class PublicIpQuarantineDaoImpl extends GenericDaoBase<PublicIpQuarantineVO, Long> implements PublicIpQuarantineDao {
@ -33,6 +35,8 @@ public class PublicIpQuarantineDaoImpl extends GenericDaoBase<PublicIpQuarantine
private SearchBuilder<IPAddressVO> ipAddressSearchBuilder;
private SearchBuilder<PublicIpQuarantineVO> quarantinedIpAddressesSearch;
@Inject
IPAddressDao ipAddressDao;
@ -47,8 +51,16 @@ public class PublicIpQuarantineDaoImpl extends GenericDaoBase<PublicIpQuarantine
publicIpAddressByIdSearch.join("quarantineJoin", ipAddressSearchBuilder, ipAddressSearchBuilder.entity().getId(),
publicIpAddressByIdSearch.entity().getPublicIpAddressId(), JoinBuilder.JoinType.INNER);
quarantinedIpAddressesSearch = createSearchBuilder();
quarantinedIpAddressesSearch.and("previousOwnerId", quarantinedIpAddressesSearch.entity().getPreviousOwnerId(), SearchCriteria.Op.NEQ);
quarantinedIpAddressesSearch.and();
quarantinedIpAddressesSearch.op("removedIsNull", quarantinedIpAddressesSearch.entity().getRemoved(), SearchCriteria.Op.NULL);
quarantinedIpAddressesSearch.and("endDate", quarantinedIpAddressesSearch.entity().getEndDate(), SearchCriteria.Op.GT);
quarantinedIpAddressesSearch.cp();
ipAddressSearchBuilder.done();
publicIpAddressByIdSearch.done();
quarantinedIpAddressesSearch.done();
}
@Override
@ -68,4 +80,14 @@ public class PublicIpQuarantineDaoImpl extends GenericDaoBase<PublicIpQuarantine
return findOneBy(sc, filter);
}
@Override
public List<PublicIpQuarantineVO> listQuarantinedIpAddressesToUser(Long userId, Date date) {
SearchCriteria<PublicIpQuarantineVO> sc = quarantinedIpAddressesSearch.create();
sc.setParameters("previousOwnerId", userId);
sc.setParameters("endDate", date);
return searchIncludingRemoved(sc, null, false, false);
}
}

91
server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
View File

@ -514,6 +514,9 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
AssignIpAddressSearch.and("allocated", AssignIpAddressSearch.entity().getAllocatedTime(), Op.NULL);
AssignIpAddressSearch.and("vlanId", AssignIpAddressSearch.entity().getVlanId(), Op.IN);
AssignIpAddressSearch.and("forSystemVms", AssignIpAddressSearch.entity().isForSystemVms(), Op.EQ);
AssignIpAddressSearch.and("id", AssignIpAddressSearch.entity().getId(), Op.NIN);
AssignIpAddressSearch.and("requestedAddress", AssignIpAddressSearch.entity().getAddress(), Op.EQ);
AssignIpAddressSearch.and("routerAddress", AssignIpAddressSearch.entity().getAddress(), Op.NEQ);
SearchBuilder<VlanVO> vlanSearch = _vlanDao.createSearchBuilder();
vlanSearch.and("type", vlanSearch.entity().getVlanType(), Op.EQ);
@ -883,10 +886,23 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
if (podId != null) {
sc = AssignIpAddressFromPodVlanSearch.create();
sc.setJoinParameters("podVlanMapSB", "podId", podId);
errorMessage.append(" pod id=" + podId);
errorMessage.append(" pod id=").append(podId);
} else {
sc = AssignIpAddressSearch.create();
errorMessage.append(" zone id=" + dcId);
errorMessage.append(" zone id=").append(dcId);
}
if (lockOneRow) {
logger.debug("Listing quarantined public IPs to ignore on search for public IP for system VM. The IPs ignored will be the ones that: were not associated to account [{}]; were not removed yet; and with quarantine end dates after [{}].", owner.getUuid(), new Date());
List<PublicIpQuarantineVO> quarantinedAddresses = publicIpQuarantineDao.listQuarantinedIpAddressesToUser(owner.getId(), new Date());
List<Long> quarantinedAddressesIDs = quarantinedAddresses.stream().map(PublicIpQuarantineVO::getPublicIpAddressId).collect(Collectors.toList());
logger.debug("Found addresses with the following IDs: [{}] that will be ignored when searching for available public IPs.", quarantinedAddressesIDs);
if (CollectionUtils.isNotEmpty(quarantinedAddressesIDs)) {
sc.setParameters("id", quarantinedAddressesIDs.toArray());
}
}
sc.setParameters("dc", dcId);
@ -894,11 +910,11 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
// for direct network take ip addresses only from the vlans belonging to the network
if (vlanUse == VlanType.DirectAttached) {
sc.setJoinParameters("vlan", "networkId", guestNetworkId);
errorMessage.append(", network id=" + guestNetworkId);
errorMessage.append(", network id=").append(guestNetworkId);
}
if (requestedGateway != null) {
sc.setJoinParameters("vlan", "vlanGateway", requestedGateway);
errorMessage.append(", requested gateway=" + requestedGateway);
errorMessage.append(", requested gateway=").append(requestedGateway);
}
sc.setJoinParameters("vlan", "type", vlanUse);
@ -908,38 +924,39 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
NetworkDetailVO routerIpDetail = _networkDetailsDao.findDetail(network.getId(), ApiConstants.ROUTER_IP);
routerIpAddress = routerIpDetail != null ? routerIpDetail.getValue() : null;
}
if (requestedIp != null) {
sc.addAnd("address", SearchCriteria.Op.EQ, requestedIp);
errorMessage.append(": requested ip " + requestedIp + " is not available");
sc.setParameters("requestedAddress", requestedIp);
errorMessage.append(": requested ip ").append(requestedIp).append(" is not available");
} else if (routerIpAddress != null) {
sc.addAnd("address", Op.NEQ, routerIpAddress);
sc.setParameters("routerAddress", routerIpAddress);
}
boolean ascOrder = ! forSystemVms;
Filter filter = new Filter(IPAddressVO.class, "forSystemVms", ascOrder, 0l, 1l);
Filter filter = new Filter(IPAddressVO.class, "forSystemVms", ascOrder, 0L, 1L);
filter.addOrderBy(IPAddressVO.class,"vlanId", true);
List<IPAddressVO> addrs = new ArrayList<>();
List<IPAddressVO> addresses = new ArrayList<>();
if (forSystemVms) {
// Get Public IPs for system vms in dedicated ranges
sc.setParameters("forSystemVms", true);
if (lockOneRow) {
addrs = _ipAddressDao.lockRows(sc, filter, true);
addresses = _ipAddressDao.lockRows(sc, filter, true);
} else {
addrs = new ArrayList<>(_ipAddressDao.search(sc, null));
addresses = new ArrayList<>(_ipAddressDao.search(sc, null));
}
}
if ((!lockOneRow || (lockOneRow && CollectionUtils.isEmpty(addrs))) &&
if ((!lockOneRow || (lockOneRow && CollectionUtils.isEmpty(addresses))) &&
!(forSystemVms && SystemVmPublicIpReservationModeStrictness.value())) {
sc.setParameters("forSystemVms", false);
// If owner has dedicated Public IP ranges, fetch IP from the dedicated range
// Otherwise fetch IP from the system pool
// Checking if network is null in the case of system VM's. At the time of allocation of IP address to systemVm, no network is present.
if (network == null || !(network.getGuestType() == GuestType.Shared && zone.getNetworkType() == NetworkType.Advanced)) {
List<AccountVlanMapVO> maps = _accountVlanMapDao.listAccountVlanMapsByAccount(owner.getId());
for (AccountVlanMapVO map : maps) {
List<AccountVlanMapVO> accountVlanMaps = _accountVlanMapDao.listAccountVlanMapsByAccount(owner.getId());
for (AccountVlanMapVO map : accountVlanMaps) {
if (vlanDbIds == null || vlanDbIds.contains(map.getVlanDbId()))
dedicatedVlanDbIds.add(map.getVlanDbId());
}
@ -958,10 +975,10 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
if (!dedicatedVlanDbIds.isEmpty()) {
fetchFromDedicatedRange = true;
sc.setParameters("vlanId", dedicatedVlanDbIds.toArray());
errorMessage.append(", vlanId id=" + Arrays.toString(dedicatedVlanDbIds.toArray()));
errorMessage.append(", vlanId id=").append(Arrays.toString(dedicatedVlanDbIds.toArray()));
} else if (!nonDedicatedVlanDbIds.isEmpty()) {
sc.setParameters("vlanId", nonDedicatedVlanDbIds.toArray());
errorMessage.append(", vlanId id=" + Arrays.toString(nonDedicatedVlanDbIds.toArray()));
errorMessage.append(", vlanId id=").append(Arrays.toString(nonDedicatedVlanDbIds.toArray()));
} else {
if (podId != null) {
InsufficientAddressCapacityException ex = new InsufficientAddressCapacityException("Insufficient address capacity", Pod.class, podId);
@ -975,13 +992,13 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
}
}
if (lockOneRow) {
addrs = _ipAddressDao.lockRows(sc, filter, true);
addresses = _ipAddressDao.lockRows(sc, filter, true);
} else {
addrs = new ArrayList<>(_ipAddressDao.search(sc, null));
addresses = new ArrayList<>(_ipAddressDao.search(sc, null));
}
// If all the dedicated IPs of the owner are in use fetch an IP from the system pool
if ((!lockOneRow || (lockOneRow && addrs.size() == 0)) && fetchFromDedicatedRange && vlanUse == VlanType.VirtualNetwork) {
if ((!lockOneRow || (lockOneRow && addresses.isEmpty())) && fetchFromDedicatedRange && vlanUse == VlanType.VirtualNetwork) {
// Verify if account is allowed to acquire IPs from the system
boolean useSystemIps = UseSystemPublicIps.valueIn(owner.getId());
if (useSystemIps && !nonDedicatedVlanDbIds.isEmpty()) {
@ -989,15 +1006,15 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
sc.setParameters("vlanId", nonDedicatedVlanDbIds.toArray());
errorMessage.append(", vlanId id=" + Arrays.toString(nonDedicatedVlanDbIds.toArray()));
if (lockOneRow) {
addrs = _ipAddressDao.lockRows(sc, filter, true);
addresses = _ipAddressDao.lockRows(sc, filter, true);
} else {
addrs.addAll(_ipAddressDao.search(sc, null));
addresses.addAll(_ipAddressDao.search(sc, null));
}
}
}
}
if (lockOneRow && addrs.size() == 0) {
if (lockOneRow && addresses.isEmpty()) {
if (podId != null) {
InsufficientAddressCapacityException ex = new InsufficientAddressCapacityException("Insufficient address capacity", Pod.class, podId);
// for now, we hardcode the table names, but we should ideally do a lookup for the tablename from the VO object.
@ -1011,13 +1028,12 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
}
if (lockOneRow) {
assert (addrs.size() == 1) : "Return size is incorrect: " + addrs.size();
IpAddress ipAddress = addrs.get(0);
boolean ipCanBeAllocated = canPublicIpAddressBeAllocated(ipAddress, owner);
IPAddressVO allocatableIp = addresses.get(0);
if (!ipCanBeAllocated) {
throw new InsufficientAddressCapacityException(String.format("Failed to allocate public IP address [%s] as it is in quarantine.", ipAddress.getAddress()),
DataCenter.class, dcId);
boolean isPublicIpAllocatable = canPublicIpAddressBeAllocated(allocatableIp, owner);
if (!isPublicIpAllocatable) {
throw new InsufficientAddressCapacityException(String.format("Failed to allocate public IP [%s] as it is in quarantine.", allocatableIp.getAddress()), DataCenter.class, dcId);
}
}
@ -1026,12 +1042,12 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
try {
_resourceLimitMgr.checkResourceLimit(owner, ResourceType.public_ip);
} catch (ResourceAllocationException ex) {
logger.warn("Failed to allocate resource of type " + ex.getResourceType() + " for account " + owner);
logger.warn("Failed to allocate resource of type {} for account {}", ex.getResourceType(), owner);
throw new AccountLimitException("Maximum number of public IP addresses for account: " + owner.getAccountName() + " has been exceeded.");
}
}
return addrs;
return addresses;
}
@DB
@ -2458,26 +2474,27 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
PublicIpQuarantineVO publicIpQuarantineVO = publicIpQuarantineDao.findByPublicIpAddressId(ip.getId());
if (publicIpQuarantineVO == null) {
logger.debug(String.format("Public IP address [%s] is not in quarantine; therefore, it is allowed to be allocated.", ip));
logger.debug("Public IP address [{}] is not in quarantine; therefore, it is allowed to be allocated.", ip);
return true;
}
if (!isPublicIpAddressStillInQuarantine(publicIpQuarantineVO, new Date())) {
logger.debug(String.format("Public IP address [%s] is no longer in quarantine; therefore, it is allowed to be allocated.", ip));
logger.debug("Public IP address [{}] is no longer in quarantine; therefore, it is allowed to be allocated.", ip);
removePublicIpAddressFromQuarantine(publicIpQuarantineVO.getId(), "IP was removed from quarantine because it was no longer in quarantine.");
return true;
}
Account previousOwner = _accountMgr.getAccount(publicIpQuarantineVO.getPreviousOwnerId());
if (Objects.equals(previousOwner.getUuid(), newOwner.getUuid())) {
logger.debug(String.format("Public IP address [%s] is in quarantine; however, the Public IP previous owner [%s] is the same as the new owner [%s]; therefore the IP" +
" can be allocated. The public IP address will be removed from quarantine.", ip, previousOwner, newOwner));
logger.debug("Public IP address [{}] is in quarantine; however, the Public IP previous owner [{}] is the same as the new owner [{}]; therefore the IP" +
" can be allocated. The public IP address will be removed from quarantine.", ip, previousOwner, newOwner);
removePublicIpAddressFromQuarantine(publicIpQuarantineVO.getId(), "IP was removed from quarantine because it has been allocated by the previous owner");
return true;
}
logger.error(String.format("Public IP address [%s] is in quarantine and the previous owner [%s] is different than the new owner [%s]; therefore, the IP cannot be " +
"allocated.", ip, previousOwner, newOwner));
logger.error("Public IP address [{}] is in quarantine and the previous owner [{}] is different than the new owner [{}]; therefore, the IP cannot be " +
"allocated.", ip, previousOwner, newOwner);
return false;
}
@ -2528,7 +2545,7 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
publicIpQuarantineVO.setRemovalReason(removalReason);
publicIpQuarantineVO.setRemoverAccountId(removerAccountId);
logger.debug(String.format("Removing public IP Address [%s] from quarantine by updating the removed date to [%s].", ipAddress, removedDate));
logger.debug("Removing public IP Address [{}] from quarantine by updating the removed date to [{}].", ipAddress, removedDate);
publicIpQuarantineDao.persist(publicIpQuarantineVO);
}

1
server/src/test/java/com/cloud/network/IpAddressManagerTest.java
View File

@ -356,6 +356,7 @@ public class IpAddressManagerTest {
Mockito.when(ipAddressMock.getId()).thenReturn(dummyID);
Mockito.when(publicIpQuarantineDaoMock.findByPublicIpAddressId(Mockito.anyLong())).thenReturn(publicIpQuarantineVOMock);
Mockito.doReturn(false).when(ipAddressManager).isPublicIpAddressStillInQuarantine(Mockito.any(PublicIpQuarantineVO.class), Mockito.any(Date.class));
Mockito.doNothing().when(ipAddressManager).removePublicIpAddressFromQuarantine(Mockito.anyLong(), Mockito.anyString());
boolean result = ipAddressManager.canPublicIpAddressBeAllocated(ipAddressMock, newOwnerMock);

8
test/integration/smoke/test_quarantined_ips.py
View File

@ -85,7 +85,7 @@ class TestQuarantineIPs(cloudstackTestCase):
self.services["root_admin"]["roletype"])
"""
Set public.ip.address.quarantine.duration to 60 minutes
Set public.ip.address.quarantine.duration to 1 minute
"""
update_configuration_cmd = updateConfiguration.updateConfigurationCmd()
update_configuration_cmd.name = "public.ip.address.quarantine.duration"
@ -168,8 +168,7 @@ class TestQuarantineIPs(cloudstackTestCase):
zoneid=self.zone.id,
vpcid=root_vpc.id,
ipaddress=ip_address)
self.assertIn(f"Failed to allocate public IP address [{ip_address}] as it is in quarantine.",
exception.exception.errorMsg)
self.assertIn("errorCode: 533", exception.exception.errorMsg)
# Owner should be able to allocate its IP in quarantine
public_ip = PublicIPAddress.create(self.domain_admin_apiclient,
@ -267,8 +266,7 @@ class TestQuarantineIPs(cloudstackTestCase):
zoneid=self.zone.id,
networkid=root_network.id,
ipaddress=ip_address)
self.assertIn(f"Failed to allocate public IP address [{ip_address}] as it is in quarantine.",
exception.exception.errorMsg)
self.assertIn("errorCode: 533", exception.exception.errorMsg)
# Owner should be able to allocate its IP in quarantine
public_ip = PublicIPAddress.create(self.domain_admin_apiclient,