Merge pull request #1971 from bvbharatk/CLOUDSTACK-9726

CLOUDSTACK-9726 Update state is not changed to UPDATE_FAILED in case …

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);
 }

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<String> 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);
 }

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<Provider> 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<DomainRouterVO> 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<Provider> 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) {

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;
@@ -2256,20 +2254,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<DomainRouterVO> 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<String > servicesNotInNewOffering = null;
         if(networkOfferingId != null)
@@ -2417,7 +2406,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){

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<DomainRouterVO> 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<DomainRouterVO> routers = _routerDao.listByNetworkAndRole(network.getId(), Role.VIRTUAL_ROUTER);
+        final List<DomainRouterVO> 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<DomainRouterVO> routerList = _routerDao.listByNetworkAndRole(network.getId(), VirtualRouter.Role.VIRTUAL_ROUTER);
+                updateToFailedState(network);
-                for (DomainRouterVO router : routerList) {
-                    router.setUpdateState(VirtualRouter.UpdateState.UPDATE_FAILED);
-                    _routerDao.persist(router);
-                }
             }
         }
         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<DomainRouterVO> routerList = _routerDao.listByNetworkAndRole(network.getId(), VirtualRouter.Role.VIRTUAL_ROUTER);
+        for (DomainRouterVO router : routerList) {
+            router.setUpdateState(VirtualRouter.UpdateState.UPDATE_FAILED);
+            _routerDao.persist(router);
+        }
+    }
+
 }

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

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