From 2618726458d2cb75a39521bf9fd71c6bfd1c0c67 Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Tue, 23 Aug 2016 15:45:25 -0400 Subject: [PATCH] functional: Make trunk tests more robust New methods for connection tester are introduced in this patch. They send certain amount of icmp packets and then compare the results, so we succeed in positive tests only when all packets were replied. We succeed in negative tests only when all packets were lost. Both approaches are wrapped by actively waiting for successful result so we don't fail in case where we test connectivity while resources are not wired yet. This change is a followup to https://review.openstack.org/#/c/335536/ to improve stability of its functional tests. Closes-Bug: 1617319 Change-Id: I907ebd790f4ba3b4ecb0dce711c9f7d2c5244765 --- neutron/tests/common/conn_testers.py | 88 +++++++++++++------ .../openvswitch/agent/test_trunk_manager.py | 18 ++-- 2 files changed, 69 insertions(+), 37 deletions(-) diff --git a/neutron/tests/common/conn_testers.py b/neutron/tests/common/conn_testers.py index f438ff207da..b0927158192 100644 --- a/neutron/tests/common/conn_testers.py +++ b/neutron/tests/common/conn_testers.py @@ -21,6 +21,7 @@ from oslo_utils import uuidutils from neutron.agent import firewall from neutron.agent.linux import ip_lib from neutron.common import constants as n_consts +from neutron.common import utils as common_utils from neutron.tests.common import machine_fixtures from neutron.tests.common import net_helpers @@ -48,6 +49,23 @@ def _validate_direction(f): return wrap +def _get_packets_sent_received(src_namespace, dst_ip, count): + pinger = net_helpers.Pinger(src_namespace, dst_ip, count=count) + pinger.start() + pinger.wait() + return pinger.sent, pinger.received + + +def all_replied(src_ns, dst_ip, count): + sent, received = _get_packets_sent_received(src_ns, dst_ip, count) + return sent == received + + +def all_lost(src_ns, dst_ip, count): + sent, received = _get_packets_sent_received(src_ns, dst_ip, count) + return received == 0 + + class ConnectionTester(fixtures.Fixture): """Base class for testers @@ -64,10 +82,11 @@ class ConnectionTester(fixtures.Fixture): ARP = n_consts.ETHERTYPE_NAME_ARP INGRESS = firewall.INGRESS_DIRECTION EGRESS = firewall.EGRESS_DIRECTION - ICMP_COUNT = 1 def __init__(self, ip_cidr): self.ip_cidr = ip_cidr + self.icmp_count = 3 + self.connectivity_timeout = 12 def _setUp(self): self._protocol_to_method = { @@ -158,8 +177,7 @@ class ConnectionTester(fixtures.Fixture): icmp_timeout = ICMP_VERSION_TIMEOUTS[ip_version] try: net_helpers.assert_ping(src_namespace, ip_address, - timeout=icmp_timeout, - count=self.ICMP_COUNT) + timeout=icmp_timeout) except RuntimeError: raise ConnectionTesterException( "ICMP packets can't get from %s namespace to %s address" % ( @@ -318,6 +336,28 @@ class ConnectionTester(fixtures.Fixture): 'No Host Destination Unreachable packets were received when ' 'sending icmp packets to %s' % destination) + def wait_for_connection(self, direction): + src_ns, dst_ip = self._get_namespace_and_address( + direction) + all_replied_predicate = functools.partial( + all_replied, src_ns, dst_ip, count=self.icmp_count) + common_utils.wait_until_true( + all_replied_predicate, timeout=self.connectivity_timeout, + exception=ConnectionTesterException( + "Not all ICMP packets replied from %s namespace to %s " + "address." % self._get_namespace_and_address(direction))) + + def wait_for_no_connection(self, direction): + src_ns, dst_ip = self._get_namespace_and_address( + direction) + all_lost_predicate = functools.partial( + all_lost, src_ns, dst_ip, count=self.icmp_count) + common_utils.wait_until_true( + all_lost_predicate, timeout=self.connectivity_timeout, + exception=ConnectionTesterException( + "At least one packet got reply from %s namespace to %s " + "address." % self._get_namespace_and_address(direction))) + class OVSBaseConnectionTester(ConnectionTester): @@ -389,7 +429,6 @@ class OVSTrunkConnectionTester(OVSBaseConnectionTester): https://bugzilla.redhat.com/show_bug.cgi?id=1160340 """ - ICMP_COUNT = 3 def __init__(self, ip_cidr, br_trunk_name): super(OVSTrunkConnectionTester, self).__init__(ip_cidr) @@ -443,30 +482,27 @@ class OVSTrunkConnectionTester(OVSBaseConnectionTester): return self._peer2.namespace, self._ip_vlan return self._vm.namespace, self._peer2.ip - def test_sub_port_icmp_connectivity(self, direction): - - src_namespace, ip_address = self._get_subport_namespace_and_address( + def wait_for_sub_port_connectivity(self, direction): + src_ns, dst_ip = self._get_subport_namespace_and_address( direction) - ip_version = ip_lib.get_ip_version(ip_address) - icmp_timeout = ICMP_VERSION_TIMEOUTS[ip_version] - try: - net_helpers.assert_ping(src_namespace, ip_address, - timeout=icmp_timeout, - count=self.ICMP_COUNT) - except RuntimeError: - raise ConnectionTesterException( - "ICMP packets can't get from %s namespace to %s address" % ( - src_namespace, ip_address)) + all_replied_predicate = functools.partial( + all_replied, src_ns, dst_ip, count=self.icmp_count) + common_utils.wait_until_true( + all_replied_predicate, timeout=self.connectivity_timeout, + exception=ConnectionTesterException( + "ICMP traffic from %s namespace to subport with address %s " + "can't get through." % (src_ns, dst_ip))) - def test_sub_port_icmp_no_connectivity(self, direction): - try: - self.test_sub_port_icmp_connectivity(direction) - except ConnectionTesterException: - pass - else: - raise ConnectionTesterException( - 'Established %s connection with protocol ICMP, ' % ( - direction)) + def wait_for_sub_port_no_connectivity(self, direction): + src_ns, dst_ip = self._get_subport_namespace_and_address( + direction) + all_lost_predicate = functools.partial( + all_lost, src_ns, dst_ip, count=self.icmp_count) + common_utils.wait_until_true( + all_lost_predicate, timeout=self.connectivity_timeout, + exception=ConnectionTesterException( + "ICMP traffic from %s namespace to subport with address %s " + "can still get through." % (src_ns, dst_ip))) class LinuxBridgeConnectionTester(ConnectionTester): diff --git a/neutron/tests/functional/services/trunk/drivers/openvswitch/agent/test_trunk_manager.py b/neutron/tests/functional/services/trunk/drivers/openvswitch/agent/test_trunk_manager.py index 3075b17e0a5..aff42c343d6 100644 --- a/neutron/tests/functional/services/trunk/drivers/openvswitch/agent/test_trunk_manager.py +++ b/neutron/tests/functional/services/trunk/drivers/openvswitch/agent/test_trunk_manager.py @@ -164,7 +164,6 @@ class TrunkManagerTestCase(base.BaseSudoTestCase): self.trunk = trunk_manager.TrunkParentPort( trunk_id, uuidutils.generate_uuid()) - # TODO(jlibosva): Replace all tester methods with more robust tests def test_connectivity(self): """Test connectivity with trunk and sub ports. @@ -193,10 +192,8 @@ class TrunkManagerTestCase(base.BaseSudoTestCase): conn_testers.OVSBaseConnectionTester.set_tag( self.trunk.patch_port_int_name, self.tester.bridge, vlan_net1) - self.tester.assert_connection(protocol=self.tester.ICMP, - direction=self.tester.INGRESS) - self.tester.assert_connection(protocol=self.tester.ICMP, - direction=self.tester.EGRESS) + self.tester.wait_for_connection(self.tester.INGRESS) + self.tester.wait_for_connection(self.tester.EGRESS) self.tester.add_vlan_interface_and_peer(sub_port_segmentation_id, self.net2_cidr) @@ -217,15 +214,14 @@ class TrunkManagerTestCase(base.BaseSudoTestCase): conn_testers.OVSBaseConnectionTester.set_tag( sub_port.patch_port_int_name, self.tester.bridge, vlan_net2) - self.tester.test_sub_port_icmp_connectivity(self.tester.INGRESS) - self.tester.test_sub_port_icmp_connectivity(self.tester.EGRESS) + self.tester.wait_for_sub_port_connectivity(self.tester.INGRESS) + self.tester.wait_for_sub_port_connectivity(self.tester.EGRESS) self.trunk_manager.remove_sub_port(sub_port.trunk_id, sub_port.port_id) - self.tester.test_sub_port_icmp_no_connectivity(self.tester.INGRESS) - self.tester.test_sub_port_icmp_no_connectivity(self.tester.EGRESS) + self.tester.wait_for_sub_port_no_connectivity(self.tester.INGRESS) + self.tester.wait_for_sub_port_no_connectivity(self.tester.EGRESS) self.trunk_manager.remove_trunk(self.trunk.trunk_id, self.trunk.port_id) - self.tester.assert_no_connection(protocol=self.tester.ICMP, - direction=self.tester.INGRESS) + self.tester.wait_for_no_connection(self.tester.INGRESS)