search

aristanetworks / avd

Arista Validated Designs
https://avd.arista.com
Apache License 2.0
288 stars 206 forks source link

Feat(eos_cli_config_gen): Better handle service_routing_configuration_bgp['no_equals_default'] in eos_cli_config_gen & eos_designs #1107

Open ryanmerolle opened 3 years ago

ryanmerolle commented 3 years ago

Issue Type

Is your feature request related to a problem? Please describe. When service_routing_configuration_bgp['no_equals_default'] is set to true, eos_designs does not properly take into account config changes required.

IE the no neighbor EVPN-OVERLAY-PEERS activate should be converted to neighbor EVPN-OVERLAY-PEERS deactivate

Furthermore, eos_cli_config_gen does not handle this correctly either.

Describe the solution you'd like

Some way to either detect service_routing_configuration_bgp['no_equals_default'] is true or set service_routing_configuration_bgp['no_equals_default'] in the eos_designs & eos_cli_config_gen .

Describe alternatives you've considered

Additional context

EOS users often set service_routing_configuration_bgp['no_equals_default'] to remove the errant config of no neighbor x.x.x.x shut from a configuration.

ryanmerolle commented 2 years ago

One blocker I found to this is that the activate: < true | false > key under bgp neighbors or peer-groups does not have logic to treat false as deactivate, it just outputs no config. This needs to be adjusted in eos_cli_config_gen first.

ryanmerolle commented 2 years ago

@ClausHolbechArista

I am happy to tackle this in the short term by setting activate false to deactivate.

IE activate false would now be deactivate, activate true would remain activate, and the absence of activate key would do nothing.

ClausHolbechArista commented 2 years ago

@ClausHolbechArista

I am happy to tackle this in the short term by setting activate false to deactivate.

IE activate false would now be deactivate, activate true would remain activate, and the absence of activate key would do nothing.

I am not convinced that this is really what we want in a config that is 100% automated. And it will not be accepted until 4.0.0 unless we can make it nonbreaking, ex having some global avd knob to enable this behavior across all knobs. (Since similar options exist on other bgp knobs). I would be more in favor of accepting < true | false | "deactivate" > for now.

ryanmerolle commented 2 years ago

I was actually going to propose that true, false and deactivate as another option. That’s fine with me as well.

I too am conflicted at how to approach this in the short term.

Let me know how you want to proceed.