Merge pull request #12418 from opensourcerouting/fix/tests_for_while_true

Fix old tests with `while true`

bgpd/bgp_route.c

@@ -4281,14 +4281,6 @@ int bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
 	    && (!CHECK_FLAG(dest->flags, BGP_NODE_FIB_INSTALLED)))
 		SET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING);
 
-	/* If maximum prefix count is configured and current prefix
-	 * count exeed it.
-	 */
-	if (bgp_maximum_prefix_overflow(peer, afi, safi, 0)) {
-		bgp_attr_flush(&new_attr);
-		return -1;
-	}
-
 	/* If neighbor soo is configured, tag all incoming routes with
 	 * this SoO tag and then filter out advertisements in
 	 * subgroup_announce_check() if it matches the configured SoO
@@ -4803,6 +4795,17 @@ int bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
 		bgp_path_info_set_flag(dest, new, BGP_PATH_VALID);
 	}
 
+	/* If maximum prefix count is configured and current prefix
+	 * count exeed it.
+	 */
+	if (bgp_maximum_prefix_overflow(peer, afi, safi, 0)) {
+		reason = "maximum-prefix overflow";
+		bgp_attr_flush(&new_attr);
+		bgp_unlink_nexthop(new);
+		bgp_path_info_delete(dest, new);
+		goto filtered;
+	}
+
 	/* Addpath ID */
 	new->addpath_rx_id = addpath_id;
 

tests/topotests/bgp_comm_list_delete/test_bgp_comm-list_delete.py

@@ -34,11 +34,13 @@ import os
 import sys
 import json
 import pytest
+import functools
 
 CWD = os.path.dirname(os.path.realpath(__file__))
 sys.path.append(os.path.join(CWD, "../"))
 
 # pylint: disable=C0413
+from lib import topotest
 from lib.topogen import Topogen, TopoRouter, get_topogen
 
 pytestmark = [pytest.mark.bgpd]
@@ -81,30 +83,34 @@ def test_bgp_maximum_prefix_invalid():
     if tgen.routers_have_failure():
         pytest.skip(tgen.errors)
 
-    def _bgp_converge(router):
+    r2 = tgen.gears["r2"]
-        while True:
-            output = json.loads(
-                tgen.gears[router].vtysh_cmd("show ip bgp neighbor 192.168.255.1 json")
-            )
-            if output["192.168.255.1"]["bgpState"] == "Established":
-                if (
-                    output["192.168.255.1"]["addressFamilyInfo"]["ipv4Unicast"][
-                        "acceptedPrefixCounter"
-                    ]
-                    == 2
-                ):
-                    return True
 
-    def _bgp_comm_list_delete(router):
+    def _bgp_converge():
-        output = json.loads(
+        output = json.loads(r2.vtysh_cmd("show ip bgp neighbor 192.168.255.1 json"))
-            tgen.gears[router].vtysh_cmd("show ip bgp 172.16.255.254/32 json")
+        expected = {
-        )
+            "192.168.255.1": {
-        if "333:333" in output["paths"][0]["community"]["list"]:
+                "bgpState": "Established",
-            return False
+                "addressFamilyInfo": {
-        return True
+                    "ipv4Unicast": {
+                        "acceptedPrefixCounter": 2,
+                    }
+                },
+            }
+        }
+        return topotest.json_cmp(output, expected)
 
-    if _bgp_converge("r2"):
+    test_func = functools.partial(_bgp_converge)
-        assert _bgp_comm_list_delete("r2") == True
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
+    assert result is None, "Can't converge initially"
+
+    def _bgp_comm_list_delete():
+        output = json.loads(r2.vtysh_cmd("show ip bgp 172.16.255.254/32 json"))
+        expected = {"paths": [{"community": {"list": ["333:333"]}}]}
+        return topotest.json_cmp(output, expected)
+
+    test_func = functools.partial(_bgp_comm_list_delete)
+    _, result = topotest.run_and_expect(test_func, not None, count=60, wait=0.5)
+    assert result is not None, "333:333 community SHOULD be stripped from r1"
 
 
 if __name__ == "__main__":

tests/topotests/bgp_local_as_private_remove/test_bgp_local_as_private_remove.py

@@ -31,13 +31,14 @@ used together with `remove-private-AS`.
 import os
 import sys
 import json
-import time
 import pytest
+import functools
 
 CWD = os.path.dirname(os.path.realpath(__file__))
 sys.path.append(os.path.join(CWD, "../"))
 
 # pylint: disable=C0413
+from lib import topotest
 from lib.topogen import Topogen, TopoRouter, get_topogen
 
 pytestmark = [pytest.mark.bgpd]
@@ -84,29 +85,43 @@ def test_bgp_remove_private_as():
     if tgen.routers_have_failure():
         pytest.skip(tgen.errors)
 
-    def _bgp_converge(router):
+    r2 = tgen.gears["r2"]
-        while True:
+    r4 = tgen.gears["r4"]
-            output = json.loads(
-                tgen.gears[router].vtysh_cmd("show ip bgp neighbor 192.168.255.1 json")
-            )
-            if output["192.168.255.1"]["bgpState"] == "Established":
-                time.sleep(1)
-                return True
 
-    def _bgp_as_path(router):
+    def _bgp_converge():
-        output = json.loads(
+        output = json.loads(r2.vtysh_cmd("show ip bgp neighbor 192.168.255.1 json"))
-            tgen.gears[router].vtysh_cmd("show ip bgp 172.16.255.254/32 json")
+        expected = {
-        )
+            "192.168.255.1": {
-        if output["prefix"] == "172.16.255.254/32":
+                "bgpState": "Established",
-            return output["paths"][0]["aspath"]["segments"][0]["list"]
+            }
+        }
+        return topotest.json_cmp(output, expected)
 
