Merge branch '4.11'

agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java

@@ -21,6 +21,9 @@ package com.cloud.agent.direct.download;
 
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.script.Script;
+import org.apache.commons.io.IOUtils;
+import org.apache.http.HttpEntity;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
@@ -33,6 +36,9 @@ import javax.net.ssl.SSLContext;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.FileOutputStream;
 import java.security.KeyManagementException;
 import java.security.KeyStore;
 import java.security.KeyStoreException;
@@ -79,11 +85,32 @@ public class HttpsDirectTemplateDownloader extends HttpDirectTemplateDownloader
 
     @Override
     public boolean downloadTemplate() {
+        CloseableHttpResponse response;
         try {
-            httpsClient.execute(req);
+            response = httpsClient.execute(req);
         } catch (IOException e) {
             throw new CloudRuntimeException("Error on HTTPS request: " + e.getMessage());
         }
-        return performDownload();
+        return consumeResponse(response);
     }
+
+    /**
+     * Consume response and persist it on getDownloadedFilePath() file
+     */
+    protected boolean consumeResponse(CloseableHttpResponse response) {
+        if (response.getStatusLine().getStatusCode() != 200) {
+            throw new CloudRuntimeException("Error on HTTPS response");
+        }
+        try {
+            HttpEntity entity = response.getEntity();
+            InputStream in = entity.getContent();
+            OutputStream out = new FileOutputStream(getDownloadedFilePath());
+            IOUtils.copy(in, out);
+        } catch (Exception e) {
+            s_logger.error("Error parsing response for template " + getTemplateId() + " due to: " + e.getMessage());
+            return false;
+        }
+        return true;
+    }
+
 }

api/src/main/java/org/apache/cloudstack/acl/RoleService.java

@@ -31,7 +31,7 @@ public interface RoleService {
     boolean isEnabled();
     Role findRole(final Long id);
     Role createRole(final String name, final RoleType roleType, final String description);
-    boolean updateRole(final Role role, final String name, final RoleType roleType, final String description);
+    Role updateRole(final Role role, final String name, final RoleType roleType, final String description);
     boolean deleteRole(final Role role);
 
     RolePermission findRolePermission(final Long id);

api/src/main/java/org/apache/cloudstack/api/command/admin/acl/CreateRoleCmd.java

@@ -34,7 +34,7 @@ import org.apache.cloudstack.context.CallContext;
         requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
         since = "4.9.0",
         authorized = {RoleType.Admin})
