Skip to content

Commit 309a60e

Browse files
swillswill
committed
Merge pull request #1483 from remibergsma/pr1413-wilder-47
CLOUDSTACK-9287 - Fix unique mac address per rVPC routerThis is work by @wilderrodrigues, see PR #1413 It contains important fixes and I think it needs to be included so I send the PR again. * pr/1483: CLOUDSTACK-9287 - Improve test by checking if pvt gw is removed and fix typos CLOUDSTACK-9287 - Fix RVR public interface CLOUDSTACK-9287 - Add integration test to cover the private gateway related changes CLOUDSTACK-9287 - Refactor the interface state configuration CLOUDSTACK-9287 - Check if the nic profile has already been removed from a certain router CLOUDSTACK-9287 - Bring up the private gw interface on state change to master CLOUDSTACK-9287 - Make sure private gw interface is not used for default gw CLOUDSTACK-9287 - Add integration test to cover the private gw interface/mac address issues CLOUDSTACK-9287 - Put private gateway interface down on backup router CLOUDSTACK-9287 - Generate new mac address if router is redundant and nic profile exists Signed-off-by: Will Stevens <[email protected]>
2 parents 5a79c2b + 799b9f2 commit 309a60e

File tree

12 files changed

+462
-169
lines changed

12 files changed

+462
-169
lines changed

‎server/src/com/cloud/network/element/VirtualRouterElement.java‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,6 @@
2424

2525
import javax.inject.Inject;
2626

27-
import org.apache.cloudstack.api.command.admin.router.ConfigureOvsElementCmd;
28-
import org.apache.cloudstack.api.command.admin.router.ConfigureVirtualRouterElementCmd;
29-
import org.apache.cloudstack.api.command.admin.router.CreateVirtualRouterElementCmd;
30-
import org.apache.cloudstack.api.command.admin.router.ListOvsElementsCmd;
31-
import org.apache.cloudstack.api.command.admin.router.ListVirtualRouterElementsCmd;
32-
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
33-
import org.apache.cloudstack.network.topology.NetworkTopology;
34-
import org.apache.cloudstack.network.topology.NetworkTopologyContext;
35-
import org.apache.log4j.Logger;
36-
import org.cloud.network.router.deployment.RouterDeploymentDefinition;
37-
import org.cloud.network.router.deployment.RouterDeploymentDefinitionBuilder;
38-
3927
import com.cloud.agent.api.to.LoadBalancerTO;
4028
import com.cloud.configuration.ConfigurationManager;
4129
import com.cloud.dc.DataCenter;
@@ -107,6 +95,18 @@
10795
import com.cloud.vm.dao.UserVmDao;
10896
import com.google.gson.Gson;
10997

98+
import org.apache.cloudstack.api.command.admin.router.ConfigureOvsElementCmd;
99+
import org.apache.cloudstack.api.command.admin.router.ConfigureVirtualRouterElementCmd;
100+
import org.apache.cloudstack.api.command.admin.router.CreateVirtualRouterElementCmd;
101+
import org.apache.cloudstack.api.command.admin.router.ListOvsElementsCmd;
102+
import org.apache.cloudstack.api.command.admin.router.ListVirtualRouterElementsCmd;
103+
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
104+
import org.apache.cloudstack.network.topology.NetworkTopology;
105+
import org.apache.cloudstack.network.topology.NetworkTopologyContext;
106+
import org.apache.log4j.Logger;
107+
import org.cloud.network.router.deployment.RouterDeploymentDefinition;
108+
import org.cloud.network.router.deployment.RouterDeploymentDefinitionBuilder;
109+
110110
public class VirtualRouterElement extends AdapterBase implements VirtualRouterElementService, DhcpServiceProvider, UserDataServiceProvider, SourceNatServiceProvider,
111111
StaticNatServiceProvider, FirewallServiceProvider, LoadBalancingServiceProvider, PortForwardingServiceProvider, RemoteAccessVPNServiceProvider, IpDeployer,
112112
NetworkMigrationResponder, AggregatedCommandExecutor {
@@ -153,6 +153,8 @@ public class VirtualRouterElement extends AdapterBase implements VirtualRouterEl
153153
IPAddressDao _ipAddressDao;
154154
@Inject
155155
DataCenterDao _dcDao;
156+
@Inject
157+
NetworkModel _networkModel;
156158

157159
@Inject
158160
NetworkTopologyContext networkTopologyContext;

‎server/src/com/cloud/network/element/VpcVirtualRouterElement.java‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,6 @@
2525

2626
import javax.inject.Inject;
2727

28-
import org.apache.cloudstack.network.topology.NetworkTopology;
29-
import org.apache.log4j.Logger;
30-
import org.cloud.network.router.deployment.RouterDeploymentDefinition;
31-
import org.cloud.network.router.deployment.RouterDeploymentDefinitionBuilder;
32-
import org.springframework.beans.factory.annotation.Autowired;
33-
import org.springframework.beans.factory.annotation.Qualifier;
34-
3528
import com.cloud.dc.DataCenter;
3629
import com.cloud.dc.DataCenterVO;
3730
import com.cloud.deploy.DeployDestination;
@@ -79,6 +72,13 @@
7972
import com.cloud.vm.VirtualMachineManager;
8073
import com.cloud.vm.VirtualMachineProfile;
8174

75+
import org.apache.cloudstack.network.topology.NetworkTopology;
76+
import org.apache.log4j.Logger;
77+
import org.cloud.network.router.deployment.RouterDeploymentDefinition;
78+
import org.cloud.network.router.deployment.RouterDeploymentDefinitionBuilder;
79+
import org.springframework.beans.factory.annotation.Autowired;
80+
import org.springframework.beans.factory.annotation.Qualifier;
81+
8282
public class VpcVirtualRouterElement extends VirtualRouterElement implements VpcProvider, Site2SiteVpnServiceProvider, NetworkACLServiceProvider {
8383

8484
private static final Logger s_logger = Logger.getLogger(VpcVirtualRouterElement.class);
@@ -466,7 +466,7 @@ public boolean deletePrivateGateway(final PrivateGateway gateway) throws Concurr
466466
}
467467
}
468468

