Applying the latest chances from the VPC Refactor branch onto the new one.

We will keep this branch as clean as possible to avoid problems with merge.

server/src/com/cloud/network/element/VirtualRouterElement.java

@@ -717,11 +717,14 @@ public class VirtualRouterElement extends AdapterBase implements VirtualRouterEl
 
         VirtualMachineProfile uservm = vm;
 
+        DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
+        NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
+
         // If any router is running then send save password command otherwise
         // save the password in DB
         for (VirtualRouter router : routers) {
             if (router.getState() == State.Running) {
-                return _routerMgr.savePasswordToRouter(network, nic, uservm, routers);
+                return networkTopology.savePasswordToRouter(network, nic, uservm, routers);
             }
         }
         String password = (String) uservm.getParameter(VirtualMachineProfile.Param.VmPassword);
@@ -735,10 +738,7 @@ public class VirtualRouterElement extends AdapterBase implements VirtualRouterEl
         userVmVO.setUpdateParameters(true);
         _userVmDao.update(userVmVO.getId(), userVmVO);
 
-        DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
+        return true;
-        NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
-
-        return networkTopology.savePasswordToRouter(network, nic, uservm, routers);
     }
 
     @Override

server/src/com/cloud/network/router/CommandSetupHelper.java

@@ -107,6 +107,8 @@ import com.cloud.offering.NetworkOffering;
 import com.cloud.offerings.NetworkOfferingVO;
 import com.cloud.offerings.dao.NetworkOfferingDao;
 import com.cloud.service.dao.ServiceOfferingDao;
+import com.cloud.storage.GuestOSVO;
+import com.cloud.storage.dao.GuestOSDao;
 import com.cloud.user.Account;
 import com.cloud.uservm.UserVm;
 import com.cloud.utils.Pair;
