RouterDeployment and VPC. Improving code and Unit Testing
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/3f021625 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/3f021625 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/3f021625 Branch: refs/heads/vpc-refactor Commit: 3f021625a2c4b6626e6e79ecc870d343bd7186ee Parents: 7064415 Author: Antonio Fornie <afor...@schubergphilis.com> Authored: Wed Jul 23 07:48:07 2014 -0500 Committer: Wilder Rodrigues <wrodrig...@schubergphilis.com> Committed: Fri Oct 3 06:21:26 2014 +0200 ---------------------------------------------------------------------- .../deployment/RouterDeploymentDefinition.java | 33 +- .../RouterDeploymentDefinitionBuilder.java | 2 +- .../VpcRouterDeploymentDefinition.java | 62 +--- .../RouterDeploymentDefinitionTest.java | 363 +++++++++++++++++++ .../VpcRouterDeploymentDefinitionTest.java | 52 +++ 5 files changed, 427 insertions(+), 85 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3f021625/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 01fc774..a5e3ec7 100644 --- a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java +++ b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java @@ -129,7 +129,7 @@ public class RouterDeploymentDefinition { this.isRedundant = isRedundant; } - public void setOffering(ServiceOfferingVO offering) { + protected void setOffering(ServiceOfferingVO offering) { this.offering = offering; } public Vpc getVpc() { @@ -138,37 +138,21 @@ public class RouterDeploymentDefinition { public Network getGuestNetwork() { return guestNetwork; } - public void setGuestNetwork(final Network guestNetwork) { - this.guestNetwork = guestNetwork; - } public DeployDestination getDest() { return dest; } - public void setDest(final DeployDestination dest) { - this.dest = dest; - } public Account getOwner() { return owner; } - public void setOwner(final Account owner) { - this.owner = owner; - } public Map<Param, Object> getParams() { return params; } - public void setParams(final Map<Param, Object> params) { - this.params = params; - } public boolean isRedundant() { return isRedundant; } - public void setRedundant(final boolean isRedundant) { - this.isRedundant = isRedundant; - } public DeploymentPlan getPlan() { return plan; } - public boolean isVpcRouter() { return false; } @@ -178,13 +162,9 @@ public class RouterDeploymentDefinition { public Long getPodId() { return dest.getPod() == null ? null : dest.getPod().getId(); } - public List<DomainRouterVO> getRouters() { return routers; } - public void setRouters(List<DomainRouterVO> routers) { - this.routers = routers; - } public boolean isBasic() { return this.dest.getDataCenter().getNetworkType() == NetworkType.Basic; @@ -194,7 +174,7 @@ public class RouterDeploymentDefinition { return offering == null ? null : offering.getId(); } - public void generateDeploymentPlan() { + protected void generateDeploymentPlan() { final long dcId = this.dest.getDataCenter().getId(); Long podId = null; if (this.isBasic()) { @@ -229,7 +209,7 @@ public class RouterDeploymentDefinition { this.dest = destination; this.planDeploymentRouters(); this.generateDeploymentPlan(); - this.proceedEffectiveDeployment(); + this.executeDeployment(); } } finally { this.unlock(); @@ -284,7 +264,10 @@ public class RouterDeploymentDefinition { final long podId = pod.getId(); final List<DomainRouterVO> virtualRouters = routerDao.listByPodIdAndStates(podId, VirtualMachine.State.Starting, VirtualMachine.State.Running); - assert (virtualRouters.size() <= 1) : "Pod can have utmost one VR in Basic Zone, please check!"; + if (virtualRouters.size() > 1) { + // FIXME Find or create a better and more specific exception for this + throw new CloudRuntimeException("Pod can have utmost one VR in Basic Zone, please check!"); + } // Add virtualRouters to the routers, this avoids the situation when // all routers are skipped and VirtualRouterElement throws exception @@ -305,7 +288,7 @@ public class RouterDeploymentDefinition { return destinations; } - protected void proceedEffectiveDeployment() + protected void executeDeployment() throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException { // 2) Figure out required routers count int routerCount = 1; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3f021625/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinitionBuilder.java ---------------------------------------------------------------------- diff --git a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinitionBuilder.java b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinitionBuilder.java index 6e2b29b..a5662b5 100644 --- a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinitionBuilder.java +++ b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinitionBuilder.java @@ -37,7 +37,7 @@ import com.cloud.vm.dao.VMInstanceDao; public class RouterDeploymentDefinitionBuilder { @Inject - private NetworkDao networkDao; + protected NetworkDao networkDao; @Inject private DomainRouterDao routerDao = null; @Inject http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3f021625/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 ef2367f..f6a99a1 100644 --- a/server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java +++ b/server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java @@ -38,7 +38,6 @@ import com.cloud.network.vpc.dao.VpcOfferingDao; import com.cloud.offering.NetworkOffering; import com.cloud.user.Account; import com.cloud.utils.Pair; -import com.cloud.utils.db.DB; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.DomainRouterVO; import com.cloud.vm.NicProfile; @@ -107,7 +106,7 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition { } @Override - protected void proceedEffectiveDeployment() + protected void executeDeployment() throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException { //2) Return routers if exist, otherwise... if (this.routers.size() < 1) { @@ -138,61 +137,6 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition { } } - @Override - @DB - protected void findOrDeployVirtualRouterOLD() - throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException { - - logger.debug("Deploying Virtual Router in VPC " + vpc); - - Vpc vpcLock = vpcDao.acquireInLockTable(vpc.getId()); - if (vpcLock == null) { - throw new ConcurrentOperationException("Unable to lock vpc " + vpc.getId()); - } - - //1) Find out the list of routers and generate deployment plan - this.planDeploymentRouters(); - this.generateDeploymentPlan(); - - //2) Return routers if exist, otherwise... - if (this.routers.size() < 1) { - try { - - Long offeringId = vpcOffDao.findById(vpc.getVpcOfferingId()).getServiceOfferingId(); - if (offeringId == null) { - offeringId = offering.getId(); - } - //3) Deploy Virtual Router - List<? extends PhysicalNetwork> pNtwks = pNtwkDao.listByZone(vpc.getZoneId()); - - VirtualRouterProvider vpcVrProvider = null; - - for (PhysicalNetwork pNtwk : pNtwks) { - PhysicalNetworkServiceProvider provider = physicalProviderDao.findByServiceProvider(pNtwk.getId(), Type.VPCVirtualRouter.toString()); - if (provider == null) { - throw new CloudRuntimeException("Cannot find service provider " + Type.VPCVirtualRouter.toString() + " in physical network " + pNtwk.getId()); - } - vpcVrProvider = vrProviderDao.findByNspIdAndType(provider.getId(), Type.VPCVirtualRouter); - if (vpcVrProvider != null) { - break; - } - } - - PublicIp sourceNatIp = vpcMgr.assignSourceNatIpAddressToVpc(this.owner, vpc); - - DomainRouterVO router = deployVpcRouter(vpcVrProvider, offeringId, sourceNatIp); - this.routers.add(router); - - } finally { - // TODO Should we do this after the pre or after the whole?? - if (vpcLock != null) { - vpcDao.releaseFromLockTable(vpc.getId()); - } - } - } - } - - protected DomainRouterVO deployVpcRouter(final VirtualRouterProvider vrProvider, final long svcOffId, final PublicIp sourceNatIp) throws ConcurrentOperationException, InsufficientAddressCapacityException, InsufficientServerCapacityException, InsufficientCapacityException, StorageUnavailableException, ResourceUnavailableException { @@ -282,11 +226,11 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition { @Override protected void planDeploymentRouters() { - this.routers = vpcHelper.getVpcRouters(this.getVpc().getId()); + this.routers = vpcHelper.getVpcRouters(this.vpc.getId()); } @Override - public void generateDeploymentPlan() { + protected void generateDeploymentPlan() { final long dcId = this.dest.getDataCenter().getId(); this.plan = new DataCenterDeployment(dcId); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3f021625/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 new file mode 100644 index 0000000..6941e9a --- /dev/null +++ b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java @@ -0,0 +1,363 @@ +package org.cloud.network.router.deployment; + +import static junit.framework.Assert.assertEquals; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Mockito.doReturn; +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.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.runners.MockitoJUnitRunner; + +import com.cloud.dc.DataCenter; +import com.cloud.dc.DataCenter.NetworkType; +import com.cloud.dc.HostPodVO; +import com.cloud.dc.Pod; +import com.cloud.deploy.DeployDestination; +import com.cloud.exception.ConcurrentOperationException; +import com.cloud.exception.ResourceUnavailableException; +import com.cloud.network.Network; +import com.cloud.network.Networks.TrafficType; +import com.cloud.network.dao.NetworkDao; +import com.cloud.network.dao.NetworkVO; +import com.cloud.user.Account; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.DomainRouterVO; +import com.cloud.vm.VirtualMachine; +import com.cloud.vm.VirtualMachineProfile.Param; +import com.cloud.vm.dao.DomainRouterDao; + +@RunWith(MockitoJUnitRunner.class) +public class RouterDeploymentDefinitionTest { + + private static final String ONLY_THE_PROVIDED_AS_DEFAULT_DESTINATION_WAS_EXPECTED = "Only the provided as default destination was expected"; + protected static final Long DATA_CENTER_ID = 100l; + protected static final Long POD_ID1 = 111l; + protected static final Long POD_ID2 = 112l; + protected static final Long POD_ID3 = 113l; + protected static final Long NW_ID = 102l; + + // General delegates (Daos, Mgrs...) + @Mock + protected NetworkDao mockNwDao; + + // Instance specific parameters to use during build + @Mock + protected DeployDestination mockDestination; + @Mock + protected DataCenter mockDataCenter; + @Mock + protected Pod mockPod; + @Mock + protected HostPodVO mockHostPodVO1; + @Mock + protected HostPodVO mockHostPodVO2; + @Mock + protected HostPodVO mockHostPodVO3; + @Mock + protected NetworkVO mockNwLock; + @Mock + protected Account mockOwner; + @Mock + protected DomainRouterDao mockRouterDao; + + protected List<HostPodVO> mockPods = new ArrayList<>(); + protected Map<Param, Object> params = new HashMap<>(); + + @InjectMocks + protected RouterDeploymentDefinitionBuilder builder = new RouterDeploymentDefinitionBuilder(); + + protected RouterDeploymentDefinition deployment; + + + @Before + public void initTest() { + when(this.mockDestination.getDataCenter()).thenReturn(this.mockDataCenter); + when(this.mockDataCenter.getId()).thenReturn(DATA_CENTER_ID); + when(this.mockPod.getId()).thenReturn(POD_ID1); + when(this.mockHostPodVO1.getId()).thenReturn(POD_ID1); + when(this.mockHostPodVO2.getId()).thenReturn(POD_ID2); + when(this.mockHostPodVO3.getId()).thenReturn(POD_ID3); + when(this.mockNwLock.getId()).thenReturn(NW_ID); + + this.deployment = this.builder.create() + .setGuestNetwork(this.mockNwLock) + .setDeployDestination(this.mockDestination) + .build(); + } + + @Test + public void testLock() { + // Prepare + when(this.mockNwDao.acquireInLockTable(NW_ID, NetworkOrchestrationService.NetworkLockTimeout.value())) + .thenReturn(mockNwLock); + + // Execute + this.deployment.lock(); + + // Assert + verify(this.mockNwDao, times(1)).acquireInLockTable(NW_ID, 600); + Assert.assertNotNull(this.deployment.tableLockId); + } + + @Test(expected = ConcurrentOperationException.class) + public void testLockFails() { + // Prepare + when(this.mockNwDao.acquireInLockTable(NW_ID, NetworkOrchestrationService.NetworkLockTimeout.value())) + .thenReturn(null); + + // Execute + this.deployment.lock(); + + // Assert + verify(this.mockNwDao, times(1)).acquireInLockTable(NW_ID, 600); + Assert.assertNotNull(this.deployment.tableLockId); + } + + @Test + public void testUnlock() { + // Prepare + this.deployment.tableLockId = NW_ID; + + // Execute + this.deployment.unlock(); + + // Assert + verify(this.mockNwDao, times(1)).releaseFromLockTable(NW_ID); + } + + @Test + public void testUnlockWithoutLock() { + // Prepare + this.deployment.tableLockId = null; + + // Execute + this.deployment.unlock(); + + // Assert + verify(this.mockNwDao, times(0)).releaseFromLockTable(anyLong()); + } + + /** + * If it's not a basic network, pod is not needed in the generated DataCenterDeployment + */ + @Test + public void testGenerateDeploymentPlanNoPodNeeded() { + // Prepare + when(mockDataCenter.getNetworkType()).thenReturn(NetworkType.Advanced); + + // Execute + this.deployment.generateDeploymentPlan(); + + // Assert + assertEquals("", DATA_CENTER_ID, (Long) this.deployment.plan.getDataCenterId()); + assertEquals("", mockDestination, this.deployment.dest); + assertEquals("", null, this.deployment.getPod()); + assertEquals("", null, this.deployment.getPodId()); + } + + /** + * If it's Basic, it should have pod + */ + @Test + public void testGenerateDeploymentPlanBasic() { + // Prepare + when(this.mockDestination.getPod()).thenReturn(mockPod); + when(mockDataCenter.getNetworkType()).thenReturn(NetworkType.Basic); + + // Execute + this.deployment.generateDeploymentPlan(); + + // Assert + assertEquals("", DATA_CENTER_ID, (Long) this.deployment.plan.getDataCenterId()); + assertEquals("", mockDestination, this.deployment.dest); + assertEquals("", mockPod, this.deployment.getPod()); + assertEquals("", POD_ID1, this.deployment.getPodId()); + } + + /** + * If it's Basic, it should have pod, otherwise fail with + * {@link CloudRuntimeException} + */ + @Test(expected = CloudRuntimeException.class) + public void testGenerateDeploymentPlanBasicFailNoPod() { + // Prepare + when(this.mockDestination.getPod()).thenReturn(null); + when(mockDataCenter.getNetworkType()).thenReturn(NetworkType.Basic); + + // Execute + this.deployment.generateDeploymentPlan(); + + // Assert + assertEquals("", DATA_CENTER_ID, (Long) this.deployment.plan.getDataCenterId()); + assertEquals("", mockDestination, this.deployment.dest); + } + + @Test + public void testCheckPreconditions() throws ResourceUnavailableException { + // Prepare + Network.State states[] = { + Network.State.Implemented, + Network.State.Setup, + Network.State.Implementing + }; + when(this.deployment.guestNetwork.getTrafficType()).thenReturn(TrafficType.Guest); + + // Drive specific tests + for (Network.State state : states) { + this.driveTestCheckPreconditionsCorrectNwState(state); + } + } + + public void driveTestCheckPreconditionsCorrectNwState(Network.State state) throws ResourceUnavailableException { + // Prepare + when(this.deployment.guestNetwork.getState()).thenReturn(state); + + // Execute + this.deployment.checkPreconditions(); + + // Assert : It just should raise no exceptions + } + + @Test(expected = ResourceUnavailableException.class) + public void testCheckPreconditionsWrongTrafficType() throws ResourceUnavailableException { + // Prepare wrong traffic type to trigger error + when(this.deployment.guestNetwork.getTrafficType()).thenReturn(TrafficType.Public); + + // Execute + this.driveTestCheckPreconditionsCorrectNwState(Network.State.Implemented); + } + + @Test(expected = ResourceUnavailableException.class) + public void testCheckPreconditionsWrongState() throws ResourceUnavailableException { + // Prepare wrong traffic type to trigger error + when(this.deployment.guestNetwork.getTrafficType()).thenReturn(TrafficType.Guest); + + // Execute + this.driveTestCheckPreconditionsCorrectNwState(Network.State.Shutdown); + } + + @Test + public void testFindDestinationsNonBasicZone() { + // Prepare + when(this.mockDataCenter.getNetworkType()).thenReturn(NetworkType.Advanced); + + // Execute + List<DeployDestination> destinations = this.deployment.findDestinations(); + + // Assert + Assert.assertEquals(ONLY_THE_PROVIDED_AS_DEFAULT_DESTINATION_WAS_EXPECTED, + 1, destinations.size()); + Assert.assertEquals(ONLY_THE_PROVIDED_AS_DEFAULT_DESTINATION_WAS_EXPECTED, + this.mockDestination, destinations.get(0)); + } + + @Test + public void testFindDestinationsPredefinedPod() { + // Prepare + when(this.mockDataCenter.getNetworkType()).thenReturn(NetworkType.Basic); + when(this.mockDestination.getPod()).thenReturn(this.mockPod); + + // Execute + List<DeployDestination> destinations = this.deployment.findDestinations(); + + // Assert + Assert.assertEquals(ONLY_THE_PROVIDED_AS_DEFAULT_DESTINATION_WAS_EXPECTED, + 1, destinations.size()); + Assert.assertEquals(ONLY_THE_PROVIDED_AS_DEFAULT_DESTINATION_WAS_EXPECTED, + this.mockDestination, destinations.get(0)); + } + + @Test + public void testFindDestinations() { + // Prepare + when(this.mockDataCenter.getNetworkType()).thenReturn(NetworkType.Basic); + when(this.mockDestination.getPod()).thenReturn(null); + + // Stub local method listByDataCenterIdVMTypeAndStates + this.mockPods.add(this.mockHostPodVO1); + this.mockPods.add(this.mockHostPodVO2); + this.mockPods.add(this.mockHostPodVO3); + RouterDeploymentDefinition deployment = Mockito.spy(this.deployment); + doReturn(mockPods).when(deployment).listByDataCenterIdVMTypeAndStates( + DATA_CENTER_ID, VirtualMachine.Type.User, + VirtualMachine.State.Starting, VirtualMachine.State.Running); + + // Leave this one empty to force adding add destination for this pod + List<DomainRouterVO> virtualRouters1 = new ArrayList<>(); + when(this.mockRouterDao.listByPodIdAndStates(POD_ID1, + VirtualMachine.State.Starting, VirtualMachine.State.Running)).thenReturn(virtualRouters1); + + // This list is not empty, so it will not add any for this pod, and continue with next pod + List<DomainRouterVO> virtualRouters2 = new ArrayList<>(); + DomainRouterVO domainRouterVO1 = mock(DomainRouterVO.class); + virtualRouters2.add(domainRouterVO1); + when(this.mockRouterDao.listByPodIdAndStates(POD_ID2, + VirtualMachine.State.Starting, VirtualMachine.State.Running)).thenReturn(virtualRouters2); + + // Leave this last one empty to check we finally added more than one afterwards + List<DomainRouterVO> virtualRouters3 = new ArrayList<>(); + when(this.mockRouterDao.listByPodIdAndStates(POD_ID3, + VirtualMachine.State.Starting, VirtualMachine.State.Running)).thenReturn(virtualRouters3); + + // Execute + List<DeployDestination> destinations = deployment.findDestinations(); + + // Assert that 2 were added (for the 1st and 3rd + Assert.assertEquals("", + 2, destinations.size()); + Assert.assertEquals("", + this.mockDataCenter, destinations.get(0).getDataCenter()); + Assert.assertEquals("", + this.mockHostPodVO1, destinations.get(0).getPod()); + Assert.assertEquals("", + this.mockDataCenter, destinations.get(1).getDataCenter()); + Assert.assertEquals("", + this.mockHostPodVO3, destinations.get(1).getPod()); + } + + @Test + public void testListByDataCenterIdVMTypeAndStates() { + // TODO Implement this test + } + + @Test + public void testFindOrDeployVirtualRouter() { + // TODO Implement this test + } + + @Test + public void testDeployVirtualRouter() { + // TODO Implement this test + } + + @Test + public void testExecuteDeployment() { + // TODO Implement this test + } + + @Test + public void testPlanDeploymentRouters() { + // TODO Implement this test + } + + @Test + public void testCreateRouterNetworks() { + // TODO Implement this test + } + +} http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3f021625/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 new file mode 100644 index 0000000..50ebed4 --- /dev/null +++ b/server/test/org/cloud/network/router/deployment/VpcRouterDeploymentDefinitionTest.java @@ -0,0 +1,52 @@ +package org.cloud.network.router.deployment; + +import org.junit.Test; + +public class VpcRouterDeploymentDefinitionTest { + + @Test + public void testGenerateDeploymentPlan() { + // TODO Implement this test + } + + @Test + public void testLock() { + // TODO Implement this test + } + + @Test + public void testUnlock() { + // TODO Implement this test + } + + @Test + public void testCheckPreconditions() { + // TODO Implement this test + } + + @Test + public void testFindDestinations() { + // TODO Implement this test + } + + @Test + public void testExecuteDeployment() { + // TODO Implement this test + } + + @Test + public void testPlanDeploymentRouters() { + // TODO Implement this test + } + + @Test + public void testDeployVpcRouter() { + // TODO Implement this test + } + + @Test + public void testCreateVpcRouterNetworks() { + // TODO Implement this test + } + +}