469-
return result > 0 ? true : false;
469+
return result == routers.size() ? true : false;
470470
}
471471

472472
@Override
@@ -559,9 +559,16 @@ public boolean applyACLItemsToPrivateGw(final PrivateGateway gateway, final List
559559
final DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
560560
final NetworkTopology networkTopology = networkTopologyContext.retrieveNetworkTopology(dcVO);
561561

562+
final Network privateNetwork = _networkModel.getNetwork(gateway.getNetworkId());
563+
562564
boolean result = true;
563565
for (final DomainRouterVO domainRouterVO : routers) {
564-
result = result && networkTopology.applyNetworkACLs(network, rules, domainRouterVO, isPrivateGateway);
566+
final NicProfile nicProfile = _networkModel.getNicProfile(domainRouterVO, privateNetwork.getId(), null);
567+
if (nicProfile != null) {
568+
result = result && networkTopology.applyNetworkACLs(network, rules, domainRouterVO, isPrivateGateway);
569+
} else {
570+
s_logger.warn("Nic Profile for router '" + domainRouterVO + "' has already been removed. Router is redundant = " + domainRouterVO.getIsRedundantRouter());
571+
}
565572
}
566573
return result;
567574
}

‎server/src/com/cloud/network/router/CommandSetupHelper.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import com.cloud.agent.api.to.IpAddressTO;
5959
import com.cloud.agent.api.to.LoadBalancerTO;
6060
import com.cloud.agent.api.to.NetworkACLTO;
61+
import com.cloud.agent.api.to.NicTO;
6162
import com.cloud.agent.api.to.PortForwardingRuleTO;
6263
import com.cloud.agent.api.to.StaticNatRuleTO;
6364
import com.cloud.agent.manager.Commands;
@@ -504,7 +505,8 @@ public void createNetworkACLsCommands(final List<? extends NetworkACLItem> rules
504505
}
505506
}
506507

507-
final SetNetworkACLCommand cmd = new SetNetworkACLCommand(rulesTO, _networkHelper.getNicTO(router, guestNetworkId, null));
508+
NicTO nicTO = _networkHelper.getNicTO(router, guestNetworkId, null);
509+
final SetNetworkACLCommand cmd = new SetNetworkACLCommand(rulesTO, nicTO);
508510
cmd.setAccessDetail(NetworkElementCommand.ROUTER_IP, _routerControlHelper.getRouterControlIp(router.getId()));
509511
cmd.setAccessDetail(NetworkElementCommand.ROUTER_GUEST_IP, _routerControlHelper.getRouterIpInNetwork(guestNetworkId, router.getId()));
510512
cmd.setAccessDetail(NetworkElementCommand.GUEST_VLAN_TAG, guestVlan);

