CLOUDSTACK-9287 - Check if the nic profile has already been removed from a certain router

- In case of redundant VPCs, the ACL items are revoked in the first iteration. Since the econd iteration
     is needed in order to remove the private network, we have to check if the nic profile is gone before trying
     to revoke the ACL items again, which would throw a NPE.
   - Some variable extraction in order to ease debugging.
This commit is contained in:
Wilder Rodrigues 2016-02-17 07:16:23 +01:00 committed by Remi Bergsma
parent 6a767732f9
commit 850fb1a557
6 changed files with 56 additions and 42 deletions

View File

@ -24,18 +24,6 @@ import java.util.Set;
import javax.inject.Inject;
import org.apache.cloudstack.api.command.admin.router.ConfigureOvsElementCmd;
import org.apache.cloudstack.api.command.admin.router.ConfigureVirtualRouterElementCmd;
import org.apache.cloudstack.api.command.admin.router.CreateVirtualRouterElementCmd;
import org.apache.cloudstack.api.command.admin.router.ListOvsElementsCmd;
import org.apache.cloudstack.api.command.admin.router.ListVirtualRouterElementsCmd;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.network.topology.NetworkTopology;
import org.apache.cloudstack.network.topology.NetworkTopologyContext;
import org.apache.log4j.Logger;
import org.cloud.network.router.deployment.RouterDeploymentDefinition;
import org.cloud.network.router.deployment.RouterDeploymentDefinitionBuilder;
import com.cloud.agent.api.to.LoadBalancerTO;
import com.cloud.configuration.ConfigurationManager;
import com.cloud.dc.DataCenter;
@ -107,6 +95,18 @@ import com.cloud.vm.dao.DomainRouterDao;
import com.cloud.vm.dao.UserVmDao;
import com.google.gson.Gson;
import org.apache.cloudstack.api.command.admin.router.ConfigureOvsElementCmd;
import org.apache.cloudstack.api.command.admin.router.ConfigureVirtualRouterElementCmd;
import org.apache.cloudstack.api.command.admin.router.CreateVirtualRouterElementCmd;
import org.apache.cloudstack.api.command.admin.router.ListOvsElementsCmd;
import org.apache.cloudstack.api.command.admin.router.ListVirtualRouterElementsCmd;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.network.topology.NetworkTopology;
import org.apache.cloudstack.network.topology.NetworkTopologyContext;
import org.apache.log4j.Logger;
import org.cloud.network.router.deployment.RouterDeploymentDefinition;
import org.cloud.network.router.deployment.RouterDeploymentDefinitionBuilder;
public class VirtualRouterElement extends AdapterBase implements VirtualRouterElementService, DhcpServiceProvider, UserDataServiceProvider, SourceNatServiceProvider,
StaticNatServiceProvider, FirewallServiceProvider, LoadBalancingServiceProvider, PortForwardingServiceProvider, RemoteAccessVPNServiceProvider, IpDeployer,
NetworkMigrationResponder, AggregatedCommandExecutor {
@ -153,6 +153,8 @@ NetworkMigrationResponder, AggregatedCommandExecutor {
IPAddressDao _ipAddressDao;
@Inject
DataCenterDao _dcDao;
@Inject
NetworkModel _networkModel;
@Inject
NetworkTopologyContext networkTopologyContext;

View File

@ -25,13 +25,6 @@ import java.util.Set;
import javax.inject.Inject;
import org.apache.cloudstack.network.topology.NetworkTopology;
import org.apache.log4j.Logger;
import org.cloud.network.router.deployment.RouterDeploymentDefinition;
import org.cloud.network.router.deployment.RouterDeploymentDefinitionBuilder;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import com.cloud.dc.DataCenter;
import com.cloud.dc.DataCenterVO;
import com.cloud.deploy.DeployDestination;
@ -79,6 +72,13 @@ import com.cloud.vm.VirtualMachine;
import com.cloud.vm.VirtualMachineManager;
import com.cloud.vm.VirtualMachineProfile;
import org.apache.cloudstack.network.topology.NetworkTopology;
import org.apache.log4j.Logger;
import org.cloud.network.router.deployment.RouterDeploymentDefinition;
import org.cloud.network.router.deployment.RouterDeploymentDefinitionBuilder;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
public class VpcVirtualRouterElement extends VirtualRouterElement implements VpcProvider, Site2SiteVpnServiceProvider, NetworkACLServiceProvider {
private static final Logger s_logger = Logger.getLogger(VpcVirtualRouterElement.class);
@ -466,7 +466,7 @@ public class VpcVirtualRouterElement extends VirtualRouterElement implements Vpc
}
}
return result > 0 ? true : false;
return result == routers.size() ? true : false;
}
@Override
@ -559,9 +559,16 @@ public class VpcVirtualRouterElement extends VirtualRouterElement implements Vpc
final DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
final NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
final Network privateNetwork = _networkModel.getNetwork(gateway.getNetworkId());
boolean result = true;
for (final DomainRouterVO domainRouterVO : routers) {
result = result && networkTopology.applyNetworkACLs(network, rules, domainRouterVO, isPrivateGateway);
final NicProfile nicProfile = _networkModel.getNicProfile(domainRouterVO, privateNetwork.getId(), null);
if (nicProfile != null) {
result = result && networkTopology.applyNetworkACLs(network, rules, domainRouterVO, isPrivateGateway);
} else {
s_logger.warn("Nic Profile for router '" + domainRouterVO + "' has already been removed. Router is redundant = " + domainRouterVO.getIsRedundantRouter());
}
}
return result;
}

