Skip to content

CLOUDSTACK-9756: Configure to ignore the ipassoc failure #1917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
karuturi merged 1 commit into from Apr 22, 2017
Merged

CLOUDSTACK-9756: Configure to ignore the ipassoc failure #1917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions engine/components-api/src/com/cloud/network/IpAddressManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ public interface IpAddressManager {
"If true, when account has dedicated public ip range(s), once the ips dedicated to the account have been consumed ips will be acquired from the system pool",
true, ConfigKey.Scope.Account);

static final ConfigKey<Boolean> RulesContinueOnError = new ConfigKey<Boolean>("Advanced", Boolean.class, "network.rule.delete.ignoreerror", "true",
Copy link
Copy Markdown

@srinivas-gandikota srinivas-gandikota Mar 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default for RulesContinueOnError is set to true, means the errors will be ignored.
Shouldn't the default be set to false ?

Copy link
Copy Markdown
Contributor Author

@jayapalu jayapalu Mar 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To continue with the existing behavior it is set to true. If some one want to enable they can set to false.

Copy link
Copy Markdown

@srinivas-gandikota srinivas-gandikota Mar 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sound appropriate.

"When true, ip address delete (ipassoc) failures are ignored", true);

/**
* Assigns a new public ip address.
*
Expand Down
12 changes: 9 additions & 3 deletions server/src/com/cloud/network/IpAddressManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,8 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
SearchBuilder<IPAddressVO> AssignIpAddressSearch;
SearchBuilder<IPAddressVO> AssignIpAddressFromPodVlanSearch;

static Boolean rulesContinueOnErrFlag = true;

@Override
public boolean configure(String name, Map<String, Object> params) {
// populate providers
Expand Down Expand Up @@ -403,7 +405,11 @@ public boolean configure(String name, Map<String, Object> params) {

Network.State.getStateMachine().registerListener(new NetworkStateListener(_configDao));

s_logger.info("Network Manager is configured.");
if (RulesContinueOnError.value() != null) {
rulesContinueOnErrFlag = RulesContinueOnError.value();
}

s_logger.info("IPAddress Manager is configured.");

return true;
}
Expand Down Expand Up @@ -601,7 +607,7 @@ public boolean disassociatePublicIpAddress(long addrId, long userId, Account cal
if (ip.getAssociatedWithNetworkId() != null) {
Network network = _networksDao.findById(ip.getAssociatedWithNetworkId());
try {
if (!applyIpAssociations(network, true)) {
if (!applyIpAssociations(network, rulesContinueOnErrFlag)) {
s_logger.warn("Unable to apply ip address associations for " + network);
success = false;
}
Expand Down Expand Up @@ -2026,6 +2032,6 @@ public String getConfigComponentName() {

@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {UseSystemPublicIps};
return new ConfigKey<?>[] {UseSystemPublicIps, RulesContinueOnError};
}
}
12 changes: 10 additions & 2 deletions server/src/com/cloud/network/firewall/FirewallManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,16 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService,
IpAddressManager _ipAddrMgr;

private boolean _elbEnabled = false;
static Boolean rulesContinueOnErrFlag = true;

@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
_name = name;
String elbEnabledString = _configDao.getValue(Config.ElasticLoadBalancerEnabled.key());
_elbEnabled = Boolean.parseBoolean(elbEnabledString);
if (_ipAddrMgr.RulesContinueOnError.value() != null) {
rulesContinueOnErrFlag = _ipAddrMgr.RulesContinueOnError.value();
}
return true;
}

Expand Down Expand Up @@ -851,8 +855,12 @@ public boolean revokeFirewallRulesForIp(long ipId, long userId, Account caller)

// now send everything to the backend
List<FirewallRuleVO> rulesToApply = _firewallDao.listByIpAndPurpose(ipId, Purpose.Firewall);
applyFirewallRules(rulesToApply, true, caller);

//apply rules
if (!applyFirewallRules(rulesToApply, rulesContinueOnErrFlag, caller)) {
if (!rulesContinueOnErrFlag) {
return false;
}
}
// Now we check again in case more rules have been inserted.
rules.addAll(_firewallDao.listByIpAndPurposeAndNotRevoked(ipId, Purpose.Firewall));

Expand Down
5 changes: 4 additions & 1 deletion server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2001,7 +2001,10 @@ protected boolean handleSystemLBIpRelease(LoadBalancerVO lb) {

@Override
public boolean removeAllLoadBalanacersForIp(long ipId, Account caller, long callerUserId) {
List<FirewallRuleVO> rules = _firewallDao.listByIpAndPurposeAndNotRevoked(ipId, Purpose.LoadBalancing);

//Included revoked rules to remove the rules of ips which are in revoke state
List<FirewallRuleVO> rules = _firewallDao.listByIpAndPurpose(ipId, Purpose.LoadBalancing);

if (rules != null) {
s_logger.debug("Found " + rules.size() + " lb rules to cleanup");
for (FirewallRule rule : rules) {
Expand Down
10 changes: 9 additions & 1 deletion server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1845,7 +1845,15 @@ protected void finalizeNetworkRulesForNetwork(final Commands cmds, final DomainR

if (_networkModel.isProviderSupportServiceInNetwork(guestNetworkId, Service.StaticNat, provider)) {
if (ip.isOneToOneNat()) {
final StaticNatImpl staticNat = new StaticNatImpl(ip.getAccountId(), ip.getDomainId(), guestNetworkId, ip.getId(), ip.getVmIp(), false);

boolean revoke = false;
if (ip.getState() == IpAddress.State.Releasing ) {
// for ips got struck in releasing state we need to delete the rule not add.
s_logger.debug("Rule revoke set to true for the ip " + ip.getAddress() +" becasue it is in releasing state");
revoke = true;
}
final StaticNatImpl staticNat = new StaticNatImpl(ip.getAccountId(), ip.getDomainId(), guestNetworkId, ip.getId(), ip.getVmIp(), revoke);

staticNats.add(staticNat);
}
}
Expand Down
12 changes: 7 additions & 5 deletions server/src/com/cloud/network/rules/RulesManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ private boolean revokePortForwardingRuleInternal(long ruleId, Account caller, lo
boolean success = false;

if (apply) {
success = applyPortForwardingRules(rule.getSourceIpAddressId(), true, caller);
success = applyPortForwardingRules(rule.getSourceIpAddressId(), _ipAddrMgr.RulesContinueOnError.value(), caller);
} else {
success = true;
}
Expand Down Expand Up @@ -736,7 +736,7 @@ private boolean revokeStaticNatRuleInternal(long ruleId, Account caller, long us
boolean success = false;

if (apply) {
success = applyStaticNatRulesForIp(rule.getSourceIpAddressId(), true, caller, true);
success = applyStaticNatRulesForIp(rule.getSourceIpAddressId(), _ipAddrMgr.RulesContinueOnError.value(), caller, true);
} else {
success = true;
}
Expand Down Expand Up @@ -769,7 +769,7 @@ public boolean revokePortForwardingRulesForVm(long vmId) {
// apply rules for all ip addresses
for (Long ipId : ipsToReprogram) {
s_logger.debug("Applying port forwarding rules for ip address id=" + ipId + " as a part of vm expunge");
if (!applyPortForwardingRules(ipId, true, _accountMgr.getSystemAccount())) {
if (!applyPortForwardingRules(ipId, _ipAddrMgr.RulesContinueOnError.value(), _accountMgr.getSystemAccount())) {
s_logger.warn("Failed to apply port forwarding rules for ip id=" + ipId);
success = false;
}
Expand Down Expand Up @@ -1098,10 +1098,10 @@ public boolean revokeAllPFAndStaticNatRulesForIp(long ipId, long userId, Account
boolean success = true;

// revoke all port forwarding rules
success = success && applyPortForwardingRules(ipId, true, caller);
success = success && applyPortForwardingRules(ipId, _ipAddrMgr.RulesContinueOnError.value(), caller);

// revoke all all static nat rules
success = success && applyStaticNatRulesForIp(ipId, true, caller, true);
success = success && applyStaticNatRulesForIp(ipId, _ipAddrMgr.RulesContinueOnError.value(), caller, true);

// revoke static nat for the ip address
success = success && applyStaticNatForIp(ipId, false, caller, true);
Expand Down Expand Up @@ -1144,9 +1144,11 @@ public boolean revokeAllPFStaticNatRulesForNetwork(long networkId, long userId,
boolean success = true;
// revoke all PF rules for the network
success = success && applyPortForwardingRulesForNetwork(networkId, true, caller);
success = success && applyPortForwardingRulesForNetwork(networkId, _ipAddrMgr.RulesContinueOnError.value(), caller);

// revoke all all static nat rules for the network
success = success && applyStaticNatRulesForNetwork(networkId, true, caller);
success = success && applyStaticNatRulesForNetwork(networkId, _ipAddrMgr.RulesContinueOnError.value(), caller);

// Now we check again in case more rules have been inserted.
rules.addAll(_portForwardingDao.listByNetworkAndNotRevoked(networkId));
Expand Down