fix nxos_pim_interface issues (#35405)

* fix nxos_pim_interface issues

* add absent test for pim_interface

lib/ansible/modules/network/nxos/nxos_pim_interface.py

@@ -52,7 +52,7 @@ options:
     description:
       - Enable/disable sparse-mode on the interface.
     required: false
-    default: true
+    default: false
     choices: ['true', 'false']
   dr_prio:
     description:
@@ -94,7 +94,7 @@ options:
     description:
       - Configures interface to be a boundary of a PIM domain.
     required: false
-    default: null
+    default: false
     choices: ['true', 'false']
   neighbor_policy:
     description:
@@ -172,6 +172,14 @@ PARAM_TO_COMMAND_KEYMAP = {
     'neighbor_type': '',
 }
 
+PARAM_TO_DEFAULT_KEYMAP = {
+    'dr_prio': '1',
+    'hello_interval': '30000',
+    'sparse': False,
+    'border': False,
+    'hello_auth_key': False,
+}
+
 
 def execute_show_command(command, module, text=False):
     if text:
@@ -249,156 +257,68 @@ def get_interface_mode(interface, intf_type, module):
     return mode
 
 
-def get_jp_policy_out(module, interface):
-    '''
-    This is a workaround for an nxos structured output problem.
-    The 'show ip pim interface' command does not display jp_policy_out
-    information properly for all supported nxos platforms when using
-    structured output.  This method uses the show running information to
-    mitigate the problem.
-    '''
-    command = 'sh run interface {0} all | grep \"jp.*out\"'.format(interface)
-    name = None
-    try:
-        body = execute_show_command(command, module, text=True)[0]
-    except IndexError:
-        return name
-
-    if body:
-        mo = re.search(r'ip pim jp-policy\s+(\S+)\s+out', body)
-        if mo:
-            name = mo.group(1)
-
-    return name
-
-
 def get_pim_interface(module, interface):
     pim_interface = {}
-    command = 'show ip pim interface {0}'.format(interface)
-    body = execute_show_command(command, module, text=True)
-
-    if body:
-        if 'not running' not in body[0]:
-            body = execute_show_command(command, module)
-
-    # Some nxos platforms have the TABLE_vrf/ROW_vrf key and some don't
-    try:
-        get_data = body[0]['TABLE_vrf']['ROW_vrf']['TABLE_iod']['ROW_iod']
-    except (KeyError, AttributeError, TypeError, IndexError):
-        try:
-            get_data = body[0]['TABLE_iod']['ROW_iod']
-        except (KeyError, AttributeError, TypeError, IndexError):
-            return pim_interface
-
-    if isinstance(get_data.get('dr-priority'), list):
-        pim_interface['dr_prio'] = get_data.get('dr-priority')[0]
-    else:
-        pim_interface['dr_prio'] = str(get_data.get('dr-priority'))
-
-    hello_interval = get_data.get('hello-interval-sec')
-    if hello_interval:
-        hello_interval_msec = int(get_data.get('hello-interval-sec')) * 1000
-        pim_interface['hello_interval'] = str(hello_interval_msec)
-
-    border = get_data.get('is-border')
-    border = border.lower() if border else border
-    if border == 'true':
-        pim_interface['border'] = True
-    elif border == 'false':
-        pim_interface['border'] = False
-
-    isauth = get_data.get('isauth-config')
-    isauth = isauth.lower() if isauth else isauth
-    if isauth == 'true':
-        pim_interface['isauth'] = True
-    elif isauth == 'false':
-        pim_interface['isauth'] = False
-
-    pim_interface['neighbor_policy'] = get_data.get('nbr-policy-name')
-    if pim_interface['neighbor_policy'] == 'none configured':
-        pim_interface['neighbor_policy'] = None
-
-    jp_in_policy = get_data.get('jp-in-policy-name')
-    pim_interface['jp_policy_in'] = jp_in_policy
-    if jp_in_policy == 'none configured':
-        pim_interface['jp_policy_in'] = None
-
-    pim_interface['jp_policy_out'] = get_jp_policy_out(module, interface)
-
     body = get_config(module, flags=['interface {0}'.format(interface)])
 
-    jp_configs = []
+    pim_interface['neighbor_type'] = None
-    neigh = None
+    pim_interface['neighbor_policy'] = None
+    pim_interface['jp_policy_in'] = None
+    pim_interface['jp_policy_out'] = None
+    pim_interface['jp_type_in'] = None
+    pim_interface['jp_type_out'] = None
+    pim_interface['jp_bidir'] = False
+    pim_interface['isauth'] = False
+
     if body:
         all_lines = body.splitlines()
 
         for each in all_lines:
             if 'jp-policy' in each:
-                jp_configs.append(str(each.strip()))
+                policy_name = \
+                    re.search(r'ip pim jp-policy(?: prefix-list)? (\S+)(?: \S+)?', each).group(1)
+                if 'prefix-list' in each:
+                    ptype = 'prefix'
+                else:
+                    ptype = 'routemap'
+                if 'out' in each:
+                    pim_interface['jp_policy_out'] = policy_name
+                    pim_interface['jp_type_out'] = ptype
+                elif 'in' in each:
+                    pim_interface['jp_policy_in'] = policy_name
+                    pim_interface['jp_type_in'] = ptype
+                else:
+                    pim_interface['jp_policy_in'] = policy_name
+                    pim_interface['jp_policy_out'] = policy_name
+                    pim_interface['jp_bidir'] = True
             elif 'neighbor-policy' in each:
-                neigh = str(each)
+                pim_interface['neighbor_policy'] = \
+                    re.search(r'ip pim neighbor-policy(?: prefix-list)? (\S+)', each).group(1)
+                if 'prefix-list' in each:
+                    pim_interface['neighbor_type'] = 'prefix'
+                else:
+                    pim_interface['neighbor_type'] = 'routemap'
+            elif 'ah-md5' in each:
+                pim_interface['isauth'] = True
             elif 'sparse-mode' in each:
                 pim_interface['sparse'] = True
-
+            elif 'border' in each:
-    pim_interface['neighbor_type'] = None
+                pim_interface['border'] = True
-    neigh_type = None
+            elif 'hello-interval' in each:
-    if neigh:
+                pim_interface['hello_interval'] = \
-        if 'prefix-list' in neigh:
+                    re.search(r'ip pim hello-interval (\d+)', body).group(1)
-            neigh_type = 'prefix'
+            elif 'dr-priority' in each:
-        else:
+                pim_interface['dr_prio'] = \
-            neigh_type = 'routemap'
+                    re.search(r'ip pim dr-priority (\d+)', body).group(1)
-    pim_interface['neighbor_type'] = neigh_type
-
-    len_existing = len(jp_configs)
-    list_of_prefix_type = len([x for x in jp_configs if 'prefix-list' in x])
-    jp_type_in = None
-    jp_type_out = None
-    jp_bidir = False
-    if len_existing == 1:
-        # determine type
-        last_word = jp_configs[0].split(' ')[-1]
-        if last_word == 'in':
-            if list_of_prefix_type:
-                jp_type_in = 'prefix'
-            else:
-                jp_type_in = 'routemap'
-        elif last_word == 'out':
-            if list_of_prefix_type:
-                jp_type_out = 'prefix'
-            else:
-                jp_type_out = 'routemap'
-        else:
-            jp_bidir = True
-            if list_of_prefix_type:
-                jp_type_in = 'prefix'
-                jp_type_out = 'routemap'
-            else:
-                jp_type_in = 'routemap'
-                jp_type_out = 'routemap'
-    else:
-        for each in jp_configs:
-            last_word = each.split(' ')[-1]
-            if last_word == 'in':
-                if 'prefix-list' in each:
-                    jp_type_in = 'prefix'
-                else:
-                    jp_type_in = 'routemap'
-            elif last_word == 'out':
-                if 'prefix-list' in each:
-                    jp_type_out = 'prefix'
-                else:
-                    jp_type_out = 'routemap'
-
-    pim_interface['jp_type_in'] = jp_type_in
-    pim_interface['jp_type_out'] = jp_type_out
-    pim_interface['jp_bidir'] = jp_bidir
 
     return pim_interface
 
 
 def fix_delta(delta, existing):
-    if delta.get('sparse') is False and existing.get('sparse') is None:
+    for key in list(delta):
-        delta.pop('sparse')
+        if key in ['dr_prio', 'hello_interval', 'sparse', 'border']:
+            if delta.get(key) == PARAM_TO_DEFAULT_KEYMAP.get(key) and existing.get(key) is None:
+                delta.pop(key)
     return delta
 
 
@@ -473,14 +393,12 @@ def config_pim_interface(delta, existing, jp_bidir, isauth):
 
 
 def get_pim_interface_defaults():
-    dr_prio = '1'
-    border = False
-    hello_interval = '30000'
-    hello_auth_key = False
 
-    args = dict(dr_prio=dr_prio, border=border,
+    args = dict(dr_prio=PARAM_TO_DEFAULT_KEYMAP.get('dr_prio'),
-                hello_interval=hello_interval,
+                border=PARAM_TO_DEFAULT_KEYMAP.get('border'),
-                hello_auth_key=hello_auth_key)
+                sparse=PARAM_TO_DEFAULT_KEYMAP.get('sparse'),
+                hello_interval=PARAM_TO_DEFAULT_KEYMAP.get('hello_interval'),
+                hello_auth_key=PARAM_TO_DEFAULT_KEYMAP.get('hello_auth_key'))
 
     default = dict((param, value) for (param, value) in args.items()
                    if value is not None)
@@ -554,7 +472,7 @@ def config_pim_interface_defaults(existing, jp_bidir, isauth):
 def main():
     argument_spec = dict(
         interface=dict(required=True),
-        sparse=dict(type='bool', default=True),
+        sparse=dict(type='bool', default=False),
         dr_prio=dict(type='str'),
         hello_auth_key=dict(type='str'),
         hello_interval=dict(type='int'),
@@ -562,7 +480,7 @@ def main():
         jp_policy_in=dict(type='str'),
         jp_type_out=dict(choices=['prefix', 'routemap']),
         jp_type_in=dict(choices=['prefix', 'routemap']),
-        border=dict(type='bool'),
+        border=dict(type='bool', default=False),
         neighbor_policy=dict(type='str'),
         neighbor_type=dict(choices=['prefix', 'routemap']),
         state=dict(choices=['present', 'absent', 'default'], default='present'),
@@ -617,22 +535,11 @@ def main():
             command = config_pim_interface(delta, existing, jp_bidir, isauth)
             if command:
                 commands.append(command)
-    elif state == 'default':
+    elif state == 'default' or state == 'absent':
         defaults = config_pim_interface_defaults(existing, jp_bidir, isauth)
         if defaults:
             commands.append(defaults)
 
-    elif state == 'absent':
-        if existing.get('sparse') is True:
-            delta['sparse'] = False
-            # defaults is a list of commands
-            defaults = config_pim_interface_defaults(existing, jp_bidir, isauth)
-            if defaults:
-                commands.append(defaults)
-
-            command = config_pim_interface(delta, existing, jp_bidir, isauth)
-            commands.append(command)
-
     if commands:
         commands.insert(0, ['interface {0}'.format(interface)])
 

test/integration/targets/nxos_pim_interface/tests/common/sanity.yaml

@@ -33,22 +33,22 @@
     state: present
     provider: "{{ connection }}"
 
-- name: Configure nxos_pim_interface state absent
-  nxos_pim_interface:
-    interface: "{{ testint }}"
-    state: absent
-    provider: "{{ connection }}"
-
 - block:
-  - name: configure pim interface
+  - name: Configure nxos_pim_interface state absent
-    nxos_pim_interface: &config
+    nxos_pim_interface: &pimabsent
       interface: "{{ testint }}"
-      dr_prio: 10
+      state: absent
-      hello_interval: 40
+      provider: "{{ connection }}"
-      border: 'false'
+
-      neighbor_policy: 'ansible_policy'
+  - name: configure jp policy and type
-      neighbor_type: 'prefix'
+    nxos_pim_interface: &configjp
-      state: present
+      interface: "{{ testint }}"
+      jp_policy_in: JPIN
+      jp_policy_out: JPOUT
+      jp_type_in: routemap
+      jp_type_out: routemap
+      sparse: True
+      border: True
       provider: "{{ connection }}"
     register: result
 
@@ -57,31 +57,96 @@
         - "result.changed == true"
 
   - name: Check idempotence
-    nxos_pim_interface: *config
+    nxos_pim_interface: *configjp
     register: result
 
   - assert: &false
       that:
         - "result.changed == false"
 
-  - name: configure gp policy and type
+  - name: configure neighbor policy and rm
-    nxos_pim_interface: &configjp
+    nxos_pim_interface: &confignpr
       interface: "{{ testint }}"
-      jp_policy_in: JPIN
+      neighbor_policy: NPR
-      jp_policy_out: JPOUT
+      neighbor_type: routemap
-      jp_type_in: routemap
-      jp_type_out: routemap
       provider: "{{ connection }}"
     register: result
 
   - assert: *true
 
   - name: Check idempotence
-    nxos_pim_interface: *configjp
+    nxos_pim_interface: *confignpr
     register: result
 
   - assert: *false
 
+  - pause:
+      seconds: 5
+
+  - name: configure neighbor policy and prefix
+    nxos_pim_interface: &confignpp
+      interface: "{{ testint }}"
+      neighbor_policy: NPPF
+      neighbor_type: prefix
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: Check idempotence
+    nxos_pim_interface: *confignpp
+    register: result
+
+  - assert: *false
+
+  - name: configure hello_auth_key
+    nxos_pim_interface: &confighak1
+      interface: "{{ testint }}"
+      hello_auth_key: password1
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: configure pim other params
+    nxos_pim_interface: &configo
+      interface: "{{ testint }}"
+      dr_prio: 10
+      hello_interval: 40
+      sparse: True
+      border: True
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: Check idempotence
+    nxos_pim_interface: *configo
+    register: result
+
+  - assert: *false
+
+  - name: configure negative
+    nxos_pim_interface: &configno
+      interface: "{{ testint }}"
+      sparse: False
+      border: False
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: Check idempotence
+    nxos_pim_interface: *configno
+    register: result
+
+  - assert: *false
+
+  - pause:
+      seconds: 5
+
   - name: configure state default
     nxos_pim_interface: &configdefault
       interface: "{{ testint }}"
@@ -97,6 +162,34 @@
 
   - assert: *false
 
+  - name: configure border
+    nxos_pim_interface: &configb
+      interface: "{{ testint }}"
+      border: True
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: Check idempotence
+    nxos_pim_interface: *configb
+    register: result
+
+  - assert: *false
+
+  - name: configure state absent
+    nxos_pim_interface: *pimabsent
+    register: result
+
+  - assert: *true
+
+  - name: Check idempotence
+    nxos_pim_interface: *pimabsent
+    register: result
+
+  - assert: *false
+
   always:
   - name: "Disable feature PIM"
     nxos_feature: *disable_feature

test/units/modules/network/nxos/test_nxos_pim_interface.py

@@ -70,7 +70,7 @@ class TestNxosIPInterfaceModule(TestNxosModule):
             changed=True,
             commands=[
                 'interface eth2/1', 'ip pim dr-priority 10', 'ip pim hello-interval 40000',
-                'ip pim sparse-mode', 'no ip pim border']
+                'ip pim sparse-mode']
         )
 
     def test_nxos_pim_interface_jp(self):
@@ -81,14 +81,14 @@ class TestNxosIPInterfaceModule(TestNxosModule):
         self.execute_module(
             changed=True,
             commands=['interface eth2/1', 'ip pim jp-policy JPOUT out',
-                      'ip pim jp-policy JPIN in', 'ip pim sparse-mode']
+                      'ip pim jp-policy JPIN in']
         )
 
     def test_nxos_pim_interface_default(self):
         set_module_args(dict(interface='eth2/1', state='default'))
         self.execute_module(
-            changed=True,
+            changed=False,
-            commands=['interface eth2/1', 'ip pim dr-priority 1', 'ip pim hello-interval 30000', 'no ip pim border']
+            commands=[]
         )
 
     def test_nxos_pim_interface_ip_absent(self):