-    if _bgp_converge("r2"):
+    test_func = functools.partial(_bgp_converge)
-        assert len(_bgp_as_path("r2")) == 1
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
-        assert 65000 not in _bgp_as_path("r2")
+    assert result is None, "Can't converge initially"
 
-    if _bgp_converge("r4"):
+    def _bgp_as_path(router, asn_path, asn_length):
-        assert len(_bgp_as_path("r4")) == 2
+        output = json.loads(router.vtysh_cmd("show ip bgp 172.16.255.254/32 json"))
-        assert 3000 in _bgp_as_path("r4")
+        expected = {
+            "paths": [
+                {
+                    "aspath": {
+                        "string": asn_path,
+                        "length": asn_length,
+                    }
+                }
+            ]
+        }
+        return topotest.json_cmp(output, expected)
+
+    test_func = functools.partial(_bgp_as_path, r2, "500", 1)
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
+    assert result is None, "Private ASNs not stripped"
+
+    test_func = functools.partial(_bgp_as_path, r4, "500 3000", 2)
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
+    assert result is None, "Private ASNs not stripped"
 
 
 if __name__ == "__main__":

tests/topotests/bgp_maximum_prefix_invalid_update/r1/bgpd.conf

@@ -1,6 +1,13 @@
 router bgp 65000
-  no bgp ebgp-requires-policy
+ no bgp ebgp-requires-policy
-  neighbor 192.168.255.2 remote-as 65001
+ neighbor 192.168.255.2 remote-as 65001
-  neighbor 192.168.255.2 timers 3 10
+ neighbor 192.168.255.2 timers 3 10
-  address-family ipv4 unicast
+ address-family ipv4 unicast
-    redistribute connected
+  redistribute connected
+  neighbor 192.168.255.2 prefix-list r2 out
+ exit-address-family
+ !
+!
+ip prefix-list r2 seq 5 permit 172.16.255.253/32
+ip prefix-list r2 seq 10 permit 172.16.255.254/32
+!

tests/topotests/bgp_maximum_prefix_invalid_update/r1/zebra.conf

@@ -1,6 +1,7 @@
 !
 interface lo
  ip address 172.16.255.254/32
+ ip address 172.16.255.253/32
 !
 interface r1-eth0
  ip address 192.168.255.1/24

tests/topotests/bgp_maximum_prefix_invalid_update/r2/bgpd.conf

@@ -1,6 +1,9 @@
 router bgp 65001
-  no bgp ebgp-requires-policy
+ no bgp ebgp-requires-policy
-  neighbor 192.168.255.1 remote-as 65000
+ neighbor 192.168.255.1 remote-as 65000
-  neighbor 192.168.255.1 timers 3 10
+ neighbor 192.168.255.1 timers 3 10
-  address-family ipv4
+ address-family ipv4
-    neighbor 192.168.255.1 maximum-prefix 1
+  neighbor 192.168.255.1 maximum-prefix 1
+ exit-address-family
+ !
+!

tests/topotests/bgp_maximum_prefix_invalid_update/test_bgp_maximum_prefix_invalid_update.py

@@ -36,11 +36,13 @@ import os
 import sys
 import json
 import pytest
+import functools
 
 CWD = os.path.dirname(os.path.realpath(__file__))
 sys.path.append(os.path.join(CWD, "../"))
 
 # pylint: disable=C0413
+from lib import topotest
 from lib.topogen import Topogen, TopoRouter, get_topogen
 
 pytestmark = [pytest.mark.bgpd]
@@ -83,29 +85,21 @@ def test_bgp_maximum_prefix_invalid():
     if tgen.routers_have_failure():
         pytest.skip(tgen.errors)
 
-    def _bgp_converge(router):
+    r2 = tgen.gears["r2"]
-        while True:
-            output = json.loads(
-                tgen.gears[router].vtysh_cmd("show ip bgp neighbor 192.168.255.1 json")
-            )
-            if output["192.168.255.1"]["connectionsEstablished"] > 0:
-                return True
 
-    def _bgp_parsing_nlri(router):
+    def _bgp_parsing_nlri():
-        cmd_max_exceeded = (
+        output = json.loads(r2.vtysh_cmd("show ip bgp neighbor 192.168.255.1 json"))
-            'grep "%MAXPFXEXCEED: No. of IPv4 Unicast prefix received" bgpd.log'
+        expected = {
-        )
+            "192.168.255.1": {
-        cmdt_error_parsing_nlri = 'grep "Error parsing NLRI" bgpd.log'
+                "lastNotificationReason": "Cease/Maximum Number of Prefixes Reached",
-        output_max_exceeded = tgen.gears[router].run(cmd_max_exceeded)
+                "lastResetDueTo": "BGP Notification send",
-        output_error_parsing_nlri = tgen.gears[router].run(cmdt_error_parsing_nlri)
+            }
+        }
+        return topotest.json_cmp(output, expected)
 
-        if len(output_max_exceeded) > 0:
+    test_func = functools.partial(_bgp_parsing_nlri)
-            if len(output_error_parsing_nlri) > 0:
+    _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5)
-                return False
+    assert result is None, "Didn't send NOTIFICATION when hitting maximum-prefix"
-        return True
-
-    if _bgp_converge("r2"):
-        assert _bgp_parsing_nlri("r2") == True
 
 
 if __name__ == "__main__":