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

Feat(eos_cli_config_gen): port-channel-interfaces enhancement #1396

Conversation

UchihaItachiSama
Copy link
Contributor

Change Summary

PR to add support for:

  • single-active multi-homing redundancy mode
  • designated-forwarder election algorithm

Reference article

Related Issue(s)

Fixes #1163

Component(s) name

arista.avd.eos_cli_config_gen

Proposed changes

Added following changes to the port-channel-interfaces.yml

  < Port-Channel_interface_2 >:
    <snipped>
    redundancy: < Multihoming redundancy mode (all-active | single-active | default --> all-active) >
    designated_forwarder_election:
      algorithm: < modulus | preference | default --> modulus >
      preference_value: < Preference value (0-65535) >
      dont_preempt: < non-revertive DF election mode ( true | false | default --> false ) >

YAML

  Port-Channel60:
    description: server01_PortChannel3
    vlans: 1-4000
    mode: trunk
    esi: 0000:0000:0101:0102:0033
    rt: 01:01:01:02:00:33
    lacp_id: 0101.0102.0033
    redundancy: single-active
    designated_forwarder_election:
      algorithm: preference
      preference_value: 1000
      dont_preempt: true

CLI

interface Port-Channel60
   description server01_PortChannel3
   switchport
   switchport trunk allowed vlan 1-4000
   switchport mode trunk
   evpn ethernet-segment
      identifier 0000:0000:0101:0102:0033
      route-target import 01:01:01:02:00:33
      redundancy single-active
      designated-forwarder election algorithm preference 1000 dont-preempt
   lacp system-id 0101.0102.0033

How to test

Ran molecule test which resulted in generation of TOC documentation and intended configuration

molecule test --scenario-name eos_cli_config_gen

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 github-actions bot added role: eos_cli_config_gen issue related to eos_cli_config_gen role state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated labels Dec 10, 2021
@tgodaA
Copy link
Contributor

tgodaA commented Dec 10, 2021

you need to run make refresh-facts from ansible_collections/arista/avd/molecule directory. This will run molecule test cases for all scenarios.

@tgodaA
Copy link
Contributor

tgodaA commented Dec 10, 2021

Also the data model doesn't match with the eos cli. Even the esi and rt should have been inside a knob evpn or evpn_es.

My suggestion for the data model would be:

evpn_es:
  redundancy: < active-active | single-active | default --> active-active >
  designated_forwarder_election:
    hold_time: < time seconds >
    algorithm: <>
    preference_value: <>
    dont_preempt: <>

I don't have a router to test the possible commands. Let's see what others will suggest.

@UchihaItachiSama
Copy link
Contributor Author

you need to run make refresh-facts from ansible_collections/arista/avd/molecule directory. This will run molecule test cases for all scenarios.

Sure, will run the make refresh-facts. Do we need this all time for eos_cli_config_gen? If so should be update the molecule README

@UchihaItachiSama
Copy link
Contributor Author

UchihaItachiSama commented Dec 13, 2021

Also the data model doesn't match with the eos cli. Even the esi and rt should have been inside a knob evpn or evpn_es.

Yeah, since we are adding the designated_forwarder_election and redundancy it would be better to include esi and rt also under evpn ethernet-segment similar to the CLI.

For the key should we keep it evpn_ethernet_segment or evpn_es?

port_channel_interfaces:
  < Port-Channel_interface_1 >:
  evpn_es / evpn_ethernet_segment:
      esi: < EVPN Ethernet Segment Identifier (Type 1 format) >
      rt: < EVPN Route Target for ESI with format xx:xx:xx:xx:xx:xx >
      redundancy: < active-active | single-active | default --> active-active >
      designated_forwarder_election:
           hold_time: < time seconds >
           algorithm: <>
           preference_value: <>
           dont_preempt: <>

@tgodaA
Copy link
Contributor

tgodaA commented Dec 13, 2021

Hi Himanshu, we can add esi and rt under evpn_ethernet_segment but I guess it is a breaking change. So let's leave them for now unless if everyone agrees to move them.

@UchihaItachiSama
Copy link
Contributor Author

Hi Himanshu, we can add esi and rt under evpn_ethernet_segment but it is a breaking change. So let's leave them for now unless if everyone agrees to move them.

Hey Tony, sure. For now i'll update the data model as follows

  < Port-Channel_interface_2 >:
    description: < description >
    vlans: "< list of vlans as string >"
    mode: < access | dot1q-tunnel | trunk | "trunk phone" >
    esi: < EVPN Ethernet Segment Identifier (Type 1 format) >
    rt: < EVPN Route Target for ESI with format xx:xx:xx:xx:xx:xx >
    lacp_id: < LACP ID with format xxxx.xxxx.xxxx >
    evpn_ethernet_segment:
       redundancy: < Multihoming redundancy mode (all-active | single-active | default --> all-active) >
       designated_forwarder_election:
         algorithm: < modulus | preference | default --> modulus >
         preference_value: < Preference value (0-65535) >
         hold_time: < time seconds >
         dont_preempt: < non-revertive DF election mode ( true | false | default --> false ) >

