Updated Branches: refs/heads/internallb 35c0273b8 -> 8057567aa
Internallb: more unittests for ApplicationLoadBalancerService Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/8057567a Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/8057567a Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/8057567a Branch: refs/heads/internallb Commit: 8057567aaab5000b2f6d3aec8ace65631092adc1 Parents: 35c0273 Author: Alena Prokharchyk <[email protected]> Authored: Tue Apr 23 13:15:36 2013 -0700 Committer: Alena Prokharchyk <[email protected]> Committed: Tue Apr 23 15:13:30 2013 -0700 ---------------------------------------------------------------------- server/src/com/cloud/api/ApiResponseHelper.java | 4 +- .../lb/ApplicationLoadBalancerManagerImpl.java | 48 ++-- .../cloudstack/lb/ApplicationLoadBalancerTest.java | 237 +++++++++++++-- 3 files changed, 241 insertions(+), 48 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8057567a/server/src/com/cloud/api/ApiResponseHelper.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/api/ApiResponseHelper.java b/server/src/com/cloud/api/ApiResponseHelper.java index 909eab1..f1422bb 100755 --- a/server/src/com/cloud/api/ApiResponseHelper.java +++ b/server/src/com/cloud/api/ApiResponseHelper.java @@ -2779,8 +2779,8 @@ public class ApiResponseHelper implements ResponseGenerator { @Override public VirtualRouterProviderResponse createVirtualRouterProviderResponse(VirtualRouterProvider result) { //generate only response of the VR/VPCVR provider type - if (!(result.getType() == VirtualRouterProvider.VirtualRouterProviderType.VirtualRouter) - || (result.getType() == VirtualRouterProvider.VirtualRouterProviderType.VPCVirtualRouter)) { + if (!(result.getType() == VirtualRouterProvider.VirtualRouterProviderType.VirtualRouter + || result.getType() == VirtualRouterProvider.VirtualRouterProviderType.VPCVirtualRouter)) { return null; } VirtualRouterProviderResponse response = new VirtualRouterProviderResponse(); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8057567a/server/src/org/apache/cloudstack/network/lb/ApplicationLoadBalancerManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/org/apache/cloudstack/network/lb/ApplicationLoadBalancerManagerImpl.java b/server/src/org/apache/cloudstack/network/lb/ApplicationLoadBalancerManagerImpl.java index 747b42b..b646bad 100644 --- a/server/src/org/apache/cloudstack/network/lb/ApplicationLoadBalancerManagerImpl.java +++ b/server/src/org/apache/cloudstack/network/lb/ApplicationLoadBalancerManagerImpl.java @@ -94,8 +94,8 @@ public class ApplicationLoadBalancerManagerImpl extends ManagerBase implements A //Validate LB rule guest network Network guestNtwk = _networkModel.getNetwork(networkId); - if (guestNtwk == null) { - throw new InvalidParameterValueException("Can't find network by id"); + if (guestNtwk == null || guestNtwk.getTrafficType() != TrafficType.Guest) { + throw new InvalidParameterValueException("Can't find guest network by id"); } Account caller = UserContext.current().getCaller(); @@ -130,10 +130,10 @@ public class ApplicationLoadBalancerManagerImpl extends ManagerBase implements A validateSourceIpNtwkForLbRule(sourceIpNtwk, scheme); //3) Get source ip address - sourceIp = getSourceIp(scheme, sourceIpNtwk, sourceIp); + Ip sourceIpAddr = getSourceIp(scheme, sourceIpNtwk, sourceIp); ApplicationLoadBalancerRuleVO newRule = new ApplicationLoadBalancerRuleVO(name, description, sourcePort, instancePort, algorithm, guestNtwk.getId(), - lbOwner.getId(), lbOwner.getDomainId(), new Ip(sourceIp), sourceIpNtwk.getId(), scheme); + lbOwner.getId(), lbOwner.getDomainId(), sourceIpAddr, sourceIpNtwk.getId(), scheme); //4) Validate Load Balancing rule on the providers LoadBalancingRule loadBalancing = new LoadBalancingRule(newRule, new ArrayList<LbDestination>(), @@ -218,7 +218,7 @@ public class ApplicationLoadBalancerManagerImpl extends ManagerBase implements A throw new InvalidParameterValueException("Invalid value for instance port: " + instancePort); } - if (!NetUtils.isValidPort(instancePort)) { + if (!NetUtils.isValidPort(sourcePort)) { throw new InvalidParameterValueException("Invalid value for source port: " + sourcePort); } @@ -236,34 +236,40 @@ public class ApplicationLoadBalancerManagerImpl extends ManagerBase implements A * @return * @throws InsufficientVirtualNetworkCapcityException */ - protected String getSourceIp(Scheme scheme, Network sourceIpNtwk, String requestedIp) throws InsufficientVirtualNetworkCapcityException { + protected Ip getSourceIp(Scheme scheme, Network sourceIpNtwk, String requestedIp) throws InsufficientVirtualNetworkCapcityException { //Get source IP address + if (_lbDao.countBySourceIp(new Ip(requestedIp), sourceIpNtwk.getId()) > 0) { + s_logger.debug("IP address " + requestedIp + " is already used by existing LB rule, returning it"); + return new Ip(requestedIp); + } + if (requestedIp != null) { validateRequestedSourceIpForLbRule(sourceIpNtwk, new Ip(requestedIp), scheme); - } else { - requestedIp = allocateSourceIpForLbRule(scheme, sourceIpNtwk); } + requestedIp = allocateSourceIpForLbRule(scheme, sourceIpNtwk, requestedIp); + if (requestedIp == null) { throw new InsufficientVirtualNetworkCapcityException("Unable to acquire IP address for network " + sourceIpNtwk, Network.class, sourceIpNtwk.getId()); } - return requestedIp; + return new Ip(requestedIp); } /** * Allocates new Source IP address for the Load Balancer rule based on LB rule scheme/sourceNetwork * @param scheme - * @param sourceIp * @param sourceIpNtwk + * @param requestedIp TODO + * @param sourceIp * @return */ - protected String allocateSourceIpForLbRule(Scheme scheme, Network sourceIpNtwk) { + protected String allocateSourceIpForLbRule(Scheme scheme, Network sourceIpNtwk, String requestedIp) { String sourceIp = null; if (scheme != Scheme.Internal) { throw new InvalidParameterValueException("Only scheme " + Scheme.Internal + " is supported"); } else { - sourceIp = allocateSourceIpForInternalLbRule(sourceIpNtwk); + sourceIp = allocateSourceIpForInternalLbRule(sourceIpNtwk, requestedIp); } return sourceIp; } @@ -272,10 +278,11 @@ public class ApplicationLoadBalancerManagerImpl extends ManagerBase implements A /** * Allocates sourceIp for the Internal LB rule * @param sourceIpNtwk + * @param requestedIp TODO * @return */ - protected String allocateSourceIpForInternalLbRule(Network sourceIpNtwk) { - return _ntwkMgr.acquireGuestIpAddress(sourceIpNtwk, null); + protected String allocateSourceIpForInternalLbRule(Network sourceIpNtwk, String requestedIp) { + return _ntwkMgr.acquireGuestIpAddress(sourceIpNtwk, requestedIp); } @@ -302,22 +309,11 @@ public class ApplicationLoadBalancerManagerImpl extends ManagerBase implements A * @param requestedSourceIp */ protected void validateRequestedSourceIpForInternalLbRule(Network sourceIpNtwk, Ip requestedSourceIp) { - //1) Check if the IP is within the network cidr + //Check if the IP is within the network cidr Pair<String, Integer> cidr = NetUtils.getCidr(sourceIpNtwk.getCidr()); if (!NetUtils.getCidrSubNet(requestedSourceIp.addr(), cidr.second()).equalsIgnoreCase(NetUtils.getCidrSubNet(cidr.first(), cidr.second()))) { throw new InvalidParameterValueException("The requested IP is not in the network's CIDR subnet."); } - - //2) Check if the IP address used by the load balancer or other nics - if (_lbDao.countBySourceIp(requestedSourceIp, sourceIpNtwk.getId()) > 0) { - s_logger.debug("IP address " + requestedSourceIp.addr() + " is already used by existing LB rule, skipping the validation"); - return; - } else { - List<String> usedIps = _networkModel.getUsedIpsInNetwork(sourceIpNtwk); - if (usedIps.contains(requestedSourceIp.toString())) { - throw new InvalidParameterValueException("Ip address " + requestedSourceIp.addr() + " is already in use"); - } - } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8057567a/server/test/org/apache/cloudstack/lb/ApplicationLoadBalancerTest.java ---------------------------------------------------------------------- diff --git a/server/test/org/apache/cloudstack/lb/ApplicationLoadBalancerTest.java b/server/test/org/apache/cloudstack/lb/ApplicationLoadBalancerTest.java index 995da2c..e18a461 100644 --- a/server/test/org/apache/cloudstack/lb/ApplicationLoadBalancerTest.java +++ b/server/test/org/apache/cloudstack/lb/ApplicationLoadBalancerTest.java @@ -16,7 +16,10 @@ // under the License. package org.apache.cloudstack.lb; +import java.lang.reflect.Field; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import javax.inject.Inject; @@ -28,6 +31,7 @@ import org.apache.cloudstack.network.lb.ApplicationLoadBalancerRule; import org.apache.cloudstack.network.lb.ApplicationLoadBalancerRuleVO; import org.apache.cloudstack.network.lb.dao.ApplicationLoadBalancerRuleDao; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mockito; @@ -39,6 +43,7 @@ import com.cloud.exception.InsufficientAddressCapacityException; import com.cloud.exception.InsufficientVirtualNetworkCapcityException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.NetworkRuleConflictException; +import com.cloud.exception.UnsupportedServiceException; import com.cloud.network.Network; import com.cloud.network.Network.Capability; import com.cloud.network.Network.Service; @@ -50,11 +55,13 @@ import com.cloud.network.lb.LoadBalancingRule; import com.cloud.network.lb.LoadBalancingRulesManager; import com.cloud.network.rules.FirewallRuleVO; import com.cloud.network.rules.LoadBalancerContainer.Scheme; +import com.cloud.offerings.NetworkOfferingVO; import com.cloud.user.AccountManager; import com.cloud.user.AccountVO; import com.cloud.user.UserContext; import com.cloud.user.UserVO; import com.cloud.utils.component.ComponentContext; +import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.Ip; import com.cloud.utils.net.NetUtils; @@ -81,11 +88,14 @@ public class ApplicationLoadBalancerTest extends TestCase{ public static long existingLbId = 1L; public static long nonExistingLbId = 2L; - public static long validNetworkId = 1L; - public static long invalidNetworkId = 2L; + public static long validGuestNetworkId = 1L; + public static long invalidGuestNetworkId = 2L; + public static long validPublicNetworkId = 3L; public static long validAccountId = 1L; public static long invalidAccountId = 2L; + + public String validRequestedIp = "10.1.1.1"; @@ -101,23 +111,25 @@ public class ApplicationLoadBalancerTest extends TestCase{ Mockito.when(_lbMgr.deleteLoadBalancerRule(nonExistingLbId, true)).thenReturn(false); //mockito for .createApplicationLoadBalancer tests - NetworkVO network = new NetworkVO(TrafficType.Guest, null, null, 1, - null, 1, 1L); + NetworkVO guestNetwork = new NetworkVO(TrafficType.Guest, null, null, 1, + null, 1, 1L); + setId(guestNetwork, validGuestNetworkId); + guestNetwork.setCidr("10.1.1.1/24"); + + NetworkVO publicNetwork = new NetworkVO(TrafficType.Public, null, null, 1, + null, 1, 1L); - Mockito.when(_ntwkModel.getNetwork(validNetworkId)).thenReturn(network); - Mockito.when(_ntwkModel.getNetwork(invalidNetworkId)).thenReturn(null); + Mockito.when(_ntwkModel.getNetwork(validGuestNetworkId)).thenReturn(guestNetwork); + Mockito.when(_ntwkModel.getNetwork(invalidGuestNetworkId)).thenReturn(null); + Mockito.when(_ntwkModel.getNetwork(validPublicNetworkId)).thenReturn(publicNetwork); + Mockito.when(_accountMgr.getAccount(validAccountId)).thenReturn(new AccountVO()); Mockito.when(_accountMgr.getAccount(invalidAccountId)).thenReturn(null); - Mockito.when(_ntwkModel.areServicesSupportedInNetwork(validNetworkId, Service.Lb)).thenReturn(true); - Mockito.when(_ntwkModel.areServicesSupportedInNetwork(invalidNetworkId, Service.Lb)).thenReturn(false); - - String supportedProtocols = NetUtils.TCP_PROTO.toLowerCase(); - Map<Network.Capability, String> capsMap = new HashMap<Network.Capability, String>(); - capsMap.put(Capability.SupportedProtocols, supportedProtocols); - Mockito.when(_ntwkModel.getNetworkServiceCapabilities(validNetworkId, Service.Lb)).thenReturn(capsMap); + Mockito.when(_ntwkModel.areServicesSupportedInNetwork(validGuestNetworkId, Service.Lb)).thenReturn(true); + Mockito.when(_ntwkModel.areServicesSupportedInNetwork(invalidGuestNetworkId, Service.Lb)).thenReturn(false); ApplicationLoadBalancerRuleVO lbRule = new ApplicationLoadBalancerRuleVO("new", "new", 22, 22, "roundrobin", - validNetworkId, validAccountId, 1L, new Ip("10.1.1.1"), validNetworkId, Scheme.Internal); + validGuestNetworkId, validAccountId, 1L, new Ip(validRequestedIp), validGuestNetworkId, Scheme.Internal); Mockito.when(_lbDao.persist(Mockito.any(ApplicationLoadBalancerRuleVO.class))).thenReturn(lbRule); Mockito.when(_lbMgr.validateLbRule(Mockito.any(LoadBalancingRule.class))).thenReturn(true); @@ -128,6 +140,15 @@ public class ApplicationLoadBalancerTest extends TestCase{ Mockito.when(_accountMgr.getSystemAccount()).thenReturn(new AccountVO(2)); UserContext.registerContext(_accountMgr.getSystemUser().getId(), _accountMgr.getSystemAccount(), null, false); + Mockito.when(_ntwkModel.areServicesSupportedInNetwork(Mockito.anyLong(), Mockito.any(Network.Service.class))).thenReturn(true); + + Map<Network.Capability, String> caps = new HashMap<Network.Capability, String>(); + caps.put(Capability.SupportedProtocols, NetUtils.TCP_PROTO); + Mockito.when(_ntwkModel.getNetworkServiceCapabilities(Mockito.anyLong(), Mockito.any(Network.Service.class))).thenReturn(caps); + + + Mockito.when(_lbDao.countBySourceIp(new Ip(validRequestedIp), validGuestNetworkId)).thenReturn(1L); + } /** @@ -190,13 +211,65 @@ public class ApplicationLoadBalancerTest extends TestCase{ /** * TESTS FOR .createApplicationLoadBalancer */ + + @Test //Positive test public void createValidLoadBalancer() { - ApplicationLoadBalancerRule rule = null; - + String expectedExcText = null; + try { + _appLbSvc.createApplicationLoadBalancer("alena", "alena", Scheme.Internal, validGuestNetworkId, validRequestedIp, + 22, 22, "roundrobin", validGuestNetworkId, validAccountId); + } catch (InsufficientAddressCapacityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (InsufficientVirtualNetworkCapcityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (NetworkRuleConflictException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (CloudRuntimeException e) { + expectedExcText = e.getMessage(); + } finally { + //expect the exception happen because persistlLbRule() method coudln't be mocked properly due to unability to mock static vars in UsageEventUtils class + assertEquals("Test failed. The rule wasn't created" + expectedExcText, expectedExcText, "Unable to add lb rule for ip address null"); + } + } + + + @Test(expected = UnsupportedServiceException.class) + //Negative test - only internal scheme value is supported in the current release + public void createPublicLoadBalancer() { + String expectedExcText = null; + try { + _appLbSvc.createApplicationLoadBalancer("alena", "alena", Scheme.Public, validGuestNetworkId, validRequestedIp, + 22, 22, "roundrobin", validGuestNetworkId, validAccountId); + } catch (InsufficientAddressCapacityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (InsufficientVirtualNetworkCapcityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (NetworkRuleConflictException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (UnsupportedServiceException e) { + expectedExcText = e.getMessage(); + throw e; + } finally { + assertEquals("Test failed. The Public lb rule was created, which shouldn't be supported" + + expectedExcText, expectedExcText, "Only scheme of type " + Scheme.Internal + " is supported"); + } + } + + + @Test(expected = InvalidParameterValueException.class) + //Negative test - invalid SourcePort + public void createWithInvalidSourcePort() { + String expectedExcText = null; try { - rule = _appLbSvc.createApplicationLoadBalancer("alena", "alena", Scheme.Internal, validNetworkId, "10.1.1.1", - 22, 22, "roundrobin", validNetworkId, validAccountId); + _appLbSvc.createApplicationLoadBalancer("alena", "alena", Scheme.Internal, validGuestNetworkId, validRequestedIp, + 65536, 22, "roundrobin", validGuestNetworkId, validAccountId); } catch (InsufficientAddressCapacityException e) { // TODO Auto-generated catch block e.printStackTrace(); @@ -206,8 +279,132 @@ public class ApplicationLoadBalancerTest extends TestCase{ } catch (NetworkRuleConflictException e) { // TODO Auto-generated catch block e.printStackTrace(); - }finally { - //assertNotNull("Failed to create application load balancer rule", rule); + } catch (InvalidParameterValueException e) { + expectedExcText = e.getMessage(); + throw e; + } finally { + assertEquals("Test failed. The rule with invalid source port was created" + + expectedExcText, expectedExcText, "Invalid value for source port: 65536"); } } + + @Test(expected = InvalidParameterValueException.class) + //Negative test - invalid instancePort + public void createWithInvalidInstandePort() { + String expectedExcText = null; + try { + _appLbSvc.createApplicationLoadBalancer("alena", "alena", Scheme.Internal, validGuestNetworkId, validRequestedIp, + 22, 65536, "roundrobin", validGuestNetworkId, validAccountId); + } catch (InsufficientAddressCapacityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (InsufficientVirtualNetworkCapcityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (NetworkRuleConflictException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (InvalidParameterValueException e) { + expectedExcText = e.getMessage(); + throw e; + } finally { + assertEquals("Test failed. The rule with invalid instance port was created" + + expectedExcText, expectedExcText, "Invalid value for instance port: 65536"); + } + } + + + @Test(expected = InvalidParameterValueException.class) + //Negative test - invalid algorithm + public void createWithInvalidAlgorithm() { + String expectedExcText = null; + try { + _appLbSvc.createApplicationLoadBalancer("alena", "alena", Scheme.Internal, validGuestNetworkId, validRequestedIp, + 22, 22, "invalidalgorithm", validGuestNetworkId, validAccountId); + } catch (InsufficientAddressCapacityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (InsufficientVirtualNetworkCapcityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (NetworkRuleConflictException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (InvalidParameterValueException e) { + expectedExcText = e.getMessage(); + throw e; + } finally { + assertEquals("Test failed. The rule with invalid algorithm was created" + + expectedExcText, expectedExcText, "Invalid algorithm: invalidalgorithm"); + } + } + + @Test(expected = InvalidParameterValueException.class) + //Negative test - invalid sourceNetworkId (of Public type, which is not supported) + public void createWithInvalidSourceIpNtwk() { + String expectedExcText = null; + try { + _appLbSvc.createApplicationLoadBalancer("alena", "alena", Scheme.Internal, validPublicNetworkId, validRequestedIp, + 22, 22, "roundrobin", validGuestNetworkId, validAccountId); + } catch (InsufficientAddressCapacityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (InsufficientVirtualNetworkCapcityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (NetworkRuleConflictException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (InvalidParameterValueException e) { + expectedExcText = e.getMessage(); + throw e; + } finally { + assertEquals("Test failed. The rule with invalid source ip network id was created" + + expectedExcText, expectedExcText, "Only traffic type Guest is supported"); + } + } + + + @Test(expected = InvalidParameterValueException.class) + //Negative test - invalid requested IP (outside of guest network cidr range) + public void createWithInvalidRequestedIp() { + String expectedExcText = null; + try { + _appLbSvc.createApplicationLoadBalancer("alena", "alena", Scheme.Internal, validGuestNetworkId, "10.2.1.1", + 22, 22, "roundrobin", validGuestNetworkId, validAccountId); + } catch (InsufficientAddressCapacityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (InsufficientVirtualNetworkCapcityException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } catch (NetworkRuleConflictException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + + } catch (InvalidParameterValueException e) { + expectedExcText = e.getMessage(); + throw e; + } finally { + assertEquals("Test failed. The rule with invalid requested ip was created" + + expectedExcText, expectedExcText, "The requested IP is not in the network's CIDR subnet."); + } + } + + + private static NetworkVO setId(NetworkVO vo, long id) { + NetworkVO voToReturn = vo; + Class<?> c = voToReturn.getClass(); + try { + Field f = c.getDeclaredField("id"); + f.setAccessible(true); + f.setLong(voToReturn, id); + } catch (NoSuchFieldException ex) { + return null; + } catch (IllegalAccessException ex) { + return null; + } + + return voToReturn; + } }
