From 55067a8692d460810c8e728cd1d14283b367f6b8 Mon Sep 17 00:00:00 2001 From: Bharat Kumar Date: Mon, 4 Jan 2016 18:36:28 +0530 Subject: [PATCH 1/2] CLOUDSTACK-9726 Update state is not changed to UPDATE_FAILED in case when Host is put in Maintenance Mode. --- .../network/element/RedundantResource.java | 1 + .../service/NetworkOrchestrationService.java | 4 ++- .../orchestration/NetworkOrchestrator.java | 27 +++++++++++++++- .../com/cloud/network/NetworkServiceImpl.java | 17 +++------- .../network/element/VirtualRouterElement.java | 32 +++++++++++++++---- .../com/cloud/vpc/MockNetworkManagerImpl.java | 7 +++- 6 files changed, 65 insertions(+), 23 deletions(-) diff --git a/api/src/com/cloud/network/element/RedundantResource.java b/api/src/com/cloud/network/element/RedundantResource.java index 39b6b97d73a..6ed2d67e7c9 100644 --- a/api/src/com/cloud/network/element/RedundantResource.java +++ b/api/src/com/cloud/network/element/RedundantResource.java @@ -24,4 +24,5 @@ import com.cloud.network.Network; public interface RedundantResource { public void configureResource(Network network); public int getResourceCount(Network network); + public void finalize(Network network, boolean success); } diff --git a/engine/api/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java b/engine/api/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java index 442919e743a..ed573006b24 100644 --- a/engine/api/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java +++ b/engine/api/src/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java @@ -227,7 +227,7 @@ public interface NetworkOrchestrationService { void prepareAllNicsForMigration(VirtualMachineProfile vm, DeployDestination dest); - boolean canUpdateInSequence(Network network); + boolean canUpdateInSequence(Network network, boolean forced); List getServicesNotSupportedInNewOffering(Network network, long newNetworkOfferingId); @@ -236,4 +236,6 @@ public interface NetworkOrchestrationService { void configureUpdateInSequence(Network network); int getResourceCount(Network network); + + void finalizeUpdateInSequence(Network network, boolean success); } diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 49058905100..2e57ff4293e 100644 --- a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -40,6 +40,8 @@ import com.cloud.network.dao.RemoteAccessVpnDao; import com.cloud.network.dao.RemoteAccessVpnVO; import com.cloud.network.dao.VpnUserDao; import com.cloud.network.element.RedundantResource; +import com.cloud.network.router.VirtualRouter; +import com.cloud.vm.DomainRouterVO; import com.cloud.vm.dao.DomainRouterDao; import org.apache.log4j.Logger; import org.apache.cloudstack.acl.ControlledEntity.ACLType; @@ -1298,7 +1300,7 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra } @Override - public boolean canUpdateInSequence(Network network){ + public boolean canUpdateInSequence(Network network, boolean forced){ List providers = getNetworkProviders(network.getId()); //check if the there are no service provider other than virtualrouter. @@ -1306,6 +1308,15 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra if(provider!=Provider.VirtualRouter) throw new UnsupportedOperationException("Cannot update the network resources in sequence when providers other than virtualrouter are used"); } + //check if routers are in correct state before proceeding with the update + List routers=_rotuerDao.listByNetworkAndRole(network.getId(), VirtualRouter.Role.VIRTUAL_ROUTER); + for(DomainRouterVO router :routers){ + if(router.getRedundantState()== VirtualRouter.RedundantState.UNKNOWN){ + if(!forced){ + throw new CloudRuntimeException("Domain router: "+router.getInstanceName()+" is in unknown state, Cannot update network. set parameter forced to true for forcing an update"); + } + } + } return true; } @@ -1442,6 +1453,20 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra return resourceCount; } + @Override + public void finalizeUpdateInSequence(Network network, boolean success) { + List providers = getNetworkProviders(network.getId()); + for (NetworkElement element : networkElements) { + if (providers.contains(element.getProvider())) { + //currently only one element implements the redundant resource interface + if (element instanceof RedundantResource) { + ((RedundantResource) element).finalize(network,success); + break; + } + } + } + } + @DB protected void updateNic(final NicVO nic, final long networkId, final int count) { diff --git a/server/src/com/cloud/network/NetworkServiceImpl.java b/server/src/com/cloud/network/NetworkServiceImpl.java index 73a35058812..a4e61360f8e 100644 --- a/server/src/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/com/cloud/network/NetworkServiceImpl.java @@ -39,7 +39,6 @@ import java.util.UUID; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.network.router.VirtualRouter; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.ApiConstants; @@ -181,7 +180,6 @@ import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.exception.ExceptionUtil; import com.cloud.utils.net.NetUtils; -import com.cloud.vm.DomainRouterVO; import com.cloud.vm.Nic; import com.cloud.vm.NicSecondaryIp; import com.cloud.vm.NicVO; @@ -2252,20 +2250,11 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { int resourceCount=1; if(updateInSequence && restartNetwork && _networkOfferingDao.findById(network.getNetworkOfferingId()).getRedundantRouter() && (networkOfferingId==null || _networkOfferingDao.findById(networkOfferingId).getRedundantRouter()) && network.getVpcId()==null) { - _networkMgr.canUpdateInSequence(network); + _networkMgr.canUpdateInSequence(network, forced); NetworkDetailVO networkDetail =new NetworkDetailVO(network.getId(),Network.updatingInSequence,"true",true); _networkDetailsDao.persist(networkDetail); _networkMgr.configureUpdateInSequence(network); resourceCount=_networkMgr.getResourceCount(network); - //check if routers are in correct state before proceeding with the update - List routers=_routerDao.listByNetworkAndRole(networkId, VirtualRouter.Role.VIRTUAL_ROUTER); - for(DomainRouterVO router :routers){ - if(router.getRedundantState()== VirtualRouter.RedundantState.UNKNOWN){ - if(!forced){ - throw new CloudRuntimeException("Domain router: "+router.getInstanceName()+" is in unknown state, Cannot update network. set parameter forced to true for forcing an update"); - } - } - } } List servicesNotInNewOffering = null; if(networkOfferingId != null) @@ -2413,7 +2402,9 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { resourceCount--; } while(updateInSequence && resourceCount>0); }catch (Exception exception){ - throw new CloudRuntimeException("failed to update network "+network.getUuid()+"due to "+exception.getMessage()); + if(updateInSequence) + _networkMgr.finalizeUpdateInSequence(network,false); + throw new CloudRuntimeException("failed to update network "+network.getUuid()+" due to "+exception.getMessage()); }finally { if(updateInSequence){ if( _networkDetailsDao.findDetail(networkId,Network.updatingInSequence)!=null){ diff --git a/server/src/com/cloud/network/element/VirtualRouterElement.java b/server/src/com/cloud/network/element/VirtualRouterElement.java index 159826b2e5b..4cc44c40478 100644 --- a/server/src/com/cloud/network/element/VirtualRouterElement.java +++ b/server/src/com/cloud/network/element/VirtualRouterElement.java @@ -226,7 +226,13 @@ NetworkMigrationResponder, AggregatedCommandExecutor, RedundantResource, DnsServ routerCounts = 2; } if (routers == null || routers.size() < routerCounts) { - throw new ResourceUnavailableException("Can't find all necessary running routers!", DataCenter.class, network.getDataCenterId()); + //we might have a router which is already deployed and running. + //so check the no of routers in network currently. + List current_routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER); + if (current_routers.size() < 2) { + updateToFailedState(network); + throw new ResourceUnavailableException("Can't find all necessary running routers!", DataCenter.class, network.getDataCenterId()); + } } return true; @@ -724,7 +730,7 @@ NetworkMigrationResponder, AggregatedCommandExecutor, RedundantResource, DnsServ @Override public boolean shutdown(final Network network, final ReservationContext context, final boolean cleanup) throws ConcurrentOperationException, ResourceUnavailableException { - final List routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER); + final List routers = getRouters(network); if (routers == null || routers.isEmpty()) { return true; } @@ -1339,11 +1345,7 @@ NetworkMigrationResponder, AggregatedCommandExecutor, RedundantResource, DnsServ } finally { if(!result && updateInSequence) { //fail the network update. even if one router fails we fail the network update. - List routerList = _routerDao.listByNetworkAndRole(network.getId(), VirtualRouter.Role.VIRTUAL_ROUTER); - for (DomainRouterVO router : routerList) { - router.setUpdateState(VirtualRouter.UpdateState.UPDATE_FAILED); - _routerDao.persist(router); - } + updateToFailedState(network); } } return result; @@ -1373,4 +1375,20 @@ NetworkMigrationResponder, AggregatedCommandExecutor, RedundantResource, DnsServ return _routerDao.listByNetworkAndRole(network.getId(), VirtualRouter.Role.VIRTUAL_ROUTER).size(); } + @Override + public void finalize(Network network, boolean success) { + if(!success){ + updateToFailedState(network); + } + } + + private void updateToFailedState(Network network){ + //fail the network update. even if one router fails we fail the network update. + List routerList = _routerDao.listByNetworkAndRole(network.getId(), VirtualRouter.Role.VIRTUAL_ROUTER); + for (DomainRouterVO router : routerList) { + router.setUpdateState(VirtualRouter.UpdateState.UPDATE_FAILED); + _routerDao.persist(router); + } + } + } diff --git a/server/test/com/cloud/vpc/MockNetworkManagerImpl.java b/server/test/com/cloud/vpc/MockNetworkManagerImpl.java index 461f84261fc..eabb3bcfef2 100644 --- a/server/test/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/test/com/cloud/vpc/MockNetworkManagerImpl.java @@ -848,7 +848,7 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkOrches } @Override - public boolean canUpdateInSequence(Network network) { + public boolean canUpdateInSequence(Network network, boolean forced) { return false; } @@ -872,6 +872,11 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkOrches return 0; } + @Override + public void finalizeUpdateInSequence(Network network, boolean success) { + return; + } + @Override public void prepareNicForMigration(VirtualMachineProfile vm, DeployDestination dest) { // TODO Auto-generated method stub From 422787e2d9bcff241df932a5b316b1690831bcf1 Mon Sep 17 00:00:00 2001 From: Bharat Kumar Date: Mon, 27 Mar 2017 05:53:58 -0700 Subject: [PATCH 2/2] added some logging and made an improvement to get_master_and_backupRouter method --- test/integration/component/maint/test_redundant_router.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/integration/component/maint/test_redundant_router.py b/test/integration/component/maint/test_redundant_router.py index cad1d341d55..94ddae255f0 100644 --- a/test/integration/component/maint/test_redundant_router.py +++ b/test/integration/component/maint/test_redundant_router.py @@ -146,6 +146,7 @@ class TestCreateRvRNetwork(cloudstackTestCase): cls.zone.id, cls.testdata["ostype"] ) + cls.testdata["small"]["zoneid"] = cls.zone.id cls.testdata["small"]["template"] = cls.template.id @@ -1546,6 +1547,8 @@ class TestRvRRedundancy(cloudstackTestCase): listall=True ) retry = retry-1 + if len(routers) < 2: + continue if not (routers[0].redundantstate == 'MASTER' or routers[1].redundantstate == 'MASTER'): continue; if routers[0].redundantstate == 'MASTER': @@ -1556,6 +1559,7 @@ class TestRvRRedundancy(cloudstackTestCase): master_router = routers[1] backup_router = routers[0] break + self.info("master_router: %s, backup_router: %s" % (master_router, backup_router)) return master_router, backup_router