‎server/src/com/cloud/network/router/NicProfileHelperImpl.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ public NicProfile createPrivateNicProfileForGateway(final VpcGateway privateGate
8585
new NicProfile(privateNic, privateNetwork, privateNic.getBroadcastUri(), privateNic.getIsolationUri(), _networkModel.getNetworkRate(
8686
privateNetwork.getId(), router.getId()), _networkModel.isSecurityGroupSupportedInNetwork(privateNetwork), _networkModel.getNetworkTag(
8787
router.getHypervisorType(), privateNetwork));
88+
89+
if (router.getIsRedundantRouter()) {
90+
String newMacAddress = NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(ipVO.getMacAddress()));
91+
privateNicProfile.setMacAddress(newMacAddress);
92+
}
8893
} else {
8994
final String netmask = NetUtils.getCidrNetmask(privateNetwork.getCidr());
9095
final PrivateIpAddress ip =

‎server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,6 @@
2626
import javax.inject.Inject;
2727
import javax.naming.ConfigurationException;
2828

29-
import org.apache.log4j.Logger;
30-
import org.springframework.stereotype.Component;
31-
3229
import com.cloud.agent.api.Answer;
3330
import com.cloud.agent.api.Command;
3431
import com.cloud.agent.api.Command.OnError;
@@ -91,6 +88,9 @@
9188
import com.cloud.vm.VirtualMachineProfile.Param;
9289
import com.cloud.vm.dao.VMInstanceDao;
9390

91+
import org.apache.log4j.Logger;
92+
import org.springframework.stereotype.Component;
93+
9494
@Component
9595
public class VpcVirtualNetworkApplianceManagerImpl extends VirtualNetworkApplianceManagerImpl implements VpcVirtualNetworkApplianceManager {
9696
private static final Logger s_logger = Logger.getLogger(VpcVirtualNetworkApplianceManagerImpl.class);
@@ -531,16 +531,18 @@ protected boolean setupVpcPrivateNetwork(final VirtualRouter router, final boole
531531

532532
@Override
533533
public boolean destroyPrivateGateway(final PrivateGateway gateway, final VirtualRouter router) throws ConcurrentOperationException, ResourceUnavailableException {
534+
boolean result = true;
534535

535536
if (!_networkModel.isVmPartOfNetwork(router.getId(), gateway.getNetworkId())) {
536537
s_logger.debug("Router doesn't have nic for gateway " + gateway + " so no need to removed it");
537-
return true;
538+
return result;
538539
}
539540

540541
final Network privateNetwork = _networkModel.getNetwork(gateway.getNetworkId());
542+
final NicProfile nicProfile = _networkModel.getNicProfile(router, privateNetwork.getId(), null);
541543

542544
s_logger.debug("Releasing private ip for gateway " + gateway + " from " + router);
543-
boolean result = setupVpcPrivateNetwork(router, false, _networkModel.getNicProfile(router, privateNetwork.getId(), null));
545+
result = setupVpcPrivateNetwork(router, false, nicProfile);
544546
if (!result) {
545547
s_logger.warn("Failed to release private ip for gateway " + gateway + " on router " + router);
546548
return false;
@@ -706,7 +708,7 @@ public boolean startRemoteAccessVpn(final RemoteAccessVpn vpn, final VirtualRout
706708
s_logger.error("Unable to start vpn: unable add users to vpn in zone " + router.getDataCenterId() + " for account " + vpn.getAccountId() + " on domR: "
707709
+ router.getInstanceName() + " due to " + answer.getDetails());
708710
throw new ResourceUnavailableException("Unable to start vpn: Unable to add users to vpn in zone " + router.getDataCenterId() + " for account " + vpn.getAccountId()
709-
+ " on domR: " + router.getInstanceName() + " due to " + answer.getDetails(), DataCenter.class, router.getDataCenterId());
711+
+ " on domR: " + router.getInstanceName() + " due to " + answer.getDetails(), DataCenter.class, router.getDataCenterId());
710712
}
711713
answer = cmds.getAnswer("startVpn");
712714
if (!answer.getResult()) {

‎server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,6 @@
2121

2222
import javax.inject.Inject;
2323

24-
import org.apache.cloudstack.context.CallContext;
25-
import org.apache.cloudstack.framework.messagebus.MessageBus;
26-
import org.apache.cloudstack.framework.messagebus.PublishScope;
27-
import org.apache.log4j.Logger;
28-
2924
import com.cloud.configuration.ConfigurationManager;
3025
import com.cloud.event.ActionEvent;
3126
import com.cloud.event.EventTypes;
@@ -52,6 +47,11 @@
5247
import com.cloud.utils.db.TransactionStatus;
5348
import com.cloud.utils.exception.CloudRuntimeException;
5449

50+
import org.apache.cloudstack.context.CallContext;
51+
import org.apache.cloudstack.framework.messagebus.MessageBus;
52+
import org.apache.cloudstack.framework.messagebus.PublishScope;
53+
import org.apache.log4j.Logger;
54+
5555
public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLManager {
5656
private static final Logger s_logger = Logger.getLogger(NetworkACLManagerImpl.class);
5757

@@ -335,10 +335,10 @@ public boolean revokeACLItemsForNetwork(final long networkId) throws ResourceUna
335335

336336
@Override
337337
public boolean revokeACLItemsForPrivateGw(final PrivateGateway gateway) throws ResourceUnavailableException {
338-
339-
final List<NetworkACLItemVO> aclItems = _networkACLItemDao.listByACL(gateway.getNetworkACLId());
338+
final long networkACLId = gateway.getNetworkACLId();
339+
final List<NetworkACLItemVO> aclItems = _networkACLItemDao.listByACL(networkACLId);
340340
if (aclItems.isEmpty()) {
341-
s_logger.debug("Found no network ACL Items for private gateway id=" + gateway.getId());
341+
s_logger.debug("Found no network ACL Items for private gateway 'id=" + gateway.getId() + "'");
342342
return true;
343343
}
344344

‎server/src/org/apache/cloudstack/network/topology/AdvancedNetworkTopology.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,6 @@
1919

2020
import java.util.List;
2121

22-
import org.apache.log4j.Logger;
23-
import org.springframework.beans.factory.annotation.Autowired;
24-
import org.springframework.beans.factory.annotation.Qualifier;
25-
import org.springframework.stereotype.Component;
26-
2722
import com.cloud.dc.DataCenter;
2823
import com.cloud.deploy.DeployDestination;
2924
import com.cloud.exception.ConcurrentOperationException;
@@ -52,6 +47,11 @@
5247
import com.cloud.vm.VirtualMachine.State;
5348
import com.cloud.vm.VirtualMachineProfile;
5449

50+
import org.apache.log4j.Logger;
51+
import org.springframework.beans.factory.annotation.Autowired;
52+
import org.springframework.beans.factory.annotation.Qualifier;
53+
import org.springframework.stereotype.Component;
54+
5555
@Component
5656
public class AdvancedNetworkTopology extends BasicNetworkTopology {
5757

@@ -223,6 +223,7 @@ public boolean applyNetworkACLs(final Network network, final List<? extends Netw
223223

224224
final NetworkAclsRules aclsRules = new NetworkAclsRules(network, rules, isPrivateGateway);
225225

226-
return applyRules(network, router, typeString, isPodLevelException, podId, failWhenDisconnect, new RuleApplierWrapper<RuleApplier>(aclsRules));
226+
final boolean result = applyRules(network, router, typeString, isPodLevelException, podId, failWhenDisconnect, new RuleApplierWrapper<RuleApplier>(aclsRules));
227+
return result;
227228
}
228229
}

‎systemvm/patches/debian/config/opt/cloud/bin/configure.py‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -733,34 +733,34 @@ def process(self):
733733

734734
#return the VR guest interface ip
735735
def getGuestIp(self):
736-
ipr = []
736+
interfaces = []
737737
ipAddr = None
738-
for ip in self.config.address().get_ips():
739-
if ip.is_guest():
740-
ipr.append(ip)
741-
if len(ipr) > 0:
742-
ipAddr = sorted(ipr)[-1]
738+
for interface in self.config.address().get_interfaces():
739+
if interface.is_guest():
740+
interfaces.append(interface)
741+
if len(interfaces) > 0:
742+
ipAddr = sorted(interfaces)[-1]
743743
if ipAddr:
744744
return ipAddr.get_ip()
745745

746746
return None
747747

748748
def getDeviceByIp(self, ipa):
749-
for ip in self.config.address().get_ips():
750-
if ip.ip_in_subnet(ipa):
751-
return ip.get_device()
749+
for interface in self.config.address().get_interfaces():
750+
if interface.ip_in_subnet(ipa):
751+
return interface.get_device()
752752
return None
753753

754754
def getNetworkByIp(self, ipa):
755-
for ip in self.config.address().get_ips():
756-
if ip.ip_in_subnet(ipa):
757-
return ip.get_network()
755+
for interface in self.config.address().get_interfaces():
756+
if interface.ip_in_subnet(ipa):
757+
return interface.get_network()
758758
return None
759759

760760
def getGatewayByIp(self, ipa):
761-
for ip in self.config.address().get_ips():
762-
if ip.ip_in_subnet(ipa):
763-
return ip.get_gateway()
761+
for interface in self.config.address().get_interfaces():
762+
if interface.ip_in_subnet(ipa):
763+
return interface.get_gateway()
764764
return None
765765

766766
def portsToString(self, ports, delimiter):

0 commit comments

Comments
 (0)