Feat(eos_designs): Add option to disable default 'redistribute connected' in VRF. #4220

laxmikantchintakindi · 2024-07-16T12:38:46Z

Change Summary

Add option to disable default 'redistribute connected ' in VRF.

Related Issue(s)

Component(s) name

arista.avd.eos_designs

Proposed changes

Add option to disable default 'redistribute connected ' in VRF. MPLS topology . Customer does not want to see connected routes in RT.

How to test

Checklist

User Checklist

Repository Checklist

My code has been rebased from devel before I start
I have read the CONTRIBUTING document.
My change requires a change to the documentation and documentation have been updated accordingly.
I have updated molecule CI testing accordingly. (check the box if not applicable)

github-actions · 2024-07-16T12:38:58Z

Review docs on Read the Docs

To test this pull request:

# Create virtual environment for this testing below the current directory
python -m venv test-avd-pr-4220
# Activate the virtual environment
source test-avd-pr-4220/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/laxmikantchintakindi/avd.git@issue4091#subdirectory=python-avd" --force
# Install Ansible collection
ansible-galaxy collection install git+https://github.com/laxmikantchintakindi/avd.git#/ansible_collections/arista/avd/,issue4091 --force
# Optional: Install AVD examples
cd test-avd-pr-4220
ansible-playbook arista.avd.install_examples

python-avd/pyavd/_eos_designs/structured_config/network_services/router_bgp.py

python-avd/pyavd/_eos_designs/structured_config/network_services/router_ospf.py

python-avd/pyavd/_eos_designs/schema/schema_fragments/defs_network_services.schema.yml

python-avd/pyavd/_eos_designs/structured_config/network_services/router_ospf.py

python-avd/pyavd/_eos_designs/structured_config/network_services/router_bgp.py

python-avd/pyavd/_eos_designs/schema/schema_fragments/defs_network_services.schema.yml

ClausHolbechArista · 2024-07-26T11:37:55Z

python-avd/pyavd/_eos_designs/structured_config/network_services/router_bgp.py

+        if get(vrf, "redistribute_connected", default=True):
+            if not self._mlag_ibgp_peering_redistribute(vrf, tenant):
+                bgp_vrf["redistribute_routes"][0]["route_map"] = "RM-CONN-2-BGP-VRFS"


Guillaume meant to move the check for get(vrf, "redistribute_connected", default=True) into the method def _mlag_ibgp_peering_redistribute(...) So anything else using this method will honor the same.

Note that you must return True from that method in the case redistribute-connected is set to false since the check here has the NOT. Maybe we should reverse the logic and rename the method to _exclude_mlag_ibgp_peering_from_redistribute. Please analyze the logic and search for other uses of the method to see that you retain the current behavior.

There is a bug in the new logic. If you try to set the new value to false on the VRF Tenant_A_WEB_ZONE you should see an error around the line mentioned above.

_exclude_mlag_ibgp_peering_redistribute was returning True when redistribute_connected was false. Now changed it to return false. This is not giving the error for Tenant_A_WEB_ZONE when redistribute_connected is set to false.

Added a new vrf in network services to test this condition.

python-avd/pyavd/_eos_designs/structured_config/network_services/router_bgp.py

github-actions · 2024-08-05T06:30:53Z

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

github-actions · 2024-08-05T13:49:01Z

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

ClausHolbechArista · 2024-08-12T07:09:28Z

python-avd/pyavd/_eos_designs/structured_config/network_services/utils.py

