Replace routing policy 'set.community.delete' with 'delete.community' (#2842)

The 'set.community.delete: True' is a ridiculous way to say 'delete the
community'. This commit resolves #2244 and implement the more logical way
of deleting communities in routing policies:

* It defines the new 'delete.community' dictionary that can contain
  lists of community or community list name
* It automatically converts old-style routing policies into the new
  format (while generating a warning)
* All device definitions and configuration templates have been updated
  to use the new attributes
* Add missing features for the "netlab show module" printout

Bonus:
* 'delete.community.list' is now implemented for Junos

Author: ipspace

docs/module/routing.md

@@ -57,6 +57,7 @@ Routing policies are lists of instructions that can **match** route parameters a
 * **sequence**: Statement sequence number. When not specified, *netlab* sets a routing policy entry's **sequence** number to ten times its list position.
 * **match**: Conditions that must match for the routing policy entry to take effect. Entries without a **match** parameter match all routes
 * **set**: Parameters that are set on matched routes.
+* **delete**: Parameters that are deleted on matched routes.
 
 You can match these route parameters in a _netlab_ routing policy:
 
@@ -70,16 +71,18 @@ You can set these route parameters in a _netlab_ routing policy:
 * **med**: Route metric (for example, BGP MED)
 * **prepend**: BGP AS-path prepending ([more details](plugin-bgp-policy-attributes))
 * **weight**: BGP weight
-* **community**: A dictionary that can be used to set, add, or remove standard, large, or extended communities.
+* **community**: A dictionary that can be used to set or add standard, large, or extended communities.
 
 The **set.community** dictionary has these parameters:
 
 * **standard**: Standard BGP communities to set
 * **large**: Large BGP communities to set
 * **extended**: Extended BGP communities to set. The value is passed to the network device as-is (the same value might not work on all devices).
 * **append**: Add communities to existing BGP communities
-* **delete**: Remove specified communities from the BGP route
-* **delete_list**: Remove communities matching the specified community list from the BGP route. Cannot be used with any other **set.community** parameters.
+
+You can delete these route parameters:
+
+* **community**: A dictionary that can be used to delete standard, large, or extended communities. The dictionary can also contain **list** element to delete all communities matching a BGP [community filter](generic-routing-community).
 
 Routing policies are specified in the global- or node-level **routing.policy** dictionary (see [](routing-object-import) and  [](routing-object-merge) for more details).
 
@@ -156,12 +159,12 @@ Apart from setting BGP communities on BGP routes, these devices can execute addi
 | Cisco IOS/XE[^18v]  | ✅ | ✅❗️| ✅ |
 | Cumulus Linux       | ✅ | ❌  | ❌  |
 | Dell OS10           | ✅ | ❌  | ❌  |
-| Junos               | ✅ | ✅ | ❌  |
+| Junos               | ✅ | ✅ | ✅ |
 | FRR                 | ✅ | ✅❗️ | ✅ |
 | VyOS                | ✅ | ❌  | ❌  |
 
 **Notes:**
-* _netlab_ is creating an internal BGP community list on FRR and Cisco IOS/XE to delete BGP communities specified with the **set.community.delete** routing policy parameter. This approach is currently limited to standard BGP communities.
+* _netlab_ is creating an internal BGP community list on FRR and Cisco IOS/XE to delete BGP communities specified with the **delete.community** routing policy parameter. This approach is currently limited to standard BGP communities.
 
 ### Shortcut Routing Policy Definitions
 

netsim/ansible/templates/routing/_route_map_ios.j2

@@ -15,6 +15,15 @@ that use "industry standard" CLI (IOS, IOS-XE, FRR, Cumulus, Arista)
    Include OS-specific route map macros
 #}
 !
+{#
+   This macro generates community set command across "industry standard CLI" devices
+#}
+{% macro common_community_element(community,action) %}
+{%   set ckw = { 'standard': 'community', 'extended': 'extcommunity', 'large': 'large-community' } %}
+{%   for kw in ('standard','extended','large') if kw in community %}
+  set {{ ckw[kw] }} {{ community[kw]|join(' ') }}{{ action }}
+{%   endfor %}
+{% endmacro %}
 {#
    This macro generates the elements of route-map entries common across all "industry standard CLI" devices
 #}
@@ -34,14 +43,13 @@ that use "industry standard" CLI (IOS, IOS-XE, FRR, Cumulus, Arista)
 {%     endif %}
 {%     if 'community' in p_entry.set %}
 {%       set cset = p_entry.set.community %}
-{%       set ckw = { 'standard': 'community', 'extended': 'extcommunity', 'large': 'large-community' } %}
-{%         for kw in ('standard','extended','large') if kw in cset %}
-  set {{ ckw[kw] }} {{ cset[kw]|join(' ') if cset[kw] is not string else cset[kw] }}{{ 
-    ' additive' if cset.append|default(False) }}{{
-    ' delete' if cset.delete|default(False) }}
-{%         endfor %}
+{%       set action = ' additive' if cset.append|default(False) else '' %}
+{{       common_community_element(cset,action) }}
 {%     endif %}
 {%   endif %}
+{%   if p_entry.delete.community is defined %}
+{{     common_community_element(p_entry.delete.community,' delete') }}
+{%   endif %}
 {%   if 'match' in p_entry %}
 {%     set ipkwd = 'ip' if match_af == 'ipv4' else 'ipv6' %}
 {%     if 'prefix' in p_entry.match %}

netsim/ansible/templates/routing/_route_policy_junos.j2

@@ -6,7 +6,7 @@
 {%   endif %}
 {%- endmacro %}
 
-{% macro community_set(comm,c_append,c_delete) -%}
+{% macro community_set(comm,c_append=False,c_delete=False) -%}
 {% if c_delete %}
       then community delete x_comm_del_{{ comm|replace(':', '_')|replace('.', '_') }};
 {% elif c_append %}
@@ -28,23 +28,30 @@
 {%     if 'med' in p_entry.set %}
       then metric {{ p_entry.set.med }};
 {%     endif %}
-{%     if 'community' in p_entry.set %}
+{%     if p_entry.set.community is defined %}
 {%       set cset = p_entry.set.community %}
-{%         for kw in ('standard','extended','large') if kw in cset %}
-{%           if cset[kw] is not string %}
-{%             for cur_com in cset[kw] %}
-{{               community_set(cur_com, cset.append|default(False), cset.delete|default(False)) }}
-{%             endfor %}
-{%           else %}
-{{             community_set(cset[kw], cset.append|default(False), cset.delete|default(False)) }}
-{%           endif %}
+{%       for kw in ('standard','extended','large') if kw in cset %}
+{%         for cur_com in cset[kw] %}
+{{           community_set(cur_com,c_append=cset.append|default(False)) }}
 {%         endfor %}
+{%       endfor %}
 {%     endif %}
-
 {%     if 'prepend' in p_entry.set %}
 {{       prepend_set(p_entry.set.prepend,bgp.as|string) }}
 {%     endif %}
 {%   endif %}
+{# - DELETE entries #}
+{%   if p_entry.delete.community is defined %}
+{%     set dset = p_entry.delete.community %}
+{%     for kw in ('standard','extended','large') if kw in dset %}
+{%       for cur_com in dset[kw] %}
+{{         community_set(cur_com,c_delete=True) }}
+{%       endfor %}
+{%     endfor %}
+{%     if dset.list is defined %}
+      then community delete {{ dset.list }};
+{%     endif %}
+{%   endif %}
 {# - MATCH entries #}
 {%   if 'match' in p_entry %}
 {%     if 'prefix' in p_entry.match %}

netsim/ansible/templates/routing/dellos10.j2

@@ -8,10 +8,10 @@
 {% if routing.prefix|default({}) %}
 {%   include '_prefix_list_ios.j2' %}
 {% endif %}
+{% if routing.aspath|default({}) %}
 !
 ! AS-path access lists
 !
-{% if routing.aspath|default({}) %}
 {%   for asp_name,asp_list in routing.aspath.items() %}
 !
 {%     for asp_line in asp_list %}
@@ -28,10 +28,12 @@ ip community-list standard {{ c_name }} {{ c_line.action }} {{ c_line._value }}
 {%     endfor %}
 {%   endfor %}
 {% endif %}
+{% for sr_data in routing.static|default([]) %}
+{%   if loop.first %}
 !
 ! Static routes (dual routes for each entry, in entry VRF and next hop VRF)
 !
-{% for sr_data in routing.static|default([]) %}
+{%   endif %}
 {%   set _vrf = sr_data.vrf|default(None) %}
 {%   set _vrf_str = 'vrf ' + _vrf + ' ' if _vrf else '' %}
 {%   set _nh_vrf = sr_data.nexthop.vrf|default(None) %}

netsim/ansible/templates/routing/eos.j2

@@ -3,11 +3,11 @@
 !
 {% if routing.policy|default({}) %}
 {%   call(p_entry,af_list) routemap.create_route_maps(routing.policy) -%}
-{%     if 'bandwidth' in p_entry.set|default({}) %}
+{%     if p_entry.set.bandwidth is defined %}
   set extcommunity lbw {{ p_entry.set.bandwidth }}M
 {%     endif %}
-{%     if p_entry.set.community.delete_list|default(False) %}
-  set community community-list {{ p_entry.set.community.delete_list }} delete
+{%     if p_entry.delete.community.list is defined %}
+  set community community-list {{ p_entry.delete.community.list }} delete
 {%     endif %}
 {{     routemap.common_route_map_entry(p_entry,af_list) }}
 {%-   endcall %}
@@ -36,10 +36,10 @@ ipv6 prefix-list {{ p_name }}-ipv6
 {%     endfor %}
 {%   endfor %}
 {% endif %}
+{% if routing.aspath|default({}) %}
 !
 ! AS-path access lists
 !
-{% if routing.aspath|default({}) %}
 ip as-path regex-mode string
 {%   for asp_name,asp_list in routing.aspath.items() %}
 !
@@ -56,10 +56,12 @@ ip community-list {{ c_value.regexp }} {{ c_name }} {{ c_line.action }} {{ c_lin
 {%     endfor %}
 {%   endfor %}
 {% endif %}
+{% for sr_data in routing.static|default([]) %}
+{%   if loop.first %}
 !
 ! Static routes
 !
-{% for sr_data in routing.static|default([]) %}
+{%   endif %}
 {%   for sr_af in ['ipv4','ipv6'] if sr_af in sr_data %}
 {%     set cmd_af = 'ip' if sr_af == 'ipv4' else sr_af %}
 {%     set cmd_vrf = 'vrf ' + sr_data.vrf + ' ' if 'vrf' in sr_data else '' %}

netsim/ansible/templates/routing/frr.j2

@@ -2,13 +2,11 @@
 !
 {% if routing.policy|default({}) %}
 {%   call(p_entry,af_list) routemap.create_route_maps(routing.policy) -%}
-{%     if 'set' in p_entry %}
-{%       if 'bandwidth' in p_entry.set %}
+{%     if p_entry.set.bandwidth is defined %}
   set extcommunity bandwidth {{ p_entry.set.bandwidth }}
-{%       endif %}
-{%       if p_entry.set.community.delete_list|default(False) %}
-  set comm-list {{ p_entry.set.community.delete_list }} delete
-{%       endif %}
+{%     endif %}
+{%     if p_entry.delete.community.list is defined %}
+  set comm-list {{ p_entry.delete.community.list }} delete
 {%     endif %}
 {{     routemap.common_route_map_entry(p_entry,af_list) }}
 {%-   endcall %}

netsim/ansible/templates/routing/ios.j2

@@ -2,19 +2,19 @@
 !
 {% if routing.policy|default({}) %}
 {%   call(p_entry,af_list) routemap.create_route_maps(routing.policy) -%}
-{%     if p_entry.set.community.delete_list|default(False) %}
-  set comm-list {{ p_entry.set.community.delete_list }} delete
+{%     if p_entry.delete.community.list is defined %}
+  set comm-list {{ p_entry.delete.community.list }} delete
 {%     endif %}
 {{     routemap.common_route_map_entry(p_entry,af_list) }}
 {%-  endcall %} 
 {% endif %}
 {% if routing.prefix|default({}) %}
 {%   include '_prefix_list_ios.j2' %}
 {% endif %}
+{% if routing.aspath|default({}) %}
 !
 ! AS-path access lists
 !
-{% if routing.aspath|default({}) %}
 {%   for asp_name,asp_list in routing.aspath.items() %}
 !
 {%     for asp_line in asp_list %}
@@ -30,10 +30,12 @@ ip community-list {{ c_value.type }} {{ c_name }} {{ c_line.action }} {{ c_line.
 {%     endfor %}
 {%   endfor %}
 {% endif %}
+{% for sr_data in routing.static|default([]) %}
+{%   if loop.first %}
 !
 ! Static routes
 !
-{% for sr_data in routing.static|default([]) %}
+{%   endif %}
 {%   set cmd_vrf = 'vrf ' + sr_data.vrf + ' ' if 'vrf' in sr_data else '' %}
 {%   set cmd_intf = sr_data.nexthop.intf + ' ' if 'intf' in sr_data.nexthop else '' %}
 {%   set leak_global = ' global' if sr_data.nexthop.vrf is defined and sr_data.nexthop.vrf is none else '' %}

netsim/devices/arubacx.yml

@@ -76,8 +76,10 @@ features:
         prepend: True
         community:
           standard: True
-          delete: True
           append: True
+      delete:
+        community:
+          standard: True
       match: [ prefix, nexthop, aspath, community ]
     prefix: True
     aspath: True

netsim/devices/dellos10.yml

@@ -55,7 +55,7 @@ features:
           large: False
           extended: True
           append: True
-          delete: False
+      delete.community: False
     prefix: True
     static:
       vrf: True

netsim/devices/eos.yml

@@ -90,6 +90,8 @@ features:
     policy:
       set: [ locpref, med, weight, prepend, community ]
       match: [ prefix, nexthop, aspath, community ]
+      delete:
+        community: True
     prefix: True
     aspath: True
     community: