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.

Author: sgoeminn

plugins/network-elements/nuage-vsp/src/com/cloud/api/commands/AssociateNuageVspDomainTemplateCmd.java

@@ -89,10 +89,10 @@ public Long getVpcId() {
     @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());

plugins/network-elements/nuage-vsp/src/com/cloud/network/element/NuageVspElement.java

@@ -90,7 +90,6 @@
 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 boolean implement(Network network, NetworkOffering offering, DeployDestin
             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);

plugins/network-elements/nuage-vsp/src/com/cloud/network/guru/NuageVspGuestNetworkGuru.java

@@ -255,7 +255,7 @@ public Network implement(Network network, NetworkOffering offering, DeployDestin
             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 Network implement(Network network, NetworkOffering offering, DeployDestin
                 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 Network implement(Network network, NetworkOffering offering, DeployDestin
                 }
             }
 
-            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 Network implement(Network network, NetworkOffering offering, DeployDestin
                 }
             }
 
+            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 @@ private static Map<String, String> constructVpcDetails(NetworkRelatedVsdIds netw
     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 void reserve(NicProfile nic, Network network, VirtualMachineProfile vm, D
             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()) {

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.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;
 
 }