Add more tests for method option change (#52228)

lib/ansible/modules/system/interfaces_file.py

@@ -265,7 +265,7 @@ def setInterfaceOption(module, lines, iface, option, raw_value, state, address_f
     if len(iface_lines) < 1:
         # interface not found
         module.fail_json(msg="Error: interface %s not found" % iface)
-        return changed
+        return changed, None
 
     iface_options = list(filter(lambda i: i['line_type'] == 'option', iface_lines))
     target_options = list(filter(lambda i: i['option'] == option, iface_options))
@@ -275,12 +275,11 @@ def setInterfaceOption(module, lines, iface, option, raw_value, state, address_f
             changed = True
             # add new option
             last_line_dict = iface_lines[-1]
-            lines = addOptionAfterLine(option, value, iface, lines, last_line_dict, iface_options, address_family)
+            changed, lines = addOptionAfterLine(option, value, iface, lines, last_line_dict, iface_options, address_family)
         else:
             if option in ["pre-up", "up", "down", "post-up"]:
                 if len(list(filter(lambda i: i['value'] == value, target_options))) < 1:
-                    changed = True
+                    changed, lines = addOptionAfterLine(option, value, iface, lines, target_options[-1], iface_options, address_family)
-                    lines = addOptionAfterLine(option, value, iface, lines, target_options[-1], iface_options, address_family)
             else:
                 # if more than one option found edit the last one
                 if target_options[-1]['value'] != value:
@@ -316,11 +315,13 @@ def setInterfaceOption(module, lines, iface, option, raw_value, state, address_f
 def addOptionAfterLine(option, value, iface, lines, last_line_dict, iface_options, address_family):
     # Changing method of interface is not an addition
     if option == 'method':
+        changed = False
         for ln in lines:
-            if ln.get('line_type', '') == 'iface' and ln.get('iface', '') == iface:
+            if ln.get('line_type', '') == 'iface' and ln.get('iface', '') == iface and value != ln.get('params', {}).get('method', ''):
+                changed = True
                 ln['line'] = re.sub(ln.get('params', {}).get('method', '') + '$', value, ln.get('line'))
                 ln['params']['method'] = value
-        return lines
+        return changed, lines
 
     last_line = last_line_dict['line']
     prefix_start = last_line.find(last_line.split()[0])
@@ -335,7 +336,7 @@ def addOptionAfterLine(option, value, iface, lines, last_line_dict, iface_option
     option_dict = optionDict(line, iface, option, value, address_family)
     index = len(lines) - lines[::-1].index(last_line_dict)
     lines.insert(index, option_dict)
-    return lines
+    return True, lines
 
 
 def write_changes(module, lines, dest):

test/units/modules/system/interfaces_file/fixtures/golden_output/address_family_change_method

@@ -3,10 +3,10 @@ auto lo eth0
 iface lo inet loopback
 
 # The primary network interface
-iface eth0 inet manual
+iface eth0 inet static
   address 192.168.0.1
   post-up echo configuring ipv4
 
-iface eth0 inet6 manual
+iface eth0 inet6 static
   address fc00::1
   post-up echo configuring ipv6

test/units/modules/system/interfaces_file/fixtures/golden_output/address_family_change_method.exceptions.txt

@@ -0,0 +1,8 @@
+fail_json message: Error: interface eth1 not found
+options:
+{
+    "iface": "eth1",
+    "option": "method",
+    "state": "present",
+    "value": "dhcp"
+}

test/units/modules/system/interfaces_file/fixtures/golden_output/address_family_change_method.json

@@ -3,7 +3,7 @@
         "address": "fc00::1",
         "address_family": "inet6",
         "down": [],
-        "method": "manual",
+        "method": "static",
         "post-up": [
             "echo configuring ipv6"
         ],

test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_change_method

@@ -3,4 +3,4 @@ auto lo eth0
 iface lo inet loopback
 
 # The primary network interface
-iface eth0 inet manual
+iface eth0 inet dhcp

test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_change_method.exceptions.txt

@@ -0,0 +1,8 @@
+fail_json message: Error: interface eth1 not found
+options:
+{
+    "iface": "eth1",
+    "option": "method",
+    "state": "present",
+    "value": "dhcp"
+}

test/units/modules/system/interfaces_file/fixtures/golden_output/default_dhcp_change_method.json

@@ -2,7 +2,7 @@
     "eth0": {
         "address_family": "inet",
         "down": [],
-        "method": "manual",
+        "method": "dhcp",
         "post-up": [],
         "pre-up": [],
         "up": []

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com.test_no_changes

@@ -52,6 +52,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet manual
+
  auto lo
  iface lo inet loopback
  

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com.test_no_changes.json

@@ -54,6 +54,14 @@
             "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi"
         ]
     },
+    "eth1": {
+        "address_family": "inet",
+        "down": [],
+        "method": "manual",
+        "post-up": [],
+        "pre-up": [],
+        "up": []
+    },
     "ext1": {
         "address_family": "inet",
         "bond-master": "agge",

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_add_aggi_up

@@ -53,6 +53,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet manual
+
  auto lo
  iface lo inet loopback
  

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_add_aggi_up.json

@@ -54,6 +54,14 @@
             "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi"
         ]
     },
+    "eth1": {
+        "address_family": "inet",
+        "down": [],
+        "method": "manual",
+        "post-up": [],
+        "pre-up": [],
+        "up": []
+    },
     "ext1": {
         "address_family": "inet",
         "bond-master": "agge",

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_add_and_delete_aggi_up

@@ -52,6 +52,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet manual
+
  auto lo
  iface lo inet loopback
  

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_add_and_delete_aggi_up.json

@@ -54,6 +54,14 @@
             "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi"
         ]
     },
+    "eth1": {
+        "address_family": "inet",
+        "down": [],
+        "method": "manual",
+        "post-up": [],
+        "pre-up": [],
+        "up": []
+    },
     "ext1": {
         "address_family": "inet",
         "bond-master": "agge",

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_ipv4

@@ -52,6 +52,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet manual
+
  auto lo
  iface lo inet loopback
  

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_ipv4.json

@@ -54,6 +54,14 @@
             "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi"
         ]
     },
+    "eth1": {
+        "address_family": "inet",
+        "down": [],
+        "method": "manual",
+        "post-up": [],
+        "pre-up": [],
+        "up": []
+    },
     "ext1": {
         "address_family": "inet",
         "bond-master": "agge",

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_ipv4_post_up

@@ -52,6 +52,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet manual
+
  auto lo
  iface lo inet loopback
  

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_ipv4_post_up.json

@@ -54,6 +54,14 @@
             "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi"
         ]
     },
