Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor(eos_designs): Dedicated MLAG peer group for overlay tenants #4881

Open
wants to merge 6 commits into
base: devel
Choose a base branch
from

Conversation

Vibhu-gslab
Copy link
Contributor

Change Summary

Currently MLAG-IPv4-UNDERLAY-PEER is used for the iBGP session between MLAG pairs in the underlay and overlay. This approach can be misleading due to the use of UNDERLAY word in the BGP peer group name.

The following is an example of the configuration that AVD generates today

router bgp xxxx 
   neighbor MLAG-IPv4-UNDERLAY-PEER peer group
   neighbor MLAG-IPv4-UNDERLAY-PEER remote-as 65000.1
   neighbor MLAG-IPv4-UNDERLAY-PEER next-hop-self
   neighbor MLAG-IPv4-UNDERLAY-PEER description DC1-CL2
   neighbor MLAG-IPv4-UNDERLAY-PEER password 7 xxxx
   neighbor MLAG-IPv4-UNDERLAY-PEER send-community
   neighbor MLAG-IPv4-UNDERLAY-PEER maximum-routes 12000
   neighbor MLAG-IPv4-UNDERLAY-PEER route-map RM-MLAG-PEER-IN in

   address-family ipv4
      neighbor MLAG-IPv4-UNDERLAY-PEER activate

   vrf BLUE
      rd 192.168.100.1:1002
      route-target import evpn 1002:1002
      route-target export evpn 1002:1002
      router-id 192.168.100.1
      neighbor 172.16.1.1 peer group MLAG-IPv4-UNDERLAY-PEER
      redistribute connected

Related Issue(s)

Fixes #4826

Component(s) name

arista.avd.eos_designs

Proposed changes

Plan for this issue to be non-breaking:
1/ introduce a new knob to use_separate_mlag_peer_group_for_overlay: with a default of false.
2/ add a peer_group in bgp_peer_groups for MLAG overlay to be used when the toggle from 1/ is true

How to test

Molecule testing

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)

Copy link

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-4881
# Activate the virtual environment
source test-avd-pr-4881/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/Vibhu-gslab/avd.git@mlag_peer_group_4826#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/Vibhu-gslab/avd.git#/ansible_collections/arista/avd/,mlag_peer_group_4826 --force
# Optional: Install AVD examples
cd test-avd-pr-4881
ansible-playbook arista.avd.install_examples

@github-actions 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 Jan 15, 2025
Comment on lines 309 to 313
if (
self.inputs.bgp_peer_groups.use_separate_mlag_peer_group_for_overlay
and self.inputs.bgp_peer_groups.mlag_ipv4_underlay_peer.name == "MLAG-IPv4-UNDERLAY-PEER"
):
peer_group_name = "MLAG-IPv4-OVERLAY-PEER"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should not be changed here.
instead when this: self.inputs.bgp_peer_groups.use_separate_mlag_peer_group_for_overlay is true, the peer group used for MLAG overlay should be the one under bgp_peer_groups.mlag_ipv4_overlay_peer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added!

@github-actions github-actions bot added state: CI Updated CI scenario have been updated in the PR and removed state: CI Updated CI scenario have been updated in the PR labels Jan 16, 2025
@Vibhu-gslab Vibhu-gslab marked this pull request as ready for review January 17, 2025 11:24
@Vibhu-gslab Vibhu-gslab requested review from a team as code owners January 17, 2025 11:24
@Vibhu-gslab Vibhu-gslab requested a review from gmuloc January 17, 2025 11:24
@@ -29,6 +29,7 @@ node_type_keys:
default_evpn_encapsulation: mpls

bgp_peer_groups:
use_separate_mlag_peer_group_for_overlay: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a test testing the mlag_ipv4_overlay_peer as well

@@ -135,6 +142,7 @@ router bgp 65001
address-family ipv4
no neighbor EVPN-OVERLAY-PEERS activate
neighbor IPv4-UNDERLAY-PEERS activate
neighbor MLAG-IPv4-OVERLAY-PEER activate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should not be rendered.

type: bool
default: false
description: Configure a different BGP peer group for MLAG for underlay and overlay.
mlag_ipv4_overlay_peer:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mlag_ipv4_overlay_peer:
mlag_ipv4_vrfs_peer:

rename the PR

Comment on lines +37 to +40
use_separate_mlag_peer_group_for_overlay:
type: bool
default: false
description: Configure a different BGP peer group for MLAG for underlay and overlay.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after discussion let's remove this boolean and rely on the name of mlag_ipv4_vrfs_peer being set different from the mlag_ipv4_underlay_peer

keys:
name:
type: str
default: MLAG-IPv4-OVERLAY-PEER
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's remove this default value and in the code:
1/ default it to the value of mlag_ipv4_underlay_peer.name
2/ the code will need to check whether mlag_ipv4_underlay_peer.name and mlag_ipv4_vrfs_peer.name are equal or not.

Create a shared_utils to get the name based on thta
if the names are the same (either because default or being set to the same name as underlay one), then the password, bfd, .. under this key will be ignored. Make a note of this behavior in the description of the key.

Comment on lines +186 to +196
# To configure a different BGP peer group for mlag_ipv4_overlay_peer
if self.inputs.bgp_peer_groups.use_separate_mlag_peer_group_for_overlay is True:
peer_groups.append(
{
**self._generate_base_peer_group("ipv4", "mlag_ipv4_overlay_peer"),
"remote_as": self.shared_utils.bgp_as,
"next_hop_self": True,
"maximum_routes": 12000,
"route_map_in": "RM-MLAG-PEER-IN",
},
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be in MLAG module

Comment on lines +229 to +231
if self.inputs.bgp_peer_groups.use_separate_mlag_peer_group_for_overlay is True:
peer_groups.append({"name": self.inputs.bgp_peer_groups.mlag_ipv4_overlay_peer.name, "activate": True})

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be removed

Comment on lines +340 to +342
peer_group_name = self.inputs.bgp_peer_groups.mlag_ipv4_underlay_peer.name
if self.inputs.bgp_peer_groups.use_separate_mlag_peer_group_for_overlay:
peer_group_name = self.inputs.bgp_peer_groups.mlag_ipv4_overlay_peer.name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create a shared_utils to get this name with the logic defined in the schema comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
role: eos_designs issue related to eos_designs role state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dedicated MLAG peer group for overlay tenants
2 participants