feat(anta.tests): Adding the test case to verify BGP ECMP paths #507

MaheshGSLAB · 2024-01-02T09:03:11Z

Description

Adding the test case to verify BGP ECMP path install

Test catalog:

anta.tests.routing:
  bgp:
      - VerifyBGPEcmpPath:
          bgp_routes:
            - route: 192.0.254.5/32
              vrf: default
            - route: 192.0.254.3/32
              vrf: default

Testcase pass if route is contributed into ECMP and at least one path is ECMP head in specified vrf.


                                                                          All tests results                                                                           
┏━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Device    ┃ Test Name         ┃ Test Status ┃ Message(s) ┃ Test description                                                                        ┃ Test category ┃
┡━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ s1-spine1 │ VerifyBGPEcmpPath │ success     │            │ Verifies the installation of BGP Equal-Cost Multipath (ECMP) path in the specified VRF. │ routing, bgp  │
└───────────┴───────────────────┴─────────────┴────────────┴─────────────────────────────────────────────────────────────────────────────────────────┴───────────────┘

Testcase fail if route is not found in the specified vrf.

                                                                                         All tests results                                                                                           
┏━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Device    ┃ Test Name         ┃ Test Status ┃ Message(s)                                                        ┃ Test description                                                 ┃ Test category ┃
┡━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ s1-spine1 │ VerifyBGPEcmpPath │ failure     │ Following BGP routes are not configured, not contributed in ecmp, │ Verifies the installation of BGP Equal-Cost Multipath (ECMP)     │ routing, bgp  │
│           │                   │             │ or ecmp head is not found:                                        │ path in the specified VRF.                                       │               │
│           │                   │             │ {'192.0.254.5/32': {'MGMT': 'Not configured'}}                    │                                                                  │               │
└───────────┴───────────────────┴─────────────┴───────────────────────────────────────────────────────────────────┴──────────────────────────────────────────────────────────────────┴───────────────┘

Testcase fail if route is not contributed in ECMP or ECMP head is not found in the specified vrf.

                                                                                         All tests results                                                                                           
┏━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Device    ┃ Test Name         ┃ Test Status ┃ Message(s)                                                        ┃ Test description                                                 ┃ Test category ┃
┡━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ s1-spine1 │ VerifyBGPEcmpPath │ failure     │ Following BGP routes are not configured, not contributed in ecmp, │ Verifies the installation of BGP Equal-Cost Multipath (ECMP)     │ routing, bgp  │
│           │                   │             │ or ecmp head is not found:                                        │ path in the specified VRF.                                       │               │
│           │                   │             │ {'192.0.255.5/32': {'default': 'ECMP path is not installed'}}     │                                                                  │               │
└───────────┴───────────────────┴─────────────┴───────────────────────────────────────────────────────────────────┴──────────────────────────────────────────────────────────────────┴───────────────┘

Fixes #505

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

github-actions · 2024-01-24T14:04:43Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

carl-baillargeon

Please hold on this one. The logic is pretty complex, we might have to split it in 2 test cases.

anta/tests/routing/bgp.py

github-actions · 2025-01-22T13:21:37Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

codspeed-hq · 2025-01-22T13:30:27Z

CodSpeed Performance Report

Merging #507 will not alter performance

_{Comparing MaheshGSLAB:issue-505 (667403d) with main (9c7a81c)}

Summary

✅ 22 untouched benchmarks

anta/tests/routing/bgp.py

tests/units/input_models/routing/test_bgp.py

…ed route

… active ecmp head

sonarqubecloud · 2025-01-30T07:34:09Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

carl-baillargeon · 2025-02-02T17:24:17Z

anta/tests/routing/bgp.py

+                self.result.is_failure(f"{route} - prefix not found in BGP table")
+                continue
+
+            route_paths = bgp_route_entry["bgpRoutePaths"]


It's better to create an iterator from the start and work with it. That way, you don't need to create a generator expression on line 1767 and remove the head on line 1774.

Suggested change

route_paths = bgp_route_entry["bgpRoutePaths"]

route_paths = iter(bgp_route_entry["bgpRoutePaths"])

carl-baillargeon · 2025-02-02T17:34:20Z

anta/tests/routing/bgp.py

+            head = next((path for path in route_paths if all(path.get("routeType", {}).get(key, False) for key in ["valid", "active", "ecmpHead"])), None)
+            # Verify if the active ECMP head exists in routepath.
+            if not head:
+                self.result.is_failure(f"{route} - valid and active ECMP head not found")
+                continue


routeType and valid, active, ecmpHead will always be there so we can safely access them not using get(). I will show you how we can find the CLI models for EOS this week.

Suggested change

head = next((path for path in route_paths if all(path.get("routeType", {}).get(key, False) for key in ["valid", "active", "ecmpHead"])), None)

