isis: T1316: refactor config retrieval and Jinja2 template

Make the entire template code more human readable by denesting it, as there can
only be one ISIS daemon instance in FRR.

4 files changed, 164 insertions, 184 deletions

diff --git a/Makefile b/Makefile
index d18676619..fffd1c9c3 100644
--- a/Makefile
+++ b/Makefile
@@ -78,6 +78,7 @@ interface_definitions: $(BUILD_DIR) $(obj)
 	rm -f $(TMPL_DIR)/interfaces/wirelessmodem/node.tag/ipv6/node.def
 	rm -f $(TMPL_DIR)/protocols/node.def
 	rm -rf $(TMPL_DIR)/protocols/nbgp
+	rm -rf $(TMPL_DIR)/protocols/isis
 	rm -f $(TMPL_DIR)/protocols/static/node.def
 	rm -f $(TMPL_DIR)/system/node.def
 	rm -f $(TMPL_DIR)/vpn/node.def
diff --git a/data/configd-include.json b/data/configd-include.json
index da6fb915f..2e44405ee 100644
--- a/data/configd-include.json
+++ b/data/configd-include.json
@@ -30,6 +30,7 @@
 "nat.py",
 "ntp.py",
 "protocols_igmp.py",
+"protocols_isis.py",
 "protocols_mpls.py",
 "protocols_pim.py",
 "protocols_rip.py",
diff --git a/data/templates/frr/isis.frr.tmpl b/data/templates/frr/isis.frr.tmpl
index 70444d0c3..929f5bdb2 100644
--- a/data/templates/frr/isis.frr.tmpl
+++ b/data/templates/frr/isis.frr.tmpl
@@ -1,158 +1,121 @@
-{%   for dname in isis -%}
 !
