nagaboinaramgopal commented on code in PR #14348:
URL: https://github.com/apache/cloudstack/pull/14348#discussion_r4224213123


##########
server/src/main/java/com/cloud/network/IpAddressManagerImpl.java:
##########
@@ -950,11 +955,30 @@ public PublicIp fetchNewPublicIp(final long dcId, final 
Long podId, final List<L
     public PublicIp fetchNewPublicIp(final long dcId, final Long podId, final 
List<Long> vlanDbIds, final Account owner, final VlanType vlanUse, final Long 
guestNetworkId,
             final boolean sourceNat, final boolean assign, final boolean 
allocate, final String requestedIp, final String requestedGateway, final 
boolean isSystem, final Long vpcId, final Boolean displayIp, final boolean 
forSystemVms)
                     throws InsufficientAddressCapacityException {
-        List<IPAddressVO> addrs = listAvailablePublicIps(dcId, podId, 
vlanDbIds, owner, vlanUse, guestNetworkId, sourceNat, assign, allocate, 
requestedIp, requestedGateway, isSystem, vpcId, displayIp, forSystemVms, true);
-        IPAddressVO addr = addrs.get(0);
-        if (assign) {
+        // Two allocations (for example the console proxy and secondary 
storage system VMs started at the same
+        // time) can be handed the same free address because 
listAvailablePublicIps selects a candidate in a
+        // separate transaction from the one that marks it Allocating. 
assignIpAddressWithLock now locks the row
+        // and re-checks it is still Free, so the loser gets back null; 
re-select a different free IP and retry
+        // rather than failing with no-free-IP while free addresses still 
exist.
+        IPAddressVO addr = null;
+        for (int attempt = 1; attempt <= MAX_PUBLIC_IP_ALLOCATION_ATTEMPTS; 
attempt++) {

Review Comment:
   Added in 43384a56a6: 
testFetchNewPublicIpRetriesWhenTheFirstPickIsTakenConcurrently drives the first 
candidate to be taken (assignAndAllocateIpAddressEntry returns null) and 
asserts fetchNewPublicIp re-selects and allocates a different free IP instead 
of failing, verifying it attempts allocation twice. 4 tests pass in 
IpAddressManagerImplTest.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to