[bgp] Fix 'test_bgp_update_timer'

tests/bgp/conftest.py

@@ -1,10 +1,17 @@
-import pytest
-import logging
+import contextlib
+import ipaddress
 import json
-from tests.common.utilities import wait_until
+import logging
+import netaddr
+import pytest
+import random
+
 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
+
 
 logger = logging.getLogger(__name__)
 
@@ -128,3 +135,100 @@ def restore_nbr_gr(node=None, results=None):
 
     if not wait_until(300, 10, duthost.check_bgp_session_state, bgp_neighbors.keys()):
         pytest.fail("not all bgp sessions are up after disable graceful restart")
+
+
+@pytest.fixture(scope="module")
+def setup_interfaces(duthost, ptfhost, request, tbinfo):
+    """Setup interfaces for the new BGP peers on PTF."""
+
+    def _is_ipv4_address(ip_addr):
+        return ipaddress.ip_address(ip_addr).version == 4
+
+    @contextlib.contextmanager
+    def _setup_interfaces_t0(mg_facts, peer_count):
+        try:
+            connections = []
+            vlan_intf = None
+            for vlan_intf in mg_facts["minigraph_vlan_interfaces"]:
+                if _is_ipv4_address(vlan_intf["addr"]):
+                    break
+            if vlan_intf is None:
+                raise ValueError("No Vlan interface defined in T0.")
+            vlan_intf_name = vlan_intf["attachto"]
+            vlan_intf_addr = "%s/%s" % (vlan_intf["addr"], vlan_intf["prefixlen"])
+            vlan_members = mg_facts["minigraph_vlans"][vlan_intf_name]["members"]
+            local_interfaces = random.sample(vlan_members, peer_count)
+            neighbor_addresses = generate_ips(
+                peer_count,
+                vlan_intf["subnet"],
+                [netaddr.IPAddress(vlan_intf["addr"])]
+            )
+
+            for local_intf, neighbor_addr in zip(local_interfaces, neighbor_addresses):
+                conn = {}
+                conn["local_intf"] = vlan_intf_name
+                conn["local_addr"] = vlan_intf_addr
+                conn["neighbor_addr"] = neighbor_addr
+                conn["neighbor_intf"] = "eth%s" % mg_facts["minigraph_port_indices"][local_intf]
+                connections.append(conn)
+
+            for conn in connections:
+                ptfhost.shell("ifconfig %s %s" % (conn["neighbor_intf"],
+                                                  conn["neighbor_addr"]))
+
+            yield connections
+
+        finally:
+            for conn in connections:
+                ptfhost.shell("ifconfig %s 0.0.0.0" % conn["neighbor_intf"])
+
+    @contextlib.contextmanager
+    def _setup_interfaces_t1(mg_facts, peer_count):
+        try:
+            connections = []
+            ipv4_interfaces = [_ for _ in mg_facts["minigraph_interfaces"] if _is_ipv4_address(_['addr'])]
+            used_subnets = [ipaddress.ip_network(_["subnet"]) for _ in ipv4_interfaces]
+            subnet_prefixlen = used_subnets[0].prefixlen
+            used_subnets = set(used_subnets)
+            for pt in mg_facts["minigraph_portchannel_interfaces"]:
+                if _is_ipv4_address(pt["addr"]):
+                    used_subnets.add(ipaddress.ip_network(pt["subnet"]))
+            _subnets = ipaddress.ip_network(u"10.0.0.0/24").subnets(new_prefix=subnet_prefixlen)
+            subnets = (_ for _ in _subnets if _ not in used_subnets)
+
+            for intf, subnet in zip(random.sample(ipv4_interfaces, peer_count), subnets):
+                conn = {}
+                local_addr, neighbor_addr = [_ for _ in subnet][:2]
+                conn["local_intf"] = "%s:0" % intf["attachto"]
+                conn["local_addr"] = "%s/%s" % (local_addr, subnet_prefixlen)
+                conn["neighbor_addr"] = "%s/%s" % (neighbor_addr, subnet_prefixlen)
+                conn["neighbor_intf"] = "eth%s" % mg_facts["minigraph_port_indices"][intf["attachto"]]
+                connections.append(conn)
+
+            for conn in connections:
+                duthost.shell("ifconfig %s %s" % (conn["local_intf"], conn["local_addr"]))
+                # Notify bgpcfgd the interface is ready
+                duthost.shell("config interface ip add %s %s" % (conn["local_intf"], conn["local_addr"]))
+                ptfhost.shell("ifconfig %s %s" % (conn["neighbor_intf"], conn["neighbor_addr"]))
+
+            yield connections
+
+        finally:
+            for conn in connections:
+                duthost.shell("ifconfig %s 0.0.0.0" % conn["local_intf"])
+                duthost.shell("config interface ip remove %s %s" % (conn["local_intf"], conn["local_addr"]))
+                ptfhost.shell("ifconfig %s 0.0.0.0" % conn["neighbor_intf"])
+
+    peer_count = getattr(request.module, "PEER_COUNT", 1)
+    if tbinfo["topo"]["type"] == "t0":
+        setup_func = _setup_interfaces_t0
+    elif tbinfo["topo"]["type"] == "t1":
+        setup_func = _setup_interfaces_t1
+    else:
+        raise TypeError("Unsupported topology: %s" % tbinfo["topo"]["type"])
+
+    mg_facts = duthost.minigraph_facts(host=duthost.hostname)["ansible_facts"]
+    with setup_func(mg_facts, peer_count) as connections:
+        yield connections
+
+    duthost.shell("sonic-clear arp")

