From bd4299732dbc94447da19968ffb8fa90969e7183 Mon Sep 17 00:00:00 2001
From: Timotej Lazar <timotej.lazar@fri.uni-lj.si>
Date: Thu, 14 Aug 2025 17:09:47 +0200
Subject: [PATCH 1/4] =?UTF-8?q?leaf:=20don=E2=80=99t=20activate=20disabled?=
 =?UTF-8?q?=20interfaces=20in=20the=20EVPN=20family?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 roles/leaf/templates/frr.conf.j2 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/roles/leaf/templates/frr.conf.j2 b/roles/leaf/templates/frr.conf.j2
index a30748b..8349ee5 100644
--- a/roles/leaf/templates/frr.conf.j2
+++ b/roles/leaf/templates/frr.conf.j2
@@ -55,8 +55,8 @@ router bgp {{ asn.asn }}
 {% endfor %}
   address-family l2vpn evpn
     neighbor fabric activate
-{% for iface in ifaces_evpn|default([]) %}
-    neighbor {{ iface }} activate
+{% for iface in interfaces | selectattr('enabled') | selectattr('name', 'in', ifaces_evpn|default([])) %}
+    neighbor {{ iface.name }} activate
 {% endfor %}
 {% if peer is defined and  interfaces | selectattr('mode') %}
     advertise-all-vni

From 6ade4f2f8a5ae2aa49c5af1006276d72b36519a6 Mon Sep 17 00:00:00 2001
From: Timotej Lazar <timotej.lazar@fri.uni-lj.si>
Date: Thu, 18 Sep 2025 13:45:16 +0200
Subject: [PATCH 2/4] access: fix VLAN database idempotency

Do not try and match the global VLAN list as printed by the switch.
Instead, only try to realize the truth: there may be some VLANs added
and some removed.

We keep the compact_numlist filter and use it instead of the built-in
vlan_parser when listing VLANs for tagged ports. This is because some
switches compact 1,2,4,5,6 as 1-2,4-6 and others as 1,2,4-6 (see next
commit).

All of this should reduce the number of cases where Ansible reports a
change in configuration where there was in fact no change.
---
 filter_plugins/netbox.py                | 42 +++++++++++--------------
 roles/access/tasks/d-link.yml           |  4 ---
 roles/access/tasks/main.yml             | 14 +++++++++
 roles/access/templates/config-d-link.j2 |  7 ++++-
 roles/access/templates/config-fs.j2     |  7 +++--
 5 files changed, 43 insertions(+), 31 deletions(-)

diff --git a/filter_plugins/netbox.py b/filter_plugins/netbox.py
index 02ad61a..0762cc1 100644
--- a/filter_plugins/netbox.py
+++ b/filter_plugins/netbox.py
@@ -14,36 +14,30 @@ class FilterModule(object):
             'iface_vlans': self.iface_vlans
         }
 
-    def compact_numlist(self, nums, sort=True, delimiter=',', range_delimiter='-', max_per_line=None):
+    def compact_numlist(self, nums, sort=True, delimiter=',', range_delimiter='-', min_join=2):
         '''Transform [1,2,3,5,7,8,9] into "1-3,5,7-9".
 
-        If max_per_line is given, return a list of such strings where each string contains at most max_per_line+1 numbers.
-        This emulates how VLAN ranges are displayed by FS switches so we can make ansible check mode work correctly.
+        Do not create a range from fewer than min_join consecutive numbers.
         '''
         if sort:
             nums = sorted(nums)
-        i = 0
-        lines = []
-        line = []
-        nums_in_line = 0
-        while i < len(nums):
-            j = i + 1
-            while j < len(nums) and nums[j]-nums[i] == j-i:
-                j += 1
-            if j > i+1:
-                line += [f'{nums[i]}{range_delimiter}{nums[j-1]}']
-                nums_in_line += 2
+        ranges = []
+        r = []
+        for num in nums:
+            if r and num > r[-1]+1:
+                ranges += [r]
+                r = []
+            r += [num]
+        if r:
+            ranges += [r]
+
+        def format_range(r):
+            if len(r) < min_join:
+                return delimiter.join(str(n) for n in r)
             else:
-                line += [f'{nums[i]}']
-                nums_in_line += 1
-            if max_per_line and nums_in_line >= max_per_line:
-                lines += [delimiter.join(line)]
-                line = []
-                nums_in_line = 0
-            i = j
-        if line:
-            lines += [delimiter.join(line)]
-        return lines if max_per_line else lines[0]
+                return f'{r[0]}{range_delimiter}{r[-1]}'
+
+        return delimiter.join(format_range(r) for r in ranges)
 
     def device_address(self, device):
         '''Return loopback IP addresses for an L3 attached device'''
