ios_static_route idempotence fix (#35912)

* Remove default admin_distance and fix the idempotence thereof

Fixes #33290

* Fix tests and use yaml anchors to shorten tests

* Add test for undefined admin_distance

* Read config from `show run` if `show ip static route` fails

* Restore flags to ios.get_config &  use get_config where appropriate

lib/ansible/module_utils/network/ios/ios.py

@@ -108,15 +108,15 @@ def get_defaults_flag(module):
 
 
 def get_config(module, flags=None):
-    global _DEVICE_CONFIGS
+    flag_str = ' '.join(to_list(flags))
 
-    if _DEVICE_CONFIGS != {}:
-        return _DEVICE_CONFIGS
-    else:
+    try:
+        return _DEVICE_CONFIGS[flag_str]
+    except KeyError:
         connection = get_connection(module)
-        out = connection.get_config()
+        out = connection.get_config(flags=flags)
         cfg = to_text(out, errors='surrogate_then_replace').strip()
-        _DEVICE_CONFIGS = cfg
+        _DEVICE_CONFIGS[flag_str] = cfg
         return cfg
 
 

lib/ansible/modules/network/ios/ios_static_route.py

@@ -50,7 +50,6 @@ options:
   admin_distance:
     description:
       - Admin distance of the static route.
-    default: 1
   aggregate:
     description: List of static route definitions.
   state:
@@ -98,14 +97,14 @@ commands:
     - ip route 192.168.2.0 255.255.255.0 10.0.0.1
 """
 from copy import deepcopy
+import re
 
 from ansible.module_utils._text import to_text
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.connection import exec_command
+from ansible.module_utils.connection import ConnectionError
 from ansible.module_utils.network.common.utils import remove_default_spec
-from ansible.module_utils.network.ios.ios import load_config, run_commands
+from ansible.module_utils.network.ios.ios import get_config, load_config, run_commands
 from ansible.module_utils.network.ios.ios import ios_argument_spec, check_args
-import re
 
 try:
     from ipaddress import ip_network
@@ -114,23 +113,39 @@ except ImportError:
     HAS_IPADDRESS = False
 
 
-def map_obj_to_commands(updates, module):
+def map_obj_to_commands(want, have, module):
     commands = list()
-    want, have = updates
 
     for w in want:
+        # Try to match an existing config with the desired config
+        for h in have:
+            for key in ['prefix', 'mask', 'next_hop']:
+                # If any key doesn't match, skip to the next set
+                if w[key] != h[key]:
+                    break
+            # If all keys match, don't execute final else
+            else:
+                break
+        # If no matches found, clear `h`
+        else:
+            h = None
+
         prefix = w['prefix']
         mask = w['mask']
         next_hop = w['next_hop']
-        admin_distance = w['admin_distance']
+        admin_distance = w.get('admin_distance')
+        if not admin_distance and h:
+            w['admin_distance'] = admin_distance = h['admin_distance']
         state = w['state']
         del w['state']
 
         if state == 'absent' and w in have:
             commands.append('no ip route %s %s %s' % (prefix, mask, next_hop))
         elif state == 'present' and w not in have:
-            commands.append('ip route %s %s %s %s' % (prefix, mask, next_hop,
-                                                      admin_distance))
+            if admin_distance:
+                commands.append('ip route %s %s %s %s' % (prefix, mask, next_hop, admin_distance))
+            else:
+                commands.append('ip route %s %s %s' % (prefix, mask, next_hop))
 
     return commands
 
@@ -138,55 +153,82 @@ def map_obj_to_commands(updates, module):
 def map_config_to_obj(module):
     obj = []
 
-    rc, out, err = exec_command(module, 'show ip static route')
-    match = re.search(r'.*Static local RIB for default\s*(.*)$', out, re.DOTALL)
+    try:
+        out = run_commands(module, 'show ip static route')[0]
+        match = re.search(r'.*Static local RIB for default\s*(.*)$', out, re.DOTALL)
 
-    if match and match.group(1):
-        for r in match.group(1).splitlines():
-            splitted_line = r.split()
+        if match and match.group(1):
+            for r in match.group(1).splitlines():
+                splitted_line = r.split()
 
-            code = splitted_line[0]
+                code = splitted_line[0]
 
-            if code != 'M':
+                if code != 'M':
+                    continue
+
+                cidr = ip_network(to_text(splitted_line[1]))
+                prefix = str(cidr.network_address)
+                mask = str(cidr.netmask)
+                next_hop = splitted_line[4]
+                admin_distance = splitted_line[2][1]
+
+                obj.append({
+                    'prefix': prefix, 'mask': mask, 'next_hop': next_hop,
+                    'admin_distance': admin_distance
+                })
+
+    except ConnectionError:
+        out = get_config(module, flags='| include ip route')
+
+        for line in out.splitlines():
+            splitted_line = line.split()
+            if len(splitted_line) not in (5, 6):
                 continue
 
-            cidr = ip_network(to_text(splitted_line[1]))
-            prefix = str(cidr.network_address)
-            mask = str(cidr.netmask)
+            prefix = splitted_line[2]
+            mask = splitted_line[3]
             next_hop = splitted_line[4]
-            admin_distance = splitted_line[2][1]
+            if len(splitted_line) == 6:
+                admin_distance = splitted_line[5]
+            else:
+                admin_distance = '1'
 
-            obj.append({'prefix': prefix, 'mask': mask,
-                        'next_hop': next_hop,
-                        'admin_distance': admin_distance})
+            obj.append({
+                'prefix': prefix, 'mask': mask, 'next_hop': next_hop,
+                'admin_distance': admin_distance
+            })
 
     return obj
 
 
 def map_params_to_obj(module, required_together=None):
+    keys = ['prefix', 'mask', 'next_hop', 'admin_distance', 'state']
     obj = []
 
     aggregate = module.params.get('aggregate')
     if aggregate:
         for item in aggregate:
-            for key in item:
-                if item.get(key) is None:
-                    item[key] = module.params[key]
+            route = item.copy()
+            for key in keys:
+                if route.get(key) is None:
+                    route[key] = module.params.get(key)
 
-            module._check_required_together(required_together, item)
-            d = item.copy()
-            d['admin_distance'] = str(module.params['admin_distance'])
-
-            obj.append(d)
+            module._check_required_together(required_together, route)
+            obj.append(route)
     else:
+        module._check_required_together(required_together, module.params)
         obj.append({
             'prefix': module.params['prefix'].strip(),
             'mask': module.params['mask'].strip(),
             'next_hop': module.params['next_hop'].strip(),
-            'admin_distance': str(module.params['admin_distance']),
-            'state': module.params['state']
+            'admin_distance': module.params.get('admin_distance'),
+            'state': module.params['state'],
         })
 
+    for route in obj:
+        if route['admin_distance']:
+            route['admin_distance'] = str(route['admin_distance'])
+
     return obj
 
 
@@ -197,7 +239,7 @@ def main():
         prefix=dict(type='str'),
         mask=dict(type='str'),
         next_hop=dict(type='str'),
-        admin_distance=dict(default=1, type='int'),
+        admin_distance=dict(type='int'),
         state=dict(default='present', choices=['present', 'absent'])
     )
 
@@ -220,7 +262,6 @@ def main():
 
     module = AnsibleModule(argument_spec=argument_spec,
                            required_one_of=required_one_of,
-                           required_together=required_together,
                            mutually_exclusive=mutually_exclusive,
                            supports_check_mode=True)
 
@@ -236,7 +277,7 @@ def main():
     want = map_params_to_obj(module, required_together=required_together)
     have = map_config_to_obj(module)
 
-    commands = map_obj_to_commands((want, have), module)
+    commands = map_obj_to_commands(want, have, module)
     result['commands'] = commands
 
     if commands:
@@ -247,5 +288,6 @@ def main():
 
     module.exit_json(**result)
 
+
 if __name__ == '__main__':
     main()

lib/ansible/plugins/cliconf/ios.py

@@ -61,8 +61,7 @@ class Cliconf(CliconfBase):
         else:
             cmd = 'show startup-config'
 
-        flags = [] if flags is None else flags
-        cmd += ' '.join(flags)
+        cmd += ' '.join(to_list(flags))
         cmd = cmd.strip()
 
         return self.send_command(cmd)

test/integration/targets/ios_static_route/tests/cli/basic.yaml

@@ -1,15 +1,17 @@
 ---
 - debug: msg="START ios cli/ios_static_route.yaml on connection={{ ansible_connection }}"
 
-- name: delete static route - setup
-  net_static_route:
-    prefix: 172.16.31.0
+- name: Clear all static routes
+  ios_static_route: &delete_all
+    aggregate:
+      - { prefix: 172.16.31.0 }
+      - { prefix: 172.16.32.0 }
+      - { prefix: 172.16.33.0 }
+      - { prefix: 172.16.34.0 }
     mask: 255.255.255.0
     next_hop: 10.0.0.8
-    admin_distance: 1
     state: absent
     provider: "{{ cli }}"
-  register: result
 
 - name: create static route
   ios_static_route:
@@ -23,13 +25,14 @@
 - assert:
     that:
       - 'result.changed == true'
-      - 'result.commands == ["ip route 172.16.31.0 255.255.255.0 10.0.0.8 1"]'
+      - 'result.commands == ["ip route 172.16.31.0 255.255.255.0 10.0.0.8"]'
 
-- name: create static route again (idempotent)
+- name: Verify idempotence with default admin_distance
   ios_static_route:
     prefix: 172.16.31.0
     mask: 255.255.255.0
     next_hop: 10.0.0.8
+    admin_distance: 1
     state: present
     provider: "{{ cli }}"
   register: result
@@ -39,7 +42,7 @@
       - 'result.changed == false'
 
 - name: modify admin distance of static route
-  ios_static_route:
+  ios_static_route: &admin2
     prefix: 172.16.31.0
     mask: 255.255.255.0
     next_hop: 10.0.0.8
@@ -54,11 +57,18 @@
       - 'result.commands == ["ip route 172.16.31.0 255.255.255.0 10.0.0.8 2"]'
 
 - name: modify admin distance of static route again (idempotent)
+  ios_static_route: *admin2
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == false'
+
+- name: Verify idempotence with unspecified admin_distance
   ios_static_route:
     prefix: 172.16.31.0
     mask: 255.255.255.0
     next_hop: 10.0.0.8
-    admin_distance: 2
     state: present
     provider: "{{ cli }}"
   register: result
@@ -68,11 +78,10 @@
       - 'result.changed == false'
 
 - name: delete static route
-  ios_static_route:
+  ios_static_route: &delete
     prefix: 172.16.31.0
     mask: 255.255.255.0
     next_hop: 10.0.0.8
-    admin_distance: 2
     state: absent
     provider: "{{ cli }}"
   register: result
@@ -83,13 +92,7 @@
       - 'result.commands == ["no ip route 172.16.31.0 255.255.255.0 10.0.0.8"]'
 
 - name: delete static route again (idempotent)
-  ios_static_route:
-    prefix: 172.16.31.0
-    mask: 255.255.255.0
-    next_hop: 10.0.0.8
-    admin_distance: 2
-    state: absent
-    provider: "{{ cli }}"
+  ios_static_route: *delete
   register: result
 
 - assert:
@@ -99,8 +102,10 @@
 - name: Add static route aggregates
   ios_static_route:
     aggregate:
-      - { prefix: 172.16.32.0, mask: 255.255.255.0, next_hop: 10.0.0.8 }
-      - { prefix: 172.16.33.0, mask: 255.255.255.0, next_hop: 10.0.0.8 }
+      - { prefix: 172.16.32.0 }
+      - { prefix: 172.16.33.0 }
+    mask: 255.255.255.0
+    next_hop: 10.0.0.8
     state: present
     provider: "{{ cli }}"
   register: result
@@ -108,14 +113,16 @@
 - assert:
     that:
       - 'result.changed == true'
-      - 'result.commands == ["ip route 172.16.32.0 255.255.255.0 10.0.0.8 1", "ip route 172.16.33.0 255.255.255.0 10.0.0.8 1"]'
+      - 'result.commands == ["ip route 172.16.32.0 255.255.255.0 10.0.0.8", "ip route 172.16.33.0 255.255.255.0 10.0.0.8"]'
 
 - name: Add and remove static route aggregates with overrides
   ios_static_route:
     aggregate:
-      - { prefix: 172.16.32.0, mask: 255.255.255.0, next_hop: 10.0.0.8 }
-      - { prefix: 172.16.33.0, mask: 255.255.255.0, next_hop: 10.0.0.8, state: absent }
-      - { prefix: 172.16.34.0, mask: 255.255.255.0, next_hop: 10.0.0.8 }
+      - { prefix: 172.16.32.0 }
+      - { prefix: 172.16.33.0, state: absent }
+      - { prefix: 172.16.34.0 }
+    mask: 255.255.255.0
+    next_hop: 10.0.0.8
     state: present
     provider: "{{ cli }}"
   register: result
@@ -123,16 +130,10 @@
 - assert:
     that:
       - 'result.changed == true'
-      - 'result.commands == ["no ip route 172.16.33.0 255.255.255.0 10.0.0.8", "ip route 172.16.34.0 255.255.255.0 10.0.0.8 1"]'
+      - 'result.commands == ["no ip route 172.16.33.0 255.255.255.0 10.0.0.8", "ip route 172.16.34.0 255.255.255.0 10.0.0.8"]'
 
 - name: Remove static route aggregates
-  ios_static_route:
-    aggregate:
-      - { prefix: 172.16.32.0, mask: 255.255.255.0, next_hop: 10.0.0.8 }
-      - { prefix: 172.16.33.0, mask: 255.255.255.0, next_hop: 10.0.0.8 }
-      - { prefix: 172.16.34.0, mask: 255.255.255.0, next_hop: 10.0.0.8 }
-    state: absent
-    provider: "{{ cli }}"
+  ios_static_route: *delete_all
   register: result
 
 - assert:

test/integration/targets/ios_static_route/tests/cli/net_static_route.yaml

@@ -5,20 +5,19 @@
 # implementation module and module run is successful.
 
 - name: delete static route - setup
-  net_static_route:
+  net_static_route: &delete
     prefix: 172.16.31.0
     mask: 255.255.255.0
     next_hop: 10.0.0.8
-    admin_distance: 1
     state: absent
     provider: "{{ cli }}"
-  register: result
 
 - name: create static route using platform agnostic module
   net_static_route:
     prefix: 172.16.31.0
     mask: 255.255.255.0
     next_hop: 10.0.0.8
+    admin_distance: 1
     state: present
     provider: "{{ cli }}"
   register: result
@@ -29,13 +28,6 @@
       - 'result.commands == ["ip route 172.16.31.0 255.255.255.0 10.0.0.8 1"]'
 
 - name: delete static route - teardown
-  net_static_route:
-    prefix: 172.16.31.0
-    mask: 255.255.255.0
-    next_hop: 10.0.0.8
-    admin_distance: 2
-    state: absent
-    provider: "{{ cli }}"
-  register: result
+  net_static_route: *delete
 
 - debug: msg="END ios cli/net_static_route.yaml on connection={{ ansible_connection }}"