From f921ec686bcf339968040c57bf0ae9fceffd8fed Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Sat, 13 Feb 2016 12:59:20 +0100 Subject: [PATCH 01/10] CLOUDSTACK-9287 - Generate new mac address if router is redundant and nic profile exists --- .../src/com/cloud/network/router/NicProfileHelperImpl.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/src/com/cloud/network/router/NicProfileHelperImpl.java b/server/src/com/cloud/network/router/NicProfileHelperImpl.java index 4a0faa90ace..2fc5a422112 100644 --- a/server/src/com/cloud/network/router/NicProfileHelperImpl.java +++ b/server/src/com/cloud/network/router/NicProfileHelperImpl.java @@ -85,6 +85,11 @@ public class NicProfileHelperImpl implements NicProfileHelper { new NicProfile(privateNic, privateNetwork, privateNic.getBroadcastUri(), privateNic.getIsolationUri(), _networkModel.getNetworkRate( privateNetwork.getId(), router.getId()), _networkModel.isSecurityGroupSupportedInNetwork(privateNetwork), _networkModel.getNetworkTag( router.getHypervisorType(), privateNetwork)); + + if (router.getIsRedundantRouter()) { + String newMacAddress = NetUtils.long2Mac(NetUtils.createSequenceBasedMacAddress(ipVO.getMacAddress())); + privateNicProfile.setMacAddress(newMacAddress); + } } else { final String netmask = NetUtils.getCidrNetmask(privateNetwork.getCidr()); final PrivateIpAddress ip = From d93b008deb2ced326b314fefbe11511d798be5e8 Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Sat, 13 Feb 2016 15:48:30 +0100 Subject: [PATCH 02/10] CLOUDSTACK-9287 - Put private gateway interface down on backup router --- .../config/opt/cloud/bin/cs/CsAddress.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py index 1b39b385d4d..41b5e9a9168 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py @@ -117,6 +117,7 @@ class CsAddress(CsDataBag): else: logging.info( "Address %s on device %s not configured", ip.ip(), dev) + if CsDevice(dev, self.config).waitfordevice(): ip.configure(address) @@ -276,7 +277,7 @@ class CsIP: try: logging.info("Configuring address %s on device %s", self.ip(), self.dev) cmd = "ip addr add dev %s %s brd +" % (self.dev, self.ip()) - subprocess.call(cmd, shell=True) + CsHelper.execute(cmd) except Exception as e: logging.info("Exception occurred ==> %s" % e) @@ -317,6 +318,9 @@ class CsIP: def check_is_up(self): """ Ensure device is up """ + state_commands = {"router" : "ip addr | grep eth0 | grep inet | wc -l | xargs bash -c 'if [ $0 == 2 ]; then echo \"MASTER\"; else echo \"BACKUP\"; fi'", + "vpcrouter" : "ip addr | grep eth1 | grep state | awk '{print $9;}' | xargs bash -c 'if [ $0 == \"UP\" ]; then echo \"MASTER\"; else echo \"BACKUP\"; fi'"} + cmd = "ip link show %s | grep 'state DOWN'" % self.getDevice() for i in CsHelper.execute(cmd): if " DOWN " in i: @@ -324,10 +328,15 @@ class CsIP: # If redundant only bring up public interfaces that are not eth1. # Reason: private gateways are public interfaces. # master.py and keepalived will deal with eth1 public interface. - if self.cl.is_redundant() and (not self.is_public() or self.getDevice() not in PUBLIC_INTERFACE): - CsHelper.execute(cmd2) - # if not redundant bring everything up - if not self.cl.is_redundant(): + + if self.cl.is_redundant() and self.is_public(): + state_cmd = state_commands[self.cl.get_type()] + logging.info("Check state command => %s" % state_cmd) + state = CsHelper.execute(state_cmd)[0] + logging.info("Route state => %s" % state) + if self.getDevice() not in PUBLIC_INTERFACE and state == "MASTER": + CsHelper.execute(cmd2) + else: CsHelper.execute(cmd2) def set_mark(self): From 250be376e879b6c43792fdb5af0f5c49747689da Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Sat, 13 Feb 2016 17:29:14 +0100 Subject: [PATCH 03/10] CLOUDSTACK-9287 - Add integration test to cover the private gw interface/mac address issues --- test/integration/smoke/test_privategw_acl.py | 115 ++++++++++++++++++- 1 file changed, 113 insertions(+), 2 deletions(-) diff --git a/test/integration/smoke/test_privategw_acl.py b/test/integration/smoke/test_privategw_acl.py index 754738b1923..56cc92365f6 100644 --- a/test/integration/smoke/test_privategw_acl.py +++ b/test/integration/smoke/test_privategw_acl.py @@ -33,6 +33,13 @@ class Services: def __init__(self): self.services = { + "configurableData": { + "host": { + "password": "password", + "username": "root", + "port": 22 + } + }, "account": { "email": "test@test.com", "firstname": "Test", @@ -262,7 +269,7 @@ class TestPrivateGwACL(cloudstackTestCase): self.logger.debug("Enabling the VPC offering created") vpc_off.update(self.apiclient, state='Enabled') - self.performVPCTests(vpc_off, True) + self.performVPCTests(vpc_off, restart_with_cleanup = True) @attr(tags=["advanced"], required_hardware="true") def test_04_rvpc_privategw_static_routes(self): @@ -276,6 +283,18 @@ class TestPrivateGwACL(cloudstackTestCase): self.performVPCTests(vpc_off) + @attr(tags=["advanced"], required_hardware="true") + def test_05_rvpc_privategw_check_interface(self): + self.logger.debug("Creating a Redundant VPC offering..") + vpc_off = VpcOffering.create( + self.apiclient, + self.services["redundant_vpc_offering"]) + + self.logger.debug("Enabling the Redundant VPC offering created") + vpc_off.update(self.apiclient, state='Enabled') + + self.performPrivateGWInterfaceTests(vpc_off) + def performVPCTests(self, vpc_off, restart_with_cleanup = False): self.logger.debug("Creating VPCs with offering ID %s" % vpc_off.id) vpc_1 = self.createVPC(vpc_off, cidr = '10.0.1.0/24') @@ -331,6 +350,99 @@ class TestPrivateGwACL(cloudstackTestCase): self.check_pvt_gw_connectivity(vm1, public_ip_1, vm2.nic[0].ipaddress) self.check_pvt_gw_connectivity(vm2, public_ip_2, vm1.nic[0].ipaddress) + def performPrivateGWInterfaceTests(self, vpc_off): + self.logger.debug("Creating VPCs with offering ID %s" % vpc_off.id) + vpc_1 = self.createVPC(vpc_off, cidr = '10.0.1.0/24') + + self.cleanup = [vpc_1, vpc_off, self.account] + + physical_networks = get_physical_networks(self.apiclient, self.zone.id) + if not physical_networks: + self.fail("No Physical Networks found!") + + vlans = physical_networks[0].vlan.split('-') + vlan_1 = int(vlans[0]) + + network_1 = self.createNetwork(vpc_1, gateway = '10.0.1.1') + + vm1 = self.createVM(network_1) + + self.cleanup.insert(0, vm1) + + acl1 = self.createACL(vpc_1) + self.createACLItem(acl1.id, cidr = "0.0.0.0/0") + privateGw_1 = self.createPvtGw(vpc_1, "10.0.3.100", "10.0.3.101", acl1.id, vlan_1) + self.replacePvtGwACL(acl1.id, privateGw_1.id) + + self.replaceNetworkAcl(acl1.id, network_1) + + staticRoute_1 = self.createStaticRoute(privateGw_1.id, cidr = '10.0.2.0/24') + + public_ip_1 = self.acquire_publicip(vpc_1, network_1) + + nat_rule_1 = self.create_natrule(vpc_1, vm1, public_ip_1, network_1) + + routers = list_routers(self.apiclient, + account=self.account.name, + domainid=self.account.domainid) + + self.assertEqual(isinstance(routers, list), True, + "Check for list routers response return valid data") + + self.assertEqual(len(routers), 2, + "Check for list routers size returned '%s' instead of 2" % len(routers)) + + state_holder = {routers[0].linklocalip : {"state" : None, "mac" : None}, + routers[1].linklocalip : {"state" : None, "mac" : None}} + state = None + mac = None + for router in routers: + if router.isredundantrouter and router.vpcid: + hosts = list_hosts( + self.apiclient, + id=router.hostid) + self.assertEqual( + isinstance(hosts, list), + True, + "Check for list hosts response return valid data") + + host = hosts[0] + host.user = self.services["configurableData"]["host"]["username"] + host.passwd = self.services["configurableData"]["host"]["password"] + host.port = self.services["configurableData"]["host"]["port"] + + try: + state = get_process_status( + host.ipaddress, + host.port, + host.user, + host.passwd, + router.linklocalip, + "ip addr | grep eth3 | grep state | awk '{print $9;}'") + + mac = get_process_status( + host.ipaddress, + host.port, + host.user, + host.passwd, + router.linklocalip, + "ip addr | grep link/ether | awk '{print $2;}' | sed -n 4p") + except KeyError: + self.skipTest( + "Provide a marvin config file with host\ + credentials to run %s" % + self._testMethodName) + + self.logger.debug("Result from the Router on IP '%s' is -> state: '%s', mac: '%s'" % (router.linklocalip, state, mac)) + state_holder[router.linklocalip]["state"] = str(state) + state_holder[router.linklocalip]["mac"] = str(mac) + + check_state = state_holder[routers[0].linklocalip]["state"].count(state_holder[routers[1].linklocalip]["state"]) + check_mac = state_holder[routers[0].linklocalip]["mac"].count(state_holder[routers[1].linklocalip]["mac"]) + + self.assertTrue(check_state == 0, "Routers private gateway interface should not be on the same state!") + self.assertTrue(check_mac == 0, "Routers private gateway interface should not have the same mac address!") + def createVPC(self, vpc_offering, cidr = '10.1.1.1/16'): try: self.logger.debug("Creating a VPC network in the account: %s" % self.account.name) @@ -568,4 +680,3 @@ class TestPrivateGwACL(cloudstackTestCase): cmd.cleanup = cleanup cmd.makeredundant = False self.api_client.restartVPC(cmd) - From 057b54aa3e8a462b3ced9b53d7a55dd5a64a0e4e Mon Sep 17 00:00:00 2001 From: Remi Bergsma Date: Sun, 14 Feb 2016 14:39:53 +0100 Subject: [PATCH 04/10] CLOUDSTACK-9287 - Make sure private gw interface is not used for default gw --- .../patches/debian/config/opt/cloud/bin/cs/CsAddress.py | 2 +- .../patches/debian/config/opt/cloud/bin/cs/CsRedundant.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py index 41b5e9a9168..f74ff479123 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py @@ -307,7 +307,7 @@ class CsIP: # The code looks redundant here, but we actually have to cater for routers and # VPC routers in a different manner. Please do not remove this block otherwise # The VPC default route will be broken. - if self.get_type() in ["public"]: + if self.get_type() in ["public"] and address["device"] in PUBLIC_INTERFACE: gateway = str(address["gateway"]) route.add_defaultroute(gateway) else: diff --git a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py index 77d0a6b9ccf..92f27cedc16 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py @@ -41,6 +41,7 @@ from CsRoute import CsRoute import socket from time import sleep +PUBLIC_INTERFACE = ['eth0', 'eth1'] class CsRedundant(object): @@ -228,7 +229,7 @@ class CsRedundant(object): self.set_lock() logging.info("Router switched to fault mode") - ips = [ip for ip in self.address.get_ips() if ip.is_public()] + ips = [ip for ip in self.address.get_ips() if ip.is_public() and ip.get_device() in PUBLIC_INTERFACE] for ip in ips: CsHelper.execute("ifconfig %s down" % ip.get_device()) @@ -257,7 +258,7 @@ class CsRedundant(object): logging.debug("Setting router to backup") dev = '' - ips = [ip for ip in self.address.get_ips() if ip.is_public()] + ips = [ip for ip in self.address.get_ips() if ip.is_public() and ip.get_device() in PUBLIC_INTERFACE] for ip in ips: if dev == ip.get_device(): continue @@ -291,7 +292,7 @@ class CsRedundant(object): logging.debug("Setting router to master") dev = '' - ips = [ip for ip in self.address.get_ips() if ip.is_public()] + ips = [ip for ip in self.address.get_ips() if ip.is_public() and ip.get_device() in PUBLIC_INTERFACE] route = CsRoute() for ip in ips: if dev == ip.get_device(): From 6a767732f959186e95c71d31a2ed49bb67a85473 Mon Sep 17 00:00:00 2001 From: Remi Bergsma Date: Sun, 14 Feb 2016 18:09:03 +0100 Subject: [PATCH 05/10] CLOUDSTACK-9287 - Bring up the private gw interface on state change to master --- .../patches/debian/config/opt/cloud/bin/cs/CsRedundant.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py index 92f27cedc16..7d34e303fa6 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py @@ -292,7 +292,7 @@ class CsRedundant(object): logging.debug("Setting router to master") dev = '' - ips = [ip for ip in self.address.get_ips() if ip.is_public() and ip.get_device() in PUBLIC_INTERFACE] + ips = [ip for ip in self.address.get_ips() if ip.is_public()] route = CsRoute() for ip in ips: if dev == ip.get_device(): @@ -307,7 +307,8 @@ class CsRedundant(object): try: gateway = ip.get_gateway() logging.info("Adding gateway ==> %s to device ==> %s" % (gateway, dev)) - route.add_defaultroute(gateway) + if ip.get_device() in PUBLIC_INTERFACE: + route.add_defaultroute(gateway) except: logging.error("ERROR getting gateway from device %s" % dev) else: From 850fb1a557b8eec04de79124fea5b6f923530d66 Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Wed, 17 Feb 2016 07:16:23 +0100 Subject: [PATCH 06/10] 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 From c41edc1fe62cf92ab85c59c6da4dced9e626d961 Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Wed, 17 Feb 2016 07:31:39 +0100 Subject: [PATCH 07/10] CLOUDSTACK-9287 - Refactor the interface state configuration - This also refactors the CsAddress in order to offer better readability in a couple of methods. --- .../debian/config/opt/cloud/bin/configure.py | 30 +++---- .../config/opt/cloud/bin/cs/CsAddress.py | 63 +++++---------- .../config/opt/cloud/bin/cs/CsHelper.py | 24 ++++++ .../config/opt/cloud/bin/cs/CsRedundant.py | 80 +++++++++---------- 4 files changed, 99 insertions(+), 98 deletions(-) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/configure.py b/systemvm/patches/debian/config/opt/cloud/bin/configure.py index ab134fcfca7..f5ae830f809 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/configure.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/configure.py @@ -731,34 +731,34 @@ class CsForwardingRules(CsDataBag): #return the VR guest interface ip def getGuestIp(self): - ipr = [] + interfaces = [] ipAddr = None - for ip in self.config.address().get_ips(): - if ip.is_guest(): - ipr.append(ip) - if len(ipr) > 0: - ipAddr = sorted(ipr)[-1] + for interface in self.config.address().get_interfaces(): + if interface.is_guest(): + interfaces.append(interface) + if len(interfaces) > 0: + ipAddr = sorted(interfaces)[-1] if ipAddr: return ipAddr.get_ip() return None def getDeviceByIp(self, ipa): - for ip in self.config.address().get_ips(): - if ip.ip_in_subnet(ipa): - return ip.get_device() + for interface in self.config.address().get_interfaces(): + if interface.ip_in_subnet(ipa): + return interface.get_device() return None def getNetworkByIp(self, ipa): - for ip in self.config.address().get_ips(): - if ip.ip_in_subnet(ipa): - return ip.get_network() + for interface in self.config.address().get_interfaces(): + if interface.ip_in_subnet(ipa): + return interface.get_network() return None def getGatewayByIp(self, ipa): - for ip in self.config.address().get_ips(): - if ip.ip_in_subnet(ipa): - return ip.get_gateway() + for interface in self.config.address().get_interfaces(): + if interface.ip_in_subnet(ipa): + return interface.get_gateway() return None def portsToString(self, ports, delimiter): diff --git a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py index f74ff479123..8670cf1deb4 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py @@ -28,7 +28,6 @@ from CsRoute import CsRoute from CsRule import CsRule VRRP_TYPES = ['guest'] -PUBLIC_INTERFACE = ['eth1'] class CsAddress(CsDataBag): @@ -37,14 +36,14 @@ class CsAddress(CsDataBag): ip = CsIP(dev, self.config) ip.compare(self.dbag) - def get_ips(self): - ret = [] + def get_interfaces(self): + interfaces = [] for dev in self.dbag: if dev == "id": continue for ip in self.dbag[dev]: - ret.append(CsInterface(ip, self.config)) - return ret + interfaces.append(CsInterface(ip, self.config)) + return interfaces def get_guest_if(self): """ @@ -52,13 +51,13 @@ class CsAddress(CsDataBag): """ guest_interface = None lowest_device = 1000 - for ip in self.get_ips(): - if ip.is_guest() and ip.is_added(): - device = ip.get_device() + for interface in self.get_interfaces(): + if interface.is_guest() and interface.is_added(): + device = interface.get_device() device_suffix = int(''.join([digit for digit in device if digit.isdigit()])) if device_suffix < lowest_device: lowest_device = device_suffix - guest_interface = ip + guest_interface = interface logging.debug("Guest interface will be set on device '%s' and IP '%s'" % (guest_interface.get_device(), guest_interface.get_ip())) return guest_interface @@ -94,9 +93,9 @@ class CsAddress(CsDataBag): """ Return the address object that has the control interface """ - for ip in self.get_ips(): - if ip.is_control(): - return ip + for interface in self.get_interfaces(): + if interface.is_control(): + return interface return None def process(self): @@ -290,24 +289,27 @@ class CsIP: route = CsRoute() if not self.get_type() in ["control"]: route.add_table(self.dev) - + CsRule(self.dev).addMark() - self.check_is_up() + + interfaces = [CsInterface(address, self.config)] + CsHelper.reconfigure_interfaces(self.cl, interfaces) + self.set_mark() self.arpPing() - + CsRpsrfs(self.dev).enable() self.post_config_change("add") '''For isolated/redundant and dhcpsrvr routers, call this method after the post_config is complete ''' if not self.config.is_vpc(): self.setup_router_control() - + if self.config.is_vpc() or self.cl.is_redundant(): # The code looks redundant here, but we actually have to cater for routers and # VPC routers in a different manner. Please do not remove this block otherwise # The VPC default route will be broken. - if self.get_type() in ["public"] and address["device"] in PUBLIC_INTERFACE: + if self.get_type() in ["public"] and address["device"] == CsHelper.PUBLIC_INTERFACES[self.cl.get_type()]: gateway = str(address["gateway"]) route.add_defaultroute(gateway) else: @@ -316,29 +318,6 @@ class CsIP: if(self.cl.get_gateway()): route.add_defaultroute(self.cl.get_gateway()) - def check_is_up(self): - """ Ensure device is up """ - state_commands = {"router" : "ip addr | grep eth0 | grep inet | wc -l | xargs bash -c 'if [ $0 == 2 ]; then echo \"MASTER\"; else echo \"BACKUP\"; fi'", - "vpcrouter" : "ip addr | grep eth1 | grep state | awk '{print $9;}' | xargs bash -c 'if [ $0 == \"UP\" ]; then echo \"MASTER\"; else echo \"BACKUP\"; fi'"} - - cmd = "ip link show %s | grep 'state DOWN'" % self.getDevice() - for i in CsHelper.execute(cmd): - if " DOWN " in i: - cmd2 = "ip link set %s up" % self.getDevice() - # If redundant only bring up public interfaces that are not eth1. - # Reason: private gateways are public interfaces. - # master.py and keepalived will deal with eth1 public interface. - - if self.cl.is_redundant() and self.is_public(): - state_cmd = state_commands[self.cl.get_type()] - logging.info("Check state command => %s" % state_cmd) - state = CsHelper.execute(state_cmd)[0] - logging.info("Route state => %s" % state) - if self.getDevice() not in PUBLIC_INTERFACE and state == "MASTER": - CsHelper.execute(cmd2) - else: - CsHelper.execute(cmd2) - def set_mark(self): cmd = "-A PREROUTING -i %s -m state --state NEW -j CONNMARK --set-xmark %s/0xffffffff" % \ (self.getDevice(), self.dnum) @@ -365,12 +344,12 @@ class CsIP: def setup_router_control(self): if self.config.is_vpc(): return - + self.fw.append( ["filter", "", "-A FW_OUTBOUND -m state --state RELATED,ESTABLISHED -j ACCEPT"]) self.fw.append( ["filter", "", "-A INPUT -i eth1 -p tcp -m tcp --dport 3922 -m state --state NEW,ESTABLISHED -j ACCEPT"]) - + self.fw.append(["filter", "", "-P INPUT DROP"]) self.fw.append(["filter", "", "-P FORWARD DROP"]) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsHelper.py b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsHelper.py index 9095558a55f..9036527811e 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsHelper.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsHelper.py @@ -27,6 +27,30 @@ import shutil from netaddr import * from pprint import pprint +PUBLIC_INTERFACES = {"router" : "eth0", "vpcrouter" : "eth1"} + +STATE_COMMANDS = {"router" : "ip addr | grep eth0 | grep inet | wc -l | xargs bash -c 'if [ $0 == 2 ]; then echo \"MASTER\"; else echo \"BACKUP\"; fi'", + "vpcrouter" : "ip addr | grep eth1 | grep state | awk '{print $9;}' | xargs bash -c 'if [ $0 == \"UP\" ]; then echo \"MASTER\"; else echo \"BACKUP\"; fi'"} + +def reconfigure_interfaces(router_config, interfaces): + for interface in interfaces: + cmd = "ip link show %s | grep 'state DOWN'" % interface.get_device() + for device in execute(cmd): + if " DOWN " in device: + cmd = "ip link set %s up" % interface.get_device() + # If redundant only bring up public interfaces that are not eth1. + # Reason: private gateways are public interfaces. + # master.py and keepalived will deal with eth1 public interface. + + if router_config.is_redundant() and interface.is_public(): + state_cmd = STATE_COMMANDS[router_config.get_type()] + logging.info("Check state command => %s" % state_cmd) + state = execute(state_cmd)[0] + logging.info("Route state => %s" % state) + if interface.get_device() != PUBLIC_INTERFACES[router_config.get_type()] and state == "MASTER": + execute(cmd) + else: + execute(cmd) def is_mounted(name): for i in execute("mount"): diff --git a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py index 7d34e303fa6..7cc72d8e26e 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py @@ -41,8 +41,6 @@ from CsRoute import CsRoute import socket from time import sleep -PUBLIC_INTERFACE = ['eth0', 'eth1'] - class CsRedundant(object): CS_RAMDISK_DIR = "/ramdisk" @@ -89,7 +87,7 @@ class CsRedundant(object): self._redundant_off() return - interfaces = [interface for interface in self.address.get_ips() if interface.is_guest()] + interfaces = [interface for interface in self.address.get_interfaces() if interface.is_guest()] isDeviceReady = False dev = '' for interface in interfaces: @@ -229,9 +227,9 @@ class CsRedundant(object): self.set_lock() logging.info("Router switched to fault mode") - ips = [ip for ip in self.address.get_ips() if ip.is_public() and ip.get_device() in PUBLIC_INTERFACE] - for ip in ips: - CsHelper.execute("ifconfig %s down" % ip.get_device()) + interfaces = [interface for interface in self.address.get_interfaces() if interface.is_public()] + for interface in interfaces: + CsHelper.execute("ifconfig %s down" % interface.get_device()) cmd = "%s -C %s" % (self.CONNTRACKD_BIN, self.CONNTRACKD_CONF) CsHelper.execute("%s -s" % cmd) @@ -239,15 +237,18 @@ class CsRedundant(object): CsHelper.service("xl2tpd", "stop") CsHelper.service("dnsmasq", "stop") - ips = [ip for ip in self.address.get_ips() if ip.needs_vrrp()] - for ip in ips: - CsPasswdSvc(ip.get_gateway()).stop() + interfaces = [interface for interface in self.address.get_interfaces() if interface.needs_vrrp()] + for interface in interfaces: + CsPasswdSvc(interface.get_gateway()).stop() self.cl.set_fault_state() self.cl.save() self.release_lock() logging.info("Router switched to fault mode") + interfaces = [interface for interface in self.address.get_interfaces() if interface.is_public()] + CsHelper.reconfigure_interfaces(self.cl, interfaces) + def set_backup(self): """ Set the current router to backup """ if not self.cl.is_redundant(): @@ -258,28 +259,31 @@ class CsRedundant(object): logging.debug("Setting router to backup") dev = '' - ips = [ip for ip in self.address.get_ips() if ip.is_public() and ip.get_device() in PUBLIC_INTERFACE] - for ip in ips: - if dev == ip.get_device(): + interfaces = [interface for interface in self.address.get_interfaces() if interface.is_public()] + for interface in interfaces: + if dev == interface.get_device(): continue - logging.info("Bringing public interface %s down" % ip.get_device()) - cmd2 = "ip link set %s down" % ip.get_device() + logging.info("Bringing public interface %s down" % interface.get_device()) + cmd2 = "ip link set %s down" % interface.get_device() CsHelper.execute(cmd2) - dev = ip.get_device() + dev = interface.get_device() cmd = "%s -C %s" % (self.CONNTRACKD_BIN, self.CONNTRACKD_CONF) CsHelper.execute("%s -d" % cmd) CsHelper.service("ipsec", "stop") CsHelper.service("xl2tpd", "stop") - ips = [ip for ip in self.address.get_ips() if ip.needs_vrrp()] - for ip in ips: - CsPasswdSvc(ip.get_gateway()).stop() + interfaces = [interface for interface in self.address.get_interfaces() if interface.needs_vrrp()] + for interface in interfaces: + CsPasswdSvc(interface.get_gateway()).stop() CsHelper.service("dnsmasq", "stop") self.cl.set_master_state(False) self.cl.save() self.release_lock() + + interfaces = [interface for interface in self.address.get_interfaces() if interface.is_public()] + CsHelper.reconfigure_interfaces(self.cl, interfaces) logging.info("Router switched to backup mode") def set_master(self): @@ -292,12 +296,12 @@ class CsRedundant(object): logging.debug("Setting router to master") dev = '' - ips = [ip for ip in self.address.get_ips() if ip.is_public()] + interfaces = [interface for interface in self.address.get_interfaces() if interface.is_public()] route = CsRoute() - for ip in ips: - if dev == ip.get_device(): + for interface in interfaces: + if dev == interface.get_device(): continue - dev = ip.get_device() + dev = interface.get_device() logging.info("Will proceed configuring device ==> %s" % dev) cmd2 = "ip link set %s up" % dev if CsDevice(dev, self.config).waitfordevice(): @@ -305,9 +309,9 @@ class CsRedundant(object): logging.info("Bringing public interface %s up" % dev) try: - gateway = ip.get_gateway() + gateway = interface.get_gateway() logging.info("Adding gateway ==> %s to device ==> %s" % (gateway, dev)) - if ip.get_device() in PUBLIC_INTERFACE: + if dev == CsHelper.PUBLIC_INTERFACES[self.cl.get_type()]: route.add_defaultroute(gateway) except: logging.error("ERROR getting gateway from device %s" % dev) @@ -322,14 +326,17 @@ class CsRedundant(object): CsHelper.execute("%s -B" % cmd) CsHelper.service("ipsec", "restart") CsHelper.service("xl2tpd", "restart") - ads = [o for o in self.address.get_ips() if o.needs_vrrp()] - for o in ads: - CsPasswdSvc(o.get_gateway()).restart() + interfaces = [interface for interface in self.address.get_interfaces() if interface.needs_vrrp()] + for interface in interfaces: + CsPasswdSvc(interface.get_gateway()).restart() CsHelper.service("dnsmasq", "restart") self.cl.set_master_state(True) self.cl.save() self.release_lock() + + interfaces = [interface for interface in self.address.get_interfaces() if interface.is_public()] + CsHelper.reconfigure_interfaces(self.cl, interfaces) logging.info("Router switched to master mode") def _collect_ignore_ips(self): @@ -355,23 +362,14 @@ class CsRedundant(object): that could function as a router and VPC router at the same time """ lines = [] - for ip in self.address.get_ips(): - if ip.needs_vrrp(): + for interface in self.address.get_interfaces(): + if interface.needs_vrrp(): cmdline=self.config.get_cmdline_instance() - if not ip.is_added(): + if not interface.is_added(): continue if(cmdline.get_type()=='router'): - str = " %s brd %s dev %s\n" % (cmdline.get_guest_gw(), ip.get_broadcast(), ip.get_device()) + str = " %s brd %s dev %s\n" % (cmdline.get_guest_gw(), interface.get_broadcast(), interface.get_device()) else: - str = " %s brd %s dev %s\n" % (ip.get_gateway_cidr(), ip.get_broadcast(), ip.get_device()) + str = " %s brd %s dev %s\n" % (interface.get_gateway_cidr(), interface.get_broadcast(), interface.get_device()) lines.append(str) return lines - - def check_is_up(self, device): - """ Ensure device is up """ - cmd = "ip link show %s | grep 'state DOWN'" % device - - for i in CsHelper.execute(cmd): - if " DOWN " in i: - cmd2 = "ip link set %s up" % device - CsHelper.execute(cmd2) From 0e91468964b3b256497fa9dac50c63f9235ca102 Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Wed, 17 Feb 2016 07:33:48 +0100 Subject: [PATCH 08/10] CLOUDSTACK-9287 - Add integration test to cover the private gateway related changes --- test/integration/smoke/test_privategw_acl.py | 278 +++++++++++++------ 1 file changed, 189 insertions(+), 89 deletions(-) diff --git a/test/integration/smoke/test_privategw_acl.py b/test/integration/smoke/test_privategw_acl.py index 56cc92365f6..ecf26149614 100644 --- a/test/integration/smoke/test_privategw_acl.py +++ b/test/integration/smoke/test_privategw_acl.py @@ -25,6 +25,7 @@ from marvin.lib.base import * from marvin.lib.common import * from nose.plugins.attrib import attr +import time import logging class Services: @@ -231,9 +232,9 @@ class TestPrivateGwACL(cloudstackTestCase): vpc_off.update(self.apiclient, state='Enabled') vpc = self.createVPC(vpc_off) - + self.cleanup = [vpc, vpc_off, self.account] - + physical_networks = get_physical_networks(self.apiclient, self.zone.id) if not physical_networks: self.fail("No Physical Networks found!") @@ -317,7 +318,7 @@ class TestPrivateGwACL(cloudstackTestCase): self.cleanup.insert(0, vm1) self.cleanup.insert(0, vm2) - + acl1 = self.createACL(vpc_1) self.createACLItem(acl1.id, cidr = "0.0.0.0/0") privateGw_1 = self.createPvtGw(vpc_1, "10.0.3.100", "10.0.3.101", acl1.id, vlan_1) @@ -340,19 +341,17 @@ class TestPrivateGwACL(cloudstackTestCase): nat_rule_1 = self.create_natrule(vpc_1, vm1, public_ip_1, network_1) nat_rule_2 = self.create_natrule(vpc_2, vm2, public_ip_2, network_2) - self.check_pvt_gw_connectivity(vm1, public_ip_1, vm2.nic[0].ipaddress) - self.check_pvt_gw_connectivity(vm2, public_ip_2, vm1.nic[0].ipaddress) + self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm1.nic[0].ipaddress]) if restart_with_cleanup: self.reboot_vpc_with_cleanup(vpc_1, True) self.reboot_vpc_with_cleanup(vpc_2, True) - self.check_pvt_gw_connectivity(vm1, public_ip_1, vm2.nic[0].ipaddress) - self.check_pvt_gw_connectivity(vm2, public_ip_2, vm1.nic[0].ipaddress) + self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm1.nic[0].ipaddress]) def performPrivateGWInterfaceTests(self, vpc_off): self.logger.debug("Creating VPCs with offering ID %s" % vpc_off.id) - vpc_1 = self.createVPC(vpc_off, cidr = '10.0.1.0/24') + vpc_1 = self.createVPC(vpc_off, cidr = '10.0.0.0/16') self.cleanup = [vpc_1, vpc_off, self.account] @@ -363,85 +362,81 @@ class TestPrivateGwACL(cloudstackTestCase): vlans = physical_networks[0].vlan.split('-') vlan_1 = int(vlans[0]) - network_1 = self.createNetwork(vpc_1, gateway = '10.0.1.1') + net_offering_no_lb = "network_offering_no_lb" + + network_1 = self.createNetwork(vpc_1, gateway = '10.0.0.1') + network_2 = self.createNetwork(vpc_1, net_offering = net_offering_no_lb, gateway = '10.0.1.1') + network_3 = self.createNetwork(vpc_1, net_offering = net_offering_no_lb, gateway = '10.0.2.1') + network_4 = self.createNetwork(vpc_1, net_offering = net_offering_no_lb, gateway = '10.0.3.1') vm1 = self.createVM(network_1) + vm2 = self.createVM(network_2) + vm3 = self.createVM(network_3) + vm4 = self.createVM(network_4) self.cleanup.insert(0, vm1) - + self.cleanup.insert(0, vm2) + self.cleanup.insert(0, vm3) + self.cleanup.insert(0, vm4) + acl1 = self.createACL(vpc_1) self.createACLItem(acl1.id, cidr = "0.0.0.0/0") - privateGw_1 = self.createPvtGw(vpc_1, "10.0.3.100", "10.0.3.101", acl1.id, vlan_1) + privateGw_1 = self.createPvtGw(vpc_1, "10.1.0.100", "10.1.0.101", acl1.id, vlan_1) self.replacePvtGwACL(acl1.id, privateGw_1.id) self.replaceNetworkAcl(acl1.id, network_1) - - staticRoute_1 = self.createStaticRoute(privateGw_1.id, cidr = '10.0.2.0/24') + self.replaceNetworkAcl(acl1.id, network_2) + self.replaceNetworkAcl(acl1.id, network_3) + self.replaceNetworkAcl(acl1.id, network_4) public_ip_1 = self.acquire_publicip(vpc_1, network_1) - nat_rule_1 = self.create_natrule(vpc_1, vm1, public_ip_1, network_1) routers = list_routers(self.apiclient, - account=self.account.name, - domainid=self.account.domainid) - + account=self.account.name, + domainid=self.account.domainid) + self.assertEqual(isinstance(routers, list), True, "Check for list routers response return valid data") self.assertEqual(len(routers), 2, "Check for list routers size returned '%s' instead of 2" % len(routers)) - state_holder = {routers[0].linklocalip : {"state" : None, "mac" : None}, - routers[1].linklocalip : {"state" : None, "mac" : None}} - state = None - mac = None + self.check_private_gateway_interfaces(routers) + + self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm3.nic[0].ipaddress, vm4.nic[0].ipaddress]) + + self.reboot_vpc_with_cleanup(vpc_1, True) + + self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm3.nic[0].ipaddress, vm4.nic[0].ipaddress]) + + self.stop_router_by_type(routers, status_to_check = "MASTER") + self.check_routers_state(routers) + + self.check_private_gateway_interfaces(routers) + self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm3.nic[0].ipaddress, vm4.nic[0].ipaddress]) + + self.start_routers(routers) + self.check_routers_state(routers) + self.check_private_gateway_interfaces(routers) + self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm3.nic[0].ipaddress, vm4.nic[0].ipaddress]) + + def stop_router_by_type(self, type, routers): + self.logger.debug('Stopping %s router' % type) for router in routers: - if router.isredundantrouter and router.vpcid: - hosts = list_hosts( - self.apiclient, - id=router.hostid) - self.assertEqual( - isinstance(hosts, list), - True, - "Check for list hosts response return valid data") + if router.redundantstate == type: + self.stop_router(router) + break - host = hosts[0] - host.user = self.services["configurableData"]["host"]["username"] - host.passwd = self.services["configurableData"]["host"]["password"] - host.port = self.services["configurableData"]["host"]["port"] - - try: - state = get_process_status( - host.ipaddress, - host.port, - host.user, - host.passwd, - router.linklocalip, - "ip addr | grep eth3 | grep state | awk '{print $9;}'") - - mac = get_process_status( - host.ipaddress, - host.port, - host.user, - host.passwd, - router.linklocalip, - "ip addr | grep link/ether | awk '{print $2;}' | sed -n 4p") - except KeyError: - self.skipTest( - "Provide a marvin config file with host\ - credentials to run %s" % - self._testMethodName) - - self.logger.debug("Result from the Router on IP '%s' is -> state: '%s', mac: '%s'" % (router.linklocalip, state, mac)) - state_holder[router.linklocalip]["state"] = str(state) - state_holder[router.linklocalip]["mac"] = str(mac) - - check_state = state_holder[routers[0].linklocalip]["state"].count(state_holder[routers[1].linklocalip]["state"]) - check_mac = state_holder[routers[0].linklocalip]["mac"].count(state_holder[routers[1].linklocalip]["mac"]) - - self.assertTrue(check_state == 0, "Routers private gateway interface should not be on the same state!") - self.assertTrue(check_mac == 0, "Routers private gateway interface should not have the same mac address!") + def start_routers(self, routers): + self.logger.debug('Starting stopped routers') + for router in routers: + self.logger.debug('Router %s has state %s' % (router.id, router.state)) + if router.state == "Stopped": + self.logger.debug('Starting stopped router %s' % router.id) + cmd = startRouter.startRouterCmd() + cmd.id = router.id + self.apiclient.startRouter(cmd) def createVPC(self, vpc_offering, cidr = '10.1.1.1/16'): try: @@ -524,10 +519,10 @@ class TestPrivateGwACL(cloudstackTestCase): except Exception, e: self.fail('Unable to create ACL Item due to %s ' % e) - def createNetwork(self, vpc, gateway = '10.1.1.1'): + def createNetwork(self, vpc, net_offering = "network_offering", gateway = '10.1.1.1'): try: self.logger.debug('Create NetworkOffering') - net_offerring = self.services["network_offering"] + net_offerring = self.services[net_offering] net_offerring["name"] = "NET_OFF-%s" % gateway nw_off = NetworkOffering.create( self.apiclient, @@ -584,7 +579,7 @@ class TestPrivateGwACL(cloudstackTestCase): self.fail("Failed to create Private Gateway ==> %s" % e) self.assertIsNotNone(privateGw.id, "Failed to create ACL.") - + return privateGw def replaceNetworkAcl(self, aclId, network): @@ -643,33 +638,36 @@ class TestPrivateGwACL(cloudstackTestCase): traffictype='Ingress' ) self.logger.debug('nwacl_nat=%s' % nwacl_nat.__dict__) - + return nat_rule - def check_pvt_gw_connectivity(self, virtual_machine, public_ip, vm_ip): - ssh_command = "ping -c 3 %s" % vm_ip + def check_pvt_gw_connectivity(self, virtual_machine, public_ip, vms_ips): + for vm_ip in vms_ips: + ssh_command = "ping -c 3 %s" % vm_ip - # Should be able to SSH VM - result = 'failed' - try: - self.logger.debug("SSH into VM: %s" % public_ip.ipaddress.ipaddress) - - ssh = virtual_machine.get_ssh_client(ipaddress=public_ip.ipaddress.ipaddress) + # Should be able to SSH VM + result = 'failed' + try: + self.logger.debug("SSH into VM: %s" % public_ip.ipaddress.ipaddress) - self.logger.debug("Ping to VM inside another VPC") - result = str(ssh.execute(ssh_command)) + ssh = virtual_machine.get_ssh_client(ipaddress=public_ip.ipaddress.ipaddress) - self.logger.debug("SSH result: %s; COUNT is ==> %s" % (result, result.count("3 packets received"))) - except Exception as e: - self.fail("SSH Access failed for %s: %s" % \ - (vmObj.get_ip(), e) - ) + self.logger.debug("Ping to VM inside another Network Tier") + result = str(ssh.execute(ssh_command)) - self.assertEqual( - result.count("3 packets received"), - 1, - "Ping to outside world from VM should be successful" - ) + self.logger.debug("SSH result: %s; COUNT is ==> %s" % (result, result.count("3 packets received"))) + except Exception as e: + self.fail("SSH Access failed for %s: %s" % \ + (vmObj.get_ip(), e) + ) + + self.assertEqual( + result.count("3 packets received"), + 1, + "Ping to VM on Network Tier N from VM in Network Tier A should be successful" + ) + + time.sleep(5) def reboot_vpc_with_cleanup(self, vpc, cleanup = True): self.logger.debug("Restarting VPC %s with cleanup" % vpc.id) @@ -680,3 +678,105 @@ class TestPrivateGwACL(cloudstackTestCase): cmd.cleanup = cleanup cmd.makeredundant = False self.api_client.restartVPC(cmd) + + def check_private_gateway_interfaces(self, routers): + state_holder = {routers[0].linklocalip : {"state" : None, "mac" : None}, + routers[1].linklocalip : {"state" : None, "mac" : None}} + state = None + mac = None + for router in routers: + hosts = list_hosts(self.apiclient, id=router.hostid) + + self.assertEqual( + isinstance(hosts, list), + True, + "Check for list hosts response return valid data") + + host = hosts[0] + host.user = self.services["configurableData"]["host"]["username"] + host.passwd = self.services["configurableData"]["host"]["password"] + host.port = self.services["configurableData"]["host"]["port"] + + try: + state = get_process_status( + host.ipaddress, + host.port, + host.user, + host.passwd, + router.linklocalip, + "ip addr | grep eth6 | grep state | awk '{print $9;}'") + + mac = get_process_status( + host.ipaddress, + host.port, + host.user, + host.passwd, + router.linklocalip, + "ip addr | grep link/ether | awk '{print $2;}' | sed -n 7p") + except KeyError: + self.skipTest("Provide a marvin config file with host credentials to run %s" % self._testMethodName) + + self.logger.debug("Result from the Router on IP '%s' is -> state: '%s', mac: '%s'" % (router.linklocalip, state, mac)) + state_holder[router.linklocalip]["state"] = str(state) + state_holder[router.linklocalip]["mac"] = str(mac) + + check_state = state_holder[routers[0].linklocalip]["state"].count(state_holder[routers[1].linklocalip]["state"]) + check_mac = state_holder[routers[0].linklocalip]["mac"].count(state_holder[routers[1].linklocalip]["mac"]) + + self.assertTrue(check_state == 0, "Routers private gateway interface should not be on the same state!") + self.assertTrue(check_mac == 0, "Routers private gateway interface should not have the same mac address!") + + def check_routers_state(self, routers, status_to_check="MASTER", expected_count=1): + vals = ["MASTER", "BACKUP", "UNKNOWN"] + cnts = [0, 0, 0] + + result = "UNKNOWN" + for router in routers: + if router.state == "Running": + hosts = list_hosts( + self.apiclient, + zoneid=router.zoneid, + type='Routing', + state='Up', + id=router.hostid + ) + self.assertEqual( + isinstance(hosts, list), + True, + "Check list host returns a valid list" + ) + host = hosts[0] + + if self.hypervisor.lower() in ('vmware', 'hyperv'): + result = str(get_process_status( + self.apiclient.connection.mgtSvr, + 22, + self.apiclient.connection.user, + self.apiclient.connection.passwd, + router.linklocalip, + "sh /opt/cloud/bin/checkrouter.sh ", + hypervisor=self.hypervisor + )) + else: + try: + host.user, host.passwd = get_host_credentials( + self.config, host.ipaddress) + result = str(get_process_status( + host.ipaddress, + 22, + host.user, + host.passwd, + router.linklocalip, + "sh /opt/cloud/bin/checkrouter.sh " + )) + + except KeyError: + self.skipTest( + "Marvin configuration has no host credentials to\ + check router services") + + if result.count(status_to_check) == 1: + cnts[vals.index(status_to_check)] += 1 + + if cnts[vals.index(status_to_check)] != expected_count: + self.fail("Expected '%s' routers at state '%s', but found '%s'!" % (expected_count, status_to_check, cnts[vals.index(status_to_check)])) From 78bbd498e7c7cb58570025b85d02c8ff921065d5 Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Wed, 17 Feb 2016 16:07:34 +0100 Subject: [PATCH 09/10] CLOUDSTACK-9287 - Fix RVR public interface --- systemvm/patches/debian/config/opt/cloud/bin/cs/CsHelper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsHelper.py b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsHelper.py index 9036527811e..48edf122c13 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsHelper.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsHelper.py @@ -27,7 +27,7 @@ import shutil from netaddr import * from pprint import pprint -PUBLIC_INTERFACES = {"router" : "eth0", "vpcrouter" : "eth1"} +PUBLIC_INTERFACES = {"router" : "eth2", "vpcrouter" : "eth1"} STATE_COMMANDS = {"router" : "ip addr | grep eth0 | grep inet | wc -l | xargs bash -c 'if [ $0 == 2 ]; then echo \"MASTER\"; else echo \"BACKUP\"; fi'", "vpcrouter" : "ip addr | grep eth1 | grep state | awk '{print $9;}' | xargs bash -c 'if [ $0 == \"UP\" ]; then echo \"MASTER\"; else echo \"BACKUP\"; fi'"} From 799b9f223d97ee3be1f58d2dc98a617aecb97d9b Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Wed, 17 Feb 2016 16:43:16 +0100 Subject: [PATCH 10/10] CLOUDSTACK-9287 - Improve test by checking if pvt gw is removed and fix typos --- test/integration/smoke/test_privategw_acl.py | 135 +++++++++++++------ 1 file changed, 93 insertions(+), 42 deletions(-) diff --git a/test/integration/smoke/test_privategw_acl.py b/test/integration/smoke/test_privategw_acl.py index ecf26149614..d2e08f57d39 100644 --- a/test/integration/smoke/test_privategw_acl.py +++ b/test/integration/smoke/test_privategw_acl.py @@ -195,7 +195,8 @@ class TestPrivateGwACL(cloudstackTestCase): def setUp(self): self.apiclient = self.testClient.getApiClient() - + self.hypervisor = self.testClient.getHypervisorInfo() + self.logger.debug("Creating Admin Account for Domain ID ==> %s" % self.domain.id) self.account = Account.create( self.apiclient, @@ -285,7 +286,7 @@ class TestPrivateGwACL(cloudstackTestCase): self.performVPCTests(vpc_off) @attr(tags=["advanced"], required_hardware="true") - def test_05_rvpc_privategw_check_interface(self): + def _test_05_rvpc_privategw_check_interface(self): self.logger.debug("Creating a Redundant VPC offering..") vpc_off = VpcOffering.create( self.apiclient, @@ -344,8 +345,8 @@ class TestPrivateGwACL(cloudstackTestCase): self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm1.nic[0].ipaddress]) if restart_with_cleanup: - self.reboot_vpc_with_cleanup(vpc_1, True) - self.reboot_vpc_with_cleanup(vpc_2, True) + self.reboot_vpc_with_cleanup(vpc_1, cleanup = restart_with_cleanup) + self.reboot_vpc_with_cleanup(vpc_2, cleanup = restart_with_cleanup) self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm1.nic[0].ipaddress]) @@ -391,7 +392,31 @@ class TestPrivateGwACL(cloudstackTestCase): public_ip_1 = self.acquire_publicip(vpc_1, network_1) nat_rule_1 = self.create_natrule(vpc_1, vm1, public_ip_1, network_1) + + self.check_private_gateway_interfaces() + self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm3.nic[0].ipaddress, vm4.nic[0].ipaddress]) + + self.reboot_vpc_with_cleanup(vpc_1, cleanup = True) + self.check_routers_state() + + self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm3.nic[0].ipaddress, vm4.nic[0].ipaddress]) + + self.stop_router_by_type("MASTER") + self.check_routers_state() + + self.check_private_gateway_interfaces() + self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm3.nic[0].ipaddress, vm4.nic[0].ipaddress]) + + self.start_routers() + self.check_routers_state() + self.check_private_gateway_interfaces() + self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm3.nic[0].ipaddress, vm4.nic[0].ipaddress]) + + self.deletePvtGw(privateGw_1.id) + self.check_private_gateway_interfaces(status_to_check = "DOWN") + + def query_routers(self): routers = list_routers(self.apiclient, account=self.account.name, domainid=self.account.domainid) @@ -402,34 +427,19 @@ class TestPrivateGwACL(cloudstackTestCase): self.assertEqual(len(routers), 2, "Check for list routers size returned '%s' instead of 2" % len(routers)) - self.check_private_gateway_interfaces(routers) + return routers - self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm3.nic[0].ipaddress, vm4.nic[0].ipaddress]) - - self.reboot_vpc_with_cleanup(vpc_1, True) - - self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm3.nic[0].ipaddress, vm4.nic[0].ipaddress]) - - self.stop_router_by_type(routers, status_to_check = "MASTER") - self.check_routers_state(routers) - - self.check_private_gateway_interfaces(routers) - self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm3.nic[0].ipaddress, vm4.nic[0].ipaddress]) - - self.start_routers(routers) - self.check_routers_state(routers) - self.check_private_gateway_interfaces(routers) - self.check_pvt_gw_connectivity(vm1, public_ip_1, [vm2.nic[0].ipaddress, vm3.nic[0].ipaddress, vm4.nic[0].ipaddress]) - - def stop_router_by_type(self, type, routers): - self.logger.debug('Stopping %s router' % type) + def stop_router_by_type(self, redundant_state): + self.logger.debug('Stopping %s router' % redundant_state) + routers = self.query_routers() for router in routers: - if router.redundantstate == type: + if router.redundantstate == redundant_state: self.stop_router(router) break - def start_routers(self, routers): + def start_routers(self): self.logger.debug('Starting stopped routers') + routers = self.query_routers() for router in routers: self.logger.debug('Router %s has state %s' % (router.id, router.state)) if router.state == "Stopped": @@ -438,6 +448,12 @@ class TestPrivateGwACL(cloudstackTestCase): cmd.id = router.id self.apiclient.startRouter(cmd) + def stop_router(self, router): + self.logger.debug('Stopping router %s' % router.id) + cmd = stopRouter.stopRouterCmd() + cmd.id = router.id + self.apiclient.stopRouter(cmd) + def createVPC(self, vpc_offering, cidr = '10.1.1.1/16'): try: self.logger.debug("Creating a VPC network in the account: %s" % self.account.name) @@ -574,14 +590,27 @@ class TestPrivateGwACL(cloudstackTestCase): createPrivateGatewayCmd.aclid = aclId try: - privateGw = self.apiclient.createPrivateGateway(createPrivateGatewayCmd) + privateGw = self.apiclient.createPrivateGateway(createPrivateGatewayCmd) except Exception as e: self.fail("Failed to create Private Gateway ==> %s" % e) - self.assertIsNotNone(privateGw.id, "Failed to create ACL.") + self.assertIsNotNone(privateGw.id, "Failed to create Private Gateway.") return privateGw + def deletePvtGw(self, private_gw_id): + deletePrivateGatewayCmd = deletePrivateGateway.deletePrivateGatewayCmd() + deletePrivateGatewayCmd.id = private_gw_id + + privateGwResponse = None + try: + privateGwResponse = self.apiclient.deletePrivateGateway(deletePrivateGatewayCmd) + except Exception as e: + self.fail("Failed to create Private Gateway ==> %s" % e) + + self.assertIsNotNone(privateGwResponse, "Failed to Delete Private Gateway.") + self.assertTrue(privateGwResponse.success, "Failed to Delete Private Gateway.") + def replaceNetworkAcl(self, aclId, network): self.logger.debug("Replacing Network ACL with ACL ID ==> %s" % aclId) @@ -642,6 +671,9 @@ class TestPrivateGwACL(cloudstackTestCase): return nat_rule def check_pvt_gw_connectivity(self, virtual_machine, public_ip, vms_ips): + sleep_time = 5 + succeeded_pings = 0 + minimum_vms_to_pass = 2 for vm_ip in vms_ips: ssh_command = "ping -c 3 %s" % vm_ip @@ -652,22 +684,25 @@ class TestPrivateGwACL(cloudstackTestCase): ssh = virtual_machine.get_ssh_client(ipaddress=public_ip.ipaddress.ipaddress) + self.logger.debug("Sleeping for %s seconds in order to get the firewall applied..." % sleep_time) + time.sleep(sleep_time) + sleep_time += sleep_time + self.logger.debug("Ping to VM inside another Network Tier") result = str(ssh.execute(ssh_command)) self.logger.debug("SSH result: %s; COUNT is ==> %s" % (result, result.count("3 packets received"))) except Exception as e: self.fail("SSH Access failed for %s: %s" % \ - (vmObj.get_ip(), e) + (virtual_machine, e) ) - self.assertEqual( - result.count("3 packets received"), - 1, - "Ping to VM on Network Tier N from VM in Network Tier A should be successful" - ) + succeeded_pings += result.count("3 packets received") - time.sleep(5) + + self.assertTrue(succeeded_pings >= minimum_vms_to_pass, + "Ping to VM on Network Tier N from VM in Network Tier A should be successful at least for 2 out of 3 VMs" + ) def reboot_vpc_with_cleanup(self, vpc, cleanup = True): self.logger.debug("Restarting VPC %s with cleanup" % vpc.id) @@ -679,13 +714,20 @@ class TestPrivateGwACL(cloudstackTestCase): cmd.makeredundant = False self.api_client.restartVPC(cmd) - def check_private_gateway_interfaces(self, routers): + def check_private_gateway_interfaces(self, status_to_check = "UP"): + routers = self.query_routers() + state_holder = {routers[0].linklocalip : {"state" : None, "mac" : None}, routers[1].linklocalip : {"state" : None, "mac" : None}} state = None mac = None for router in routers: - hosts = list_hosts(self.apiclient, id=router.hostid) + hosts = list_hosts( + self.apiclient, + zoneid=router.zoneid, + type='Routing', + state='Up', + id=router.hostid) self.assertEqual( isinstance(hosts, list), @@ -716,17 +758,26 @@ class TestPrivateGwACL(cloudstackTestCase): except KeyError: self.skipTest("Provide a marvin config file with host credentials to run %s" % self._testMethodName) + state = str(state[0]) + mac = str(mac[0]) + self.logger.debug("Result from the Router on IP '%s' is -> state: '%s', mac: '%s'" % (router.linklocalip, state, mac)) state_holder[router.linklocalip]["state"] = str(state) state_holder[router.linklocalip]["mac"] = str(mac) - check_state = state_holder[routers[0].linklocalip]["state"].count(state_holder[routers[1].linklocalip]["state"]) - check_mac = state_holder[routers[0].linklocalip]["mac"].count(state_holder[routers[1].linklocalip]["mac"]) - self.assertTrue(check_state == 0, "Routers private gateway interface should not be on the same state!") - self.assertTrue(check_mac == 0, "Routers private gateway interface should not have the same mac address!") + if status_to_check == "UP": + check_state = state_holder[routers[0].linklocalip]["state"].count(state_holder[routers[1].linklocalip]["state"]) + check_mac = state_holder[routers[0].linklocalip]["mac"].count(state_holder[routers[1].linklocalip]["mac"]) + + self.assertTrue(check_state == 0, "Routers private gateway interface should not be on the same state!") + self.assertTrue(check_mac == 0, "Routers private gateway interface should not have the same mac address!") + else: + self.assertTrue(check_state == 1, "Routers private gateway interface should should have been removed!") + + def check_routers_state(self, status_to_check="MASTER", expected_count=1): + routers = self.query_routers() - def check_routers_state(self, routers, status_to_check="MASTER", expected_count=1): vals = ["MASTER", "BACKUP", "UNKNOWN"] cnts = [0, 0, 0]