Skip to content

Commit

Permalink
fix int options idempotence bug and add new test to check it (ansible…
Browse files Browse the repository at this point in the history 
…-collections#5443)
  • Loading branch information
@haddystuff @russoz
haddystuff authored and russoz committed Nov 10, 2022
1 parent f140c48 commit a8c4ba2
Show file tree
Hide file tree
Showing 3 changed files with 186 additions and 1 deletion.
2 changes: 2 additions & 0 deletions changelogs/fragments/4998-nmcli-fix-int-options-idempotence.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- nmcli - fix int options idempotence (https://github.com/ansible-collections/community.general/issues/4998).
3 changes: 2 additions & 1 deletion plugins/modules/net_tools/nmcli.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
182 changes: 182 additions & 0 deletions tests/unit/plugins/modules/net_tools/test_nmcli.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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
Expand All @@ -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',
Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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

0 comments on commit a8c4ba2

Please sign in to comment.