feat(anta): Added the test case to verify BGP NLRI prefixes #792

vitthalmagadum · 2024-08-19T05:35:53Z

Description

Verifies BGP IPv4 peer(s) consistency of NLRIs received and accepted in a BGP session.

Expected Results
----------------
* Success: The test will pass if the `nlrisReceived` equals `nlrisAccepted`, indicating that all received NLRIs were accepted..
* Failure: The test will fail if the `nlrisReceived` is not equal to `nlrisAccepted`, indicating that some NLRIs were rejected or filtered out.

Fixes #786

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)

MaheshGSLAB · 2024-08-20T09:51:59Z

anta/tests/routing/bgp.py

+    * Success: The test will pass if the `nlrisReceived` equals `nlrisAccepted`, indicating that all received NLRIs were accepted..
+    * Failure: The test will fail if the `nlrisReceived` is not equal to `nlrisAccepted`, indicating that some NLRIs were rejected or filtered out.


Suggested change

* Success: The test will pass if the `nlrisReceived` equals `nlrisAccepted`, indicating that all received NLRIs were accepted..

* Failure: The test will fail if the `nlrisReceived` is not equal to `nlrisAccepted`, indicating that some NLRIs were rejected or filtered out.

* Success: The test will pass if the `NLRI Rcd` equals `NLRI Acc`, indicating that all received NLRIs were accepted.

* Failure: The test will fail if the `NLRI Rcd` is not equal to `NLRI Acc`, indicating that some NLRIs were rejected or filtered out.

MaheshGSLAB · 2024-08-20T09:56:08Z

anta/tests/routing/bgp.py

+                continue
+
+            # Verifies the NLRIs received is equal to accepted.
+            if (nlri_rec := get_value(peer_details, "ipv4Unicast.nlrisReceived")) != (nlri_acc := get_value(peer_details, "ipv4Unicast.nlrisAccepted")):


The ipv4Unicast is not static we can have different AFI.

wan1-dc1(config-router-bgp)#show bgp summary BGP summary information for VRF default Router identifier 10.100.254.1, local AS number 65000 Neighbor AS Session State AFI/SAFI AFI/SAFI State NLRI Rcd NLRI Acc ---------- ----------- ------------- ----------------------- -------------- ---------- ---------- 10.255.0.1 65000 Established IPv4 SR-TE Negotiated 8 8 10.255.0.1 65000 Established IPv4 Dps Negotiated 9 9 10.255.0.1 65000 Established L2VPN EVPN Negotiated 52 52 10.255.0.1 65000 Established Link State Link State Negotiated 0 0 10.255.0.2 65000 Established IPv4 SR-TE Negotiated 8 8 10.255.0.2 65000 Established IPv4 Dps Negotiated 9 9 10.255.0.2 65000 Established L2VPN EVPN Negotiated 52 52 10.255.0.2 65000 Established Link State Link State Negotiated 0 0

Added support for AFI and SAFI

MaheshGSLAB · 2024-08-20T09:57:17Z

anta/tests/routing/bgp.py

