Skip to content

Commit

Permalink
Fix dummy interface returning changed (#3625) (#3688)
Browse files Browse the repository at this point in the history
* fix dummy interface bug

* fix dummy interface bug

* Update nmcli.py

* Update nmcli.py

* Update nmcli.py

* Update nmcli.py

* adding tests and requested conditional

* Fix pylint problems and remove 2 lines from previous version of bugfix

* Fix pep8 issue

* add changelog

* Update changelogs/fragments/3625-nmcli_false_changed_mtu_fix.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 85085bc)

Co-authored-by: Alex Groshev <38885591+haddystuff@users.noreply.github.com>
  • Loading branch information
patchback[bot] and haddystuff authored Nov 10, 2021
1 parent 0e3c1f8 commit 4fd7a65
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 0 deletions.
4 changes: 4 additions & 0 deletions changelogs/fragments/3625-nmcli_false_changed_mtu_fix.yml
Original file line number Diff line number Diff line change
@@ -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).
4 changes: 4 additions & 0 deletions plugins/modules/net_tools/nmcli.py
Original file line number Diff line number Diff line change
Expand Up @@ -1695,6 +1695,8 @@ def _compare_conn_params(self, conn_info, options):
# 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
Expand All @@ -1703,6 +1705,8 @@ def _compare_conn_params(self, conn_info, options):
# 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
Expand Down
99 changes: 99 additions & 0 deletions tests/unit/plugins/modules/net_tools/test_nmcli.py
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,45 @@
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 = [
{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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):
"""
Expand Down

0 comments on commit 4fd7a65

Please sign in to comment.