diff --git a/roles/access/tasks/d-link.yml b/roles/access/tasks/d-link.yml
index 413fe87..b9f525c 100644
--- a/roles/access/tasks/d-link.yml
+++ b/roles/access/tasks/d-link.yml
@@ -13,10 +13,6 @@
   set_fact:
     snmp_hashes: '{{ (snmp_config.stdout | from_yaml).snmpv3.hashes }}'
 
-- name: Get switch facts
-  cisco.ios.ios_facts:
-    gather_subset: config
-
 - name: Get SNMP users
   set_fact:
     snmp_current: "{{ ansible_net_config | split('\n') | select('match', '^snmp-server user '+manager.snmp_user+' public v3') }}"
diff --git a/roles/access/tasks/main.yml b/roles/access/tasks/main.yml
index 0b759f8..eee6521 100644
--- a/roles/access/tasks/main.yml
+++ b/roles/access/tasks/main.yml
@@ -11,6 +11,20 @@
   set_fact:
     snmp_engine_id: '{{ (serial | sha1)[:24] }}'
 
+- name: Get switch facts
+  cisco.ios.ios_facts:
+    gather_subset: config
+
+# Determine VLANs to add and remove from switch.
+- set_fact:
+    actual_vlans: "{{ vlans | map(attribute='vid') }}"
+    switch_vlans: "{{ ansible_net_config | split('\n') | select('match', '^ *vlan (range )?[0-9]')
+        | map('regex_search', '[0-9,-]+') | join(',') | ansible.netcommon.vlan_expander }}"
+
+- set_fact:
+    add_vlans: "{{ actual_vlans | difference(switch_vlans) }}"
+    del_vlans: "{{ switch_vlans | difference(actual_vlans) }}"
+
 - name: Set configuration
   ansible.netcommon.cli_config:
     config: '{{ lookup("template", "config-"~manufacturer~"-"~device_type~".j2") }}'
diff --git a/roles/access/templates/config-d-link.j2 b/roles/access/templates/config-d-link.j2
index 1a4b3b5..78680f0 100644
--- a/roles/access/templates/config-d-link.j2
+++ b/roles/access/templates/config-d-link.j2
@@ -10,7 +10,12 @@ port-channel load-balance src-dst-ip
 
 ip ssh server
 
-vlan {{ vlans | map(attribute='vid') | compact_numlist }}
+{% for vlan in add_vlans %}
+vlan {{ vlan }}
+{% endfor %}
+{% for vlan in del_vlans %}
+no vlan {{ vlan }}
+{% endfor %}
 
 {# bond members #}
 {% for iface in interfaces | selectattr('lag') %}
diff --git a/roles/access/templates/config-fs.j2 b/roles/access/templates/config-fs.j2
index cc17f14..036b709 100644
--- a/roles/access/templates/config-fs.j2
+++ b/roles/access/templates/config-fs.j2
@@ -6,8 +6,11 @@ no enable service telnet-server
 no enable service web-server http
 no enable service web-server https
 
-{% for vlan_range in vlans | map(attribute='vid') | union([1]) | compact_numlist(max_per_line=19) %}
-vlan range {{ vlan_range }}
+{% for vlan in add_vlans %}
+ vlan {{ vlan }}
+{% endfor %}
+{% for vlan in del_vlans | difference([1]) %} {# VLAN 1 can not be deleted #}
+ no vlan {{ vlan }}
 {% endfor %}
 
 {% for iface in interfaces %}

From 9ec6241e4a5e77c715b49a67329800202a5c2747 Mon Sep 17 00:00:00 2001
From: Timotej Lazar <timotej.lazar@fri.uni-lj.si>
Date: Thu, 18 Sep 2025 16:47:39 +0200
Subject: [PATCH 3/4] access: fix parsing of existing switch VLANs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If ansible won’t handle edge cases then we must.
---
 roles/access/tasks/main.yml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/roles/access/tasks/main.yml b/roles/access/tasks/main.yml
index eee6521..069784c 100644
--- a/roles/access/tasks/main.yml
+++ b/roles/access/tasks/main.yml
@@ -18,8 +18,9 @@
 # Determine VLANs to add and remove from switch.
 - set_fact:
     actual_vlans: "{{ vlans | map(attribute='vid') }}"
-    switch_vlans: "{{ ansible_net_config | split('\n') | select('match', '^ *vlan (range )?[0-9]')
-        | map('regex_search', '[0-9,-]+') | join(',') | ansible.netcommon.vlan_expander }}"
+    switch_vlans: "{{ ansible_net_config | split('\n')
+        | select('match', '^ *vlan (range )?[0-9]') | map('regex_search', '[0-9,-]+') | join(',')
+        | default('0', true) |  ansible.netcommon.vlan_expander | reject('eq', 0) }}" # vlan_expander barfs on empty string so add/remove a fake VLAN 0
 
 - set_fact:
     add_vlans: "{{ actual_vlans | difference(switch_vlans) }}"

