cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From d...@apache.org
Subject [27/50] [abbrv] git commit: updated refs/heads/vpc-refactor to a8cb7c1
Date Fri, 03 Oct 2014 11:12:05 GMT
Factor out prepareDeployment and update tests


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/f0cae57e
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/f0cae57e
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/f0cae57e

Branch: refs/heads/vpc-refactor
Commit: f0cae57e2d697a1e0ad6f35434f36cb41fee6bcb
Parents: eff9b8d
Author: Antonio Fornie <afornie@schubergphilis.com>
Authored: Tue Jul 29 06:47:50 2014 -0500
Committer: Wilder Rodrigues <wrodrigues@schubergphilis.com>
Committed: Fri Oct 3 06:21:27 2014 +0200

----------------------------------------------------------------------
 .../deployment/RouterDeploymentDefinition.java  |  53 +++++----
 .../VpcRouterDeploymentDefinition.java          |  37 +++---
 .../RouterDeploymentDefinitionTest.java         | 118 +++++++++++++------
 .../VpcRouterDeploymentDefinitionTest.java      |   7 +-
 4 files changed, 137 insertions(+), 78 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f0cae57e/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java
----------------------------------------------------------------------
diff --git a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java
b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java
index ea22faf..09192d3 100644
--- a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java
+++ b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java
@@ -302,6 +302,32 @@ public class RouterDeploymentDefinition {
     }
 
     /**
+     * It executes last pending tasks to prepare the deployment and checks the deployment
+     * can proceed. If it can't it return false
+     *
+     * @return if the deployment can proceed
+     */
+    protected boolean prepareDeployment() {
+        boolean canProceed = true;
+        if (networkModel.isNetworkSystem(guestNetwork) || guestNetwork.getGuestType() ==
Network.GuestType.Shared) {
+            this.owner = accountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM);
+        }
+
+        // Check if public network has to be set on VR
+        this.isPublicNetwork = networkModel.isProviderSupportServiceInNetwork(
+                        guestNetwork.getId(), Service.SourceNat, Provider.VirtualRouter);
+
+        if (this.isRedundant && !this.isPublicNetwork) {
+            // TODO Shouldn't be this throw an exception instead of log error and empty list
of routers
+            logger.error("Didn't support redundant virtual router without public network!");
+            this.routers = new ArrayList<>();
+            canProceed = false;
+        }
+
+        return canProceed;
+    }
+
+    /**
      * Executes preparation and deployment of the routers. After this method ends, {@link
this#routers}
      * should have all of the deployed routers ready for start, and no more.
      *
@@ -313,28 +339,13 @@ public class RouterDeploymentDefinition {
             throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException
{
 
         //Check current redundant routers, if possible(all routers are stopped), reset the
priority
-        // TODO Why shouldn't we call this method also for VPC
-        setupPriorityOfRedundantRouter();
+        this.setupPriorityOfRedundantRouter();
 
-        if (this.getNumberOfRoutersToDeploy() > 0) {
-            if (networkModel.isNetworkSystem(guestNetwork) || guestNetwork.getGuestType()
== Network.GuestType.Shared) {
-                this.owner = accountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM);
-            }
-
-            // Check if public network has to be set on VR
-            this.isPublicNetwork = networkModel.isProviderSupportServiceInNetwork(
-                            guestNetwork.getId(), Service.SourceNat, Provider.VirtualRouter);
-
-            if (this.isRedundant && !this.isPublicNetwork) {
-                // TODO Shouldn't be this throw an exception instead of log error and empty
list of routers
-                logger.error("Didn't support redundant virtual router without public network!");
-                this.routers = new ArrayList<>();
-            } else {
-                this.findVirtualProvider();
-                this.findOfferingId();
-                this.findSourceNatIP();
-                this.deployAllVirtualRouters();
-            }
+        if (this.getNumberOfRoutersToDeploy() > 0 && this.prepareDeployment())
{
+            this.findVirtualProvider();
+            this.findOfferingId();
+            this.findSourceNatIP();
+            this.deployAllVirtualRouters();
         }
     }
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f0cae57e/server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java
----------------------------------------------------------------------
diff --git a/server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java
b/server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java
index 3baa4e7..a1e1bc4 100644
--- a/server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java
+++ b/server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java
@@ -31,14 +31,11 @@ import com.cloud.deploy.DeployDestination;
 import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.exception.InsufficientAddressCapacityException;
 import com.cloud.exception.InsufficientCapacityException;
-import com.cloud.exception.InsufficientServerCapacityException;
 import com.cloud.exception.ResourceUnavailableException;
-import com.cloud.exception.StorageUnavailableException;
 import com.cloud.network.Network;
 import com.cloud.network.PhysicalNetwork;
 import com.cloud.network.PhysicalNetworkServiceProvider;
 import com.cloud.network.VirtualRouterProvider.Type;
-import com.cloud.network.addr.PublicIp;
 import com.cloud.network.dao.PhysicalNetworkDao;
 import com.cloud.network.router.VpcVirtualNetworkHelperImpl;
 import com.cloud.network.vpc.Vpc;
@@ -123,21 +120,19 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition
{
     }
 
     /**
-     * @see RouterDeploymentDefinition#executeDeployment()
+     * @see RouterDeploymentDefinition#prepareDeployment()
+     *
+     * @return if the deployment can proceed
      */
     @Override
