Refactor hypervisor retrieval from VpcNwHelper and NwHelper Conflicts: server/src/com/cloud/network/router/NetworkHelperImpl.java server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java server/test/org/cloud/network/router/deployment/VpcRouterDeploymentDefinitionTest.java
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/cd8cadaf Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/cd8cadaf Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/cd8cadaf Branch: refs/heads/master Commit: cd8cadaf9c275d223f9121115a8fbaa40568b6ad Parents: 65fb216 Author: Antonio Fornie <afor...@schubergphilis.com> Authored: Thu Aug 21 09:52:28 2014 -0500 Committer: wilderrodrigues <wrodrig...@schubergphilis.com> Committed: Tue Oct 14 15:08:09 2014 +0200 ---------------------------------------------------------------------- .../com/cloud/network/router/NetworkHelper.java | 3 +- .../cloud/network/router/NetworkHelperImpl.java | 80 ++++++++++---------- .../network/router/VpcNetworkHelperImpl.java | 21 ++++- .../com/cloud/network/vpc/VpcManagerImpl.java | 23 ++++-- .../deployment/RouterDeploymentDefinition.java | 7 ++ .../VpcRouterDeploymentDefinition.java | 3 +- .../RouterDeploymentDefinitionTest.java | 2 +- .../VpcRouterDeploymentDefinitionTest.java | 40 +++++++++- 8 files changed, 117 insertions(+), 62 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/cd8cadaf/server/src/com/cloud/network/router/NetworkHelper.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/router/NetworkHelper.java b/server/src/com/cloud/network/router/NetworkHelper.java index 5cfae8e..635e56c 100644 --- a/server/src/com/cloud/network/router/NetworkHelper.java +++ b/server/src/com/cloud/network/router/NetworkHelper.java @@ -30,7 +30,6 @@ import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.InsufficientServerCapacityException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.exception.StorageUnavailableException; -import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.user.Account; import com.cloud.user.User; import com.cloud.vm.DomainRouterVO; @@ -79,7 +78,7 @@ public interface NetworkHelper { public abstract DomainRouterVO deployRouter( RouterDeploymentDefinition routerDeploymentDefinition, - boolean startRouter, List<HypervisorType> supportedHypervisors) + boolean startRouter) throws InsufficientAddressCapacityException, InsufficientServerCapacityException, InsufficientCapacityException, StorageUnavailableException, ResourceUnavailableException; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/cd8cadaf/server/src/com/cloud/network/router/NetworkHelperImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/router/NetworkHelperImpl.java b/server/src/com/cloud/network/router/NetworkHelperImpl.java index 689ef3b..fd1fc17 100644 --- a/server/src/com/cloud/network/router/NetworkHelperImpl.java +++ b/server/src/com/cloud/network/router/NetworkHelperImpl.java @@ -18,15 +18,18 @@ package com.cloud.network.router; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import javax.annotation.PostConstruct; import javax.ejb.Local; import javax.inject.Inject; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.log4j.Logger; import org.cloud.network.router.deployment.RouterDeploymentDefinition; @@ -142,9 +145,17 @@ public class NetworkHelperImpl implements NetworkHelper { @Inject protected NetworkOrchestrationService _networkMgr; - /* (non-Javadoc) - * @see com.cloud.network.router.NetworkHelper#getRouterControlIp(long) - */ + protected final Map<HypervisorType, ConfigKey<String>> hypervisorsMap = new HashMap<>(); + + @PostConstruct + protected void setupHypervisorsMap() { + hypervisorsMap.put(HypervisorType.XenServer, VirtualNetworkApplianceManager.RouterTemplateXen); + hypervisorsMap.put(HypervisorType.KVM, VirtualNetworkApplianceManager.RouterTemplateKvm); + hypervisorsMap.put(HypervisorType.VMware, VirtualNetworkApplianceManager.RouterTemplateVmware); + hypervisorsMap.put(HypervisorType.Hyperv, VirtualNetworkApplianceManager.RouterTemplateHyperV); + hypervisorsMap.put(HypervisorType.LXC, VirtualNetworkApplianceManager.RouterTemplateLxc); + } + @Override public String getRouterControlIp(final long routerId) { String routerControlIpAddress = null; @@ -499,18 +510,17 @@ public class NetworkHelperImpl implements NetworkHelper { @Override public DomainRouterVO deployRouter(final RouterDeploymentDefinition routerDeploymentDefinition, - final boolean startRouter, final List<HypervisorType> supportedHypervisors) + final boolean startRouter) throws InsufficientAddressCapacityException, InsufficientServerCapacityException, InsufficientCapacityException, StorageUnavailableException, ResourceUnavailableException { final ServiceOfferingVO routerOffering = _serviceOfferingDao.findById(routerDeploymentDefinition.getOfferingId()); - final DeployDestination dest = routerDeploymentDefinition.getDest(); final Account owner = routerDeploymentDefinition.getOwner(); // Router is the network element, we don't know the hypervisor type yet. // Try to allocate the domR twice using diff hypervisors, and when failed both times, throw the exception up - final List<HypervisorType> hypervisors = getHypervisors(routerDeploymentDefinition, supportedHypervisors); + final List<HypervisorType> hypervisors = getHypervisors(routerDeploymentDefinition); int allocateRetry = 0; int startRetry = 0; @@ -520,29 +530,13 @@ public class NetworkHelperImpl implements NetworkHelper { try { final long id = _routerDao.getNextInSequence(Long.class, "id"); if (s_logger.isDebugEnabled()) { - s_logger.debug("Allocating the VR i=" + id + " in datacenter " + dest.getDataCenter() + "with the hypervisor type " + hType); + s_logger.debug(String.format( + "Allocating the VR with id=%s in datacenter %s with the hypervisor type %s", + id, routerDeploymentDefinition.getDest().getDataCenter(), hType)); } - String templateName = null; - switch (hType) { - case XenServer: - templateName = VirtualNetworkApplianceManager.RouterTemplateXen.valueIn(dest.getDataCenter().getId()); - break; - case KVM: - templateName = VirtualNetworkApplianceManager.RouterTemplateKvm.valueIn(dest.getDataCenter().getId()); - break; - case VMware: - templateName = VirtualNetworkApplianceManager.RouterTemplateVmware.valueIn(dest.getDataCenter().getId()); - break; - case Hyperv: - templateName = VirtualNetworkApplianceManager.RouterTemplateHyperV.valueIn(dest.getDataCenter().getId()); - break; - case LXC: - templateName = VirtualNetworkApplianceManager.RouterTemplateLxc.valueIn(dest.getDataCenter().getId()); - break; - default: - break; - } + String templateName = hypervisorsMap.get(hType) + .valueIn(routerDeploymentDefinition.getDest().getDataCenter().getId()); final VMTemplateVO template = _templateDao.findRoutingTemplate(hType, templateName); if (template == null) { @@ -567,7 +561,7 @@ public class NetworkHelperImpl implements NetworkHelper { router.setDynamicallyScalable(template.isDynamicallyScalable()); router.setRole(Role.VIRTUAL_ROUTER); router = _routerDao.persist(router); - LinkedHashMap<Network, List<? extends NicProfile>> networks = this.createRouterNetworks(routerDeploymentDefinition); + LinkedHashMap<Network, List<? extends NicProfile>> networks = createRouterNetworks(routerDeploymentDefinition); _itMgr.allocate(router.getInstanceName(), template, routerOffering, networks, routerDeploymentDefinition.getPlan(), null); router = _routerDao.findById(router.getId()); } catch (final InsufficientCapacityException ex) { @@ -606,7 +600,15 @@ public class NetworkHelperImpl implements NetworkHelper { return router; } - protected List<HypervisorType> getHypervisors(final RouterDeploymentDefinition routerDeploymentDefinition, final List<HypervisorType> supportedHypervisors) + protected void filterSupportedHypervisors(final List<HypervisorType> hypervisors) { + // For non vpc we keep them all assuming all types in the list are supported + } + + protected String getNoHypervisorsErrMsgDetails() { + return ""; + } + + protected List<HypervisorType> getHypervisors(final RouterDeploymentDefinition routerDeploymentDefinition) throws InsufficientServerCapacityException { final DeployDestination dest = routerDeploymentDefinition.getDest(); List<HypervisorType> hypervisors = new ArrayList<HypervisorType>(); @@ -627,23 +629,17 @@ public class NetworkHelperImpl implements NetworkHelper { } } - //keep only elements defined in supported hypervisors - final StringBuilder hTypesStr = new StringBuilder(); - if (supportedHypervisors != null && !supportedHypervisors.isEmpty()) { - hypervisors.retainAll(supportedHypervisors); - for (final HypervisorType hType : supportedHypervisors) { - hTypesStr.append(hType).append(" "); - } - } + filterSupportedHypervisors(hypervisors); if (hypervisors.isEmpty()) { - final String errMsg = hTypesStr.capacity() > 0 ? "supporting hypervisors " + hTypesStr.toString() : ""; if (routerDeploymentDefinition.getPodId() != null) { - throw new InsufficientServerCapacityException("Unable to create virtual router, " + "there are no clusters in the pod " + errMsg, Pod.class, - routerDeploymentDefinition.getPodId()); + throw new InsufficientServerCapacityException( + "Unable to create virtual router, there are no clusters in the pod." + getNoHypervisorsErrMsgDetails(), + Pod.class, routerDeploymentDefinition.getPodId()); } - throw new InsufficientServerCapacityException("Unable to create virtual router, " + "there are no clusters in the zone " + errMsg, DataCenter.class, - dest.getDataCenter().getId()); + throw new InsufficientServerCapacityException( + "Unable to create virtual router, there are no clusters in the zone." + getNoHypervisorsErrMsgDetails(), + DataCenter.class, dest.getDataCenter().getId()); } return hypervisors; } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/cd8cadaf/server/src/com/cloud/network/router/VpcNetworkHelperImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/router/VpcNetworkHelperImpl.java b/server/src/com/cloud/network/router/VpcNetworkHelperImpl.java index c6c3785..bbd08fd 100644 --- a/server/src/com/cloud/network/router/VpcNetworkHelperImpl.java +++ b/server/src/com/cloud/network/router/VpcNetworkHelperImpl.java @@ -25,12 +25,14 @@ import java.util.TreeSet; import javax.inject.Inject; +import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; import org.cloud.network.router.deployment.RouterDeploymentDefinition; import com.cloud.dc.dao.VlanDao; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.InsufficientAddressCapacityException; +import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.network.IpAddress; import com.cloud.network.Network; import com.cloud.network.Networks.BroadcastDomainType; @@ -50,10 +52,21 @@ public class VpcNetworkHelperImpl extends NetworkHelperImpl { @Inject private VlanDao _vlanDao; @Inject - protected VpcManager _vpcMgr; + protected VpcManager vpcMgr; @Inject protected NicProfileHelper nicProfileHelper; + protected final String noHypervisorsErrMsgDetails = StringUtils.join(this.vpcMgr.getSupportedVpcHypervisors(), ','); + + @Override + protected String getNoHypervisorsErrMsgDetails() { + return this.noHypervisorsErrMsgDetails; + } + + @Override + protected void filterSupportedHypervisors(final List<HypervisorType> hypervisors) { + hypervisors.retainAll(this.vpcMgr.getSupportedVpcHypervisors()); + } @Override public LinkedHashMap<Network, List<? extends NicProfile>> createRouterNetworks( @@ -70,7 +83,7 @@ public class VpcNetworkHelperImpl extends NetworkHelperImpl { final Long vpcId = vpcRouterDeploymentDefinition.getVpc().getId(); //2) allocate nic for private gateways if needed - final List<PrivateGateway> privateGateways = this._vpcMgr.getVpcPrivateGateways(vpcId); + final List<PrivateGateway> privateGateways = this.vpcMgr.getVpcPrivateGateways(vpcId); if (privateGateways != null && !privateGateways.isEmpty()) { for (PrivateGateway privateGateway : privateGateways) { NicProfile privateNic = this.nicProfileHelper.createPrivateNicProfileForGateway(privateGateway); @@ -80,7 +93,7 @@ public class VpcNetworkHelperImpl extends NetworkHelperImpl { } //3) allocate nic for guest gateway if needed - List<? extends Network> guestNetworks = this._vpcMgr.getVpcNetworks(vpcId); + List<? extends Network> guestNetworks = this.vpcMgr.getVpcNetworks(vpcId); for (Network guestNetwork : guestNetworks) { if (_networkModel.isPrivateGateway(guestNetwork.getId())) { continue; @@ -97,7 +110,7 @@ public class VpcNetworkHelperImpl extends NetworkHelperImpl { Network publicNetwork = null; for (IPAddressVO ip : ips) { PublicIp publicIp = PublicIp.createFromAddrAndVlan(ip, this._vlanDao.findById(ip.getVlanId())); - if ((ip.getState() == IpAddress.State.Allocated || ip.getState() == IpAddress.State.Allocating) && this._vpcMgr.isIpAllocatedToVpc(ip) && + if ((ip.getState() == IpAddress.State.Allocated || ip.getState() == IpAddress.State.Allocating) && this.vpcMgr.isIpAllocatedToVpc(ip) && !publicVlans.contains(publicIp.getVlanTag())) { s_logger.debug("Allocating nic for router in vlan " + publicIp.getVlanTag()); NicProfile publicNic = new NicProfile(); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/cd8cadaf/server/src/com/cloud/network/vpc/VpcManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/com/cloud/network/vpc/VpcManagerImpl.java index c49da15..fbb94c7 100644 --- a/server/src/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/com/cloud/network/vpc/VpcManagerImpl.java @@ -20,6 +20,7 @@ package com.cloud.network.vpc; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -31,6 +32,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import javax.annotation.PostConstruct; import javax.ejb.Local; import javax.inject.Inject; import javax.naming.ConfigurationException; @@ -214,6 +216,18 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis int _maxNetworks; SearchBuilder<IPAddressVO> IpAddressSearch; + protected final List<HypervisorType> hTypes = new ArrayList<HypervisorType>(); + + @PostConstruct + protected void setupSupportedVpcHypervisorsList() { + this.hTypes.add(HypervisorType.XenServer); + this.hTypes.add(HypervisorType.VMware); + this.hTypes.add(HypervisorType.KVM); + this.hTypes.add(HypervisorType.Simulator); + this.hTypes.add(HypervisorType.LXC); + this.hTypes.add(HypervisorType.Hyperv); + } + @Override @DB public boolean configure(String name, Map<String, Object> params) throws ConfigurationException { @@ -2396,14 +2410,7 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis @Override public List<HypervisorType> getSupportedVpcHypervisors() { - List<HypervisorType> hTypes = new ArrayList<HypervisorType>(); - hTypes.add(HypervisorType.XenServer); - hTypes.add(HypervisorType.VMware); - hTypes.add(HypervisorType.KVM); - hTypes.add(HypervisorType.Simulator); - hTypes.add(HypervisorType.LXC); - hTypes.add(HypervisorType.Hyperv); - return hTypes; + return Collections.unmodifiableList(this.hTypes); } private List<Provider> getVpcProviders(long vpcId) { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/cd8cadaf/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 9852085..3342004 100644 --- a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java +++ b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java @@ -385,11 +385,18 @@ public class RouterDeploymentDefinition { protected void deployAllVirtualRouters() throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException { +<<<<<<< HEAD int routersToDeploy = getNumberOfRoutersToDeploy(); for (int i = 0; i < routersToDeploy; i++) { // Don't start the router as we are holding the network lock that // needs to be released at the end of router allocation DomainRouterVO router = nwHelper.deployRouter(this, false, null); +======= + int routersToDeploy = this.getNumberOfRoutersToDeploy(); + for(int i = 0; i < routersToDeploy; i++) { + // Don't start the router as we are holding the network lock that needs to be released at the end of router allocation + DomainRouterVO router = this.nwHelper.deployRouter(this, false); +>>>>>>> 2e8879f... Refactor hypervisor retrieval from VpcNwHelper and NwHelper if (router != null) { routerDao.addRouterToGuestNetwork(router, guestNetwork); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/cd8cadaf/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 4dc2d87..8e8a6ea 100644 --- a/server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java +++ b/server/src/org/cloud/network/router/deployment/VpcRouterDeploymentDefinition.java @@ -110,7 +110,6 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition { /** * @see RouterDeploymentDefinition#prepareDeployment() - * * @return if the deployment can proceed */ @Override @@ -157,7 +156,7 @@ public class VpcRouterDeploymentDefinition extends RouterDeploymentDefinition { @Override protected void deployAllVirtualRouters() throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException { - DomainRouterVO router = nwHelper.deployRouter(this, true, vpcMgr.getSupportedVpcHypervisors()); + DomainRouterVO router = nwHelper.deployRouter(this, true); if (router != null) { routers.add(router); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/cd8cadaf/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 68c66da..021e8d6 100644 --- a/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java +++ b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java @@ -801,7 +801,7 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe final DomainRouterVO routerVO1 = mock(DomainRouterVO.class); final DomainRouterVO routerVO2 = mock(DomainRouterVO.class); - when(this.mockNetworkHelper.deployRouter(deploymentUT, false, null)) + when(this.mockNetworkHelper.deployRouter(deploymentUT, false)) .thenReturn(routerVO1).thenReturn(routerVO2); // Execute http://git-wip-us.apache.org/repos/asf/cloudstack/blob/cd8cadaf/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 7b06aa0..4e4b777 100644 --- a/server/test/org/cloud/network/router/deployment/VpcRouterDeploymentDefinitionTest.java +++ b/server/test/org/cloud/network/router/deployment/VpcRouterDeploymentDefinitionTest.java @@ -34,6 +34,12 @@ import org.mockito.Mock; 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.addr.PublicIp; import com.cloud.network.dao.PhysicalNetworkDao; import com.cloud.network.dao.PhysicalNetworkServiceProviderDao; import com.cloud.network.router.NicProfileHelper; @@ -184,8 +190,24 @@ public class VpcRouterDeploymentDefinitionTest extends RouterDeploymentDefinitio } @Test - public void testDeployVpcRouter() { - // TODO Implement this test + public void testDeployAllVirtualRoutersWithNoDeployedRouter() throws InsufficientAddressCapacityException, InsufficientServerCapacityException, StorageUnavailableException, + InsufficientCapacityException, ResourceUnavailableException { + + driveTestDeployAllVirtualRouters(null); + + // Assert + assertTrue("No router should have been set as deployed", deployment.routers.isEmpty()); + + } + + public void driveTestDeployAllVirtualRouters(final DomainRouterVO router) throws InsufficientAddressCapacityException, InsufficientServerCapacityException, + StorageUnavailableException, InsufficientCapacityException, ResourceUnavailableException { + // Prepare + VpcRouterDeploymentDefinition vpcDeployment = (VpcRouterDeploymentDefinition) deployment; + when(vpcDeployment.nwHelper.deployRouter(vpcDeployment, true)).thenReturn(router); + + // Execute + vpcDeployment.deployAllVirtualRouters(); } @Test @@ -193,4 +215,16 @@ public class VpcRouterDeploymentDefinitionTest extends RouterDeploymentDefinitio // TODO Implement this test } -} + @Test + public void testFindSourceNatIP() throws InsufficientAddressCapacityException, ConcurrentOperationException { + // Prepare + PublicIp publicIp = mock(PublicIp.class); + when(vpcMgr.assignSourceNatIpAddressToVpc(mockOwner, mockVpc)).thenReturn(publicIp); + + // Execute + deployment.findSourceNatIP(); + + // Assert + assertEquals("SourceNatIp returned by the VpcManager was not correctly set", publicIp, deployment.sourceNatIp); + } +} \ No newline at end of file