feat(anta.tests): Adding the test case to verify BGP neighbor routes #484

MaheshGSLAB · 2023-12-05T11:55:54Z

Description

Add the test case to verify the BGP neighbor routes

Run the testcase by adding catalog as below:

anta.tests.routing:
  bgp:
    - VerifyBGPExchangedRoutes:
        bgp_peers:
          - peer_address: 172.30.255.5
            vrf: default
            advertised_routes:
              - 192.0.254.5/32
            received_routes:
              - 192.0.255.4/32
          - peer_address: 172.30.255.1
            vrf: default
            advertised_routes:
              - 192.0.255.1/32
              - 192.0.254.5/32
            received_routes:
              - 192.0.254.3/32

Testcase pass when routes are correct:

                                                                                          All tests results                                                                                           
┏━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Device    ┃ Test Name                ┃ Test Status ┃ Message(s) ┃ Test description                                                                                                 ┃ Test category ┃
┡━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ s1-spine1 │ VerifyBGPExchangedRoutes │ success     │            │ Verifies if BGP peers have correctly advertised/received routes with type as valid and active for a specified    │ routing, bgp  │
│           │                          │             │            │ VRF.                                                                                                             │               │
└───────────┴──────────────────────────┴─────────────┴────────────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────┴───────────────┘

Testcase fail when routes are missing:

                                                                                          All tests results                                                                                           
┏━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Device    ┃ Test Name                ┃ Test Status ┃ Message(s)                                                    ┃ Test description                                              ┃ Test category ┃
┡━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ s1-spine1 │ VerifyBGPExchangedRoutes │ failure     │ Following BGP peers are not found or routes are not exchanged │ Verifies if BGP peers have correctly advertised/received      │ routing, bgp  │
│           │                          │             │ properly:                                                     │ routes with type as valid and active for a specified VRF.     │               │
│           │                          │             │ {'bgp_peers': {'172.30.11.1': {'MGMT': 'Not configured'}}}    │                                                               │               │
└───────────┴──────────────────────────┴─────────────┴───────────────────────────────────────────────────────────────┴───────────────────────────────────────────────────────────────┴───────────────┘

Testcase fail when routes are invalid or inactive

 
                                                                                          All tests results                                                                                           
┏━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Device    ┃ Test Name                ┃ Test Status ┃ Message(s)                                                    ┃ Test description                                              ┃ Test category ┃
┡━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ s1-spine1 │ VerifyBGPExchangedRoutes │ failure     │ Following BGP peers are not found or routes are not exchanged │ Verifies if BGP peers have correctly advertised/received      │ routing, bgp  │
│           │                          │             │ properly:                                                     │ routes with type as valid and active for a specified VRF.     │               │
│           │                          │             │ {'bgp_peers': {'172.30.11.1': {'default':                     │                                                               │               │
│           │                          │             │ {'advertised_routes': {'192.0.254.31/32': 'Not found'},       │                                                               │               │
│           │                          │             │ 'received_routes': {'192.0.255.4/32': {'valid': False,        │                                                               │               │
│           │                          │             │ 'active': True}}}}, '172.30.11.5': {'default':                │                                                               │               │
│           │                          │             │ {'advertised_routes': {'192.2.254.31/32': 'Not found'},       │                                                               │               │
│           │                          │             │ 'received_routes': {'192.0.255.41/32': 'Not found'}}}}}       │                                                               │               │
└───────────┴──────────────────────────┴─────────────┴───────────────────────────────────────────────────────────────┴───────────────────────────────────────────────────────────────┴───────────────┘

Fixes #482

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my code
I have run pre-commit for code linting and typing (pre-commit run)
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes (tox -e testenv)

anta/tests/routing/bgp.py

tests/units/anta_tests/routing/test_bgp.py

anta/tests/routing/bgp.py

gmuloc · 2024-01-03T16:10:40Z

anta/tests/routing/bgp.py

@@ -132,6 +132,61 @@ def _check_peer_issues(peer_data: Optional[dict[str, Any]]) -> dict[str, Any]:
    return {}