tests/bgp/test_bgp_update_timer.py

@@ -17,6 +17,7 @@
     pytest.mark.topology("any"),
 ]
 
+PEER_COUNT = 2
 BGP_SAVE_DEST_TMPL = "/tmp/bgp_%s.j2"
 NEIGHBOR_SAVE_DEST_TMPL = "/tmp/neighbor_%s.j2"
 BGP_LOG_TMPL = "/tmp/bgp%d.pcap"
@@ -35,7 +36,7 @@
 
 def _write_variable_from_j2_to_configdb(duthost, template_file, **kwargs):
     save_dest_path = kwargs.pop("save_dest_path", "/tmp/temp.j2")
-    keep_dest_file = kwargs.pop("keep_dest_file", False)
+    keep_dest_file = kwargs.pop("keep_dest_file", True)
     config_template = jinja2.Template(open(template_file).read())
     duthost.copy(content=config_template.render(**kwargs), dest=save_dest_path)
     duthost.shell("sonic-cfggen -j %s --write-to-db" % save_dest_path)
@@ -45,13 +46,12 @@ def _write_variable_from_j2_to_configdb(duthost, template_file, **kwargs):
 
 class BGPNeighbor(object):
 
-    def __init__(self, duthost, ptfhost, name, iface,
+    def __init__(self, duthost, ptfhost, name,
                  neighbor_ip, neighbor_asn,
                  dut_ip, dut_asn, port, is_quagga=False):
         self.duthost = duthost
         self.ptfhost = ptfhost
         self.ptfip = ptfhost.mgmt_ip
-        self.iface = iface
         self.name = name
         self.ip = neighbor_ip
         self.asn = neighbor_asn
@@ -63,7 +63,6 @@ def __init__(self, duthost, ptfhost, name, iface,
     def start_session(self):
         """Start the BGP session."""
         logging.debug("start bgp session %s", self.name)
-        self.ptfhost.shell("ifconfig %s %s/32" % (self.iface, self.ip))
         self.ptfhost.exabgp(
             name=self.name,
             state="started",
@@ -109,16 +108,12 @@ def start_session(self):
             allow_ebgp_multihop_cmd %= (self.peer_asn, self.ip)
             self.duthost.shell(allow_ebgp_multihop_cmd)
 
-        # populate DUT arp table
-        self.duthost.shell("ping -c 3 %s" % (self.ip))
-
     def stop_session(self):
         """Stop the BGP session."""
         logging.debug("stop bgp session %s", self.name)
         self.duthost.shell("redis-cli -n 4 -c DEL 'BGP_NEIGHBOR|%s'" % self.ip)
         self.duthost.shell("redis-cli -n 4 -c DEL 'DEVICE_NEIGHBOR_METADATA|%s'" % self.name)
         self.ptfhost.exabgp(name=self.name, state="absent")
-        self.ptfhost.shell("ifconfig %s 0.0.0.0" % self.iface)
 
     # TODO: let's put those BGP utility functions in a common place.
     def announce_route(self, route):
@@ -167,22 +162,18 @@ def is_quagga(duthost):
 
 
 @pytest.fixture
-def common_setup_teardown(duthost, is_quagga, ptfhost):
+def common_setup_teardown(duthost, is_quagga, ptfhost, setup_interfaces):
     mg_facts = duthost.minigraph_facts(host=duthost.hostname)["ansible_facts"]
-
+    conn0, conn1 = setup_interfaces
     dut_asn = mg_facts["minigraph_bgp_asn"]
-    dut_lo_addr = mg_facts["minigraph_lo_interfaces"][0]["addr"]
-    dut_mgmt_iface = mg_facts["minigraph_mgmt_interface"]["alias"]
-    dut_mgmt_addr = mg_facts["minigraph_mgmt_interface"]["addr"]
     bgp_neighbors = (
         BGPNeighbor(
             duthost,
             ptfhost,
             "pseudoswitch0",
-            "mgmt:0",
-            "10.10.10.10",
+            conn0["neighbor_addr"].split("/")[0],
             NEIGHBOR_ASN0,
-            dut_lo_addr,
+            conn0["local_addr"].split("/")[0],
             dut_asn,
             NEIGHBOR_PORT0,
             is_quagga=is_quagga
@@ -191,32 +182,20 @@ def common_setup_teardown(duthost, is_quagga, ptfhost):
             duthost,
             ptfhost,
             "pseudoswitch1",
-            "mgmt:1",
-            "10.10.10.11",
+            conn1["neighbor_addr"].split("/")[0],
             NEIGHBOR_ASN1,
-            dut_lo_addr,
+            conn1["local_addr"].split("/")[0],
             dut_asn,
             NEIGHBOR_PORT1,
             is_quagga=is_quagga
         )
     )
 
-    add_route_tmpl = "ip route add %s/32 via %s dev %s"
-    ptfhost.shell(add_route_tmpl % (dut_lo_addr, dut_mgmt_addr, "mgmt"))
-    duthost.shell(add_route_tmpl % (bgp_neighbors[0].ip, bgp_neighbors[0].ptfip, dut_mgmt_iface))
-    duthost.shell(add_route_tmpl % (bgp_neighbors[1].ip, bgp_neighbors[0].ptfip, dut_mgmt_iface))
-
-    yield bgp_neighbors, dut_mgmt_iface
-
-    flush_route_tmpl = "ip route flush %s/32"
-    ptfhost.shell(flush_route_tmpl % dut_lo_addr)
-    duthost.shell(flush_route_tmpl % bgp_neighbors[0].ip)
-    duthost.shell(flush_route_tmpl % bgp_neighbors[1].ip)
-    duthost.shell("sonic-clear arp")
+    return bgp_neighbors
 
 
 @pytest.fixture
-def constants(is_quagga, ptfhost):
+def constants(is_quagga, setup_interfaces):
     class _C(object):
         """Dummy class to save test constants."""
         pass
@@ -229,10 +208,11 @@ class _C(object):
         _constants.sleep_interval = 5
         _constants.update_interval_threshold = 1
 
+    conn0 = setup_interfaces[0]
     _constants.routes = []
     for subnet in ANNOUNCED_SUBNETS:
         _constants.routes.append(
-            {"prefix": subnet, "nexthop": ptfhost.mgmt_ip}
+            {"prefix": subnet, "nexthop": conn0["neighbor_addr"].split("/")[0]}
         )
     return _constants
 
@@ -261,7 +241,7 @@ def match_bgp_update(packet, src_ip, dst_ip, action, route):
         else:
             return False
 
-    (n0, n1), dut_mgmt_iface = common_setup_teardown
+    n0, n1 = common_setup_teardown
     try:
         n0.start_session()
         n1.start_session()
@@ -280,7 +260,7 @@ def match_bgp_update(packet, src_ip, dst_ip, action, route):
         withdraw_intervals = []
         for i, route in enumerate(constants.routes):
             bgp_pcap = BGP_LOG_TMPL % i
-            with log_bgp_updates(duthost, dut_mgmt_iface, bgp_pcap):
+            with log_bgp_updates(duthost, "any", bgp_pcap):
                 n0.announce_route(route)
                 time.sleep(constants.sleep_interval)
                 n0.withdraw_route(route)