-    protected void executeDeployment()
-            throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException
{
-        //2) Return routers if exist, otherwise...
-        if (getNumberOfRoutersToDeploy() > 0) {
-            this.findVirtualProvider();
-            this.findOfferingId();
-            this.findSourceNatIP();
-
-            //3) Deploy Virtual Router
-            DomainRouterVO router = deployVpcRouter(sourceNatIp);
-            this.routers.add(router);
-        }
+    protected boolean prepareDeployment() {
+        return true;
+    }
+
+    @Override
+    protected void setupPriorityOfRedundantRouter() {
+        // Nothing to do for now
+        // TODO Shouldn't we add this behavior once Redundant Router works for Vpc too
     }
 
     @Override
@@ -171,16 +166,18 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition
{
         }
     }
 
-    protected DomainRouterVO deployVpcRouter(final PublicIp sourceNatIp)
-            throws ConcurrentOperationException, InsufficientAddressCapacityException,
-            InsufficientServerCapacityException, InsufficientCapacityException, StorageUnavailableException,
ResourceUnavailableException {
+    @Override
+    protected void deployAllVirtualRouters()
+            throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException
{
 
         LinkedHashMap<Network, List<? extends NicProfile>> networks = this.nwHelper.createVpcRouterNetworks(this);
 
         DomainRouterVO router =
                 nwHelper.deployRouter(this, networks, true, vpcMgr.getSupportedVpcHypervisors());
 
-        return router;
+        if (router != null) {
+            this.routers.add(router);
+        }
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f0cae57e/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java
----------------------------------------------------------------------
diff --git a/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java
b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java
index 026996e..92d2546 100644
--- a/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java
+++ b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java
@@ -42,7 +42,6 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
-import org.mockito.Mockito;
 import org.mockito.runners.MockitoJUnitRunner;
 
 import com.cloud.dc.DataCenter.NetworkType;
@@ -391,7 +390,7 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe
         this.mockPods.add(this.mockHostPodVO1);
         this.mockPods.add(this.mockHostPodVO2);
         // Deployment under test is a Mockito spy
-        RouterDeploymentDefinition deploymentUT = Mockito.spy(this.deployment);
+        RouterDeploymentDefinition deploymentUT = spy(this.deployment);
         doReturn(mockPods).when(deploymentUT).listByDataCenterIdVMTypeAndStates(
                 DATA_CENTER_ID, VirtualMachine.Type.User,
                 VirtualMachine.State.Starting, VirtualMachine.State.Running);
@@ -797,7 +796,7 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe
         this.deployment.routers = new ArrayList<>();
         this.deployment.isRedundant = true;
         //this.deployment.routers.add(routerVO1);
-        RouterDeploymentDefinition deploymentUT = Mockito.spy(this.deployment);
+        RouterDeploymentDefinition deploymentUT = spy(this.deployment);
         doReturn(2).when(deploymentUT).getNumberOfRoutersToDeploy();
         doReturn(null).when(this.mockNetworkGeneralHelper).createRouterNetworks(deploymentUT);
 
@@ -818,18 +817,20 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe
                 routerVO2, this.deployment.routers.get(1));
     }
 
+
+
+
+
+
+
+
+
+
+
     @Test