@@ -162,15 +162,18 @@ def _mlag_ibgp_peering_vlan_vrf(self: AvdStructuredConfigNetworkServices, vrf: d

        return vlan_id

-    def _mlag_ibgp_peering_redistribute(self: AvdStructuredConfigNetworkServices, vrf: dict, tenant: dict) -> bool:
+    def _exclude_mlag_ibgp_peering_from_redistribute(self: AvdStructuredConfigNetworkServices, vrf: dict, tenant: dict) -> bool:
        """
        Returns True if MLAG IBGP Peering subnet should be redistributed for the given vrf/tenant.


Suggested change

Returns True if MLAG IBGP Peering subnet should be redistributed for the given vrf/tenant.

Returns True if MLAG IBGP Peering subnet should be _excluded_ from redistribution for the given vrf/tenant.

Corrected the docstring.

ClausHolbechArista · 2024-08-12T07:16:19Z

python-avd/pyavd/_eos_designs/structured_config/network_services/router_bgp.py

+        if get(vrf, "redistribute_connected", default=True):
+            if not self._mlag_ibgp_peering_redistribute(vrf, tenant):
+                bgp_vrf["redistribute_routes"][0]["route_map"] = "RM-CONN-2-BGP-VRFS"


There is a bug in the new logic. If you try to set the new value to false on the VRF Tenant_A_WEB_ZONE you should see an error around the line mentioned above.

…ted ' in VRF.

for more information, see https://pre-commit.ci

… for issue in code.

gmuloc · 2024-08-28T07:18:24Z

python-avd/pyavd/_eos_designs/schema/schema_fragments/defs_network_services.schema.yml

+              redistribute_connected:
+                type: bool
+                default: true
+                description: Non-selectively enabling or disabling redistribution of connected routes to BGP inside the VRF.


Suggested change

description: Non-selectively enabling or disabling redistribution of connected routes to BGP inside the VRF.

description: Enable or disable the redistribution of all connected routes to BGP in the VRF.

Added this in latest commit.

gmuloc · 2024-08-28T07:19:06Z

python-avd/pyavd/_eos_designs/schema/schema_fragments/defs_network_services.schema.yml

@@ -892,7 +892,11 @@ $defs:
                        type: str
              redistribute_static:
                type: bool
-                description: Non-selectively enabling or disabling redistribute static inside the VRF.
+                description: Non-selectively enabling or disabling redistribute static to BGP inside the VRF.


Suggested change

description: Non-selectively enabling or disabling redistribute static to BGP inside the VRF.

description: Enable or disable the redistribution of all static routes to BGP in the VRF.

Added this in latest commit.

sonarcloud · 2024-08-29T08:34:40Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

github-actions bot assigned laxmikantchintakindi Jul 16, 2024

laxmikantchintakindi changed the title ~~Feat(eos_designs): Add option to disable default 'redistribute connected ' in VRF.~~ Feat(eos_designs): Add option to disable default 'redistribute connected' in VRF. Jul 16, 2024

github-actions bot added state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated role: eos_designs issue related to eos_designs role labels Jul 16, 2024

Shivani-gslab reviewed Jul 18, 2024

View reviewed changes

python-avd/pyavd/_eos_designs/structured_config/network_services/router_bgp.py Outdated Show resolved Hide resolved

Shivani-gslab reviewed Jul 18, 2024

View reviewed changes

python-avd/pyavd/_eos_designs/structured_config/network_services/router_ospf.py Outdated Show resolved Hide resolved

MaheshGSLAB reviewed Jul 22, 2024

View reviewed changes

python-avd/pyavd/_eos_designs/structured_config/network_services/router_ospf.py Outdated Show resolved Hide resolved

laxmikantchintakindi marked this pull request as ready for review July 25, 2024 07:00

laxmikantchintakindi requested review from a team as code owners July 25, 2024 07:00

ClausHolbechArista requested changes Jul 26, 2024

View reviewed changes

laxmikantchintakindi force-pushed the issue4091 branch from f0b1eb7 to d8650dc Compare July 26, 2024 07:02

ClausHolbechArista approved these changes Jul 26, 2024

View reviewed changes

ClausHolbechArista added the one approval This PR has one approval and is only missing one more. label Jul 26, 2024

ClausHolbechArista requested a review from a team July 26, 2024 07:05

gmuloc added this to the v4.10.0 milestone Jul 26, 2024

gmuloc requested changes Jul 26, 2024

View reviewed changes

ClausHolbechArista requested changes Jul 26, 2024

View reviewed changes

ClausHolbechArista removed this from the v4.10.0 milestone Jul 26, 2024

laxmikantchintakindi commented Jul 26, 2024

View reviewed changes

python-avd/pyavd/_eos_designs/structured_config/network_services/router_bgp.py Outdated Show resolved Hide resolved

laxmikantchintakindi marked this pull request as draft July 26, 2024 12:23

ClausHolbechArista removed the one approval This PR has one approval and is only missing one more. label Jul 29, 2024

github-actions bot added type: documentation Improvements or additions to documentation role: eos_cli_config_gen issue related to eos_cli_config_gen role labels Jul 29, 2024

laxmikantchintakindi force-pushed the issue4091 branch from 1bb67dd to d912e44 Compare July 29, 2024 09:19

github-actions bot removed type: documentation Improvements or additions to documentation role: eos_cli_config_gen issue related to eos_cli_config_gen role labels Jul 29, 2024

laxmikantchintakindi force-pushed the issue4091 branch from d912e44 to 1f68379 Compare July 29, 2024 09:21

github-actions bot added the state: conflict PR with conflict label Aug 5, 2024

laxmikantchintakindi force-pushed the issue4091 branch from 0ab2b39 to 270010e Compare August 5, 2024 13:48

github-actions bot removed the state: conflict PR with conflict label Aug 5, 2024

ClausHolbechArista requested changes Aug 12, 2024

View reviewed changes

Laxmikant Chintakindi and others added 10 commits August 12, 2024 12:52

Feat(eos_designs): Add option to disable default 'redistribute connec…

c986fa5

…ted ' in VRF.

Address comments.

4370a25

[pre-commit.ci] auto fixes from pre-commit.com hooks

8fbd252

for more information, see https://pre-commit.ci

Applying suggestion.

53522af

Reverting the change for redistribute to ospf.

5185f0d

Update as per the comment.

fa2e84b

Fix.

97e424d

fix CI

37c09e9

[pre-commit.ci] auto fixes from pre-commit.com hooks

deec2d5

for more information, see https://pre-commit.ci

fix CI,

fd3406a

laxmikantchintakindi force-pushed the issue4091 branch from 1902e7f to fd3406a Compare August 12, 2024 07:23

laxmikantchintakindi marked this pull request as draft August 12, 2024 07:26

Laxmikant Chintakindi added 3 commits August 12, 2024 16:52

Removing changes from past tests, added new tests and address comment…

1229f5a

… for issue in code.

Update docstring.

4cc4baf

Fix CI.

57f59d1

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

MaheshGSLAB approved these changes Aug 14, 2024

View reviewed changes

laxmikantchintakindi requested a review from ClausHolbechArista August 21, 2024 06:36

ClausHolbechArista and others added 3 commits August 28, 2024 07:27

Merge branch 'devel' into issue4091

912c229

Merge branch 'devel' into issue4091

664e8f4

Fix CI.

1e83c5f

gmuloc reviewed Aug 28, 2024

View reviewed changes

Address comment.

5e1d83f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat(eos_designs): Add option to disable default 'redistribute connected' in VRF. #4220

Feat(eos_designs): Add option to disable default 'redistribute connected' in VRF. #4220

laxmikantchintakindi commented Jul 16, 2024 •

edited

Loading

github-actions bot commented Jul 16, 2024

ClausHolbechArista Jul 26, 2024

ClausHolbechArista Jul 26, 2024

laxmikantchintakindi Jul 29, 2024

ClausHolbechArista Aug 12, 2024

laxmikantchintakindi Aug 13, 2024

laxmikantchintakindi Aug 13, 2024

laxmikantchintakindi Aug 13, 2024

github-actions bot commented Aug 5, 2024

github-actions bot commented Aug 5, 2024

ClausHolbechArista Aug 12, 2024

laxmikantchintakindi Aug 12, 2024

ClausHolbechArista Aug 12, 2024

gmuloc Aug 28, 2024

laxmikantchintakindi Aug 29, 2024

gmuloc Aug 28, 2024

laxmikantchintakindi Aug 29, 2024

sonarcloud bot commented Aug 29, 2024

	Returns True if MLAG IBGP Peering subnet should be redistributed for the given vrf/tenant.
	Returns True if MLAG IBGP Peering subnet should be _excluded_ from redistribution for the given vrf/tenant.

	description: Non-selectively enabling or disabling redistribution of connected routes to BGP inside the VRF.
	description: Enable or disable the redistribution of all connected routes to BGP in the VRF.

Feat(eos_designs): Add option to disable default 'redistribute connected' in VRF. #4220

Are you sure you want to change the base?

Feat(eos_designs): Add option to disable default 'redistribute connected' in VRF. #4220

Conversation

laxmikantchintakindi commented Jul 16, 2024 • edited Loading

Change Summary

Related Issue(s)

Component(s) name

Proposed changes

How to test

Checklist

User Checklist

Repository Checklist

github-actions bot commented Jul 16, 2024

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 Aug 5, 2024

github-actions bot commented Aug 5, 2024

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

sonarcloud bot commented Aug 29, 2024

Quality Gate passed

laxmikantchintakindi commented Jul 16, 2024 •

edited

Loading