From 7305a71722ae15ae2e356e52276a823a699bed7d Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Sun, 22 Nov 2020 20:28:06 +0100 Subject: bgp: T2174: refactor Jinja template and reduce redundant paths The Jinja2 template contained a lot of redundant paths which only differed in either the address-family or neighbor vs. peer-group. This paths have been combined into for loops and a macro for generating a neighbor statement as peer-groups and regular neighbors share ~95% of the config. --- src/conf_mode/protocols_bgp.py | 66 ++++++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 22 deletions(-) (limited to 'src/conf_mode') diff --git a/src/conf_mode/protocols_bgp.py b/src/conf_mode/protocols_bgp.py index 654874232..981ff9fe9 100755 --- a/src/conf_mode/protocols_bgp.py +++ b/src/conf_mode/protocols_bgp.py @@ -14,16 +14,16 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -import os - from sys import exit from vyos.config import Config from vyos.util import call +from vyos.util import dict_search from vyos.template import render from vyos.template import render_to_string +from vyos import ConfigError from vyos import frr -from vyos import ConfigError, airbag +from vyos import airbag airbag.enable() config_file = r'/tmp/bgp.frr' @@ -31,8 +31,10 @@ config_file = r'/tmp/bgp.frr' def get_config(): conf = Config() base = ['protocols', 'nbgp'] - bgp = conf.get_config_dict(base, key_mangling=('-', '_')) + bgp = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True) + # XXX: any reason we can not move this into the FRR template? + # we shall not call vtysh directly, especially not in get_config() if not conf.exists(base): bgp = {} call('vtysh -c \"conf t\" -c \"no ip protocol bgp\" ') @@ -40,9 +42,6 @@ def get_config(): if not conf.exists(base + ['route-map']): call('vtysh -c \"conf t\" -c \"no ip protocol bgp\" ') - from pprint import pprint - pprint(bgp) - return bgp def verify(bgp): @@ -50,9 +49,23 @@ def verify(bgp): return None # Check if declared more than one ASN - for asn in bgp['nbgp'].items(): - if len(bgp['nbgp']) > 1: - raise ConfigError('Only one bgp ASN process can be definded') + if len(bgp) > 1: + raise ConfigError('Only one BGP AS can be defined!') + + for asn, asn_config in bgp.items(): + # Common verification for both peer-group and neighbor statements + for neigh in ['neighbor', 'peer_group']: + # bail out early if there is no neighbor or peer-group statement + # this also saves one indention level + if neigh not in asn_config: + continue + + for neighbor, config in asn_config[neigh].items(): + if 'remote_as' not in config and 'peer_group' not in config: + raise ConfigError(f'BGP remote-as must be specified for "{neighbor}"!') + + if 'remote_as' in config and 'peer_group' in config: + raise ConfigError(f'BGP peer-group member "{neighbor}" cannot override remote-as of peer-group!') return None @@ -61,33 +74,42 @@ def generate(bgp): bgp['new_frr_config'] = '' return None + # only one BGP AS is supported, so we can directly send the first key + # of the config dict + asn = list(bgp.keys())[0] + bgp[asn]['asn'] = asn + # render(config) not needed, its only for debug - render(config_file, 'frr/bgp.frr.tmpl', bgp) + render(config_file, 'frr/bgp.frr.tmpl', bgp[asn], trim_blocks=True) - bgp['new_frr_config'] = render_to_string('frr/bgp.frr.tmpl', bgp) + bgp['new_frr_config'] = render_to_string('frr/bgp.frr.tmpl', bgp[asn], + trim_blocks=True) return None def apply(bgp): - # Save original configration prior to starting any commit actions - bgp['original_config'] = frr.get_configuration(daemon='bgpd') - bgp['modified_config'] = frr.replace_section(bgp['original_config'], bgp['new_frr_config'], from_re='router bgp .*') + # Save original configuration prior to starting any commit actions + frr_cfg = {} + frr_cfg['original_config'] = frr.get_configuration(daemon='bgpd') + frr_cfg['modified_config'] = frr.replace_section(frr_cfg['original_config'], bgp['new_frr_config'], from_re='router bgp .*') # Debugging + print('') print('--------- DEBUGGING ----------') - print(f'Existing config:\n{bgp["original_config"]}\n\n') + print(f'Existing config:\n{frr_cfg["original_config"]}\n\n') print(f'Replacement config:\n{bgp["new_frr_config"]}\n\n') - print(f'Modified config:\n{bgp["modified_config"]}\n\n') + print(f'Modified config:\n{frr_cfg["modified_config"]}\n\n') - # Frr Mark configuration will test for syntax errors and exception out if any syntax errors are detected - frr.mark_configuration(bgp['modified_config']) + # FRR mark configuration will test for syntax errors and throws an + # exception if any syntax errors is detected + frr.mark_configuration(frr_cfg['modified_config']) - # Commit the resulting new configuration to frr, this will render an frr.CommitError() Exception on fail - frr.reload_configuration(bgp['modified_config'], daemon='bgpd') + # Commit resulting configuration to FRR, this will throw CommitError + # on failure + frr.reload_configuration(frr_cfg['modified_config'], daemon='bgpd') return None - if __name__ == '__main__': try: c = get_config() -- cgit v1.2.3