CLOUDSTACK-7605: Fix basic zone multiple pod network restart with cleanup=true
1. getRouters() doesn't handle RestartNetwork with cleanup=true for basic zone, because pod wouldn't be specific at the time. 2. The regression caused by the following fix. The variable "routers" was overrided with some local values, result in only one of the routers in multiple pods would return, thus only one router would be started. commit 6dd5c3fd42c70855d75156243dddc4933436baaf Author: Rohit Yadav <bhais...@apache.org> Date: Thu Oct 11 18:30:00 2012 +0530 CLOUDSTACK-70: Improve restart network behaviour for basic network (cherry picked from commit aaeadc5c44e3fe16a1deea5348b085b08b5f4f4d) Signed-off-by: Rohit Yadav <rohit.ya...@shapeblue.com> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/405ed3ca Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/405ed3ca Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/405ed3ca Branch: refs/heads/4.5 Commit: 405ed3ca50a33adfcb04f960a4420d54619ab4f1 Parents: 40e296d Author: Sheng Yang <sheng.y...@citrix.com> Authored: Mon Sep 22 18:34:36 2014 -0700 Committer: Rohit Yadav <rohit.ya...@shapeblue.com> Committed: Fri Feb 6 16:44:19 2015 +0530 ---------------------------------------------------------------------- .../network/element/VirtualRouterElement.java | 45 ++++++++++---------- .../VirtualNetworkApplianceManagerImpl.java | 12 +++--- 2 files changed, 28 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/405ed3ca/server/src/com/cloud/network/element/VirtualRouterElement.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/element/VirtualRouterElement.java b/server/src/com/cloud/network/element/VirtualRouterElement.java index ae4c933..a01a89d 100755 --- a/server/src/com/cloud/network/element/VirtualRouterElement.java +++ b/server/src/com/cloud/network/element/VirtualRouterElement.java @@ -16,27 +16,6 @@ // under the License. package com.cloud.network.element; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; - -import javax.ejb.Local; -import javax.inject.Inject; - -import com.cloud.utils.net.NetUtils; -import org.apache.log4j.Logger; - -import com.google.gson.Gson; - -import org.apache.cloudstack.api.command.admin.router.ConfigureOvsElementCmd; -import org.apache.cloudstack.api.command.admin.router.ConfigureVirtualRouterElementCmd; -import org.apache.cloudstack.api.command.admin.router.CreateVirtualRouterElementCmd; -import org.apache.cloudstack.api.command.admin.router.ListOvsElementsCmd; -import org.apache.cloudstack.api.command.admin.router.ListVirtualRouterElementsCmd; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; - import com.cloud.agent.api.to.LoadBalancerTO; import com.cloud.configuration.ConfigurationManager; import com.cloud.dc.DataCenter; @@ -94,6 +73,7 @@ import com.cloud.utils.crypt.DBEncryptionUtil; import com.cloud.utils.db.QueryBuilder; import com.cloud.utils.db.SearchCriteria.Op; import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.net.NetUtils; import com.cloud.vm.DomainRouterVO; import com.cloud.vm.NicProfile; import com.cloud.vm.ReservationContext; @@ -104,6 +84,22 @@ import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.VirtualMachineProfile; import com.cloud.vm.dao.DomainRouterDao; import com.cloud.vm.dao.UserVmDao; +import com.google.gson.Gson; +import org.apache.cloudstack.api.command.admin.router.ConfigureOvsElementCmd; +import org.apache.cloudstack.api.command.admin.router.ConfigureVirtualRouterElementCmd; +import org.apache.cloudstack.api.command.admin.router.CreateVirtualRouterElementCmd; +import org.apache.cloudstack.api.command.admin.router.ListOvsElementsCmd; +import org.apache.cloudstack.api.command.admin.router.ListVirtualRouterElementsCmd; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.log4j.Logger; + +import javax.ejb.Local; +import javax.inject.Inject; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; @Local(value = {NetworkElement.class, FirewallServiceProvider.class, DhcpServiceProvider.class, UserDataServiceProvider.class, @@ -968,17 +964,20 @@ NetworkMigrationResponder, AggregatedCommandExecutor { if (publicNetwork) { routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER); } else { - if (isPodBased) { + if (isPodBased && dest.getPod() != null) { Long podId = dest.getPod().getId(); routers = _routerDao.listByNetworkAndPodAndRole(network.getId(), podId, Role.VIRTUAL_ROUTER); } else { + // With pod == null, it's network restart case, we would add all router to it + // Ignore DnsBasicZoneUpdate() parameter here routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER); } } // for Basic zone, add all Running routers - we have to send Dhcp/vmData/password info to them when // network.dns.basiczone.updates is set to "all" - if (isPodBased && _routerMgr.getDnsBasicZoneUpdate().equalsIgnoreCase("all")) { + // With pod == null, it's network restart case, we already add all routers to it + if (isPodBased && dest.getPod() != null && _routerMgr.getDnsBasicZoneUpdate().equalsIgnoreCase("all")) { Long podId = dest.getPod().getId(); List<DomainRouterVO> allRunningRoutersOutsideThePod = _routerDao.findByNetworkOutsideThePod(network.getId(), podId, State.Running, Role.VIRTUAL_ROUTER); routers.addAll(allRunningRoutersOutsideThePod); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/405ed3ca/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index b5d2284..65a4a34 100755 --- a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -1632,7 +1632,7 @@ VirtualMachineGuru, Listener, Configurable, StateListener<State, VirtualMachine. // Except for Basic Zone, the for loop will iterate only once for (final DeployDestination destination : destinations) { final Pair<DeploymentPlan, List<DomainRouterVO>> planAndRouters = getDeploymentPlanAndRouters(isPodBased, destination, guestNetwork.getId()); - routers = planAndRouters.second(); + List<DomainRouterVO> destRouters = planAndRouters.second(); // 2) Figure out required routers count int routerCount = 1; @@ -1640,16 +1640,16 @@ VirtualMachineGuru, Listener, Configurable, StateListener<State, VirtualMachine. routerCount = 2; //Check current redundant routers, if possible(all routers are stopped), reset the priority if (routers.size() != 0) { - checkAndResetPriorityOfRedundantRouter(routers); + checkAndResetPriorityOfRedundantRouter(destRouters); } } // If old network is redundant but new is single router, then routers.size() = 2 but routerCount = 1 - if (routers.size() >= routerCount) { - return routers; + if (destRouters.size() >= routerCount) { + return destRouters; } - if (routers.size() >= 5) { + if (destRouters.size() >= 5) { s_logger.error("Too much redundant routers!"); } @@ -1690,7 +1690,7 @@ VirtualMachineGuru, Listener, Configurable, StateListener<State, VirtualMachine. } // 3) deploy virtual router(s) - final int count = routerCount - routers.size(); + final int count = routerCount - destRouters.size(); final DeploymentPlan plan = planAndRouters.first(); for (int i = 0; i < count; i++) { LinkedHashMap<Network, List<? extends NicProfile>> networks = createRouterNetworks(owner, isRedundant, plan, guestNetwork, new Pair<Boolean, PublicIp>(