diff --git a/lib/ansible/modules/system/interfaces_file.py b/lib/ansible/modules/system/interfaces_file.py index 27b084ae585..73660832d71 100755 --- a/lib/ansible/modules/system/interfaces_file.py +++ b/lib/ansible/modules/system/interfaces_file.py @@ -290,11 +290,11 @@ def setInterfaceOption(module, lines, iface, option, raw_value, state): if option in ["pre-up", "up", "down", "post-up"] and value is not None and value != "None": for target_option in filter(lambda i: i['value'] == value, target_options): changed = True - lines = list(filter(lambda l: l != target_option, lines)) + lines = list(filter(lambda ln: ln != target_option, lines)) else: changed = True for target_option in target_options: - lines = list(filter(lambda l: l != target_option, lines)) + lines = list(filter(lambda ln: ln != target_option, lines)) else: module.fail_json(msg="Error: unsupported state %s, has to be either present or absent" % state) @@ -322,7 +322,7 @@ def write_changes(module, lines, dest): tmpfd, tmpfile = tempfile.mkstemp() f = os.fdopen(tmpfd, 'wb') - f.writelines(to_bytes(lines, errors='surrogate_or_strict')) + f.write(to_bytes(''.join(lines), errors='surrogate_or_strict')) f.close() module.atomic_move(tmpfile, os.path.realpath(dest)) diff --git a/test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_revert b/test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_revert new file mode 100644 index 00000000000..bd4522ec09f --- /dev/null +++ b/test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_revert @@ -0,0 +1,6 @@ +# The loopback network interface +auto lo eth0 +iface lo inet loopback + +# The primary network interface +iface eth0 inet dhcp diff --git a/test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_revert.exceptions.txt b/test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_revert.exceptions.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_revert.json b/test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_revert.json new file mode 100644 index 00000000000..bffc17a9897 --- /dev/null +++ b/test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_revert.json @@ -0,0 +1,18 @@ +{ + "eth0": { + "address_family": "inet", + "down": [], + "method": "dhcp", + "post-up": [], + "pre-up": [], + "up": [] + }, + "lo": { + "address_family": "inet", + "down": [], + "method": "loopback", + "post-up": [], + "pre-up": [], + "up": [] + } +} \ No newline at end of file diff --git a/test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert b/test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert new file mode 100644 index 00000000000..4356aa47d7e --- /dev/null +++ b/test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert @@ -0,0 +1,58 @@ + auto aggi + iface aggi inet static + hwaddress ether 22:44:77:88:D5:96 + address 10.44.15.196 + netmask 255.255.255.248 + mtu 1500 + slaves int1 int2 + bond_mode 4 + bond_miimon 100 + bond_downdelay 200 + bond_updelay 200 + bond_lacp_rate slow + bond_xmit_hash_policy layer3+4 + post-up /sbin/ethtool -K aggi tx off tso off + + auto agge + iface agge inet manual + + auto br0 + iface br0 inet static + bridge_ports agge + hwaddress ether 22:44:77:88:D5:98 + address 188.44.133.76 + netmask 255.255.255.248 + gateway 188.44.133.75 + slaves ext1 ext2 + bond_mode 4 + bond_miimon 100 + bond_downdelay 200 + bond_updelay 200 + bond_lacp_rate slow + bond_xmit_hash_policy layer3+4 + post-up /sbin/ethtool -K agge tx off tso off + + up route add -net 10.0.0.0/8 gw 10.44.15.117 dev aggi + up route add -net 192.168.0.0/16 gw 10.44.15.117 dev aggi + up route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi + + auto int1 + iface int1 inet manual + bond-master aggi + + auto int2 + iface int2 inet manual + bond-master aggi + + auto ext1 + iface ext1 inet manual + bond-master agge + + auto ext2 + iface ext2 inet manual + bond-master agge + + auto lo + iface lo inet loopback + +source /etc/network/interfaces.d/*.cfg diff --git a/test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert.exceptions.txt b/test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert.exceptions.txt new file mode 100644 index 00000000000..fddf3b3b0aa --- /dev/null +++ b/test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert.exceptions.txt @@ -0,0 +1,8 @@ +fail_json message: Error: interface eth0 not found +options: +{ + "iface": "eth0", + "option": "mtu", + "state": "absent", + "value": "1350" +} \ No newline at end of file diff --git a/test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert.json b/test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert.json new file mode 100644 index 00000000000..0460b552a9d --- /dev/null +++ b/test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert.json @@ -0,0 +1,101 @@ +{ + "agge": { + "address_family": "inet", + "down": [], + "method": "manual", + "post-up": [], + "pre-up": [], + "up": [] + }, + "aggi": { + "address": "10.44.15.196", + "address_family": "inet", + "bond_downdelay": "200", + "bond_lacp_rate": "slow", + "bond_miimon": "100", + "bond_mode": "4", + "bond_updelay": "200", + "bond_xmit_hash_policy": "layer3+4", + "down": [], + "hwaddress": "ether 22:44:77:88:D5:96", + "method": "static", + "mtu": "1500", + "netmask": "255.255.255.248", + "post-up": [ + "/sbin/ethtool -K aggi tx off tso off" + ], + "pre-up": [], + "slaves": "int1 int2", + "up": [] + }, + "br0": { + "address": "188.44.133.76", + "address_family": "inet", + "bond_downdelay": "200", + "bond_lacp_rate": "slow", + "bond_miimon": "100", + "bond_mode": "4", + "bond_updelay": "200", + "bond_xmit_hash_policy": "layer3+4", + "bridge_ports": "agge", + "down": [], + "gateway": "188.44.133.75", + "hwaddress": "ether 22:44:77:88:D5:98", + "method": "static", + "netmask": "255.255.255.248", + "post-up": [ + "/sbin/ethtool -K agge tx off tso off" + ], + "pre-up": [], + "slaves": "ext1 ext2", + "up": [ + "route add -net 10.0.0.0/8 gw 10.44.15.117 dev aggi", + "route add -net 192.168.0.0/16 gw 10.44.15.117 dev aggi", + "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi" + ] + }, + "ext1": { + "address_family": "inet", + "bond-master": "agge", + "down": [], + "method": "manual", + "post-up": [], + "pre-up": [], + "up": [] + }, + "ext2": { + "address_family": "inet", + "bond-master": "agge", + "down": [], + "method": "manual", + "post-up": [], + "pre-up": [], + "up": [] + }, + "int1": { + "address_family": "inet", + "bond-master": "aggi", + "down": [], + "method": "manual", + "post-up": [], + "pre-up": [], + "up": [] + }, + "int2": { + "address_family": "inet", + "bond-master": "aggi", + "down": [], + "method": "manual", + "post-up": [], + "pre-up": [], + "up": [] + }, + "lo": { + "address_family": "inet", + "down": [], + "method": "loopback", + "post-up": [], + "pre-up": [], + "up": [] + } +} \ No newline at end of file diff --git a/test/units/modules/system/interfaces_file/test_interfaces_file.py b/test/units/modules/system/interfaces_file/test_interfaces_file.py index 717cea6d372..920e582453d 100644 --- a/test/units/modules/system/interfaces_file/test_interfaces_file.py +++ b/test/units/modules/system/interfaces_file/test_interfaces_file.py @@ -16,14 +16,13 @@ # along with Ansible. If not, see . from ansible.compat.tests import unittest -import ansible.module_utils.basic from ansible.modules.system import interfaces_file import os import json -import sys import io import inspect -import json +from shutil import copyfile, move +import difflib class AnsibleFailJson(Exception): @@ -31,6 +30,14 @@ class AnsibleFailJson(Exception): class ModuleMocked(): + def atomic_move(self, src, dst): + move(src, dst) + + def backup_local(self, path): + backupp = os.path.join("/tmp", os.path.basename(path) + ".bak") + copyfile(path, backupp) + return backupp + def fail_json(self, msg): raise AnsibleFailJson(msg) @@ -44,6 +51,18 @@ class TestInterfacesFileModule(unittest.TestCase): def getTestFiles(self): return next(os.walk(fixture_path))[2] + def compareFileToBackup(self, path, backup): + with open(path) as f1: + with open(backup) as f2: + diffs = difflib.context_diff(f1.readlines(), + f2.readlines(), + fromfile=os.path.basename(path), + tofile=os.path.basename(backup)) + # Restore backup + move(backup, path) + deltas = [d for d in diffs] + self.assertTrue(len(deltas) == 0) + def compareInterfacesLinesToFile(self, interfaces_lines, path, testname=None): if not testname: testname = "%s.%s" % (path, inspect.stack()[1][3]) @@ -137,3 +156,35 @@ class TestInterfacesFileModule(unittest.TestCase): self.compareInterfacesLinesToFile(lines, testfile, "%s_%s" % (testfile, testname)) self.compareInterfacesToFile(ifaces, testfile, "%s_%s.json" % (testfile, testname)) + + def test_revert(self): + testcases = { + "revert": [ + { + 'iface': 'eth0', + 'option': 'mtu', + 'value': '1350', + } + ], + } + for testname, options_list in testcases.items(): + for testfile in self.getTestFiles(): + path = os.path.join(fixture_path, testfile) + lines, ifaces = interfaces_file.read_interfaces_file(module, path) + backupp = module.backup_local(path) + options = options_list[0] + for state in ['present', 'absent']: + fail_json_iterations = [] + options['state'] = state + try: + _, lines = interfaces_file.setInterfaceOption(module, lines, options['iface'], options['option'], options['value'], options['state']) + except AnsibleFailJson as e: + fail_json_iterations.append("fail_json message: %s\noptions:\n%s" % + (str(e), json.dumps(options, sort_keys=True, indent=4, separators=(',', ': ')))) + interfaces_file.write_changes(module, [d['line'] for d in lines if 'line' in d], path) + + self.compareStringWithFile("\n=====\n".join(fail_json_iterations), "%s_%s.exceptions.txt" % (testfile, testname)) + + self.compareInterfacesLinesToFile(lines, testfile, "%s_%s" % (testfile, testname)) + self.compareInterfacesToFile(ifaces, testfile, "%s_%s.json" % (testfile, testname)) + self.compareFileToBackup(path, backupp)