From a907c1d09ffa116f5db4e443ce29514db569c9ea Mon Sep 17 00:00:00 2001 From: Chiradeep Vittal Date: Mon, 1 Nov 2010 23:46:08 -0700 Subject: [PATCH] bug 6807: split associateIp into single ip and ip list cases since it is impossible to assure correct operation with previous approach status 6807: resolved fixed --- .../com/cloud/network/dao/IPAddressDao.java | 2 +- .../cloud/network/dao/IPAddressDaoImpl.java | 8 +- scripts/network/domr/ipassoc.sh | 22 +++-- .../src/com/cloud/network/NetworkManager.java | 9 ++ .../com/cloud/network/NetworkManagerImpl.java | 98 +++++++++++++++---- .../cloud/server/ManagementServerImpl.java | 4 +- 6 files changed, 107 insertions(+), 36 deletions(-) diff --git a/core/src/com/cloud/network/dao/IPAddressDao.java b/core/src/com/cloud/network/dao/IPAddressDao.java index 45ec216f20b..e87e9e3558a 100644 --- a/core/src/com/cloud/network/dao/IPAddressDao.java +++ b/core/src/com/cloud/network/dao/IPAddressDao.java @@ -42,7 +42,7 @@ public interface IPAddressDao extends GenericDao { public int countIPs(long dcId, long vlanDbId, boolean onlyCountAllocated); - public int countIPs(long dcId, String vlanId, String vlanGateway, String vlanNetmask, boolean onlyCountAllocated); + public int countIPs(long dcId, Long accountId, String vlanId, String vlanGateway, String vlanNetmask); public boolean mark(long dcId, String ip); diff --git a/core/src/com/cloud/network/dao/IPAddressDaoImpl.java b/core/src/com/cloud/network/dao/IPAddressDaoImpl.java index 38f6de25ee9..f97c1c85e89 100644 --- a/core/src/com/cloud/network/dao/IPAddressDaoImpl.java +++ b/core/src/com/cloud/network/dao/IPAddressDaoImpl.java @@ -214,21 +214,19 @@ public class IPAddressDaoImpl extends GenericDaoBase implem } @Override @DB - public int countIPs(long dcId, String vlanId, String vlanGateway, String vlanNetmask, boolean onlyCountAllocated) { + public int countIPs(long dcId, Long accountId, String vlanId, String vlanGateway, String vlanNetmask) { Transaction txn = Transaction.currentTxn(); int ipCount = 0; try { - String sql = "SELECT count(*) FROM user_ip_address u INNER JOIN vlan v on (u.vlan_db_id = v.id AND v.data_center_id = ? AND v.vlan_id = ? AND v.vlan_gateway = ? AND v.vlan_netmask = ?)"; + String sql = "SELECT count(*) FROM user_ip_address u INNER JOIN vlan v on (u.vlan_db_id = v.id AND v.data_center_id = ? AND v.vlan_id = ? AND v.vlan_gateway = ? AND v.vlan_netmask = ? AND u.account_id = ?)"; - if (onlyCountAllocated) { - sql += " WHERE allocated IS NOT NULL"; - } PreparedStatement pstmt = txn.prepareAutoCloseStatement(sql); pstmt.setLong(1, dcId); pstmt.setString(2, vlanId); pstmt.setString(3, vlanGateway); pstmt.setString(4, vlanNetmask); + pstmt.setLong(5, accountId); ResultSet rs = pstmt.executeQuery(); if (rs.next()) { diff --git a/scripts/network/domr/ipassoc.sh b/scripts/network/domr/ipassoc.sh index 192b91c0708..0d7551f758e 100755 --- a/scripts/network/domr/ipassoc.sh +++ b/scripts/network/domr/ipassoc.sh @@ -3,7 +3,7 @@ # ipassoc.sh -- associate/disassociate a public ip with an instance # # -# @VERSION@ +# 2.1.4 usage() { printf "Usage:\n %s -A -i -l -r [-f] \n" $(basename $0) >&2 printf " %s -D -i -l -r [-f] \n" $(basename $0) >&2 @@ -54,10 +54,11 @@ check_gw() { add_nat_entry() { local dRIp=$1 local pubIp=$2 + local ipNoMask=$(echo $2 | awk -F'/' '{print $1}') ssh -p 3922 -o StrictHostKeyChecking=no -i $cert root@$dRIp "\ ip addr add dev $correctVif $pubIp - iptables -t nat -I POSTROUTING -j SNAT -o $correctVif --to-source $pubIp ; - /sbin/arping -c 3 -I $correctVif -A -U -s $pubIp $pubIp; + iptables -t nat -I POSTROUTING -j SNAT -o $correctVif --to-source $ipNoMask ; + /sbin/arping -c 3 -I $correctVif -A -U -s $ipNoMask $ipNoMask; " if [ $? -gt 0 -a $? -ne 2 ] then @@ -71,9 +72,12 @@ add_nat_entry() { del_nat_entry() { local dRIp=$1 local pubIp=$2 + local ipNoMask=$(echo $2 | awk -F'/' '{print $1}') + local mask=$(echo $2 | awk -F'/' '{print $2}') + [ "$mask" == "" ] && mask="32" ssh -p 3922 -o StrictHostKeyChecking=no -i $cert root@$dRIp "\ - iptables -t nat -D POSTROUTING -j SNAT -o $correctVif --to-source $pubIp; - ip addr del dev $correctVif $pubIp/32 + iptables -t nat -D POSTROUTING -j SNAT -o $correctVif --to-source $ipNoMask; + ip addr del dev $correctVif "$ipNoMask/$mask" " if [ $? -gt 0 -a $? -ne 2 ] @@ -88,10 +92,11 @@ del_nat_entry() { add_an_ip () { local dRIp=$1 local pubIp=$2 + local ipNoMask=$(echo $2 | awk -F'/' '{print $1}') ssh -p 3922 -o StrictHostKeyChecking=no -i $cert root@$dRIp "\ ifconfig $correctVif up; ip addr add dev $correctVif $pubIp ; - /sbin/arping -c 3 -I $correctVif -A -U -s $pubIp $pubIp; + /sbin/arping -c 3 -I $correctVif -A -U -s $ipNoMask $ipNoMask; " return $? } @@ -99,8 +104,11 @@ add_an_ip () { remove_an_ip () { local dRIp=$1 local pubIp=$2 + local ipNoMask=$(echo $2 | awk -F'/' '{print $1}') + local mask=$(echo $2 | awk -F'/' '{print $2}') + [ "$mask" == "" ] && mask="32" ssh -p 3922 -o StrictHostKeyChecking=no -i $cert root@$dRIp "\ - ip addr del dev $correctVif $pubIp/32 + ip addr del dev $correctVif "$ipNoMask/$mask" " if [ $? -gt 0 -a $? -ne 2 ] then diff --git a/server/src/com/cloud/network/NetworkManager.java b/server/src/com/cloud/network/NetworkManager.java index 0e67108301f..cbd3fde5bd7 100644 --- a/server/src/com/cloud/network/NetworkManager.java +++ b/server/src/com/cloud/network/NetworkManager.java @@ -174,6 +174,15 @@ public interface NetworkManager extends Manager { */ boolean associateIP(DomainRouterVO router, List ipAddrList, boolean add) throws ResourceAllocationException; + /** + * Associates or disassociates a single IP address for a router. + * @param router router object to send the association to + * @param ipAddress public IP addresses + * @param add true if associate, false if disassociate + * @return + */ + boolean associateIP(DomainRouterVO router, String ipAddress, boolean add) throws ResourceAllocationException; + boolean updateFirewallRule(FirewallRuleVO fwRule, String oldPrivateIP, String oldPrivatePort); boolean executeAssignToLoadBalancer(AssignToLoadBalancerExecutor executor, LoadBalancerParam param); diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java index 7cbac7738a6..b30483e07a6 100644 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -18,6 +18,9 @@ package com.cloud.network; import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -97,6 +100,7 @@ import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor; +import com.cloud.network.IpAddrAllocator.networkInfo; import com.cloud.network.dao.FirewallRulesDao; import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.LoadBalancerDao; @@ -1286,26 +1290,44 @@ public class NetworkManagerImpl implements NetworkManager, VirtualMachineManager final Command [] cmds = new Command[ipAddrList.size()]; int i=0; boolean sourceNat = false; + Map> vlanIpMap = new HashMap>(); for (final String ipAddress: ipAddrList) { - if (ipAddress.equalsIgnoreCase(router.getPublicIpAddress())) - sourceNat=true; - IPAddressVO ip = _ipAddressDao.findById(ipAddress); + VlanVO vlan = _vlanDao.findById(ip.getVlanDbId()); - String vlanId = vlan.getVlanId(); - String vlanGateway = vlan.getVlanGateway(); - String vlanNetmask = vlan.getVlanNetmask(); - boolean firstIP = (!sourceNat && (_ipAddressDao.countIPs(vlan.getDataCenterId(), vlan.getVlanId(), vlan.getVlanGateway(), vlan.getVlanNetmask(), true) == 1)); - - String vifMacAddress = null; - if (firstIP) { - String[] macAddresses = _dcDao.getNextAvailableMacAddressPair(ip.getDataCenterId()); - vifMacAddress = macAddresses[1]; - } + ArrayList ipList = vlanIpMap.get(vlan.getId()); + if (ipList == null) { + ipList = new ArrayList(); + } + ipList.add(ip); + vlanIpMap.put(vlan, ipList); + } + for (Map.Entry> vlanAndIp: vlanIpMap.entrySet()) { + boolean firstIP = true; + ArrayList ipList = vlanAndIp.getValue(); + Collections.sort(ipList, new Comparator() { + @Override + public int compare(IPAddressVO o1, IPAddressVO o2) { + return o1.getAddress().compareTo(o2.getAddress()); + } }); - cmds[i++] = new IPAssocCommand(router.getInstanceName(), router.getPrivateIpAddress(), ipAddress, add, firstIP, sourceNat, vlanId, vlanGateway, vlanNetmask, vifMacAddress); - - sourceNat = false; + for (final IPAddressVO ip: ipList) { + sourceNat = ip.getSourceNat(); + VlanVO vlan = vlanAndIp.getKey(); + String vlanId = vlan.getVlanId(); + String vlanGateway = vlan.getVlanGateway(); + String vlanNetmask = vlan.getVlanNetmask(); + + String vifMacAddress = null; + if (firstIP && add) { + String[] macAddresses = _dcDao.getNextAvailableMacAddressPair(ip.getDataCenterId()); + vifMacAddress = macAddresses[1]; + } + + cmds[i++] = new IPAssocCommand(router.getInstanceName(), router.getPrivateIpAddress(), ip.getAddress(), add, firstIP, sourceNat, vlanId, vlanGateway, vlanNetmask, vifMacAddress); + + firstIP = false; + } } Answer[] answers = null; @@ -1326,13 +1348,49 @@ public class NetworkManagerImpl implements NetworkManager, VirtualMachineManager if (answers.length != ipAddrList.size()) { return false; } - + boolean result = true; for (int i1=0; i1 < answers.length; i1++) { Answer ans = answers[i1]; - return ans.getResult(); + result = result && ans.getResult(); } - return true; + return result; + } + + public boolean associateIP(final DomainRouterVO router, final String ipAddress, final boolean add) { + IPAddressVO ip = _ipAddressDao.findById(ipAddress); + VlanVO vlan = _vlanDao.findById(ip.getVlanDbId()); + boolean sourceNat = ip.isSourceNat(); + boolean firstIP = (!sourceNat && (_ipAddressDao.countIPs(vlan.getDataCenterId(), router.getAccountId(), vlan.getVlanId(), vlan.getVlanGateway(), vlan.getVlanNetmask()) == 1)); + String vlanId = vlan.getVlanId(); + String vlanGateway = vlan.getVlanGateway(); + String vlanNetmask = vlan.getVlanNetmask(); + String vifMacAddress = null; + if (firstIP && add) { + String[] macAddresses = _dcDao.getNextAvailableMacAddressPair(ip.getDataCenterId()); + vifMacAddress = macAddresses[1]; + } + IPAssocCommand cmd = new IPAssocCommand(router.getInstanceName(), router.getPrivateIpAddress(), ip.getAddress(), add, firstIP, sourceNat, vlanId, vlanGateway, vlanNetmask, vifMacAddress); + Answer[] answers = null; + try { + answers = _agentMgr.send(router.getHostId(), new Command[]{cmd}, false); + } catch (final AgentUnavailableException e) { + s_logger.warn("Agent unavailable", e); + return false; + } catch (final OperationTimedoutException e) { + s_logger.warn("Timed Out", e); + return false; + } + + if (answers == null) { + return false; + } + + if (answers.length != 1) { + return false; + } + return answers[0].getResult(); + } @Override @@ -1645,7 +1703,7 @@ public class NetworkManagerImpl implements NetworkManager, VirtualMachineManager s_logger.debug("Disassociate ip " + router.getName()); } - if (associateIP(router, ipAddrs, false)) { + if (associateIP(router, ip.getAddress(), false)) { _ipAddressDao.unassignIpAddress(ipAddress); } else { if (s_logger.isDebugEnabled()) { diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index 6e07376e110..105247d689b 100755 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -1473,7 +1473,6 @@ public class ManagementServerImpl implements ManagementServer { // Associate the IP's to DomR boolean success = true; String params = "\nsourceNat=" + false + "\ndcId=" + zoneId; - ArrayList dummyipAddrList = new ArrayList(); success = _networkMgr.associateIP(router,ipAddrsList, true); String errorMsg = "Unable to assign public IP address pool"; if (!success) { @@ -1557,7 +1556,7 @@ public class ManagementServerImpl implements ManagementServer { ipAddrs.add(ipAddress); if (router.getState() == State.Running) { - success = _networkMgr.associateIP(router, ipAddrs, true); + success = _networkMgr.associateIP(router, ipAddress, true); if (!success) { errorMsg = "Unable to assign public IP address."; } @@ -2415,7 +2414,6 @@ public class ManagementServerImpl implements ManagementServer { _vmInstanceDao.update(started.getId(), updatedInstance); started = _userVmDao.findById(started.getId()); } - String params = "\nsourceNat=" + false + "\ndcId=" + dc.getId(); try { associateIpAddressListToAccount(userId, accountId, dc.getId(),null); } catch (InsufficientAddressCapacityException e) {