# Verify if the active ECMP head exists in routepath.

if not head:

self.result.is_failure(f"{route} - valid and active ECMP head not found")

continue

head = next(route_paths, None)

# Verify if the active ECMP head exists.

if head is None or not all(head[key] for key in ["valid", "active", "ecmpHead"]):

self.result.is_failure(f"{route} - valid and active ECMP head not found")

continue

carl-baillargeon · 2025-02-02T17:36:40Z

anta/tests/routing/bgp.py

+                self.result.is_failure(f"{route} - valid and active ECMP head not found")
+                continue
+
+            bgp_nexthops = [head.get("nextHop")]


In BGP, we can safely assume that any route marked as valid + active + ecmpHead will have a nextHop value.

Suggested change

bgp_nexthops = [head.get("nextHop")]

bgp_nexthops = {head["nextHop"]}

carl-baillargeon · 2025-02-02T17:39:50Z

anta/tests/routing/bgp.py

+                continue
+
+            bgp_nexthops = [head.get("nextHop")]
+            route_paths.remove(head)


Not needed if we use the same iterator.

carl-baillargeon · 2025-02-02T17:49:58Z

anta/tests/routing/bgp.py

+
+            bgp_nexthops = [head.get("nextHop")]
+            route_paths.remove(head)
+            bgp_nexthops.extend([path.get("nextHop") for path in route_paths if all(path.get("routeType")[key] for key in ["valid", "ecmp", "ecmpContributor"])])


Same as above, no need to use get().

carl-baillargeon · 2025-02-02T17:58:22Z

anta/tests/routing/bgp.py

+            bgp_nexthops = [head.get("nextHop")]
+            route_paths.remove(head)
+            bgp_nexthops.extend([path.get("nextHop") for path in route_paths if all(path.get("routeType")[key] for key in ["valid", "ecmp", "ecmpContributor"])])
+            bgp_nexthops = ["linked-locally" if nexthop == "" else nexthop for nexthop in bgp_nexthops]


Where does linked-locally come from? I don't think it's required since we can also safely assume that any route marked as valid + ecmp + ecmpContributor will have a nextHop value.

carl-baillargeon · 2025-02-02T18:02:10Z

anta/tests/routing/bgp.py

+            rib_nexthops = [via.get("nexthopAddr", "linked-locally") for via in route_entry["vias"] if route_entry["routeType"] in {"iBGP", "eBGP"}]
+            # Verify BGP and RIB nexthops are same.
+            if sorted(bgp_nexthops) != sorted(rib_nexthops):
+                self.result.is_failure(f"{route} - nexthops mismatch - BGP: {', '.join(sorted(bgp_nexthops))}, RIB: {', '.join(sorted(rib_nexthops))}")


We should use sets to do the comparison and sort them in the failure message.

carl-baillargeon · 2025-02-02T18:04:35Z

tests/units/anta_tests/routing/test_bgp.py