@lermilov
Copy link
Contributor

Hi Himanshu!
From EVPN-VXLAN perspective looks good to me. However, for EVPN-MPLS MH I would recommend to add followig knob to cli_config_gen:
interface Ethernet4
switchport access vlan 1000
!
evpn ethernet-segment
identifier 0022:2222:2222:2222:2222
mpls shared index 100 <<--- shared ESI index
route-target import 00:02:00:02:00:02

EOS TOI:
https://eos.arista.com/eos-4-25-2f/l2-evpn-mpls-shared-esi-label/

@UchihaItachiSama
Copy link
Contributor Author

UchihaItachiSama commented Dec 15, 2021

Hi Himanshu!
From EVPN-VXLAN perspective looks good to me. However, for EVPN-MPLS MH I would recommend to add followig knob to cli_config_gen:
interface Ethernet4
switchport access vlan 1000
!
evpn ethernet-segment
identifier 0022:2222:2222:2222:2222
mpls shared index 100 <<--- shared ESI index
route-target import 00:02:00:02:00:02

EOS TOI:
https://eos.arista.com/eos-4-25-2f/l2-evpn-mpls-shared-esi-label/

Hey Leonid,

Sure, i'll check the TOI and update the data model as follows:

    evpn_ethernet_segment:
      redundancy: < Multihoming redundancy mode (all-active | single-active | default --> all-active) >
      designated_forwarder_election:
        algorithm: < modulus | preference | default --> modulus >
        preference_value: < Preference value (0-65535) >
        dont_preempt: < non-revertive DF election mode ( true | false | default --> false ) >
        hold_time: < time seconds (1-1800) >
        subsequent_hold_time: < time milliseconds (10-10000) >
        candidate_reachability_required: < true | false | default --> false >
        mpls_shared_index: < (1-1024) >
        

@UchihaItachiSama UchihaItachiSama force-pushed the evpn-single-active-mulit-homing-eos-cli branch from 4082f8f to 8daf79f Compare December 22, 2021 16:26
@UchihaItachiSama UchihaItachiSama force-pushed the evpn-single-active-mulit-homing-eos-cli branch from 8daf79f to 9e96a98 Compare December 24, 2021 07:58
@UchihaItachiSama
Copy link
Contributor Author

Updated the requested changes.

@tgodaA
Copy link
Contributor

tgodaA commented Jan 14, 2022

Hi, FYI, here is a PR that is closely related to your's: #1437. Forget about the esi/identifier, and rt/route_target. But you can implement rest of the knobs similarly.

@github-actions github-actions bot added the state: conflict PR with conflict label Jan 26, 2022
@github-actions
Copy link

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

@tgodaA
Copy link
Contributor

tgodaA commented Jan 27, 2022

Hi Himanshu. Do you want me to take this PR? I have a requirement of this feature, not immediately but by next month. We have lot of time but let me know if you want me to take this. I did something similar tho this feature on ethernet-interfaces, #1437

@tgodaA tgodaA self-assigned this Jan 31, 2022
@UchihaItachiSama
Copy link
Contributor Author

UchihaItachiSama commented Feb 1, 2022

Hey Tony, sure. sorry got a bit backed up on other work.

@tgodaA tgodaA force-pushed the evpn-single-active-mulit-homing-eos-cli branch from 9e96a98 to 6b17fd0 Compare February 1, 2022 20:29
@UchihaItachiSama UchihaItachiSama requested a review from a team as a code owner February 1, 2022 20:29
@github-actions github-actions bot removed the state: conflict PR with conflict label Feb 1, 2022
@github-actions
Copy link

github-actions bot commented Feb 1, 2022

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

@tgodaA tgodaA requested review from tgodaA and removed request for tgodaA February 1, 2022 20:30
Copy link
Contributor

@ClausHolbechArista ClausHolbechArista left a comment

Choose a reason for hiding this comment

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

LGTM

@tgodaA tgodaA added this to the v3.3.0 milestone Feb 9, 2022
@carlbuchmann carlbuchmann force-pushed the evpn-single-active-mulit-homing-eos-cli branch from 6b17fd0 to a50d5a0 Compare February 11, 2022 01:47
Copy link
Member

@carlbuchmann carlbuchmann left a comment

Choose a reason for hiding this comment

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

LGTM!

@carlbuchmann carlbuchmann merged commit e117eda into aristanetworks:devel Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
role: eos_cli_config_gen issue related to eos_cli_config_gen 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.

Support for EVPN Single-active Multihoming
5 participants