+    "eth1": {
+        "address_family": "inet",
+        "down": [],
+        "method": "manual",
+        "post-up": [],
+        "pre-up": [],
+        "up": []
+    },
     "ext1": {
         "address_family": "inet",
         "bond-master": "agge",

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_ipv4_pre_up

@@ -52,6 +52,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet manual
+
  auto lo
  iface lo inet loopback
  

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_ipv4_pre_up.json

@@ -54,6 +54,14 @@
             "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi"
         ]
     },
+    "eth1": {
+        "address_family": "inet",
+        "down": [],
+        "method": "manual",
+        "post-up": [],
+        "pre-up": [],
+        "up": []
+    },
     "ext1": {
         "address_family": "inet",
         "bond-master": "agge",

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_ipv6

@@ -52,6 +52,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet manual
+
  auto lo
  iface lo inet loopback
  

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_ipv6.json

@@ -54,6 +54,14 @@
             "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi"
         ]
     },
+    "eth1": {
+        "address_family": "inet",
+        "down": [],
+        "method": "manual",
+        "post-up": [],
+        "pre-up": [],
+        "up": []
+    },
     "ext1": {
         "address_family": "inet",
         "bond-master": "agge",

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_ipv6_post_up

@@ -52,6 +52,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet manual
+
  auto lo
  iface lo inet loopback
  

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_ipv6_post_up.json

@@ -54,6 +54,14 @@
             "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi"
         ]
     },
+    "eth1": {
+        "address_family": "inet",
+        "down": [],
+        "method": "manual",
+        "post-up": [],
+        "pre-up": [],
+        "up": []
+    },
     "ext1": {
         "address_family": "inet",
         "bond-master": "agge",

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_ipv6_pre_up

@@ -52,6 +52,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet manual
+
  auto lo
  iface lo inet loopback
  

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_ipv6_pre_up.json

@@ -54,6 +54,14 @@
             "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi"
         ]
     },
