Refactor(eos_designs): Refactoring MLAG peer group (#6723)

Co-authored-by: Guillaume Mulocher <gmulocher@arista.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Mahesh Kumar <122076792+MaheshGSLAB@users.noreply.github.com>
Co-authored-by: Claus Holbech <holbech@arista.com>

Author: 5 people

ansible_collections/arista/avd/extensions/molecule/eos_designs_unit_tests/intended/configs/bgp-peer-groups-1.cfg

@@ -116,13 +116,6 @@ router bgp 65001
    neighbor MLAG-IPv4-UNDERLAY-PEER route-map RM-MLAG-PEER-IN in
    neighbor MLAG-IPv4-UNDERLAY-PEER send-community
    neighbor MLAG-IPv4-UNDERLAY-PEER maximum-routes 12002
-   neighbor MLAG-IPv4-VRFS-PEER peer group
-   neighbor MLAG-IPv4-VRFS-PEER remote-as 65001
-   neighbor MLAG-IPv4-VRFS-PEER next-hop-self
-   neighbor MLAG-IPv4-VRFS-PEER description Description for mlag_ipv4_vrfs_peer via structured_config
-   neighbor MLAG-IPv4-VRFS-PEER route-map RM-MLAG-PEER-IN in
-   neighbor MLAG-IPv4-VRFS-PEER send-community
-   neighbor MLAG-IPv4-VRFS-PEER maximum-routes 12003
    neighbor 192.168.252.204 peer group IPv4-UNDERLAY-PEERS
    neighbor 192.168.252.204 remote-as 65001
    neighbor 192.168.252.204 description bgp-peer-groups-3_Ethernet1
@@ -140,6 +133,5 @@ router bgp 65001
    address-family ipv4
       neighbor IPv4-UNDERLAY-PEERS activate
       neighbor MLAG-IPv4-UNDERLAY-PEER activate
-      neighbor MLAG-IPv4-VRFS-PEER activate
 !
 end

ansible_collections/arista/avd/extensions/molecule/eos_designs_unit_tests/intended/structured_configs/bgp-peer-groups-1.yml

@@ -118,15 +118,6 @@ router_bgp:
     send_community: all
     maximum_routes: 12002
     route_map_in: RM-MLAG-PEER-IN
-  - name: MLAG-IPv4-VRFS-PEER
-    metadata:
-      type: ipv4
-    remote_as: '65001'
-    description: Description for mlag_ipv4_vrfs_peer via structured_config
-    next_hop_self: true
-    send_community: all
-    maximum_routes: 12003
-    route_map_in: RM-MLAG-PEER-IN
   - name: IPv4-UNDERLAY-PEERS
     metadata:
       type: ipv4
@@ -175,8 +166,6 @@ router_bgp:
     peer_groups:
     - name: MLAG-IPv4-UNDERLAY-PEER
       activate: true
-    - name: MLAG-IPv4-VRFS-PEER
-      activate: true
     - name: IPv4-UNDERLAY-PEERS
       activate: true
 service_routing_protocols_model: multi-agent

ansible_collections/arista/avd/extensions/molecule/eos_designs_unit_tests/inventory/host_vars/bgp-peer-groups-1.yml

@@ -0,0 +1,3 @@
+avd_design_future:
+  remove_redundant_ipv4_unicast_for_peer_groups: true
+  only_configure_mlag_vrfs_peer_group_when_used: true

ansible_collections/arista/avd/roles/eos_designs/docs/tables/avd-design-future.md

@@ -26,3 +26,8 @@ keys:
         description: |-
           Raise an error if an L3 Port-Channel is configured without any member interfaces.
         default: false
+      only_configure_mlag_vrfs_peer_group_when_used:
+        type: bool
+        description: |-
+          Configure the `mlag_ipv4_vrfs_peer` BGP peer group only when needed.
+        default: false

python-avd/pyavd/_eos_designs/schema/__init__.py

@@ -7,18 +7,14 @@
 from re import findall
 from typing import TYPE_CHECKING, Protocol, cast
 
-from pyavd._eos_cli_config_gen.schema import EosCliConfigGen
 from pyavd._errors import AristaAvdInvalidInputsError, AristaAvdMissingVariableError
 from pyavd._utils import default, get_ip_from_ip_prefix
-from pyavd._utils.format_string import AvdStringFormatter
 from pyavd.j2filters import natural_sort, range_expand
 
 if TYPE_CHECKING:
     from typing import Literal
 
     from pyavd._eos_designs.eos_designs_facts.schema.protocol import EosDesignsFactsProtocol
-    from pyavd._eos_designs.schema import EosDesigns
-    from pyavd._eos_designs.structured_config.structured_config_generator import StructCfgs
 
     from . import SharedUtilsProtocol
 
@@ -235,69 +231,6 @@ def mlag_vrfs_peer_group_name(self: SharedUtilsProtocol) -> str:
             return self.inputs.bgp_peer_groups.mlag_ipv4_vrfs_peer.name
         return self.inputs.bgp_peer_groups.mlag_ipv4_underlay_peer.name
 
-    def update_router_bgp_with_mlag_peer_group(self: SharedUtilsProtocol, router_bgp: EosCliConfigGen.RouterBgp, custom_structured_configs: StructCfgs) -> None:
-        """
-        Update router_bgp structured_config covering the MLAG peer_group(s) and associated address_family activations.
-
-        Inserts custom structured configuration into the given custom_structured_configs instance.
-
-        This is called from MLAG in the case of BGP underlay routing protocol.
-        In the case of another underlay routing protocol, it may be called from network_services instead in case there are VRFs with iBGP peerings.
-        """
-        # Only create the underlay peer group if the underlay is BGP or if we reuse the same peer-group from network services.
-        if self.underlay_bgp or not self.use_separate_peer_group_for_mlag_vrfs:
-            bgp_peer_group = self.inputs.bgp_peer_groups.mlag_ipv4_underlay_peer
-            router_bgp.peer_groups.append(self.get_mlag_peer_group(bgp_peer_group, custom_structured_configs))
-            if not self.underlay_ipv6_numbered:
-                router_bgp.address_family_ipv4.peer_groups.append(self.get_mlag_peer_group_address_familiy_ipv4(bgp_peer_group, self.inputs.underlay_rfc5549))
-            if self.underlay_ipv6:
-                router_bgp.address_family_ipv6.peer_groups.append_new(name=bgp_peer_group.name, activate=True)
-
-        if self.use_separate_peer_group_for_mlag_vrfs:
-            bgp_peer_group = self.inputs.bgp_peer_groups.mlag_ipv4_vrfs_peer
-            router_bgp.peer_groups.append(self.get_mlag_peer_group(bgp_peer_group, custom_structured_configs))
-            router_bgp.address_family_ipv4.peer_groups.append(self.get_mlag_peer_group_address_familiy_ipv4(bgp_peer_group, self.inputs.overlay_mlag_rfc5549))
-
-    def get_mlag_peer_group(
-        self: SharedUtilsProtocol,
-        bgp_peer_group: EosDesigns.BgpPeerGroups.MlagIpv4UnderlayPeer | EosDesigns.BgpPeerGroups.MlagIpv4VrfsPeer,
-        custom_structured_configs: StructCfgs,
-    ) -> EosCliConfigGen.RouterBgp.PeerGroupsItem:
-        """Return structured_config for one MLAG peer_group."""
-        peer_group_name = bgp_peer_group.name
-        peer_group = EosCliConfigGen.RouterBgp.PeerGroupsItem(
-            name=peer_group_name,
-            remote_as=self.formatted_bgp_as,
-            next_hop_self=True,
-            description=AvdStringFormatter().format(self.inputs.mlag_bgp_peer_group_description, mlag_peer=self.mlag_peer),
-            password=self.get_bgp_password(bgp_peer_group),
-            bfd=bgp_peer_group.bfd or None,
-            maximum_routes=bgp_peer_group.maximum_routes,
-            send_community="all",
-        )
-        peer_group.metadata.type = "ipv4"
-
-        if bgp_peer_group.structured_config:
-            custom_structured_configs.nested.router_bgp.peer_groups.obtain(peer_group_name)._deepmerge(
-                bgp_peer_group.structured_config, list_merge=custom_structured_configs.list_merge_strategy
-            )
-
-        if self.node_config.mlag_ibgp_origin_incomplete:
-            peer_group.route_map_in = "RM-MLAG-PEER-IN"
-
-        return peer_group
-
-    def get_mlag_peer_group_address_familiy_ipv4(
-        self: SharedUtilsProtocol,
-        bgp_peer_group: EosDesigns.BgpPeerGroups.MlagIpv4UnderlayPeer | EosDesigns.BgpPeerGroups.MlagIpv4VrfsPeer,
-        rfc5549: bool,
-    ) -> EosCliConfigGen.RouterBgp.AddressFamilyIpv4.PeerGroupsItem:
-        """Return structured_config for activation of one MLAG peer_group under address-family IPv4."""
-        address_family_peer_group = EosCliConfigGen.RouterBgp.AddressFamilyIpv4.PeerGroupsItem(name=bgp_peer_group.name, activate=True)
-        if rfc5549:
-            address_family_peer_group.next_hop.address_family_ipv6._update(enabled=True, originate=True)
-        return address_family_peer_group
-
     @cached_property
     def underlay_multicast_pim_mlag_enabled(self: SharedUtilsProtocol) -> bool:
         """

python-avd/pyavd/_eos_designs/schema/eos_designs.schema.yml

@@ -250,27 +250,6 @@ def mlag_configuration(self) -> None:
             mlag_configuration.dual_primary_detection_delay = 5
         self.structured_config.mlag_configuration = mlag_configuration
 
-    @structured_config_contributor
-    def route_maps(self) -> None:
-        """
-        Set list of route-maps.
-
-        Origin Incomplete for MLAG iBGP learned routes.
-
-        TODO: Partially duplicated in network_services. Should be moved to a common class
-        """
-        if not (self.shared_utils.mlag_l3 and self.shared_utils.node_config.mlag_ibgp_origin_incomplete and self.shared_utils.underlay_bgp):
-            return
-
-        route_map = EosCliConfigGen.RouteMapsItem(name="RM-MLAG-PEER-IN")
-        route_map.sequence_numbers.append_new(
-            sequence=10,
-            type="permit",
-            description="Make routes learned over MLAG Peer-link less preferred on spines to ensure optimal routing",
-            set=EosCliConfigGen.RouteMapsItem.SequenceNumbersItem.Set(["origin incomplete"]),
-        )
-        self.structured_config.route_maps.append(route_map)
-
     @structured_config_contributor
     def router_bgp(self) -> None:
         """
@@ -283,7 +262,9 @@ def router_bgp(self) -> None:
             return
 
         # MLAG Peer group
-        self.shared_utils.update_router_bgp_with_mlag_peer_group(self.structured_config.router_bgp, self.custom_structured_configs)
+        self.structured_config_utils.set_once_peer_group_mlag_ipv4_underlay_peer()
+        if not self.inputs.avd_design_future.only_configure_mlag_vrfs_peer_group_when_used and self.shared_utils.use_separate_peer_group_for_mlag_vrfs:
+            self.structured_config_utils.set_once_peer_group_mlag_ipv4_vrfs_peer()
 
         vlan = default(self.shared_utils.mlag_peer_l3_vlan, self.shared_utils.node_config.mlag_peer_vlan)
         interface_name = f"Vlan{vlan}"

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

@@ -54,11 +54,6 @@ def route_maps(self: AvdStructuredConfigNetworkServicesProtocol) -> None:
                     self.structured_config.route_maps.append(route_maps_item)
         self._route_maps_vrf_default()
 
-        # Note we check the 'flag need_mlag_peer_group' here which is being set by router_bgp logic. So this must run after.
-        # TODO: Move this logic to a single place instead.
-        if self.need_mlag_peer_group and self.shared_utils.node_config.mlag_ibgp_origin_incomplete:
-            self._bgp_mlag_peer_group_route_map()
-
         if self._mlag_ibgp_peering_subnets_without_redistribution:
             self.set_once_route_map_connected_to_bgp_vrfs()
 
@@ -89,23 +84,6 @@ def _route_maps_vrf_default_check(self: AvdStructuredConfigNetworkServicesProtoc
 
         return bool(self.shared_utils.wan_role and self._vrf_default_ipv4_static_routes["redistribute_in_overlay"])
 
-    def _bgp_mlag_peer_group_route_map(self: AvdStructuredConfigNetworkServicesProtocol) -> None:
-        """
-        Set one route-map item.
-
-        Origin Incomplete for MLAG iBGP learned routes.
-
-        TODO: Partially duplicated from mlag. Should be moved to a common class
-        """
-        route_maps_item = EosCliConfigGen.RouteMapsItem(name="RM-MLAG-PEER-IN")
-        route_maps_item.sequence_numbers.append_new(
-            sequence=10,
-            type="permit",
-            set=EosCliConfigGen.RouteMapsItem.SequenceNumbersItem.Set(["origin incomplete"]),
-            description="Make routes learned over MLAG Peer-link less preferred on spines to ensure optimal routing",
-        )
-        self.structured_config.route_maps.append(route_maps_item)
-
     @run_once_method
     def set_once_route_map_connected_to_bgp_vrfs(self: AvdStructuredConfigNetworkServicesProtocol) -> None:
         """