You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2018/02/10 17:24:06 UTC

[cloudstack] branch 4.11 updated: CLOUDSTACK-10218: Fix for forced network update in a nuage network (#2445)

This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch 4.11
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/4.11 by this push:
     new 8949efe  CLOUDSTACK-10218: Fix for forced network update in a nuage network (#2445)
8949efe is described below

commit 8949efe8d1bf67fda0b8deaccedc2c2d4de0f246
Author: Sigert Goeminne <si...@gmail.com>
AuthorDate: Sat Feb 10 18:24:01 2018 +0100

    CLOUDSTACK-10218: Fix for forced network update in a nuage network (#2445)
    
    Fix for forced network update to a nuage network offering with vr fails with IllegalArgumentException.
    Addressed review comments DaanHoogland.
---
 .../AssociateNuageVspDomainTemplateCmd.java        |   4 +-
 .../com/cloud/network/element/NuageVspElement.java |  11 +-
 .../network/guru/NuageVspGuestNetworkGuru.java     |  27 ++--
 .../com/cloud/network/manager/NuageVspManager.java |  34 +++--
 .../cloud/network/manager/NuageVspManagerImpl.java | 146 ++++++++++++++++++-
 .../src/com/cloud/util/NuageVspEntityBuilder.java  | 154 +--------------------
 .../nuage-vsp/src/com/cloud/util/NuageVspUtil.java |  11 ++
 .../nuage-vsp/test/com/cloud/NuageTest.java        |   2 -
 .../cloud/network/element/NuageVspElementTest.java |   9 +-
 .../plugins/nuagevsp/test_nuage_internal_dns.py    |  60 ++++++++
 10 files changed, 269 insertions(+), 189 deletions(-)

diff --git a/plugins/network-elements/nuage-vsp/src/com/cloud/api/commands/AssociateNuageVspDomainTemplateCmd.java b/plugins/network-elements/nuage-vsp/src/com/cloud/api/commands/AssociateNuageVspDomainTemplateCmd.java
index 760916a..9b36aaf 100644
--- a/plugins/network-elements/nuage-vsp/src/com/cloud/api/commands/AssociateNuageVspDomainTemplateCmd.java
+++ b/plugins/network-elements/nuage-vsp/src/com/cloud/api/commands/AssociateNuageVspDomainTemplateCmd.java
@@ -89,10 +89,10 @@ public class AssociateNuageVspDomainTemplateCmd extends BaseCmd {
     @Override
     public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException {
         try {
-            boolean result =_nuageVspManager.associateNuageVspDomainTemplate(this);
+            _nuageVspManager.associateNuageVspDomainTemplate(this);
             SuccessResponse response = new SuccessResponse(getCommandName());
             response.setResponseName(getCommandName());
-            response.setSuccess(result);
+            response.setSuccess(true);
             this.setResponseObject(response);
         } catch (InvalidParameterValueException invalidParamExcp) {
             throw new ServerApiException(ApiErrorCode.PARAM_ERROR, invalidParamExcp.getMessage());
diff --git a/plugins/network-elements/nuage-vsp/src/com/cloud/network/element/NuageVspElement.java b/plugins/network-elements/nuage-vsp/src/com/cloud/network/element/NuageVspElement.java
index abe7a71..d5496b0 100644
--- a/plugins/network-elements/nuage-vsp/src/com/cloud/network/element/NuageVspElement.java
+++ b/plugins/network-elements/nuage-vsp/src/com/cloud/network/element/NuageVspElement.java
@@ -90,7 +90,6 @@ import com.cloud.network.dao.IPAddressDao;
 import com.cloud.network.dao.IPAddressVO;
 import com.cloud.network.dao.NetworkDao;
 import com.cloud.network.dao.NetworkServiceMapDao;
-import com.cloud.network.dao.NetworkVO;
 import com.cloud.network.dao.PhysicalNetworkDao;
 import com.cloud.network.dao.PhysicalNetworkVO;
 import com.cloud.network.manager.NuageVspManager;
@@ -276,14 +275,8 @@ public class NuageVspElement extends AdapterBase implements ConnectivityProvider
             return false;
         }
 
-        if (!_nuageVspEntityBuilder.usesVirtualRouter(offering.getId())) {
-            // Update broadcast uri if VR is no longer used
-            NetworkVO networkToUpdate = _networkDao.findById(network.getId());
-            String broadcastUriStr = networkToUpdate.getUuid() + "/null";
-            networkToUpdate.setBroadcastUri(Networks.BroadcastDomainType.Vsp.toUri(broadcastUriStr));
-            _networkDao.update(network.getId(), networkToUpdate);
-        }
-
+        _nuageVspManager.updateBroadcastUri(network);
+        network = _networkDao.findById(network.getId());
         VspNetwork vspNetwork = _nuageVspEntityBuilder.buildVspNetwork(network);
         List<VspAclRule> ingressFirewallRules = getFirewallRulesToApply(network, FirewallRule.TrafficType.Ingress);
         List<VspAclRule> egressFirewallRules = getFirewallRulesToApply(network, FirewallRule.TrafficType.Egress);
diff --git a/plugins/network-elements/nuage-vsp/src/com/cloud/network/guru/NuageVspGuestNetworkGuru.java b/plugins/network-elements/nuage-vsp/src/com/cloud/network/guru/NuageVspGuestNetworkGuru.java
index 23a0efc..b17840c 100644
--- a/plugins/network-elements/nuage-vsp/src/com/cloud/network/guru/NuageVspGuestNetworkGuru.java
+++ b/plugins/network-elements/nuage-vsp/src/com/cloud/network/guru/NuageVspGuestNetworkGuru.java
@@ -255,7 +255,7 @@ public class NuageVspGuestNetworkGuru extends GuestNetworkGuru implements Networ
             VpcDetailVO detail = _vpcDetailsDao.findDetail(network.getVpcId(), NuageVspManager.nuageDomainTemplateDetailName);
             if (detail != null && network.getNetworkACLId() != null) {
                 s_logger.error("Pre-configured DT are used in combination with ACL lists. Which is not supported.");
-                throw new IllegalArgumentException("CloudStack ACLs are not supported with Nuage Preconfigured Domain Template");
+                throw new IllegalArgumentException("CloudStack ACLs are not supported with Nuage Pre-configured Domain Template");
             }
 
             if(detail != null && !_nuageVspManager.checkIfDomainTemplateExist(network.getDomainId(),detail.getValue(),network.getDataCenterId(),null)){
@@ -302,7 +302,9 @@ public class NuageVspGuestNetworkGuru extends GuestNetworkGuru implements Networ
                 implemented.setCidr(network.getCidr());
             }
 
-            VspNetwork vspNetwork = _nuageVspEntityBuilder.buildVspNetwork(implemented, true);
+            implemented.setBroadcastUri(_nuageVspManager.calculateBroadcastUri(implemented));
+            implemented.setBroadcastDomainType(Networks.BroadcastDomainType.Vsp);
+            VspNetwork vspNetwork = _nuageVspEntityBuilder.buildVspNetwork(implemented);
 
             if (vspNetwork.isShared()) {
                 Boolean previousUnderlay= null;
@@ -321,11 +323,6 @@ public class NuageVspGuestNetworkGuru extends GuestNetworkGuru implements Networ
                 }
             }
 
-            String tenantId = context.getDomain().getName() + "-" + context.getAccount().getAccountId();
-            String broadcastUriStr = implemented.getUuid() + "/" + vspNetwork.getVirtualRouterIp();
-            implemented.setBroadcastUri(Networks.BroadcastDomainType.Vsp.toUri(broadcastUriStr));
-            implemented.setBroadcastDomainType(Networks.BroadcastDomainType.Vsp);
-
             boolean implementSucceeded = implement(network.getVpcId(), physicalNetworkId, vspNetwork, implemented, _nuageVspEntityBuilder.buildNetworkDhcpOption(network, offering));
 
             if (!implementSucceeded) {
@@ -340,6 +337,7 @@ public class NuageVspGuestNetworkGuru extends GuestNetworkGuru implements Networ
                 }
             }
 
+            String tenantId = context.getDomain().getName() + "-" + context.getAccount().getAccountId();
             s_logger.info("Implemented OK, network " + implemented.getUuid() + " in tenant " + tenantId + " linked to " + implemented.getBroadcastUri());
         } finally {
             _networkDao.releaseFromLockTable(network.getId());
@@ -430,7 +428,7 @@ public class NuageVspGuestNetworkGuru extends GuestNetworkGuru implements Networ
     public NicProfile allocate(Network network, NicProfile nic, VirtualMachineProfile vm) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException {
         if (vm.getType() != VirtualMachine.Type.DomainRouter && _nuageVspEntityBuilder.usesVirtualRouter(network.getNetworkOfferingId())) {
             VspNetwork vspNetwork = _nuageVspEntityBuilder.buildVspNetwork(network);
-            if (nic != null && nic.getRequestedIPv4() != null && vspNetwork.getVirtualRouterIp().equals(nic.getRequestedIPv4())) {
+            if (nic != null && nic.getRequestedIPv4() != null && nic.getRequestedIPv4().equals(vspNetwork.getVirtualRouterIp())) {
                 DataCenter dc = _dcDao.findById(network.getDataCenterId());
                 s_logger.error("Unable to acquire requested Guest IP address " + nic.getRequestedIPv4() + " because it is reserved for the VR in network " + network);
                 throw new InsufficientVirtualNetworkCapacityException("Unable to acquire requested Guest IP address " + nic.getRequestedIPv4() + " because it is reserved " +
@@ -470,14 +468,13 @@ public class NuageVspGuestNetworkGuru extends GuestNetworkGuru implements Networ
             HostVO nuageVspHost = _nuageVspManager.getNuageVspHost(network.getPhysicalNetworkId());
             VspNetwork vspNetwork = _nuageVspEntityBuilder.buildVspNetwork(vm.getVirtualMachine().getDomainId(), network);
 
-            if (vm.getType() == VirtualMachine.Type.DomainRouter && vspNetwork.getVirtualRouterIp().equals("null")) {
-                //In case of upgrade network offering
-                vspNetwork = _nuageVspEntityBuilder.buildVspNetwork(vm.getVirtualMachine().getDomainId(), network, null, true);
-                String broadcastUriStr = network.getUuid() + "/" + vspNetwork.getVirtualRouterIp();
-                NetworkVO updatedNetwork = _networkDao.createForUpdate(network.getId());
-                updatedNetwork.setBroadcastUri(Networks.BroadcastDomainType.Vsp.toUri(broadcastUriStr));
-                _networkDao.update(updatedNetwork.getId(), updatedNetwork);
+            boolean vrAddedToNuage = vm.getType() == VirtualMachine.Type.DomainRouter && vspNetwork.getVirtualRouterIp()
+                                                                                          .equals("null");
+            if (vrAddedToNuage) {
+                //In case a VR is added due to upgrade network offering - recalculate the broadcast uri before using it.
+                _nuageVspManager.updateBroadcastUri(network);
                 network = _networkDao.findById(network.getId());
+                vspNetwork = _nuageVspEntityBuilder.buildVspNetwork(vm.getVirtualMachine().getDomainId(), network, null);
             }
 
             if (vspNetwork.isShared()) {
diff --git a/plugins/network-elements/nuage-vsp/src/com/cloud/network/manager/NuageVspManager.java b/plugins/network-elements/nuage-vsp/src/com/cloud/network/manager/NuageVspManager.java
index 6f91dc3..8030630 100644
--- a/plugins/network-elements/nuage-vsp/src/com/cloud/network/manager/NuageVspManager.java
+++ b/plugins/network-elements/nuage-vsp/src/com/cloud/network/manager/NuageVspManager.java
@@ -19,6 +19,7 @@
 
 package com.cloud.network.manager;
 
+import java.net.URI;
 import java.util.List;
 
 import org.apache.cloudstack.framework.config.ConfigKey;
@@ -35,6 +36,7 @@ import com.cloud.api.response.NuageVlanIpRangeResponse;
 import com.cloud.api.response.NuageVspDeviceResponse;
 import com.cloud.dc.Vlan;
 import com.cloud.api.response.NuageVspDomainTemplateResponse;
+import com.cloud.exception.InsufficientVirtualNetworkCapacityException;
 import com.cloud.host.HostVO;
 import com.cloud.network.Network;
 import com.cloud.network.NuageVspDeviceVO;
@@ -119,10 +121,9 @@ public interface NuageVspManager extends PluggableService {
 
     /**
      * Associates a Nuage Vsp domain template with a
-     * @param cmd
-     * @return
+     * @param cmd Associate cmd which contains all the data
      */
-    boolean associateNuageVspDomainTemplate(AssociateNuageVspDomainTemplateCmd cmd);
+    void associateNuageVspDomainTemplate(AssociateNuageVspDomainTemplateCmd cmd);
 
     /**
      * Queries the VSD to check if the entity provided in the entityCmd exists on the VSD
@@ -134,26 +135,41 @@ public interface NuageVspManager extends PluggableService {
 
     /**
      * Sets the preconfigured domain template for a given network
-     * @param network
-     * @param domainTemplateName
+     * @param network the network for which we want to set the domain template
+     * @param domainTemplateName the domain template name we want to use
      */
     void setPreConfiguredDomainTemplateName(Network network, String domainTemplateName);
 
     /**
      * Returns the current pre configured domain template for a given network
-     * @param network
-     * @return
+     * @param network the network for which we want the domain template name
+     * @return the domain template name
      */
     String getPreConfiguredDomainTemplateName(Network network);
 
     /**
      * Checks if a given domain template exists or not on the VSD.
-     * @param domainId
+     * @param domainId Id of the domain to search in.
      * @param domainTemplate The name of the domain template for which we need to query the VSD.
      * @param zoneId zoneId OR PhysicalNetworkId needs to be provided.
      * @param physicalNetworkId zoneId OR PhysicalNetworkId needs to be provided.
      * @return true if the domain template exists on the VSD else false if it does not exist on the VSD
      */
-    public boolean checkIfDomainTemplateExist(Long domainId, String domainTemplate, Long zoneId, Long physicalNetworkId);
+    boolean checkIfDomainTemplateExist(Long domainId, String domainTemplate, Long zoneId, Long physicalNetworkId);
+
+    /**
+     * calculates the new broadcast uri of a network and persists it in the database
+     * @param network the network for which you want to calculate the broadcast uri
+     * @throws InsufficientVirtualNetworkCapacityException in case there is no free ip that can be used as the VR ip.
+     */
+    void updateBroadcastUri(Network network) throws InsufficientVirtualNetworkCapacityException;
+
+    /**
+     * Calculates the broadcast uri based on the network and the offering of the given network
+     * @param network the network for which you want to calculate the broadcast uri
+     * @return the calculated broadcast uri
+     * @throws InsufficientVirtualNetworkCapacityException in case there is no free ip that can be used as the VR ip.
+     */
+    URI calculateBroadcastUri(Network network) throws InsufficientVirtualNetworkCapacityException;
 
 }
diff --git a/plugins/network-elements/nuage-vsp/src/com/cloud/network/manager/NuageVspManagerImpl.java b/plugins/network-elements/nuage-vsp/src/com/cloud/network/manager/NuageVspManagerImpl.java
index 748de10..0a3a966 100644
--- a/plugins/network-elements/nuage-vsp/src/com/cloud/network/manager/NuageVspManagerImpl.java
+++ b/plugins/network-elements/nuage-vsp/src/com/cloud/network/manager/NuageVspManagerImpl.java
@@ -35,6 +35,7 @@ import net.nuage.vsp.acs.client.api.model.VspDomainTemplate;
 import net.nuage.vsp.acs.client.api.model.VspHost;
 import net.nuage.vsp.acs.client.common.NuageVspApiVersion;
 import net.nuage.vsp.acs.client.common.NuageVspConstants;
+import net.nuage.vsp.acs.client.common.model.Pair;
 import net.nuage.vsp.acs.client.exception.NuageVspException;
 import org.apache.cloudstack.api.ResponseGenerator;
 import org.apache.cloudstack.context.CallContext;
@@ -53,15 +54,19 @@ import org.apache.log4j.Logger;
 import javax.annotation.Nonnull;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
+import java.net.URI;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.UUID;
 import java.util.stream.Collectors;
 
@@ -105,6 +110,7 @@ import com.cloud.dc.dao.VlanDetailsDao;
 import com.cloud.domain.Domain;
 import com.cloud.domain.DomainVO;
 import com.cloud.domain.dao.DomainDao;
+import com.cloud.exception.InsufficientVirtualNetworkCapacityException;
 import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.host.DetailVO;
 import com.cloud.host.Host;
@@ -113,6 +119,7 @@ import com.cloud.host.Status;
 import com.cloud.host.dao.HostDao;
 import com.cloud.host.dao.HostDetailsDao;
 import com.cloud.network.Network;
+import com.cloud.network.NetworkModel;
 import com.cloud.network.Networks;
 import com.cloud.network.NuageVspDeviceVO;
 import com.cloud.network.PhysicalNetwork;
@@ -154,6 +161,11 @@ import com.cloud.utils.db.TransactionStatus;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.fsm.StateListener;
 import com.cloud.utils.fsm.StateMachine2;
+import com.cloud.utils.net.NetUtils;
+import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.dao.NicDao;
+import com.cloud.vm.dao.VMInstanceDao;
 
 import static com.cloud.agent.api.sync.SyncNuageVspCmsIdCommand.SyncType;
 
@@ -213,6 +225,12 @@ public class NuageVspManagerImpl extends ManagerBase implements NuageVspManager,
     ResponseGenerator _responseGenerator;
     @Inject
     MessageBus _messageBus;
+    @Inject
+    VMInstanceDao _vmInstanceDao;
+    @Inject
+    NicDao _nicDao;
+    @Inject
+    NetworkModel _networkModel;
 
     static {
         Set<Network.Provider> nuageVspProviders = ImmutableSet.of(Network.Provider.NuageVsp);
@@ -902,7 +920,7 @@ public class NuageVspManagerImpl extends ManagerBase implements NuageVspManager,
     }
 
     @Override
-    public boolean associateNuageVspDomainTemplate(AssociateNuageVspDomainTemplateCmd cmd){
+    public void associateNuageVspDomainTemplate(AssociateNuageVspDomainTemplateCmd cmd){
         VpcVO vpc = _vpcDao.findById(cmd.getVpcId());
         Long physicalNetworkId;
         if (cmd.getPhysicalNetworkId() != null) {
@@ -923,7 +941,6 @@ public class NuageVspManagerImpl extends ManagerBase implements NuageVspManager,
             throw new InvalidParameterValueException("Could not find a Domain Template with name: " + cmd.getDomainTemplate());
         }
         setPreConfiguredDomainTemplateName(cmd.getVpcId(), cmd.getDomainTemplate());
-        return true;
     }
 
     @Override
@@ -940,6 +957,131 @@ public class NuageVspManagerImpl extends ManagerBase implements NuageVspManager,
     }
 
     @Override
+    public void updateBroadcastUri(Network network) throws InsufficientVirtualNetworkCapacityException {
+        NetworkVO updatedNetwork = _networkDao.createForUpdate(network.getId());
+        URI broadcastUri = calculateBroadcastUri(network);
+        if (!broadcastUri.equals(network.getBroadcastUri())) {
+            updatedNetwork.setBroadcastUri(broadcastUri);
+            _networkDao.update(network.getId(), updatedNetwork);
+        }
+    }
+
+    @Override
+    public URI calculateBroadcastUri(Network network) throws InsufficientVirtualNetworkCapacityException {
+        String vrIp = calculateVirtualRouterIp(network);
+        return Networks.BroadcastDomainType.Vsp.toUri(network.getUuid() + "/" + vrIp);
+    }
+
+    private boolean usesVirtualRouter(long networkOfferingId) {
+        return _networkOfferingServiceMapDao.isProviderForNetworkOffering(networkOfferingId, Network.Provider.VirtualRouter) ||
+                _networkOfferingServiceMapDao.isProviderForNetworkOffering(networkOfferingId, Network.Provider.VPCVirtualRouter);
+    }
+
+    private String calculateVirtualRouterIp(Network network)
+            throws InsufficientVirtualNetworkCapacityException {
+        if (!usesVirtualRouter(network.getNetworkOfferingId())) {
+            return null;
+        }
+
+        List<Pair<String, String>> ipAddressRanges =
+                network.getGuestType() == Network.GuestType.Shared ? getSharedIpAddressRanges(network.getId()) : getIpAddressRanges(network);
+
+        //check if a vr might be present already or not? CLOUD-1216 - before we always picked .2
+        List<VMInstanceVO> vrs =_vmInstanceDao.listNonRemovedVmsByTypeAndNetwork(network.getId(), VirtualMachine.Type.DomainRouter);
+
+        for (VMInstanceVO vr : vrs) {
+            return _nicDao.listByVmIdAndNicIdAndNtwkId(vr.getId(), null, network.getId()).get(0).getIPv4Address();
+        }
+
+        ensureIpCapacity(network, ipAddressRanges);
+
+        if(network.getGuestType() == Network.GuestType.Shared) {
+            return ipAddressRanges.stream()
+                                  .sorted(Comparator.comparingLong(p -> NetUtils.ip2Long(p.getLeft())))
+                                  .findFirst()
+                                  .map(Pair::getLeft)
+                                  .orElseThrow(() -> new IllegalArgumentException("Shared network without ip ranges? How can this happen?"));
+        }
+
+        Network networkToCheck;
+        if (isMigratingNetwork(network)) {
+            networkToCheck = _networkDao.findById(network.getRelated());
+        } else {
+            networkToCheck = network;
+        }
+
+        Long freeIp = _networkModel.getAvailableIps(networkToCheck, null)
+                                   .stream()
+                                   .findFirst()
+                                   .orElseThrow(() -> new InsufficientVirtualNetworkCapacityException("There is no free ip available for the VirtualRouter.",
+                                                                                                      Network.class,
+                                                                                                      network.getId()));
+
+        return NetUtils.long2Ip(freeIp);
+    }
+
+    private List<Pair<String, String>> getSharedIpAddressRanges(long networkId) {
+        List<VlanVO> vlans = _vlanDao.listVlansByNetworkId(networkId);
+        List<Pair<String, String>> ipAddressRanges = Lists.newArrayList();
+        for (VlanVO vlan : vlans) {
+            Pair<String, String> ipAddressRange = NuageVspUtil.getIpAddressRange(vlan);
+            if (ipAddressRange != null) {
+                ipAddressRanges.add(ipAddressRange);
+            }
+        }
+        return ipAddressRanges;
+    }
+
+    private List<Pair<String, String>> getIpAddressRanges(Network network) {
+        List<Pair<String, String>> ipAddressRanges = Lists.newArrayList();
+        String subnet = NetUtils.getCidrSubNet(network.getCidr());
+        String netmask = NetUtils.getCidrNetmask(network.getCidr());
+        long cidrSize = NetUtils.getCidrSize(netmask);
+        Set<Long> allIPsInCidr = NetUtils.getAllIpsFromCidr(subnet, cidrSize, new HashSet<Long>());
+        if (allIPsInCidr == null || !(allIPsInCidr instanceof TreeSet)) {
+            throw new IllegalStateException("The IPs in CIDR for subnet " + subnet + " where null or returned in a non-ordered set.");
+        }
+
+        Iterator<Long> ipIterator = allIPsInCidr.iterator();
+        long ip =  ipIterator.next();
+        long gatewayIp = NetUtils.ip2Long(network.getGateway());
+        String lastIp = NetUtils.getIpRangeEndIpFromCidr(subnet, cidrSize);
+        if (gatewayIp == ip) {
+            ip = ipIterator.next();
+            ipAddressRanges.add(Pair.of(NetUtils.long2Ip(ip), lastIp));
+        } else if (!network.getGateway().equals(lastIp)) {
+            ipAddressRanges.add(Pair.of(NetUtils.long2Ip(ip), NetUtils.long2Ip(gatewayIp - 1)));
+            ipAddressRanges.add(Pair.of(NetUtils.long2Ip(gatewayIp + 1), lastIp));
+        } else {
+            ipAddressRanges.add(Pair.of(NetUtils.long2Ip(ip), NetUtils.long2Ip(gatewayIp - 1)));
+        }
+
+        return ipAddressRanges;
+    }
+
+    private void ensureIpCapacity(Network network, List<Pair<String, String>> ipAddressRanges) throws InsufficientVirtualNetworkCapacityException {
+        long ipCount = ipAddressRanges.stream()
+                                      .mapToLong(this::getIpCount)
+                                      .sum();
+
+        if (ipCount == 0) {
+            throw new InsufficientVirtualNetworkCapacityException("VSP allocates an IP for VirtualRouter." + " But no ip address ranges are specified", Network.class,
+                                                                  network.getId());
+        } else if (ipCount < 3) {
+            throw new InsufficientVirtualNetworkCapacityException("VSP allocates an IP for VirtualRouter." + " So, subnet should have atleast minimum 3 hosts", Network.class,
+                                                                  network.getId());
+        }
+    }
+
+    private boolean isMigratingNetwork(Network network) {
+        return network.getRelated() != network.getId();
+    }
+
+    private long getIpCount(Pair<String, String> ipAddressRange) {
+        return NetUtils.ip2Long(ipAddressRange.getRight()) - NetUtils.ip2Long(ipAddressRange.getLeft()) + 1;
+    }
+
+    @Override
     public boolean entityExist(EntityExistsCommand cmd, Long physicalNetworkId){
         Long hostId = getNuageVspHostId(physicalNetworkId);
         if (hostId == null) {
diff --git a/plugins/network-elements/nuage-vsp/src/com/cloud/util/NuageVspEntityBuilder.java b/plugins/network-elements/nuage-vsp/src/com/cloud/util/NuageVspEntityBuilder.java
index cd986e5..5b1419f 100644
--- a/plugins/network-elements/nuage-vsp/src/com/cloud/util/NuageVspEntityBuilder.java
+++ b/plugins/network-elements/nuage-vsp/src/com/cloud/util/NuageVspEntityBuilder.java
@@ -19,13 +19,8 @@
 
 package com.cloud.util;
 
-import java.util.Comparator;
-import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
-import java.util.TreeSet;
 import java.util.stream.Collectors;
 
 import javax.inject.Inject;
@@ -44,10 +39,8 @@ import net.nuage.vsp.acs.client.api.model.VspVm;
 import net.nuage.vsp.acs.client.common.model.Pair;
 
 import org.apache.commons.collections.MapUtils;
-import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 
@@ -60,7 +53,6 @@ import com.cloud.dc.dao.VlanDetailsDao;
 import com.cloud.domain.Domain;
 import com.cloud.domain.DomainVO;
 import com.cloud.domain.dao.DomainDao;
-import com.cloud.exception.InsufficientVirtualNetworkCapacityException;
 import com.cloud.network.Network;
 import com.cloud.network.NetworkModel;
 import com.cloud.network.dao.IPAddressDao;
@@ -79,7 +71,6 @@ import com.cloud.offerings.dao.NetworkOfferingDao;
 import com.cloud.offerings.dao.NetworkOfferingServiceMapDao;
 import com.cloud.user.AccountVO;
 import com.cloud.user.dao.AccountDao;
-import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.net.NetUtils;
 import com.cloud.vm.NicProfile;
 import com.cloud.vm.NicVO;
@@ -157,30 +148,18 @@ public class NuageVspEntityBuilder {
     }
 
     public VspNetwork buildVspNetwork(Network network) {
-        return buildVspNetwork(network.getDomainId(), network, null, false);
-    }
-
-    public VspNetwork buildVspNetwork(Network network, boolean recalculateBroadcastUri) {
-        return buildVspNetwork(network.getDomainId(), network, null, recalculateBroadcastUri);
+        return buildVspNetwork(network.getDomainId(), network, null);
     }
 
     public VspNetwork buildVspNetwork(Network network, String vsdSubnetId) {
-        return buildVspNetwork(network.getDomainId(), network, vsdSubnetId, false);
+        return buildVspNetwork(network.getDomainId(), network, vsdSubnetId);
     }
 
     public VspNetwork buildVspNetwork(long domainId, Network network) {
-        return buildVspNetwork(domainId, network, null, false);
-    }
-
-    public VspNetwork buildVspNetwork(long domainId, Network network, boolean recalculateBroadcastUri) {
-        return buildVspNetwork(domainId, network, null, recalculateBroadcastUri);
+        return buildVspNetwork(domainId, network, null);
     }
 
     public VspNetwork buildVspNetwork(long domainId, Network network, String vsdSubnetId) {
-        return buildVspNetwork(domainId, network, vsdSubnetId, false);
-    }
-
-    public VspNetwork buildVspNetwork(long domainId, Network network, String vsdSubnetId, boolean recalculateBroadcastUri) {
         VspNetwork.Builder vspNetworkBuilder = new VspNetwork.Builder()
                 .id(network.getId())
                 .uuid(network.getUuid())
@@ -254,15 +233,8 @@ public class NuageVspEntityBuilder {
         vspNetworkBuilder.domainTemplateName(preConfiguredDomainTemplateName);
 
         if (usesVirtualRouter(networkOffering.getId())) {
-            try {
-                List<Pair<String, String>> ipAddressRanges =
-                        networkOffering.getGuestType() == Network.GuestType.Shared ? getSharedIpAddressRanges(network.getId()) : getIpAddressRanges(network);
-                String virtualRouterIp = getVirtualRouterIP(network, ipAddressRanges, recalculateBroadcastUri);
+                String virtualRouterIp = getVirtualRouterIP(network);
                 vspNetworkBuilder.virtualRouterIp(virtualRouterIp);
-            } catch (InsufficientVirtualNetworkCapacityException ex) {
-                s_logger.error("There is an insufficient network capacity in network " + network.getId(), ex);
-                throw new CloudRuntimeException("There is an insufficient network capacity in network " + network.getId(), ex);
-            }
         }
 
         return vspNetworkBuilder.build();
@@ -298,128 +270,16 @@ public class NuageVspEntityBuilder {
     }
 
     private boolean isVlanContainingIp(Vlan vlan, long ip) {
-        Pair<String, String> ipAddressRange = getIpAddressRange(vlan);
+        Pair<String, String> ipAddressRange =  NuageVspUtil.getIpAddressRange(vlan);
         long startIp = NetUtils.ip2Long(ipAddressRange.getLeft());
         long endIp = NetUtils.ip2Long(ipAddressRange.getRight());
         return startIp <= ip && ip <= endIp;
     }
 
-    private List<Pair<String, String>> getSharedIpAddressRanges(long networkId) {
-        List<VlanVO> vlans = _vlanDao.listVlansByNetworkId(networkId);
-        List<Pair<String, String>> ipAddressRanges = Lists.newArrayList();
-        for (VlanVO vlan : vlans) {
-            Pair<String, String> ipAddressRange = getIpAddressRange(vlan);
-            if (ipAddressRange != null) {
-                ipAddressRanges.add(ipAddressRange);
-            }
-        }
-        return ipAddressRanges;
+    private String getVirtualRouterIP(Network network) {
+        return network.getBroadcastUri() != null ? network.getBroadcastUri().getPath().substring(1) : null;
     }
 
-    private List<Pair<String, String>> getIpAddressRanges(Network network) {
-        List<Pair<String, String>> ipAddressRanges = Lists.newArrayList();
-        String subnet = NetUtils.getCidrSubNet(network.getCidr());
-        String netmask = NetUtils.getCidrNetmask(network.getCidr());
-        long cidrSize = NetUtils.getCidrSize(netmask);
-        Set<Long> allIPsInCidr = NetUtils.getAllIpsFromCidr(subnet, cidrSize, new HashSet<Long>());
-        if (allIPsInCidr == null || !(allIPsInCidr instanceof TreeSet)) {
-            throw new IllegalStateException("The IPs in CIDR for subnet " + subnet + " where null or returned in a non-ordered set.");
-        }
-
-        Iterator<Long> ipIterator = allIPsInCidr.iterator();
-        long ip =  ipIterator.next();
-        long gatewayIp = NetUtils.ip2Long(network.getGateway());
-        String lastIp = NetUtils.getIpRangeEndIpFromCidr(subnet, cidrSize);
-        if (gatewayIp == ip) {
-            ip = ipIterator.next();
-            ipAddressRanges.add(Pair.of(NetUtils.long2Ip(ip), lastIp));
-        } else if (!network.getGateway().equals(lastIp)) {
-            ipAddressRanges.add(Pair.of(NetUtils.long2Ip(ip), NetUtils.long2Ip(gatewayIp - 1)));
-            ipAddressRanges.add(Pair.of(NetUtils.long2Ip(gatewayIp + 1), lastIp));
-        } else {
-            ipAddressRanges.add(Pair.of(NetUtils.long2Ip(ip), NetUtils.long2Ip(gatewayIp - 1)));
-        }
-
-        return ipAddressRanges;
-    }
-
-    public Pair<String, String> getIpAddressRange(Vlan vlan) {
-        boolean isIpv4 = StringUtils.isNotBlank(vlan.getIpRange());
-        String[] range = isIpv4 ? vlan.getIpRange().split("-") : vlan.getIp6Range().split("-");
-        if (range.length == 2) {
-            return Pair.of(range[0], range[1]);
-        }
-        return null;
-    }
-
-    private String getVirtualRouterIP(Network network, List<Pair<String, String>> ipAddressRanges, boolean recalculateBroadcastUri) throws InsufficientVirtualNetworkCapacityException {
-        //Add a check to see if a VR should be present in the offering or not?
-        if (!recalculateBroadcastUri && network.getBroadcastUri() != null) {
-            return network.getBroadcastUri().getPath().substring(1);
-        }
-        ensureIpCapacity(network, ipAddressRanges);
-
-        if(network.getGuestType() == Network.GuestType.Shared) {
-            return ipAddressRanges.stream()
-                                  .sorted(Comparator.comparingLong(p -> NetUtils.ip2Long(p.getLeft())))
-                                  .findFirst()
-                                  .map(Pair::getLeft)
-                                  .orElseThrow(() -> new IllegalArgumentException("Shared network without ip ranges? How can this happen?"));
-        }
-
-        Pair<String, String> lowestIpAddressRange = null;
-        long ipCount = 0;
-        if (ipAddressRanges.size() == 1) {
-            lowestIpAddressRange = Iterables.getOnlyElement(ipAddressRanges);
-            ipCount = NetUtils.ip2Long(lowestIpAddressRange.getRight()) - NetUtils.ip2Long(lowestIpAddressRange.getLeft()) + 1;
-        } else {
-            for (Pair<String, String> ipAddressRange : ipAddressRanges) {
-                if (lowestIpAddressRange == null || NetUtils.ip2Long(ipAddressRange.getLeft()) < NetUtils.ip2Long(lowestIpAddressRange.getLeft())) {
-                    lowestIpAddressRange = ipAddressRange;
-                }
-                ipCount += NetUtils.ip2Long(ipAddressRange.getRight()) - NetUtils.ip2Long(ipAddressRange.getLeft()) + 1;
-            }
-        }
-
-
-        Network networkToCheck;
-        if (isMigratingNetwork(network)) {
-            networkToCheck = _networkDao.findById(network.getRelated());
-        } else {
-            networkToCheck = network;
-        }
-
-        Long freeIp = _networkModel.getAvailableIps(networkToCheck, null)
-                                   .stream()
-                                   .findFirst()
-                                   .orElseThrow(() -> new InsufficientVirtualNetworkCapacityException("There is no free ip available for the VirtualRouter.",
-                                                                                                      Network.class,
-                                                                                                      network.getId()));
-
-        return NetUtils.long2Ip(freeIp);
-    }
-
-    private boolean isMigratingNetwork(Network network) {
-        return network.getRelated() != network.getId();
-    }
-
-    private void ensureIpCapacity(Network network, List<Pair<String, String>> ipAddressRanges) throws InsufficientVirtualNetworkCapacityException {
-        long ipCount = ipAddressRanges.stream()
-                                      .mapToLong(this::getIpCount)
-                                      .sum();
-
-        if (ipCount == 0) {
-            throw new InsufficientVirtualNetworkCapacityException("VSP allocates an IP for VirtualRouter." + " But no ip address ranges are specified", Network.class,
-                    network.getId());
-        } else if (ipCount < 3) {
-            throw new InsufficientVirtualNetworkCapacityException("VSP allocates an IP for VirtualRouter." + " So, subnet should have atleast minimum 3 hosts", Network.class,
-                    network.getId());
-        }
-    }
-
-    private long getIpCount(Pair<String, String> ipAddressRange) {
-        return NetUtils.ip2Long(ipAddressRange.getRight()) - NetUtils.ip2Long(ipAddressRange.getLeft()) + 1;
-    }
 
     public VspVm buildVspVm(VirtualMachine vm, Network network) {
         VspVm.Builder vspVmBuilder = new VspVm.Builder()
diff --git a/plugins/network-elements/nuage-vsp/src/com/cloud/util/NuageVspUtil.java b/plugins/network-elements/nuage-vsp/src/com/cloud/util/NuageVspUtil.java
index 16d28b6..e5266d0 100644
--- a/plugins/network-elements/nuage-vsp/src/com/cloud/util/NuageVspUtil.java
+++ b/plugins/network-elements/nuage-vsp/src/com/cloud/util/NuageVspUtil.java
@@ -24,6 +24,8 @@ import com.cloud.dc.VlanDetailsVO;
 import com.cloud.dc.dao.VlanDetailsDao;
 import com.cloud.network.manager.NuageVspManager;
 import com.cloud.utils.StringUtils;
+
+import net.nuage.vsp.acs.client.common.model.Pair;
 import org.apache.commons.codec.binary.Base64;
 
 public class NuageVspUtil {
@@ -44,4 +46,13 @@ public class NuageVspUtil {
         VlanDetailsVO nuageUnderlayDetail = vlanDetailsDao.findDetail(vlan.getId(), NuageVspManager.nuageUnderlayVlanIpRangeDetailKey);
         return nuageUnderlayDetail != null && nuageUnderlayDetail.getValue().equalsIgnoreCase(String.valueOf(true));
     }
+
+    public static Pair<String, String> getIpAddressRange(Vlan vlan) {
+        boolean isIpv4 = StringUtils.isNotBlank(vlan.getIpRange());
+        String[] range = isIpv4 ? vlan.getIpRange().split("-") : vlan.getIp6Range().split("-");
+        if (range.length == 2) {
+            return Pair.of(range[0], range[1]);
+        }
+        return null;
+    }
 }
diff --git a/plugins/network-elements/nuage-vsp/test/com/cloud/NuageTest.java b/plugins/network-elements/nuage-vsp/test/com/cloud/NuageTest.java
index ae383bf..0249799 100644
--- a/plugins/network-elements/nuage-vsp/test/com/cloud/NuageTest.java
+++ b/plugins/network-elements/nuage-vsp/test/com/cloud/NuageTest.java
@@ -92,9 +92,7 @@ public class NuageTest {
 
         VspNetwork vspNetwork = buildVspNetwork();
         when(_nuageVspEntityBuilder.buildVspNetwork(any(Network.class))).thenReturn(vspNetwork);
-        when(_nuageVspEntityBuilder.buildVspNetwork(any(Network.class), anyBoolean())).thenReturn(vspNetwork);
         when(_nuageVspEntityBuilder.buildVspNetwork(anyLong(), any(Network.class))).thenReturn(vspNetwork);
-        when(_nuageVspEntityBuilder.buildVspNetwork(anyLong(), any(Network.class), anyBoolean())).thenReturn(vspNetwork);
 
         when(_nuageVspEntityBuilder.buildVspVm(any(VirtualMachine.class), any(Network.class))).thenReturn(buildVspVm());
 
diff --git a/plugins/network-elements/nuage-vsp/test/com/cloud/network/element/NuageVspElementTest.java b/plugins/network-elements/nuage-vsp/test/com/cloud/network/element/NuageVspElementTest.java
index 46046fd..e48d9a4 100644
--- a/plugins/network-elements/nuage-vsp/test/com/cloud/network/element/NuageVspElementTest.java
+++ b/plugins/network-elements/nuage-vsp/test/com/cloud/network/element/NuageVspElementTest.java
@@ -26,6 +26,8 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.Set;
 
+import com.cloud.network.dao.NetworkDao;
+import com.cloud.network.dao.NetworkVO;
 import com.cloud.tags.dao.ResourceTagDao;
 import org.junit.Before;
 import org.junit.Test;
@@ -60,9 +62,7 @@ import com.cloud.network.NuageVspDeviceVO;
 import com.cloud.network.dao.FirewallRulesDao;
 import com.cloud.network.dao.IPAddressDao;
 import com.cloud.network.dao.IPAddressVO;
-import com.cloud.network.dao.NetworkDao;
 import com.cloud.network.dao.NetworkServiceMapDao;
-import com.cloud.network.dao.NetworkVO;
 import com.cloud.network.dao.NuageVspDao;
 import com.cloud.network.dao.PhysicalNetworkDao;
 import com.cloud.network.dao.PhysicalNetworkVO;
@@ -89,6 +89,7 @@ import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.withSettings;
 
 public class NuageVspElementTest extends NuageTest {
 
@@ -119,6 +120,7 @@ public class NuageVspElementTest extends NuageTest {
         _nuageVspElement._nuageVspEntityBuilder = _nuageVspEntityBuilder;
         _nuageVspElement._vpcDetailsDao = _vpcDetailsDao;
         _nuageVspElement._routerDao = _domainRouterDao;
+        _nuageVspElement._networkDao = _networkDao;
 
         when(_networkServiceMapDao.canProviderSupportServiceInNetwork(NETWORK_ID, Service.Connectivity, Provider.NuageVsp)).thenReturn(true);
         when(_networkServiceMapDao.canProviderSupportServiceInNetwork(NETWORK_ID, Service.SourceNat, Provider.NuageVsp)).thenReturn(true);
@@ -166,7 +168,7 @@ public class NuageVspElementTest extends NuageTest {
 
     @Test
     public void testImplement() throws ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException, URISyntaxException {
-        final Network network = mock(Network.class);
+        final Network network = mock(NetworkVO.class, withSettings().extraInterfaces(Network.class));
         when(network.getBroadcastDomainType()).thenReturn(BroadcastDomainType.Vsp);
         when(network.getId()).thenReturn(NETWORK_ID);
         when(network.getVpcId()).thenReturn(null);
@@ -209,6 +211,7 @@ public class NuageVspElementTest extends NuageTest {
         when(_firewallRulesDao.listByNetworkPurposeTrafficType(NETWORK_ID, FirewallRule.Purpose.Firewall, FirewallRule.TrafficType.Egress)).thenReturn(new ArrayList<FirewallRuleVO>());
         when(_ipAddressDao.listStaticNatPublicIps(NETWORK_ID)).thenReturn(new ArrayList<IPAddressVO>());
         when(_nuageVspManager.getDnsDetails(network.getDataCenterId())).thenReturn(new ArrayList<String>());
+        when(_networkDao.findById(network.getId())).thenReturn((NetworkVO)network);
 
         assertTrue(_nuageVspElement.implement(network, offering, deployDest, context));
     }
diff --git a/test/integration/plugins/nuagevsp/test_nuage_internal_dns.py b/test/integration/plugins/nuagevsp/test_nuage_internal_dns.py
index b0026d7..0c2a6b6 100644
--- a/test/integration/plugins/nuagevsp/test_nuage_internal_dns.py
+++ b/test/integration/plugins/nuagevsp/test_nuage_internal_dns.py
@@ -947,3 +947,63 @@ class TestNuageInternalDns(nuageTestCase):
                 self.debug("excepted value found in vm: " + item)
             else:
                 self.fail("excepted value not found in vm: " + item)
+
+    @attr(tags=["advanced", "nuagevsp"], required_hardware="true")
+    def test_09_update_network_offering_isolated_network(self):
+        """Test Update network offering for isolated Networks
+           with Nuage VSP SDN plugin
+        """
+        #    Create an Isolated Network with Nuage VSP Isolated Network
+        #    offering specifying Services which don't need a VR.
+        #    Update the network offering of this network to one that
+        #    needs a VR, check that a VR is spawn
+        #    After that update network to previous offering
+        #    Check that VR is destroyed and removed.
+
+        self.debug("+++Create an Isolated network with a network "
+                   "offering which has only services without VR")
+        cmd = updateZone.updateZoneCmd()
+        cmd.id = self.zone.id
+        cmd.domain = "isolated.com"
+        self.apiclient.updateZone(cmd)
+        self.debug("Creating and enabling Nuage Vsp Isolated Network "
+                   "offering which has only service without VR...")
+        network_offering = self.create_NetworkOffering(
+                self.test_data["nuagevsp"]
+                ["isolated_network_offering_without_vr"])
+        self.validate_NetworkOffering(network_offering, state="Enabled")
+
+        network_1 = self.create_Network(network_offering)
+        self.validate_Network(network_1, state="Allocated")
+
+        self.debug("+++Deploy VM in the created Isolated network "
+                   "with only services without VR")
+        vm_1 = self.create_VM(network_1)
+
+        # VSD verification
+        self.verify_vsd_network(self.domain.id, network_1)
+        self.verify_vsd_vm(vm_1)
+
+        with self.assertRaises(Exception):
+            self.get_Router(network_1)
+        self.debug("+++Verified no VR is spawned for this network ")
+
+        self.debug("+++ Upgrade offering of created Isolated network with "
+                   "a dns offering which spins a VR")
+        self.upgrade_Network(self.test_data["nuagevsp"][
+                            "isolated_network_offering"],
+                            network_1)
+        vr = self.get_Router(network_1)
+        self.check_Router_state(vr, state="Running")
+        # VSD verification
+        self.verify_vsd_network(self.domain.id, network_1)
+        self.verify_vsd_router(vr)
+
+        self.debug("+++ Upgrade offering of created Isolated network with "
+                   "an offering which removes the VR...")
+        self.upgrade_Network(self.test_data["nuagevsp"][
+                    "isolated_network_offering_without_vr"],
+                    network_1)
+        with self.assertRaises(Exception):
+            self.get_Router(network_1)
+        self.debug("+++Verified no VR is spawned for this network ")

-- 
To stop receiving notification emails like this one, please contact
rohit@apache.org.