From 99b7b73d7f816d4c9fd23d6495ea50fafdac5d4e Mon Sep 17 00:00:00 2001 From: Sheng Yang Date: Tue, 7 Feb 2012 14:16:24 -0800 Subject: [PATCH] bug 12747: release ip when no static nat rule existed status 12747: resolved fixed --- .../com/cloud/network/NetworkManagerImpl.java | 33 +++++++++++++++++-- .../cloud/network/rules/RulesManagerImpl.java | 6 ++-- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java index 27ecf212ef9..8d1c9d1e37d 100644 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -670,9 +670,6 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag throw new CloudRuntimeException("Multiply generic source NAT IPs provided for network " + ip.getAssociatedWithNetworkId()); } } - if (ip.isOneToOneNat()) { - services.add(Service.StaticNat); - } ipToServices.put(ip, services); // if IP in allocating state then it will not have any rules attached so skip IPAssoc to network service @@ -683,10 +680,23 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag // check if any active rules are applied on the public IP Set purposes = getPublicIpPurposeInRules(ip, false, includingFirewall); + // Firewall rules didn't cover static NAT + if (ip.isOneToOneNat() && ip.getAssociatedWithVmId() != null) { + if (purposes == null) { + purposes = new HashSet(); + } + purposes.add(Purpose.StaticNat); + } if (purposes == null || purposes.isEmpty()) { // since no active rules are there check if any rules are applied on the public IP but are in // revoking state purposes = getPublicIpPurposeInRules(ip, true, includingFirewall); + if (ip.isOneToOneNat()) { + if (purposes == null) { + purposes = new HashSet(); + } + purposes.add(Purpose.StaticNat); + } if (purposes == null || purposes.isEmpty()) { // IP is not being used for any purpose so skip IPAssoc to network service provider continue; @@ -4185,6 +4195,23 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag } } + // For revoked static nat IP, set the vm_id to null, indicate it should be revoked + for (StaticNat staticNat : staticNats) { + if (staticNat.isForRevoke()) { + for (PublicIp publicIp : publicIps) { + if (publicIp.getId() == staticNat.getSourceIpAddressId()) { + publicIps.remove(publicIp); + IPAddressVO ip = _ipAddressDao.findByIdIncludingRemoved(staticNat.getSourceIpAddressId()); + // ip can't be null, otherwise something wrong happened + ip.setAssociatedWithVmId(null); + publicIp = new PublicIp(ip, _vlanDao.findById(ip.getVlanId()), NetUtils.createSequenceBasedMacAddress(ip.getMacAddress())); + publicIps.add(publicIp); + break; + } + } + } + } + // if all the rules configured on public IP are revoked then, dis-associate IP with network service provider applyIpAssociations(network, true, continueOnError, publicIps); diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index fee773e9121..af5a70086ba 100755 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -863,8 +863,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { revokeStaticNatRuleInternal(rule.getId(), caller, userId, false); } - // revoke static nat for the ip address - boolean success = applyStaticNatForIp(ipId, false, caller, true); + boolean success = true; // revoke all port forwarding rules success = success && applyPortForwardingRules(ipId, true, caller); @@ -872,6 +871,9 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { // revoke all all static nat rules success = success && applyStaticNatRules(ipId, true, caller); + // revoke static nat for the ip address + success = success && applyStaticNatForIp(ipId, false, caller, true); + // Now we check again in case more rules have been inserted. rules.addAll(_portForwardingDao.listByIpAndNotRevoked(ipId)); rules.addAll(_firewallDao.listByIpAndPurposeAndNotRevoked(ipId, Purpose.StaticNat));