+                                        "nextHop": "",
+                                        "routeType": {
+                                            "valid": True,
+                                            "active": False,
+                                            "ecmpHead": False,
+                                            "ecmp": True,
+                                            "ecmpContributor": True,


Where you able to reproduce this on EOS?

carl-baillargeon · 2025-02-02T18:04:59Z

tests/units/anta_tests/routing/test_bgp.py

+                                        "nextHop": "",
+                                        "routeType": {
+                                            "valid": True,
+                                            "active": False,
+                                            "ecmpHead": False,
+                                            "ecmp": True,
+                                            "ecmpContributor": True,


Same as above.

carl-baillargeon · 2025-02-02T18:06:47Z

tests/units/anta_tests/routing/test_bgp.py

+                        "allRoutesProgrammedKernel": True,
+                        "defaultRouteState": "notSet",
+                        "routes": {
+                            "10.111.112.0/24": {"routeAction": "forward", "vias": [{"interface": "Vlan112"}]},


Doesn't really matter for this but routeType is missing.

MaheshGSLAB self-assigned this Jan 2, 2024

MaheshGSLAB marked this pull request as ready for review January 2, 2024 09:07

MaheshGSLAB requested a review from carl-baillargeon January 2, 2024 14:19

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

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

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

gmuloc changed the title ~~feat(anta.tests)!: Adding the test case to verify BGP ECMP path install~~ feat(anta.tests): Adding the test case to verify BGP ECMP path install Jan 5, 2024

carl-baillargeon requested changes Jan 6, 2024

View reviewed changes

MaheshGSLAB requested review from carl-baillargeon and gmuloc January 12, 2024 11:14

MaheshGSLAB force-pushed the issue-505 branch from 95d7613 to dcd5b0d Compare January 12, 2024 11:34

MaheshGSLAB commented Jan 24, 2024

View reviewed changes

anta/tests/routing/bgp.py Outdated Show resolved Hide resolved

github-actions bot added the state: conflict label Jan 24, 2024

carl-baillargeon requested changes Jan 29, 2024

View reviewed changes

anta/tests/routing/bgp.py Outdated Show resolved Hide resolved

anta/tests/routing/bgp.py Outdated Show resolved Hide resolved

anta/tests/routing/bgp.py Outdated Show resolved Hide resolved

MaheshGSLAB changed the title ~~feat(anta.tests): Adding the test case to verify BGP ECMP path install~~ feat(anta.tests): Adding the test case to verify BGP ECMP path install(On Hold) Feb 1, 2024

MaheshGSLAB marked this pull request as draft February 23, 2024 11:29

gmuloc added the network-tests label Apr 22, 2024

carl-baillargeon added this to the v1.1.0 milestone Apr 22, 2024

gmuloc modified the milestones: v1.1.0, v1.2.0 Oct 11, 2024

carl-baillargeon modified the milestones: v1.2.0, v1.3.0 Nov 25, 2024

refactor ECMP test

3c4b595

carl-baillargeon force-pushed the issue-505 branch from dcd5b0d to 3c4b595 Compare January 22, 2025 13:21

github-actions bot removed the state: conflict label Jan 22, 2025

carl-baillargeon changed the title ~~feat(anta.tests): Adding the test case to verify BGP ECMP path install(On Hold)~~ feat(anta.tests): Adding the test case to verify BGP ECMP paths Jan 22, 2025

Merge branch 'main' into issue-505

5d69998

Merge branch 'main' into issue-505

7525d6b

geetanjalimanegslab assigned geetanjalimanegslab and unassigned MaheshGSLAB Jan 28, 2025

Issue05: Updated test logic, added unit test for test and input model

bdbabae

geetanjalimanegslab requested a review from vitthalmagadum January 29, 2025 12:35

Issue-505: Added unit tests for VerifyBGPRoutePaths input model

59204f1

vitthalmagadum requested changes Jan 30, 2025

View reviewed changes

anta/tests/routing/bgp.py Outdated Show resolved Hide resolved

anta/tests/routing/bgp.py Outdated Show resolved Hide resolved

tests/units/input_models/routing/test_bgp.py Outdated Show resolved Hide resolved

geetanjalimanegslab added 2 commits January 30, 2025 05:55

Issue_505: Updated unit test and test logic to support direct connect…

cad434e

…ed route

Issue-505: Updated test with handling directly connected nexthop with…

667403d

… active ecmp head

vitthalmagadum marked this pull request as ready for review January 30, 2025 11:35

vitthalmagadum requested a review from carl-baillargeon January 30, 2025 11:35

vitthalmagadum approved these changes Jan 30, 2025

View reviewed changes

carl-baillargeon requested changes Feb 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(anta.tests): Adding the test case to verify BGP ECMP paths #507

feat(anta.tests): Adding the test case to verify BGP ECMP paths #507

MaheshGSLAB commented Jan 2, 2024 •

edited by carl-baillargeon

Loading

github-actions bot commented Jan 24, 2024

carl-baillargeon left a comment

github-actions bot commented Jan 22, 2025

codspeed-hq bot commented Jan 22, 2025 •

edited

Loading

sonarqubecloud bot commented Jan 30, 2025

carl-baillargeon Feb 2, 2025

carl-baillargeon Feb 2, 2025

carl-baillargeon Feb 2, 2025

carl-baillargeon Feb 2, 2025

carl-baillargeon Feb 2, 2025

carl-baillargeon Feb 2, 2025

carl-baillargeon Feb 2, 2025

carl-baillargeon Feb 2, 2025

carl-baillargeon Feb 2, 2025

carl-baillargeon Feb 2, 2025

	route_paths = bgp_route_entry["bgpRoutePaths"]
	route_paths = iter(bgp_route_entry["bgpRoutePaths"])

	bgp_nexthops = [head.get("nextHop")]
	bgp_nexthops = {head["nextHop"]}

feat(anta.tests): Adding the test case to verify BGP ECMP paths #507

Are you sure you want to change the base?

feat(anta.tests): Adding the test case to verify BGP ECMP paths #507

Conversation

MaheshGSLAB commented Jan 2, 2024 • edited by carl-baillargeon Loading

Description

Checklist:

github-actions bot commented Jan 24, 2024

carl-baillargeon left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 22, 2025

codspeed-hq bot commented Jan 22, 2025 • edited Loading

Merging #507 will not alter performance

Summary

sonarqubecloud bot commented Jan 30, 2025

Quality Gate passed

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaheshGSLAB commented Jan 2, 2024 •

edited by carl-baillargeon

Loading

codspeed-hq bot commented Jan 22, 2025 •

edited

Loading