[GitHub] GabrielBrascher commented on a change in pull request #3129: Fix “test_nic_secondaryip_add_remove” test

[GitBox <gi...@apache.org>]

GabrielBrascher commented on a change in pull request #3129: Fix “test_nic_secondaryip_add_remove” test
URL: https://github.com/apache/cloudstack/pull/3129#discussion_r247476431
 
 

 ##########
 File path: server/src/main/java/com/cloud/network/NetworkServiceImpl.java
 ##########
 @@ -711,15 +710,11 @@ public NicSecondaryIp allocateSecondaryGuestIP(final long nicId, IpAddresses req
         String ip6addr = null;
         //Isolated network can exist in Basic zone only, so no need to verify the zone type
         if (network.getGuestType() == Network.GuestType.Isolated) {
-            try {
-                if (ipv4Address != null) {
-                    ipaddr = _ipAddrMgr.allocateGuestIP(network, ipv4Address);
-                }
-                if (ipv6Address != null) {
-                    ip6addr = ipv6AddrMgr.allocateGuestIpv6(network, ipv6Address);
-                }
-            } catch (InsufficientAddressCapacityException e) {
-                throw new InvalidParameterValueException("Allocating guest ip for nic failed");
+            if ((ipv4Address != null || NetUtils.isIpv4(network.getGateway()) && org.apache.commons.lang3.StringUtils.isBlank(ipv6Address))) {
 
 Review comment:
   That is the idea indeed @rafaelweingartner @rhtyd.
   It was designed to allow allocating ipv4 or ipv6 (does not accept null ipv6). If a user wants to allocate both, IPv4 and IPv6, he/she needs to call the API one time per IP address.
   My mistake here was adding a null verification for ipv4 addresses as well.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services