From 343fd0daad5742950b6ca264c43fc403448ae6d6 Mon Sep 17 00:00:00 2001
From: Timotej Lazar <timotej.lazar@fri.uni-lj.si>
Date: Fri, 19 Sep 2025 10:56:20 +0200
Subject: [PATCH 4/4] access: add support for the FS S5800 switch

---
 roles/access/tasks/fs-s5800-48t4s.yml         |  1 +
 roles/access/tasks/main.yml                   |  2 +-
 ...{config-fs.j2 => config-fs-s5800-48t4s.j2} | 43 +++++++-------
 .../templates/config-fs-s5860-48xmg-u.j2      | 58 ++++++++++++++++++-
 4 files changed, 80 insertions(+), 24 deletions(-)
 create mode 120000 roles/access/tasks/fs-s5800-48t4s.yml
 rename roles/access/templates/{config-fs.j2 => config-fs-s5800-48t4s.j2} (57%)
 mode change 120000 => 100644 roles/access/templates/config-fs-s5860-48xmg-u.j2

diff --git a/roles/access/tasks/fs-s5800-48t4s.yml b/roles/access/tasks/fs-s5800-48t4s.yml
new file mode 120000
index 0000000..b136bed
--- /dev/null
+++ b/roles/access/tasks/fs-s5800-48t4s.yml
@@ -0,0 +1 @@
+fs.yml
\ No newline at end of file
diff --git a/roles/access/tasks/main.yml b/roles/access/tasks/main.yml
index 069784c..c06a8be 100644
--- a/roles/access/tasks/main.yml
+++ b/roles/access/tasks/main.yml
@@ -34,7 +34,7 @@
     ansible_terminal_stderr_re: [] # some errors are not actually errors
   register: result
   # These lines are not displayed by 'sho ru' and always reported as different, so ignore them.
-  changed_when: result.commands | reject('match', '^(no shutdown|no switchport access vlan|no switchport trunk native vlan|no voice vlan.*|switchport mode access|switchport mode hybrid|interface .*|no enable service web-server https?|no ip dhcp snooping|no ip dhcp snooping trust|no switchport port-security.*)$')
+  changed_when: result.commands | reject('match', '^(no shutdown|no switchport access vlan|no switchport trunk native vlan|no voice vlan.*|switchport mode access|switchport mode hybrid|interface .*|service http disable|no enable service web-server https?|no ip dhcp snooping|no ip dhcp snooping trust|no switchport port-security.*)$')
   notify: write config
 
 - name: Run model-specific tasks
diff --git a/roles/access/templates/config-fs.j2 b/roles/access/templates/config-fs-s5800-48t4s.j2
similarity index 57%
rename from roles/access/templates/config-fs.j2
rename to roles/access/templates/config-fs-s5800-48t4s.j2
index 036b709..3d5c7e9 100644
--- a/roles/access/templates/config-fs.j2
+++ b/roles/access/templates/config-fs-s5800-48t4s.j2
@@ -1,43 +1,41 @@
 hostname {{ inventory_hostname }}
 
-no netconf enable
-
-no enable service telnet-server
-no enable service web-server http
-no enable service web-server https
+service http disable
+service telnet disable
 
+vlan database
 {% for vlan in add_vlans %}
  vlan {{ vlan }}
 {% endfor %}
