This is an automated email from the ASF dual-hosted git repository.
weizhou pushed a commit to branch 4.18
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.18 by this push:
new 72e3491cefa server: Fix allocation of more public IPs than the
account's limit (#7832)
72e3491cefa is described below
commit 72e3491cefa1e51d2c884dfe36a637a25adbfba4
Author: Fabricio Duarte <[email protected]>
AuthorDate: Mon Aug 14 05:33:29 2023 -0300
server: Fix allocation of more public IPs than the account's limit (#7832)
---
.../com/cloud/network/IpAddressManagerImpl.java | 37 ++++++++++----
.../com/cloud/network/IpAddressManagerTest.java | 59 ++++++++++++++++++++++
2 files changed, 86 insertions(+), 10 deletions(-)
diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
index b690acd7dd9..5436dd6acb1 100644
--- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
+++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
@@ -991,7 +991,8 @@ public class IpAddressManagerImpl extends ManagerBase
implements IpAddressManage
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) {
+ 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
@@ -1004,7 +1005,7 @@ public class IpAddressManagerImpl extends ManagerBase
implements IpAddressManage
addr.getAddress().toString(),
addr.isSourceNat(), guestType, addr.getSystem(), usageHidden,
addr.getClass().getName(),
addr.getUuid());
}
- if (updateIpResourceCount(addr)) {
+ if (shouldUpdateIpResourceCount) {
_resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.public_ip);
}
}
@@ -1020,7 +1021,7 @@ public class IpAddressManagerImpl extends ManagerBase
implements IpAddressManage
}
}
- private boolean isIpDedicated(IPAddressVO addr) {
+ protected boolean isIpDedicated(IPAddressVO addr) {
List<AccountVlanMapVO> maps =
_accountVlanMapDao.listAccountVlanMapsByVlan(addr.getVlanId());
if (maps != null && !maps.isEmpty())
return true;
@@ -1113,7 +1114,7 @@ public class IpAddressManagerImpl extends ManagerBase
implements IpAddressManage
// rule is applied. Similarly when last rule on the acquired IP is
revoked, IP is not associated with any provider
// but still be associated with the account. At this point just mark
IP as allocated or released.
for (IPAddressVO addr : userIps) {
- if (addr.getState() == IpAddress.State.Allocating) {
+ if (addr.getState() == IpAddress.State.Allocating ||
addr.getState() == IpAddress.State.Reserved) {
addr.setAssociatedWithNetworkId(network.getId());
markPublicIpAsAllocated(addr);
} else if (addr.getState() == IpAddress.State.Releasing) {
@@ -1510,7 +1511,6 @@ public class IpAddressManagerImpl extends ManagerBase
implements IpAddressManage
IPAddressVO ip = _ipAddressDao.findById(ipId);
//update ip address with networkId
- ip.setState(State.Allocated);
ip.setAssociatedWithNetworkId(networkId);
ip.setSourceNat(isSourceNat);
_ipAddressDao.update(ipId, ip);
@@ -1523,7 +1523,7 @@ public class IpAddressManagerImpl extends ManagerBase
implements IpAddressManage
} else {
s_logger.warn("Failed to associate ip address " +
ip.getAddress().addr() + " to network " + network);
}
- return ip;
+ return _ipAddressDao.findById(ipId);
} finally {
if (!success && releaseOnFailure) {
if (ip != null) {
@@ -1919,7 +1919,7 @@ public class IpAddressManagerImpl extends ManagerBase
implements IpAddressManage
return Transaction.execute(new TransactionCallback<IPAddressVO>() {
@Override
public IPAddressVO doInTransaction(TransactionStatus status) {
- if (updateIpResourceCount(ip)) {
+ if (checkIfIpResourceCountShouldBeUpdated(ip)) {
_resourceLimitMgr.decrementResourceCount(_ipAddressDao.findById(addrId).getAllocatedToAccountId(),
ResourceType.public_ip);
}
@@ -1944,9 +1944,26 @@ public class IpAddressManagerImpl extends ManagerBase
implements IpAddressManage
return ip;
}
- protected boolean updateIpResourceCount(IPAddressVO ip) {
- // don't increment resource count for direct and dedicated ip addresses
- return (ip.getAssociatedWithNetworkId() != null || ip.getVpcId() !=
null) && !isIpDedicated(ip);
+ protected boolean checkIfIpResourceCountShouldBeUpdated(IPAddressVO ip) {
+ boolean isDirectIp = ip.getAssociatedWithNetworkId() == null &&
ip.getVpcId() == null;
+ if (isDirectIp) {
+ s_logger.debug(String.format("IP address [%s] is direct;
therefore, the resource count should not be updated.", ip));
+ return false;
+ }
+
+ if (isIpDedicated(ip)) {
+ s_logger.debug(String.format("IP address [%s] is dedicated;
therefore, the resource count should not be updated.", ip));
+ return false;
+ }
+
+ boolean isReservedIp = ip.getState() == IpAddress.State.Reserved;
+ if (isReservedIp) {
+ s_logger.debug(String.format("IP address [%s] is reserved;
therefore, the resource count should not be updated.", ip));
+ return false;
+ }
+
+ s_logger.debug(String.format("IP address [%s] is not direct, dedicated
or reserved; therefore, the resource count should be updated.", ip));
+ return true;
}
@Override
diff --git a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java
b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java
index 50ad62e0543..94974572267 100644
--- a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java
+++ b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java
@@ -65,6 +65,9 @@ public class IpAddressManagerTest {
@Mock
NetworkOfferingDao networkOfferingDao;
+ @Mock
+ IPAddressVO ipAddressVoMock;
+
@Spy
@InjectMocks
IpAddressManagerImpl ipAddressManager;
@@ -230,4 +233,60 @@ public class IpAddressManagerTest {
return network;
}
+ private void prepareForCheckIfIpResourceCountShouldBeUpdatedTests() {
+
Mockito.when(ipAddressVoMock.getAssociatedWithNetworkId()).thenReturn(1L);
+ Mockito.when(ipAddressVoMock.getVpcId()).thenReturn(1L);
+ doReturn(false).when(ipAddressManager).isIpDedicated(Mockito.any());
+
Mockito.when(ipAddressVoMock.getState()).thenReturn(IpAddress.State.Allocating);
+ }
+
+ @Test
+ public void
checkIfIpResourceCountShouldBeUpdatedTestIpIsDirectReturnFalse() {
+ prepareForCheckIfIpResourceCountShouldBeUpdatedTests();
+
Mockito.when(ipAddressVoMock.getAssociatedWithNetworkId()).thenReturn(null);
+ Mockito.when(ipAddressVoMock.getVpcId()).thenReturn(null);
+
+ boolean result =
ipAddressManager.checkIfIpResourceCountShouldBeUpdated(ipAddressVoMock);
+
+ Assert.assertFalse(result);
+ }
+
+ @Test
+ public void
checkIfIpResourceCountShouldBeUpdatedTestIpIsDedicatedReturnFalse() {
+ prepareForCheckIfIpResourceCountShouldBeUpdatedTests();
+ doReturn(true).when(ipAddressManager).isIpDedicated(Mockito.any());
+
+ boolean result =
ipAddressManager.checkIfIpResourceCountShouldBeUpdated(ipAddressVoMock);
+
+ Assert.assertFalse(result);
+ }
+
+ @Test
+ public void
checkIfIpResourceCountShouldBeUpdatedTestIpIsReservedReturnFalse() {
+ prepareForCheckIfIpResourceCountShouldBeUpdatedTests();
+
Mockito.when(ipAddressVoMock.getState()).thenReturn(IpAddress.State.Reserved);
+
+ boolean result =
ipAddressManager.checkIfIpResourceCountShouldBeUpdated(ipAddressVoMock);
+
+ Assert.assertFalse(result);
+ }
+
+ @Test
+ public void
checkIfIpResourceCountShouldBeUpdatedTestIpIsAssociatedToNetworkAndNotDedicatedAndNotReservedReturnTrue()
{
+ prepareForCheckIfIpResourceCountShouldBeUpdatedTests();
+
+ boolean result =
ipAddressManager.checkIfIpResourceCountShouldBeUpdated(ipAddressVoMock);
+
+ Assert.assertTrue(result);
+ }
+
+ @Test
+ public void
checkIfIpResourceCountShouldBeUpdatedTestIpIsAssociatedToVpcAndNotDedicatedAndNotReservedReturnTrue()
{
+ prepareForCheckIfIpResourceCountShouldBeUpdatedTests();
+
Mockito.when(ipAddressVoMock.getAssociatedWithNetworkId()).thenReturn(null);
+
+ boolean result =
ipAddressManager.checkIfIpResourceCountShouldBeUpdated(ipAddressVoMock);
+
+ Assert.assertTrue(result);
+ }
}
