feat(anta): Added testcase to verify the BGP Redistributed Routes #993

geetanjalimanegslab · 2025-01-07T05:46:43Z

Description

Added testcase to verify the BGP Redistributed Routes

  This test performs the following checks for each specified route:

  1. Ensures that the expected address-family is configured on the device.
  2. Confirms that the redistributed route protocol and route map match the expected values for a route.
     
 Expected Results
----------------
* Success: If all of the following conditions are met:
    - The expected address-family is configured on the device.
    - The redistributed route protocol and route map align with the expected values for the route.
* Failure: If any of the following occur:
    - The expected address-family is not configured on device.
    - The redistributed route protocol or route map does not match the expected value for a route.

Fixes #1009

Note - Added pylint disable for number of line check (C0302) with TODO.

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)

codspeed-hq · 2025-01-07T05:55:12Z

CodSpeed Performance Report

Merging #993 will not alter performance

_{Comparing geetanjalimanegslab:test_bgp_redistributed_routes (b4e2ba6) with main (164b736)}

Summary

✅ 22 untouched benchmarks

anta/custom_types.py

vitthalmagadum · 2025-01-13T07:35:31Z

anta/input_models/routing/bgp.py

-    """
+    Can be enabled in the `VerifyBGPPeerCount` tests."""
+    redistributed_route_protocol: Redistributed_Protocol | None = None
+    """Specify redistributed route protocol."""


Suggested change

"""Specify redistributed route protocol."""

"""Specify redistributed route protocol. Required field in the `VerifyBGPRedistributedRoutes` test."""

Also add validator in test case.

anta/input_models/routing/bgp.py

anta/tests/routing/bgp.py

github-actions · 2025-01-14T13:34:20Z

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

github-actions · 2025-01-15T07:15:52Z

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

github-actions · 2025-01-15T13:10:57Z

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

github-actions · 2025-01-17T11:53:26Z

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

carl-baillargeon

Please update the test with the suggested input models.

carl-baillargeon · 2025-01-20T18:13:21Z

anta/custom_types.py

@@ -263,3 +263,4 @@ def validate_regex(value: str) -> str:
 SnmpHashingAlgorithm = Literal["MD5", "SHA", "SHA-224", "SHA-256", "SHA-384", "SHA-512"]
 SnmpEncryptionAlgorithm = Literal["AES-128", "AES-192", "AES-256", "DES"]
 DynamicVlanSource = Literal["dmf", "dot1x", "dynvtep", "evpn", "mlag", "mlagsync", "mvpn", "swfwd", "vccbfd"]
+RedistributedProtocol = Literal["AttachedHost", "Bgp", "Connected", "Dynamic", "IS-IS", "OSPF Internal", "OSPFv3 Internal", "RIP", "Static", "User"]


We need to add OSPF External, OSPF Nssa-External, same for OSPFv3. We can also redistribute specific IS-IS levels; level-1, level-2, level-1-2 so please double check the options.

carl-baillargeon · 2025-01-20T18:16:06Z

anta/tests/routing/bgp.py

@@ -1685,3 +1688,89 @@ def test(self) -> None:

                if (actual_origin := get_value(route_path, "routeType.origin")) != origin:
                    self.result.is_failure(f"{route} {path} - Origin mismatch - Actual: {actual_origin}")
+
+
+class VerifyBGPRedistributedRoutes(AntaTest):


I would call this test VerifyBGPRedistribution instead since we are not checking anything related to redistributed routes.

carl-baillargeon · 2025-01-20T18:42:48Z

anta/input_models/routing/bgp.py