-public class CreateRoleCmd extends BaseCmd {
+public class CreateRoleCmd extends RoleCmd {
     public static final String APINAME = "createRole";
 
     /////////////////////////////////////////////////////
@@ -83,16 +83,6 @@ public class CreateRoleCmd extends BaseCmd {
         return Account.ACCOUNT_ID_SYSTEM;
     }
 
-    private void setupResponse(final Role role) {
-        final RoleResponse response = new RoleResponse();
-        response.setId(role.getUuid());
-        response.setRoleName(role.getName());
-        response.setRoleType(role.getRoleType());
-        response.setResponseName(getCommandName());
-        response.setObjectName("role");
-        setResponseObject(response);
-    }
-
     @Override
     public void execute() {
         CallContext.current().setEventDetails("Role: " + getRoleName() + ", type:" + getRoleType() + ", description: " + getRoleDescription());

api/src/main/java/org/apache/cloudstack/api/command/admin/acl/RoleCmd.java

@@ -0,0 +1,36 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.admin.acl;
+
+import org.apache.cloudstack.acl.Role;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.response.RoleResponse;
+
+public abstract class RoleCmd extends BaseCmd {
+
+    protected void setupResponse(final Role role) {
+        final RoleResponse response = new RoleResponse();
+        response.setId(role.getUuid());
+        response.setRoleName(role.getName());
+        response.setRoleType(role.getRoleType());
+        response.setDescription(role.getDescription());
+        response.setResponseName(getCommandName());
+        response.setObjectName("role");
+        setResponseObject(response);
+    }
+}

api/src/main/java/org/apache/cloudstack/api/command/admin/acl/UpdateRoleCmd.java

@@ -22,21 +22,20 @@ import com.google.common.base.Strings;
 import org.apache.cloudstack.acl.Role;
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiArgValidator;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.ApiErrorCode;
 import org.apache.cloudstack.api.BaseCmd;
 import org.apache.cloudstack.api.Parameter;
 import org.apache.cloudstack.api.ServerApiException;
-import org.apache.cloudstack.api.ApiArgValidator;
 import org.apache.cloudstack.api.response.RoleResponse;
-import org.apache.cloudstack.api.response.SuccessResponse;
 import org.apache.cloudstack.context.CallContext;
 
-@APICommand(name = UpdateRoleCmd.APINAME, description = "Updates a role", responseObject = SuccessResponse.class,
+@APICommand(name = UpdateRoleCmd.APINAME, description = "Updates a role", responseObject = RoleResponse.class,
         requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
         since = "4.9.0",
         authorized = {RoleType.Admin})
-public class UpdateRoleCmd extends BaseCmd {
+public class UpdateRoleCmd extends RoleCmd {
     public static final String APINAME = "updateRole";
 
     /////////////////////////////////////////////////////
@@ -100,9 +99,7 @@ public class UpdateRoleCmd extends BaseCmd {
             throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid role id provided");
         }
         CallContext.current().setEventDetails("Role: " + getRoleName() + ", type:" + getRoleType() + ", description: " + getRoleDescription());
-        boolean result = roleService.updateRole(role, getRoleName(), getRoleType(), getRoleDescription());
+        role = roleService.updateRole(role, getRoleName(), getRoleType(), getRoleDescription());
-        SuccessResponse response = new SuccessResponse(getCommandName());
+        setupResponse(role);
-        response.setSuccess(result);
-        setResponseObject(response);
     }
 }

api/test/org/apache/cloudstack/api/command/test/UpdateRoleCmdTest.java

@@ -0,0 +1,70 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.test;
+
+import junit.framework.TestCase;
+import org.apache.cloudstack.acl.Role;
+import org.apache.cloudstack.acl.RoleService;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.command.admin.acl.UpdateRoleCmd;
+import org.apache.cloudstack.api.response.RoleResponse;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+import org.springframework.test.util.ReflectionTestUtils;
+
+import static org.mockito.Mockito.when;
+
+
+public class UpdateRoleCmdTest extends TestCase{
+
+    private UpdateRoleCmd updateRoleCmd;
+    private RoleService roleService;
+    private Role role;
+
+    @Override
+    @Before
+    public void setUp() {
+        roleService = Mockito.spy(RoleService.class);
+        updateRoleCmd = new UpdateRoleCmd();
+        ReflectionTestUtils.setField(updateRoleCmd,"roleService",roleService);
+        ReflectionTestUtils.setField(updateRoleCmd,"roleId",1L);
+        ReflectionTestUtils.setField(updateRoleCmd,"roleName","user");
+        ReflectionTestUtils.setField(updateRoleCmd,"roleType", "User");
+        ReflectionTestUtils.setField(updateRoleCmd,"roleDescription","Description Initial");
+        role = Mockito.mock(Role.class);
+    }
+
+    @Test
+    public void testUpdateSuccess() {
+        when(roleService.findRole(updateRoleCmd.getRoleId())).thenReturn(role);
+        when(role.getId()).thenReturn(1L);
+        when(role.getUuid()).thenReturn("12345-abcgdkajd");
+        when(role.getDescription()).thenReturn("Defualt user");
+        when(role.getName()).thenReturn("User");
+        when(role.getRoleType()).thenReturn(RoleType.User);
+        when(roleService.updateRole(role,updateRoleCmd.getRoleName(),updateRoleCmd.getRoleType(),updateRoleCmd.getRoleDescription())).thenReturn(role);
+        when(role.getId()).thenReturn(1L);
+        when(role.getDescription()).thenReturn("Description Initial");
+        when(role.getName()).thenReturn("User");
+        updateRoleCmd.execute();
+        RoleResponse response = (RoleResponse) updateRoleCmd.getResponseObject();
+        assertEquals((String)ReflectionTestUtils.getField(response, "roleName"),role.getName());
+        assertEquals((String)ReflectionTestUtils.getField(response, "roleDescription"),role.getDescription());
+    }
+}

plugins/network-elements/nuage-vsp/src/main/java/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());

plugins/network-elements/nuage-vsp/src/main/java/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())) {
+        _nuageVspManager.updateBroadcastUri(network);
-            // Update broadcast uri if VR is no longer used
+        network = _networkDao.findById(network.getId());
-            NetworkVO networkToUpdate = _networkDao.findById(network.getId());
-            String broadcastUriStr = networkToUpdate.getUuid() + "/null";
-            networkToUpdate.setBroadcastUri(Networks.BroadcastDomainType.Vsp.toUri(broadcastUriStr));
-            _networkDao.update(network.getId(), networkToUpdate);
-        }
-
         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/main/java/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")) {
+            boolean vrAddedToNuage = vm.getType() == VirtualMachine.Type.DomainRouter && vspNetwork.getVirtualRouterIp()
-                //In case of upgrade network offering
+                                                                                          .equals("null");
-                vspNetwork = _nuageVspEntityBuilder.buildVspNetwork(vm.getVirtualMachine().getDomainId(), network, null, true);
+            if (vrAddedToNuage) {
-                String broadcastUriStr = network.getUuid() + "/" + vspNetwork.getVirtualRouterIp();
+                //In case a VR is added due to upgrade network offering - recalculate the broadcast uri before using it.
-                NetworkVO updatedNetwork = _networkDao.createForUpdate(network.getId());
+                _nuageVspManager.updateBroadcastUri(network);
-                updatedNetwork.setBroadcastUri(Networks.BroadcastDomainType.Vsp.toUri(broadcastUriStr));
-                _networkDao.update(updatedNetwork.getId(), updatedNetwork);
                 network = _networkDao.findById(network.getId());
+                vspNetwork = _nuageVspEntityBuilder.buildVspNetwork(vm.getVirtualMachine().getDomainId(), network, null);
             }
 
             if (vspNetwork.isShared()) {

plugins/network-elements/nuage-vsp/src/main/java/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
+     * @param cmd Associate cmd which contains all the data
-     * @return
      */
-    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 network the network for which we want to set the domain template
-     * @param domainTemplateName
+     * @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
+     * @param network the network for which we want the domain template name
-     * @return
+     * @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;
 
 }

plugins/network-elements/nuage-vsp/src/main/java/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
@@ -939,6 +956,131 @@ public class NuageVspManagerImpl extends ManagerBase implements NuageVspManager,
         return false;
     }
 
