From 612d0d66343420f3913d372197f3bb7c6a04087b Mon Sep 17 00:00:00 2001 From: Olivier Bourdon Date: Wed, 28 Mar 2018 11:21:43 +0200 Subject: [PATCH] Fix interfaces_file for proper file contents (#37818) The generated file was completely unusable by the system therefore the fix which ensures that diffing the file prior to changes and after only shows diffs Furthermore the code did not work for Python 3.6 > f.writelines(to_bytes(lines, errors='surrogate_or_strict')) E TypeError: a bytes-like object is required, not 'int' The other modifications (lambda variable renaming) is to comply with default flake8 rules --- lib/ansible/modules/system/interfaces_file.py | 6 +- .../golden_output/default_dhcp_revert | 6 ++ .../default_dhcp_revert.exceptions.txt | 0 .../golden_output/default_dhcp_revert.json | 18 ++++ .../fixtures/golden_output/servers.com_revert | 58 ++++++++++ .../servers.com_revert.exceptions.txt | 8 ++ .../golden_output/servers.com_revert.json | 101 ++++++++++++++++++ .../interfaces_file/test_interfaces_file.py | 57 +++++++++- 8 files changed, 248 insertions(+), 6 deletions(-) create mode 100644 test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_revert create mode 100644 test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_revert.exceptions.txt create mode 100644 test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_revert.json create mode 100644 test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert create mode 100644 test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert.exceptions.txt create mode 100644 test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert.json 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)