+            # Verifies the NLRIs received is equal to accepted.
+            if (nlri_rec := get_value(peer_details, "ipv4Unicast.nlrisReceived")) != (nlri_acc := get_value(peer_details, "ipv4Unicast.nlrisAccepted")):
+                failures[peer_address] = {
+                    vrf: f"The NLRIs received and accepted should be consistent, but found NLRI received `{nlri_rec}` and NLRI accepted `{nlri_acc}` instead."


As testcase description is clear what the test is so having The NLRIs received and accepted should be consistent, but found line for each peer is not ok as you are already mentioning it in self.result.is_failure

So just NLRI received {nlri_rec} and NLRI accepted {nlri_acc} is enough or {"NLRI received" "{nlri_rec}, "NLRI accepted": {nlri_acc}}

MaheshGSLAB · 2024-09-02T06:32:14Z

anta/tests/routing/bgp.py

+
+    Expected Results
+    ----------------
+    * Success: The test will pass if the `prefixReceived` equals `prefixAccepted`, indicating that all received prefix(s) were accepted.


prefixReceived and prefixAccepted seems to be json keys, please use text heading of command output in doc string and failure message to for ease of understanding

MaheshGSLAB · 2024-09-02T06:36:40Z

anta/tests/routing/bgp.py

+            command_output = command.json_output
+            afi = command.params.afi
+            safi = command.params.safi if hasattr(command.params, "safi") else None
+            afi_vrf = command.params.vrf if hasattr(command.params, "vrf") else "default"


default is the default vrf then i dont think we need any condition to parse the vrf. Same for safi

MaheshGSLAB · 2024-09-02T06:46:05Z

anta/tests/routing/bgp.py

+      bgp:
+        - VerifyBGPPeerPrefixes:
+            address_families:
+              - afi: ipv4


I believe we have to check for specific peer for specific AFI/SAFI. Please recheck.

Updated. Thanks!

MaheshGSLAB · 2024-09-06T10:03:53Z

anta/tests/routing/bgp.py

+
+    # Update the peer details as per input peer addresses for verification.
+    if bgp_peers:
+        details: dict[Any, Any] = {}


Please use the more meaning full variables.

MaheshGSLAB · 2024-09-06T10:08:36Z

anta/tests/routing/bgp.py

+        if (prefix_acc and prefix_rcv) == "Not Found":
+            failure[peer] = {"prefix received": prefix_rcv, "prefix accepted": prefix_acc}
+            continue
+        if prefix_rcv != prefix_acc:
+            failure[peer] = {"prefix received": prefix_rcv, "prefix accepted": prefix_acc}


Suggested change

if (prefix_acc and prefix_rcv) == "Not Found":

failure[peer] = {"prefix received": prefix_rcv, "prefix accepted": prefix_acc}

continue

if prefix_rcv != prefix_acc:

failure[peer] = {"prefix received": prefix_rcv, "prefix accepted": prefix_acc}

if (prefix_acc and prefix_rcv) == "Not Found" or prefix_rcv != prefix_acc:

failure[peer] = {"prefix received": prefix_rcv, "prefix accepted": prefix_acc}

MaheshGSLAB · 2024-09-06T10:09:45Z

anta/tests/routing/bgp.py

+                continue
+
+            # Verify the received and accepted prefix(s).
+            failure_logs = _get_inconsistent_peers(peer_details, peers)


please separate out the variable name by input or actual peers

Updated. Thanks!!

github-actions · 2024-09-17T15:59:13Z

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

github-actions · 2024-09-23T11:32:14Z

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

MaheshGSLAB · 2024-09-30T04:55:11Z

anta/tests/routing/bgp.py

+    """Identify BGP peers with inconsistency of prefix(s) received and accepted in a BGP session.
+
+    bgp_peers: list of IPv4 address of a BGP peer to be verified. If not provided, test will verifies all the BGP peers.
+
+    Parameters
+    ----------
+        peer_details: The BGP peer data dictionary.
+        bgp_peers: The list of IPv4 address of a BGP peer(s) to be verified.


Please correct the doc string format.

MaheshGSLAB · 2024-09-30T04:58:16Z

examples/tests.yaml

@@ -678,4 +691,4 @@ anta.tests.routing:
        - endpoint: 1.0.0.14/32
          vias:
            - type: ip
-              nexthop: 1.1.1.1
+              nexthop: 1.1.1.1


please add an empty line at the end

codspeed-hq · 2024-10-01T04:02:35Z

CodSpeed Performance Report

Merging #792 will not alter performance

_{Comparing vitthalmagadum:issue_786 (7594a4c) with main (fc644a1)}

Summary

✅ 4 untouched benchmarks

MaheshGSLAB · 2024-10-04T09:20:31Z

tests/units/anta_tests/routing/test_bgp.py

+            "result": "failure",
+            "messages": [
+                "The following BGP address family(s), peers are not configured or prefix(s) received and accepted are not consistent:\n"
+                "{'ipv4': {'unicast': {'10.100.0.8': {'prefix received': 18, 'prefix accepted': 17}, '10.100.0.10': {'prefix received': 20, 'prefix accepted': 15}},"


please add the information of vrf in failure log

sonarcloud · 2024-10-04T15:03:02Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

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

See analysis details on SonarCloud

vitthalmagadum marked this pull request as draft August 19, 2024 05:36

github-actions bot assigned vitthalmagadum Aug 19, 2024

MaheshGSLAB reviewed Aug 20, 2024

View reviewed changes

vitthalmagadum force-pushed the issue_786 branch from f50d10e to 1b98a8b Compare August 27, 2024 10:51

vitthalmagadum marked this pull request as ready for review August 29, 2024 13:29

vitthalmagadum requested a review from MaheshGSLAB August 30, 2024 10:10

MaheshGSLAB reviewed Sep 2, 2024

View reviewed changes

vitthalmagadum force-pushed the issue_786 branch from c2f80c6 to de886f7 Compare September 4, 2024 05:38

vitthalmagadum requested a review from MaheshGSLAB September 4, 2024 09:08

MaheshGSLAB reviewed Sep 6, 2024

View reviewed changes

VitthalMagadum added 4 commits September 11, 2024 23:53

issue_786 Added TC for BGP NLRIs

7f3b715

issue_786 handling review comment: added support for AFI

9902512

issue_786 Handling review comments: updated specific peer check

4c8a17e

issue_786 Handling review comments: updated helper function

d8439a9

vitthalmagadum force-pushed the issue_786 branch from de886f7 to d8439a9 Compare September 12, 2024 04:15

vitthalmagadum requested a review from MaheshGSLAB September 12, 2024 04:16

github-actions bot added the state: conflict label Sep 17, 2024

Merge branch 'main' into issue_786

92fc165

github-actions bot removed the state: conflict label Sep 23, 2024

MaheshGSLAB reviewed Sep 30, 2024

View reviewed changes

VitthalMagadum added 2 commits September 30, 2024 23:33

Merge branch 'main' into issue_786

202f508

issue_786 Handling review comments: updated tests.yaml

2545e65

vitthalmagadum force-pushed the issue_786 branch from 5d11dce to 2545e65 Compare October 1, 2024 03:52

VitthalMagadum added 2 commits October 1, 2024 00:27

issue_786 fixed the sonarlint issues

2d8ee8d

issue_786 Handling review comments: updated conditional statement

7d567ab

vitthalmagadum requested a review from MaheshGSLAB October 1, 2024 10:16

MaheshGSLAB reviewed Oct 4, 2024

View reviewed changes

Merge branch 'main' into issue_786

7594a4c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(anta): Added the test case to verify BGP NLRI prefixes #792

feat(anta): Added the test case to verify BGP NLRI prefixes #792

vitthalmagadum commented Aug 19, 2024

MaheshGSLAB Aug 20, 2024

vitthalmagadum Aug 30, 2024

MaheshGSLAB Aug 20, 2024

vitthalmagadum Aug 30, 2024

MaheshGSLAB Aug 20, 2024

vitthalmagadum Aug 30, 2024

MaheshGSLAB Sep 2, 2024

vitthalmagadum Sep 4, 2024

MaheshGSLAB Sep 2, 2024

vitthalmagadum Sep 4, 2024

MaheshGSLAB Sep 2, 2024

vitthalmagadum Sep 4, 2024

MaheshGSLAB Sep 6, 2024

vitthalmagadum Sep 12, 2024

MaheshGSLAB Sep 6, 2024

vitthalmagadum Sep 12, 2024

MaheshGSLAB Sep 6, 2024

vitthalmagadum Sep 12, 2024

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 23, 2024

MaheshGSLAB Sep 30, 2024

MaheshGSLAB Sep 30, 2024

codspeed-hq bot commented Oct 1, 2024 •

edited

Loading

MaheshGSLAB Oct 4, 2024

sonarcloud bot commented Oct 4, 2024

		* Success: The test will pass if the `nlrisReceived` equals `nlrisAccepted`, indicating that all received NLRIs were accepted..
		* Failure: The test will fail if the `nlrisReceived` is not equal to `nlrisAccepted`, indicating that some NLRIs were rejected or filtered out.

feat(anta): Added the test case to verify BGP NLRI prefixes #792

Are you sure you want to change the base?

feat(anta): Added the test case to verify BGP NLRI prefixes #792

Conversation

vitthalmagadum commented Aug 19, 2024

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

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

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codspeed-hq bot commented Oct 1, 2024 • edited Loading

Merging #792 will not alter performance

Summary

Choose a reason for hiding this comment

sonarcloud bot commented Oct 4, 2024

Quality Gate passed

codspeed-hq bot commented Oct 1, 2024 •

edited

Loading