+    @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);

plugins/network-elements/nuage-vsp/src/main/java/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);
+        return buildVspNetwork(network.getDomainId(), network, null);
-    }
-
-    public VspNetwork buildVspNetwork(Network network, boolean recalculateBroadcastUri) {
-        return buildVspNetwork(network.getDomainId(), network, null, recalculateBroadcastUri);
     }
 
     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);
+        return buildVspNetwork(domainId, network, null);
-    }
-
-    public VspNetwork buildVspNetwork(long domainId, Network network, boolean recalculateBroadcastUri) {
-        return buildVspNetwork(domainId, network, null, recalculateBroadcastUri);
     }
 
     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 {
+                String virtualRouterIp = getVirtualRouterIP(network);
-                List<Pair<String, String>> ipAddressRanges =
-                        networkOffering.getGuestType() == Network.GuestType.Shared ? getSharedIpAddressRanges(network.getId()) : getIpAddressRanges(network);
-                String virtualRouterIp = getVirtualRouterIP(network, ipAddressRanges, recalculateBroadcastUri);
                 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) {
+    private String getVirtualRouterIP(Network network) {
-        List<VlanVO> vlans = _vlanDao.listVlansByNetworkId(networkId);
+        return network.getBroadcastUri() != null ? network.getBroadcastUri().getPath().substring(1) : null;
-        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 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()

plugins/network-elements/nuage-vsp/src/main/java/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;
+    }
 }

