Added bgpmon verification for test_bgp_allowlist (#2977)

Added BGPMON verification for allow list test case. This was done to make so sure BGPMON (since it is iBGP Peer) receive all the routes irrespective if allow list action being permit or deny

Also to support above made bgpmon session creation and route verification as common fixture and helper routines.

tests/bgp/bgp_helpers.py

@@ -1,6 +1,7 @@
 import os
 import re
 import time
+import json
 
 BASE_DIR = os.path.dirname(os.path.realpath(__file__))
 DUT_TMP_DIR = os.path.join('tmp', os.path.basename(BASE_DIR))
@@ -9,6 +10,14 @@
 BGP_NO_EXPORT_TEMPLATE = 'bgp_no_export.j2'
 BGP_CONFIG_BACKUP = 'backup_bgpd.conf.j2'
 DEFAULT_BGP_CONFIG = 'bgp:/usr/share/sonic/templates/bgpd/bgpd.conf.j2'
+DUMP_FILE = "/tmp/bgp_monitor_dump.log"
+CUSTOM_DUMP_SCRIPT = "bgp/bgp_monitor_dump.py"
+CUSTOM_DUMP_SCRIPT_DEST = "/usr/share/exabgp/bgp_monitor_dump.py"
+BGPMON_TEMPLATE_FILE = 'bgp/templates/bgp_template.j2'
+BGPMON_CONFIG_FILE = '/tmp/bgpmon.json'
+BGP_MONITOR_NAME = "bgp_monitor"
+BGP_MONITOR_PORT = 7000
+BGP_ANNOUNCE_TIME = 30 #should be enough to receive and parse bgp updates
 
 
 def apply_bgp_config(duthost, template_name):
@@ -74,3 +83,38 @@ def apply_default_bgp_config(duthost, copy=False):
         # Skip 'start-limit-hit' threshold
         duthost.shell('systemctl reset-failed bgp')
         restart_bgp(duthost)
+
+def parse_exabgp_dump(host):
+    """
+    Parse the dump file of exabgp, and build a set for checking routes
+    """
+    routes = set()
+    output_lines = host.shell("cat {}".format(DUMP_FILE))['stdout_lines']
+    for line in output_lines:
+        routes.add(line)
+    return routes
+
+def parse_rib(host, ip_ver):
+    """
+    Parse output of 'show bgp ipv4/6' and parse into a dict for checking routes
+    """
+    routes = {}
+    cmd = "vtysh -c \"show bgp ipv%d json\"" % ip_ver
+    route_data = json.loads(host.shell(cmd)['stdout'])
+    for ip, nexthops in route_data['routes'].iteritems():
+        aspath = set()
+        for nexthop in nexthops:
+            aspath.add(nexthop['path'])
+        routes[ip] = aspath
+    return routes
+
+def verify_all_routes_announce_to_bgpmon(duthost, ptfhost):
+    time.sleep(BGP_ANNOUNCE_TIME)
+    bgpmon_routes = parse_exabgp_dump(ptfhost)
+    rib_v4 = parse_rib(duthost, 4)
+    rib_v6 = parse_rib(duthost, 6)
+    routes_dut = dict(rib_v4.items() + rib_v6.items())
+    for route in routes_dut.keys():
+        if route not in bgpmon_routes:
+            return False
+    return True

tests/bgp/conftest.py

@@ -7,18 +7,21 @@
 import pytest
 import random
 
+from jinja2 import Template
 from tests.common.helpers.assertions import pytest_assert as pt_assert
 from tests.common.helpers.generators import generate_ips
 from tests.common.helpers.parallel import parallel_run
 from tests.common.helpers.parallel import reset_ansible_local_tmp
 from tests.common.utilities import wait_until
+from tests.common.utilities import wait_tcp_connection
 from tests.common import config_reload
 from bgp_helpers import define_config
 from bgp_helpers import apply_default_bgp_config
 from bgp_helpers import DUT_TMP_DIR
 from bgp_helpers import TEMPLATE_DIR
 from bgp_helpers import BGP_PLAIN_TEMPLATE
 from bgp_helpers import BGP_NO_EXPORT_TEMPLATE
+from bgp_helpers import DUMP_FILE, CUSTOM_DUMP_SCRIPT, CUSTOM_DUMP_SCRIPT_DEST, BGPMON_TEMPLATE_FILE, BGPMON_CONFIG_FILE, BGP_MONITOR_NAME, BGP_MONITOR_PORT
 
 
 logger = logging.getLogger(__name__)
@@ -294,3 +297,47 @@ def backup_bgp_config(duthost):
     except Exception:
         config_reload(duthost)
         apply_default_bgp_config(duthost)
+
+@pytest.fixture(scope="module")
+def bgpmon_setup_teardown(ptfhost, duthost, localhost, setup_interfaces):
+    connection = setup_interfaces[0]
+    dut_lo_addr = connection['local_addr'].split("/")[0]
+    peer_addr = connection['neighbor_addr'].split("/")[0]
+    mg_facts = duthost.minigraph_facts(host=duthost.hostname)['ansible_facts']
+    asn = mg_facts['minigraph_bgp_asn']
+    # TODO: Add a common method to load BGPMON config for test_bgpmon and test_traffic_shift
+    logger.info("Configuring bgp monitor session on DUT")
+    bgpmon_args = {
+        'db_table_name': 'BGP_MONITORS',
+        'peer_addr': peer_addr,
+        'asn': asn,
+        'local_addr': dut_lo_addr,
+        'peer_name': BGP_MONITOR_NAME
+    }
+    bgpmon_template = Template(open(BGPMON_TEMPLATE_FILE).read())
+    duthost.copy(content=bgpmon_template.render(**bgpmon_args),
+                 dest=BGPMON_CONFIG_FILE)
+    # Start bgpmon on DUT
+    logger.info("Starting bgpmon on DUT")
+    duthost.command("sonic-cfggen -j {} -w".format(BGPMON_CONFIG_FILE))
+
+    logger.info("Starting bgp monitor session on PTF")
+    ptfhost.file(path=DUMP_FILE, state="absent")
+    ptfhost.copy(src=CUSTOM_DUMP_SCRIPT, dest=CUSTOM_DUMP_SCRIPT_DEST)
+    ptfhost.exabgp(name=BGP_MONITOR_NAME,
+                   state="started",
+                   local_ip=peer_addr,
+                   router_id=peer_addr,
+                   peer_ip=dut_lo_addr,
+                   local_asn=asn,
+                   peer_asn=asn,
+                   port=BGP_MONITOR_PORT,
+                   dump_script=CUSTOM_DUMP_SCRIPT_DEST)
+    pt_assert(wait_tcp_connection(localhost, ptfhost.mgmt_ip, BGP_MONITOR_PORT),
+                  "Failed to start bgp monitor session on PTF")
+    yield
+    # Cleanup bgp monitor
+    duthost.shell("redis-cli -n 4 -c DEL 'BGP_MONITORS|{}'".format(peer_addr))
+    ptfhost.exabgp(name=BGP_MONITOR_NAME, state="absent")
+    ptfhost.file(path=CUSTOM_DUMP_SCRIPT_DEST, state="absent")
+    ptfhost.file(path=DUMP_FILE, state="absent")

tests/bgp/test_bgp_allow_list.py

@@ -14,6 +14,7 @@
 from tests.common.helpers.assertions import pytest_assert
 from tests.common.helpers.parallel import reset_ansible_local_tmp
 from tests.common.helpers.parallel import parallel_run
+from bgp_helpers import verify_all_routes_announce_to_bgpmon 
 
 pytestmark = [
     pytest.mark.topology('t1'),
@@ -355,20 +356,24 @@ def check_other_neigh(nbrhosts, permit, node=None, results=None):
         results = parallel_run(check_other_neigh, (nbrhosts, permit), {}, other_neighbors, timeout=180)
         self.check_results(results)
 
-    def test_default_allow_list_preconfig(self, duthosts, rand_one_dut_hostname, setup, nbrhosts):
+    def test_default_allow_list_preconfig(self, duthosts, rand_one_dut_hostname, setup, nbrhosts, ptfhost, bgpmon_setup_teardown):
         permit = True if DEFAULT_ACTION == "permit" else False
         duthost = duthosts[rand_one_dut_hostname]
         self.check_routes_on_tor1(setup, nbrhosts)
         self.check_routes_on_dut(duthost)
         self.check_routes_on_neighbors_empty_allow_list(nbrhosts, setup, permit)
-
+        pytest_assert(verify_all_routes_announce_to_bgpmon(duthost, ptfhost),
+                      "Not all routes are announced to bgpmon")
+
     @pytest.mark.parametrize('load_remove_allow_list', ["permit", "deny"], indirect=['load_remove_allow_list'])
-    def test_allow_list(self, duthosts, rand_one_dut_hostname, setup, nbrhosts, load_remove_allow_list):
+    def test_allow_list(self, duthosts, rand_one_dut_hostname, setup, nbrhosts, load_remove_allow_list, ptfhost, bgpmon_setup_teardown):
         permit = True if load_remove_allow_list == "permit" else False
         duthost = duthosts[rand_one_dut_hostname]
         self.check_routes_on_tor1(setup, nbrhosts)
         self.check_routes_on_dut(duthost)
         self.check_routes_on_neighbors(nbrhosts, setup, permit)
+        pytest_assert(verify_all_routes_announce_to_bgpmon(duthost, ptfhost),
+                      "Not all routes are announced to bgpmon")
 
-    def test_default_allow_list_postconfig(self, duthosts, rand_one_dut_hostname, setup, nbrhosts):
-        self.test_default_allow_list_preconfig(duthosts, rand_one_dut_hostname, setup, nbrhosts)
+    def test_default_allow_list_postconfig(self, duthosts, rand_one_dut_hostname, setup, nbrhosts, ptfhost, bgpmon_setup_teardown):
+        self.test_default_allow_list_preconfig(duthosts, rand_one_dut_hostname, setup, nbrhosts, ptfhost, bgpmon_setup_teardown)

tests/bgp/test_bgpmon.py

@@ -11,18 +11,14 @@
 from tests.common.helpers.generators import generate_ips as generate_ips
 from tests.common.helpers.assertions import pytest_assert
 from tests.common.utilities import wait_until
-
+from bgp_helpers import BGPMON_TEMPLATE_FILE, BGPMON_CONFIG_FILE, BGP_MONITOR_NAME, BGP_MONITOR_PORT
 pytestmark = [
     pytest.mark.topology('any'),
 ]
 
-BGPMON_TEMPLATE_FILE = 'bgp/templates/bgp_template.j2'
-BGPMON_CONFIG_FILE = '/tmp/bgpmon.json'
 BGP_PORT = 179
 BGP_CONNECT_TIMEOUT = 121
 ZERO_ADDR = r'0.0.0.0/0'
-BGP_MONITOR_NAME = "bgp_monitor"
-BGP_MONITOR_PORT = 7000
 logger = logging.getLogger(__name__)
 
 def route_through_default_routes(host, ip_addr):

tests/bgp/test_traffic_shift.py

@@ -1,11 +1,8 @@
 import pytest
 import logging
-import json
-import time
 import ipaddr as ipaddress
+from bgp_helpers import parse_rib, verify_all_routes_announce_to_bgpmon 
 from tests.common.helpers.assertions import pytest_assert
-from tests.common.utilities import wait_tcp_connection
-from jinja2 import Template
 import re
 
 pytestmark = [
@@ -18,65 +15,11 @@
 TS_MAINTENANCE = "System Mode: Maintenance"
 TS_INCONSISTENT = "System Mode: Not consistent"
 
-DUMP_FILE = "/tmp/bgp_monitor_dump.log"
-CUSTOM_DUMP_SCRIPT = "bgp/bgp_monitor_dump.py"
-CUSTOM_DUMP_SCRIPT_DEST = "/usr/share/exabgp/bgp_monitor_dump.py"
-BGP_MONITOR_PORT = 7000
-BGP_MONITOR_NAME = "bgp_monitor"
-BGP_ANNOUNCE_TIME = 30 #should be enough to receive and parse bgp updates
-
-# TODO: remove me
-BGPMON_TEMPLATE_FILE = 'bgp/templates/bgp_template.j2'
-BGPMON_CONFIG_FILE = '/tmp/bgpmon.json'
-
-PEER_COUNT = 1
-
 @pytest.fixture
 def traffic_shift_community(duthost):
     community = duthost.shell('sonic-cfggen -y /etc/sonic/constants.yml -v constants.bgp.traffic_shift_community')['stdout']
     return community
 
-@pytest.fixture
-def common_setup_teardown(ptfhost, duthost, localhost, setup_interfaces):
-    connection = setup_interfaces[0]
-    dut_lo_addr = connection['local_addr'].split("/")[0]
-    peer_addr = connection['neighbor_addr'].split("/")[0]
-    mg_facts = duthost.minigraph_facts(host=duthost.hostname)['ansible_facts']
-    asn = mg_facts['minigraph_bgp_asn']
-    # TODO: Add a common method to load BGPMON config for test_bgpmon and test_traffic_shift
-    logger.info("Configuring bgp monitor session on DUT")
-    bgpmon_args = {
-        'db_table_name': 'BGP_MONITORS',
-        'peer_addr': peer_addr,
-        'asn': asn,
-        'local_addr': dut_lo_addr,
-        'peer_name': BGP_MONITOR_NAME
-    }
-    bgpmon_template = Template(open(BGPMON_TEMPLATE_FILE).read())
-    duthost.copy(content=bgpmon_template.render(**bgpmon_args),
-                 dest=BGPMON_CONFIG_FILE)
-
-    logger.info("Starting bgp monitor session on PTF")
-    ptfhost.file(path=DUMP_FILE, state="absent")
-    ptfhost.copy(src=CUSTOM_DUMP_SCRIPT, dest=CUSTOM_DUMP_SCRIPT_DEST)
-    ptfhost.exabgp(name=BGP_MONITOR_NAME,
-                   state="started",
-                   local_ip=peer_addr,
-                   router_id=peer_addr,
-                   peer_ip=dut_lo_addr,
-                   local_asn=asn,
-                   peer_asn=asn,
-                   port=BGP_MONITOR_PORT,
-                   dump_script=CUSTOM_DUMP_SCRIPT_DEST)
-    pytest_assert(wait_tcp_connection(localhost, ptfhost.mgmt_ip, BGP_MONITOR_PORT),
-                  "Failed to start bgp monitor session on PTF")
-    yield
-    # Cleanup bgp monitor
-    duthost.shell("redis-cli -n 4 -c DEL 'BGP_MONITORS|{}'".format(peer_addr))
-    ptfhost.exabgp(name=BGP_MONITOR_NAME, state="absent")
-    ptfhost.file(path=CUSTOM_DUMP_SCRIPT_DEST, state="absent")
-    ptfhost.file(path=DUMP_FILE, state="absent")
-
 def get_traffic_shift_state(host):
     outputs = host.shell('TSC')['stdout_lines']
     for out in outputs:
@@ -88,37 +31,6 @@ def get_traffic_shift_state(host):
             return TS_INCONSISTENT
     pytest.fail("TSC return unexpected state {}".format(out))
 
-def parse_exabgp_dump(host):
-    """
-    Parse the dump file of exabgp, and build a set for checking routes
-    """
-    routes = set()
-    output_lines = host.shell("cat {}".format(DUMP_FILE))['stdout_lines']
-    for line in output_lines:
-        routes.add(line)
-    return routes
-
-def parse_rib(host, ip_ver):
-    """
-    Parse output of 'show bgp ipv4/6' and parse into a dict for checking routes
-    """
-    routes = {}
-    cmd = "vtysh -c \"show bgp ipv%d json\"" % ip_ver
-    route_data = json.loads(host.shell(cmd)['stdout'])
-    for ip, nexthops in route_data['routes'].iteritems():
-        aspath = set()
-        for nexthop in nexthops:
-            aspath.add(nexthop['path'])
-        routes[ip] = aspath
-    return routes
-
-def verify_all_routes_announce_to_bgpmon(routes_bgpmon, routes_dut):
-    logger.info("Verifying all routes are announced to BGPMON")
-    for route in routes_dut.keys():
-        if route not in routes_bgpmon:
-            return False
-    return True
-
 def parse_routes_on_eos(dut_host, neigh_hosts, ip_ver):
     """
     Parse the output of 'show ip bgp neigh received-routes' on eos, and store in a dict
@@ -225,7 +137,7 @@ def verify_only_loopback_routes_are_announced_to_neighs(dut_host, neigh_hosts, c
     return verify_loopback_route_with_community(dut_host, neigh_hosts, 4, community) and \
         verify_loopback_route_with_community(dut_host, neigh_hosts, 6, community)
 
-def test_TSA(duthost, ptfhost, nbrhosts, common_setup_teardown, traffic_shift_community):
+def test_TSA(duthost, ptfhost, nbrhosts, bgpmon_setup_teardown, traffic_shift_community):
     """
     Test TSA
     Verify all routes are announced to bgp monitor, and only loopback routes are announced to neighs
@@ -236,22 +148,15 @@ def test_TSA(duthost, ptfhost, nbrhosts, common_setup_teardown, traffic_shift_co
         # Verify DUT is in maintenance state.
         pytest_assert(TS_MAINTENANCE == get_traffic_shift_state(duthost),
                       "DUT is not in maintenance state")
-        # Start bgpmon on DUT
-        logger.info("Starting bgpmon on DUT")
-        duthost.command("sonic-cfggen -j {} -w".format(BGPMON_CONFIG_FILE))
-        time.sleep(BGP_ANNOUNCE_TIME)
-        bgpmon_routes = parse_exabgp_dump(ptfhost)
-        rib_v4 = parse_rib(duthost, 4)
-        rib_v6 = parse_rib(duthost, 6)
-        pytest_assert(verify_all_routes_announce_to_bgpmon(bgpmon_routes, dict(rib_v4.items() + rib_v6.items())),
+        pytest_assert(verify_all_routes_announce_to_bgpmon(duthost, ptfhost),
                       "Not all routes are announced to bgpmon")
         pytest_assert(verify_only_loopback_routes_are_announced_to_neighs(duthost, nbrhosts, traffic_shift_community),
                       "Failed to verify routes on eos in TSA")
     finally:
         # Recover to Normal state
         duthost.shell("TSB")
 
-def test_TSB(duthost, ptfhost, nbrhosts, common_setup_teardown):
+def test_TSB(duthost, ptfhost, nbrhosts, bgpmon_setup_teardown):
     """
     Test TSB.
     Establish BGP session between PTF and DUT, and verify all routes are announced to bgp monitor,
@@ -262,16 +167,9 @@ def test_TSB(duthost, ptfhost, nbrhosts, common_setup_teardown):
     # Verify DUT is in normal state.
     pytest_assert(TS_NORMAL == get_traffic_shift_state(duthost),
                   "DUT is not in normal state")
-    # Start bgpmon on DUT
-    logger.info("Starting bgpmon on DUT")
-    duthost.command("sonic-cfggen -j {} -w".format(BGPMON_CONFIG_FILE))
-    time.sleep(BGP_ANNOUNCE_TIME)
-    bgpmon_routes = parse_exabgp_dump(ptfhost)
-    rib_v4 = parse_rib(duthost, 4)
-    rib_v6 = parse_rib(duthost, 6)
-    pytest_assert(verify_all_routes_announce_to_bgpmon(bgpmon_routes, dict(rib_v4.items() + rib_v6.items())),
+    pytest_assert(verify_all_routes_announce_to_bgpmon(duthost, ptfhost),
                   "Not all routes are announced to bgpmon")
-    pytest_assert(verify_all_routes_announce_to_neighs(duthost, nbrhosts, rib_v4, 4),
+    pytest_assert(verify_all_routes_announce_to_neighs(duthost, nbrhosts, parse_rib(duthost, 4), 4),
                   "Not all ipv4 routes are announced to neighbors")
-    pytest_assert(verify_all_routes_announce_to_neighs(duthost, nbrhosts, rib_v6, 6),
+    pytest_assert(verify_all_routes_announce_to_neighs(duthost, nbrhosts, parse_rib(duthost, 6), 6),
                   "Not all ipv6 routes are announced to neighbors")