diff --git a/changelogs/fragments/3625-nmcli_false_changed_mtu_fix.yml b/changelogs/fragments/3625-nmcli_false_changed_mtu_fix.yml new file mode 100644 index 0000000000..53dda43f7f --- /dev/null +++ b/changelogs/fragments/3625-nmcli_false_changed_mtu_fix.yml @@ -0,0 +1,4 @@ +--- +bugfixes: + - nmcli - fixed falsely reported changed status when ``mtu`` is omitted with ``dummy`` connections + (https://github.com/ansible-collections/community.general/issues/3612, https://github.com/ansible-collections/community.general/pull/3625). \ No newline at end of file diff --git a/plugins/modules/net_tools/nmcli.py b/plugins/modules/net_tools/nmcli.py index 83202f6ebc..c1b824be59 100644 --- a/plugins/modules/net_tools/nmcli.py +++ b/plugins/modules/net_tools/nmcli.py @@ -1695,6 +1695,8 @@ class Nmcli(object): # Depending on version nmcli adds double-qoutes to gsm.apn # Need to strip them in order to compare both current_value = current_value.strip('"') + if key == self.mtu_setting and self.mtu is None: + self.mtu = 0 else: # parameter does not exist current_value = None @@ -1703,6 +1705,8 @@ class Nmcli(object): # compare values between two lists if sorted(current_value) != sorted(value): changed = True + 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): changed = True diff --git a/tests/unit/plugins/modules/net_tools/test_nmcli.py b/tests/unit/plugins/modules/net_tools/test_nmcli.py index 33954e594d..9c136aa5da 100644 --- a/tests/unit/plugins/modules/net_tools/test_nmcli.py +++ b/tests/unit/plugins/modules/net_tools/test_nmcli.py @@ -675,6 +675,45 @@ ipv6.method: manual ipv6.addresses: 2001:db8::1/128 """ +TESTCASE_DUMMY_STATIC_WITHOUT_MTU_SHOW_OUTPUT = """\ +connection.id: non_existent_nw_device +connection.interface-name: dummy_non_existant +connection.autoconnect: yes +ipv4.method: manual +ipv4.addresses: 10.10.10.10/24 +ipv4.gateway: 10.10.10.1 +ipv4.ignore-auto-dns: no +ipv4.ignore-auto-routes: no +ipv4.never-default: no +ipv4.may-fail: yes +ipv4.dns: 1.1.1.1,8.8.8.8 +ipv6.method: auto +ipv6.ignore-auto-dns: no +ipv6.ignore-auto-routes: no +ipv6.method: manual +ipv6.addresses: 2001:db8::1/128 +""" + +TESTCASE_DUMMY_STATIC_WITH_CUSTOM_MTU_SHOW_OUTPUT = """\ +connection.id: non_existent_nw_device +connection.interface-name: dummy_non_existant +connection.autoconnect: yes +802-3-ethernet.mtu: 1500 +ipv4.method: manual +ipv4.addresses: 10.10.10.10/24 +ipv4.gateway: 10.10.10.1 +ipv4.ignore-auto-dns: no +ipv4.ignore-auto-routes: no +ipv4.never-default: no +ipv4.may-fail: yes +ipv4.dns: 1.1.1.1,8.8.8.8 +ipv6.method: auto +ipv6.ignore-auto-dns: no +ipv6.ignore-auto-routes: no +ipv6.method: manual +ipv6.addresses: 2001:db8::1/128 +""" + TESTCASE_GSM = [ { @@ -955,6 +994,24 @@ def mocked_dummy_connection_static_unchanged(mocker): execute_return=(0, TESTCASE_DUMMY_STATIC_SHOW_OUTPUT, "")) +@pytest.fixture +def mocked_dummy_connection_static_without_mtu_unchanged(mocker): + mocker_set(mocker, + connection_exists=True, + execute_return=(0, TESTCASE_DUMMY_STATIC_WITHOUT_MTU_SHOW_OUTPUT, "")) + + +@pytest.fixture +def mocked_dummy_connection_static_with_custom_mtu_modify(mocker): + mocker_set(mocker, + connection_exists=True, + execute_return=None, + execute_side_effect=( + (0, TESTCASE_DUMMY_STATIC_WITH_CUSTOM_MTU_SHOW_OUTPUT, ""), + (0, "", ""), + )) + + @pytest.fixture def mocked_gsm_connection_unchanged(mocker): mocker_set(mocker, @@ -2283,6 +2340,48 @@ def test_dummy_connection_static_unchanged(mocked_dummy_connection_static_unchan assert not results['changed'] +@pytest.mark.parametrize('patch_ansible_module', TESTCASE_DUMMY_STATIC, indirect=['patch_ansible_module']) +def test_dummy_connection_static_without_mtu_unchanged(mocked_dummy_connection_static_without_mtu_unchanged, capfd): + """ + Test : Dummy connection with static IP configuration and no mtu set unchanged + """ + with pytest.raises(SystemExit): + nmcli.main() + + out, err = capfd.readouterr() + results = json.loads(out) + assert not results.get('failed') + assert not results['changed'] + + +@pytest.mark.parametrize('patch_ansible_module', TESTCASE_DUMMY_STATIC, indirect=['patch_ansible_module']) +def test_dummy_connection_static_with_custom_mtu_modify(mocked_dummy_connection_static_with_custom_mtu_modify, capfd): + """ + Test : Dummy connection with static IP configuration and no mtu set modify + """ + with pytest.raises(SystemExit): + nmcli.main() + + assert nmcli.Nmcli.execute_command.call_count == 2 + + arg_list = nmcli.Nmcli.execute_command.call_args_list + args, kwargs = arg_list[1] + + assert args[0][0] == '/usr/bin/nmcli' + assert args[0][1] == 'con' + assert args[0][2] == 'modify' + assert args[0][3] == 'non_existent_nw_device' + + args_text = list(map(to_text, args[0])) + for param in ['802-3-ethernet.mtu', '0']: + assert param in args_text + + out, err = capfd.readouterr() + results = json.loads(out) + assert not results.get('failed') + assert results['changed'] + + @pytest.mark.parametrize('patch_ansible_module', TESTCASE_GSM, indirect=['patch_ansible_module']) def test_create_gsm(mocked_generic_connection_create, capfd): """