Implement PR feedback

plugins/module_utils/main/interface_lagg.py

@@ -8,6 +8,7 @@
 
 
 class Lagg(BaseModule):
+    FIELD_ID = 'device'
     CMDS = {
         'add': 'addItem',
         'del': 'delItem',
@@ -19,7 +20,7 @@ class Lagg(BaseModule):
     API_CONT = 'lagg_settings'
     API_CMD_REL = 'reconfigure'
     FIELDS_CHANGE = ['members', 'primary_member', 'proto', 'lacp_fast_timeout', 'use_flowid', 'lagghash', 'lacp_strict', 'mtu', 'description']
-    FIELDS_ALL = ['device']
+    FIELDS_ALL = [FIELD_ID]
     FIELDS_ALL.extend(FIELDS_CHANGE)
     FIELDS_TRANSLATE = {
         'device': 'laggif',
@@ -43,7 +44,8 @@ def check(self) -> None:
         if self.p['state'] == 'present':
             if is_unset(self.p['members']):
                 self.m.fail_json("You need to provide a list of 'members' to create a lagg!")
-
+            if is_unset(self.p['lagghash']):
+                self.m.fail_json("You need to provide a list of 'lagghash' to create a lagg!")
 
             validate_int_fields(module=self.m, data=self.p, field_minmax=self.INT_VALIDATIONS)
 

plugins/modules/interface_lagg.py

@@ -32,7 +32,7 @@ def run_module():
             description="Optional 'device' of the entry. Needs to start with 'lagg'",
         ),
         members=dict(
-            type='list', elements='str', required=False, aliases=['port', 'int', 'if'],
+            type='list', elements='str', required=False, aliases=['port', 'int', 'if', 'parent'],
             description='Existing LAGG capable interface - you must provide the network '
                         "port as shown in 'Interfaces - Assignments - Network port'"
         ),
@@ -46,22 +46,22 @@ def run_module():
             choices=['none', 'lacp', 'failover', 'fec', 'loadbalance', 'roundrobin'],
             description="The protocol to use."
         ),
-        lacp_fast_timeout=dict(type='bool', required=False, default=False,
+        lacp_fast_timeout=dict(type='bool', required=False, default=False, aliases=['fast_timeout'],
             description='Enable lacp fast-timeout on the interface.'
         ),
         use_flowid=dict(
-            type='str', required=False, choices=['default', 'yes', 'no'],
+            type='str', required=False, choices=['yes', 'no'], aliases=['flowid'],
             description='Use the RSS hash from the network card if available, otherwise a hash is locally calculated. '
                         'The default depends on the system tunable in net.link.lagg.default_use_flowid.'
         ),
         lagghash=dict(
-            type='list', elements='str', required=False, default=['l2'],
+            type='list', elements='str', required=False, aliases=['hash', 'hash_layers'],
             choices=['l2', 'l3', 'l4'],
             description='Set the packet layers to hash for aggregation protocols which load balance.'
         ),
         lacp_strict=dict(
             type='str', required=False,
-            choices=['default', 'yes', 'no'],
+            choices=['yes', 'no'],
             description='Enable lacp strict compliance on the interface. The default depends on the '
                         'system tunable in net.link.lagg.lacp.default_strict_mode.',
         ),
@@ -70,13 +70,6 @@ def run_module():
             description='If you leave this field blank, the smallest mtu of this laggs children will be used.'
         ),
         description=dict(type='str', required=False, aliases=['desc', 'name']),
-        match_fields=dict(
-            type='list', required=False, elements='str',
-            description='Fields that are used to match configured LAGG with the running config - '
-                        "if any of those fields are changed, the module will think it's a new entry",
-            choices=['device', 'members', 'primary_member', 'proto', 'description'],
-            default=['members'],
-        ),
         **RELOAD_MOD_ARG,
         **STATE_ONLY_MOD_ARG,
         **OPN_MOD_ARGS,

tests/interface_lagg.yml

@@ -12,6 +12,9 @@
     ansibleguy.opnsense.list:
       target: 'interface_lagg'
 
+  vars:
+    if_lag: "{{ lookup('ansible.builtin.env', 'TEST_FINTERFACE_LAGG_IF') | default('lan', true) }}"
+
   tasks:
     - name: Listing
       ansibleguy.opnsense.list:
@@ -22,8 +25,7 @@
 
     - name: Removing - does not exist
       ansibleguy.opnsense.interface_lagg:
-        members:
-          - ansibletest11
+        device: 'lagg99'
         state: 'absent'
       register: opn_pre2
       failed_when: >
@@ -32,27 +34,32 @@
 
     - name: Adding 1 - failing because of invalid interface (server-side)
       ansibleguy.opnsense.interface_lagg:
+        device: 'lagg0'
         members:
-          - lan
+          - 'DOES-NOT-EXIST'
       register: opn_fail1
       failed_when: not opn_fail1.failed
       when: not ansible_check_mode
 
     - name: Adding 1
       ansibleguy.opnsense.interface_lagg:
+        device: 'lagg0'
         description: 'ANSIBLE_TEST_1_1'
         members:
-          - 'vtnet0'
+          - '{{ if_lag }}'
+        lagghash: l2
       register: opn1
       failed_when: >
         opn1.failed or
         not opn1.changed
 
     - name: Adding 1 - nothing changed
-      ansibleguy.opnsense.interface_vlan:
+      ansibleguy.opnsense.interface_lagg:
+        device: 'lagg0'
         description: 'ANSIBLE_TEST_1_1'
         members:
-          - 'vtnet0'
+          - '{{ if_lag }}'
+        lagghash: l2
       register: opn2
       failed_when: >
         opn2.failed or
@@ -64,13 +71,12 @@
       register: opn3
       failed_when: >
         'data' not in opn3 or
-        opn3.data | length != 1
+        opn3.data | length != 4
       when: not ansible_check_mode
 
     - name: Removing 2
       ansibleguy.opnsense.interface_lagg:
-        members:
-          - 'vtnet0'
+        device: 'lagg0'
         state: 'absent'
       register: opn4
       failed_when: >