fix int options idempotence bug and add new test to check it (#5443) (#5455)

(cherry picked from commit dc66aefa40)

Co-authored-by: Alex Groshev <38885591+haddystuff@users.noreply.github.com>

changelogs/fragments/4998-nmcli-fix-int-options-idempotence.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - nmcli - fix int options idempotence (https://github.com/ansible-collections/community.general/issues/4998).

plugins/modules/net_tools/nmcli.py

@@ -2135,7 +2135,8 @@ class Nmcli(object):
             elif all([key == self.mtu_setting, self.type == 'dummy', current_value is None, value == 'auto', self.mtu is None]):
                 value = None
             else:
-                if current_value != to_text(value):
+                value = to_text(value)
+                if current_value != value:
                     changed = True
 
             diff_before[key] = current_value

tests/unit/plugins/modules/net_tools/test_nmcli.py

@@ -11,6 +11,7 @@ import pytest
 
 from ansible.module_utils.common.text.converters import to_text
 from ansible_collections.community.general.plugins.modules.net_tools import nmcli
+from ansible.module_utils.basic import AnsibleModule
 
 pytestmark = pytest.mark.usefixtures('patch_ansible_module')
 
@@ -132,6 +133,7 @@ connection.autoconnect:                 yes
 ipv4.method:                            manual
 ipv4.addresses:                         10.10.10.10/24
 ipv4.gateway:                           10.10.10.1
+ipv4.route-metric:                      -1
 ipv4.ignore-auto-dns:                   no
 ipv4.ignore-auto-routes:                no
 ipv4.never-default:                     no
@@ -141,6 +143,19 @@ ipv6.ignore-auto-dns:                   no
 ipv6.ignore-auto-routes:                no
 """
 
+TESTCASE_GENERIC_DIFF_CHECK = [
+    {
+        'type': 'generic',
+        'conn_name': 'non_existent_nw_device',
+        'ifname': 'generic_non_existant',
+        'ip4': '10.10.10.10/24',
+        'gw4': '10.10.10.2',
+        'route_metric4': -1,
+        'state': 'present',
+        '_ansible_check_mode': False,
+    },
+]
+
 TESTCASE_GENERIC_MODIFY_ROUTING_RULES = [
     {
         'type': 'generic',
@@ -1726,6 +1741,13 @@ def mocked_infiniband_connection_static_transport_mode_connected_modify(mocker):
                ))
 
 
+@pytest.fixture
+def mocked_generic_connection_diff_check(mocker):
+    mocker_set(mocker,
+               connection_exists=True,
+               execute_return=(0, TESTCASE_GENERIC_SHOW_OUTPUT, ""))
+
+
 @pytest.mark.parametrize('patch_ansible_module', TESTCASE_BOND, indirect=['patch_ansible_module'])
 def test_bond_connection_create(mocked_generic_connection_create, capfd):
     """
@@ -3810,3 +3832,163 @@ def test_infiniband_connection_static_transport_mode_connected(
 
     assert results.get('changed') is True
     assert not results.get('failed')
+
+
+@pytest.mark.parametrize('patch_ansible_module', TESTCASE_GENERIC_DIFF_CHECK, indirect=['patch_ansible_module'])
+def test_bond_connection_unchanged(mocked_generic_connection_diff_check, capfd):
+    """
+    Test : Bond connection unchanged
+    """
+
+    module = AnsibleModule(
+        argument_spec=dict(
+            ignore_unsupported_suboptions=dict(type='bool', default=False),
+            autoconnect=dict(type='bool', default=True),
+            state=dict(type='str', required=True, choices=['absent', 'present']),
+            conn_name=dict(type='str', required=True),
+            master=dict(type='str'),
+            ifname=dict(type='str'),
+            type=dict(type='str',
+                      choices=[
+                          'bond',
+                          'bond-slave',
+                          'bridge',
+                          'bridge-slave',
+                          'dummy',
+                          'ethernet',
+                          'generic',
+                          'gre',
+                          'infiniband',
+                          'ipip',
+                          'sit',
+                          'team',
+                          'team-slave',
+                          'vlan',
+                          'vxlan',
+                          'wifi',
+                          'gsm',
+                          'wireguard',
+                          'vpn',
+                      ]),
+            ip4=dict(type='list', elements='str'),
+            gw4=dict(type='str'),
+            gw4_ignore_auto=dict(type='bool', default=False),
+            routes4=dict(type='list', elements='str'),
+            routes4_extended=dict(type='list',
+                                  elements='dict',
+                                  options=dict(
+                                      ip=dict(type='str', required=True),
+                                      next_hop=dict(type='str'),
+                                      metric=dict(type='int'),
+                                      table=dict(type='int'),
+                                      tos=dict(type='int'),
+                                      cwnd=dict(type='int'),
+                                      mtu=dict(type='int'),
+                                      onlink=dict(type='bool')
+                                  )),
+            route_metric4=dict(type='int'),
+            routing_rules4=dict(type='list', elements='str'),
+            never_default4=dict(type='bool', default=False),
+            dns4=dict(type='list', elements='str'),
+            dns4_search=dict(type='list', elements='str'),
+            dns4_ignore_auto=dict(type='bool', default=False),
+            method4=dict(type='str', choices=['auto', 'link-local', 'manual', 'shared', 'disabled']),
+            may_fail4=dict(type='bool', default=True),
+            dhcp_client_id=dict(type='str'),
+            ip6=dict(type='list', elements='str'),
+            gw6=dict(type='str'),
+            gw6_ignore_auto=dict(type='bool', default=False),
+            dns6=dict(type='list', elements='str'),
+            dns6_search=dict(type='list', elements='str'),
+            dns6_ignore_auto=dict(type='bool', default=False),
+            routes6=dict(type='list', elements='str'),
+            routes6_extended=dict(type='list',
+                                  elements='dict',
+                                  options=dict(
+                                      ip=dict(type='str', required=True),
+                                      next_hop=dict(type='str'),
+                                      metric=dict(type='int'),
+                                      table=dict(type='int'),
+                                      cwnd=dict(type='int'),
+                                      mtu=dict(type='int'),
+                                      onlink=dict(type='bool')
+                                  )),
+            route_metric6=dict(type='int'),
+            method6=dict(type='str', choices=['ignore', 'auto', 'dhcp', 'link-local', 'manual', 'shared', 'disabled']),
+            ip_privacy6=dict(type='str', choices=['disabled', 'prefer-public-addr', 'prefer-temp-addr', 'unknown']),
+            addr_gen_mode6=dict(type='str', choices=['eui64', 'stable-privacy']),
+            # Bond Specific vars
+            mode=dict(type='str', default='balance-rr',
+                      choices=['802.3ad', 'active-backup', 'balance-alb', 'balance-rr', 'balance-tlb', 'balance-xor', 'broadcast']),
+            miimon=dict(type='int'),
+            downdelay=dict(type='int'),
+            updelay=dict(type='int'),
+            xmit_hash_policy=dict(type='str'),
+            arp_interval=dict(type='int'),
+            arp_ip_target=dict(type='str'),
+            primary=dict(type='str'),
+            # general usage
+            mtu=dict(type='int'),
+            mac=dict(type='str'),
+            zone=dict(type='str'),
+            # bridge specific vars
+            stp=dict(type='bool', default=True),
+            priority=dict(type='int', default=128),
+            slavepriority=dict(type='int', default=32),
+            forwarddelay=dict(type='int', default=15),
+            hellotime=dict(type='int', default=2),
+            maxage=dict(type='int', default=20),
+            ageingtime=dict(type='int', default=300),
+            hairpin=dict(type='bool'),
+            path_cost=dict(type='int', default=100),
+            # team specific vars
+            runner=dict(type='str', default='roundrobin',
+                             choices=['broadcast', 'roundrobin', 'activebackup', 'loadbalance', 'lacp']),
+            # team active-backup runner specific options
+            runner_hwaddr_policy=dict(type='str', choices=['same_all', 'by_active', 'only_active']),
+            # vlan specific vars
+            vlanid=dict(type='int'),
+            vlandev=dict(type='str'),
+            flags=dict(type='str'),
+            ingress=dict(type='str'),
+            egress=dict(type='str'),
+            # vxlan specific vars
+            vxlan_id=dict(type='int'),
+            vxlan_local=dict(type='str'),
+            vxlan_remote=dict(type='str'),
+            # ip-tunnel specific vars
+            ip_tunnel_dev=dict(type='str'),
+            ip_tunnel_local=dict(type='str'),
+            ip_tunnel_remote=dict(type='str'),
+            # ip-tunnel type gre specific vars
+            ip_tunnel_input_key=dict(type='str', no_log=True),
+            ip_tunnel_output_key=dict(type='str', no_log=True),
+            # 802-11-wireless* specific vars
+            ssid=dict(type='str'),
+            wifi=dict(type='dict'),
+            wifi_sec=dict(type='dict', no_log=True),
+            gsm=dict(type='dict'),
+            wireguard=dict(type='dict'),
+            vpn=dict(type='dict'),
+            transport_mode=dict(type='str', choices=['datagram', 'connected']),
+        ),
+        mutually_exclusive=[['never_default4', 'gw4'],
+                            ['routes4_extended', 'routes4'],
+                            ['routes6_extended', 'routes6']],
+        required_if=[("type", "wifi", [("ssid")])],
+        supports_check_mode=True,
+    )
+    module.run_command_environ_update = dict(LANG='C', LC_ALL='C', LC_MESSAGES='C', LC_CTYPE='C')
+
+    nmcli_module = nmcli.Nmcli(module)
+
+    changed, diff = nmcli_module.is_connection_changed()
+
+    assert changed
+
+    num_of_diff_params = 0
+    for parameter, value in diff.get('before').items():
+        if value != diff['after'][parameter]:
+            num_of_diff_params += 1
+
+    assert num_of_diff_params == 1