From 850fb1a557b8eec04de79124fea5b6f923530d66 Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Wed, 17 Feb 2016 07:16:23 +0100 Subject: [PATCH] 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. --- .../network/element/VirtualRouterElement.java | 26 ++++++++++--------- .../element/VpcVirtualRouterElement.java | 25 +++++++++++------- .../network/router/CommandSetupHelper.java | 4 ++- ...VpcVirtualNetworkApplianceManagerImpl.java | 14 +++++----- .../network/vpc/NetworkACLManagerImpl.java | 16 ++++++------ .../topology/AdvancedNetworkTopology.java | 13 +++++----- 6 files changed, 56 insertions(+), 42 deletions(-) diff --git a/server/src/com/cloud/network/element/VirtualRouterElement.java b/server/src/com/cloud/network/element/VirtualRouterElement.java index ef6c6f97f0b..d802188e4c4 100644 --- a/server/src/com/cloud/network/element/VirtualRouterElement.java +++ b/server/src/com/cloud/network/element/VirtualRouterElement.java @@ -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; diff --git a/server/src/com/cloud/network/element/VpcVirtualRouterElement.java b/server/src/com/cloud/network/element/VpcVirtualRouterElement.java index 6ef2ed36faf..9999ee62cb8 100644 --- a/server/src/com/cloud/network/element/VpcVirtualRouterElement.java +++ b/server/src/com/cloud/network/element/VpcVirtualRouterElement.java @@ -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; } diff --git a/server/src/com/cloud/network/router/CommandSetupHelper.java b/server/src/com/cloud/network/router/CommandSetupHelper.java index 04427baf749..7208b256813 100644 --- a/server/src/com/cloud/network/router/CommandSetupHelper.java +++ b/server/src/com/cloud/network/router/CommandSetupHelper.java @@ -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); diff --git a/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java index 712c7472393..5785e2a6b5e 100644 --- a/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java +++ b/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java @@ -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()) { diff --git a/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java b/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java index 8a9a799575b..c64a36b7c9f 100644 --- a/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java +++ b/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java @@ -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 aclItems = _networkACLItemDao.listByACL(gateway.getNetworkACLId()); + final long networkACLId = gateway.getNetworkACLId(); + final List 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; } diff --git a/server/src/org/apache/cloudstack/network/topology/AdvancedNetworkTopology.java b/server/src/org/apache/cloudstack/network/topology/AdvancedNetworkTopology.java index e587c752c26..f456fcee177 100644 --- a/server/src/org/apache/cloudstack/network/topology/AdvancedNetworkTopology.java +++ b/server/src/org/apache/cloudstack/network/topology/AdvancedNetworkTopology.java @@ -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(aclsRules)); + final boolean result = applyRules(network, router, typeString, isPodLevelException, podId, failWhenDisconnect, new RuleApplierWrapper(aclsRules)); + return result; } } \ No newline at end of file