From 198bb55ba784adabe38a09d4db86277b55674938 Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Fri, 3 Jun 2016 11:30:17 +0200 Subject: [PATCH] Fix of ping usage in net_helpers.async_ping() net_helpers.async_ping() should pass when each of the ICMP echo requests is matched by a corresponding ICMP echo reply, and it should fail if a response is not received for some request. The async_ping implementation relies on the ping exit code: when ping returns nonzero exit code, RuntimeException would be consequentially thrown and async_ping assertion would thus fail. Current implementation of net_helpers.async_ping() is broken due to its usage of -c parameter of ping command and assumption that if _some_ of the ICMP replies does not arrive, ping would return nonzero exit code. However, Linux ping works in the way that if _at least one_ reply is received from any number of ICMP ping requests, result code is 0 (success) and thus no RuntimeException is thrown. This commit fixes assert_ping to be a reliable assertion guaranteeing that no ping request stays without reply. For simple bash reproducer and more thorough discussion of possible solutions, see the bug description. Closes-Bug: #1588731 Change-Id: I9257b94a8ebbfaf1c4266c1f8ce3097657bacee5 (cherry picked from commit 2dcacaae88970365350200ca58982a18e21c703d) --- neutron/tests/common/net_helpers.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/neutron/tests/common/net_helpers.py b/neutron/tests/common/net_helpers.py index 8bc34a826ec..5085c220d22 100644 --- a/neutron/tests/common/net_helpers.py +++ b/neutron/tests/common/net_helpers.py @@ -24,6 +24,7 @@ import select import shlex import signal import subprocess +import time import fixtures import netaddr @@ -95,12 +96,22 @@ def set_namespace_gateway(port_dev, gateway_ip): port_dev.route.add_gateway(gateway_ip) -def assert_ping(src_namespace, dst_ip, timeout=1, count=1): +def assert_ping(src_namespace, dst_ip, timeout=1, count=1, interval=1): ipversion = netaddr.IPAddress(dst_ip).version ping_command = 'ping' if ipversion == 4 else 'ping6' ns_ip_wrapper = ip_lib.IPWrapper(src_namespace) - ns_ip_wrapper.netns.execute([ping_command, '-c', count, '-W', timeout, - dst_ip]) + + # See bug 1588731 for explanation why using -c count ping option + # cannot be used and it needs to be done using the following workaround. + for _index in range(count): + start_time = time.time() + ns_ip_wrapper.netns.execute([ping_command, '-c', '1', '-W', timeout, + dst_ip]) + end_time = time.time() + diff = end_time - start_time + if 0 < diff < interval: + # wait at most "interval" seconds between individual pings + time.sleep(interval - diff) @contextlib.contextmanager