we have to check if VPC is null bfore calling it. VPC is not used in …

…gest networks, so deploying a new VM was broken.
asfgit · Oct 14, 2014 · Jul 3, 2014 · Jul 10, 2014 · Jul 11, 2014 · Jul 13, 2014
commit 7e98e3e1327af3d0bcaeae211d41c1270123f891
diff --git a/server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml b/server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
@@ -182,6 +182,7 @@
     <bean id="vpcManagerImpl" class="com.cloud.network.vpc.VpcManagerImpl" >
         <property name="vpcElements" value="#{vpcProvidersRegistry.registered}"></property>
     </bean>
+
     <bean id="vpcVirtualNetworkApplianceManagerImpl"
         class="com.cloud.network.router.VpcVirtualNetworkApplianceManagerImpl" />
 

diff --git a/server/src/com/cloud/network/element/VirtualRouterElement.java b/server/src/com/cloud/network/element/VirtualRouterElement.java
@@ -125,6 +125,7 @@ public class VirtualRouterElement extends AdapterBase implements VirtualRouterEl
     LoadBalancingRulesManager _lbMgr;
     @Inject
     NetworkOfferingDao _networkOfferingDao;
+
     @Inject
     VpcVirtualNetworkApplianceManager _routerMgr;
     @Inject

diff --git a/server/src/com/cloud/network/router/NetworkGeneralHelper.java b/server/src/com/cloud/network/router/NetworkGeneralHelper.java
@@ -147,7 +147,7 @@ public String getRouterIpInNetwork(final long networkId, final long instanceId)
     }
 
 
