Feat(eos_designs): l3_edge with rfc5549 underlay and next-hop address-family ipv6 #4491

nathanmusser · 2024-09-18T17:46:51Z

Change Summary

When routing_protocol is set to ebgp and the underlay is set to rfc5549 we now override the next-hop address-family ipv6 directive the neighbor inherits from the underlay peer-group.

Related Issue(s)

Partially fixes #4379

Component(s) name

arista.avd.eos_designs

Proposed changes

Override the next-hop address-family ipv6 directive the neighbor inherits from the underlay peer-group.

How to test

Checklist

User Checklist

N/A

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-09-18T17:47:05Z

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-4491
# Activate the virtual environment
source test-avd-pr-4491/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/nathanmusser/avd.git@issue4379_p2p_links#subdirectory=python-avd" --force
# Point Ansible collections path to the Python virtual environment
export ANSIBLE_COLLECTIONS_PATH=$VIRTUAL_ENV/ansible_collections
# Install Ansible collection
ansible-galaxy collection install git+https://github.com/nathanmusser/avd.git#/ansible_collections/arista/avd/,issue4379_p2p_links --force
# Optional: Install AVD examples
cd test-avd-pr-4491
ansible-playbook arista.avd.install_examples

ClausHolbechArista

Please update the description for the routing_protocol key in the schema fragment to explain the new behavior.

ClausHolbechArista · 2024-09-25T11:03:48Z

Also please add a test in molecule for this scenario. It should be enough to add another interface to one of the existing tests. I suggest ansible_collections/arista/avd/molecule/eos_designs_unit_tests/inventory/host_vars/l3_edge_bgp.yml. To update the outputs (and verify that is is doing what you expect) run

cd ansible_collections/arista/avd
molecule converge -s eos_designs_unit_tests -- --limit l3_edge_bgp

nathanmusser · 2024-10-09T01:16:43Z

