fix nxos_vpc issues (#35868)

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

@@ -74,12 +74,12 @@ options:
   peer_gw:
     description:
       - Enables/Disables peer gateway
-    required: true
+    required: false
     choices: ['true','false']
   auto_recovery:
     description:
       - Enables/Disables auto recovery
-    required: true
+    required: false
     choices: ['true','false']
   delay_restore:
     description:
@@ -125,6 +125,7 @@ commands:
             "auto-recovery", "peer-gateway"]
 '''
 
+import re
 from ansible.module_utils.network.nxos.nxos import get_config, load_config, run_commands
 from ansible.module_utils.network.nxos.nxos import nxos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
@@ -138,6 +139,13 @@ CONFIG_ARGS = {
     'auto_recovery': '{auto_recovery} auto-recovery',
 }
 
+PARAM_TO_DEFAULT_KEYMAP = {
+    'delay_restore': '60',
+    'role_priority': '32667',
+    'system_priority': '32667',
+    'peer_gw': False,
+}
+
 
 def flatten_list(command_lists):
     flat_command_list = []
@@ -165,65 +173,60 @@ def get_vrf_list(module):
     return vrf_list
 
 
+def get_auto_recovery_default(module):
+    auto = False
+    data = run_commands(module, ['show inventory | json'])[0]
+    pid = data['TABLE_inv']['ROW_inv'][0]['productid']
+    if re.search(r'N7K', pid):
+        auto = True
+    elif re.search(r'N9K', pid):
+        data = run_commands(module, ['show hardware | json'])[0]
+        ver = data['kickstart_ver_str']
+        if re.search(r'7.0\(3\)F', ver):
+            auto = True
+
+    return auto
+
+
 def get_vpc(module):
     body = run_commands(module, ['show vpc | json'])[0]
-
     domain = str(body['vpc-domain-id'])
-    auto_recovery = 'enabled' in str(body['vpc-auto-recovery-status']).lower()
-
     vpc = {}
     if domain != 'not configured':
-        delay_restore = None
-        pkl_src = None
-        role_priority = '32667'
-        system_priority = None
-        pkl_dest = None
-        pkl_vrf = None
-        peer_gw = False
-
         run = get_config(module, flags=['vpc'])
         if run:
+            vpc['domain'] = domain
+            for key in PARAM_TO_DEFAULT_KEYMAP.keys():
+                vpc[key] = PARAM_TO_DEFAULT_KEYMAP.get(key)
+            vpc['auto_recovery'] = get_auto_recovery_default(module)
             vpc_list = run.split('\n')
             for each in vpc_list:
-                if 'delay restore' in each:
-                    line = each.split()
-                    if len(line) == 3:
-                        delay_restore = line[-1]
-                if 'peer-keepalive destination' in each:
-                    line = each.split()
-                    pkl_dest = line[2]
-                    for word in line:
-                        if 'source' in word:
-                            index = line.index(word)
-                            pkl_src = line[index + 1]
                 if 'role priority' in each:
                     line = each.split()
-                    role_priority = line[-1]
+                    vpc['role_priority'] = line[-1]
                 if 'system-priority' in each:
                     line = each.split()
-                    system_priority = line[-1]
+                    vpc['system_priority'] = line[-1]
+                if 'delay restore' in each:
+                    line = each.split()
+                    vpc['delay_restore'] = line[-1]
+                if 'no auto-recovery' in each:
+                    vpc['auto_recovery'] = False
+                elif 'auto-recovery' in each:
+                    vpc['auto_recovery'] = True
                 if 'peer-gateway' in each:
-                    peer_gw = True
-
-        body = run_commands(module, ['show vpc peer-keepalive | json'])[0]
-
-        if body:
-            pkl_dest = body['vpc-keepalive-dest']
-            if 'N/A' in pkl_dest:
-                pkl_dest = None
-            elif len(pkl_dest) == 2:
-                pkl_dest = pkl_dest[0]
-            pkl_vrf = str(body['vpc-keepalive-vrf'])
-
-        vpc['domain'] = domain
-        vpc['auto_recovery'] = auto_recovery
-        vpc['delay_restore'] = delay_restore
-        vpc['pkl_src'] = pkl_src
-        vpc['role_priority'] = role_priority
-        vpc['system_priority'] = system_priority
-        vpc['pkl_dest'] = pkl_dest
-        vpc['pkl_vrf'] = pkl_vrf
-        vpc['peer_gw'] = peer_gw
+                    vpc['peer_gw'] = True
+                if 'peer-keepalive destination' in each:
+                    line = each.split()
+                    vpc['pkl_dest'] = line[2]
+                    vpc['pkl_vrf'] = 'management'
+                    if 'source' in each:
+                        vpc['pkl_src'] = line[4]
+                        if 'vrf' in each:
+                            vpc['pkl_vrf'] = line[6]
+                    else:
+                        if 'vrf' in each:
+                            vpc['pkl_vrf'] = line[4]
 
     return vpc
 
@@ -232,40 +235,24 @@ def get_commands_to_config_vpc(module, vpc, domain, existing):
     vpc = dict(vpc)
 
     domain_only = vpc.get('domain')
-    pkl_src = vpc.get('pkl_src')
-    pkl_dest = vpc.get('pkl_dest')
-    pkl_vrf = vpc.get('pkl_vrf') or existing.get('pkl_vrf')
-    vpc['pkl_vrf'] = pkl_vrf
 
     commands = []
-    if pkl_src or pkl_dest:
-        if pkl_src is None:
-            vpc['pkl_src'] = existing.get('pkl_src')
-        elif pkl_dest is None:
-            vpc['pkl_dest'] = existing.get('pkl_dest')
-        pkl_command = 'peer-keepalive destination {pkl_dest}'.format(**vpc) \
-                      + ' source {pkl_src} vrf {pkl_vrf}'.format(**vpc)
+    if 'pkl_dest' in vpc:
+        pkl_command = 'peer-keepalive destination {pkl_dest}'.format(**vpc)
+        if 'pkl_src' in vpc:
+            pkl_command += ' source {pkl_src}'.format(**vpc)
+        if 'pkl_vrf' in vpc and vpc['pkl_vrf'] != 'management':
+            pkl_command += ' vrf {pkl_vrf}'.format(**vpc)
         commands.append(pkl_command)
-    elif pkl_vrf:
-        pkl_src = existing.get('pkl_src')
-        pkl_dest = existing.get('pkl_dest')
-        if pkl_src and pkl_dest:
-            pkl_command = ('peer-keepalive destination {0}'
-                           ' source {1} vrf {2}'.format(pkl_dest, pkl_src, pkl_vrf))
-            commands.append(pkl_command)
-
-    if vpc.get('auto_recovery') is False:
-        vpc['auto_recovery'] = 'no'
-    else:
-        vpc['auto_recovery'] = ''
 
-    if 'peer_gw' in vpc:
-        if vpc.get('peer_gw') is False:
-            vpc['peer_gw'] = 'no'
+    if 'auto_recovery' in vpc:
+        if not vpc.get('auto_recovery'):
+            vpc['auto_recovery'] = 'no'
         else:
-            vpc['peer_gw'] = ''
-    else:
-        if existing.get('peer_gw') is False:
+            vpc['auto_recovery'] = ''
+
+    if 'peer_gw' in vpc:
+        if not vpc.get('peer_gw'):
             vpc['peer_gw'] = 'no'
         else:
             vpc['peer_gw'] = ''
@@ -283,14 +270,6 @@ def get_commands_to_config_vpc(module, vpc, domain, existing):
     return commands
 
 
-def get_commands_to_remove_vpc_interface(portchannel, config_value):
-    commands = []
-    command = 'no vpc {0}'.format(config_value)
-    commands.append(command)
-    commands.insert(0, 'interface port-channel{0}'.format(portchannel))
-    return commands
-
-
 def main():
     argument_spec = dict(
         domain=dict(required=True, type='str'),
@@ -298,9 +277,9 @@ def main():
         system_priority=dict(required=False, type='str'),
         pkl_src=dict(required=False),
         pkl_dest=dict(required=False),
-        pkl_vrf=dict(required=False, default='management'),
-        peer_gw=dict(required=True, type='bool'),
-        auto_recovery=dict(required=True, type='bool'),
+        pkl_vrf=dict(required=False),
+        peer_gw=dict(required=False, type='bool'),
+        auto_recovery=dict(required=False, type='bool'),
         delay_restore=dict(required=False, type='str'),
         state=dict(choices=['absent', 'present'], default='present'),
     )
@@ -331,18 +310,17 @@ def main():
                 auto_recovery=auto_recovery,
                 delay_restore=delay_restore)
 
-    if not (pkl_src and pkl_dest and pkl_vrf):
-        # if only the source or dest is set, it'll fail and ask to set the
-        # other
-        if pkl_src or pkl_dest:
-            module.fail_json(msg='source AND dest IP for pkl are required at '
-                                 'this time (although source is technically not '
-                                 ' required by the device.)')
-
-        args.pop('pkl_src')
-        args.pop('pkl_dest')
-        args.pop('pkl_vrf')
-
+    if not pkl_dest:
+        if pkl_src:
+            module.fail_json(msg='dest IP for peer-keepalive is required'
+                                 ' when src IP is present')
+        elif pkl_vrf:
+            if pkl_vrf != 'management':
+                module.fail_json(msg='dest and src IP for peer-keepalive are required'
+                                     ' when vrf is present')
+            else:
+                module.fail_json(msg='dest IP for peer-keepalive is required'
+                                     ' when vrf is present')
     if pkl_vrf:
         if pkl_vrf.lower() not in get_vrf_list(module):
             module.fail_json(msg='The VRF you are trying to use for the peer '
@@ -353,7 +331,12 @@ def main():
 
     commands = []
     if state == 'present':
-        delta = set(proposed.items()).difference(existing.items())
+        delta = {}
+        for key, value in proposed.items():
+            if str(value).lower() == 'default':
+                value = PARAM_TO_DEFAULT_KEYMAP.get(key)
+            if existing.get(key) != value:
+                delta[key] = value
         if delta:
             command = get_commands_to_config_vpc(module, delta, domain, existing)
             commands.append(command)

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

@@ -19,14 +19,9 @@
     nxos_vpc: &conf_vpc
       state: present
       domain: 100
-      role_priority: 500
-      system_priority: 2000
       pkl_dest: 192.168.100.4
       pkl_src: 10.1.100.20
       pkl_vrf: ntc
-      peer_gw: true
-      delay_restore: 5
-      auto_recovery: true
       provider: "{{ connection }}"
     register: result
 
@@ -42,18 +37,134 @@
       that:
         - "result.changed == false"
 
+  - name: Configure vpc1
+    nxos_vpc: &conf_vpc1
+      state: present
+      domain: 100
+      role_priority: 500
+      system_priority: 2000
+      peer_gw: True
+      delay_restore: 5
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Conf Idempotence"
+    nxos_vpc: *conf_vpc1
+    register: result
+
+  - assert: *false
+
+  - block:
+    - name: Configure auto1
+      nxos_vpc: &auto_false
+        state: present
+        domain: 100
+        auto_recovery: False
+        provider: "{{ connection }}"
+      register: result
+
+    - assert: *true
+
+    - name: "Conf Idempotence"
+      nxos_vpc: *auto_false
+      register: result
+
+    - assert: *false
+
+    - name: Configure auto2
+      nxos_vpc: &auto_true
+        state: present
+        domain: 100
+        auto_recovery: True
+        provider: "{{ connection }}"
+      register: result
+
+    - assert: *true
+
+    - name: "Conf Idempotence"
+      nxos_vpc: *auto_true
+      register: result
+
+    - assert: *false
+
+    when: (platform is search("N7K|N9K-F"))
+
+  - block:
+    - name: Configure auto1
+      nxos_vpc: &auto_true1
+        state: present
+        domain: 100
+        auto_recovery: True
+        provider: "{{ connection }}"
+      register: result
+
+    - assert: *true
+
+    - name: "Conf Idempotence"
+      nxos_vpc: *auto_true1
+      register: result
+
+    - assert: *false
+
+    - name: Configure auto2
+      nxos_vpc: &auto_false1
+        state: present
+        domain: 100
+        auto_recovery: False
+        provider: "{{ connection }}"
+      register: result
+
+    - assert: *true
+
+    - name: "Conf Idempotence"
+      nxos_vpc: *auto_false1
+      register: result
+
+    - assert: *false
+
+    when: not (platform is search("N7K|N9K-F"))
+
+  - name: Configure vpc2
+    nxos_vpc: &conf_vpc2
+      state: present
+      domain: 100
+      role_priority: default
+      system_priority: default
+      peer_gw: True
+      delay_restore: default
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Conf Idempotence"
+    nxos_vpc: *conf_vpc2
+    register: result
+
+  - assert: *false
+
+  - name: Configure vpc3
+    nxos_vpc: &conf_vpc3
+      state: present
+      domain: 100
+      peer_gw: False
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Conf Idempotence"
+    nxos_vpc: *conf_vpc3
+    register: result
+
+  - assert: *false
+
   - name: remove vpc
     nxos_vpc: &rem_vpc
       state: absent
       domain: 100
-      role_priority: 32667
-      system_priority: 2000
-      pkl_dest: 192.168.100.4
-      pkl_src: 10.1.100.20
-      pkl_vrf: ntc
-      peer_gw: true
-      delay_restore: 5
-      auto_recovery: false
       provider: "{{ connection }}"
     register: result
 

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

@@ -61,6 +61,6 @@ class TestNxosVpcModule(TestNxosModule):
                              peer_gw=True, auto_recovery=True))
         self.execute_module(changed=True, commands=[
             'vpc domain 100', 'terminal dont-ask', 'role priority 32667', 'system-priority 2000',
-            'peer-keepalive destination 192.168.100.4 source 10.1.100.20 vrf management',
+            'peer-keepalive destination 192.168.100.4 source 10.1.100.20',
             'peer-gateway', 'auto-recovery',
         ])