-//    @Override
+    //    @Override
     public boolean sendCommandsToRouter(final VirtualRouter router, final Commands cmds) throws AgentUnavailableException {
         if(!checkRouterVersion(router)){
             s_logger.debug("Router requires upgrade. Unable to send command to router:" + router.getId() + ", router template version : " + router.getTemplateVersion()
@@ -184,14 +184,14 @@ public boolean sendCommandsToRouter(final VirtualRouter router, final Commands c
     }
 
 
-//    @Override
-    public NicTO getNicTO(final VirtualRouter router, Long networkId, String broadcastUri) {
+    //    @Override
+    public NicTO getNicTO(final VirtualRouter router, final Long networkId, final String broadcastUri) {
         NicProfile nicProfile = _networkModel.getNicProfile(router, networkId, broadcastUri);
 
         return _itMgr.toNicTO(nicProfile, router.getHypervisorType());
     }
 
-//    @Override
+    //    @Override
     public VirtualRouter destroyRouter(final long routerId, final Account caller, final Long callerUserId) throws ResourceUnavailableException, ConcurrentOperationException {
 
         if (s_logger.isDebugEnabled()) {
@@ -216,7 +216,7 @@ public VirtualRouter destroyRouter(final long routerId, final Account caller, fi
      * @param router
      * @return
      */
-//    @Override
+    //    @Override
     public boolean checkRouterVersion(final VirtualRouter router) {
         if(!VirtualNetworkApplianceManagerImpl.routerVersionCheckEnabled.value()){
             //Router version check is disabled.
@@ -252,11 +252,12 @@ protected DomainRouterVO start(DomainRouterVO router, final User user, final Acc
         return _routerDao.findById(router.getId());
     }
 
-    protected DomainRouterVO waitRouter(DomainRouterVO router) {
+    protected DomainRouterVO waitRouter(final DomainRouterVO router) {
         DomainRouterVO vm = _routerDao.findById(router.getId());
 
-        if (s_logger.isDebugEnabled())
+        if (s_logger.isDebugEnabled()) {
             s_logger.debug("Router " + router.getInstanceName() + " is not fully up yet, we will wait");
+        }
         while (vm.getState() == State.Starting) {
             try {
                 Thread.sleep(1000);
@@ -268,8 +269,9 @@ protected DomainRouterVO waitRouter(DomainRouterVO router) {
         }
 
         if (vm.getState() == State.Running) {
-            if (s_logger.isDebugEnabled())
+            if (s_logger.isDebugEnabled()) {
                 s_logger.debug("Router " + router.getInstanceName() + " is now fully up");
+            }
 
             return router;
         }
@@ -279,7 +281,7 @@ protected DomainRouterVO waitRouter(DomainRouterVO router) {
     }
 
 
-//    @Override
+    //    @Override
     public List<DomainRouterVO> startRouters(final Map<Param, Object> params, final List<DomainRouterVO> routers) throws StorageUnavailableException,
     InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException {
         List<DomainRouterVO> runningRouters = null;
@@ -309,13 +311,13 @@ public List<DomainRouterVO> startRouters(final Map<Param, Object> params, final
         return runningRouters;
     }
 
-//    @Override
-    public DomainRouterVO startVirtualRouter(DomainRouterVO router, User user, Account caller, Map<Param, Object> params)
+    //    @Override
+    public DomainRouterVO startVirtualRouter(final DomainRouterVO router, final User user, final Account caller, final Map<Param, Object> params)
             throws StorageUnavailableException, InsufficientCapacityException,
-    ConcurrentOperationException, ResourceUnavailableException {
+            ConcurrentOperationException, ResourceUnavailableException {
 
         if (router.getRole() != Role.VIRTUAL_ROUTER || !router.getIsRedundantRouter()) {
-            return this.start(router, user, caller, params, null);
+            return start(router, user, caller, params, null);
         }
 
         if (router.getState() == State.Running) {
@@ -355,7 +357,7 @@ public DomainRouterVO startVirtualRouter(DomainRouterVO router, User user, Accou
             }
         }
         if (routerToBeAvoid == null) {
-            return this.start(router, user, caller, params, null);
+            return start(router, user, caller, params, null);
         }
         // We would try best to deploy the router to another place
         final int retryIndex = 5;
@@ -380,7 +382,7 @@ public DomainRouterVO startVirtualRouter(DomainRouterVO router, User user, Accou
             }
             plan.setAvoids(avoids[i]);
             try {
-                result = this.start(router, user, caller, params, plan);
+                result = start(router, user, caller, params, plan);
             } catch (final InsufficientServerCapacityException ex) {
                 result = null;
             }
@@ -392,14 +394,14 @@ public DomainRouterVO startVirtualRouter(DomainRouterVO router, User user, Accou
     }
 
 
-//    @Override
+    //    @Override
     public DomainRouterVO deployRouter(final RouterDeploymentDefinition routerDeploymentDefinition,
-            VirtualRouterProvider vrProvider, long svcOffId,
-            LinkedHashMap<Network, List<? extends NicProfile>> networks,
-            boolean startRouter, List<HypervisorType> supportedHypervisors)
-            throws InsufficientAddressCapacityException,
-            InsufficientServerCapacityException, InsufficientCapacityException,
-            StorageUnavailableException, ResourceUnavailableException {
+            final VirtualRouterProvider vrProvider, final long svcOffId,
+            final LinkedHashMap<Network, List<? extends NicProfile>> networks,
+            final boolean startRouter, final List<HypervisorType> supportedHypervisors)
+                    throws InsufficientAddressCapacityException,
+                    InsufficientServerCapacityException, InsufficientCapacityException,
+                    StorageUnavailableException, ResourceUnavailableException {
 
         final ServiceOfferingVO routerOffering = _serviceOfferingDao.findById(svcOffId);
         final DeployDestination dest = routerDeploymentDefinition.getDest();
@@ -422,23 +424,23 @@ public DomainRouterVO deployRouter(final RouterDeploymentDefinition routerDeploy
 
                 String templateName = null;
                 switch (hType) {
-                case XenServer:
-                    templateName = VirtualNetworkApplianceManager.RouterTemplateXen.valueIn(dest.getDataCenter().getId());
-                    break;
-                case KVM:
-                    templateName = VirtualNetworkApplianceManager.RouterTemplateKvm.valueIn(dest.getDataCenter().getId());
-                    break;
-                case VMware:
-                    templateName = VirtualNetworkApplianceManager.RouterTemplateVmware.valueIn(dest.getDataCenter().getId());
-                    break;
-                case Hyperv:
-                    templateName = VirtualNetworkApplianceManager.RouterTemplateHyperV.valueIn(dest.getDataCenter().getId());
-                    break;
-                case LXC:
-                    templateName = VirtualNetworkApplianceManager.RouterTemplateLxc.valueIn(dest.getDataCenter().getId());
-                    break;
-                default:
-                    break;
+                    case XenServer:
+                        templateName = VirtualNetworkApplianceManager.RouterTemplateXen.valueIn(dest.getDataCenter().getId());
+                        break;
+                    case KVM:
+                        templateName = VirtualNetworkApplianceManager.RouterTemplateKvm.valueIn(dest.getDataCenter().getId());
+                        break;
+                    case VMware:
+                        templateName = VirtualNetworkApplianceManager.RouterTemplateVmware.valueIn(dest.getDataCenter().getId());
+                        break;
+                    case Hyperv:
+                        templateName = VirtualNetworkApplianceManager.RouterTemplateHyperV.valueIn(dest.getDataCenter().getId());
+                        break;
+                    case LXC:
+                        templateName = VirtualNetworkApplianceManager.RouterTemplateLxc.valueIn(dest.getDataCenter().getId());
+                        break;
+                    default:
+                        break;
                 }
                 final VMTemplateVO template = _templateDao.findRoutingTemplate(hType, templateName);
 
@@ -453,10 +455,14 @@ public DomainRouterVO deployRouter(final RouterDeploymentDefinition routerDeploy
                     offerHA = false;
                 }
 
+                // routerDeploymentDefinition.getVpc().getId() ==> do not use VPC because it is not a VPC offering.
+                Long vpcId = routerDeploymentDefinition.getVpc() != null ? routerDeploymentDefinition.getVpc().getId() : null;
+
                 router = new DomainRouterVO(id, routerOffering.getId(), vrProvider.getId(),
                         VirtualMachineName.getRouterName(id, VirtualNwStatus.instance), template.getId(), template.getHypervisorType(),
                         template.getGuestOSId(), owner.getDomainId(), owner.getId(), routerDeploymentDefinition.isRedundant(), 0,
-                        false, RedundantState.UNKNOWN, offerHA, false, routerDeploymentDefinition.getVpc().getId());
+                        false, RedundantState.UNKNOWN, offerHA, false, vpcId);
+
                 router.setDynamicallyScalable(template.isDynamicallyScalable());
                 router.setRole(Role.VIRTUAL_ROUTER);
                 router = _routerDao.persist(router);
@@ -565,21 +571,21 @@ protected HypervisorType getClusterToStartDomainRouterForOvm(final long podId) {
 
         final String errMsg =
                 new StringBuilder("Cannot find an available cluster in Pod ")
-                .append(podId)
-                .append(" to start domain router for Ovm. \n Ovm won't support any system vm including domain router, ")
-                .append("please make sure you have a cluster with hypervisor type of any of xenserver/KVM/Vmware in the same pod")
-                .append(" with Ovm cluster. And there is at least one host in UP status in that cluster.")
-                .toString();
+        .append(podId)
+        .append(" to start domain router for Ovm. \n Ovm won't support any system vm including domain router, ")
+        .append("please make sure you have a cluster with hypervisor type of any of xenserver/KVM/Vmware in the same pod")
+        .append(" with Ovm cluster. And there is at least one host in UP status in that cluster.")
+        .toString();
         throw new CloudRuntimeException(errMsg);
     }
 
 
-//    @Override
+    //    @Override
     public LinkedHashMap<Network, List<? extends NicProfile>> createRouterNetworks(
             final RouterDeploymentDefinition routerDeploymentDefinition,
-            Network guestNetwork, Pair<Boolean, PublicIp> publicNetwork)
-            throws ConcurrentOperationException,
-            InsufficientAddressCapacityException {
+            final Network guestNetwork, final Pair<Boolean, PublicIp> publicNetwork)
+                    throws ConcurrentOperationException,
+                    InsufficientAddressCapacityException {
 
         boolean setupPublicNetwork = false;
         if (publicNetwork != null) {

diff --git a/server/src/com/cloud/network/router/RouterDeploymentDefinition.java b/server/src/com/cloud/network/router/RouterDeploymentDefinition.java
@@ -1,3 +1,19 @@
+// 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 java.util.Map;
@@ -19,9 +35,9 @@ public class RouterDeploymentDefinition {
     protected boolean isRedundant;
     protected DeploymentPlan plan;
 
-    public RouterDeploymentDefinition(Vpc vpc, DeployDestination dest,
-            Account owner, Map<Param, Object> params,
-            boolean isRedundant) {
+    public RouterDeploymentDefinition(final Vpc vpc, final DeployDestination dest,
+            final Account owner, final Map<Param, Object> params,
+            final boolean isRedundant) {
 
         this.vpc = vpc;
         this.dest = dest;
@@ -30,8 +46,8 @@ public RouterDeploymentDefinition(Vpc vpc, DeployDestination dest,
         this.isRedundant = isRedundant;
     }
 
-    public RouterDeploymentDefinition(Network guestNetwork, DeployDestination dest,
-            Account owner, Map<Param, Object> params, boolean isRedundant) {
+    public RouterDeploymentDefinition(final Network guestNetwork, final DeployDestination dest,
+            final Account owner, final Map<Param, Object> params, final boolean isRedundant) {
 
         this.guestNetwork = guestNetwork;
         this.dest = dest;
@@ -43,51 +59,50 @@ public RouterDeploymentDefinition(Network guestNetwork, DeployDestination dest,
     public Vpc getVpc() {
         return vpc;
     }
-    public void setVpc(Vpc vpc) {
+    public void setVpc(final Vpc vpc) {
         this.vpc = vpc;
     }
     public Network getGuestNetwork() {
         return guestNetwork;
     }
-    public void setGuestNetwork(Network guestNetwork) {
+    public void setGuestNetwork(final Network guestNetwork) {
         this.guestNetwork = guestNetwork;
     }
     public DeployDestination getDest() {
         return dest;
     }
-    public void setDest(DeployDestination dest) {
+    public void setDest(final DeployDestination dest) {
         this.dest = dest;
     }
     public Account getOwner() {
         return owner;
     }
-    public void setOwner(Account owner) {
+    public void setOwner(final Account owner) {
         this.owner = owner;
     }
     public Map<Param, Object> getParams() {
         return params;
     }
-    public void setParams(Map<Param, Object> params) {
+    public void setParams(final Map<Param, Object> params) {
         this.params = params;
     }
     public boolean isRedundant() {
         return isRedundant;
     }
-    public void setRedundant(boolean isRedundant) {
+    public void setRedundant(final boolean isRedundant) {
         this.isRedundant = isRedundant;
     }
     public DeploymentPlan getPlan() {
         return plan;
     }
-    public void setPlan(DeploymentPlan plan) {
+    public void setPlan(final DeploymentPlan plan) {
         this.plan = plan;
     }
 
     public boolean isVpcRouter() {
-        return this.vpc != null;
+        return vpc != null;
     }
     public Long getPodId() {
-        return this.plan.getPodId();
+        return plan.getPodId();
     }
-
-}
+}
diff --git a/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
@@ -176,7 +176,13 @@ public boolean configure(final String name, final Map<String, Object> params) th
     public List<DomainRouterVO> deployVirtualRouter(final RouterDeploymentDefinition routerDeploymentDefinition)
             throws InsufficientCapacityException,
             ConcurrentOperationException, ResourceUnavailableException {
-        return routerDeploymentManager.deployVirtualRouterInVpc(routerDeploymentDefinition);
+
+        Vpc vpc = routerDeploymentDefinition.getVpc();
+        if (vpc != null) {
+            return routerDeploymentManager.deployVirtualRouterInVpc(routerDeploymentDefinition);
+        } else {
+            return super.deployVirtualRouter(routerDeploymentDefinition);
+        }
     }
 
     @Override
-Original file line number
+Diff line change
@@ Expand Up @@
         LoadBalancingRulesManager _lbMgr;
         @Inject
         NetworkOfferingDao _networkOfferingDao;
         @Inject
         VpcVirtualNetworkApplianceManager _routerMgr;
         @Inject
@@ Expand Down @@