-router isis {{dname}}
-{% if isis[dname]['net'] is string -%}
- net {{ isis[dname]['net'] }}
-{% else -%}
-{%   for net_iso in isis[dname]['net'] -%}
- net {{ net_iso }}
-{%   endfor -%}
-{% endif -%}
-{#- end isis net -#}
-{% if 'dynamic_hostname' in isis[dname] -%}
+router isis {{ process }}
+ net {{ net }}
+{% if dynamic_hostname is defined %}
  hostname dynamic
-{% endif -%}
-{% if 'purge_originator' in isis[dname] -%}
+{% endif %}
+{% if purge_originator is defined %}
  purge-originator
-{% endif -%}
-{% if 'set_attached_bit' in isis[dname] -%}
+{% endif %}
+{% if set_attached_bit is defined %}
  set-attached-bit
-{% endif -%}
-{% if 'set_overload_bit' in isis[dname] -%}
+{% endif %}
+{% if set_overload_bit is defined %}
  set-overload-bit
-{% endif -%}
-{% if isis[dname].domain_password is defined -%}
-{%   if isis[dname].domain_password.plaintext_password is defined -%}
- domain-password clear {{ isis[dname].domain_password.plaintext_password }}
-{%   endif -%}
-{% endif -%}
-{#- end isis domain-password -#}
-{% if isis[dname]['lsp_gen_interval'] -%}
- lsp-gen-interval {{ isis[dname]['lsp_gen_interval'] }}
-{% endif -%}
-{% if isis[dname]['lsp_mtu'] -%}
- lsp-mtu {{ isis[dname]['lsp_mtu'] }}
-{% endif -%}
-{% if isis[dname]['lsp_refresh_interval'] -%}
- lsp-mtu {{ isis[dname]['lsp_refresh_interval'] }}
-{% endif -%}
-{% if isis[dname]['max_lsp_lifetime'] -%}
- lsp-mtu {{ isis[dname]['max_lsp_lifetime'] }}
-{% endif -%}
-{% if isis[dname]['spf_interval'] -%}
- spf-interval {{ isis[dname]['spf_interval'] }}
-{% endif -%}
-{% for t in isis[dname]['spf_delay_ietf'] -%}
-{% if t == "holddown" -%}
- spf-delay-ietf holddown {{ isis[dname]['spf_delay_ietf'][t] }}
-{% endif -%}
-{% endfor -%}
-{#- end lsp timers and intervals -#}
-{% if isis[dname]['area_password'] is defined -%}
-{% for password in isis[dname]['area_password'] -%}
-{%   if password == "md5" -%}
- area-password md5 {{ isis[dname]['area_password'][password] }}
-{%   endif -%}
-{%   if password == "plaintext_password" -%}
- area-password clear {{ isis[dname]['area_password'][password] }}
-{%   endif -%}
-{% endfor -%}
-{% endif -%}
-{#- end isis area-password -#}
-{% if 'originate' in isis[dname]['default_information'] -%}
-{% for ipv in isis[dname]['default_information']['originate'] -%}
-{%   for lvl in isis[dname]['default_information']['originate'][ipv] -%}
-{%     if ipv == 'ipv4' -%}
-    default-information originate {{ ipv }} {{ lvl | replace('_', '-') }}
-{%     elif ipv == 'ipv6' -%}
-    default-information originate {{ ipv }} {{ lvl | replace('_', '-') }} always
-{%     endif -%}
-{%   endfor -%}
-{% endfor -%}
-{% endif -%}
-{#- end isis default-information originate -#}
-{% if isis[dname]['redistribute'] is defined -%}
-{% for protocol in isis[dname]['redistribute']['ipv4']  -%}
-{%   for lvl in isis[dname]['redistribute']['ipv4'][protocol] -%}
-{%     if isis[dname]['redistribute']['ipv4'][protocol][lvl]['metric'] is defined -%}
- redistribute ipv4 {{ protocol }} {{ lvl | replace('_', '-') }} metric {{ isis[dname]['redistribute']['ipv4'][protocol][lvl]['metric'] }}
-{%     elif isis[dname]['redistribute']['ipv4'][protocol][lvl]['route_map'] is defined -%}
- redistribute ipv4 {{ protocol }} {{ lvl | replace('_', '-') }} route-map {{ isis[dname]['redistribute']['ipv4'][protocol][lvl]['route_map'] }}
-{%     else -%}
- redistribute ipv4 {{ protocol }} {{ lvl | replace('_', '-') }}
-{%     endif -%}
-{%   endfor -%}
-{% endfor -%}
-{% endif -%}
-{#- end isis redistribute -#}
-{% if isis[dname]['level'] -%}
-{%   if isis[dname]['level'] == 'level-1' -%}
+{% endif %}
+{% if domain_password is defined and domain_password.plaintext_password is defined and domain_password.plaintext_password is not none %}
+ domain-password clear {{ domain_password.plaintext_password }}
+{% endif %}
+{% if lsp_gen_interval is defined and lsp_gen_interval is not none %}
+ lsp-gen-interval {{ lsp_gen_interval }}
+{% endif %}
+{% if lsp_mtu is defined and lsp_mtu is not none %}
+ lsp-mtu {{ lsp_mtu }}
+{% endif %}
+{% if lsp_refresh_interval is defined and lsp_refresh_interval is not none %}
+ lsp-refresh-interval  {{ lsp_refresh_interval }}
+{% endif %}
+{% if max_lsp_lifetime is defined and max_lsp_lifetime is not none %}
+ max-lsp-lifetime {{ max_lsp_lifetime }}
+{% endif %}
+{% if spf_interval is defined and spf_interval is not none %}
+ spf-interval {{ spf_interval }}
+{% endif %}
+{% if spf_delay_ietf is defined and spf_delay_ietf.init_delay is defined and spf_delay_ietf.init_delay is not none %}
+ spf-delay-ietf init-delay {{ spf_delay_ietf.init_delay }}
+{% endif %}
+{% if area_password is defined and area_password.md5 is defined and area_password.md5 is not none %}
+ area-password md5 {{ area_password.md5 }}
+{% elif area_password is defined and area_password.plaintext_password is defined and area_password.plaintext_password is not none %}
+ area-password clear {{ area_password.plaintext_password }}
+{% endif %}
+{% if default_information is defined and default_information.originate is defined and default_information.originate is not none %}
+{%   for level in default_information.originate.ipv4 if default_information.originate.ipv4 is defined %}
+ default-information originate ipv4 {{ level | replace('_', '-') }}
+{%   endfor %}
+{%   for level in default_information.originate.ipv6 if default_information.originate.ipv6 is defined %}
+ default-information originate ipv6 {{ level | replace('_', '-') }} always
+{%   endfor %}
+{% endif %}
+{% if redistribute is defined and redistribute.ipv4 is defined and redistribute.ipv4 is not none %}
+{%   for protocol in redistribute.ipv4 %}
+{%     for level, level_config in redistribute.ipv4[protocol].items() %}
+{%       if level_config.metric is defined and level_config.metric is not none %}
+ redistribute ipv4 {{ protocol }} {{ level | replace('_', '-') }} metric {{ level_config.metric }}
+{%       elif level_config.route_map is defined and level_config.route_map is not none %}
+ redistribute ipv4 {{ protocol }} {{ level | replace('_', '-') }} route-map {{ level_config.route_map }}
+{%       else %}
+ redistribute ipv4 {{ protocol }} {{ level | replace('_', '-') }}
+{%       endif %}
+{%     endfor %}
+{%   endfor %}
+{% endif %}
+{% if level is defined and level is not none %}
+{%   if level == 'level-1' %}
  is-type level-1
-{%   elif isis[dname]['level'] == 'level-2'  -%}
+{%   elif level == 'level-2'  %}
  is-type level-2-only
-{%   elif isis[dname]['level'] == 'level-1-2'  -%}
+{%   elif level == 'level-1-2'  %}
  is-type level-1-2
-{%   endif -%}
-{% endif -%}
-{#- end isis level -#}
-{% if isis[dname]['spf_delay_ietf'] is defined -%}
-{%   if isis[dname]['spf_delay_ietf']['init_delay'] or isis[dname]['spf_delay_ietf']['short_delay'] or isis[dname]['spf_delay_ietf']['long_delay'] or isis[dname]['spf_delay_ietf']['holddown'] or isis[dname]['spf_delay_ietf']['time_to_learn'] -%}
- init-delay {{ isis[dname]['spf_delay_ietf']['init_delay'] }} short-delay {{ isis[dname]['spf_delay_ietf']['short_delay'] }} long-delay {{ isis[dname]['spf_delay_ietf']['long_delay'] }} holddown {{ isis[dname]['spf_delay_ietf']['holddown'] }} time-to-learn {{ isis[dname]['spf_delay_ietf']['time_to_learn'] }}
-{%   endif -%}
-{% endif -%}
+{%   endif %}
+{% endif %}
 !
-{% set isis_iface = isis[dname]['interface'] -%}
-{% if isis_iface is string -%}
-interface {{ isis_iface }} 
-ip router isis {{ dname }}
-{% else -%}
-{% for value in isis_iface -%}
-interface {{ value }} 
-ip router isis {{ dname }}
-{%   if 'bfd' in isis_iface[value] -%}
-  isis bfd
-{%   endif -%}
-{%   if 'point_to_point' in isis_iface[value]['network'] -%}
-  isis network point-to-point
-{%   endif -%}
-{%   if isis_iface[value]['circuit_type'] == 'level-1' -%}
- isis circuit-type level-1
-{%   elif isis_iface[value]['circuit_type'] == 'level-2-only' -%}
- isis circuit-type level-2-only
-{%   elif isis_iface[value]['circuit_type'] == 'level-1-2' -%}
- isis circuit-type level-1-2 
-{%   endif -%}
-{%   if isis_iface[value]['hello_interval'] -%}
- isis hello-interval {{ isis_iface[value]['hello_interval'] }}
-{%   endif -%}
-{%   if isis_iface[value]['hello_multiplier'] -%}
- isis hello-multiplier {{ isis_iface[value]['hello_multiplier'] }}
-{%   endif -%}
-{%   if 'hello_padding' in isis_iface[value] -%}
+{% if interface_remove is defined and interface_remove is not none %}
+{%   for iface in interface_remove %}
+interface {{ iface }}
+ no ip router isis
+{%   endfor %}
+{% endif %}
+!
+{% if interface is defined and interface is not none %}
+{%   for iface, iface_config in interface.items() %}
+interface {{ iface }}
+ ip router isis {{ process }}
+{%     if iface_config.bfd is defined %}
+ isis bfd
+{%     endif %}
+{%     if iface_config.network is defined and iface_config.network.point_to_point is defined %}
+ isis network point-to-point
+{%     endif %}
+{%     if iface_config.circuit_type is defined %}
+ isis circuit-type {{ iface_config.circuit_type }}
+{%     endif %}
+{%     if iface_config.hello_interval is defined and iface_config.hello_interval is not none %}
+ isis hello-interval {{ iface_config.hello_interval }}
+{%     endif %}
+{%     if iface_config.hello_multiplier is defined and iface_config.hello_multiplier is not none %}
+ isis hello-multiplier {{ iface_config.hello_multiplier }}
+{%     endif %}
+{%     if iface_config.hello_padding is defined %}
  isis hello padding
-{%   endif -%}
-{%   if isis_iface[value]['metric'] -%}
- isis metric {{ isis_iface[value]['metric'] }}
-{%   endif -%}
-{%   if 'passive' in isis_iface[value] -%}
+{%     endif %}
+{%     if iface_config.metric is defined and iface_config.metric is not none %}
+ isis metric {{ iface_config.metric }}
+{%     endif %}
+{%     if iface_config.passive is defined %}
  isis passive
-{%   endif -%}
-{%   if isis_iface[value]['password'] is defined -%}
-{%     if isis_iface[value]['password']['plaintext_password'] is defined -%}
- isis password clear {{ isis_iface[value]['password']['plaintext_password'] }}
-{%     endif -%}
-{%   endif -%}
-{%   if isis_iface[value]['priority'] -%}
- isis priority {{ isis_iface[value]['priority'] }}
-{%   endif -%}
-{%   if isis_iface[value]['psnp_interval'] -%}
- isis psnp-interval {{ isis_iface[value]['psnp_interval'] }}
-{%   endif -%}
-{%   if 'three_way_handshake' in isis_iface[value] -%}
+{%     endif %}
+{%     if iface_config.password is defined and iface_config.password.plaintext_password is defined and iface_config.password.plaintext_password is not none %}
+ isis password clear {{ iface_config.password.plaintext_password }}
+{%     endif %}
+{%     if iface_config.priority is defined and iface_config.priority is not none %}
+ isis priority {{ iface_config.priority }}
+{%     endif %}
+{%     if iface_config.psnp_interval is defined and iface_config.psnp_interval is not none %}
+ isis psnp-interval {{ iface_config.psnp_interval }}
+{%     endif %}
+{%     if iface_config.three_way_handshake is defined %}
  isis three-way-handshake
-{%   endif -%}
-{% endfor -%}
-{% endif -%}
-{#- end isis interface -#}
-!
-{%   endfor -%}
+{%     endif %}
+{%   endfor %}
+{% endif %}
diff --git a/src/conf_mode/protocols_isis.py b/src/conf_mode/protocols_isis.py
index 4ee6b31f6..d5e5b64fb 100755
--- a/src/conf_mode/protocols_isis.py
+++ b/src/conf_mode/protocols_isis.py
@@ -19,29 +19,33 @@ import os
 from sys import exit
 
 from vyos.config import Config
+from vyos.configdict import node_changed
 from vyos import ConfigError
 from vyos.util import call
 from vyos.template import render
 from vyos.template import render_to_string
 from vyos import frr
 from vyos import airbag
-from pprint import pprint
 airbag.enable()
 
 config_file = r'/tmp/isis.frr'
 
-default_config_data = {
-    'interface':  ''
-}
-
-def get_config():
-    conf = Config()
+def get_config(config=None):
+    if config:
+        conf = config
+    else:
+        conf = Config()
     base = ['protocols', 'isis']
-    isis = conf.get_config_dict(base, key_mangling=('-', '_'))
-    if not conf.exists(base):
-        isis = {}
 
-    pprint(isis)
+    isis = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True)
+
+    # determine which members have been removed
+    for instance in isis:
+        conf.set_level(base + [instance])
+        tmp = node_changed(conf, ['interface'])
+        if tmp:
+            isis[instance].update({'interface_remove': tmp})
+
     return isis
 
 def verify(isis):
@@ -49,32 +53,31 @@ def verify(isis):
     if not isis:
         return None
 
-    for proc_id, foo_config in isis['isis'].items():
-
+    for process, isis_config in isis.items():
         # If more then one isis process is defined (Frr only supports one)
         # http://docs.frrouting.org/en/latest/isisd.html#isis-router
-        if len(isis['isis']) > 1:
+        if len(isis) > 1:
             raise ConfigError('Only one isis process can be definded')
 
         # If network entity title (net) not defined
-        if not "net" in  foo_config:
-          raise ConfigError('Define net format iso is mandatory in \"isis {} net"!'.format(proc_id))
+        if 'net' not in isis_config:
+            raise ConfigError('ISIS net format iso is mandatory!')
 
         # If interface not set
-        if not "interface" in  foo_config:
-          raise ConfigError('Define interface is mandatory in \"isis {} interface"!'.format(proc_id))
+        if 'interface' not in isis_config:
+            raise ConfigError('ISIS interface is mandatory!')
 
         # If md5 and plaintext-password set at the same time
-        if 'area_password' in foo_config:
-            if "md5" in foo_config['area_password'] and "plaintext_password" in foo_config['area_password']:
-                raise ConfigError('Only one password type should be used in \"isis {} area-password"!'.format(proc_id))
+        if 'area_password' in isis_config:
+            if {'md5', 'plaintext_password'} <= set(isis_config['encryption']):
+                raise ConfigError('Can not use both md5 and plaintext-password for ISIS area-password!')
 
         # If one param from deley set, but not set others
-        if 'spf_delay_ietf' in foo_config:
+        if 'spf_delay_ietf' in isis_config:
             required_timers = ['holddown', 'init_delay', 'long_delay', 'short_delay', 'time_to_learn']
             exist_timers = []
             for elm_timer in required_timers:
-                if elm_timer in foo_config['spf_delay_ietf']:
+                if elm_timer in isis_config['spf_delay_ietf']:
                     exist_timers.append(elm_timer)
 
             exist_timers = set(required_timers).difference(set(exist_timers))
@@ -82,14 +85,14 @@ def verify(isis):
                 raise ConfigError('All types of delay must be specified: ' + ', '.join(exist_timers).replace('_', '-'))
 
         # If Redistribute set, but level don't set
-        if 'redistribute' in foo_config:
-            proc_level = foo_config.get('level','').replace('-','_')
-            for proto, proto_config in foo_config.get('redistribute', {}).get('ipv4', {}).items():
+        if 'redistribute' in isis_config:
+            proc_level = isis_config.get('level','').replace('-','_')
+            for proto, proto_config in isis_config.get('redistribute', {}).get('ipv4', {}).items():
                 if 'level_1' not in proto_config and 'level_2' not in proto_config:
-                    raise ConfigError('Redistribute level-1 or level-2 should be specified in \"protocols isis {} redistribute ipv4 {}\"'.format(proc_id, proto))
+                    raise ConfigError('Redistribute level-1 or level-2 should be specified in \"protocols isis {} redistribute ipv4 {}\"'.format(process, proto))
             for redistribute_level in proto_config.keys():
                 if proc_level and proc_level != 'level_1_2' and proc_level != redistribute_level:
-                    raise ConfigError('\"protocols isis {0} redistribute ipv4 {2} {3}\" cannot be used with \"protocols isis {0} level {1}\"'.format(proc_id, proc_level, proto, redistribute_level))
+                    raise ConfigError('\"protocols isis {0} redistribute ipv4 {2} {3}\" cannot be used with \"protocols isis {0} level {1}\"'.format(process, proc_level, proto, redistribute_level))
 
     return None
 
@@ -98,31 +101,43 @@ def generate(isis):
         isis['new_frr_config'] = ''
         return None
 
+    # only one ISIS process is supported, so we can directly send the first key
+    # of the config dict
+    process = list(isis.keys())[0]
+    isis[process]['process'] = process
+
+    import pprint
+    pprint.pprint(isis[process])
+
     # render(config) not needed, its only for debug
-    render(config_file, 'frr/isis.frr.tmpl', isis)
+    render(config_file, 'frr/isis.frr.tmpl', isis[process], trim_blocks=True)
 
-    isis['new_frr_config'] = render_to_string('frr/isis.frr.tmpl', isis)
+    isis['new_frr_config'] = render_to_string('frr/isis.frr.tmpl',
+                                              isis[process], trim_blocks=True)
 
     return None
 
 def apply(isis):
 
     # Save original configration prior to starting any commit actions
-    isis['original_config'] = frr.get_configuration(daemon='isisd')
-    isis['modified_config'] = frr.replace_section(isis['original_config'], isis['new_frr_config'], from_re='router isis .*')
+    frr_cfg = {}
+    frr_cfg['original_config'] = frr.get_configuration(daemon='isisd')
+    frr_cfg['modified_config'] = frr.replace_section(frr_cfg['original_config'], isis['new_frr_config'], from_re='router isis .*')
 
     # Debugging
     print('')
     print('--------- DEBUGGING ----------')
-    print(f'Existing config:\n{isis["original_config"]}\n\n')
+    print(f'Existing config:\n{frr_cfg["original_config"]}\n\n')
     print(f'Replacement config:\n{isis["new_frr_config"]}\n\n')
-    print(f'Modified config:\n{isis["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(isis['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(isis['modified_config'], daemon='isisd')
+    # Commit resulting configuration to FRR, this will throw CommitError
+    # on failure
+    frr.reload_configuration(frr_cfg['modified_config'], daemon='isisd')
 
     return None