diff --git a/api/src/com/cloud/network/lb/LoadBalancingRulesService.java b/api/src/com/cloud/network/lb/LoadBalancingRulesService.java index ee1df33cd41..6643de62b45 100644 --- a/api/src/com/cloud/network/lb/LoadBalancingRulesService.java +++ b/api/src/com/cloud/network/lb/LoadBalancingRulesService.java @@ -92,7 +92,7 @@ public interface LoadBalancingRulesService { boolean deleteLBHealthCheckPolicy(long healthCheckPolicyId, boolean apply); /** - * Assign a virtual machine, or list of virtual machines, to a load balancer. + * Assign a virtual machine or list of virtual machines, or Map of to a load balancer. */ boolean assignToLoadBalancer(long lbRuleId, List vmIds, Map> vmIdIpMap); @@ -151,4 +151,6 @@ public interface LoadBalancingRulesService { public void updateLBHealthChecks(Scheme scheme) throws ResourceUnavailableException; Map getLbInstances(long lbId); + + boolean isLbRuleMappedToVmGuestIp(String vmSecondaryIp); } diff --git a/api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java b/api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java index fad2c2a0c1d..bc76e978f5e 100644 --- a/api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java @@ -119,9 +119,8 @@ public class AssignToLoadBalancerRuleCmd extends BaseAsyncCmd { public Map> getVmIdIpListMap() { - Map> vmIdIpsMap = null; + Map> vmIdIpsMap = new HashMap>(); if (vmIdIpMap != null && !vmIdIpMap.isEmpty()) { - vmIdIpsMap = new HashMap>(); Collection idIpsCollection = vmIdIpMap.values(); Iterator iter = idIpsCollection.iterator(); while (iter.hasNext()) { diff --git a/server/src/com/cloud/network/NetworkServiceImpl.java b/server/src/com/cloud/network/NetworkServiceImpl.java index 1c7786e9f35..f23991cd0c4 100755 --- a/server/src/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/com/cloud/network/NetworkServiceImpl.java @@ -40,6 +40,7 @@ import javax.ejb.Local; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.network.lb.LoadBalancingRulesService; import org.apache.log4j.Logger; import org.apache.cloudstack.acl.ControlledEntity.ACLType; @@ -106,7 +107,6 @@ import com.cloud.network.dao.FirewallRulesDao; import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.LoadBalancerVMMapDao; -import com.cloud.network.dao.LoadBalancerVMMapVO; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkDomainDao; import com.cloud.network.dao.NetworkServiceMapDao; @@ -312,6 +312,9 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { @Inject LoadBalancerVMMapDao _lbVmMapDao; + @Inject + LoadBalancingRulesService _lbService; + int _cidrLimit; boolean _allowSubdomainNetworkAccess; @@ -811,11 +814,9 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { throw new InvalidParameterValueException("Can' remove the ip " + secondaryIp + "is associate with static NAT rule public IP address id " + publicIpVO.getId()); } - List lbVmMap = _lbVmMapDao.listByInstanceIp(secondaryIp); - if (lbVmMap != null && lbVmMap.size() != 0) { + if (_lbService.isLbRuleMappedToVmGuestIp(secondaryIp)) { s_logger.debug("VM nic IP " + secondaryIp + " is mapped to load balancing rule"); throw new InvalidParameterValueException("Can't remove the secondary ip " + secondaryIp + " is mapped to load balancing rule"); - } } else if (dc.getNetworkType() == NetworkType.Basic || ntwkOff.getGuestType() == Network.GuestType.Shared) { diff --git a/server/src/com/cloud/network/as/AutoScaleManagerImpl.java b/server/src/com/cloud/network/as/AutoScaleManagerImpl.java index b6e917dac92..e6cbd7c4772 100644 --- a/server/src/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/com/cloud/network/as/AutoScaleManagerImpl.java @@ -1414,7 +1414,7 @@ public class AutoScaleManagerImpl extends ManagerBase implements AutoScale } } lstVmId.add(new Long(vmId)); - return _loadBalancingRulesService.assignToLoadBalancer(lbId, lstVmId, null); + return _loadBalancingRulesService.assignToLoadBalancer(lbId, lstVmId, new HashMap>()); } diff --git a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java index 8a7e0298b45..f514cbf0e96 100755 --- a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java +++ b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java @@ -937,12 +937,12 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements } - if (instanceIds == null && vmIdIpMap == null) { + if (instanceIds == null && vmIdIpMap.isEmpty()) { throw new InvalidParameterValueException("Both instanceids and vmidipmap can't be null"); } // instanceIds and vmIdipmap is passed - if (instanceIds != null && vmIdIpMap != null) { + if (instanceIds != null && !vmIdIpMap.isEmpty()) { for(long instanceId: instanceIds) { if (!vmIdIpMap.containsKey(instanceId)) { vmIdIpMap.put(instanceId, null); @@ -951,7 +951,7 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements } //only instanceids list passed - if (instanceIds != null && vmIdIpMap == null){ + if (instanceIds != null && vmIdIpMap.isEmpty()){ vmIdIpMap = new HashMap>(); for (long instanceId: instanceIds){ vmIdIpMap.put(instanceId, null); @@ -978,9 +978,6 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements existingVmIdIps.put(mappedInstance.getInstanceId(), ipsList); } - - - final List vmsToAdd = new ArrayList(); // check for conflict @@ -1047,12 +1044,12 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements //check if the ips belongs to nic secondary ip for (String ip: vmIpsList) { if(_nicSecondaryIpDao.findByIp4AddressAndNicId(ip,nicInSameNetwork.getId()) == null) { - throw new InvalidParameterValueException("VM ip specified " + ip + " is not belongs to nic in network " + nicInSameNetwork.getNetworkId()); + throw new InvalidParameterValueException("VM ip "+ ip + " specified does not belong to " + + "nic in network " + nicInSameNetwork.getNetworkId()); } } } - if (s_logger.isDebugEnabled()) { s_logger.debug("Adding " + vm + " to the load balancer pool"); } @@ -2316,6 +2313,15 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements return dstList; } + @Override + public boolean isLbRuleMappedToVmGuestIp(String vmSecondaryIp) { + List lbVmMap = _lb2VmMapDao.listByInstanceIp(vmSecondaryIp); + if (lbVmMap == null || lbVmMap.isEmpty()) { + return false; + } + return true; + } + @Override public void isLbServiceSupportedInNetwork(long networkId, Scheme scheme) { Network network = _networkDao.findById(networkId); diff --git a/server/test/com/cloud/network/lb/AssignToLoadBalancerTest.java b/server/test/com/cloud/network/lb/AssignToLoadBalancerTest.java new file mode 100644 index 00000000000..f8f96a712f2 --- /dev/null +++ b/server/test/com/cloud/network/lb/AssignToLoadBalancerTest.java @@ -0,0 +1,231 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.network.lb; + +import com.cloud.domain.DomainVO; +import com.cloud.domain.dao.DomainDao; +import com.cloud.exception.ResourceAllocationException; +import com.cloud.exception.ResourceUnavailableException; +import com.cloud.exception.InsufficientCapacityException; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.network.NetworkModelImpl; +import com.cloud.network.dao.LoadBalancerDao; +import com.cloud.network.dao.LoadBalancerVMMapDao; +import com.cloud.network.dao.LoadBalancerVMMapVO; +import com.cloud.network.dao.LoadBalancerVO; +import com.cloud.network.rules.FirewallRule; +import com.cloud.network.rules.RulesManagerImpl; +import com.cloud.user.Account; +import com.cloud.user.AccountManager; +import com.cloud.user.AccountVO; +import com.cloud.user.UserVO; +import com.cloud.user.dao.AccountDao; +import com.cloud.uservm.UserVm; +import com.cloud.vm.Nic; +import com.cloud.vm.NicVO; +import com.cloud.vm.UserVmVO; +import com.cloud.vm.dao.NicSecondaryIpDao; +import com.cloud.vm.dao.UserVmDao; +import org.apache.cloudstack.api.ResponseGenerator; +import org.apache.cloudstack.api.command.user.loadbalancer.AssignToLoadBalancerRuleCmd; +import org.apache.cloudstack.api.response.SuccessResponse; +import org.apache.cloudstack.context.CallContext; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.Spy; + +import javax.inject.Inject; + +import java.util.UUID; +import java.util.HashMap; +import java.util.Map; +import java.util.List; +import java.util.ArrayList; + +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.when; + +public class AssignToLoadBalancerTest { + + @Inject + AccountManager _accountMgr; + + @Inject + AccountManager _acctMgr; + + @Inject + AccountDao _accountDao; + + @Inject + DomainDao _domainDao; + + @Mock + List _lbvmMapList; + + @Mock + List nic; + + @Mock + UserVmDao userDao; + + @Spy + RulesManagerImpl _rulesMgr = new RulesManagerImpl() { + @Override + public void checkRuleAndUserVm (FirewallRule rule, UserVm userVm, Account caller) { + + } + }; + + + @Spy + NicVO nicvo = new NicVO() { + + }; + + @Spy + NetworkModelImpl _networkModel = new NetworkModelImpl() { + @Override + public List getNics(long vmId) { + nic = new ArrayList(); + nicvo.setNetworkId(204L); + nic.add(nicvo); + return nic; + } + }; + + + LoadBalancingRulesManagerImpl _lbMgr = new LoadBalancingRulesManagerImpl(); + + private AssignToLoadBalancerRuleCmd assignToLbRuleCmd; + private ResponseGenerator responseGenerator; + private SuccessResponse successResponseGenerator; + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + private static long domainId = 5L; + private static long accountId = 5L; + private static String accountName = "admin"; + + @Before + public void setUp() { + assignToLbRuleCmd = new AssignToLoadBalancerRuleCmd() { + }; + + // ComponentContext.initComponentsLifeCycle(); + AccountVO account = new AccountVO(accountName, domainId, "networkDomain", Account.ACCOUNT_TYPE_NORMAL, "uuid"); + DomainVO domain = new DomainVO("rootDomain", 5L, 5L, "networkDomain"); + + UserVO user = new UserVO(1, "testuser", "password", "firstname", "lastName", "email", "timezone", UUID.randomUUID().toString()); + + CallContext.register(user, account); + + } + + @Test(expected = InvalidParameterValueException.class) + public void testBothArgsEmpty() throws ResourceAllocationException, ResourceUnavailableException, InsufficientCapacityException { + + Map> emptyMap = new HashMap>(); + + LoadBalancerDao lbdao = Mockito.mock(LoadBalancerDao.class); + _lbMgr._lbDao = lbdao; + + when(lbdao.findById(anyLong())).thenReturn(Mockito.mock(LoadBalancerVO.class)); + + _lbMgr.assignToLoadBalancer(1L, null, emptyMap); + + } + + @Test(expected = InvalidParameterValueException.class) + public void testNicIsNotInNw() throws ResourceAllocationException, ResourceUnavailableException, InsufficientCapacityException { + + Map> vmIdIpMap = new HashMap>(); + List secIp = new ArrayList(); + secIp.add("10.1.1.175"); + vmIdIpMap.put(1L,secIp); + + List vmIds = new ArrayList(); + vmIds.add(2L); + + LoadBalancerDao lbDao = Mockito.mock(LoadBalancerDao.class); + LoadBalancerVMMapDao lb2VmMapDao = Mockito.mock(LoadBalancerVMMapDao.class); + UserVmDao userVmDao = Mockito.mock(UserVmDao.class); + + _lbMgr._lbDao = lbDao; + _lbMgr._lb2VmMapDao = lb2VmMapDao; + _lbMgr._vmDao = userVmDao; + _lbvmMapList = new ArrayList<>(); + _lbMgr._rulesMgr = _rulesMgr; + _lbMgr._networkModel = _networkModel; + + when(lbDao.findById(anyLong())).thenReturn(Mockito.mock(LoadBalancerVO.class)); + when(userVmDao.findById(anyLong())).thenReturn(Mockito.mock(UserVmVO.class)); + when(lb2VmMapDao.listByLoadBalancerId(anyLong(), anyBoolean())).thenReturn(_lbvmMapList); + + _lbMgr.assignToLoadBalancer(1L, null, vmIdIpMap); + } + + + @Test(expected = InvalidParameterValueException.class) + public void tesSecIpNotSetToVm() throws ResourceAllocationException, ResourceUnavailableException, InsufficientCapacityException { + + AssignToLoadBalancerRuleCmd assignLbRuleCmd = Mockito.mock(AssignToLoadBalancerRuleCmd.class); + + Map> vmIdIpMap = new HashMap>(); + List secIp = new ArrayList(); + secIp.add("10.1.1.175"); + vmIdIpMap.put(1L,secIp); + + List vmIds = new ArrayList(); + vmIds.add(2L); + + LoadBalancerVO lbVO = new LoadBalancerVO("1", "L1", "Lbrule", 1, 22, 22, "rb", 204, 0, 0, "tcp"); + + LoadBalancerDao lbDao = Mockito.mock(LoadBalancerDao.class); + LoadBalancerVMMapDao lb2VmMapDao = Mockito.mock(LoadBalancerVMMapDao.class); + UserVmDao userVmDao = Mockito.mock(UserVmDao.class); + NicSecondaryIpDao nicSecIpDao = Mockito.mock(NicSecondaryIpDao.class); + + _lbMgr._lbDao = lbDao; + _lbMgr._lb2VmMapDao = lb2VmMapDao; + _lbMgr._vmDao = userVmDao; + _lbMgr._nicSecondaryIpDao = nicSecIpDao; + _lbvmMapList = new ArrayList<>(); + _lbMgr._rulesMgr = _rulesMgr; + _lbMgr._networkModel = _networkModel; + + when(lbDao.findById(anyLong())).thenReturn(lbVO); + when(userVmDao.findById(anyLong())).thenReturn(Mockito.mock(UserVmVO.class)); + when(lb2VmMapDao.listByLoadBalancerId(anyLong(), anyBoolean())).thenReturn(_lbvmMapList); + when (nicSecIpDao.findByIp4AddressAndNicId(anyString(), anyLong())).thenReturn(null); + + _lbMgr.assignToLoadBalancer(1L, null, vmIdIpMap); + } + + @After + public void tearDown() { + CallContext.unregister(); + } + +} \ No newline at end of file