plugins/network-elements/nuage-vsp/src/test/java/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());
 

plugins/network-elements/nuage-vsp/src/test/java/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));
     }

server/src/main/java/com/cloud/network/NetworkModelImpl.java

@@ -930,7 +930,7 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
     @Override
     public String getIpOfNetworkElementInVirtualNetwork(long accountId, long dataCenterId) {
 
-        List<NetworkVO> virtualNetworks = _networksDao.listByZoneAndGuestType(accountId, dataCenterId, Network.GuestType.Isolated, false);
+        List<NetworkVO> virtualNetworks = _networksDao.listByZoneAndGuestType(accountId, dataCenterId, GuestType.Isolated, false);
 
         if (virtualNetworks.isEmpty()) {
             s_logger.trace("Unable to find default Virtual network account id=" + accountId);
@@ -950,13 +950,13 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
     }
 
     @Override
-    public List<NetworkVO> listNetworksForAccount(long accountId, long zoneId, Network.GuestType type) {
+    public List<NetworkVO> listNetworksForAccount(long accountId, long zoneId, GuestType type) {
         List<NetworkVO> accountNetworks = new ArrayList<NetworkVO>();
         List<NetworkVO> zoneNetworks = _networksDao.listByZone(zoneId);
 
         for (NetworkVO network : zoneNetworks) {
             if (!isNetworkSystem(network)) {
-                if (network.getGuestType() == Network.GuestType.Shared || !_networksDao.listBy(accountId, network.getId()).isEmpty()) {
+                if (network.getGuestType() == GuestType.Shared || !_networksDao.listBy(accountId, network.getId()).isEmpty()) {
                     if (type == null || type == network.getGuestType()) {
                         accountNetworks.add(network);
                     }
@@ -967,7 +967,7 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
     }
 
     @Override
-    public List<NetworkVO> listAllNetworksInAllZonesByType(Network.GuestType type) {
+    public List<NetworkVO> listAllNetworksInAllZonesByType(GuestType type) {
         List<NetworkVO> networks = new ArrayList<NetworkVO>();
         for (NetworkVO network : _networksDao.listAll()) {
             if (!isNetworkSystem(network)) {
@@ -1637,7 +1637,8 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
             throw new CloudRuntimeException("cannot check permissions on (Network) <null>");
         }
         // Perform account permission check
-        if (network.getGuestType() != Network.GuestType.Shared || (network.getGuestType() == Network.GuestType.Shared && network.getAclType() == ACLType.Account)) {
+        if ((network.getGuestType() != GuestType.Shared && network.getGuestType() != GuestType.L2) ||
+                (network.getGuestType() == GuestType.Shared && network.getAclType() == ACLType.Account)) {
             AccountVO networkOwner = _accountDao.findById(network.getAccountId());
             if (networkOwner == null)
                 throw new PermissionDeniedException("Unable to use network with id= " + ((NetworkVO)network).getUuid() +
@@ -1802,14 +1803,14 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi
     public boolean isNetworkAvailableInDomain(long networkId, long domainId) {
         Long networkDomainId = null;
         Network network = getNetwork(networkId);
-        if (network.getGuestType() != Network.GuestType.Shared) {
+        if (network.getGuestType() != GuestType.Shared && network.getGuestType() != GuestType.L2) {
-            s_logger.trace("Network id=" + networkId + " is not shared");
+            s_logger.trace("Network id=" + networkId + " is not shared or L2");
             return false;
         }
 
         NetworkDomainVO networkDomainMap = _networkDomainDao.getDomainNetworkMapByNetworkId(networkId);
         if (networkDomainMap == null) {
-            s_logger.trace("Network id=" + networkId + " is shared, but not domain specific");
+            s_logger.trace("Network id=" + networkId + " is shared or L2, but not domain specific");
             return true;
         } else {
             networkDomainId = networkDomainMap.getDomainId();

server/src/main/java/com/cloud/network/NetworkServiceImpl.java

@@ -1101,8 +1101,8 @@ public class NetworkServiceImpl extends ManagerBase implements  NetworkService {
         }
 
         // Only Admin can create Shared networks
-        if (ntwkOff.getGuestType() == GuestType.Shared && !_accountMgr.isAdmin(caller.getId())) {
+        if ((ntwkOff.getGuestType() == GuestType.Shared || ntwkOff.getGuestType() == GuestType.L2) && !_accountMgr.isAdmin(caller.getId())) {
-            throw new InvalidParameterValueException("Only Admins can create network with guest type " + GuestType.Shared);
+            throw new InvalidParameterValueException("Only Admins can create network with guest type " + GuestType.Shared + " or " + GuestType.L2);
         }
 
         // Check if the network is domain specific

server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java

@@ -119,11 +119,9 @@ public class RoleManagerImpl extends ManagerBase implements RoleService, Configu
 
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_ROLE_UPDATE, eventDescription = "updating Role")
-    public boolean updateRole(final Role role, final String name, final RoleType roleType, final String description) {
+    public Role updateRole(final Role role, final String name, final RoleType roleType, final String description) {
         checkCallerAccess();
-        if (role == null) {
+
-            return false;
-        }
         if (roleType != null && roleType == RoleType.Unknown) {
             throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Unknown is not a valid role type");
         }
@@ -145,7 +143,9 @@ public class RoleManagerImpl extends ManagerBase implements RoleService, Configu
         if (!Strings.isNullOrEmpty(description)) {
             roleVO.setDescription(description);
         }
-        return roleDao.update(role.getId(), roleVO);
+
+        roleDao.update(role.getId(), roleVO);
+        return role;
     }
 
     @Override

systemvm/debian/opt/cloud/bin/cs/CsHelper.py

@@ -183,13 +183,19 @@ def get_hostname():
 
 def execute(command):
     """ Execute command """
-    p = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
+    returncode = -1
-    p.wait()
+    try:
-    rc = p.returncode
+        logging.info("Executing: %s" % command)
-
+        result = subprocess.check_output(command, shell=True)
-    logging.debug("Executed: %s - exitstatus=%s " % (command, rc))
+        returncode = 0
-    result = p.communicate()[0]
         return result.splitlines()
+    except subprocess.CalledProcessError as e:
+        logging.error(e)
+        returncode = e.returncode
+    finally:
+        logging.debug("Executed: %s - exitstatus=%s " % (command, returncode))
+
+    return list()
 
 
 def save_iptables(command, iptables_file):

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 ")

test/integration/smoke/test_dynamicroles.py

@@ -209,7 +209,8 @@ class TestDynamicRoles(cloudstackTestCase):
         """
         self.account.delete(self.apiclient)
         new_role_name = self.getRandomRoleName()
-        self.role.update(self.apiclient, name=new_role_name, type='Admin')
+        new_role_description = "Fake role description created after update"
+        self.role.update(self.apiclient, name=new_role_name, type='Admin', description=new_role_description)
         update_role = Role.list(self.apiclient, id=self.role.id)[0]
         self.assertEqual(
             update_role.name,
@@ -221,7 +222,11 @@ class TestDynamicRoles(cloudstackTestCase):
             'Admin',
             msg="Role type does not match updated role type"
         )
-
+        self.assertEqual(
+            update_role.description,
+            new_role_description,
+            msg="Role description does not match updated role description"
+            )
 
     @attr(tags=['advanced', 'simulator', 'basic', 'sg'], required_hardware=False)
     def test_role_lifecycle_update_role_inuse(self):