From 78c9db79dae3bc38699050b05baae32fd02204f6 Mon Sep 17 00:00:00 2001 From: Alena Prokharchyk Date: Fri, 12 Apr 2013 10:05:28 -0700 Subject: [PATCH] InternalLbVm: destroy the internal lb vm when the last rule for the ip is being revoked --- .../element/InternalLoadBalancerElement.java | 105 +++++++++++++----- .../lb/InternalLoadBalancerManager.java | 5 +- .../lb/InternalLoadBalancerManagerImpl.java | 28 +++-- .../lb/LoadBalancingRulesManagerImpl.java | 2 +- 4 files changed, 97 insertions(+), 43 deletions(-) diff --git a/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/element/InternalLoadBalancerElement.java b/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/element/InternalLoadBalancerElement.java index f3fb00f7eb6..0c662a8a999 100644 --- a/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/element/InternalLoadBalancerElement.java +++ b/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/element/InternalLoadBalancerElement.java @@ -19,6 +19,7 @@ package org.apache.cloudstack.network.element; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -64,7 +65,9 @@ import com.cloud.network.router.VirtualRouter.Role; import com.cloud.network.rules.FirewallRule; import com.cloud.network.rules.LoadBalancerContainer; import com.cloud.offering.NetworkOffering; +import com.cloud.user.Account; import com.cloud.user.AccountManager; +import com.cloud.user.User; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.db.SearchCriteria.Op; import com.cloud.utils.db.SearchCriteria2; @@ -162,13 +165,13 @@ public class InternalLoadBalancerElement extends AdapterBase implements LoadBala boolean result = true; for (DomainRouterVO internalLbVm : internalLbVms) { result = result && _internalLbMgr.destroyInternalLbVm(internalLbVm.getId(), - context.getAccount(), context.getCaller().getId()) != null; + context.getAccount(), context.getCaller().getId()); if (cleanup) { if (!result) { s_logger.warn("Failed to stop internal lb element " + internalLbVm + ", but would try to process clean up anyway."); } result = (_internalLbMgr.destroyInternalLbVm(internalLbVm.getId(), - context.getAccount(), context.getCaller().getId()) != null); + context.getAccount(), context.getCaller().getId())); if (!result) { s_logger.warn("Failed to clean up internal lb element " + internalLbVm); } @@ -186,7 +189,7 @@ public class InternalLoadBalancerElement extends AdapterBase implements LoadBala boolean result = true; for (DomainRouterVO internalLbVm : internalLbVms) { result = result && (_internalLbMgr.destroyInternalLbVm(internalLbVm.getId(), - context.getAccount(), context.getCaller().getId()) != null); + context.getAccount(), context.getCaller().getId())); } return result; } @@ -214,7 +217,7 @@ public class InternalLoadBalancerElement extends AdapterBase implements LoadBala boolean result = true; for (DomainRouterVO internalLbVm : internalLbVms) { result = result && (_internalLbMgr.destroyInternalLbVm(internalLbVm.getId(), - context.getAccount(), context.getCaller().getId()) != null); + context.getAccount(), context.getCaller().getId())); } _vrProviderDao.remove(elementId); @@ -240,31 +243,47 @@ public class InternalLoadBalancerElement extends AdapterBase implements LoadBala public boolean applyLBRules(Network network, List rules) throws ResourceUnavailableException { Map> rulesToApply = getLbRulesToApply(rules); + Set vmsToDestroy = getVmsToDestroy(rules); for (Ip sourceIp : rulesToApply.keySet()) { - //2.1 Start Internal LB vm per IP address - List internalLbVms; - try { - DeployDestination dest = new DeployDestination(_configMgr.getZone(network.getDataCenterId()), null, null, null); - internalLbVms = _internalLbMgr.deployInternalLbVm(network, sourceIp, dest, _accountMgr.getAccount(network.getAccountId()), null); - } catch (InsufficientCapacityException e) { - s_logger.warn("Failed to apply lb rule(s) on the element " + this.getName() + " due to:", e); - return false; - } catch (ConcurrentOperationException e) { - s_logger.warn("Failed to apply lb rule(s) on the element " + this.getName() + " due to:", e); - return false; - } - - if ((internalLbVms == null) || (internalLbVms.size() == 0)) { - throw new ResourceUnavailableException("Can't find/deploy internal lb vm to handle LB rules", - DataCenter.class, network.getDataCenterId()); - } - - //2.2 Apply Internal LB rules on the VM - if (!_internalLbMgr.applyLoadBalancingRules(network, rules, internalLbVms)) { - throw new CloudRuntimeException("Failed to apply load balancing rules in network " + network.getId() + " on element " + this.getName()); + if (vmsToDestroy.contains(sourceIp)) { + //2.1 Destroy internal lb vm + List vms = _internalLbMgr.findInternalLbVms(network.getId(), sourceIp); + //only one internal lb per IP exists + try { + s_logger.debug("Destroying internal lb vm for ip " + sourceIp.addr() + " as all the rules for this vm are in Revoke state"); + return _internalLbMgr.destroyInternalLbVm(vms.get(0).getId(), _accountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM), + _accountMgr.getUserIncludingRemoved(User.UID_SYSTEM).getId()); + } catch (ConcurrentOperationException e) { + s_logger.warn("Failed to apply lb rule(s) for ip " + sourceIp.addr() + " on the element " + this.getName() + " due to:", e); + return false; + } } else { - return true; + //2.2 Start Internal LB vm per IP address + List internalLbVms; + try { + DeployDestination dest = new DeployDestination(_configMgr.getZone(network.getDataCenterId()), null, null, null); + internalLbVms = _internalLbMgr.deployInternalLbVm(network, sourceIp, dest, _accountMgr.getAccount(network.getAccountId()), null); + } catch (InsufficientCapacityException e) { + s_logger.warn("Failed to apply lb rule(s) for ip " + sourceIp.addr() + "on the element " + this.getName() + " due to:", e); + return false; + } catch (ConcurrentOperationException e) { + s_logger.warn("Failed to apply lb rule(s) for ip " + sourceIp.addr() + "on the element " + this.getName() + " due to:", e); + return false; + } + + if ((internalLbVms == null) || (internalLbVms.size() == 0)) { + throw new ResourceUnavailableException("Can't find/deploy internal lb vm to handle LB rules", + DataCenter.class, network.getDataCenterId()); + } + + //2.3 Apply Internal LB rules on the VM + if (!_internalLbMgr.applyLoadBalancingRules(network, rulesToApply.get(sourceIp), internalLbVms)) { + throw new CloudRuntimeException("Failed to apply load balancing rules for ip " + sourceIp.addr() + + " in network " + network.getId() + " on element " + this.getName()); + } else { + return true; + } } } @@ -275,7 +294,7 @@ public class InternalLoadBalancerElement extends AdapterBase implements LoadBala //1) Group rules by the source ip address as NetworkManager always passes the entire network lb config to the element Map> groupedRules = groupBySourceIp(rules); - //2) Apply only set containing LB rules in transition state (Add/Revoke) + //2) Apply only sets containing LB rules in transition state (Add/Revoke). Map> rulesToApply = new HashMap>(); for (Ip sourceIp : groupedRules.keySet()) { @@ -287,14 +306,42 @@ public class InternalLoadBalancerElement extends AdapterBase implements LoadBala break; } } + if (apply) { rulesToApply.put(sourceIp, rulesToCheck); } else { - s_logger.debug("Not applying the lb rules for soure ip " + sourceIp + " on element " + this.getName() + " as there are no rules in transition state"); + s_logger.debug("Not applying the lb rules for soure ip " + sourceIp + " on element " + this.getName() + + " as there are no rules in transition state"); } - } + } return rulesToApply; } + + + + protected Set getVmsToDestroy(List rules) { + //1) Group rules by the source ip address as NetworkManager always passes the entire network lb config to the element + Map> groupedRules = groupBySourceIp(rules); + + //2) Count rules in revoke state + Set vmsToDestroy = new HashSet(); + + for (Ip sourceIp : groupedRules.keySet()) { + List rulesToCheck = groupedRules.get(sourceIp); + int revoke = 0; + for (LoadBalancingRule ruleToCheck : rulesToCheck) { + if (ruleToCheck.getState() == FirewallRule.State.Revoke){ + revoke++; + } + } + + if (revoke == rulesToCheck.size()) { + s_logger.debug("Have to destroy internal lb vm for source ip " + sourceIp); + vmsToDestroy.add(sourceIp); + } + } + return vmsToDestroy; + } protected Map> groupBySourceIp(List rules) { Map> groupedRules = new HashMap>(); diff --git a/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerManager.java b/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerManager.java index fd2228544f2..0d817deb53d 100644 --- a/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerManager.java +++ b/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerManager.java @@ -44,7 +44,7 @@ public interface InternalLoadBalancerManager extends Manager{ * @throws ResourceUnavailableException * @throws ConcurrentOperationException */ - VirtualRouter destroyInternalLbVm(long vmId, Account caller, Long callerUserId) + boolean destroyInternalLbVm(long vmId, Account caller, Long callerUserId) throws ResourceUnavailableException, ConcurrentOperationException; @@ -107,4 +107,7 @@ public interface InternalLoadBalancerManager extends Manager{ boolean applyLoadBalancingRules(Network network, List rules, List internalLbVms) throws ResourceUnavailableException; + + List findInternalLbVms(long guestNetworkId, Ip requestedGuestIp); + } diff --git a/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerManagerImpl.java b/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerManagerImpl.java index d45784e3503..c371892765a 100644 --- a/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerManagerImpl.java +++ b/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerManagerImpl.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.network.lb; +import java.lang.Thread.State; import java.util.ArrayList; import java.util.Iterator; import java.util.List; @@ -522,7 +523,7 @@ InternalLoadBalancerManager, VirtualMachineGuru { } @Override - public VirtualRouter destroyInternalLbVm(long vmId, Account caller, Long callerUserId) + public boolean destroyInternalLbVm(long vmId, Account caller, Long callerUserId) throws ResourceUnavailableException, ConcurrentOperationException { if (s_logger.isDebugEnabled()) { s_logger.debug("Attempting to destroy Internal LB vm " + vmId); @@ -530,17 +531,12 @@ InternalLoadBalancerManager, VirtualMachineGuru { DomainRouterVO internalLbVm = _routerDao.findById(vmId); if (internalLbVm == null) { - return null; + return true; } _accountMgr.checkAccess(caller, null, true, internalLbVm); - boolean result = _itMgr.expunge(internalLbVm, _accountMgr.getActiveUser(callerUserId), caller); - - if (result) { - return internalLbVm; - } - return null; + return _itMgr.expunge(internalLbVm, _accountMgr.getActiveUser(callerUserId), caller); } @Override @@ -582,12 +578,13 @@ InternalLoadBalancerManager, VirtualMachineGuru { } for (DomainRouterVO internalLbVm : internalLbVms) { - internalLbVm = startInternalLbVm(internalLbVm, _accountMgr.getSystemUser(), _accountMgr.getSystemAccount(), params); + if (internalLbVm.getState() != VirtualMachine.State.Running) { + internalLbVm = startInternalLbVm(internalLbVm, _accountMgr.getSystemUser(), _accountMgr.getSystemAccount(), params); + } if (internalLbVm != null) { runningInternalLbVms.add(internalLbVm); } - } return runningInternalLbVms; } @@ -699,6 +696,14 @@ InternalLoadBalancerManager, VirtualMachineGuru { protected Pair> getDeploymentPlanAndInternalLbVms(DeployDestination dest, long guestNetworkId, Ip requestedGuestIp) { long dcId = dest.getDataCenter().getId(); DeploymentPlan plan = new DataCenterDeployment(dcId); + List internalLbVms = findInternalLbVms(guestNetworkId, requestedGuestIp); + + return new Pair>(plan, internalLbVms); + + } + + @Override + public List findInternalLbVms(long guestNetworkId, Ip requestedGuestIp) { List internalLbVms = _routerDao.listByNetworkAndRole(guestNetworkId, Role.INTERNAL_LB_VM); if (requestedGuestIp != null) { Iterator it = internalLbVms.iterator(); @@ -710,8 +715,7 @@ InternalLoadBalancerManager, VirtualMachineGuru { } } } - - return new Pair>(plan, internalLbVms); + return internalLbVms; } diff --git a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java index 0a9c5b65c39..59ff43529d5 100755 --- a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java +++ b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java @@ -1602,7 +1602,7 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements } txn.commit(); - if (checkForReleaseElasticIp) { + if (checkForReleaseElasticIp && lb.getSourceIpAddressId() != null) { boolean success = true; long count = _firewallDao.countRulesByIpId(lb.getSourceIpAddressId()); if (count == 0) {