+    Can be enabled in the `VerifyBGPPeerCount` tests."""
+    redistributed_route_protocol: RedistributedProtocol | None = None
+    """Specify redistributed route protocol. Required field in the `VerifyBGPRedistributedRoutes` test."""
+    route_map: str | None = None
+    """Specify redistributed route protocol route map. Required field in the `VerifyBGPRedistributedRoutes` test."""


Let's use separate models for the show bgp instance related tests. Something like this:

class RedistributedRoute(BaseModel): proto: RedistributedProtocol # Custom type that you already created include_leaked: bool | None = None # We should also check this for protocols that it applies route_map: str | None = None # This is optional, we can redistribute without a route-map class AfiSafiConfig(BaseModel): name: Literal["v4u", "v4m"] # Here we should also support other formats like `IPv4 Unicast` or `ipv4Unicast` redistributed_routes: list[RedistributedRoute] class BgpVrf(BaseModel): name: str = "default" address_families: list[AfiSafiConfig] # With this structure we can easily add more fields in the future for other tests. # router_id: str # local_as: int

Actually AddressFamilyConfig might be more appropriate versus AfiSafiConfig

Hi @carl-baillargeon.
Thank you for providing the input model. I've implemented the test case based on that. Here's the current input model:

- VerifyBGPRedistribution: vrfs: - vrf: default address_families: - afi_safi: ipv4Unicast redistributed_routes: - proto: Connected include_leaked: True route_map: RM-CONN-2-BGP - proto: Static include_leaked: True route_map: RM-CONN-2-BGP - afi_safi: IPv6 Unicast redistributed_routes: - proto: Dynamic route_map: RM-CONN-2-BGP - proto: Static include_leaked: True route_map: RM-CONN-2-BGP

However, I'm encountering an issue with Cognitive Complexity. To resolve this, I've proposed an alternative input model approach:

- VerifyBGPRedistribution: address_families: - afi_safi: ipv4Unicast vrf: default redistributed_routes: - proto: Connected include_leaked: True route_map: RM-CONN-2-BGP - proto: Static include_leaked: True route_map: RM-CONN-2-BGP - afi_safi: IPv4 Unicast vrf: test redistributed_routes: - proto: Dynamic route_map: RM-CONN-2-BGP - proto: Static include_leaked: True route_map: RM-CONN-2-BGP - afi_safi: IPv4Unicast vrf: mgmt redistributed_routes: - proto: Dynamic route_map: RM-CONN-2-BGP - proto: Static include_leaked: True route_map: RM-CONN-2-BGP

Key Reasons for the Proposed Change

It aligns with the structure already used in other BGP tests, ensuring consistency.

It eliminates unnecessary looping and conditional logic.

It reduces code complexity, making the test case easier to maintain.

As discussed with @gmuloc we will revisit this once Carl returns and then finalize the approach, In the meantime, I’m putting this on hold.

Thanks,
Geetanjali

carl-baillargeon · 2025-01-20T18:46:33Z

anta/tests/routing/bgp.py

+                if address_family.afi not in ["ipv4", "ipv6"] or address_family.safi not in ["unicast", "multicast"]:
+                    msg = f"{address_family}; redistributed route protocol is not supported for address family `{address_family.eos_key}`"
+                    raise ValueError(msg)


This check could go in the AfiSafiConfig model directly.

github-actions · 2025-01-20T19:04:57Z

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

github-actions · 2025-01-21T11:49:28Z

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

sonarqubecloud · 2025-01-28T14:03:09Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

github-actions bot assigned geetanjalimanegslab Jan 7, 2025

geetanjalimanegslab changed the title ~~Added testcase to verify the BGP Redistributed Routes~~ feat(anta):Added testcase to verify the BGP Redistributed Routes Jan 7, 2025

geetanjalimanegslab changed the title ~~feat(anta):Added testcase to verify the BGP Redistributed Routes~~ feat(anta): Added testcase to verify the BGP Redistributed Routes Jan 7, 2025

geetanjalimanegslab requested a review from vitthalmagadum January 7, 2025 07:12

vitthalmagadum requested changes Jan 13, 2025

View reviewed changes

geetanjalimanegslab requested a review from vitthalmagadum January 14, 2025 02:29

vitthalmagadum approved these changes Jan 14, 2025

View reviewed changes

vitthalmagadum marked this pull request as ready for review January 14, 2025 08:55

vitthalmagadum requested a review from carl-baillargeon January 14, 2025 08:55

github-actions bot added the state: conflict label Jan 14, 2025

geetanjalimanegslab added 5 commits January 15, 2025 07:00

Added testcase to verify the BGP Redistributed Routes

a52988d

Removed redundant testcase and updated customtypes

9215731

Fixed pre-commit issue

ddcf15c

Updated test inputs with validator and added UT for the same.

3a4eed8

Updated field validator and UT error messages

145dae6

geetanjalimanegslab force-pushed the test_bgp_redistributed_routes branch from 16ed8ec to 145dae6 Compare January 15, 2025 07:15

github-actions bot added state: conflict and removed state: conflict labels Jan 15, 2025

Merge branch 'main' into test_bgp_redistributed_routes

e8d0f11

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

Added pylint disable for number of line check (C0302) with TODO.

688b67b

carl-baillargeon requested changes Jan 20, 2025

View reviewed changes

github-actions bot added the state: conflict label Jan 20, 2025

Merge branch 'main' into test_bgp_redistributed_routes

75f2a83

Fixed precommit issues

1a326a6

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

geetanjalimanegslab added 2 commits January 22, 2025 11:43

Updated testcase with the latest input models

e65000c

Updated testcase with docstrings and Unit testcases

f4cb68f

gmuloc added the on-hold Waiting for additional elements label Jan 24, 2025

geetanjalimanegslab requested a review from gmuloc January 24, 2025 09:40

Merge branch 'main' into test_bgp_redistributed_routes

b4e2ba6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(anta): Added testcase to verify the BGP Redistributed Routes #993

feat(anta): Added testcase to verify the BGP Redistributed Routes #993

geetanjalimanegslab commented Jan 7, 2025 •

edited

Loading

codspeed-hq bot commented Jan 7, 2025 •

edited

Loading

vitthalmagadum Jan 13, 2025

github-actions bot commented Jan 14, 2025

github-actions bot commented Jan 15, 2025

github-actions bot commented Jan 15, 2025

github-actions bot commented Jan 17, 2025

carl-baillargeon left a comment

carl-baillargeon Jan 20, 2025

carl-baillargeon Jan 20, 2025

carl-baillargeon Jan 20, 2025

carl-baillargeon Jan 20, 2025

geetanjalimanegslab Jan 24, 2025

carl-baillargeon Jan 20, 2025

github-actions bot commented Jan 20, 2025

github-actions bot commented Jan 21, 2025

sonarqubecloud bot commented Jan 28, 2025

	"""Specify redistributed route protocol."""
	"""Specify redistributed route protocol. Required field in the `VerifyBGPRedistributedRoutes` test."""

feat(anta): Added testcase to verify the BGP Redistributed Routes #993

Are you sure you want to change the base?

feat(anta): Added testcase to verify the BGP Redistributed Routes #993

Conversation

geetanjalimanegslab commented Jan 7, 2025 • edited Loading

Description

Note - Added pylint disable for number of line check (C0302) with TODO.

Checklist:

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

Merging #993 will not alter performance

Summary

Choose a reason for hiding this comment

github-actions bot commented Jan 14, 2025

github-actions bot commented Jan 15, 2025

github-actions bot commented Jan 15, 2025

github-actions bot commented Jan 17, 2025

carl-baillargeon left a 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 Jan 20, 2025

github-actions bot commented Jan 21, 2025

sonarqubecloud bot commented Jan 28, 2025

Quality Gate passed

geetanjalimanegslab commented Jan 7, 2025 •

edited

Loading

codspeed-hq bot commented Jan 7, 2025 •

edited

Loading