T7162: interface preflight (#397)

* T7162: interface preflight

* fix: update with changes from PR 396

* fix: re-run pre-commit for missed updates

* fix: typo

14 files changed, 105 insertions, 52 deletions

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index cab10556..d5617c09 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -12,7 +12,7 @@ repos:
       - id: update-docs
 
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.6.0
+    rev: v5.0.0
     hooks:
       - id: check-merge-conflict
       - id: check-symlinks
@@ -39,18 +39,18 @@ repos:
           - prettier-plugin-toml
 
   - repo: https://github.com/PyCQA/isort
-    rev: 5.13.2
+    rev: 6.0.0
     hooks:
       - id: isort
         name: Sort import statements using isort
         args: [--filter-files]
 
   - repo: https://github.com/psf/black
-    rev: 24.4.2
+    rev: 25.1.0
     hooks:
       - id: black
 
   - repo: https://github.com/pycqa/flake8
-    rev: 7.1.0
+    rev: 7.1.2
     hooks:
       - id: flake8
diff --git a/README.md b/README.md
index 5d8e1e8b..6650cb67 100644
--- a/README.md
+++ b/README.md
@@ -210,12 +210,15 @@ Additionally:
   on the local network
 - eth1 and eth2 should be defined and uncofirgured (they'll be overwritten by the tests)
 - eth3 and beyond should not be present or interface-related tests will fail
+- when using VMs for testing, ensure that the interfaces don't use `virtio`, as it will supress
+  some interface configurations. `e1000e` is a good choice for testing.
 - eth0 is also expected to show `duplex auto` and `speed auto` in the output of `show interfaces`,
   however others are not due to the fact that they are repeatedly deleted and recreated
   which causes the default values to be hidden.
 
 ## Changelogs
-<!--Add a link to a changelog.md file or an external docsite to cover this information. -->
+
+Change logs are available [here](https://github.com/vyos/vyos.vyos/blob/main/CHANGELOG.rst).
 
 ## Release notes
 
diff --git a/changelogs/fragments/T7162-interface-preflight.yaml b/changelogs/fragments/T7162-interface-preflight.yaml
new file mode 100644
index 00000000..c829ca8b
--- /dev/null
+++ b/changelogs/fragments/T7162-interface-preflight.yaml
@@ -0,0 +1,5 @@
+---
+trivial:
+  - fix pre-flight sets for interfaces
+  - restore previously-removed interface test duplex and speed
+  - remove unnecessary debug statements in integration tests
diff --git a/plugins/module_utils/network/vyos/config/firewall_global/firewall_global.py b/plugins/module_utils/network/vyos/config/firewall_global/firewall_global.py
index b7bff53e..0d73d209 100644
--- a/plugins/module_utils/network/vyos/config/firewall_global/firewall_global.py
+++ b/plugins/module_utils/network/vyos/config/firewall_global/firewall_global.py
@@ -31,11 +31,11 @@ from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.utils.utils
     list_diff_want_only,
     in_target_not_none,
 )
-
+from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.utils.version import (
+    LooseVersion,
+)
 from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.vyos import get_os_version
 
-from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.utils.version import LooseVersion
-
 
 class Firewall_global(ConfigBase):
     """
@@ -258,11 +258,7 @@ class Firewall_global(ConfigBase):
                             self._form_attr_cmd(attr=key, key=self._bool_to_str(val), opr=opr),
                         )
                         continue
-                    if (
-                        key in l_set
-                        and not self._in_target(h, key)
-                        and not self._is_del(l_set, h)
-                    ):
+                    if key in l_set and not self._in_target(h, key) and not self._is_del(l_set, h):
                         commands.append(
                             self._form_attr_cmd(attr=key, val=self._bool_to_str(val), opr=opr),
                         )
@@ -483,7 +479,9 @@ class Firewall_global(ConfigBase):
                     for key, val in iteritems(w):
                         if val and key != "connection_type":
                             if opr and key in l_set and not (h and self._is_w_same(w, h, key)):
-                                if key == "log" and LooseVersion(get_os_version(self._module)) >= LooseVersion("1.4"):
+                                if key == "log" and LooseVersion(
+                                    get_os_version(self._module),
+                                ) >= LooseVersion("1.4"):
                                     commands.append(
                                         self._form_attr_cmd(
                                             key=attr + " " + w["connection_type"],
@@ -509,7 +507,9 @@ class Firewall_global(ConfigBase):
                                         ),
                                     )
                                     break  # delete the whole thing and move on
-                                if (not self._in_target(h, key) or h[key] is None) and (self._in_target(w, key) and w[key]):
+                                if (not self._in_target(h, key) or h[key] is None) and (
+                                    self._in_target(w, key) and w[key]
+                                ):
                                     # delete if not being replaced and value currently exists
                                     commands.append(
                                         self._form_attr_cmd(
@@ -541,11 +541,11 @@ class Firewall_global(ConfigBase):
         if want:
             for w in want:
                 h = self.search_attrib_in_have(have, w, "afi")
-                if 'afi' in w:
-                    afi = w['afi']
+                if "afi" in w:
+                    afi = w["afi"]
                 else:
-                    if h and 'afi' in h:
-                        afi = h['afi']
+                    if h and "afi" in h:
+                        afi = h["afi"]
                     else:
                         afi = None
                     afi = None
@@ -557,7 +557,7 @@ class Firewall_global(ConfigBase):
                                     attr=key,
                                     val=self._bool_to_str(val),
                                     opr=opr,
-                                    type=afi
+                                    type=afi,
                                 ),
                             )
                         elif not opr and key in l_set:
@@ -567,7 +567,7 @@ class Firewall_global(ConfigBase):
                                         attr=key,
                                         val=self._bool_to_str(val),
                                         opr=opr,
-                                        type=afi
+                                        type=afi,
                                     ),
                                 )
                                 continue
@@ -577,7 +577,7 @@ class Firewall_global(ConfigBase):
                                         attr=key,
                                         val=self._bool_to_str(val),
                                         opr=opr,
-                                        type=afi
+                                        type=afi,
                                     ),
                                 )
                         elif key == "icmp_redirects":
@@ -597,11 +597,11 @@ class Firewall_global(ConfigBase):
         commands = []
         h_red = {}
         l_set = ("send", "receive")
-        if w and 'afi' in w:
-            afi = w['afi']
+        if w and "afi" in w:
+            afi = w["afi"]
         else:
-            if h and 'afi' in h:
-                afi = h['afi']
+            if h and "afi" in h:
+                afi = h["afi"]
             else:
                 afi = None
         if w[attr]:
@@ -610,7 +610,12 @@ class Firewall_global(ConfigBase):
             for item, value in iteritems(w[attr]):
                 if opr and item in l_set and not (h_red and self._is_w_same(w[attr], h_red, item)):
                     commands.append(
-                        self._form_attr_cmd(attr=item, val=self._bool_to_str(value), opr=opr, type=afi)
+                        self._form_attr_cmd(
+                            attr=item,
+                            val=self._bool_to_str(value),
+                            opr=opr,
+                            type=afi,
+                        ),
                     )
                 elif (
                     not opr
@@ -644,7 +649,12 @@ class Firewall_global(ConfigBase):
         :param type: AF type of attribute.
         :return: generated command.
         """
-        command = self._compute_command(key=key, attr=self._map_attrib(attr, type=type), val=val, opr=opr)
+        command = self._compute_command(
+            key=key,
+            attr=self._map_attrib(attr, type=type),
+            val=val,
+            opr=opr,
+        )
         return command
 
     def _compute_command(self, key=None, attr=None, val=None, remove=False, opr=True):
@@ -661,14 +671,20 @@ class Firewall_global(ConfigBase):
             cmd = "delete firewall "
         else:
             cmd = "set firewall "
-        if attr and key != "group" and LooseVersion(get_os_version(self._module)) >= LooseVersion("1.4"):
+        if (
+            attr
+            and key != "group"
+            and LooseVersion(get_os_version(self._module)) >= LooseVersion("1.4")
+        ):
             cmd += "global-options "
         if key:
             cmd += key.replace("_", "-") + " "
         if attr:
             cmd += attr.replace("_", "-")
         if val and opr:
-            if key == "state_policy" and LooseVersion(get_os_version(self._module)) >= LooseVersion("1.4"):
+            if key == "state_policy" and LooseVersion(get_os_version(self._module)) >= LooseVersion(
+                "1.4",
+            ):
                 cmd += ""
             else:
                 cmd += " '" + str(val) + "'"
diff --git a/tests/integration/targets/vyos_interfaces/tests/cli/_populate.yaml b/tests/integration/targets/vyos_interfaces/tests/cli/_populate.yaml
index 45bd9b6a..cbc994ba 100644
--- a/tests/integration/targets/vyos_interfaces/tests/cli/_populate.yaml
+++ b/tests/integration/targets/vyos_interfaces/tests/cli/_populate.yaml
@@ -9,6 +9,8 @@
     config: |-
       {% for intf in ('eth1','eth2') %}
       set interfaces ethernet "{{ intf }}" description 'Configured by Ansible'
+      set interfaces ethernet "{{ intf }}" speed 'auto'
+      set interfaces ethernet "{{ intf }}" duplex 'auto'
       set interfaces ethernet "{{ intf }}" mtu '1500'
       set interfaces ethernet "{{ intf }}" vif 200
       set interfaces ethernet "{{ intf }}" vif 200 description 'VIF - 200'
diff --git a/tests/integration/targets/vyos_interfaces/tests/cli/deleted.yaml b/tests/integration/targets/vyos_interfaces/tests/cli/deleted.yaml
index 620bf53f..9f0734b4 100644
--- a/tests/integration/targets/vyos_interfaces/tests/cli/deleted.yaml
+++ b/tests/integration/targets/vyos_interfaces/tests/cli/deleted.yaml
@@ -17,17 +17,17 @@
     - name: Assert that the before dicts were correctly generated
       assert:
         that:
-          - "{{ populate | symmetric_difference(result['before']) |length == 0 }}"
+          - populate | symmetric_difference(result['before']) |length == 0
 
     - name: Assert that the correct set of commands were generated
       assert:
         that:
-          - "{{ deleted['commands'] | symmetric_difference(result['commands']) |length == 0 }}"
+          - deleted['commands'] | symmetric_difference(result['commands']) |length == 0
 
     - name: Assert that the after dicts were correctly generated
       assert:
         that:
-          - "{{ deleted['after'] | symmetric_difference(result['after']) |length == 0 }}"
+          - deleted['after'] | symmetric_difference(result['after']) |length == 0
 
     - name: Delete attributes of given interfaces (IDEMPOTENT)
       register: result
@@ -41,6 +41,6 @@
     - name: Assert that the before dicts were correctly generated
       assert:
         that:
-          - "{{ deleted['after'] | symmetric_difference(result['before']) |length == 0 }}"
+          - deleted['after'] | symmetric_difference(result['before']) |length == 0
   always:
     - include_tasks: _remove_config.yaml
diff --git a/tests/integration/targets/vyos_interfaces/tests/cli/gathered.yaml b/tests/integration/targets/vyos_interfaces/tests/cli/gathered.yaml
index 88e53762..46a0e166 100644
--- a/tests/integration/targets/vyos_interfaces/tests/cli/gathered.yaml
+++ b/tests/integration/targets/vyos_interfaces/tests/cli/gathered.yaml
@@ -14,7 +14,7 @@
     - name: Assert that gathered dicts was correctly generated
       assert:
         that:
-          - "{{ populate | symmetric_difference(result['gathered']) |length == 0 }}"
+          - populate | symmetric_difference(result['gathered']) |length == 0
 
   always:
     - include_tasks: _remove_config.yaml
diff --git a/tests/integration/targets/vyos_interfaces/tests/cli/merged.yaml b/tests/integration/targets/vyos_interfaces/tests/cli/merged.yaml
index 5c719b39..decdeca2 100644
--- a/tests/integration/targets/vyos_interfaces/tests/cli/merged.yaml
+++ b/tests/integration/targets/vyos_interfaces/tests/cli/merged.yaml
@@ -12,6 +12,8 @@
           - name: eth1
             description: Configured by Ansible - Interface 1
             mtu: 1500
+            speed: auto
+            duplex: auto
             vifs:
               - vlan_id: 100
                 description: Eth1 - VIF 100
@@ -28,17 +30,17 @@
 
     - name: Assert that before dicts were correctly generated
       assert:
-        that: "{{ merged['before'] | symmetric_difference(result['before']) |length == 0 }}"
+        that: merged['before'] | symmetric_difference(result['before']) |length == 0
 
     - name: Assert that correct set of commands were generated
       assert:
         that:
-          - "{{ merged['commands'] | symmetric_difference(result['commands']) |length == 0 }}"
+          - merged['commands'] | symmetric_difference(result['commands']) |length == 0
 
     - name: Assert that after dicts was correctly generated
       assert:
         that:
-          - "{{ merged['after'] | symmetric_difference(result['after']) |length == 0 }}"
+          - merged['after'] | symmetric_difference(result['after']) |length == 0
 
     - name: Merge the provided configuration with the existing running configuration (IDEMPOTENT)
       register: result
@@ -52,6 +54,6 @@
     - name: Assert that before dicts were correctly generated
       assert:
         that:
-          - "{{ merged['after'] | symmetric_difference(result['before']) |length == 0 }}"
+          - merged['after'] | symmetric_difference(result['before']) |length == 0
   always:
     - include_tasks: _remove_config.yaml
diff --git a/tests/integration/targets/vyos_interfaces/tests/cli/overridden.yaml b/tests/integration/targets/vyos_interfaces/tests/cli/overridden.yaml
index 7e86d3e4..5d87ab68 100644
--- a/tests/integration/targets/vyos_interfaces/tests/cli/overridden.yaml
+++ b/tests/integration/targets/vyos_interfaces/tests/cli/overridden.yaml
@@ -21,17 +21,17 @@
     - name: Assert that before dicts were correctly generated
       assert:
         that:
-          - "{{ populate | symmetric_difference(result['before']) |length == 0 }}"
+          - populate | symmetric_difference(result['before']) |length == 0
 
     - name: Assert that correct commands were generated
       assert:
         that:
-          - "{{ overridden['commands'] | symmetric_difference(result['commands']) |length == 0 }}"
+          - overridden['commands'] | symmetric_difference(result['commands']) |length == 0
 
     - name: Assert that after dicts were correctly generated
       assert:
         that:
-          - "{{ overridden['after'] | symmetric_difference(result['after']) |length == 0 }}"
+          - overridden['after'] | symmetric_difference(result['after']) |length == 0
 
     - name: Overrides all device configuration with provided configurations (IDEMPOTENT)
       register: result
@@ -45,6 +45,6 @@
     - name: Assert that before dicts were correctly generated
       assert:
         that:
-          - "{{ overridden['after'] | symmetric_difference(result['before']) |length == 0 }}"
+          - overridden['after'] | symmetric_difference(result['before']) |length == 0
   always:
     - include_tasks: _remove_config.yaml
diff --git a/tests/integration/targets/vyos_interfaces/tests/cli/parsed.yaml b/tests/integration/targets/vyos_interfaces/tests/cli/parsed.yaml
index 0ebfd322..329d6b50 100644
--- a/tests/integration/targets/vyos_interfaces/tests/cli/parsed.yaml
+++ b/tests/integration/targets/vyos_interfaces/tests/cli/parsed.yaml
@@ -11,4 +11,4 @@
 - name: Assert that config was correctly parsed
   assert:
     that:
-      - "{{ parsed['after'] | symmetric_difference(result['parsed']) |length == 0 }}"
+      - parsed['after'] | symmetric_difference(result['parsed']) |length == 0
diff --git a/tests/integration/targets/vyos_interfaces/tests/cli/rendered.yaml b/tests/integration/targets/vyos_interfaces/tests/cli/rendered.yaml
index c03347fa..7b66c50e 100644
--- a/tests/integration/targets/vyos_interfaces/tests/cli/rendered.yaml
+++ b/tests/integration/targets/vyos_interfaces/tests/cli/rendered.yaml
@@ -11,9 +11,13 @@
         config:
           - name: eth0
             enabled: true
+            duplex: auto
+            speed: auto
           - name: eth1
             description: Configured by Ansible - Interface 1
             mtu: 1500
+            duplex: auto
+            speed: auto
             enabled: true
             vifs:
               - vlan_id: 100
diff --git a/tests/integration/targets/vyos_interfaces/tests/cli/replaced.yaml b/tests/integration/targets/vyos_interfaces/tests/cli/replaced.yaml
index 9d0a3a8b..5cfa4523 100644
--- a/tests/integration/targets/vyos_interfaces/tests/cli/replaced.yaml
+++ b/tests/integration/targets/vyos_interfaces/tests/cli/replaced.yaml
@@ -25,17 +25,11 @@
         that:
           - replaced['commands'] | symmetric_difference(result['commands']) |length == 0
 
-    - debug:
-        var: populate | symmetric_difference(result['before'])
-
     - name: Assert that before dicts are correctly generated
       assert:
         that:
           - populate | symmetric_difference(result['before']) |length == 0
 
-    - debug:
-        var: replaced['after'] | symmetric_difference(result['after'])
-
     - name: Assert that after dict is correctly generated
       assert:
         that:
diff --git a/tests/integration/targets/vyos_interfaces/tests/cli/rtt.yaml b/tests/integration/targets/vyos_interfaces/tests/cli/rtt.yaml
index cfe1b0f0..e6753cba 100644
--- a/tests/integration/targets/vyos_interfaces/tests/cli/rtt.yaml
+++ b/tests/integration/targets/vyos_interfaces/tests/cli/rtt.yaml
@@ -11,6 +11,8 @@
         config:
           - name: eth0
             enabled: true
+            duplex: auto
+            speed: auto
 
           - name: eth1
             description: Interface - 1
@@ -62,7 +64,7 @@
 
     - name: Assert that changes were applied
       assert:
-        that: "{{ round_trip['after'] | symmetric_difference(result['after']) |length == 0 }}"
+        that: round_trip['after'] | symmetric_difference(result['after']) |length == 0
 
     - name: Revert back to base config using facts round trip
       register: revert
@@ -72,6 +74,6 @@
 
     - name: Assert that config was reverted
       assert:
-        that: "{{ base_config['after'] | symmetric_difference(revert['after']) |length == 0 }}"
+        that: base_config['after'] | symmetric_difference(revert['after']) |length == 0
   always:
     - include_tasks: _remove_config.yaml
diff --git a/tests/integration/targets/vyos_interfaces/vars/main.yaml b/tests/integration/targets/vyos_interfaces/vars/main.yaml
index 4e66747b..c65771f2 100644
--- a/tests/integration/targets/vyos_interfaces/vars/main.yaml
+++ b/tests/integration/targets/vyos_interfaces/vars/main.yaml
@@ -12,6 +12,8 @@ merged:
   commands:
     - set interfaces ethernet eth1 description 'Configured by Ansible - Interface 1'
     - set interfaces ethernet eth1 mtu '1500'
+    - set interfaces ethernet eth1 duplex 'auto'
+    - set interfaces ethernet eth1 speed 'auto'
     - set interfaces ethernet eth1 vif 100 description 'Eth1 - VIF 100'
     - set interfaces ethernet eth1 vif 100 mtu '1404'
     - set interfaces ethernet eth1 vif 101 description 'Eth1 - VIF 101'
@@ -27,6 +29,8 @@ merged:
       description: Configured by Ansible - Interface 1
       mtu: 1500
       enabled: true
+      duplex: auto
+      speed: auto
       vifs:
         - vlan_id: 100
           description: Eth1 - VIF 100
@@ -44,6 +48,8 @@ populate:
     enabled: true
     description: Configured by Ansible
     mtu: 1500
+    duplex: auto
+    speed: auto
     vifs:
       - vlan_id: 200
         enabled: true
@@ -52,6 +58,8 @@ populate:
     enabled: true
     description: Configured by Ansible
     mtu: 1500
+    duplex: auto
+    speed: auto
     vifs:
       - vlan_id: 200
         enabled: true
@@ -63,10 +71,14 @@ populate:
 replaced:
   commands:
     - delete interfaces ethernet eth1 mtu
+    - delete interfaces ethernet eth1 speed
+    - delete interfaces ethernet eth1 duplex
     - delete interfaces ethernet eth1 vif 200
     - set interfaces ethernet eth1 description 'Replaced by Ansible'
     - set interfaces ethernet eth1 vif 100 description 'VIF 100 - Replaced by Ansible'
     - delete interfaces ethernet eth2 vif 200
+    - delete interfaces ethernet eth2 speed
+    - delete interfaces ethernet eth2 duplex
     - set interfaces ethernet eth2 description 'Replaced by Ansible'
     - set interfaces ethernet eth2 mtu '1400'
   after:
@@ -107,8 +119,12 @@ overridden:
   commands:
     - delete interfaces ethernet eth1 description
     - delete interfaces ethernet eth1 mtu
+    - delete interfaces ethernet eth1 speed
+    - delete interfaces ethernet eth1 duplex
     - delete interfaces ethernet eth1 vif 200
     - delete interfaces ethernet eth2 vif 200
+    - delete interfaces ethernet eth2 speed
+    - delete interfaces ethernet eth2 duplex
     - set interfaces ethernet eth2 description 'Overridden by Ansible'
     - set interfaces ethernet eth2 mtu '1402'
   after:
@@ -124,6 +140,10 @@ overridden:
       mtu: 1402
 rendered:
   commands:
+    - set interfaces ethernet eth0 duplex 'auto'
+    - set interfaces ethernet eth0 speed 'auto'
+    - set interfaces ethernet eth1 duplex 'auto'
+    - set interfaces ethernet eth1 speed 'auto'
     - set interfaces ethernet eth1 description 'Configured by Ansible - Interface 1'
     - set interfaces ethernet eth1 mtu '1500'
     - set interfaces ethernet eth1 vif 100 description 'Eth1 - VIF 100'
@@ -137,9 +157,13 @@ deleted:
     - delete interfaces ethernet eth1 description
     - delete interfaces ethernet eth1 mtu
     - delete interfaces ethernet eth1 vif 200
+    - delete interfaces ethernet eth1 speed
+    - delete interfaces ethernet eth1 duplex
     - delete interfaces ethernet eth2 description
     - delete interfaces ethernet eth2 mtu
     - delete interfaces ethernet eth2 vif 200
+    - delete interfaces ethernet eth2 speed
+    - delete interfaces ethernet eth2 duplex
   after:
     - name: eth0
       enabled: true
@@ -149,6 +173,7 @@ deleted:
       enabled: true
     - name: eth2
       enabled: true
+
 round_trip:
   after:
     - name: eth0