View File

@ -58,6 +58,7 @@ import com.cloud.agent.api.to.FirewallRuleTO;
import com.cloud.agent.api.to.IpAddressTO;
import com.cloud.agent.api.to.LoadBalancerTO;
import com.cloud.agent.api.to.NetworkACLTO;
import com.cloud.agent.api.to.NicTO;
import com.cloud.agent.api.to.PortForwardingRuleTO;
import com.cloud.agent.api.to.StaticNatRuleTO;
import com.cloud.agent.manager.Commands;
@ -504,7 +505,8 @@ public class CommandSetupHelper {
}
}
final SetNetworkACLCommand cmd = new SetNetworkACLCommand(rulesTO, _networkHelper.getNicTO(router, guestNetworkId, null));
NicTO nicTO = _networkHelper.getNicTO(router, guestNetworkId, null);
final SetNetworkACLCommand cmd = new SetNetworkACLCommand(rulesTO, nicTO);
cmd.setAccessDetail(NetworkElementCommand.ROUTER_IP, _routerControlHelper.getRouterControlIp(router.getId()));
cmd.setAccessDetail(NetworkElementCommand.ROUTER_GUEST_IP, _routerControlHelper.getRouterIpInNetwork(guestNetworkId, router.getId()));
cmd.setAccessDetail(NetworkElementCommand.GUEST_VLAN_TAG, guestVlan);

View File

