bug 11462: 1) when delete PF rule, revoke corresponding firewall first (if exists) 2) never remove PF rule from the table when corresponding firewall rule wasn't removed yet

status 11462: resolved fixed

Reviewed-by: edison@cloud.com

Conflicts:

	server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java
This commit is contained in:
alena 2011-09-20 10:32:57 -07:00
parent 459b32cc6a
commit b6f58b77b8
8 changed files with 32 additions and 20 deletions

View File

@ -123,12 +123,13 @@ public class CreateIpForwardingRuleCmd extends BaseAsyncCreateCmd implements Sta
this.setResponseObject(fwResponse);
} finally {
if (!result || rule == null) {
_rulesService.revokeStaticNatRule(getEntityId(), true);
if (getOpenFirewall()) {
_firewallService.revokeRelatedFirewallRule(getEntityId(), true);
}
_rulesService.revokeStaticNatRule(getEntityId(), true);
throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Error in creating ip forwarding rule on the domr");
}
}

View File

@ -170,13 +170,13 @@ public class CreateLoadBalancerRuleCmd extends BaseAsyncCreateCmd /*implements
lbResponse.setResponseName(getCommandName());
} finally {
if (!success || rule == null) {
// no need to apply the rule on the backend as it exists in the db only
_lbService.deleteLoadBalancerRule(getEntityId(), false);
if (getOpenFirewall()) {
_firewallService.revokeRelatedFirewallRule(getEntityId(), true);
}
// no need to apply the rule on the backend as it exists in the db only
_lbService.deleteLoadBalancerRule(getEntityId(), false);
throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to create load balancer rule");
}
}

View File

@ -145,12 +145,13 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P
fwResponse.setResponseName(getCommandName());
} finally {
if (!success || rule == null) {
_rulesService.revokePortForwardingRule(getEntityId(), true);
if (getOpenFirewall()) {
_firewallService.revokeRelatedFirewallRule(getEntityId(), true);
}
_rulesService.revokePortForwardingRule(getEntityId(), true);
throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to apply port forwarding rule");
}
}

View File

@ -69,8 +69,8 @@ public class DeleteIpForwardingRuleCmd extends BaseAsyncCmd {
@Override
public void execute(){
UserContext.current().setEventDetails("Rule Id: "+id);
boolean result = _rulesService.revokeStaticNatRule(id, true);
result = result && _firewallService.revokeRelatedFirewallRule(id, true);
boolean result = _firewallService.revokeRelatedFirewallRule(id, true);
result = result && _rulesService.revokeStaticNatRule(id, true);
if (result) {
SuccessResponse response = new SuccessResponse(getCommandName());

View File

@ -84,8 +84,8 @@ public class DeleteLoadBalancerRuleCmd extends BaseAsyncCmd {
@Override
public void execute(){
UserContext.current().setEventDetails("Load balancer Id: "+getId());
boolean result = _lbService.deleteLoadBalancerRule(id, true);
result = result && _firewallService.revokeRelatedFirewallRule(id, true);
boolean result = _firewallService.revokeRelatedFirewallRule(id, true);
result = result && _lbService.deleteLoadBalancerRule(id, true);
if (result) {
SuccessResponse response = new SuccessResponse(getCommandName());

View File

@ -28,7 +28,6 @@ import com.cloud.api.ServerApiException;
import com.cloud.api.response.SuccessResponse;
import com.cloud.event.EventTypes;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.network.rules.PortForwardingRule;
import com.cloud.user.UserContext;
@ -90,9 +89,10 @@ public class DeletePortForwardingRuleCmd extends BaseAsyncCmd {
@Override
public void execute(){
UserContext.current().setEventDetails("Rule Id: "+id);
boolean result = _rulesService.revokePortForwardingRule(id, true);
result = result && _firewallService.revokeRelatedFirewallRule(id, true);
//revoke corresponding firewall rule first
boolean result = _firewallService.revokeRelatedFirewallRule(id, true);
result = result && _rulesService.revokePortForwardingRule(id, true);
if (result) {
SuccessResponse response = new SuccessResponse(getCommandName());
this.setResponseObject(response);

View File

@ -370,7 +370,12 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma
} else {
for (FirewallRule rule : rules) {
if (rule.getState() == FirewallRule.State.Revoke) {
_firewallDao.remove(rule.getId());
FirewallRuleVO relatedRule = _firewallDao.findByRelatedId(rule.getId());
if (relatedRule != null) {
s_logger.debug("Not removing the firewall rule id=" + rule.getId() + " as it has related firewall rule id=" + relatedRule.getId() + "; leaving it in Revoke state");
} else {
_firewallDao.remove(rule.getId());
}
} else if (rule.getState() == FirewallRule.State.Add) {
FirewallRuleVO ruleVO = _firewallDao.findById(rule.getId());
ruleVO.setState(FirewallRule.State.Active);

View File

@ -104,8 +104,6 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
@Inject
IPAddressDao _ipAddressDao;
@Inject
FirewallRulesDao _rulesDao;
@Inject
LoadBalancerDao _lbDao;
@Inject
VlanDao _vlanDao;
@ -350,7 +348,13 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
}
}
_rulesDao.remove(lb.getId());
FirewallRuleVO relatedRule = _firewallDao.findByRelatedId(lb.getId());
if (relatedRule != null) {
s_logger.debug("Not removing the firewall rule id=" + lb.getId() + " as it has related firewall rule id=" + relatedRule.getId() + "; leaving it in Revoke state");
} else {
_firewallDao.remove(lb.getId());
}
_elbMgr.handleDeleteLoadBalancerRule(lb, callerUserId, caller);
s_logger.debug("Load balancer with id " + lb.getId() + " is removed successfully");
return true;
@ -430,7 +434,7 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
try {
_firewallMgr.detectRulesConflict(newRule, ipAddr);
if (!_rulesDao.setStateToAdd(newRule)) {
if (!_firewallDao.setStateToAdd(newRule)) {
throw new CloudRuntimeException("Unable to update the state to add for " + newRule);
}
s_logger.debug("Load balancer " + newRule.getId() + " for Ip address id=" + ipId + ", public port " + srcPortStart + ", private port " + defPortStart + " is added successfully.");
@ -453,6 +457,7 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
_firewallMgr.revokeRelatedFirewallRule(newRule.getId(), false);
_lbDao.remove(newRule.getId());
txn.commit();
}
}
}
@ -528,7 +533,7 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
@Override
public boolean removeAllLoadBalanacersForIp(long ipId, Account caller, long callerUserId) {
List<FirewallRuleVO> rules = _rulesDao.listByIpAndPurposeAndNotRevoked(ipId, Purpose.LoadBalancing);
List<FirewallRuleVO> rules = _firewallDao.listByIpAndPurposeAndNotRevoked(ipId, Purpose.LoadBalancing);
if (rules != null)
s_logger.debug("Found " + rules.size() + " lb rules to cleanup");
for (FirewallRule rule : rules) {
@ -543,7 +548,7 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
@Override
public boolean removeAllLoadBalanacersForNetwork(long networkId, Account caller, long callerUserId) {
List<FirewallRuleVO> rules = _rulesDao.listByNetworkAndPurposeAndNotRevoked(networkId, Purpose.LoadBalancing);
List<FirewallRuleVO> rules = _firewallDao.listByNetworkAndPurposeAndNotRevoked(networkId, Purpose.LoadBalancing);
if (rules != null)
s_logger.debug("Found " + rules.size() + " lb rules to cleanup");
for (FirewallRule rule : rules) {