-{% for vlan in del_vlans | difference([1]) %} {# VLAN 1 can not be deleted #}
+{% for vlan in del_vlans %}
  no vlan {{ vlan }}
 {% endfor %}
+exit
 
-{% for iface in interfaces %}
-interface {{ iface.name }}
-{% if iface.enabled %} no{% endif %} shutdown
-{% if iface.lag %}
- port-group {{ iface.lag.name | select('in', '0123456789') | join('') }} mode active
-
-{% elif iface.mgmt_only %}
+{# sort to ensure LAG interfaces are added last #}
+{% for iface in interfaces | sort(attribute="type.value") | sort(attribute="mgmt_only") %}
+{% if iface.mgmt_only %}
 {% for address in iface.ip_addresses %}
 {% set subnet = address.address | ipaddr('subnet') %}
 {% set prefix = prefixes | selectattr('prefix', '==', subnet) | first %}
 {% if address.family.value == 4 %}
- ip address {{ address.address | ipaddr('address') }} {{ address.address | ipaddr('netmask') }}
+management ip address {{ address.address }}
 {% if prefix.custom_fields.gateway %}
- gateway {{ prefix.custom_fields.gateway.address | ipaddr('address') }}
+management route add gateway {{ prefix.custom_fields.gateway.address | ipaddr('address') }}
 {% endif %}
 {% else %}
- ipv6 address {{ address.address | upper }}
-{% if prefix.custom_fields.gateway %}
- ipv6 gateway {{ prefix.custom_fields.gateway.address | ipaddr('address') | upper }}
-{% endif %}
+management ipv6 address {{ address.address }}
 {% endif %}
 {% endfor %}
 
 {% else %}
- mtu {{ iface.mtu | default('9216', true) }}
+
+interface {{ iface.name }}
+{% if iface.enabled %} no{% endif %} shutdown
+{% if iface.lag %}
+ channel-group {{ iface.lag.name | select('in', '0123456789') | join('') }} mode active
+
+{% else %}
 {% if iface.mode and iface.mode.value == 'access' %}
  switchport mode access
 {% if iface.untagged_vlan and iface.untagged_vlan.vid != 1 %}
@@ -49,9 +47,10 @@ interface {{ iface.name }}
  switchport mode trunk
  switchport trunk allowed vlan only {{ (iface.tagged_vlans or vlans) | map(attribute='vid') | compact_numlist }}
 {%- elif iface.mode and iface.mode.value == 'tagged-all' %}
- switchport mode uplink
- switchport trunk allowed vlan only 2-4094
+ switchport mode trunk
+ switchport trunk allowed vlan all
 {% endif %}
 
+{% endif %}
 {% endif %}
 {% endfor %}
diff --git a/roles/access/templates/config-fs-s5860-48xmg-u.j2 b/roles/access/templates/config-fs-s5860-48xmg-u.j2
deleted file mode 120000
index ffb3016..0000000
--- a/roles/access/templates/config-fs-s5860-48xmg-u.j2
+++ /dev/null
@@ -1 +0,0 @@
-config-fs.j2
\ No newline at end of file
diff --git a/roles/access/templates/config-fs-s5860-48xmg-u.j2 b/roles/access/templates/config-fs-s5860-48xmg-u.j2
new file mode 100644
index 0000000..036b709
--- /dev/null
+++ b/roles/access/templates/config-fs-s5860-48xmg-u.j2
@@ -0,0 +1,57 @@
+hostname {{ inventory_hostname }}
+
+no netconf enable
+
+no enable service telnet-server
+no enable service web-server http
+no enable service web-server https
+
+{% for vlan in add_vlans %}
+ vlan {{ vlan }}
+{% endfor %}
+{% for vlan in del_vlans | difference([1]) %} {# VLAN 1 can not be deleted #}
+ no vlan {{ vlan }}
+{% endfor %}
+
+{% for iface in interfaces %}
+interface {{ iface.name }}
+{% if iface.enabled %} no{% endif %} shutdown
+{% if iface.lag %}
+ port-group {{ iface.lag.name | select('in', '0123456789') | join('') }} mode active
+
+{% elif iface.mgmt_only %}
+{% for address in iface.ip_addresses %}
+{% set subnet = address.address | ipaddr('subnet') %}
+{% set prefix = prefixes | selectattr('prefix', '==', subnet) | first %}
+{% if address.family.value == 4 %}
+ ip address {{ address.address | ipaddr('address') }} {{ address.address | ipaddr('netmask') }}
+{% if prefix.custom_fields.gateway %}
+ gateway {{ prefix.custom_fields.gateway.address | ipaddr('address') }}
+{% endif %}
+{% else %}
+ ipv6 address {{ address.address | upper }}
+{% if prefix.custom_fields.gateway %}
+ ipv6 gateway {{ prefix.custom_fields.gateway.address | ipaddr('address') | upper }}
+{% endif %}
+{% endif %}
+{% endfor %}
+
+{% else %}
+ mtu {{ iface.mtu | default('9216', true) }}
+{% if iface.mode and iface.mode.value == 'access' %}
+ switchport mode access
+{% if iface.untagged_vlan and iface.untagged_vlan.vid != 1 %}
+ switchport access vlan {{ iface.untagged_vlan.vid }}
+{% else %}
+ no switchport access vlan
+{% endif %}
+{%- elif iface.mode and iface.mode.value == 'tagged' %}
+ switchport mode trunk
+ switchport trunk allowed vlan only {{ (iface.tagged_vlans or vlans) | map(attribute='vid') | compact_numlist }}
+{%- elif iface.mode and iface.mode.value == 'tagged-all' %}
+ switchport mode uplink
+ switchport trunk allowed vlan only 2-4094
+{% endif %}
+
+{% endif %}
+{% endfor %}