From 8ef52325e704e2923a107d20f873b828526f7427 Mon Sep 17 00:00:00 2001 From: Jayapal Date: Tue, 7 Feb 2017 11:40:53 +0530 Subject: [PATCH] CLOUDSTACK-9317: When there 1 static nat removing PF rules is handled There is 1 static nat rule and 2 PF rule. Removing 2 PF rules was deleting static nat rule. Fixed this issue in this commit. --- api/src/com/cloud/network/IpAddress.java | 4 ++++ .../agent/api/routing/NetworkElementCommand.java | 2 +- .../src/com/cloud/network/addr/PublicIp.java | 9 +++++++++ .../src/com/cloud/network/dao/IPAddressVO.java | 14 ++++++++++++++ .../cloud/network/router/CommandSetupHelper.java | 12 ++++-------- .../com/cloud/network/rules/RulesManagerImpl.java | 8 ++++++++ setup/db/db/schema-4920to41000.sql | 1 + 7 files changed, 41 insertions(+), 9 deletions(-) diff --git a/api/src/com/cloud/network/IpAddress.java b/api/src/com/cloud/network/IpAddress.java index 2061771ed37..2447809d66f 100644 --- a/api/src/com/cloud/network/IpAddress.java +++ b/api/src/com/cloud/network/IpAddress.java @@ -92,4 +92,8 @@ public interface IpAddress extends ControlledEntity, Identity, InternalIdentity, public Date getCreated(); + State getRuleState(); + + void setRuleState(State ruleState); + } diff --git a/core/src/com/cloud/agent/api/routing/NetworkElementCommand.java b/core/src/com/cloud/agent/api/routing/NetworkElementCommand.java index a23b1a3d3e7..a7b5b5e6d17 100644 --- a/core/src/com/cloud/agent/api/routing/NetworkElementCommand.java +++ b/core/src/com/cloud/agent/api/routing/NetworkElementCommand.java @@ -38,7 +38,7 @@ public abstract class NetworkElementCommand extends Command { public static final String VPC_PRIVATE_GATEWAY = "vpc.gateway.private"; public static final String FIREWALL_EGRESS_DEFAULT = "firewall.egress.default"; public static final String ROUTER_MONITORING_ENABLE = "router.monitor.enable"; - public static final String NETWORK_PUB_LAST_IP = "newtork.public.last.ip"; + public static final String NETWORK_PUB_LAST_IP = "network.public.last.ip"; private String routerAccessIp; diff --git a/engine/components-api/src/com/cloud/network/addr/PublicIp.java b/engine/components-api/src/com/cloud/network/addr/PublicIp.java index f6bf73475de..8e9d7995ad8 100644 --- a/engine/components-api/src/com/cloud/network/addr/PublicIp.java +++ b/engine/components-api/src/com/cloud/network/addr/PublicIp.java @@ -255,4 +255,13 @@ public class PublicIp implements PublicIpAddress { return IpAddress.class; } + @Override + public State getRuleState() { + return _addr.getRuleState(); + } + + @Override + public void setRuleState(State ruleState) { + _addr.setRuleState(ruleState); + } } diff --git a/engine/schema/src/com/cloud/network/dao/IPAddressVO.java b/engine/schema/src/com/cloud/network/dao/IPAddressVO.java index 0be781b500b..b5aed3f0dc6 100644 --- a/engine/schema/src/com/cloud/network/dao/IPAddressVO.java +++ b/engine/schema/src/com/cloud/network/dao/IPAddressVO.java @@ -117,6 +117,10 @@ public class IPAddressVO implements IpAddress { @Column(name = "display", updatable = true, nullable = false) protected boolean display = true; + @Enumerated(value = EnumType.STRING) + @Column(name = "rule_state") + State ruleState; + @Column(name= GenericDao.REMOVED_COLUMN) private Date removed; @@ -367,4 +371,14 @@ public class IPAddressVO implements IpAddress { public Date getCreated() { return created; } + + @Override + public State getRuleState() { + return ruleState; + } + + @Override + public void setRuleState(State ruleState) { + this.ruleState = ruleState; + } } diff --git a/server/src/com/cloud/network/router/CommandSetupHelper.java b/server/src/com/cloud/network/router/CommandSetupHelper.java index e1d5ef7b7fe..232c908d180 100644 --- a/server/src/com/cloud/network/router/CommandSetupHelper.java +++ b/server/src/com/cloud/network/router/CommandSetupHelper.java @@ -858,7 +858,8 @@ public class CommandSetupHelper { ipsWithrules++; } - if (ip.isOneToOneNat()) { + // check onetoonenat and also check if the ip "add":false. If there are 2 PF remove 1 static nat add + if (ip.isOneToOneNat() && ip.getRuleState() == null) { ipsStaticNat++; } } @@ -870,13 +871,8 @@ public class CommandSetupHelper { final DataCenterVO dcVo = _dcDao.findById(router.getDataCenterId()); cmd.setAccessDetail(NetworkElementCommand.ZONE_NETWORK_TYPE, dcVo.getNetworkType().toString()); - boolean remove = false; - // if there is only one static nat then it will be checked for remove at the resource - if (ipsWithrules == 0 && (ipsStaticNat == 0 || ipsStaticNat == 1)) { - remove = true; - } - - if (remove) { + // if there 1 static nat then it will be checked for remove at the resource + if (ipsWithrules == 0 && ipsStaticNat == 0 ) { // there is only one ip address for the network. cmd.setAccessDetail(NetworkElementCommand.NETWORK_PUB_LAST_IP, "true"); } diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index 5cfebba168a..629e35f5ba3 100644 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -1259,6 +1259,10 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules throw ex; } + ipAddress.setRuleState(IpAddress.State.Releasing); + _ipAddressDao.update(ipAddress.getId(), ipAddress); + ipAddress = _ipAddressDao.findById(ipId); + // Revoke all firewall rules for the ip try { s_logger.debug("Revoking all " + Purpose.Firewall + "rules as a part of disabling static nat for public IP id=" + ipId); @@ -1280,6 +1284,7 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules boolean isIpSystem = ipAddress.getSystem(); ipAddress.setOneToOneNat(false); ipAddress.setAssociatedWithVmId(null); + ipAddress.setRuleState(null); ipAddress.setVmIp(null); if (isIpSystem && !releaseIpIfElastic) { ipAddress.setSystem(false); @@ -1295,6 +1300,9 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules return true; } else { s_logger.warn("Failed to disable one to one nat for the ip address id" + ipId); + ipAddress = _ipAddressDao.findById(ipId); + ipAddress.setRuleState(null); + _ipAddressDao.update(ipAddress.getId(), ipAddress); return false; } } diff --git a/setup/db/db/schema-4920to41000.sql b/setup/db/db/schema-4920to41000.sql index 8718cc8b013..2b9afc3b7a8 100644 --- a/setup/db/db/schema-4920to41000.sql +++ b/setup/db/db/schema-4920to41000.sql @@ -245,3 +245,4 @@ CREATE TABLE `cloud`.`guest_os_details` ( CONSTRAINT `fk_guest_os_details__guest_os_id` FOREIGN KEY `fk_guest_os_details__guest_os_id`(`guest_os_id`) REFERENCES `guest_os`(`id`) ON DELETE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8; +ALTER TABLE `user_ip_address` ADD COLUMN `rule_state` VARCHAR(32) COMMENT 'static rule state while removing'; \ No newline at end of file