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

@@ -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))
 

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

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": []
+    }
+}

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

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"
+}

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": []
+    }
+}

test/units/modules/system/interfaces_file/test_interfaces_file.py

@@ -16,14 +16,13 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 
 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)