+def _add_bgp_routes_failure(
+    bgp_routes: List[str], bgp_output: dict[str, Any], neighbor: str, vrf: str, route_type: str = "advertised_routes"


Suggested change

bgp_routes: List[str], bgp_output: dict[str, Any], neighbor: str, vrf: str, route_type: str = "advertised_routes"

bgp_routes: list[str], bgp_output: dict[str, Any], neighbor: str, vrf: str, route_type: str = "advertised_routes"

gmuloc · 2024-01-03T16:11:27Z

anta/tests/routing/bgp.py

+    It identifies any missing routes as well as any routes that are invalid or inactive. The results are returned in a dictionary.
+
+    Parameters:
+        bgp_routes (List[str]): The list of expected routes.


Suggested change

bgp_routes (List[str]): The list of expected routes.

bgp_routes (list[str]): The list of expected routes.

gmuloc · 2024-01-03T16:14:11Z

anta/tests/routing/bgp.py

+    missing_routes = [route for route in bgp_routes if route not in bgp_output]
+    invalid_or_inactive = [
+        route
+        for route in bgp_routes
+        if route in bgp_output
+        and (not bgp_output[route]["bgpRoutePaths"][0]["routeType"]["valid"] or not bgp_output[route]["bgpRoutePaths"][0]["routeType"]["active"])
+    ]


Here you are looping twice over the bgp_routes so it is probably more efficient to use a for loop and loop only once:

for route in bgp_routes: if not route in bgp_output: missing_routes.append(route) continue if (big condition): invalid_or_inactive.append(route)

gmuloc · 2024-01-03T16:17:11Z

anta/tests/routing/bgp.py

+            received_routes = command.params.get("received_routes", [])
+
+            # Verify if BGP neighbor is configured with provided vrf
+            if (bgp_routes := get_value(command.json_output, f"vrfs..{vrf}", separator="..")) is None:


Do you need .. as separator here?

gmuloc · 2024-01-03T16:19:42Z

anta/tests/routing/bgp.py

+            # Verify if BGP neighbor is configured with provided vrf
+            if (bgp_routes := get_value(command.json_output, f"vrfs..{vrf}", separator="..")) is None:
+                self.result.is_failure(f"BGP neighbor {str(neighbor)} is not configured for `{vrf}` VRF.")
+                return


if you return here then you won't check all the commands right? So maybe you will miss some failures in other commands that you could report

Would it not be better with

Suggested change

return

continue

?

(same below)

tests/units/anta_tests/routing/test_bgp.py

gmuloc · 2024-01-03T16:23:36Z

tests/units/anta_tests/routing/test_bgp.py

+                {
+                    "neighbor": "172.30.11.1",
+                    "vrf": "default",
+                    "advertised_routes": ["192.0.254.31/32"],
+                    "received_routes": ["192.0.255.31/32"],
+                },


test with more than one neighbor in your inputs, in particular with one success and one failure and check that the test is failing

gmuloc · 2024-01-10T15:49:17Z

anta/tests/routing/bgp.py

+        # Check if the route is missing in the BGP output
+        if route not in bgp_output:
+            # If missing, add it to the failure routes dictionary
+            failure_routes.setdefault("bgp_peers", {}).setdefault(peer, {}).setdefault(vrf, {}).setdefault(route_type, {})[route] = "Not found"


initialize this at the begining of the function with the proper structure and use a flag to track if a failure happened

this chain of setdefault is dificult to understand

carl-baillargeon · 2024-01-10T16:07:03Z

anta/tests/routing/bgp.py

+            peer = str(command.params.get("peer", ""))
+            vrf = command.params.get("vrf", "")
+            advertised_routes = command.params.get("advertised_routes", [])
+            received_routes = command.params.get("received_routes", [])


As discussed, in recent ANTA refactor, it looks like command.params is properly typed as Dict[str, Any] so we can simply do a dict lookup without mypy complaining. No need to bother with .get() or cast anymore.

done

…aristanetworks#484)

MaheshGSLAB self-assigned this Dec 5, 2023

MaheshGSLAB marked this pull request as draft December 5, 2023 11:56

MaheshGSLAB requested review from carl-baillargeon and removed request for carl-baillargeon December 5, 2023 11:56

MaheshGSLAB marked this pull request as ready for review December 7, 2023 10:07

carl-baillargeon requested changes Dec 12, 2023

View reviewed changes

MaheshGSLAB requested a review from carl-baillargeon December 13, 2023 09:54

gmuloc requested changes Jan 3, 2024

View reviewed changes

MaheshGSLAB requested a review from gmuloc January 4, 2024 10:34

MaheshGSLAB changed the title ~~feat: Adding the test case to verify BGP neighbor routes~~ feat(anta.tests)!: Adding the test case to verify BGP neighbor routes Jan 4, 2024

MaheshGSLAB added the rn: feat(anta.tests)! label Jan 4, 2024

gmuloc removed the rn: feat(anta.tests)! label Jan 5, 2024

carl-baillargeon added the one-more-review label Jan 10, 2024

gmuloc requested changes Jan 10, 2024

View reviewed changes

carl-baillargeon previously requested changes Jan 10, 2024

View reviewed changes

arista and others added 11 commits January 12, 2024 05:10

issue-482: Adding the testcase for BGP neighbor routes

cecba90

issue-482: Updated failure logic

59356c5

issue-482: Added the BGP exchanged routes testcase

4aea367

issue-482: fix thhe mypy errors

2d218e5

issue-482: Addressed review comments for doc string and helper function

b6b7b35

issue-482: fix typo

da0f98b

issue-482: Improved code quality

12c753c

issue-482: Updated unit tests with more bgp peers

224d7f2

issue-482: removed ipv6 support

fec7ef9

issue-482: used Ipv4 Network module

60fa966

issue-482: removed setdefaults method

6cf2896

MaheshGSLAB force-pushed the issue-482 branch from d4bef55 to 6cf2896 Compare January 12, 2024 06:20

MaheshGSLAB requested review from gmuloc and carl-baillargeon January 12, 2024 06:21

gmuloc approved these changes Jan 24, 2024

View reviewed changes

gmuloc merged commit d6bb7cb into aristanetworks:main Jan 24, 2024
19 checks passed

github-actions bot added the rn: feat(anta.tests)! label Jan 24, 2024

MaheshGSLAB added a commit to MaheshGSLAB/anta that referenced this pull request Jan 30, 2024

feat(anta.tests)!: Adding the test case to verify BGP neighbor routes (…

5eea441

…aristanetworks#484)

titom73 removed the one-more-review label Feb 7, 2024

gmuloc changed the title ~~feat(anta.tests)!: Adding the test case to verify BGP neighbor routes~~ feat(anta.tests): Adding the test case to verify BGP neighbor routes Mar 4, 2024

github-actions bot added rn: feat(anta.tests) and removed rn: feat(anta.tests)! labels Mar 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(anta.tests): Adding the test case to verify BGP neighbor routes #484

feat(anta.tests): Adding the test case to verify BGP neighbor routes #484

MaheshGSLAB commented Dec 5, 2023 •

edited

Loading

gmuloc Jan 3, 2024

gmuloc Jan 3, 2024

gmuloc Jan 3, 2024

gmuloc Jan 3, 2024

gmuloc Jan 3, 2024

gmuloc Jan 3, 2024

gmuloc Jan 10, 2024

carl-baillargeon Jan 10, 2024

	bgp_routes: List[str], bgp_output: dict[str, Any], neighbor: str, vrf: str, route_type: str = "advertised_routes"
	bgp_routes: list[str], bgp_output: dict[str, Any], neighbor: str, vrf: str, route_type: str = "advertised_routes"

	bgp_routes (List[str]): The list of expected routes.
	bgp_routes (list[str]): The list of expected routes.

feat(anta.tests): Adding the test case to verify BGP neighbor routes #484

feat(anta.tests): Adding the test case to verify BGP neighbor routes #484

Conversation

MaheshGSLAB commented Dec 5, 2023 • edited Loading

Description

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaheshGSLAB commented Dec 5, 2023 •

edited

Loading