Started a refactor of this after a discussion with @ClausHolbechArista . New iteration is such that when the underlay is set to RFC5549 and routing_protocol is set to ebgp we explicitly override the next-hop ipv6 command for that specific neighbor (since it's part of the underlay peer-group that gets it due to enabling RFC5549).

eos_cli_config_gen had the next-hop ipv6 knob for peer groups in router_bgp.address_family_ipv4 and for neighbors in the router_bgp.vrfs[].address_family_ipv4 path but did not have it exposed for neighbors for the default vrf router_bgp.address_family_ipv4. Since that's what we needed that key(s) had to be added.

I haven't done extensive testing of the latest commit yet, just verified it did what I wanted against a prod fabric. Need to finish testing the various edge cases and make sure it's implemented correctly. Need to update molecule, probably need to flesh out the documentation to correctly describe the eos_cli_config_gen keys as well as the behavior in eos_designs.

Ultimately I still want to explore having the ability to override the peer group for l3_edge neighbors, but not sure I want to tackle that here.

ClausHolbechArista · 2024-10-09T07:13:26Z

Thanks Nathan

Ultimately I still want to explore having the ability to override the peer group for l3_edge neighbors, but not sure I want to tackle that here.

We would not want that capability in the l3_edge model. If you need custom peers and peer-groups, you should use the models under tenants to do it. There you can build interfaces, peers and peer-groups. With AVD 5.0 it also works well for VRF default.

nathanmusser · 2024-10-09T14:29:23Z

We would not want that capability in the l3_edge model. If you need custom peers and peer-groups, you should use the models under tenants to do it. There you can build interfaces, peers and peer-groups. With AVD 5.0 it also works well for VRF default.

Respectfully I disagree here. I started to type a novel justifying my thoughts, but I figure we can revisit after 5.0. I did successfully prototype custom peer_groups yesterday after my initial comment. I'm sure I haven't considered all the implications but it wasn't as difficult as I imagined.

In the meantime I'll keep this PR focused on the above solution, I'll update the title and description today to better reflect the new focus.

github-actions · 2024-10-10T08:59:20Z

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

…erlay" is set to False and the protocol is ebgp.

… ebgp

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

…rmation.

…commands (aristanetworks#4580)

…eos for `port-channel-interfaces` (aristanetworks#4557) Co-authored-by: Laxmikant Chintakindi <[email protected]> Co-authored-by: Claus Holbech <[email protected]> Co-authored-by: Guillaume Mulocher <[email protected]>

…ion templates (aristanetworks#4558) Co-authored-by: JulioPDX <[email protected]> Co-authored-by: Claus Holbech <[email protected]>

…gregation to match with EOS (aristanetworks#4585) Co-authored-by: Claus Holbech <[email protected]>

…et-interfaces to match with EOS (aristanetworks#4569)

github-actions · 2024-10-16T03:04:27Z

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

sonarcloud · 2024-10-16T04:10:01Z

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

nathanmusser · 2024-10-16T04:12:55Z

Added a new molecule test l3_edge_rfc5549 to validate the various interactions between rfc5549 and l3_edge. Though this edge case was present in some of the existing molecule tests so some of those artifacts have been updated to reflect this.

github-actions · 2024-10-16T08:58:30Z

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

ClausHolbechArista · 2024-10-23T15:55:11Z

...llections/arista/avd/molecule/eos_designs_unit_tests/inventory/host_vars/l3_edge_rfc5549.yml

+      include_in_underlay_protocol: false
+      routing_protocol: ebgp


Not sure why we are rendering BGP for this neighbor

ClausHolbechArista · 2024-10-23T16:42:43Z

...ections/arista/avd/molecule/evpn_underlay_rfc5549_overlay_ebgp/intended/configs/DC1-BL1A.cfg

@@ -250,6 +250,7 @@ router bgp 65104
      no neighbor EVPN-OVERLAY-PEERS activate
      neighbor UNDERLAY_PEERS activate
      neighbor UNDERLAY_PEERS next-hop address-family ipv6 originate
+      no neighbor 10.23.23.2 next-hop address-family ipv6


@kornoa you implemented this 9 months ago. If we implement this change, we will no add this extra line, but as I see it, the config would actually be more correct with this change. Do you have a running network using the rfc5549 + the ebgp knob under l3_edge without ipv6_enable?

Sorry, the customer's project had finished. I don't have access to this implementation anymore.
I think this line is redundant. An IPv4-based neighbor session with an IPv4 address-family defaults to a IPv4 next-hop within the NLRI. But anyway, it doesn't harm.

An IPv4-based neighbor session with an IPv4 address-family defaults to a IPv4 next-hop within the NLRI.

This is true, but if the neighbor is is a member of the UNDERLAY_PEERS group (which l3_edge does) then line 252 forces the advertisements to advertise with ipv6 next-hops instead. Or at least this was my experience in my testing. Hence the need for the suggested 253.

ClausHolbechArista · 2024-10-23T16:46:37Z

...ections/arista/avd/molecule/evpn_underlay_rfc5549_overlay_ebgp/intended/configs/DC1-BL1B.cfg

@@ -251,6 +251,7 @@ router bgp 65105
      no neighbor EVPN-OVERLAY-PEERS activate
      neighbor UNDERLAY_PEERS activate
      neighbor UNDERLAY_PEERS next-hop address-family ipv6 originate
+      no neighbor 10.23.23.6 next-hop address-family ipv6


@nathanmusser I think we need to keep this neighbor with the next-hop, since we have ipv6_enable. Now I understand why you asked about this a long time ago :-/

laxmikantchintakindi · 2024-10-24T08:03:14Z

python-avd/pyavd/_eos_cli_config_gen/j2templates/eos/router-bgp.j2

+{%             if neighbor.next_hop.address_family_ipv6.enabled is arista.avd.defined %}
+{%                 if neighbor.next_hop.address_family_ipv6.enabled is arista.avd.defined(true) %}
+{%                     set ipv6_originate_cli = "neighbor " ~ neighbor.ip_address ~ " next-hop address-family ipv6" %}
+{%                     if neighbor.next_hop.address_family_ipv6.originate is arista.avd.defined(true) %}
+{%                         set ipv6_originate_cli = ipv6_originate_cli ~ " originate" %}
+{%                     endif %}
+{%                 elif neighbor.next_hop.address_family_ipv6.enabled is arista.avd.defined(false) %}
+{%                     set ipv6_originate_cli = "no neighbor " ~ neighbor.ip_address ~ " next-hop address-family ipv6" %}
+{%                 endif %}
+      {{ ipv6_originate_cli }}
+{%             endif %}


Suggested change

{% if neighbor.next_hop.address_family_ipv6.enabled is arista.avd.defined %}

{% if neighbor.next_hop.address_family_ipv6.enabled is arista.avd.defined(true) %}

{% set ipv6_originate_cli = "neighbor " ~ neighbor.ip_address ~ " next-hop address-family ipv6" %}

{% if neighbor.next_hop.address_family_ipv6.originate is arista.avd.defined(true) %}

{% set ipv6_originate_cli = ipv6_originate_cli ~ " originate" %}

{% endif %}

{% elif neighbor.next_hop.address_family_ipv6.enabled is arista.avd.defined(false) %}

{% set ipv6_originate_cli = "no neighbor " ~ neighbor.ip_address ~ " next-hop address-family ipv6" %}

{% endif %}

{{ ipv6_originate_cli }}

{% endif %}

{% if neighbor.next_hop.address_family_ipv6.enabled is arista.avd.defined(true) %}

{% set ipv6_originate_cli = "neighbor " ~ neighbor.ip_address ~ " next-hop address-family ipv6" %}

{% if neighbor.next_hop.address_family_ipv6.originate is arista.avd.defined(true) %}

{% set ipv6_originate_cli = ipv6_originate_cli ~ " originate" %}

{% endif %}

{{ ipv6_originate_cli }}

{% elif neighbor.next_hop.address_family_ipv6.enabled is arista.avd.defined(false) %}

{% set ipv6_originate_cli = "no neighbor " ~ neighbor.ip_address ~ " next-hop address-family ipv6" %}

{{ ipv6_originate_cli }}

{% endif %}

ClausHolbechArista · 2024-11-11T08:12:26Z

Moving to draft until comments have been addressed.

nathanmusser · 2024-11-11T15:20:40Z

I will likely not have the cycles for this until mid/late December or January. I plan to complete at that time but just adding that for transparency here.

nathanmusser requested review from a team as code owners September 18, 2024 17:46

github-actions bot assigned nathanmusser Sep 18, 2024

ClausHolbechArista requested changes Sep 25, 2024

View reviewed changes

carlbuchmann marked this pull request as draft September 30, 2024 20:19

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

nathanmusser marked this pull request as ready for review October 3, 2024 14:10

nathanmusser requested a review from ClausHolbechArista October 3, 2024 14:11

nathanmusser marked this pull request as draft October 8, 2024 14:25

github-actions bot added the role: eos_cli_config_gen issue related to eos_cli_config_gen role label Oct 9, 2024

github-actions bot added the state: conflict PR with conflict label Oct 10, 2024

github-actions bot removed the state: CI Updated CI scenario have been updated in the PR label Oct 10, 2024

nathanmusser and others added 8 commits October 10, 2024 12:37

Added check to render neighbors without peer group if "include_in_und…

0e45c44

…erlay" is set to False and the protocol is ebgp.

Updated p2p links description

4541268

Added molecule test for l3_edge no underlay and ebgp

807bd0c

Refactored to explicity remove ipv6 next-hop when routing_protocol is…

68a69c4

… ebgp

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

753bb4c

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

Small bug fixes

2c33680

Cleaned up documentation for p2p_links rfc5549 and next-hop ipv6 info…

f70ed73

…rmation.

Removed molecule tests from previous attempt

7126f28

nathanmusser force-pushed the issue4379_p2p_links branch from 37f3bf0 to 7126f28 Compare October 10, 2024 18:05

github-actions bot removed the state: conflict PR with conflict label Oct 10, 2024

gmuloc and others added 8 commits October 15, 2024 21:35

Refactor(eos_cli_config_gen)!: Reorder hardware and hardware-counter …

f5511fd

…commands (aristanetworks#4580)

Bump: anta>=1.1.0 (aristanetworks#4586)

640b064

Doc(eos_designs): How-to guide and porting guide updates for descript…

88b1057

…ion templates (aristanetworks#4558) Co-authored-by: JulioPDX <[email protected]> Co-authored-by: Claus Holbech <[email protected]>

Refactor(eos_cli_config_gen): Rearrange the eos-cli config for tap-ag…

17b568c

…gregation to match with EOS (aristanetworks#4585) Co-authored-by: Claus Holbech <[email protected]>

Refactor(eos_cli_config_gen): Rearrange the eos-cli config for ethern…

692dd65

…et-interfaces to match with EOS (aristanetworks#4569)

Bump: Pre-release 5.0.0-rc1 (aristanetworks#4587)

5fc7031

Reversed auto-formatting on p2p_links schema

1a19e93

github-actions bot added type: documentation Improvements or additions to documentation state: CI Updated CI scenario have been updated in the PR labels Oct 16, 2024

nathanmusser and others added 2 commits October 15, 2024 22:01

Added l3_edge_rfc5549 molecule testing

55e6466

Merge branch 'devel' into issue4379_p2p_links

c3dc43b

github-actions bot removed state: conflict PR with conflict type: documentation Improvements or additions to documentation labels Oct 16, 2024

nathanmusser changed the title ~~Feat(eos_designs): Exclude l3_edge neighbors from peer_group~~ Feat(eos_designs): l3_edge with rfc5549 underlay and next-hop address-family ipv6 Oct 16, 2024

nathanmusser added 2 commits October 15, 2024 22:59

Removed artificat from rebase

23980f1

Update existing rfc5549 molecule artifacts

fe7a86d

nathanmusser marked this pull request as ready for review October 16, 2024 04:10

github-actions bot added the state: conflict PR with conflict label Oct 16, 2024

ClausHolbechArista reviewed Oct 23, 2024

View reviewed changes

laxmikantchintakindi reviewed Oct 24, 2024

View reviewed changes

ClausHolbechArista marked this pull request as draft November 11, 2024 08:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat(eos_designs): l3_edge with rfc5549 underlay and next-hop address-family ipv6 #4491

Feat(eos_designs): l3_edge with rfc5549 underlay and next-hop address-family ipv6 #4491

nathanmusser commented Sep 18, 2024 •

edited

Loading

github-actions bot commented Sep 18, 2024

ClausHolbechArista left a comment

ClausHolbechArista commented Sep 25, 2024

nathanmusser commented Oct 9, 2024

ClausHolbechArista commented Oct 9, 2024

nathanmusser commented Oct 9, 2024

github-actions bot commented Oct 10, 2024

github-actions bot commented Oct 16, 2024

sonarcloud bot commented Oct 16, 2024

nathanmusser commented Oct 16, 2024 •

edited

Loading

github-actions bot commented Oct 16, 2024

ClausHolbechArista Oct 23, 2024

ClausHolbechArista Oct 23, 2024 •

edited

Loading

kornoa Nov 11, 2024

nathanmusser Nov 11, 2024

kornoa Nov 12, 2024

ClausHolbechArista Oct 23, 2024

laxmikantchintakindi Oct 24, 2024

ClausHolbechArista commented Nov 11, 2024

nathanmusser commented Nov 11, 2024

Feat(eos_designs): l3_edge with rfc5549 underlay and next-hop address-family ipv6 #4491

Are you sure you want to change the base?

Feat(eos_designs): l3_edge with rfc5549 underlay and next-hop address-family ipv6 #4491

Conversation

nathanmusser commented Sep 18, 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 Sep 18, 2024

ClausHolbechArista left a comment

Choose a reason for hiding this comment

ClausHolbechArista commented Sep 25, 2024

nathanmusser commented Oct 9, 2024

ClausHolbechArista commented Oct 9, 2024

nathanmusser commented Oct 9, 2024

github-actions bot commented Oct 10, 2024

github-actions bot commented Oct 16, 2024

sonarcloud bot commented Oct 16, 2024

Quality Gate passed

nathanmusser commented Oct 16, 2024 • edited Loading

github-actions bot commented Oct 16, 2024

ClausHolbechArista Oct 23, 2024

Choose a reason for hiding this comment

ClausHolbechArista Oct 23, 2024 • edited Loading

Choose a reason for hiding this comment

kornoa Nov 11, 2024

Choose a reason for hiding this comment

nathanmusser Nov 11, 2024

Choose a reason for hiding this comment

kornoa Nov 12, 2024

Choose a reason for hiding this comment

ClausHolbechArista Oct 23, 2024

Choose a reason for hiding this comment

laxmikantchintakindi Oct 24, 2024

Choose a reason for hiding this comment

ClausHolbechArista commented Nov 11, 2024

nathanmusser commented Nov 11, 2024

nathanmusser commented Sep 18, 2024 •

edited

Loading

nathanmusser commented Oct 16, 2024 •

edited

Loading

ClausHolbechArista Oct 23, 2024 •

edited

Loading