Increasing test coverage for Vpc Deployment
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/0d81cf09 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/0d81cf09 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/0d81cf09 Branch: refs/heads/master Commit: 0d81cf09f69b7d7318f0da58e950443bc98ecb0b Parents: 240a539 Author: Antonio Fornie <afor...@schubergphilis.com> Authored: Tue Jul 29 09:20:33 2014 -0500 Committer: wilderrodrigues <wrodrig...@schubergphilis.com> Committed: Tue Oct 14 15:01:16 2014 +0200 ---------------------------------------------------------------------- .../deployment/RouterDeploymentDefinition.java | 12 +- .../VpcRouterDeploymentDefinition.java | 4 +- .../RouterDeploymentDefinitionTest.java | 201 +++++++------------ .../RouterDeploymentDefinitionTestBase.java | 5 +- .../VpcRouterDeploymentDefinitionTest.java | 65 +++++- 5 files changed, 147 insertions(+), 140 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/0d81cf09/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 09192d3..c8742be 100644 --- a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java +++ b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java @@ -301,6 +301,12 @@ public class RouterDeploymentDefinition { 0 : routersExpected - this.routers.size(); } + protected void setupAccountOwner() { + if (networkModel.isNetworkSystem(guestNetwork) || guestNetwork.getGuestType() == Network.GuestType.Shared) { + this.owner = accountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM); + } + } + /** * It executes last pending tasks to prepare the deployment and checks the deployment * can proceed. If it can't it return false @@ -308,15 +314,13 @@ public class RouterDeploymentDefinition { * @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); - } + this.setupAccountOwner(); // Check if public network has to be set on VR this.isPublicNetwork = networkModel.isProviderSupportServiceInNetwork( guestNetwork.getId(), Service.SourceNat, Provider.VirtualRouter); + boolean canProceed = true; 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!"); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/0d81cf09/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 a1e1bc4..8ec05d6 100644 --- a/server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java +++ b/server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java @@ -116,7 +116,7 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition { @Override protected int getNumberOfRoutersToDeploy() { // TODO Should we make our changes here in order to enable Redundant Router for VPC? - return 1 - this.routers.size(); + return this.routers.isEmpty() ? 1 : 0; } /** @@ -144,8 +144,6 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition { protected void findVirtualProvider() { List<? extends PhysicalNetwork> pNtwks = pNtwkDao.listByZone(vpc.getZoneId()); - this.vrProvider = null; - for (PhysicalNetwork pNtwk : pNtwks) { PhysicalNetworkServiceProvider provider = physicalProviderDao.findByServiceProvider(pNtwk.getId(), Type.VPCVirtualRouter.toString()); if (provider == null) { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/0d81cf09/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 92d2546..e76186f 100644 --- a/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java +++ b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java @@ -87,7 +87,7 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe when(this.mockHostPodVO1.getId()).thenReturn(POD_ID1); when(this.mockHostPodVO2.getId()).thenReturn(POD_ID2); when(this.mockHostPodVO3.getId()).thenReturn(POD_ID3); - when(this.mockNw.getId()).thenReturn(NW_ID); + when(this.mockNw.getId()).thenReturn(NW_ID_1); } @Before @@ -149,22 +149,22 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe @Test public void testLock() { // Prepare - when(this.mockNwDao.acquireInLockTable(NW_ID, NetworkOrchestrationService.NetworkLockTimeout.value())) + when(this.mockNwDao.acquireInLockTable(NW_ID_1, NetworkOrchestrationService.NetworkLockTimeout.value())) .thenReturn(mockNw); // Execute this.deployment.lock(); // Assert - verify(this.mockNwDao, times(1)).acquireInLockTable(NW_ID, 600); + verify(this.mockNwDao, times(1)).acquireInLockTable(NW_ID_1, 600); assertNotNull(LOCK_NOT_CORRECTLY_GOT, this.deployment.tableLockId); - assertEquals(LOCK_NOT_CORRECTLY_GOT, NW_ID, NW_ID, this.deployment.tableLockId.longValue()); + assertEquals(LOCK_NOT_CORRECTLY_GOT, NW_ID_1, NW_ID_1, this.deployment.tableLockId.longValue()); } @Test(expected = ConcurrentOperationException.class) public void testLockFails() { // Prepare - when(this.mockNwDao.acquireInLockTable(NW_ID, NetworkOrchestrationService.NetworkLockTimeout.value())) + when(this.mockNwDao.acquireInLockTable(NW_ID_1, NetworkOrchestrationService.NetworkLockTimeout.value())) .thenReturn(null); // Execute @@ -172,7 +172,7 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe this.deployment.lock(); } finally { // Assert - verify(this.mockNwDao, times(1)).acquireInLockTable(NW_ID, 600); + verify(this.mockNwDao, times(1)).acquireInLockTable(NW_ID_1, 600); assertNull(this.deployment.tableLockId); } @@ -181,13 +181,13 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe @Test public void testUnlock() { // Prepare - this.deployment.tableLockId = NW_ID; + this.deployment.tableLockId = NW_ID_1; // Execute this.deployment.unlock(); // Assert - verify(this.mockNwDao, times(1)).releaseFromLockTable(NW_ID); + verify(this.mockNwDao, times(1)).releaseFromLockTable(NW_ID_1); } @Test @@ -817,134 +817,87 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe routerVO2, this.deployment.routers.get(1)); } - - - - - - - - - - @Test - public void testPrepareDeploymentPublicNw() { + public void testSetupAccountOwner() { // Prepare - this.deployment.isRedundant = true; - 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 - boolean canProceedDeployment = this.deployment.prepareDeployment(); - + //Execute + this.deployment.setupAccountOwner(); // Assert assertEquals("New account owner not properly set", newAccountOwner, this.deployment.owner); } @Test - public void testPrepareDeploymentNonRedundant() { + public void testSetupAccountOwnerNotNetworkSystem() { // Prepare - this.deployment.isRedundant = false; - - when(this.mockNetworkModel.isNetworkSystem(this.mockNw)).thenReturn(true); + when(this.mockNetworkModel.isNetworkSystem(this.mockNw)).thenReturn(false); + when(this.mockNw.getGuestType()).thenReturn(Network.GuestType.Shared); 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(); - + //Execute + this.deployment.setupAccountOwner(); // Assert - assertEquals("New account owner not properly set", newAccountOwner, deployment.owner); + assertEquals("New account owner not properly set", newAccountOwner, this.deployment.owner); } @Test - public void testPrepareDeploymentRedundantNonPublicNw() { + public void testSetupAccountOwnerNotSharedNeitherNetworkSystem() { // 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.isNetworkSystem(this.mockNw)).thenReturn(false); + when(this.mockNw.getGuestType()).thenReturn(Network.GuestType.Isolated); + when(this.mockAccountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM)).thenReturn(null); + //Execute + this.deployment.setupAccountOwner(); + // Assert + assertEquals("New account shouldn't have been updated", this.mockOwner, this.deployment.owner); + } - 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 { + protected void driveTestPrepareDeployment(final boolean isRedundant, final boolean isPublicNw) { // 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(); - + this.deployment.isRedundant = isRedundant; + when(this.mockNetworkModel.isProviderSupportServiceInNetwork( + NW_ID_1, Service.SourceNat, Provider.VirtualRouter)).thenReturn(isPublicNw); // Execute - deploymentUT.executeDeployment(); - + final boolean canProceedDeployment = this.deployment.prepareDeployment(); // Assert - verify(deploymentUT, times(1)).prepareDeployment(); - verify(deploymentUT, times(1)).findVirtualProvider(); - verify(deploymentUT, times(1)).findOfferingId(); - verify(deploymentUT, times(1)).findSourceNatIP(); - verify(deploymentUT, times(1)).deployAllVirtualRouters(); + boolean shouldProceedDeployment = true; + if (isRedundant && !isPublicNw) { + shouldProceedDeployment = false; + } + assertEquals(shouldProceedDeployment, canProceedDeployment); + if (!shouldProceedDeployment) { + assertEquals("Since deployment cannot proceed we should empty the list of routers", + 0, this.deployment.routers.size()); + } } @Test - public void testExecuteDeploymentNonRedundant() - throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException { - // Prepare - this.deployment.isRedundant = false; - 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(); + public void testPrepareDeploymentPublicNw() { + this.driveTestPrepareDeployment(true, true); + } - // Assert - verify(deploymentUT, times(0)).prepareDeployment(); - verify(deploymentUT, times(1)).findVirtualProvider(); - verify(deploymentUT, times(1)).findOfferingId(); - verify(deploymentUT, times(1)).findSourceNatIP(); - verify(deploymentUT, times(1)).deployAllVirtualRouters(); + @Test + public void testPrepareDeploymentNonRedundant() { + this.driveTestPrepareDeployment(false, true); } @Test - public void testExecuteDeploymentRedundantNonPublicNw() + public void testPrepareDeploymentRedundantNonPublicNw() { + this.driveTestPrepareDeployment(true, false); + } + + protected void driveTestExecuteDeployment(final int noOfRoutersToDeploy, boolean passPreparation) throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException { // Prepare - this.deployment.isRedundant = true; RouterDeploymentDefinition deploymentUT = spy(this.deployment); doNothing().when(deploymentUT).setupPriorityOfRedundantRouter(); - doReturn(2).when(deploymentUT).getNumberOfRoutersToDeploy(); + doReturn(noOfRoutersToDeploy).when(deploymentUT).getNumberOfRoutersToDeploy(); + doReturn(passPreparation).when(deploymentUT).prepareDeployment(); doNothing().when(deploymentUT).findVirtualProvider(); doNothing().when(deploymentUT).findOfferingId(); doNothing().when(deploymentUT).findSourceNatIP(); @@ -954,41 +907,37 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe deploymentUT.executeDeployment(); // Assert - verify(deploymentUT, times(0)).findVirtualProvider(); - verify(deploymentUT, times(0)).findOfferingId(); - verify(deploymentUT, times(0)).findSourceNatIP(); - verify(deploymentUT, times(0)).deployAllVirtualRouters(); + verify(deploymentUT, times(1)).setupPriorityOfRedundantRouter(); + verify(deploymentUT, times(1)).getNumberOfRoutersToDeploy(); + int proceedToDeployment = 0; + if (noOfRoutersToDeploy > 0) { + verify(deploymentUT, times(1)).prepareDeployment(); + if (passPreparation) { + proceedToDeployment = 1; + } + } + verify(deploymentUT, times(proceedToDeployment)).findVirtualProvider(); + verify(deploymentUT, times(proceedToDeployment)).findOfferingId(); + verify(deploymentUT, times(proceedToDeployment)).findSourceNatIP(); + verify(deploymentUT, times(proceedToDeployment)).deployAllVirtualRouters(); } @Test public void testExecuteDeploymentNoRoutersToDeploy() throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException { - // Prepare - this.deployment.isRedundant = true; - this.deployment.isPublicNetwork = false; - RouterDeploymentDefinition deploymentUT = spy(this.deployment); - doNothing().when(deploymentUT).setupPriorityOfRedundantRouter(); - doReturn(0).when(deploymentUT).getNumberOfRoutersToDeploy(); - doNothing().when(deploymentUT).findVirtualProvider(); - doNothing().when(deploymentUT).findOfferingId(); - doNothing().when(deploymentUT).findSourceNatIP(); - doNothing().when(deploymentUT).deployAllVirtualRouters(); - - // Execute - deploymentUT.executeDeployment(); + this.driveTestExecuteDeployment(0, true); + } - // Assert - assertEquals("New account owner not properly set", this.mockOwner, deploymentUT.owner); - verify(this.mockNetworkModel, times(0)).isNetworkSystem((Network)anyObject()); - verify(deploymentUT, times(0)).findVirtualProvider(); - verify(deploymentUT, times(0)).findOfferingId(); - verify(deploymentUT, times(0)).findSourceNatIP(); - verify(deploymentUT, times(0)).deployAllVirtualRouters(); + @Test + public void testExecuteDeploymentFailPreparation() + throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException { + this.driveTestExecuteDeployment(2, false); } @Test - public void testCreateRouterNetworks() { - // TODO Implement this test + public void testExecuteDeployment() + throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException { + this.driveTestExecuteDeployment(2, true); } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/0d81cf09/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTestBase.java ---------------------------------------------------------------------- diff --git a/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTestBase.java b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTestBase.java index bcfae93..2d9c133 100644 --- a/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTestBase.java +++ b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTestBase.java @@ -57,7 +57,8 @@ public class RouterDeploymentDefinitionTestBase { protected static final long OFFERING_ID = 16L; protected static final Long DATA_CENTER_ID = 100l; - protected static final Long NW_ID = 102l; + protected static final Long NW_ID_1 = 101l; + protected static final Long NW_ID_2= 102l; protected static final Long POD_ID1 = 111l; protected static final Long POD_ID2 = 112l; protected static final Long POD_ID3 = 113l; @@ -125,6 +126,6 @@ public class RouterDeploymentDefinitionTestBase { when(this.mockHostPodVO1.getId()).thenReturn(POD_ID1); when(this.mockHostPodVO2.getId()).thenReturn(POD_ID2); when(this.mockHostPodVO3.getId()).thenReturn(POD_ID3); - when(this.mockNw.getId()).thenReturn(NW_ID); + when(this.mockNw.getId()).thenReturn(NW_ID_1); } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/0d81cf09/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 2260e2c..a1bb204 100644 --- a/server/test/org/cloud/network/router/deployment/VpcRouterDeploymentDefinitionTest.java +++ b/server/test/org/cloud/network/router/deployment/VpcRouterDeploymentDefinitionTest.java @@ -20,24 +20,38 @@ import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertNotNull; import static junit.framework.Assert.assertNull; import static junit.framework.Assert.assertTrue; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.List; + import org.junit.Before; import org.junit.Test; import org.mockito.Mock; +import com.cloud.deploy.DeployDestination; import com.cloud.exception.ConcurrentOperationException; +import com.cloud.network.dao.PhysicalNetworkDao; +import com.cloud.network.dao.PhysicalNetworkServiceProviderDao; import com.cloud.network.vpc.VpcVO; import com.cloud.network.vpc.dao.VpcDao; +import com.cloud.vm.DomainRouterVO; public class VpcRouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTestBase { + private static final String FOR_VPC_ONLY_THE_GIVEN_DESTINATION_SHOULD_BE_USED = "For Vpc only the given destination should be used"; + private static final long VPC_ID = 201L; + private static final long ZONE_ID = 211L; @Mock protected VpcDao mockVpcDao; + @Mock + protected PhysicalNetworkDao mockPhNwDao; + protected PhysicalNetworkServiceProviderDao mockPhProviderDao; @Mock protected VpcVO mockVpc; @@ -48,6 +62,7 @@ public class VpcRouterDeploymentDefinitionTest extends RouterDeploymentDefinitio protected void initMocks() { super.initMocks(); when(this.mockVpc.getId()).thenReturn(VPC_ID); + when(this.mockVpc.getZoneId()).thenReturn(VPC_ID); } @Before @@ -105,21 +120,61 @@ public class VpcRouterDeploymentDefinitionTest extends RouterDeploymentDefinitio @Test public void testUnlock() { - // TODO Implement this test + // Prepare + this.deployment.tableLockId = VPC_ID; + + // Execute + this.deployment.unlock(); + + // Assert + verify(this.mockVpcDao, times(1)).releaseFromLockTable(VPC_ID); } @Test - public void testGenerateDeploymentPlan() { - // TODO Implement this test + public void testUnlockWithoutLock() { + // Prepare + this.deployment.tableLockId = null; + + // Execute + this.deployment.unlock(); + + // Assert + verify(this.mockVpcDao, times(0)).releaseFromLockTable(anyLong()); } @Test - public void testCheckPreconditions() { + public void testFindDestinations() { + // Execute + List<DeployDestination> foundDestinations = this.deployment.findDestinations(); + // Assert + assertEquals(FOR_VPC_ONLY_THE_GIVEN_DESTINATION_SHOULD_BE_USED, + this.deployment.dest, foundDestinations.get(0)); + assertEquals(FOR_VPC_ONLY_THE_GIVEN_DESTINATION_SHOULD_BE_USED, + 1, foundDestinations.size()); + } + + @Test + public void testGetNumberOfRoutersToDeploy() { + assertEquals("If there are no routers, it should deploy one", + 1, this.deployment.getNumberOfRoutersToDeploy()); + this.deployment.routers.add(mock(DomainRouterVO.class)); + assertEquals("If there is already a router found, there is no need to deploy more", + 0, this.deployment.getNumberOfRoutersToDeploy()); + } + + @Test + public void testPrepareDeployment() { + assertTrue("There are no preconditions for Vpc Deployment, thus it should always pass", + this.deployment.prepareDeployment()); + } + + @Test + public void testGenerateDeploymentPlan() { // TODO Implement this test } @Test - public void testFindDestinations() { + public void testCheckPreconditions() { // TODO Implement this test }