@@ -173,6 +175,8 @@ public class CommandSetupHelper {
     private VlanDao _vlanDao;
     @Inject
     private IPAddressDao _ipAddressDao;
+    @Inject
+    private GuestOSDao _guestOSDao;
 
     @Inject
     private RouterControlHelper _routerControlHelper;
@@ -216,12 +220,17 @@ public class CommandSetupHelper {
     public void createDhcpEntryCommand(final VirtualRouter router, final UserVm vm, final NicVO nic, final Commands cmds) {
         final DhcpEntryCommand dhcpCommand = new DhcpEntryCommand(nic.getMacAddress(), nic.getIp4Address(), vm.getHostName(), nic.getIp6Address(),
                 _networkModel.getExecuteInSeqNtwkElmtCmd());
-        final DataCenterVO dcVo = _dcDao.findById(router.getDataCenterId());
+
-        final Nic defaultNic = findGatewayIp(vm.getId());
+        String gatewayIp = nic.getGateway();
-        String gatewayIp = defaultNic.getGateway();
+        if (!nic.isDefaultNic()) {
-        if (gatewayIp != null && !gatewayIp.equals(nic.getGateway())) {
+            GuestOSVO guestOS = _guestOSDao.findById(vm.getGuestOSId());
+            if (guestOS == null || !guestOS.getDisplayName().toLowerCase().contains("windows")) {
                 gatewayIp = "0.0.0.0";
             }
+        }
+
+        final DataCenterVO dcVo = _dcDao.findById(router.getDataCenterId());
+
         dhcpCommand.setDefaultRouter(gatewayIp);
         dhcpCommand.setIp6Gateway(nic.getIp6Gateway());
         String ipaddress = null;

server/src/com/cloud/network/router/NetworkHelperImpl.java

@@ -174,7 +174,7 @@ public class NetworkHelperImpl implements NetworkHelper {
             throw new AgentUnavailableException("Unable to send commands to virtual router ", router.getHostId(), e);
         }
 
-        if (answers == null || answers.length != cmds.size()) {
+        if ((answers == null) || (answers.length != cmds.size())) {
             return false;
         }
 
@@ -195,7 +195,7 @@ public class NetworkHelperImpl implements NetworkHelper {
         if (connectedRouters.isEmpty() || disconnectedRouters.isEmpty()) {
             return;
         }
-        if (connectedRouters.size() != 1 || disconnectedRouters.size() != 1) {
+        if ((connectedRouters.size() != 1) || (disconnectedRouters.size() != 1)) {
             s_logger.warn("How many redundant routers do we have?? ");
             return;
         }
@@ -240,12 +240,6 @@ public class NetworkHelperImpl implements NetworkHelper {
         }
     }
 
-    /*
-     * (non-Javadoc)
-     * 
-     * @see com.cloud.network.router.NetworkHelper#getRealPriority(com.cloud.vm.
-     * DomainRouterVO)
-     */
     @Override
     public int getRealPriority(final DomainRouterVO router) {
         int priority = router.getPriority();
@@ -255,14 +249,6 @@ public class NetworkHelperImpl implements NetworkHelper {
         return priority;
     }
 
-    // @Override
-    /*
-     * (non-Javadoc)
-     * 
-     * @see
-     * com.cloud.network.router.NetworkHelper#getNicTO(com.cloud.network.router
-     * .VirtualRouter, java.lang.Long, java.lang.String)
-     */
     @Override
     public NicTO getNicTO(final VirtualRouter router, final Long networkId, final String broadcastUri) {
         NicProfile nicProfile = _networkModel.getNicProfile(router, networkId, broadcastUri);
@@ -270,13 +256,6 @@ public class NetworkHelperImpl implements NetworkHelper {
         return _itMgr.toNicTO(nicProfile, router.getHypervisorType());
     }
 
-    // @Override
-    /*
-     * (non-Javadoc)
-     * 
-     * @see com.cloud.network.router.NetworkHelper#destroyRouter(long,
-     * com.cloud.user.Account, java.lang.Long)
-     */
     @Override
     public VirtualRouter destroyRouter(final long routerId, final Account caller, final Long callerUserId) throws ResourceUnavailableException, ConcurrentOperationException {
 
@@ -296,14 +275,6 @@ public class NetworkHelperImpl implements NetworkHelper {
         return router;
     }
 
-    /*
-     * (non-Javadoc)
-     * 
-     * @see
-     * com.cloud.network.router.NetworkHelper#checkRouterVersion(com.cloud.network
-     * .router.VirtualRouter)
-     */
-    // @Override
     @Override
     public boolean checkRouterVersion(final VirtualRouter router) {
         if (!VirtualNetworkApplianceManagerImpl.routerVersionCheckEnabled.value()) {
@@ -377,9 +348,9 @@ public class NetworkHelperImpl implements NetworkHelper {
         for (DomainRouterVO router : routerDeploymentDefinition.getRouters()) {
             boolean skip = false;
             final State state = router.getState();
-            if (router.getHostId() != null && state != State.Running) {
+            if ((router.getHostId() != null) && (state != State.Running)) {
                 final HostVO host = _hostDao.findById(router.getHostId());
-                if (host == null || host.getState() != Status.Up) {
+                if ((host == null) || (host.getState() != Status.Up)) {
                     skip = true;
                 }
             }
@@ -399,7 +370,7 @@ public class NetworkHelperImpl implements NetworkHelper {
     public DomainRouterVO startVirtualRouter(final DomainRouterVO router, final User user, final Account caller, final Map<Param, Object> params)
             throws StorageUnavailableException, InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException {
 
-        if (router.getRole() != Role.VIRTUAL_ROUTER || !router.getIsRedundantRouter()) {
+        if ((router.getRole() != Role.VIRTUAL_ROUTER) || !router.getIsRedundantRouter()) {
             return start(router, user, caller, params, null);
         }
 
@@ -429,7 +400,7 @@ public class NetworkHelperImpl implements NetworkHelper {
         if (networkIds.size() != 0) {
             final List<DomainRouterVO> routerList = _routerDao.findByNetwork(networkIds.get(0));
             for (final DomainRouterVO rrouter : routerList) {
-                if (rrouter.getHostId() != null && rrouter.getIsRedundantRouter() && rrouter.getState() == State.Running) {
+                if ((rrouter.getHostId() != null) && rrouter.getIsRedundantRouter() && (rrouter.getState() == State.Running)) {
                     if (routerToBeAvoid != null) {
                         throw new ResourceUnavailableException("Try to start router " + router.getInstanceName() + "(" + router.getId() + ")"
                                 + ", but there are already two redundant routers with IP " + router.getPublicIpAddress() + ", they are " + rrouter.getInstanceName() + "("
@@ -452,7 +423,7 @@ public class NetworkHelperImpl implements NetworkHelper {
         avoids[1].addCluster(_hostDao.findById(routerToBeAvoid.getHostId()).getClusterId());
         avoids[2] = new ExcludeList();
         final List<VolumeVO> volumes = _volumeDao.findByInstanceAndType(routerToBeAvoid.getId(), Volume.Type.ROOT);
-        if (volumes != null && volumes.size() != 0) {
+        if ((volumes != null) && (volumes.size() != 0)) {
             avoids[2].addPool(volumes.get(0).getPoolId());
         }
         avoids[2].addHost(routerToBeAvoid.getHostId());
@@ -553,7 +524,7 @@ public class NetworkHelperImpl implements NetworkHelper {
                 _itMgr.allocate(router.getInstanceName(), template, routerOffering, networks, routerDeploymentDefinition.getPlan(), null);
                 router = _routerDao.findById(router.getId());
             } catch (final InsufficientCapacityException ex) {
-                if (allocateRetry < 2 && iter.hasNext()) {
+                if ((allocateRetry < 2) && iter.hasNext()) {
                     s_logger.debug("Failed to allocate the VR with hypervisor type " + hType + ", retrying one more time");
                     continue;
                 } else {
@@ -568,7 +539,7 @@ public class NetworkHelperImpl implements NetworkHelper {
                     router = startVirtualRouter(router, _accountMgr.getSystemUser(), _accountMgr.getSystemAccount(), routerDeploymentDefinition.getParams());
                     break;
                 } catch (final InsufficientCapacityException ex) {
-                    if (startRetry < 2 && iter.hasNext()) {
+                    if ((startRetry < 2) && iter.hasNext()) {
                         s_logger.debug("Failed to start the VR  " + router + " with hypervisor type " + hType + ", " + "destroying it and recreating one more time");
                         // destroy the router
                         destroyRouter(router.getId(), _accountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM), User.UID_SYSTEM);
@@ -637,12 +608,12 @@ public class NetworkHelperImpl implements NetworkHelper {
     protected HypervisorType getClusterToStartDomainRouterForOvm(final long podId) {
         final List<ClusterVO> clusters = _clusterDao.listByPodId(podId);
         for (final ClusterVO cv : clusters) {
-            if (cv.getHypervisorType() == HypervisorType.Ovm || cv.getHypervisorType() == HypervisorType.BareMetal) {
+            if ((cv.getHypervisorType() == HypervisorType.Ovm) || (cv.getHypervisorType() == HypervisorType.BareMetal)) {
                 continue;
             }
 
             final List<HostVO> hosts = _resourceMgr.listAllHostsInCluster(cv.getId());
-            if (hosts == null || hosts.isEmpty()) {
+            if ((hosts == null) || hosts.isEmpty()) {
                 continue;
             }
 
@@ -674,13 +645,14 @@ public class NetworkHelperImpl implements NetworkHelper {
             if (!routerDeploymentDefinition.isPublicNetwork()) {
                 final Nic placeholder = _networkModel.getPlaceholderNicForRouter(routerDeploymentDefinition.getGuestNetwork(), routerDeploymentDefinition.getPodId());
                 if (routerDeploymentDefinition.getGuestNetwork().getCidr() != null) {
-                    if (placeholder != null && placeholder.getIp4Address() != null) {
+                    if ((placeholder != null) && (placeholder.getIp4Address() != null)) {
                         s_logger.debug("Requesting ipv4 address " + placeholder.getIp4Address() + " stored in placeholder nic for the network "
                                 + routerDeploymentDefinition.getGuestNetwork());
                         defaultNetworkStartIp = placeholder.getIp4Address();
                     } else {
                         final String startIp = _networkModel.getStartIpAddress(routerDeploymentDefinition.getGuestNetwork().getId());
-                        if (startIp != null && _ipAddressDao.findByIpAndSourceNetworkId(routerDeploymentDefinition.getGuestNetwork().getId(), startIp).getAllocatedTime() == null) {
+                        if ((startIp != null)
+                                && (_ipAddressDao.findByIpAndSourceNetworkId(routerDeploymentDefinition.getGuestNetwork().getId(), startIp).getAllocatedTime() == null)) {
                             defaultNetworkStartIp = startIp;
                         } else if (s_logger.isDebugEnabled()) {
                             s_logger.debug("First ipv4 " + startIp + " in network id=" + routerDeploymentDefinition.getGuestNetwork().getId()
@@ -690,13 +662,13 @@ public class NetworkHelperImpl implements NetworkHelper {
                 }
 
                 if (routerDeploymentDefinition.getGuestNetwork().getIp6Cidr() != null) {
-                    if (placeholder != null && placeholder.getIp6Address() != null) {
+                    if ((placeholder != null) && (placeholder.getIp6Address() != null)) {
                         s_logger.debug("Requesting ipv6 address " + placeholder.getIp6Address() + " stored in placeholder nic for the network "
                                 + routerDeploymentDefinition.getGuestNetwork());
                         defaultNetworkStartIpv6 = placeholder.getIp6Address();
                     } else {
                         final String startIpv6 = _networkModel.getStartIpv6Address(routerDeploymentDefinition.getGuestNetwork().getId());
-                        if (startIpv6 != null && _ipv6Dao.findByNetworkIdAndIp(routerDeploymentDefinition.getGuestNetwork().getId(), startIpv6) == null) {
+                        if ((startIpv6 != null) && (_ipv6Dao.findByNetworkIdAndIp(routerDeploymentDefinition.getGuestNetwork().getId(), startIpv6) == null)) {
                             defaultNetworkStartIpv6 = startIpv6;
                         } else if (s_logger.isDebugEnabled()) {
                             s_logger.debug("First ipv6 " + startIpv6 + " in network id=" + routerDeploymentDefinition.getGuestNetwork().getId()

server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManager.java

@@ -23,9 +23,7 @@ import com.cloud.exception.ResourceUnavailableException;
 import com.cloud.network.RemoteAccessVpn;
 import com.cloud.network.Site2SiteVpnConnection;
 import com.cloud.network.VpcVirtualNetworkApplianceService;
-import com.cloud.network.VpnUser;
 import com.cloud.network.vpc.PrivateGateway;
-import com.cloud.network.vpc.StaticRouteProfile;
 import com.cloud.vm.DomainRouterVO;
 
 public interface VpcVirtualNetworkApplianceManager extends VirtualNetworkApplianceManager, VpcVirtualNetworkApplianceService {
@@ -76,13 +74,4 @@ public interface VpcVirtualNetworkApplianceManager extends VirtualNetworkApplian
      * @throws ResourceUnavailableException
      */
     boolean stopRemoteAccessVpn(RemoteAccessVpn vpn, VirtualRouter router) throws ResourceUnavailableException;
-
-    /**
-     * @param vpn
-     * @param users
-     * @param routers
-     * @return
-     * @throws ResourceUnavailableException
-     */
-    String[] applyVpnUsers(RemoteAccessVpn vpn, List<? extends VpnUser> users, VirtualRouter router) throws ResourceUnavailableException;
 }

server/src/com/cloud/network/rules/BasicVpnRules.java

@@ -26,11 +26,11 @@ import com.cloud.network.Network;
 import com.cloud.network.VpnUser;
 import com.cloud.network.router.VirtualRouter;
 
-public class VpnRules extends RuleApplier {
+public class BasicVpnRules extends RuleApplier {
 
     private final List<? extends VpnUser> _users;
 
-    public VpnRules(final Network network, final List<? extends VpnUser> users) {
+    public BasicVpnRules(final Network network, final List<? extends VpnUser> users) {
         super(network);
         _users = users;
     }

server/src/com/cloud/network/rules/DhcpEntryRules.java

@@ -62,8 +62,12 @@ public class DhcpEntryRules extends RuleApplier {
         return visitor.visit(this);
     }
 
-    public NicProfile getNic() {
+    public VirtualMachineProfile getProfile() {
-        return _nic;
+        return _profile;
+    }
+
+    public DeployDestination getDestination() {
+        return _destination;
     }
 
     public NicVO getNicVo() {

server/src/com/cloud/network/rules/DhcpPvlanRules.java

@@ -27,7 +27,7 @@ import com.cloud.network.dao.NetworkDao;
 import com.cloud.network.router.VirtualRouter;
 import com.cloud.vm.NicProfile;
 
-public class DhcpRules extends RuleApplier {
+public class DhcpPvlanRules extends RuleApplier {
 
     private final boolean _isAddPvlan;
     private final NicProfile _nic;

server/src/com/cloud/network/rules/LoadBalancingRules.java

@@ -53,8 +53,7 @@ public class LoadBalancingRules extends RuleApplier {
         // For load balancer we have to resend all lb rules for the network
         final List<LoadBalancerVO> lbs = loadBalancerDao.listByNetworkIdAndScheme(_network.getId(), Scheme.Public);
 
-        // We are cleaning it before because all the rules have to be sent to
+        // We are cleaning it before because all the rules have to be sent to the router.
-        // the router.
         _rules.clear();
 
         LoadBalancingRulesManager lbMgr = visitor.getVirtualNetworkApplianceFactory().getLbMgr();

server/src/com/cloud/network/rules/VirtualNetworkApplianceFactory.java

@@ -18,6 +18,9 @@ package com.cloud.network.rules;
 
 import javax.inject.Inject;
 
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Qualifier;
+
 import com.cloud.dc.dao.DataCenterDao;
 import com.cloud.dc.dao.HostPodDao;
 import com.cloud.dc.dao.VlanDao;
@@ -80,8 +83,11 @@ public class VirtualNetworkApplianceFactory {
     private IpAddressManager _ipAddrMgr;
     @Inject
     private NetworkACLManager _networkACLMgr;
-    @Inject
+
+    @Autowired
+    @Qualifier("networkHelper")
     private NetworkHelper _networkHelper;
+
     @Inject
     private NicProfileHelper _nicProfileHelper;
 

server/src/org/apache/cloudstack/network/topology/AdvancedNetworkTopology.java

@@ -35,7 +35,7 @@ import com.cloud.network.VpnUser;
 import com.cloud.network.router.VirtualRouter;
 import com.cloud.network.rules.AdvancedVpnRules;
 import com.cloud.network.rules.DhcpEntryRules;
-import com.cloud.network.rules.DhcpSubNetRules;
+import com.cloud.network.rules.DhcpPvlanRules;
 import com.cloud.network.rules.NetworkAclsRules;
 import com.cloud.network.rules.NicPlugInOutRules;
 import com.cloud.network.rules.PrivateGatewayRules;
@@ -90,6 +90,8 @@ public class AdvancedNetworkTopology extends BasicNetworkTopology {
     @Override
     public boolean applyStaticRoutes(final List<StaticRouteProfile> staticRoutes, final List<DomainRouterVO> routers) throws ResourceUnavailableException {
 
+        s_logger.debug("APPLYING STATIC ROUTES RULES");
+
         if (staticRoutes == null || staticRoutes.isEmpty()) {
             s_logger.debug("No static routes to apply");
             return true;
@@ -129,24 +131,6 @@ public class AdvancedNetworkTopology extends BasicNetworkTopology {
         return pvlanRules.accept(_advancedVisitor, router);
     }
 
-    @Override
-    public boolean configDhcpForSubnet(final Network network, final NicProfile nic, final VirtualMachineProfile profile, final DeployDestination dest,
-            final List<DomainRouterVO> routers) throws ResourceUnavailableException {
-
-        s_logger.debug("CONFIG DHCP FOR SUBNETS RULES");
-
-        // Asuming we have only one router per network For Now.
-        final DomainRouterVO router = routers.get(0);
-        if (router.getState() != State.Running) {
-            s_logger.warn("Failed to configure dhcp: router not in running state");
-            throw new ResourceUnavailableException("Unable to assign ip addresses, domR is not in right state " + router.getState(), DataCenter.class, network.getDataCenterId());
-        }
-
-        DhcpSubNetRules subNetRules = new DhcpSubNetRules(network, nic, profile);
-
-        return subNetRules.accept(_advancedVisitor, router);
-    }
-
     @Override
     public boolean setupPrivateGateway(final PrivateGateway gateway, final VirtualRouter router) throws ConcurrentOperationException, ResourceUnavailableException {
         s_logger.debug("SETUP PRIVATE GATEWAY RULES");

server/src/org/apache/cloudstack/network/topology/AdvancedNetworkVisitor.java

@@ -26,7 +26,6 @@ import org.springframework.stereotype.Component;
 
 import com.cloud.agent.api.Command;
 import com.cloud.agent.api.PvlanSetupCommand;
-import com.cloud.agent.api.routing.IpAliasTO;
 import com.cloud.agent.manager.Commands;
 import com.cloud.dc.DataCenter;
 import com.cloud.exception.ResourceUnavailableException;
@@ -36,7 +35,7 @@ import com.cloud.network.VpnUser;
 import com.cloud.network.router.VirtualRouter;
 import com.cloud.network.rules.AdvancedVpnRules;
 import com.cloud.network.rules.DhcpEntryRules;
-import com.cloud.network.rules.DhcpSubNetRules;
+import com.cloud.network.rules.DhcpPvlanRules;
 import com.cloud.network.rules.NetworkAclsRules;
 import com.cloud.network.rules.NicPlugInOutRules;
 import com.cloud.network.rules.PrivateGatewayRules;
@@ -53,7 +52,6 @@ import com.cloud.vm.NicVO;
 import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VirtualMachine.State;
 import com.cloud.vm.VirtualMachineProfile;
-import com.cloud.vm.dao.NicIpAliasVO;
 
 @Component
 public class AdvancedNetworkVisitor extends BasicNetworkVisitor {
@@ -190,27 +188,6 @@ public class AdvancedNetworkVisitor extends BasicNetworkVisitor {
         }
     }
 
-    @Override
-    public boolean visit(final DhcpSubNetRules subnet) throws ResourceUnavailableException {
-        final VirtualRouter router = subnet.getRouter();
-        final Network network = subnet.getNetwork();
-        final NicIpAliasVO nicAlias = subnet.getNicAlias();
-        final String routerAliasIp = subnet.getRouterAliasIp();
-
-        final Commands cmds = new Commands(Command.OnError.Stop);
-
-        final List<IpAliasTO> ipaliasTo = new ArrayList<IpAliasTO>();
-        ipaliasTo.add(new IpAliasTO(routerAliasIp, nicAlias.getNetmask(), nicAlias.getAliasCount().toString()));
-
-        _commandSetupHelper.createIpAlias(router, ipaliasTo, nicAlias.getNetworkId(), cmds);
-
-        // also add the required configuration to the dnsmasq for supporting
-        // dhcp and dns on the new ip.
-        _commandSetupHelper.configDnsMasq(router, network, cmds);
-
-        return _networkGeneralHelper.sendCommandsToRouter(router, cmds);
-    }
-
     @Override
     public boolean visit(final StaticRoutesRules staticRoutesRules) throws ResourceUnavailableException {
         final VirtualRouter router = staticRoutesRules.getRouter();

server/src/org/apache/cloudstack/network/topology/BasicNetworkTopology.java

@@ -45,7 +45,9 @@ import com.cloud.network.VpnUser;
 import com.cloud.network.lb.LoadBalancingRule;
 import com.cloud.network.router.NetworkHelper;
 import com.cloud.network.router.VirtualRouter;
+import com.cloud.network.rules.BasicVpnRules;
 import com.cloud.network.rules.DhcpEntryRules;
+import com.cloud.network.rules.DhcpSubNetRules;
 import com.cloud.network.rules.FirewallRule;
 import com.cloud.network.rules.FirewallRules;
 import com.cloud.network.rules.IpAssociationRules;
@@ -58,18 +60,15 @@ import com.cloud.network.rules.StaticNat;
 import com.cloud.network.rules.StaticNatRules;
 import com.cloud.network.rules.UserdataPwdRules;
 import com.cloud.network.rules.UserdataToRouterRules;
-import com.cloud.network.rules.VpnRules;
 import com.cloud.network.vpc.NetworkACLItem;
 import com.cloud.network.vpc.PrivateGateway;
 import com.cloud.network.vpc.StaticRouteProfile;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.DomainRouterVO;
 import com.cloud.vm.NicProfile;
-import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.VirtualMachine.State;
 import com.cloud.vm.VirtualMachineProfile;
-import com.cloud.vm.dao.UserVmDao;
 
 @Component
 public class BasicNetworkTopology implements NetworkTopology {
@@ -90,14 +89,6 @@ public class BasicNetworkTopology implements NetworkTopology {
     @Qualifier("networkHelper")
     protected NetworkHelper _networkHelper;
 
-    @Inject
-    protected UserVmDao _userVmDao;
-
-    @Override
-    public StringBuilder createGuestBootLoadArgs(final NicProfile guestNic, final String defaultDns1, final String defaultDns2, final DomainRouterVO router) {
-        return null;
-    }
-
     @Override
     public NetworkTopologyVisitor getVisitor() {
         return _basicVisitor;
@@ -133,7 +124,18 @@ public class BasicNetworkTopology implements NetworkTopology {
     public boolean configDhcpForSubnet(final Network network, final NicProfile nic, final VirtualMachineProfile profile, final DeployDestination dest,
             final List<DomainRouterVO> routers) throws ResourceUnavailableException {
 
-        throw new CloudRuntimeException("configDhcpForSubnet not implemented in Basic Network Topology.");
+        s_logger.debug("CONFIG DHCP FOR SUBNETS RULES");
+
+        // Assuming we have only one router per network For Now.
+        final DomainRouterVO router = routers.get(0);
+        if (router.getState() != State.Running) {
+            s_logger.warn("Failed to configure dhcp: router not in running state");
+            throw new ResourceUnavailableException("Unable to assign ip addresses, domR is not in right state " + router.getState(), DataCenter.class, network.getDataCenterId());
+        }
+
+        DhcpSubNetRules subNetRules = new DhcpSubNetRules(network, nic, profile);
+
+        return subNetRules.accept(_basicVisitor, router);
     }
 
     @Override
@@ -270,7 +272,7 @@ public class BasicNetworkTopology implements NetworkTopology {
             throw new ResourceUnavailableException("Unable to assign ip addresses, domR doesn't exist for network " + network.getId(), DataCenter.class, network.getDataCenterId());
         }
 
-        s_logger.debug("APPLYING VPN RULES");
+        s_logger.debug("APPLYING BASIC VPN RULES");
 
         BasicVpnRules vpnRules = new BasicVpnRules(network, users);
         boolean agentResults = true;
@@ -282,8 +284,6 @@ public class BasicNetworkTopology implements NetworkTopology {
                         network.getDataCenterId());
             }
 
-            VpnRules vpnRules = _virtualNetworkApplianceFactory.createVpnRules(network, users);
-
             // Currently we receive just one answer from the agent. In the
             // future we have to parse individual answers and set
             // results accordingly
@@ -307,8 +307,6 @@ public class BasicNetworkTopology implements NetworkTopology {
     public boolean savePasswordToRouter(final Network network, final NicProfile nic, final VirtualMachineProfile profile, final List<? extends VirtualRouter> routers)
             throws ResourceUnavailableException {
 
-        _userVmDao.loadDetails((UserVmVO) profile.getVirtualMachine());
-
         s_logger.debug("SAVE PASSWORD TO ROUTE RULES");
 
         final String typeString = "save password entry";

server/src/org/apache/cloudstack/network/topology/BasicNetworkVisitor.java

@@ -17,14 +17,18 @@
 
 package org.apache.cloudstack.network.topology;
 
+import java.util.ArrayList;
 import java.util.List;
 
 import javax.inject.Inject;
 
 import org.apache.log4j.Logger;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.stereotype.Component;
 
 import com.cloud.agent.api.Command;
+import com.cloud.agent.api.routing.IpAliasTO;
 import com.cloud.agent.manager.Commands;
 import com.cloud.deploy.DeployDestination;
 import com.cloud.exception.ResourceUnavailableException;
@@ -35,7 +39,10 @@ import com.cloud.network.lb.LoadBalancingRule;
 import com.cloud.network.router.CommandSetupHelper;
 import com.cloud.network.router.NetworkHelper;
 import com.cloud.network.router.VirtualRouter;
+import com.cloud.network.rules.AdvancedVpnRules;
+import com.cloud.network.rules.BasicVpnRules;
 import com.cloud.network.rules.DhcpEntryRules;
+import com.cloud.network.rules.DhcpPvlanRules;
 import com.cloud.network.rules.DhcpSubNetRules;
 import com.cloud.network.rules.FirewallRule;
 import com.cloud.network.rules.FirewallRule.Purpose;
@@ -62,17 +69,15 @@ import com.cloud.vm.DomainRouterVO;
 import com.cloud.vm.NicVO;
 import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VirtualMachineProfile;
+import com.cloud.vm.dao.NicIpAliasVO;
 
 @Component
 public class BasicNetworkVisitor extends NetworkTopologyVisitor {
 
     private static final Logger s_logger = Logger.getLogger(BasicNetworkVisitor.class);
 
-    public BasicNetworkVisitor(final NetworkTopology networkTopology) {
+    @Autowired
-        super(networkTopology);
+    @Qualifier("networkHelper")
-    }
-
-    @Inject
     protected NetworkHelper _networkGeneralHelper;
 
     @Inject
@@ -257,13 +262,29 @@ public class BasicNetworkVisitor extends NetworkTopologyVisitor {
     }
 
     @Override
-    public boolean visit(final DhcpPvlanRules vpn) throws ResourceUnavailableException {
+    public boolean visit(final DhcpSubNetRules subnet) throws ResourceUnavailableException {
-        throw new CloudRuntimeException("DhcpPvlanRules not implemented in Basic Network Topology.");
+        final VirtualRouter router = subnet.getRouter();
+        final Network network = subnet.getNetwork();
+        final NicIpAliasVO nicAlias = subnet.getNicAlias();
+        final String routerAliasIp = subnet.getRouterAliasIp();
+
+        final Commands cmds = new Commands(Command.OnError.Stop);
+
+        final List<IpAliasTO> ipaliasTo = new ArrayList<IpAliasTO>();
+        ipaliasTo.add(new IpAliasTO(routerAliasIp, nicAlias.getNetmask(), nicAlias.getAliasCount().toString()));
+
+        _commandSetupHelper.createIpAlias(router, ipaliasTo, nicAlias.getNetworkId(), cmds);
+
+        // also add the required configuration to the dnsmasq for supporting
+        // dhcp and dns on the new ip.
+        _commandSetupHelper.configDnsMasq(router, network, cmds);
+
+        return _networkGeneralHelper.sendCommandsToRouter(router, cmds);
     }
 
     @Override
-    public boolean visit(final DhcpSubNetRules vpn) throws ResourceUnavailableException {
+    public boolean visit(final DhcpPvlanRules dhcpRules) throws ResourceUnavailableException {
-        throw new CloudRuntimeException("DhcpSubNetRules not implemented in Basic Network Topology.");
+        throw new CloudRuntimeException("DhcpPvlanRules not implemented in Basic Network Topology.");
     }
 
     @Override
@@ -272,17 +293,17 @@ public class BasicNetworkVisitor extends NetworkTopologyVisitor {
     }
 
     @Override
-    public boolean visit(final NetworkAclsRules nat) throws ResourceUnavailableException {
+    public boolean visit(final NetworkAclsRules aclsRules) throws ResourceUnavailableException {
         throw new CloudRuntimeException("NetworkAclsRules not implemented in Basic Network Topology.");
     }
 
     @Override
-    public boolean visit(final VpcIpAssociationRules nat) throws ResourceUnavailableException {
+    public boolean visit(final VpcIpAssociationRules ipRules) throws ResourceUnavailableException {
         throw new CloudRuntimeException("VpcIpAssociationRules not implemented in Basic Network Topology.");
     }
 
     @Override
-    public boolean visit(final PrivateGatewayRules userdata) throws ResourceUnavailableException {
+    public boolean visit(final PrivateGatewayRules pvtGatewayRules) throws ResourceUnavailableException {
         throw new CloudRuntimeException("PrivateGatewayRules not implemented in Basic Network Topology.");
     }
 
@@ -290,4 +311,9 @@ public class BasicNetworkVisitor extends NetworkTopologyVisitor {
     public boolean visit(final StaticRoutesRules staticRoutesRules) throws ResourceUnavailableException {
         throw new CloudRuntimeException("StaticRoutesRules not implemented in Basic Network Topology.");
     }
+
+    @Override
+    public boolean visit(final AdvancedVpnRules vpnRules) throws ResourceUnavailableException {
+        throw new CloudRuntimeException("AdvancedVpnRules not implemented in Basic Network Topology.");
+    }
 }

server/src/org/apache/cloudstack/network/topology/NetworkTopology.java

@@ -19,12 +19,12 @@ package org.apache.cloudstack.network.topology;
 
 import java.util.List;
 
-import com.cloud.dc.DataCenter;
 import com.cloud.deploy.DeployDestination;
 import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.exception.ResourceUnavailableException;
 import com.cloud.network.Network;
 import com.cloud.network.PublicIpAddress;
+import com.cloud.network.RemoteAccessVpn;
 import com.cloud.network.VpnUser;
 import com.cloud.network.lb.LoadBalancingRule;
 import com.cloud.network.router.VirtualRouter;
@@ -41,23 +41,9 @@ import com.cloud.vm.VirtualMachineProfile;
 
 public interface NetworkTopology {
 
-<<<<<<< HEAD
-<<<<<<< HEAD
-    StringBuilder createGuestBootLoadArgs(final NicProfile guestNic, final String defaultDns1, final String defaultDns2, DomainRouterVO router);
-
-    String retrieveGuestDhcpRange(final NicProfile guestNic, final Network guestNetwork, final DataCenter dc);
-
-    NicProfile retrieveControlNic(final VirtualMachineProfile profile);
-
-
-    // ====== USER FOR VPC ONLY ====== //
-=======
-=======
     NetworkTopologyVisitor getVisitor();
 
->>>>>>> 06c77f1... refactor all private methods in the virtual router appliance and VPC appliance
     // ====== USED FOR VPC ONLY ====== //
->>>>>>> 0b8b22f... adding setupPrivateGateway to new style;
 
     boolean setupDhcpForPvlan(final boolean add, final DomainRouterVO router, final Long hostId, final NicProfile nic) throws ResourceUnavailableException;
 
@@ -68,17 +54,10 @@ public interface NetworkTopology {
             throws ResourceUnavailableException;
 
     boolean applyStaticRoutes(final List<StaticRouteProfile> staticRoutes, final List<DomainRouterVO> routers) throws ResourceUnavailableException;
-<<<<<<< HEAD
-<<<<<<< HEAD
-=======
 
-=======
-
->>>>>>> 5e11fba... fixing check style issues
     boolean setupPrivateGateway(final PrivateGateway gateway, final VirtualRouter router) throws ConcurrentOperationException, ResourceUnavailableException;
 
     String[] applyVpnUsers(final RemoteAccessVpn vpn, final List<? extends VpnUser> users, final VirtualRouter router) throws ResourceUnavailableException;
->>>>>>> 0b8b22f... adding setupPrivateGateway to new style;
 
     // ====== USED FOR GUEST NETWORK AND VCP ====== //
 

server/src/org/apache/cloudstack/network/topology/NetworkTopologyVisitor.java

@@ -18,7 +18,10 @@
 package org.apache.cloudstack.network.topology;
 
 import com.cloud.exception.ResourceUnavailableException;
-import com.cloud.network.rules.DhcpRules;
+import com.cloud.network.rules.AdvancedVpnRules;
+import com.cloud.network.rules.BasicVpnRules;
+import com.cloud.network.rules.DhcpEntryRules;
+import com.cloud.network.rules.DhcpPvlanRules;
 import com.cloud.network.rules.DhcpSubNetRules;
 import com.cloud.network.rules.FirewallRules;
 import com.cloud.network.rules.IpAssociationRules;
@@ -34,43 +37,27 @@ import com.cloud.network.rules.UserdataPwdRules;
 import com.cloud.network.rules.UserdataToRouterRules;
 import com.cloud.network.rules.VirtualNetworkApplianceFactory;
 import com.cloud.network.rules.VpcIpAssociationRules;
-import com.cloud.network.rules.VpnRules;
 
 public abstract class NetworkTopologyVisitor {
 
     public abstract VirtualNetworkApplianceFactory getVirtualNetworkApplianceFactory();
 
     public abstract boolean visit(StaticNatRules nat) throws ResourceUnavailableException;
-
     public abstract boolean visit(LoadBalancingRules loadbalancing) throws ResourceUnavailableException;
-
     public abstract boolean visit(FirewallRules firewall) throws ResourceUnavailableException;
-
     public abstract boolean visit(IpAssociationRules ipAddresses) throws ResourceUnavailableException;
-
     public abstract boolean visit(UserdataPwdRules userdata) throws ResourceUnavailableException;
-
+    public abstract boolean visit(DhcpEntryRules dhcp) throws ResourceUnavailableException;
-    public abstract boolean visit(DhcpRules dhcp) throws ResourceUnavailableException;
-
     public abstract boolean visit(SshKeyToRouterRules ssh) throws ResourceUnavailableException;
-
     public abstract boolean visit(PasswordToRouterRules pwd) throws ResourceUnavailableException;
-
     public abstract boolean visit(NetworkAclsRules acl) throws ResourceUnavailableException;
-
     public abstract boolean visit(VpcIpAssociationRules vpcIp) throws ResourceUnavailableException;
-
     public abstract boolean visit(UserdataToRouterRules userdata) throws ResourceUnavailableException;
-
+    public abstract boolean visit(BasicVpnRules vpnRules) throws ResourceUnavailableException;
-    public abstract boolean visit(VpnRules userdata) throws ResourceUnavailableException;
+    public abstract boolean visit(AdvancedVpnRules vpnRules) throws ResourceUnavailableException;
-
+    public abstract boolean visit(PrivateGatewayRules pvtGatewayRules) throws ResourceUnavailableException;
-    public abstract boolean visit(PrivateGatewayRules userdata) throws ResourceUnavailableException;
+    public abstract boolean visit(DhcpPvlanRules dhcpRules) throws ResourceUnavailableException;
-
+    public abstract boolean visit(DhcpSubNetRules dhcpRules) throws ResourceUnavailableException;
-    public abstract boolean visit(DhcpPvlanRules vpn) throws ResourceUnavailableException;
-
-    public abstract boolean visit(DhcpSubNetRules subnet) throws ResourceUnavailableException;
-
     public abstract boolean visit(NicPlugInOutRules nicPlugInOutRules) throws ResourceUnavailableException;
-
     public abstract boolean visit(StaticRoutesRules staticRoutesRules) throws ResourceUnavailableException;
 }

server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java

@@ -100,7 +100,8 @@ public class RouterDeploymentDefinition {
     protected boolean isPublicNetwork;
     protected PublicIp sourceNatIp;
 
-    protected RouterDeploymentDefinition(final Network guestNetwork, final DeployDestination dest, final Account owner, final Map<Param, Object> params, final boolean isRedundant) {
+    protected RouterDeploymentDefinition(final Network guestNetwork, final DeployDestination dest,
+            final Account owner, final Map<Param, Object> params, final boolean isRedundant) {
 
         this.guestNetwork = guestNetwork;
         this.dest = dest;
@@ -110,105 +111,98 @@ public class RouterDeploymentDefinition {
     }
 
     public Long getOfferingId() {
-        return offeringId;
+        return this.offeringId;
     }
 
     public Vpc getVpc() {
         return null;
     }
-
     public Network getGuestNetwork() {
         return guestNetwork;
     }
-
     public DeployDestination getDest() {
         return dest;
     }
-
     public Account getOwner() {
         return owner;
     }
-
     public Map<Param, Object> getParams() {
         return params;
     }
-
     public boolean isRedundant() {
         return isRedundant;
     }
-
     public DeploymentPlan getPlan() {
         return plan;
     }
-
     public boolean isVpcRouter() {
         return false;
     }
-
     public Pod getPod() {
         return dest.getPod();
     }
-
     public Long getPodId() {
         return dest.getPod() == null ? null : dest.getPod().getId();
     }
-
     public List<DomainRouterVO> getRouters() {
         return routers;
     }
 
     public VirtualRouterProvider getVirtualProvider() {
-        return vrProvider;
+        return this.vrProvider;
     }
 
     public boolean isBasic() {
-        return dest.getDataCenter().getNetworkType() == NetworkType.Basic;
+        return this.dest.getDataCenter().getNetworkType() == NetworkType.Basic;
     }
 
     public boolean isPublicNetwork() {
-        return isPublicNetwork;
+        return this.isPublicNetwork;
     }
 
     public PublicIp getSourceNatIP() {
-        return sourceNatIp;
+        return this.sourceNatIp;
     }
 
     protected void generateDeploymentPlan() {
-        final long dcId = dest.getDataCenter().getId();
+        final long dcId = this.dest.getDataCenter().getId();
         Long podId = null;
-        if (isBasic()) {
+        if (this.isBasic()) {
-            if (dest.getPod() == null) {
+            if (this.dest.getPod() == null) {
                 throw new CloudRuntimeException("Pod id is expected in deployment destination");
             }
-            podId = dest.getPod().getId();
+            podId = this.dest.getPod().getId();
         }
-        plan = new DataCenterDeployment(dcId, podId, null, null, null, null);
+        this.plan = new DataCenterDeployment(dcId, podId, null, null, null, null);
     }
 
-    public List<DomainRouterVO> deployVirtualRouter() throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException {
+    public List<DomainRouterVO> deployVirtualRouter()
+            throws InsufficientCapacityException,
+            ConcurrentOperationException, ResourceUnavailableException {
 
-        findOrDeployVirtualRouter();
+        this.findOrDeployVirtualRouter();
 
         return nwHelper.startRouters(this);
     }
 
     @DB
-    protected void findOrDeployVirtualRouter() throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
+    protected void findOrDeployVirtualRouter()
+            throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
 
         try {
-            lock();
+            this.lock();
-            checkPreconditions();
+            this.checkPreconditions();
             // dest has pod=null, for Basic Zone findOrDeployVRs for all Pods
             final List<DeployDestination> destinations = findDestinations();
 
             for (final DeployDestination destination : destinations) {
-                dest = destination;
+                this.dest = destination;
-                planDeploymentRouters();
+                this.planDeploymentRouters();
-                generateDeploymentPlan();
+                this.generateDeploymentPlan();
-                executeDeployment();
+                this.executeDeployment();
             }
         } finally {
-            unlock();
+            this.unlock();
         }
     }
 
@@ -217,25 +211,30 @@ public class RouterDeploymentDefinition {
         if (lock == null) {
             throw new ConcurrentOperationException("Unable to lock network " + guestNetwork.getId());
         }
-        tableLockId = lock.getId();
+        this.tableLockId = lock.getId();
     }
 
     protected void unlock() {
-        if (tableLockId != null) {
+        if (this.tableLockId != null) {
-            networkDao.releaseFromLockTable(tableLockId);
+            networkDao.releaseFromLockTable(this.tableLockId);
             if (logger.isDebugEnabled()) {
-                logger.debug("Lock is released for network id " + tableLockId + " as a part of router startup in " + dest);
+                logger.debug("Lock is released for network id " + this.tableLockId
+                        + " as a part of router startup in " + dest);
             }
         }
     }
 
     protected void checkPreconditions() throws ResourceUnavailableException {
-        if (guestNetwork.getState() != Network.State.Implemented && guestNetwork.getState() != Network.State.Setup && guestNetwork.getState() != Network.State.Implementing) {
+        if (guestNetwork.getState() != Network.State.Implemented &&
-            throw new ResourceUnavailableException("Network is not yet fully implemented: " + guestNetwork, Network.class, guestNetwork.getId());
+                guestNetwork.getState() != Network.State.Setup &&
+                guestNetwork.getState() != Network.State.Implementing) {
+            throw new ResourceUnavailableException("Network is not yet fully implemented: " + guestNetwork,
+                    Network.class, this.guestNetwork.getId());
         }
 
         if (guestNetwork.getTrafficType() != TrafficType.Guest) {
-            throw new ResourceUnavailableException("Network is not type Guest as expected: " + guestNetwork, Network.class, guestNetwork.getId());
+            throw new ResourceUnavailableException("Network is not type Guest as expected: " + guestNetwork,
+                    Network.class, this.guestNetwork.getId());
         }
     }
 
@@ -243,11 +242,9 @@ public class RouterDeploymentDefinition {
         // dest has pod=null, for Basic Zone findOrDeployVRs for all Pods
         final List<DeployDestination> destinations = new ArrayList<DeployDestination>();
 
-        // for basic zone, if 'dest' has pod set to null then this is network
+        // for basic zone, if 'dest' has pod set to null then this is network restart scenario otherwise it is a vm deployment scenario
-        // restart scenario otherwise it is a vm deployment scenario
+        if (this.isBasic() && dest.getPod() == null) {
-        if (isBasic() && dest.getPod() == null) {
+            // Find all pods in the data center with running or starting user vms
-            // Find all pods in the data center with running or starting user
-            // vms
             final long dcId = dest.getDataCenter().getId();
             final List<HostPodVO> pods = listByDataCenterIdVMTypeAndStates(dcId, VirtualMachine.Type.User, VirtualMachine.State.Starting, VirtualMachine.State.Running);
 
@@ -258,19 +255,15 @@ public class RouterDeploymentDefinition {
                 final List<DomainRouterVO> virtualRouters = routerDao.listByPodIdAndStates(podId, VirtualMachine.State.Starting, VirtualMachine.State.Running);
 
                 if (virtualRouters.size() > 1) {
-                    // FIXME Find or create a better and more specific exception
+                    // FIXME Find or create a better and more specific exception for this
-                    // for this
                     throw new CloudRuntimeException("Pod can have utmost one VR in Basic Zone, please check!");
                 }
 
-                // Add virtualRouters to the routers, this avoids the situation
+                // Add virtualRouters to the routers, this avoids the situation when
-                // when
+                // all routers are skipped and VirtualRouterElement throws exception
-                // all routers are skipped and VirtualRouterElement throws
+                this.routers.addAll(virtualRouters);
-                // exception
-                routers.addAll(virtualRouters);
 
-                // If List size is one, we already have a starting or running
+                // If List size is one, we already have a starting or running VR, skip deployment
-                // VR, skip deployment
                 if (virtualRouters.size() == 1) {
                     logger.debug("Skipping VR deployment: Found a running or starting VR in Pod " + pod.getName() + " id=" + podId);
                     continue;
@@ -287,43 +280,43 @@ public class RouterDeploymentDefinition {
 
     protected int getNumberOfRoutersToDeploy() {
         // TODO Are we sure this makes sense? Somebody said 5 was too many?
-        if (routers.size() >= 5) {
+        if (this.routers.size() >= 5) {
             logger.error("Too many redundant routers!");
         }
 
-        // If old network is redundant but new is single router, then
+        // If old network is redundant but new is single router, then routers.size() = 2 but routerCount = 1
-        // routers.size() = 2 but routerCount = 1
         int routersExpected = 1;
-        if (isRedundant) {
+        if (this.isRedundant) {
             routersExpected = 2;
         }
-        return routersExpected < routers.size() ? 0 : routersExpected - routers.size();
+        return routersExpected < this.routers.size() ?
+                0 : routersExpected - this.routers.size();
     }
 
     protected void setupAccountOwner() {
         if (networkModel.isNetworkSystem(guestNetwork) || guestNetwork.getGuestType() == Network.GuestType.Shared) {
-            owner = accountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM);
+            this.owner = accountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM);
         }
     }
 
     /**
-     * It executes last pending tasks to prepare the deployment and checks the
+     * It executes last pending tasks to prepare the deployment and checks the deployment
-     * deployment can proceed. If it can't it return false
+     * can proceed. If it can't it return false
      *
      * @return if the deployment can proceed
      */
     protected boolean prepareDeployment() {
-        setupAccountOwner();
+        this.setupAccountOwner();
 
         // Check if public network has to be set on VR
-        isPublicNetwork = networkModel.isProviderSupportServiceInNetwork(guestNetwork.getId(), Service.SourceNat, Provider.VirtualRouter);
+        this.isPublicNetwork = networkModel.isProviderSupportServiceInNetwork(
+                        guestNetwork.getId(), Service.SourceNat, Provider.VirtualRouter);
 
         boolean canProceed = true;
-        if (isRedundant && !isPublicNetwork) {
+        if (this.isRedundant && !this.isPublicNetwork) {
-            // TODO Shouldn't be this throw an exception instead of log error
+            // TODO Shouldn't be this throw an exception instead of log error and empty list of routers
-            // and empty list of routers
             logger.error("Didn't support redundant virtual router without public network!");
-            routers = new ArrayList<>();
+            this.routers = new ArrayList<>();
             canProceed = false;
         }
 
@@ -331,39 +324,39 @@ public class RouterDeploymentDefinition {
     }
 
     /**
-     * Executes preparation and deployment of the routers. After this method
+     * Executes preparation and deployment of the routers. After this method ends, {@link this#routers}
-     * ends, {@link this#routers} should have all of the deployed routers ready
+     * should have all of the deployed routers ready for start, and no more.
-     * for start, and no more.
      *
      * @throws ConcurrentOperationException
      * @throws InsufficientCapacityException
      * @throws ResourceUnavailableException
      */
-    protected void executeDeployment() throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
+    protected void executeDeployment()
+            throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
 
-        // Check current redundant routers, if possible(all routers are
+        //Check current redundant routers, if possible(all routers are stopped), reset the priority
-        // stopped), reset the priority
+        this.setupPriorityOfRedundantRouter();
-        setupPriorityOfRedundantRouter();
 
-        if (getNumberOfRoutersToDeploy() > 0 && prepareDeployment()) {
+        if (this.getNumberOfRoutersToDeploy() > 0 && this.prepareDeployment()) {
-            findVirtualProvider();
+            this.findVirtualProvider();
-            findOfferingId();
+            this.findOfferingId();
-            findSourceNatIP();
+            this.findSourceNatIP();
-            deployAllVirtualRouters();
+            this.deployAllVirtualRouters();
         }
     }
 
     protected void findSourceNatIP() throws InsufficientAddressCapacityException, ConcurrentOperationException {
-        sourceNatIp = null;
+        this.sourceNatIp = null;
-        if (isPublicNetwork) {
+        if (this.isPublicNetwork) {
-            sourceNatIp = ipAddrMgr.assignSourceNatIpAddressToGuestNetwork(owner, guestNetwork);
+            this.sourceNatIp = this.ipAddrMgr.assignSourceNatIpAddressToGuestNetwork(
+                    this.owner,this.guestNetwork);
         }
     }
 
     protected void findOfferingId() {
         Long networkOfferingId = networkOfferingDao.findById(guestNetwork.getNetworkOfferingId()).getServiceOfferingId();
         if (networkOfferingId != null) {
-            offeringId = networkOfferingId;
+            this.offeringId = networkOfferingId;
         }
     }
 
@@ -371,43 +364,41 @@ public class RouterDeploymentDefinition {
         // Check if providers are supported in the physical networks
         final Type type = Type.VirtualRouter;
         final Long physicalNetworkId = networkModel.getPhysicalNetworkId(guestNetwork);
-        final PhysicalNetworkServiceProvider provider = physicalProviderDao.findByServiceProvider(physicalNetworkId, type.toString());
+        final PhysicalNetworkServiceProvider provider =
+                physicalProviderDao.findByServiceProvider(physicalNetworkId, type.toString());
 
         if (provider == null) {
-            throw new CloudRuntimeException(String.format("Cannot find service provider %s  in physical network %s", type.toString(), physicalNetworkId));
+            throw new CloudRuntimeException(
+                    String.format("Cannot find service provider %s  in physical network %s",
+                            type.toString(), physicalNetworkId));
         }
 
-        vrProvider = vrProviderDao.findByNspIdAndType(provider.getId(), type);
+        this.vrProvider = vrProviderDao.findByNspIdAndType(provider.getId(), type);
-        if (vrProvider == null) {
+        if (this.vrProvider == null) {
-            throw new CloudRuntimeException(String.format("Cannot find virtual router provider %s as service provider %s", type.toString(), provider.getId()));
+            throw new CloudRuntimeException(
+                    String.format("Cannot find virtual router provider %s as service provider %s",
+                            type.toString(), provider.getId()));
         }
     }
 
-    protected void deployAllVirtualRouters() throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
+    protected void deployAllVirtualRouters()
+            throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
 
-<<<<<<< HEAD
-        int routersToDeploy = getNumberOfRoutersToDeploy();
-        for (int i = 0; i < routersToDeploy; i++) {
-            // Don't start the router as we are holding the network lock that
-            // needs to be released at the end of router allocation
-            DomainRouterVO router = nwHelper.deployRouter(this, false, null);
-=======
         int routersToDeploy = this.getNumberOfRoutersToDeploy();
         for(int i = 0; i < routersToDeploy; i++) {
             // Don't start the router as we are holding the network lock that needs to be released at the end of router allocation
             DomainRouterVO router = this.nwHelper.deployRouter(this, false);
->>>>>>> 2e8879f... Refactor hypervisor retrieval from VpcNwHelper and NwHelper
 
             if (router != null) {
-                routerDao.addRouterToGuestNetwork(router, guestNetwork);
+                this.routerDao.addRouterToGuestNetwork(router, this.guestNetwork);
-                routers.add(router);
+                this.routers.add(router);
             }
         }
     }
 
     /**
-     * Lists all pods given a Data Center Id, a {@link VirtualMachine.Type} and
+     * Lists all pods given a Data Center Id, a {@link VirtualMachine.Type} and a list of
-     * a list of {@link VirtualMachine.State}
+     * {@link VirtualMachine.State}
      *
      * @param id
      * @param type
@@ -428,27 +419,28 @@ public class RouterDeploymentDefinition {
         final SearchCriteria<HostPodVO> sc = podIdSearch.create();
         sc.setParameters("dc", id);
         sc.setJoinParameters("vmInstanceSearch", "type", type);
-        sc.setJoinParameters("vmInstanceSearch", "states", (Object[]) states);
+        sc.setJoinParameters("vmInstanceSearch", "states", (Object[])states);
         return podDao.search(sc, null);
     }
 
     protected void planDeploymentRouters() {
-        if (isBasic()) {
+        if (this.isBasic()) {
-            routers = routerDao.listByNetworkAndPodAndRole(guestNetwork.getId(), getPodId(), Role.VIRTUAL_ROUTER);
+            this.routers = routerDao.listByNetworkAndPodAndRole(this.guestNetwork.getId(),
+                    this.getPodId(), Role.VIRTUAL_ROUTER);
         } else {
-            routers = routerDao.listByNetworkAndRole(guestNetwork.getId(), Role.VIRTUAL_ROUTER);
+            this.routers = routerDao.listByNetworkAndRole(this.guestNetwork.getId(),
+                    Role.VIRTUAL_ROUTER);
         }
     }
 
     /**
-     * Routers need reset if at least one of the routers is not redundant or
+     * Routers need reset if at least one of the routers is not redundant or stopped.
-     * stopped.
      *
      * @return
      */
     protected boolean routersNeedReset() {
         boolean needReset = true;
-        for (final DomainRouterVO router : routers) {
+        for (final DomainRouterVO router : this.routers) {
             if (!router.getIsRedundantRouter() || router.getState() != VirtualMachine.State.Stopped) {
                 needReset = false;
                 break;
@@ -459,12 +451,12 @@ public class RouterDeploymentDefinition {
     }
 
     /**
-     * Only for redundant deployment and if any routers needed reset, we shall
+     * Only for redundant deployment and if any routers needed reset, we shall reset all
-     * reset all routers priorities
+     * routers priorities
      */
     protected void setupPriorityOfRedundantRouter() {
-        if (isRedundant && routersNeedReset()) {
+            if (this.isRedundant && this.routersNeedReset()) {
-            for (final DomainRouterVO router : routers) {
+                for (final DomainRouterVO router : this.routers) {
                     // getUpdatedPriority() would update the value later
                     router.setPriority(0);
                     router.setIsPriorityBumpUp(false);

server/src/org/cloud/network/router/deployment/RouterDeploymentDefinitionBuilder.java

@@ -23,6 +23,8 @@ import java.util.Map;
 import javax.inject.Inject;
 
 import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Qualifier;
 
 import com.cloud.dc.dao.HostPodDao;
 import com.cloud.dc.dao.VlanDao;
@@ -92,9 +94,11 @@ public class RouterDeploymentDefinitionBuilder {
     @Inject
     private VlanDao vlanDao;
 
-    @Inject
+    @Autowired
+    @Qualifier("networkHelper")
     protected NetworkHelper nwHelper;
-    @Inject
+    @Autowired
+    @Qualifier("vpcNetworkHelper")
     protected VpcNetworkHelperImpl vpcNwHelper;
 
     protected Long offeringId;
@@ -107,7 +111,8 @@ public class RouterDeploymentDefinitionBuilder {
         return new IntermediateStateBuilder(this);
     }
 
-    protected RouterDeploymentDefinition injectDependencies(final RouterDeploymentDefinition routerDeploymentDefinition) {
+    protected RouterDeploymentDefinition injectDependencies(
+            final RouterDeploymentDefinition routerDeploymentDefinition) {
 
         routerDeploymentDefinition.networkDao = networkDao;
         routerDeploymentDefinition.routerDao = routerDao;
@@ -134,7 +139,8 @@ public class RouterDeploymentDefinitionBuilder {
         return routerDeploymentDefinition;
     }
 
-    protected void injectVpcDependencies(final VpcRouterDeploymentDefinition routerDeploymentDefinition) {
+    protected void injectVpcDependencies(
+            final VpcRouterDeploymentDefinition routerDeploymentDefinition) {
 
         routerDeploymentDefinition.vpcDao = vpcDao;
         routerDeploymentDefinition.vpcOffDao = vpcOffDao;
@@ -207,4 +213,5 @@ public class RouterDeploymentDefinitionBuilder {
             return builder.injectDependencies(routerDeploymentDefinition);
         }
     }
+
 }

server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java

@@ -53,7 +53,9 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
 
     protected Vpc vpc;
 
-    protected VpcRouterDeploymentDefinition(final Vpc vpc, final DeployDestination dest, final Account owner, final Map<Param, Object> params, final boolean isRedundant) {
+
+    protected VpcRouterDeploymentDefinition(final Vpc vpc, final DeployDestination dest, final Account owner,
+            final Map<Param, Object> params, final boolean isRedundant) {
 
         super(null, dest, owner, params, isRedundant);
 
@@ -62,7 +64,7 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
 
     @Override
     public Vpc getVpc() {
-        return vpc;
+        return this.vpc;
     }
 
     @Override
@@ -81,15 +83,16 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
         if (vpcLock == null) {
             throw new ConcurrentOperationException("Unable to lock vpc " + vpc.getId());
         }
-        tableLockId = vpcLock.getId();
+        this.tableLockId = vpcLock.getId();
     }
 
     @Override
     protected void unlock() {
-        if (tableLockId != null) {
+        if (this.tableLockId != null) {
-            vpcDao.releaseFromLockTable(tableLockId);
+            vpcDao.releaseFromLockTable(this.tableLockId);
             if (logger.isDebugEnabled()) {
-                logger.debug("Lock is released for vpc id " + tableLockId + " as a part of router startup in " + dest);
+                logger.debug("Lock is released for vpc id " + this.tableLockId
+                        + " as a part of router startup in " + dest);
             }
         }
     }
@@ -102,19 +105,19 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
     @Override
     protected List<DeployDestination> findDestinations() {
         final List<DeployDestination> destinations = new ArrayList<>();
-        destinations.add(dest);
+        destinations.add(this.dest);
         return destinations;
     }
 
     @Override
     protected int getNumberOfRoutersToDeploy() {
-        // TODO Should we make our changes here in order to enable Redundant
+        // TODO Should we make our changes here in order to enable Redundant Router for VPC?
-        // Router for VPC?
+        return this.routers.isEmpty() ? 1 : 0;
-        return routers.isEmpty() ? 1 : 0;
     }
 
     /**
      * @see RouterDeploymentDefinition#prepareDeployment()
+     *
      * @return if the deployment can proceed
      */
     @Override
@@ -125,13 +128,12 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
     @Override
     protected void setupPriorityOfRedundantRouter() {
         // Nothing to do for now
-        // TODO Shouldn't we add this behavior once Redundant Router works for
+        // TODO Shouldn't we add this behavior once Redundant Router works for Vpc too
-        // Vpc too
     }
 
     @Override
     protected void findSourceNatIP() throws InsufficientAddressCapacityException, ConcurrentOperationException {
-        sourceNatIp = vpcMgr.assignSourceNatIpAddressToVpc(owner, vpc);
+        this.sourceNatIp = vpcMgr.assignSourceNatIpAddressToVpc(this.owner, vpc);
     }
 
     @Override
@@ -143,8 +145,8 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
             if (provider == null) {
                 throw new CloudRuntimeException("Cannot find service provider " + Type.VPCVirtualRouter.toString() + " in physical network " + pNtwk.getId());
             }
-            vrProvider = vrProviderDao.findByNspIdAndType(provider.getId(), Type.VPCVirtualRouter);
+            this.vrProvider = vrProviderDao.findByNspIdAndType(provider.getId(), Type.VPCVirtualRouter);
-            if (vrProvider != null) {
+            if (this.vrProvider != null) {
                 break;
             }
         }
@@ -154,28 +156,28 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition {
     protected void findOfferingId() {
         Long vpcOfferingId = vpcOffDao.findById(vpc.getVpcOfferingId()).getServiceOfferingId();
         if (vpcOfferingId != null) {
-            offeringId = vpcOfferingId;
+            this.offeringId = vpcOfferingId;
         }
     }
 
     @Override
-    protected void deployAllVirtualRouters() throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
+    protected void deployAllVirtualRouters()
+            throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException {
 
-        DomainRouterVO router = nwHelper.deployRouter(this, true);
+        DomainRouterVO router = this.nwHelper.deployRouter(this, true);
 
         if (router != null) {
-            routers.add(router);
+            this.routers.add(router);
         }
     }
 
     @Override
     protected void planDeploymentRouters() {
-        routers = routerDao.listByVpcId(vpc.getId());
+        this.routers = this.routerDao.listByVpcId(this.vpc.getId());
     }
 
     @Override
     protected void generateDeploymentPlan() {
-        final long dcId = dest.getDataCenter().getId();
+        this.plan = new DataCenterDeployment(this.dest.getDataCenter().getId());
-        plan = new DataCenterDeployment(dcId);
     }
 }

server/test/com/cloud/network/router/VirtualNetworkApplianceManagerImplTest.java

@@ -1,32 +1,3 @@
-// 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 com.cloud.network.router;
-
-import static org.junit.Assert.assertEquals;
-import static org.mockito.Matchers.any;
-import static org.mockito.Mockito.when;
-
-import java.util.LinkedHashMap;
-import java.util.List;
-
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.InjectMocks;
 package com.cloud.network.router;
 
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
@@ -36,19 +7,10 @@ import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
 
 import com.cloud.cluster.dao.ManagementServerHostDao;
-import com.cloud.dc.DataCenter;
-import com.cloud.dc.DataCenter.NetworkType;
-import com.cloud.dc.DataCenterVO;
 import com.cloud.dc.dao.ClusterDao;
 import com.cloud.dc.dao.DataCenterDao;
 import com.cloud.dc.dao.HostPodDao;
 import com.cloud.dc.dao.VlanDao;
-import com.cloud.deploy.DataCenterDeployment;
-import com.cloud.deploy.DeployDestination;
-import com.cloud.deploy.DeploymentPlan;
-import com.cloud.host.dao.HostDao;
-import com.cloud.hypervisor.Hypervisor.HypervisorType;
-import com.cloud.network.Network;
 import com.cloud.host.dao.HostDao;
 import com.cloud.network.dao.FirewallRulesDao;
 import com.cloud.network.dao.IPAddressDao;
@@ -65,26 +27,6 @@ import com.cloud.network.dao.Site2SiteVpnGatewayDao;
 import com.cloud.network.dao.UserIpv6AddressDao;
 import com.cloud.network.dao.VirtualRouterProviderDao;
 import com.cloud.network.dao.VpnUserDao;
-import com.cloud.network.element.VirtualRouterProviderVO;
-import com.cloud.network.router.VirtualRouter.RedundantState;
-import com.cloud.network.rules.dao.PortForwardingRulesDao;
-import com.cloud.offerings.dao.NetworkOfferingDao;
-import com.cloud.resource.ResourceManager;
-import com.cloud.service.ServiceOfferingVO;
-import com.cloud.service.dao.ServiceOfferingDao;
-import com.cloud.storage.Storage.ProvisioningType;
-import com.cloud.storage.VMTemplateVO;
-import com.cloud.storage.dao.GuestOSDao;
-import com.cloud.storage.dao.VMTemplateDao;
-import com.cloud.storage.dao.VolumeDao;
-import com.cloud.user.AccountManager;
-import com.cloud.user.AccountVO;
-import com.cloud.user.dao.UserDao;
-import com.cloud.user.dao.UserStatisticsDao;
-import com.cloud.user.dao.UserStatsLogDao;
-import com.cloud.vm.DomainRouterVO;
-import com.cloud.vm.NicProfile;
-import com.cloud.vm.VirtualMachine;
 import com.cloud.network.rules.dao.PortForwardingRulesDao;
 import com.cloud.offerings.dao.NetworkOfferingDao;
 import com.cloud.service.dao.ServiceOfferingDao;
@@ -104,7 +46,6 @@ import com.cloud.vm.dao.VMInstanceDao;
 
 @RunWith(MockitoJUnitRunner.class)
 public class VirtualNetworkApplianceManagerImplTest {
-
     @Mock
     private ClusterDao _clusterDao;
 

server/test/com/cloud/vpc/MockVpcVirtualNetworkApplianceManager.java

@@ -35,11 +35,9 @@ import com.cloud.network.Network;
 import com.cloud.network.RemoteAccessVpn;
 import com.cloud.network.Site2SiteVpnConnection;
 import com.cloud.network.VpcVirtualNetworkApplianceService;
-import com.cloud.network.VpnUser;
 import com.cloud.network.router.VirtualRouter;
 import com.cloud.network.router.VpcVirtualNetworkApplianceManager;
 import com.cloud.network.vpc.PrivateGateway;
-import com.cloud.network.vpc.StaticRouteProfile;
 import com.cloud.user.Account;
 import com.cloud.user.User;
 import com.cloud.utils.component.ManagerBase;
@@ -81,7 +79,7 @@ public class MockVpcVirtualNetworkApplianceManager extends ManagerBase implement
      * @see com.cloud.network.router.VirtualNetworkApplianceManager#stop(com.cloud.network.router.VirtualRouter, boolean, com.cloud.user.User, com.cloud.user.Account)
      */
     @Override
-    public VirtualRouter stop(final VirtualRouter router, final boolean forced, final User callingUser, final Account callingAccount) throws ConcurrentOperationException,
+    public VirtualRouter stop(final VirtualRouter router, final boolean forced, final User callingUser, final Account callingAccount) throws ConcurrentOperationException {
         // TODO Auto-generated method stub
         return null;
     }
@@ -104,7 +102,7 @@ public class MockVpcVirtualNetworkApplianceManager extends ManagerBase implement
      * @see com.cloud.network.VirtualNetworkApplianceService#startRouter(long, boolean)
      */
     @Override
-    public VirtualRouter startRouter(final long routerId, final boolean reprogramNetwork) throws ConcurrentOperationException, ResourceUnavailableException,
+    public VirtualRouter startRouter(final long routerId, final boolean reprogramNetwork) throws ConcurrentOperationException, ResourceUnavailableException {
         // TODO Auto-generated method stub
         return null;
     }
@@ -113,7 +111,7 @@ public class MockVpcVirtualNetworkApplianceManager extends ManagerBase implement
      * @see com.cloud.network.VirtualNetworkApplianceService#rebootRouter(long, boolean)
      */
     @Override
-    public VirtualRouter rebootRouter(final long routerId, final boolean reprogramNetwork) throws ConcurrentOperationException, ResourceUnavailableException,
+    public VirtualRouter rebootRouter(final long routerId, final boolean reprogramNetwork) throws ConcurrentOperationException, ResourceUnavailableException {
         // TODO Auto-generated method stub
         return null;
     }
@@ -203,7 +201,7 @@ public class MockVpcVirtualNetworkApplianceManager extends ManagerBase implement
      * @see com.cloud.network.VpcVirtualNetworkApplianceService#removeVpcRouterFromGuestNetwork(com.cloud.network.router.VirtualRouter, com.cloud.network.Network, boolean)
      */
     @Override
-    public boolean removeVpcRouterFromGuestNetwork(final VirtualRouter router, final Network network, final boolean isRedundant) throws ConcurrentOperationException,
+    public boolean removeVpcRouterFromGuestNetwork(final VirtualRouter router, final Network network, final boolean isRedundant) throws ConcurrentOperationException {
         // TODO Auto-generated method stub
         return false;
     }
@@ -273,10 +271,4 @@ public class MockVpcVirtualNetworkApplianceManager extends ManagerBase implement
         // TODO Auto-generated method stub
         return false;
     }
-
-    @Override
-    public String[] applyVpnUsers(final RemoteAccessVpn vpn, final List<? extends VpnUser> users, final VirtualRouter router) throws ResourceUnavailableException {
-        // TODO Auto-generated method stub
-        return null;
-    }
 }