@ -26,9 +26,6 @@ import java.util.Map;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
import com.cloud.agent.api.Answer;
import com.cloud.agent.api.Command;
import com.cloud.agent.api.Command.OnError;
@ -91,6 +88,9 @@ import com.cloud.vm.VirtualMachineProfile;
import com.cloud.vm.VirtualMachineProfile.Param;
import com.cloud.vm.dao.VMInstanceDao;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
@Component
public class VpcVirtualNetworkApplianceManagerImpl extends VirtualNetworkApplianceManagerImpl implements VpcVirtualNetworkApplianceManager {
private static final Logger s_logger = Logger.getLogger(VpcVirtualNetworkApplianceManagerImpl.class);
@ -531,16 +531,18 @@ public class VpcVirtualNetworkApplianceManagerImpl extends VirtualNetworkApplian
@Override
public boolean destroyPrivateGateway(final PrivateGateway gateway, final VirtualRouter router) throws ConcurrentOperationException, ResourceUnavailableException {
boolean result = true;
if (!_networkModel.isVmPartOfNetwork(router.getId(), gateway.getNetworkId())) {
s_logger.debug("Router doesn't have nic for gateway " + gateway + " so no need to removed it");
return true;
return result;
}
final Network privateNetwork = _networkModel.getNetwork(gateway.getNetworkId());
final NicProfile nicProfile = _networkModel.getNicProfile(router, privateNetwork.getId(), null);
s_logger.debug("Releasing private ip for gateway " + gateway + " from " + router);
boolean result = setupVpcPrivateNetwork(router, false, _networkModel.getNicProfile(router, privateNetwork.getId(), null));
result = setupVpcPrivateNetwork(router, false, nicProfile);
if (!result) {
s_logger.warn("Failed to release private ip for gateway " + gateway + " on router " + router);
return false;
@ -706,7 +708,7 @@ public class VpcVirtualNetworkApplianceManagerImpl extends VirtualNetworkApplian
s_logger.error("Unable to start vpn: unable add users to vpn in zone " + router.getDataCenterId() + " for account " + vpn.getAccountId() + " on domR: "
+ router.getInstanceName() + " due to " + answer.getDetails());
throw new ResourceUnavailableException("Unable to start vpn: Unable to add users to vpn in zone " + router.getDataCenterId() + " for account " + vpn.getAccountId()
+ " on domR: " + router.getInstanceName() + " due to " + answer.getDetails(), DataCenter.class, router.getDataCenterId());
+ " on domR: " + router.getInstanceName() + " due to " + answer.getDetails(), DataCenter.class, router.getDataCenterId());
}
answer = cmds.getAnswer("startVpn");
if (!answer.getResult()) {

View File

@ -21,11 +21,6 @@ import java.util.List;
import javax.inject.Inject;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.framework.messagebus.MessageBus;
import org.apache.cloudstack.framework.messagebus.PublishScope;
import org.apache.log4j.Logger;
import com.cloud.configuration.ConfigurationManager;
import com.cloud.event.ActionEvent;
import com.cloud.event.EventTypes;
@ -52,6 +47,11 @@ import com.cloud.utils.db.TransactionCallback;
import com.cloud.utils.db.TransactionStatus;
import com.cloud.utils.exception.CloudRuntimeException;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.framework.messagebus.MessageBus;
import org.apache.cloudstack.framework.messagebus.PublishScope;
import org.apache.log4j.Logger;
public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLManager {
private static final Logger s_logger = Logger.getLogger(NetworkACLManagerImpl.class);
@ -335,10 +335,10 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana
@Override
public boolean revokeACLItemsForPrivateGw(final PrivateGateway gateway) throws ResourceUnavailableException {
final List<NetworkACLItemVO> aclItems = _networkACLItemDao.listByACL(gateway.getNetworkACLId());
final long networkACLId = gateway.getNetworkACLId();
final List<NetworkACLItemVO> aclItems = _networkACLItemDao.listByACL(networkACLId);
if (aclItems.isEmpty()) {
s_logger.debug("Found no network ACL Items for private gateway id=" + gateway.getId());
s_logger.debug("Found no network ACL Items for private gateway 'id=" + gateway.getId() + "'");
return true;
}

View File

@ -19,11 +19,6 @@ package org.apache.cloudstack.network.topology;
import java.util.List;
import org.apache.log4j.Logger;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.stereotype.Component;
import com.cloud.dc.DataCenter;
import com.cloud.deploy.DeployDestination;
import com.cloud.exception.ConcurrentOperationException;
@ -52,6 +47,11 @@ import com.cloud.vm.NicProfile;
import com.cloud.vm.VirtualMachine.State;
import com.cloud.vm.VirtualMachineProfile;
import org.apache.log4j.Logger;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.stereotype.Component;
@Component
public class AdvancedNetworkTopology extends BasicNetworkTopology {
@ -223,6 +223,7 @@ public class AdvancedNetworkTopology extends BasicNetworkTopology {
final NetworkAclsRules aclsRules = new NetworkAclsRules(network, rules, isPrivateGateway);
return applyRules(network, router, typeString, isPodLevelException, podId, failWhenDisconnect, new RuleApplierWrapper<RuleApplier>(aclsRules));
final boolean result = applyRules(network, router, typeString, isPodLevelException, podId, failWhenDisconnect, new RuleApplierWrapper<RuleApplier>(aclsRules));
return result;
}
}