-    public void testExecuteDeploymentPublicNw()
-            throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException
{
+    public void testPrepareDeploymentPublicNw() {
         // Prepare
         this.deployment.isRedundant = true;
-        RouterDeploymentDefinition deploymentUT = Mockito.spy(this.deployment);
-        doNothing().when(deploymentUT).setupPriorityOfRedundantRouter();
-        doReturn(2).when(deploymentUT).getNumberOfRoutersToDeploy();
-        doNothing().when(deploymentUT).findVirtualProvider();
-        doNothing().when(deploymentUT).findOfferingId();
-        doNothing().when(deploymentUT).findSourceNatIP();
-        doNothing().when(deploymentUT).deployAllVirtualRouters();
 
         when(this.mockNetworkModel.isNetworkSystem(this.mockNw)).thenReturn(true);
         Account newAccountOwner = mock(Account.class);
@@ -839,10 +840,72 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe
                             NW_ID, Service.SourceNat, Provider.VirtualRouter)).thenReturn(true);
 
         // Execute
+        boolean canProceedDeployment = this.deployment.prepareDeployment();
+
+        // Assert
+        assertEquals("New account owner not properly set", newAccountOwner, this.deployment.owner);
+    }
+
+    @Test
+    public void testPrepareDeploymentNonRedundant() {
+        // Prepare
+        this.deployment.isRedundant = false;
+
+        when(this.mockNetworkModel.isNetworkSystem(this.mockNw)).thenReturn(true);
+        Account newAccountOwner = mock(Account.class);
+        when(this.mockAccountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM)).thenReturn(newAccountOwner);
+
+        when(this.mockNetworkModel.isProviderSupportServiceInNetwork(
+                            NW_ID, Service.SourceNat, Provider.VirtualRouter)).thenReturn(false);
+
+        // Execute
+        boolean canProceedDeployment = this.deployment.prepareDeployment();
+
+        // Assert
+        assertEquals("New account owner not properly set", newAccountOwner, deployment.owner);
+    }
+
+    @Test
+    public void testPrepareDeploymentRedundantNonPublicNw() {
+        // Prepare
+        this.deployment.isRedundant = false;
+
+        when(this.mockNetworkModel.isNetworkSystem(this.mockNw)).thenReturn(true);
+        Account newAccountOwner = mock(Account.class);
+        when(this.mockAccountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM)).thenReturn(newAccountOwner);
+
+        when(this.mockNetworkModel.isProviderSupportServiceInNetwork(
+                            NW_ID, Service.SourceNat, Provider.VirtualRouter)).thenReturn(false);
+
+        // Execute
+        boolean canProceedDeployment = this.deployment.prepareDeployment();
+
+        // Assert
+        assertEquals("New account owner not properly set", newAccountOwner, this.deployment.owner);
+        assertEquals("Since is redundant deployment in non public nw there should be 0 routers
to start",
+                0, this.deployment.routers.size());
+        verify(this.mockNetworkModel, times(1)).isNetworkSystem(this.mockNw);
+    }
+
+    @Test
+    public void testExecuteDeploymentPublicNw()
+            throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException
{
+        // Prepare
+        this.deployment.isRedundant = true;
+        RouterDeploymentDefinition deploymentUT = spy(this.deployment);
+        doNothing().when(deploymentUT).setupPriorityOfRedundantRouter();
+        doReturn(1).when(deploymentUT).getNumberOfRoutersToDeploy();
+        doReturn(true).when(deploymentUT).prepareDeployment();
+        doNothing().when(deploymentUT).findVirtualProvider();
+        doNothing().when(deploymentUT).findOfferingId();
+        doNothing().when(deploymentUT).findSourceNatIP();
+        doNothing().when(deploymentUT).deployAllVirtualRouters();
+
+        // Execute
         deploymentUT.executeDeployment();
 
         // Assert
-        assertEquals("New account owner not properly set", newAccountOwner, deploymentUT.owner);
+        verify(deploymentUT, times(1)).prepareDeployment();
         verify(deploymentUT, times(1)).findVirtualProvider();
         verify(deploymentUT, times(1)).findOfferingId();
         verify(deploymentUT, times(1)).findSourceNatIP();
@@ -854,26 +917,20 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe
             throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException
{
         // Prepare
         this.deployment.isRedundant = false;
-        RouterDeploymentDefinition deploymentUT = Mockito.spy(this.deployment);
+        RouterDeploymentDefinition deploymentUT = spy(this.deployment);
         doNothing().when(deploymentUT).setupPriorityOfRedundantRouter();
-        doReturn(2).when(deploymentUT).getNumberOfRoutersToDeploy();
+        doReturn(1).when(deploymentUT).getNumberOfRoutersToDeploy();
+        doReturn(true).when(deploymentUT).prepareDeployment();
         doNothing().when(deploymentUT).findVirtualProvider();
         doNothing().when(deploymentUT).findOfferingId();
         doNothing().when(deploymentUT).findSourceNatIP();
         doNothing().when(deploymentUT).deployAllVirtualRouters();
 
-        when(this.mockNetworkModel.isNetworkSystem(this.mockNw)).thenReturn(true);
-        Account newAccountOwner = mock(Account.class);
-        when(this.mockAccountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM)).thenReturn(newAccountOwner);
-
-        when(this.mockNetworkModel.isProviderSupportServiceInNetwork(
-                            NW_ID, Service.SourceNat, Provider.VirtualRouter)).thenReturn(true);
-
         // Execute
         deploymentUT.executeDeployment();
 
         // Assert
-        assertEquals("New account owner not properly set", newAccountOwner, deploymentUT.owner);
+        verify(deploymentUT, times(0)).prepareDeployment();
         verify(deploymentUT, times(1)).findVirtualProvider();
         verify(deploymentUT, times(1)).findOfferingId();
         verify(deploymentUT, times(1)).findSourceNatIP();
@@ -885,7 +942,7 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe
             throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException
{
         // Prepare
         this.deployment.isRedundant = true;
-        RouterDeploymentDefinition deploymentUT = Mockito.spy(this.deployment);
+        RouterDeploymentDefinition deploymentUT = spy(this.deployment);
         doNothing().when(deploymentUT).setupPriorityOfRedundantRouter();
         doReturn(2).when(deploymentUT).getNumberOfRoutersToDeploy();
         doNothing().when(deploymentUT).findVirtualProvider();
@@ -893,21 +950,10 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe
         doNothing().when(deploymentUT).findSourceNatIP();
         doNothing().when(deploymentUT).deployAllVirtualRouters();
 
-        when(this.mockNetworkModel.isNetworkSystem(this.mockNw)).thenReturn(true);
-        Account newAccountOwner = mock(Account.class);
-        when(this.mockAccountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM)).thenReturn(newAccountOwner);
-
-        when(this.mockNetworkModel.isProviderSupportServiceInNetwork(
-                            NW_ID, Service.SourceNat, Provider.VirtualRouter)).thenReturn(false);
-
         // Execute
         deploymentUT.executeDeployment();
 
         // Assert
-        assertEquals("New account owner not properly set", newAccountOwner, deploymentUT.owner);
-        assertEquals("Since is redundant deployment in non public nw there should be 0 routers
to start",
-                0, this.deployment.routers.size());
-        verify(this.mockNetworkModel, times(1)).isNetworkSystem(this.mockNw);
         verify(deploymentUT, times(0)).findVirtualProvider();
         verify(deploymentUT, times(0)).findOfferingId();
         verify(deploymentUT, times(0)).findSourceNatIP();
@@ -920,7 +966,7 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe
         // Prepare
         this.deployment.isRedundant = true;
         this.deployment.isPublicNetwork = false;
-        RouterDeploymentDefinition deploymentUT = Mockito.spy(this.deployment);
+        RouterDeploymentDefinition deploymentUT = spy(this.deployment);
         doNothing().when(deploymentUT).setupPriorityOfRedundantRouter();
         doReturn(0).when(deploymentUT).getNumberOfRoutersToDeploy();
         doNothing().when(deploymentUT).findVirtualProvider();

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f0cae57e/server/test/org/cloud/network/router/deployment/VpcRouterDeploymentDefinitionTest.java
----------------------------------------------------------------------
diff --git a/server/test/org/cloud/network/router/deployment/VpcRouterDeploymentDefinitionTest.java
b/server/test/org/cloud/network/router/deployment/VpcRouterDeploymentDefinitionTest.java
index 829acf7..2260e2c 100644
--- a/server/test/org/cloud/network/router/deployment/VpcRouterDeploymentDefinitionTest.java
+++ b/server/test/org/cloud/network/router/deployment/VpcRouterDeploymentDefinitionTest.java
@@ -64,7 +64,12 @@ public class VpcRouterDeploymentDefinitionTest extends RouterDeploymentDefinitio
 
     @Test
     public void testConstructionFieldsAndFlags() {
-        assertTrue("", this.deployment instanceof VpcRouterDeploymentDefinition);
+        assertTrue("Not really a VpcRouterDeploymentDefinition what the builder created",
+                this.deployment instanceof VpcRouterDeploymentDefinition);
+        assertTrue("A VpcRouterDeploymentDefinition should declare it is",
+                this.deployment.isVpcRouter());
+        assertEquals("A VpcRouterDeploymentDefinition should have a Vpc",
+                this.mockVpc, this.deployment.getVpc());
     }
 
     @Test


Mime
View raw message