utils: fix NetUtils method to retrieve all IPs for a CIDR (#7026)

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
apache · Dec 30, 2022 · d5f0100 · d5f0100
1 parent 63bc5a8
commit d5f0100
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 5 deletions.
diff --git a/server/src/main/java/com/cloud/network/NetworkModelImpl.java b/server/src/main/java/com/cloud/network/NetworkModelImpl.java
@@ -2001,7 +2001,7 @@ public Set<Long> getAvailableIps(Network network, String requestedIp) {
             usedIps.add(NetUtils.ip2Long(ip));
         }
 
-        Set<Long> allPossibleIps = NetUtils.getAllIpsFromCidr(cidr[0], Integer.parseInt(cidr[1]), usedIps);
+        Set<Long> allPossibleIps = NetUtils.getAllIpsFromCidr(cidr[0], Integer.parseInt(cidr[1]), usedIps, -1);
 
         String gateway = network.getGateway();
         if ((gateway != null) && (allPossibleIps.contains(NetUtils.ip2Long(gateway))))

diff --git a/utils/src/main/java/com/cloud/utils/net/NetUtils.java b/utils/src/main/java/com/cloud/utils/net/NetUtils.java
@@ -617,7 +617,7 @@ public static String[] getIpRangeFromCidr(final String cidr, final long size) {
         return result;
     }
 
-    public static Set<Long> getAllIpsFromCidr(final String cidr, final long size, final Set<Long> usedIps) {
+    public static Set<Long> getAllIpsFromCidr(final String cidr, final long size, final Set<Long> usedIps, int maxIps) {
         assert size < MAX_CIDR : "You do know this is not for ipv6 right?  Keep it smaller than 32 but you have " + size;
         final Set<Long> result = new TreeSet<Long>();
         final long ip = ip2Long(cidr);
@@ -629,11 +629,9 @@ public static Set<Long> getAllIpsFromCidr(final String cidr, final long size, fi
 
         end++;
         end = (end << MAX_CIDR - size) - 2;
-        int maxIps = 255; // get 255 ips as maximum
-        while (start <= end && maxIps > 0) {
+        while (start <= end && (maxIps == -1 || result.size() < maxIps)) {
             if (!usedIps.contains(start)) {
                 result.add(start);
-                maxIps--;
             }
             start++;
         }

diff --git a/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java b/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java
@@ -34,8 +34,12 @@
 import java.math.BigInteger;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeSet;
+import java.util.stream.Collectors;
 
 import org.apache.log4j.Logger;
 import org.junit.Test;
@@ -48,6 +52,8 @@
 public class NetUtilsTest {
 
     private static final Logger s_logger = Logger.getLogger(NetUtilsTest.class);
+    private static final String WIDE_SHARED_NET_CIDR_IP = "10.20.0.0";
+    private static final List<String> WIDE_SHARED_NET_USED_IPS = List.of("10.20.0.22", "10.20.1.22", "10.20.2.22");
 
     @Test
     public void testGetRandomIpFromCidrWithSize24() throws Exception {
@@ -741,4 +747,50 @@ public void testCidrNetmask() {
         assertEquals("255.255.0.0", NetUtils.cidr2Netmask("169.254.0.0/16"));
         assertEquals("255.255.240.0", NetUtils.cidr2Netmask("169.254.240.0/20"));
     }
+
+    private void runTestGetAllIpsFromCidr(int cidrSize, int maxIps, boolean usedIpPresent, int resultSize) {
+        Set<Long> usedIps = new TreeSet<>();
+        if (usedIpPresent) {
+            for (String ip : WIDE_SHARED_NET_USED_IPS) {
+                usedIps.add(NetUtils.ip2Long(ip));
+            }
+        }
+        Set<Long> result = NetUtils.getAllIpsFromCidr(WIDE_SHARED_NET_CIDR_IP, cidrSize, usedIps, maxIps);
+        assertNotNull(result);
+        assertEquals(resultSize, result.size());
+        if (usedIpPresent) {
+            for (String ip : WIDE_SHARED_NET_USED_IPS) {
+                assertFalse(result.contains(NetUtils.ip2Long(ip)));
+            }
+        }
+    }
+
+    @Test
+    public void testGetAllIpsFromCidrNoneUsedNoLimit() {
+        runTestGetAllIpsFromCidr(22, -1, false, 1022);
+    }
+
+    @Test
+    public void testGetAllIpsFromCidrNoneUsedLimit() {
+        runTestGetAllIpsFromCidr(22, 255, false, 255);
+    }
+
+    @Test
+    public void testGetAllIpsFromCidrNoneUsedLessLimit() {
+        runTestGetAllIpsFromCidr(22, 10, false, 10);
+    }
+
+
+    @Test
+    public void testGetAllIpsFromCidrUsedNoLimit() {
+        runTestGetAllIpsFromCidr(22, -1, true, 1022 - WIDE_SHARED_NET_USED_IPS.size());
+    }
+
+    @Test
+    public void testGetAllIpsFromCidrUsedLimit() {
+        runTestGetAllIpsFromCidr(22, 50, true, 50);
+        List<String> usedIpsInRange = new ArrayList<>(WIDE_SHARED_NET_USED_IPS);
+        usedIpsInRange = usedIpsInRange.stream().filter(x -> x.startsWith("10.20.0.")).collect(Collectors.toList());
+        runTestGetAllIpsFromCidr(24, 255, true, 254 - usedIpsInRange.size());
+    }
 }