From a8c4ba2542851a5fbc0035f999af3f9cb1624ef3 Mon Sep 17 00:00:00 2001 From: Alex Groshev <38885591+haddystuff@users.noreply.github.com> Date: Tue, 1 Nov 2022 21:40:17 +0100 Subject: [PATCH] fix int options idempotence bug and add new test to check it (#5443) --- ...4998-nmcli-fix-int-options-idempotence.yml | 2 + plugins/modules/net_tools/nmcli.py | 3 +- .../plugins/modules/net_tools/test_nmcli.py | 182 ++++++++++++++++++ 3 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 changelogs/fragments/4998-nmcli-fix-int-options-idempotence.yml diff --git a/changelogs/fragments/4998-nmcli-fix-int-options-idempotence.yml b/changelogs/fragments/4998-nmcli-fix-int-options-idempotence.yml new file mode 100644 index 00000000000..823b894982d --- /dev/null +++ b/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). diff --git a/plugins/modules/net_tools/nmcli.py b/plugins/modules/net_tools/nmcli.py index 49565b91224..aa2ddafe1d4 100644 --- a/plugins/modules/net_tools/nmcli.py +++ b/plugins/modules/net_tools/nmcli.py @@ -2135,7 +2135,8 @@ def _compare_conn_params(self, conn_info, options): 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 diff --git a/tests/unit/plugins/modules/net_tools/test_nmcli.py b/tests/unit/plugins/modules/net_tools/test_nmcli.py index 276d318185c..885bdf3a967 100644 --- a/tests/unit/plugins/modules/net_tools/test_nmcli.py +++ b/tests/unit/plugins/modules/net_tools/test_nmcli.py @@ -11,6 +11,7 @@ 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 @@ 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-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