diff --git a/test/lib/base.py b/test/lib/base.py index 27483847c..eefdd3781 100644 --- a/test/lib/base.py +++ b/test/lib/base.py @@ -18,6 +18,7 @@ import subprocess import time import itertools +import math import textwrap from colored import fg, attr @@ -25,11 +26,16 @@ from docker import APIClient as Client import netaddr +from lib.noseplugin import parser_option + DEFAULT_TEST_PREFIX = '' DEFAULT_TEST_BASE_DIR = '/tmp/gobgp' TEST_PREFIX = DEFAULT_TEST_PREFIX TEST_BASE_DIR = DEFAULT_TEST_BASE_DIR +DEFAULT_WAIT_TIMEOUT = 120 +DEFAULT_REACHABILITY_TIMEOUT = 20 +DEFAULT_ASSERT_RETRIES = 30 BGP_FSM_IDLE = 'idle' BGP_FSM_ACTIVE = 'active' @@ -81,10 +87,47 @@ TEST_NETWORK_LABEL = TEST_CONTAINER_LABEL -def local(s, capture=False): +def _timeout_scale(): + return max(float(getattr(parser_option, 'timeout_scale', 1.0)), 1.0) + + +def scale_timeout(timeout): + if timeout is None: + return None + return int(math.ceil(float(timeout) * _timeout_scale())) + + +def scale_count(count): + return int(math.ceil(float(count) * _timeout_scale())) + + +def local(s, capture=False, timeout=None): print('[localhost] local:', s) _env = {'NOSE_NOLOGCAPTURE': '1' if capture else '0'} - return subprocess.check_output(s, shell=True, env=_env).decode('utf-8').strip() + # Keep stderr visible in pytest logs. The Docker CLI often puts the useful + # failure reason there, while check_output only preserved stdout. + result = subprocess.run( + s, + shell=True, + env=_env, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=scale_timeout(timeout), + ) + stdout = result.stdout.decode('utf-8') + stderr = result.stderr.decode('utf-8') + if stderr: + print(stderr, end='') + if result.returncode != 0: + if stdout: + print(stdout, end='') + raise subprocess.CalledProcessError( + result.returncode, + s, + output=result.stdout, + stderr=result.stderr, + ) + return stdout.strip() def yellow(s): @@ -110,21 +153,28 @@ def community_str(i): return ':'.join(reversed(values)) -def wait_for_completion(f, timeout=120): - interval = 1 +def wait_for(f, timeout=DEFAULT_WAIT_TIMEOUT, interval=1, timeout_message='timeout'): + timeout = scale_timeout(timeout) count = 0 while True: if f(): return time.sleep(interval) - count += interval - if count >= timeout: - raise Exception('timeout') + if timeout is not None: + count += interval + if timeout is not None and count >= timeout: + if callable(timeout_message): + timeout_message = timeout_message() + raise Exception('{0} after {1}s'.format(timeout_message, timeout)) + + +def wait_for_completion(f, timeout=DEFAULT_WAIT_TIMEOUT): + wait_for(f, timeout=timeout) def try_several_times(f, t=3, s=1): - for _ in range(t): + for _ in range(scale_count(t)): try: r = f() except RuntimeError: @@ -134,9 +184,9 @@ def try_several_times(f, t=3, s=1): raise Exception -def assert_several_times(f, t=30, s=1): +def assert_several_times(f, t=DEFAULT_ASSERT_RETRIES, s=1): e = AssertionError - for _ in range(t): + for _ in range(scale_count(t)): try: f() except AssertionError as ae: @@ -222,7 +272,7 @@ def addif(self, ctn, ip_addr=''): if self.subnet.version == 6: ip = '--ip6 {0}'.format(ip_addr) local("docker network connect {0} {1} {2}".format(ip, self.name, ctn.docker_name())) - i = [x for x in list(Client(timeout=60, version='auto').inspect_network(self.id)['Containers'].values()) if x['Name'] == ctn.docker_name()][0] + i = [x for x in list(Client(timeout=scale_timeout(60), version='auto').inspect_network(self.id)['Containers'].values()) if x['Name'] == ctn.docker_name()][0] if self.subnet.version == 4: eth = 'eth{0}'.format(len(ctn.ip_addrs)) addr = i['IPv4Address'] @@ -288,14 +338,19 @@ def remove(self): self.is_running = False return ret - def local(self, cmd, capture=False, stream=False, detach=False, tty=True): + def local(self, cmd, capture=False, stream=False, detach=False, tty=True, + timeout=None): if stream: - dckr = Client(timeout=120, version='auto') + dckr = Client(timeout=scale_timeout(120), version='auto') i = dckr.exec_create(container=self.docker_name(), cmd=cmd) return dckr.exec_start(i['Id'], tty=tty, stream=stream, detach=detach) else: flag = '-d' if detach else '' - return local('docker exec {0} {1} {2}'.format(flag, self.docker_name(), cmd), capture) + return local( + 'docker exec {0} {1} {2}'.format(flag, self.docker_name(), cmd), + capture, + timeout=timeout, + ) def get_pid(self): if self.is_running: @@ -533,7 +588,7 @@ def get_global_rib(self, rf): def get_neighbor_state(self, peer_id): raise Exception('implement get_neighbor() method') - def get_reachability(self, prefix, timeout=20): + def get_reachability(self, prefix, timeout=DEFAULT_REACHABILITY_TIMEOUT): version = netaddr.IPNetwork(prefix).version addr = prefix.split('/')[0] if version == 4: @@ -543,34 +598,30 @@ def get_reachability(self, prefix, timeout=20): else: raise Exception('unsupported route family: {0}'.format(version)) cmd = '/bin/bash -c "/bin/{0} -c 1 -w 1 {1} | xargs echo"'.format(ping_cmd, addr) - interval = 1 - count = 0 - while True: + + def _reachable(): res = self.local(cmd, capture=True) print(yellow(res)) - if ('1 packets received' in res or '1 received' in res) and '0% packet loss' in res: - break - time.sleep(interval) - count += interval - if count >= timeout: - raise Exception('timeout') + return ('1 packets received' in res or '1 received' in res) and '0% packet loss' in res + + wait_for(_reachable, timeout=timeout, timeout_message='reachability timeout') return True - def wait_for(self, expected_state, peer, timeout=120): - interval = 1 - count = 0 - while True: + def wait_for(self, expected_state, peer, timeout=DEFAULT_WAIT_TIMEOUT): + def _state_matches(): state = self.get_neighbor_state(peer) print(yellow("{0}'s peer {1} state: {2}".format(self.router_id, peer.router_id, state))) - if state == expected_state: - return - - time.sleep(interval) - count += interval - if count >= timeout: - raise Exception('timeout') + return state == expected_state + + wait_for( + _state_matches, + timeout=timeout, + timeout_message="timeout waiting for {0}'s peer {1} to be {2}".format( + self.router_id, peer.router_id, expected_state, + ), + ) def add_static_route(self, network, next_hop): cmd = '/sbin/ip route add {0} via {1}'.format(network, next_hop) diff --git a/test/lib/noseplugin.py b/test/lib/noseplugin.py index 3d9d9598a..03d1bd531 100644 --- a/test/lib/noseplugin.py +++ b/test/lib/noseplugin.py @@ -6,6 +6,7 @@ class _ParserOption: gobgp_log_level = "info" test_index = 0 config_format = "yaml" + timeout_scale = 1.0 parser_option = _ParserOption() diff --git a/test/scenario_test/bgp_router_test.py b/test/scenario_test/bgp_router_test.py index d49bfaee5..7ae16f250 100644 --- a/test/scenario_test/bgp_router_test.py +++ b/test/scenario_test/bgp_router_test.py @@ -83,29 +83,15 @@ def test_01_neighbor_established(self): def test_02_check_gobgp_global_rib(self): for q in self.quaggas.values(): - # paths expected to exist in gobgp's global rib - routes = list(q.routes.keys()) - timeout = 120 - interval = 1 - count = 0 - - while True: - # gobgp's global rib + expected_routes = set(q.routes.keys()) + + def _has_expected_routes(): state = self.gobgp.get_neighbor_state(q) self.assertEqual(state, BGP_FSM_ESTABLISHED) - global_rib = [p['prefix'] for p in self.gobgp.get_global_rib()] - - for p in global_rib: - if p in routes: - routes.remove(p) - - if len(routes) == 0: - break + global_rib = {p['prefix'] for p in self.gobgp.get_global_rib()} + return expected_routes.issubset(global_rib) - time.sleep(interval) - count += interval - if count >= timeout: - raise Exception('timeout') + wait_for_completion(_has_expected_routes) # check gobgp properly add it's own asn to aspath def test_03_check_gobgp_adj_out_rib(self): @@ -116,29 +102,21 @@ def test_03_check_gobgp_adj_out_rib(self): # check routes are properly advertised to all BGP speaker def test_04_check_quagga_global_rib(self): - interval = 1 - timeout = int(120 / interval) + expected_routes = {r for c in self.quaggas.values() for r in c.routes} for q in self.quaggas.values(): - done = False - for _ in range(timeout): - if done: - break + def _has_expected_routes(): global_rib = q.get_global_rib() global_rib = [p['prefix'] for p in global_rib] if len(global_rib) < len(self.quaggas): - time.sleep(interval) - continue + return False self.assertEqual(len(global_rib), len(self.quaggas)) - for c in self.quaggas.values(): - for r in c.routes: - self.assertTrue(r in global_rib) - done = True - if done: - continue - # should not reach here - raise AssertionError + for route in expected_routes: + self.assertTrue(route in global_rib) + return True + + wait_for_completion(_has_expected_routes) def test_05_add_quagga(self): q4 = QuaggaBGPContainer(name='q4', asn=65004, router_id='192.168.0.5') @@ -202,23 +180,18 @@ def test_09_add_distant_relative(self): q2.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=q5) q3.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=q5) - timeout = 120 - interval = 1 - count = 0 - while True: + def _has_expected_nexthop(): paths = self.gobgp.get_adj_rib_out(q1, '10.0.6.0/24') - if len(paths) > 0: - path = paths[0] - print("{0}'s nexthop is {1}".format(path['nlri']['prefix'], - path['nexthop'])) - n_addrs = [i[1].split('/')[0] for i in self.gobgp.ip_addrs] - if path['nexthop'] in n_addrs: - break + if not paths: + return False - time.sleep(interval) - count += interval - if count >= timeout: - raise Exception('timeout') + path = paths[0] + print("{0}'s nexthop is {1}".format(path['nlri']['prefix'], + path['nexthop'])) + n_addrs = [i[1].split('/')[0] for i in self.gobgp.ip_addrs] + return path['nexthop'] in n_addrs + + wait_for_completion(_has_expected_nexthop) def test_10_originate_path(self): self.gobgp.add_route('10.10.0.0/24') @@ -574,4 +547,3 @@ def test_27_dynamic_peer_md5(self): # config doesn't explicitly "know about" the peer q9.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=g6, timeout=30) - diff --git a/test/scenario_test/bgp_zebra_test.py b/test/scenario_test/bgp_zebra_test.py index 0224f9ce3..ccee9354c 100644 --- a/test/scenario_test/bgp_zebra_test.py +++ b/test/scenario_test/bgp_zebra_test.py @@ -29,6 +29,7 @@ Bridge, BGP_FSM_ESTABLISHED, local, + wait_for, ) from lib.gobgp import GoBGPContainer from lib.quagga import QuaggaBGPContainer @@ -261,25 +262,32 @@ def nexthops(): return n def validate_nexthops(peers): - interval = 1 - count = 0 - timeout = 30 - while True: - valid = False - nhs = nexthops() - if len(nhs) == len(peers): - valid = True - for peer in peers: - if g1.peers[peer]['neigh_addr'].split('/')[0] not in nhs: - valid = False - break - if valid: - return - - time.sleep(interval) - count += interval - if count >= timeout: - raise Exception(nhs) + current_nexthops = [] + expected_nexthops = [ + g1.peers[peer]['neigh_addr'].split('/')[0] for peer in peers + ] + + def _valid_nexthops(): + nonlocal current_nexthops + current_nexthops = nexthops() + if len(current_nexthops) != len(expected_nexthops): + return False + for nexthop in expected_nexthops: + if nexthop not in current_nexthops: + return False + return True + + def _timeout_message(): + return 'timeout waiting for zebra nexthops: expected {0}, got {1}'.format( + expected_nexthops, + current_nexthops, + ) + + wait_for( + _valid_nexthops, + timeout=30, + timeout_message=_timeout_message, + ) validate_nexthops([g4, g5]) @@ -301,5 +309,3 @@ def validate_nexthops(peers): g2.local('gobgp g ri add 10.0.10.0/24 med 20') validate_nexthops([g3]) - - diff --git a/test/scenario_test/conftest.py b/test/scenario_test/conftest.py index 0cd9245b4..935be897a 100644 --- a/test/scenario_test/conftest.py +++ b/test/scenario_test/conftest.py @@ -1,7 +1,21 @@ +import math +import os + import pytest from lib.noseplugin import parser_option +def _parse_timeout_scale(value): + try: + scale = float(value) + except (TypeError, ValueError): + raise pytest.UsageError('--timeout-scale must be a number') + + if not math.isfinite(scale) or scale < 1.0: + raise pytest.UsageError('--timeout-scale must be a finite number greater than or equal to 1.0') + return scale + + def pytest_addoption(parser): parser.addoption('--test-prefix', default='') parser.addoption('--gobgp-image', default='osrg/gobgp') @@ -10,6 +24,11 @@ def pytest_addoption(parser): parser.addoption('--gobgp-log-level', default='info') parser.addoption('--test-index', type=int, default=0) parser.addoption('--config-format', default='yaml') + parser.addoption( + '--timeout-scale', + default=os.environ.get('GOBGP_TEST_TIMEOUT_SCALE', '1.0'), + help='multiply default scenario-test wait timeouts by this factor', + ) def pytest_configure(config): @@ -20,3 +39,4 @@ def pytest_configure(config): parser_option.gobgp_log_level = config.getoption('--gobgp-log-level') parser_option.test_index = config.getoption('--test-index') parser_option.config_format = config.getoption('--config-format') + parser_option.timeout_scale = _parse_timeout_scale(config.getoption('--timeout-scale')) diff --git a/test/scenario_test/evpn_test.py b/test/scenario_test/evpn_test.py index 17e78c8e0..c6e662197 100644 --- a/test/scenario_test/evpn_test.py +++ b/test/scenario_test/evpn_test.py @@ -30,6 +30,7 @@ BGP_FSM_ESTABLISHED, BGP_ATTR_TYPE_EXTENDED_COMMUNITIES, local, + wait_for_completion, ) from lib.gobgp import GoBGPContainer @@ -92,17 +93,11 @@ def test_02_add_evpn_route(self): path = dst['paths'][0] self.assertEqual(path['nexthop'], '0.0.0.0') - interval = 1 - timeout = int(30 / interval) - done = False - for _ in range(timeout): - if done: - break + def _has_expected_route(): grib = self.g2.get_global_rib(rf='evpn') if len(grib) < 1: - time.sleep(interval) - continue + return False self.assertEqual(len(grib), 1) dst = grib[0] @@ -110,7 +105,9 @@ def test_02_add_evpn_route(self): path = dst['paths'][0] n_addrs = [i[1].split('/')[0] for i in self.g1.ip_addrs] self.assertTrue(path['nexthop'] in n_addrs) - done = True + return True + + wait_for_completion(_has_expected_route, timeout=30) def test_03_check_mac_mobility(self): self.g2.local('gobgp global rib add ' @@ -174,4 +171,3 @@ def test_05_check_mac_mobility_per_mac_vrf(self): # no mac mobility for this route self.assertEqual(get_mac_mobility_sequence(path['attrs']), -1) - diff --git a/test/scenario_test/graceful_restart_test.py b/test/scenario_test/graceful_restart_test.py index 2fb78c544..59759f260 100644 --- a/test/scenario_test/graceful_restart_test.py +++ b/test/scenario_test/graceful_restart_test.py @@ -31,6 +31,7 @@ BGP_FSM_ESTABLISHED, GRACEFUL_RESTART_TIME, local, + wait_for_completion, ) from lib.gobgp import GoBGPContainer @@ -239,15 +240,12 @@ def test_11_multineighbor_graceful_restart(self): g1.start_gobgp(graceful_restart=True) - count = 0 - while (g1.get_neighbor_state(g2) != BGP_FSM_ESTABLISHED - or g1.get_neighbor_state(g3) != BGP_FSM_ESTABLISHED): - count += 1 + def _neighbors_reestablished(): # assert connections are not refused self.assertTrue(g1.get_neighbor_state(g2) != BGP_FSM_IDLE) self.assertTrue(g1.get_neighbor_state(g3) != BGP_FSM_IDLE) - if count > 120: - raise Exception('timeout') - time.sleep(1) + return (g1.get_neighbor_state(g2) == BGP_FSM_ESTABLISHED + and g1.get_neighbor_state(g3) == BGP_FSM_ESTABLISHED) + wait_for_completion(_neighbors_reestablished) diff --git a/test/scenario_test/ibgp_router_test.py b/test/scenario_test/ibgp_router_test.py index 97a4d9acc..89d378d05 100644 --- a/test/scenario_test/ibgp_router_test.py +++ b/test/scenario_test/ibgp_router_test.py @@ -75,28 +75,15 @@ def test_01_neighbor_established(self): def test_02_check_gobgp_global_rib(self): for q in self.quaggas.values(): - # paths expected to exist in gobgp's global rib - routes = list(q.routes.keys()) - timeout = 120 - interval = 1 - count = 0 - while True: - # gobgp's global rib + expected_routes = set(q.routes.keys()) + + def _has_expected_routes(): state = self.gobgp.get_neighbor_state(q) self.assertEqual(state, BGP_FSM_ESTABLISHED) - global_rib = [p['prefix'] for p in self.gobgp.get_global_rib()] - - for p in global_rib: - if p in routes: - routes.remove(p) - - if len(routes) == 0: - break + global_rib = {p['prefix'] for p in self.gobgp.get_global_rib()} + return expected_routes.issubset(global_rib) - time.sleep(interval) - count += interval - if count >= timeout: - raise Exception('timeout') + wait_for_completion(_has_expected_routes) def test_03_check_gobgp_adj_rib_out(self): for q in self.quaggas.values(): @@ -126,19 +113,13 @@ def test_05_check_gobgp_adj_rib_out(self): # check routes are properly advertised to all BGP speaker def test_06_check_quagga_global_rib(self): - interval = 1 - timeout = int(120 / interval) for q in self.quaggas.values(): - done = False - for _ in range(timeout): - if done: - break + def _has_expected_routes(): global_rib = q.get_global_rib() # quagga's global_rib must have two routes at least, # a self-generated route and a gobgp-generated route if len(global_rib) < len(q.routes) + len(self.gobgp.routes): - time.sleep(interval) - continue + return False peer_info = self.gobgp.peers[q] local_addr = peer_info['local_addr'].split('/')[0] @@ -154,11 +135,9 @@ def test_06_check_quagga_global_rib(self): if rr['prefix'] == r: self.assertEqual(rr['nexthop'], '0.0.0.0') - done = True - if done: - continue - # should not reach here - raise AssertionError + return True + + wait_for_completion(_has_expected_routes) def test_07_add_ebgp_peer(self): q3 = QuaggaBGPContainer(name='q3', asn=65001, router_id='192.168.0.4') @@ -290,4 +269,3 @@ def f3(): return len(l) == 1 and len(l[0]['aspath']) == 0 wait_for_completion(f3) - diff --git a/test/scenario_test/mup_test.py b/test/scenario_test/mup_test.py index 49e02e6ab..8564bc16f 100644 --- a/test/scenario_test/mup_test.py +++ b/test/scenario_test/mup_test.py @@ -32,6 +32,7 @@ BGP_FSM_ESTABLISHED, BGP_ATTR_TYPE_EXTENDED_COMMUNITIES, local, + wait_for_completion, ) from lib.gobgp import GoBGPContainer @@ -105,38 +106,27 @@ def test_02_add_del_mup_route(self): path = dst['paths'][0] self.assertEqual(path['nexthop'], nh) - interval = 1 - timeout = int(30 / interval) - done = False - for _ in range(timeout): - if done: - break + def _has_expected_route(): grib = self.g2.get_global_rib(rf=rf) if len(grib) < 1: - time.sleep(interval) - continue + return False self.assertEqual(len(grib), 1) dst = grib[0] self.assertEqual(len(dst['paths']), 1) path = dst['paths'][0] - n_addrs = [i[1].split('/')[0] for i in self.g1.ip_addrs] self.assertEqual(path['nexthop'], nh) - done = True + return True + + wait_for_completion(_has_expected_route, timeout=30) self.g1.local('gobgp global rib del ' '-a {} {}'.format(rf, route)) - done = False - for _ in range(timeout): - if done: - break + def _route_removed(): grib = self.g2.get_global_rib(rf=rf) + return len(grib) == 0 - if len(grib) > 0: - time.sleep(interval) - continue - done = True - + wait_for_completion(_route_removed, timeout=30) diff --git a/test/scenario_test/route_reflector_test.py b/test/scenario_test/route_reflector_test.py index f2a8812c3..b0bee7e81 100644 --- a/test/scenario_test/route_reflector_test.py +++ b/test/scenario_test/route_reflector_test.py @@ -25,24 +25,11 @@ from lib.noseplugin import parser_option from lib import base -from lib.base import BGP_FSM_ESTABLISHED, local +from lib.base import BGP_FSM_ESTABLISHED, local, wait_for from lib.gobgp import GoBGPContainer from lib.quagga import QuaggaBGPContainer -def wait_for(f, timeout=120): - interval = 1 - count = 0 - while True: - if f(): - return - - time.sleep(interval) - count += interval - if count >= timeout: - raise Exception('timeout') - - class GoBGPTestBase(unittest.TestCase): def assert_adv_count(self, src, dst, rf, count): self.assertEqual(count, len(src.get_adj_rib_out(dst, rf=rf))) @@ -285,4 +272,3 @@ def test_12_routes_from_separate_rts_peers_are_isolated_by_rr(self): self.assertEqual(self.rr.peer_name(self.acme1), ar0['nexthop']) self.assertEqual(100, ar0['local-pref']) - diff --git a/test/scenario_test/route_server_as2_test.py b/test/scenario_test/route_server_as2_test.py index fc9f3b4e7..35758ddde 100644 --- a/test/scenario_test/route_server_as2_test.py +++ b/test/scenario_test/route_server_as2_test.py @@ -29,6 +29,7 @@ BGP_FSM_IDLE, BGP_FSM_ESTABLISHED, local, + wait_for, ) from lib.gobgp import GoBGPContainer from lib.exabgp import ExaBGPContainer @@ -82,29 +83,24 @@ def test_01_neighbor_established(self): def test_02_check_gobgp_local_rib(self): for rs_client in self.quaggas.values(): - done = False - for _ in range(self.retry_limit): - if done: - break - + def _has_expected_routes(): state = self.gobgp.get_neighbor_state(rs_client) self.assertEqual(state, BGP_FSM_ESTABLISHED) local_rib = self.gobgp.get_local_rib(rs_client) if len(local_rib) < (len(self.quaggas) - 1): - time.sleep(self.wait_per_retry) - continue + return False self.assertEqual(len(local_rib), 4) - done = True + return True - if done: - continue - # should not reach here - raise AssertionError + wait_for( + _has_expected_routes, + timeout=self.retry_limit * self.wait_per_retry, + interval=self.wait_per_retry, + ) def test_03_stop_q2_and_check_neighbor_status(self): q2 = self.quaggas['q2'] q2.remove() self.gobgp.wait_for(expected_state=BGP_FSM_IDLE, peer=q2) - diff --git a/test/scenario_test/route_server_ipv4_v6_test.py b/test/scenario_test/route_server_ipv4_v6_test.py index 0f45fda11..8792401d6 100644 --- a/test/scenario_test/route_server_ipv4_v6_test.py +++ b/test/scenario_test/route_server_ipv4_v6_test.py @@ -25,7 +25,7 @@ from lib.noseplugin import parser_option from lib import base -from lib.base import BGP_FSM_ESTABLISHED, local +from lib.base import BGP_FSM_ESTABLISHED, local, wait_for from lib.gobgp import GoBGPContainer from lib.quagga import QuaggaBGPContainer @@ -78,18 +78,13 @@ def setUpClass(cls): def check_gobgp_local_rib(self, ctns, rf): for rs_client in ctns.values(): - done = False - for _ in range(self.retry_limit): - if done: - break - + def _has_expected_routes(): state = self.gobgp.get_neighbor_state(rs_client) self.assertEqual(state, BGP_FSM_ESTABLISHED) local_rib = self.gobgp.get_local_rib(rs_client, rf=rf) local_rib = [p["prefix"] for p in local_rib] if len(local_rib) < (len(ctns) - 1): - time.sleep(self.wait_per_retry) - continue + return False self.assertEqual(len(local_rib), (len(ctns) - 1)) @@ -98,23 +93,21 @@ def check_gobgp_local_rib(self, ctns, rf): for r in c.routes: self.assertTrue(r in local_rib) - done = True - if done: - continue - # should not reach here - raise AssertionError + return True + + wait_for( + _has_expected_routes, + timeout=self.retry_limit * self.wait_per_retry, + interval=self.wait_per_retry, + ) def check_rs_client_rib(self, ctns, rf): for rs_client in ctns.values(): - done = False - for _ in range(self.retry_limit): - if done: - break + def _has_expected_routes(): global_rib = rs_client.get_global_rib(rf=rf) global_rib = [p['prefix'] for p in global_rib] if len(global_rib) < len(ctns): - time.sleep(self.wait_per_retry) - continue + return False self.assertEqual(len(global_rib), len(ctns)) @@ -122,11 +115,13 @@ def check_rs_client_rib(self, ctns, rf): for r in c.routes: self.assertTrue(r in global_rib) - done = True - if done: - continue - # should not reach here - raise AssertionError + return True + + wait_for( + _has_expected_routes, + timeout=self.retry_limit * self.wait_per_retry, + interval=self.wait_per_retry, + ) # test each neighbor state is turned establish def test_01_neighbor_established(self): @@ -166,4 +161,3 @@ def test_08_check_rib(self): self.assertEqual(len(self.gobgp.get_adj_rib_out(q, rf='ipv6')), 0) self.assertEqual(len(q.get_global_rib(rf='ipv6')), len(q.routes)) - diff --git a/test/scenario_test/route_server_malformed_test.py b/test/scenario_test/route_server_malformed_test.py index 26a67e961..7e5e1b049 100644 --- a/test/scenario_test/route_server_malformed_test.py +++ b/test/scenario_test/route_server_malformed_test.py @@ -26,7 +26,7 @@ from lib.noseplugin import parser_option from lib import base -from lib.base import BGP_FSM_ESTABLISHED, local +from lib.base import BGP_FSM_ESTABLISHED, local, wait_for from lib.gobgp import GoBGPContainer from lib.exabgp import ExaBGPContainer @@ -48,19 +48,6 @@ def lookup_scenario(name): return None -def wait_for(f, timeout=120): - interval = 1 - count = 0 - while True: - if f(): - return - - time.sleep(interval) - count += interval - if count >= timeout: - raise Exception('timeout') - - @register_scenario class MalformedMpReachNlri(object): """ @@ -497,4 +484,3 @@ def test(self): e(self) print('[PASS] %s' % e.__qualname__.split('.')[0], file=sys.stderr, flush=True) - diff --git a/test/scenario_test/route_server_policy_grpc_test.py b/test/scenario_test/route_server_policy_grpc_test.py index 2f453f682..2eb81d305 100644 --- a/test/scenario_test/route_server_policy_grpc_test.py +++ b/test/scenario_test/route_server_policy_grpc_test.py @@ -31,6 +31,7 @@ BGP_ATTR_TYPE_COMMUNITIES, BGP_ATTR_TYPE_EXTENDED_COMMUNITIES, local, + wait_for, ) from lib.gobgp import GoBGPContainer from lib.quagga import QuaggaBGPContainer @@ -54,19 +55,6 @@ def lookup_scenario(name): return None -def wait_for(f, timeout=120): - interval = 1 - count = 0 - while True: - if f(): - return - - time.sleep(interval) - count += interval - if count >= timeout: - raise Exception('timeout') - - @register_scenario class ImportPolicy(object): """ @@ -2709,4 +2697,3 @@ def test(self): e(self) print('[PASS] %s' % e.__qualname__.split('.')[0], file=sys.stderr, flush=True) - diff --git a/test/scenario_test/route_server_policy_test.py b/test/scenario_test/route_server_policy_test.py index 49c8ac464..ead947c1e 100644 --- a/test/scenario_test/route_server_policy_test.py +++ b/test/scenario_test/route_server_policy_test.py @@ -31,6 +31,7 @@ BGP_ATTR_TYPE_COMMUNITIES, BGP_ATTR_TYPE_EXTENDED_COMMUNITIES, local, + wait_for, ) from lib.gobgp import GoBGPContainer from lib.quagga import QuaggaBGPContainer @@ -54,19 +55,6 @@ def lookup_scenario(name): return None -def wait_for(f, timeout=120): - interval = 1 - count = 0 - while True: - if f(): - return - - time.sleep(interval) - count += interval - if count >= timeout: - raise Exception('timeout') - - @register_scenario class ImportPolicy(object): """ @@ -3184,4 +3172,3 @@ def test(self): e(self) print('[PASS] %s' % e.__qualname__.split('.')[0], file=sys.stderr, flush=True) - diff --git a/test/scenario_test/route_server_test.py b/test/scenario_test/route_server_test.py index b9d1ccf4c..3e20ee9c8 100644 --- a/test/scenario_test/route_server_test.py +++ b/test/scenario_test/route_server_test.py @@ -29,6 +29,7 @@ BGP_FSM_ACTIVE, BGP_FSM_ESTABLISHED, local, + wait_for, ) from lib.gobgp import GoBGPContainer from lib.quagga import QuaggaBGPContainer @@ -76,18 +77,14 @@ def setUpClass(cls): def check_gobgp_local_rib(self): for rs_client in self.quaggas.values(): - done = False - for _ in range(self.retry_limit): - if done: - break + def _has_expected_routes(): local_rib = self.gobgp.get_local_rib(rs_client) local_rib = [p['prefix'] for p in local_rib] state = self.gobgp.get_neighbor_state(rs_client) self.assertEqual(state, BGP_FSM_ESTABLISHED) if len(local_rib) < (len(self.quaggas) - 1): - time.sleep(self.wait_per_retry) - continue + return False self.assertEqual(len(local_rib), (len(self.quaggas) - 1)) @@ -96,23 +93,21 @@ def check_gobgp_local_rib(self): for r in c.routes: self.assertTrue(r in local_rib) - done = True - if done: - continue - # should not reach here - raise AssertionError + return True + + wait_for( + _has_expected_routes, + timeout=self.retry_limit * self.wait_per_retry, + interval=self.wait_per_retry, + ) def check_rs_client_rib(self): for rs_client in self.quaggas.values(): - done = False - for _ in range(self.retry_limit): - if done: - break + def _has_expected_routes(): global_rib = rs_client.get_global_rib() global_rib = [p['prefix'] for p in global_rib] if len(global_rib) < len(self.quaggas): - time.sleep(self.wait_per_retry) - continue + return False self.assertEqual(len(global_rib), len(self.quaggas)) @@ -120,11 +115,13 @@ def check_rs_client_rib(self): for r in c.routes: self.assertTrue(r in global_rib) - done = True - if done: - continue - # should not reach here - raise AssertionError + return True + + wait_for( + _has_expected_routes, + timeout=self.retry_limit * self.wait_per_retry, + interval=self.wait_per_retry, + ) # test each neighbor state is turned establish def test_01_neighbor_established(self): @@ -217,11 +214,7 @@ def test_10_add_distant_relative(self): self.gobgp.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=q3) def check_nexthop(target_prefix, expected_nexthop): - is_done = False - for _ in range(self.retry_limit): - if is_done: - break - time.sleep(self.wait_per_retry) + def _has_expected_nexthop(): for path in q1.get_global_rib(): if path['prefix'] == target_prefix: print("{0}'s nexthop is {1}".format(path['prefix'], @@ -229,9 +222,15 @@ def check_nexthop(target_prefix, expected_nexthop): n_addrs = [i[1].split('/')[0] for i in expected_nexthop.ip_addrs] if path['nexthop'] in n_addrs: - is_done = True - break - return is_done + return True + return False + + wait_for( + _has_expected_nexthop, + timeout=self.retry_limit * self.wait_per_retry, + interval=self.wait_per_retry, + ) + return True done = check_nexthop('10.0.6.0/24', q3) self.assertTrue(done) @@ -247,5 +246,3 @@ def check_nexthop(target_prefix, expected_nexthop): done = check_nexthop('10.0.6.0/24', q2) self.assertTrue(done) - -