+    "eth1": {
+        "address_family": "inet",
+        "down": [],
+        "method": "manual",
+        "post-up": [],
+        "pre-up": [],
+        "up": []
+    },
     "ext1": {
         "address_family": "inet",
         "bond-master": "agge",

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_method

@@ -52,6 +52,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet dhcp
+
  auto lo
  iface lo inet loopback
  

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_method.exceptions.txt

@@ -1,8 +0,0 @@
-fail_json message: Error: interface eth0 not found
-options:
-{
-    "iface": "eth0",
-    "option": "method",
-    "state": "present",
-    "value": "manual"
-}

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_change_method.json

@@ -54,6 +54,14 @@
             "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi"
         ]
     },
+    "eth1": {
+        "address_family": "inet",
+        "down": [],
+        "method": "dhcp",
+        "post-up": [],
+        "pre-up": [],
+        "up": []
+    },
     "ext1": {
         "address_family": "inet",
         "bond-master": "agge",

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert

@@ -52,6 +52,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet manual
+
  auto lo
  iface lo inet loopback
  

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_revert.json

@@ -54,6 +54,14 @@
             "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi"
         ]
     },
+    "eth1": {
+        "address_family": "inet",
+        "down": [],
+        "method": "manual",
+        "post-up": [],
+        "pre-up": [],
+        "up": []
+    },
     "ext1": {
         "address_family": "inet",
         "bond-master": "agge",

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_set_aggi_and_eth0_mtu

@@ -52,6 +52,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet manual
+
  auto lo
  iface lo inet loopback
  

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_set_aggi_and_eth0_mtu.json

@@ -54,6 +54,14 @@
             "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi"
         ]
     },
+    "eth1": {
+        "address_family": "inet",
+        "down": [],
+        "method": "manual",
+        "post-up": [],
+        "pre-up": [],
+        "up": []
+    },
     "ext1": {
         "address_family": "inet",
         "bond-master": "agge",

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_set_aggi_slaves

@@ -52,6 +52,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet manual
+
  auto lo
  iface lo inet loopback
  

test/units/modules/system/interfaces_file/fixtures/golden_output/servers.com_set_aggi_slaves.json

@@ -54,6 +54,14 @@
             "route add -net 188.44.208.0/21 gw 10.44.15.117 dev aggi"
         ]
     },
+    "eth1": {
+        "address_family": "inet",
+        "down": [],
+        "method": "manual",
+        "post-up": [],
+        "pre-up": [],
+        "up": []
+    },
     "ext1": {
         "address_family": "inet",
         "bond-master": "agge",

test/units/modules/system/interfaces_file/fixtures/input/servers.com

@@ -52,6 +52,9 @@
  iface ext2 inet manual
  bond-master agge
  
+ auto eth1
+ iface eth1 inet manual
+
  auto lo
  iface lo inet loopback
  

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

@@ -205,9 +205,9 @@ class TestInterfacesFileModule(unittest.TestCase):
         testcases = {
             "change_method": [
                 {
-                    'iface': 'eth0',
+                    'iface': 'eth1',
                     'option': 'method',
-                    'value': 'manual',
+                    'value': 'dhcp',
                     'state': 'present',
                 }
             ],
@@ -223,7 +223,15 @@ class TestInterfacesFileModule(unittest.TestCase):
                     options = options_list[0]
                     fail_json_iterations = []
                     try:
-                        _, lines = interfaces_file.setInterfaceOption(module, lines, options['iface'], options['option'], options['value'], options['state'])
+                        changed, lines = interfaces_file.setInterfaceOption(module, lines, options['iface'], options['option'],
+                                                                            options['value'], options['state'])
+                        # When a changed is made try running it again for proper idempotency
+                        if changed:
+                            changed_again, lines = interfaces_file.setInterfaceOption(module, lines, options['iface'],
+                                                                                      options['option'], options['value'], options['state'])
+                            self.assertFalse(changed_again,
+                                             msg='Second request for change should return false for {0} running on {1}'.format(testname,
+                                                                                                                               testfile))
                     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=(',', ': '))))