From 1554befd952a5f486fb6bf4b1650f4f569663db4 Mon Sep 17 00:00:00 2001 From: Chris Van Heuveln Date: Fri, 10 May 2019 03:47:02 -0400 Subject: [PATCH] nxos_vpc:Fix multiple idempotency issues, add missing commands (#55735) * nxos_vpc:Fix idempotency issues with multiple attributes Several attributes were causing idempotency problems on various platforms: - `auto_recovery` - This command can be disabled on certain platforms and will nvgen as `no auto-recovery` - When enabled it has an additional optional-keyword for changing the `reload-delay` timer value - This was addressed by adding a new attribute `auto_recovery_reload_delay` to handle setting the timer value - This new attribute is mutually exclusive with `auto_recovery` - `/show run vpc/show run vpc all/` - Changed the command that gets state to `all` so that it could differentiate between `auto-recovery` and `auto-recovery reload-delay` - This change resulted in also changing some attribute handling withing `get_vpc`, since some attributes like `peer_gw` relied on presence of the config to determine state true or false. With `all` the config is always there so these attrs must specifically check for `'no '` in the string. - `delay_restore` - This command has two additional, optional keywords that exist on some platforms and not others. - New attrs: - `delay_restore_interface_vlan` - `delay_restore_orphan_port` - Modified the `sanity` test to include the new attributes and to fix the platform issues. - Bugfix Pull Request `modules/network/nxos/nxos_vpc.py` - Validated `nxos_vpc` `sanity` test on these platforms, all are now 100% Pass: N35, N3K, N3K-F, N6K, N7K, N9K, N9K-F - TBD: Future work is needed to add support for `peer_gw_exclude_gw` timers. This could be addressed in the same way as the `auto_recovery_reload_delay` changes included here. * lint fix * Add 'version_added' tags for new options --- lib/ansible/modules/network/nxos/nxos_vpc.py | 68 ++++++++++++--- .../targets/nxos_vpc/tests/common/sanity.yaml | 87 +++++++++---------- 2 files changed, 101 insertions(+), 54 deletions(-) diff --git a/lib/ansible/modules/network/nxos/nxos_vpc.py b/lib/ansible/modules/network/nxos/nxos_vpc.py index 517f9c9c46..b4a1823d04 100644 --- a/lib/ansible/modules/network/nxos/nxos_vpc.py +++ b/lib/ansible/modules/network/nxos/nxos_vpc.py @@ -66,14 +66,39 @@ options: description: - Enables/Disables peer gateway type: bool + # TBD: no support for peer_gw_exclude_gw + # peer_gw_exclude_gw: + # description: + # - Range of VLANs to be excluded from peer-gateway + # type: str auto_recovery: description: - - Enables/Disables auto recovery + - Enables/Disables auto recovery on platforms that support disable + - timers are not modifiable with this attribute + - mutually exclusive with auto_recovery_reload_delay type: bool + auto_recovery_reload_delay: + description: + - Manages auto-recovery reload-delay timer in seconds + - mutually exclusive with auto_recovery + type: str + version_added: "2.9" delay_restore: description: - manages delay restore command and config value in seconds type: str + delay_restore_interface_vlan: + description: + - manages delay restore interface-vlan command and config value in seconds + - not supported on all platforms + type: str + version_added: "2.9" + delay_restore_orphan_port: + description: + - manages delay restore orphan-port command and config value in seconds + - not supported on all platforms + type: str + version_added: "2.9" state: description: - Manages desired state of the resource @@ -135,15 +160,21 @@ CONFIG_ARGS = { 'role_priority': 'role priority {role_priority}', 'system_priority': 'system-priority {system_priority}', 'delay_restore': 'delay restore {delay_restore}', + 'delay_restore_interface_vlan': 'delay restore interface-vlan {delay_restore_interface_vlan}', + 'delay_restore_orphan_port': 'delay restore orphan-port {delay_restore_orphan_port}', 'peer_gw': '{peer_gw} peer-gateway', 'auto_recovery': '{auto_recovery} auto-recovery', + 'auto_recovery_reload_delay': 'auto-recovery reload-delay {auto_recovery_reload_delay}', } PARAM_TO_DEFAULT_KEYMAP = { 'delay_restore': '60', + 'delay_restore_interface_vlan': '10', + 'delay_restore_orphan_port': '0', 'role_priority': '32667', 'system_priority': '32667', 'peer_gw': False, + 'auto_recovery_reload_delay': 240, } @@ -201,7 +232,7 @@ def get_vpc(module): vpc = {} if domain != 'not configured': - run = get_config(module, flags=['vpc']) + run = get_config(module, flags=['vpc all']) if run: vpc['domain'] = domain for key in PARAM_TO_DEFAULT_KEYMAP.keys(): @@ -215,15 +246,21 @@ def get_vpc(module): if 'system-priority' in each: line = each.split() vpc['system_priority'] = line[-1] - if 'delay restore' in each: + if re.search(r'delay restore \d+', 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 'delay restore interface-vlan' in each: + line = each.split() + vpc['delay_restore_interface_vlan'] = line[-1] + if 'delay restore orphan-port' in each: + line = each.split() + vpc['delay_restore_orphan_port'] = line[-1] + if 'auto-recovery' in each: + vpc['auto_recovery'] = False if 'no ' in each else True + line = each.split() + vpc['auto_recovery_reload_delay'] = line[-1] if 'peer-gateway' in each: - vpc['peer_gw'] = True + vpc['peer_gw'] = False if 'no ' in each else True if 'peer-keepalive destination' in each: line = each.split() vpc['pkl_dest'] = line[2] @@ -235,7 +272,6 @@ def get_vpc(module): else: if 'vrf' in each: vpc['pkl_vrf'] = line[4] - return vpc @@ -288,13 +324,18 @@ def main(): pkl_vrf=dict(required=False), peer_gw=dict(required=False, type='bool'), auto_recovery=dict(required=False, type='bool'), + auto_recovery_reload_delay=dict(required=False, type='str'), delay_restore=dict(required=False, type='str'), + delay_restore_interface_vlan=dict(required=False, type='str'), + delay_restore_orphan_port=dict(required=False, type='str'), state=dict(choices=['absent', 'present'], default='present'), ) argument_spec.update(nxos_argument_spec) + mutually_exclusive = [('auto_recovery', 'auto_recovery_reload_delay')] module = AnsibleModule(argument_spec=argument_spec, + mutually_exclusive=mutually_exclusive, supports_check_mode=True) warnings = list() @@ -309,14 +350,21 @@ def main(): pkl_vrf = module.params['pkl_vrf'] peer_gw = module.params['peer_gw'] auto_recovery = module.params['auto_recovery'] + auto_recovery_reload_delay = module.params['auto_recovery_reload_delay'] delay_restore = module.params['delay_restore'] + delay_restore_interface_vlan = module.params['delay_restore_interface_vlan'] + delay_restore_orphan_port = module.params['delay_restore_orphan_port'] state = module.params['state'] args = dict(domain=domain, role_priority=role_priority, system_priority=system_priority, pkl_src=pkl_src, pkl_dest=pkl_dest, pkl_vrf=pkl_vrf, peer_gw=peer_gw, auto_recovery=auto_recovery, - delay_restore=delay_restore) + auto_recovery_reload_delay=auto_recovery_reload_delay, + delay_restore=delay_restore, + delay_restore_interface_vlan=delay_restore_interface_vlan, + delay_restore_orphan_port=delay_restore_orphan_port, + ) if not pkl_dest: if pkl_src: diff --git a/test/integration/targets/nxos_vpc/tests/common/sanity.yaml b/test/integration/targets/nxos_vpc/tests/common/sanity.yaml index 771d4bbe58..09e48520fb 100644 --- a/test/integration/targets/nxos_vpc/tests/common/sanity.yaml +++ b/test/integration/targets/nxos_vpc/tests/common/sanity.yaml @@ -4,6 +4,17 @@ when: ansible_connection == "local" - block: + - set_fact: delay_restore_orphan_port=25 + - set_fact: def_delay_restore_orphan_port=default + when: platform is not search("N35|N5K|N6K") + +- block: + - name: disable vpc for initial vpc config cleanup + nxos_feature: + feature: vpc + provider: "{{ connection }}" + state: disabled + - name: enable feature vpc nxos_feature: feature: vpc @@ -46,6 +57,8 @@ system_priority: 2000 peer_gw: True delay_restore: 5 + delay_restore_interface_vlan: 15 + delay_restore_orphan_port: "{{ delay_restore_orphan_port|default(omit) }}" register: result - assert: *true @@ -57,74 +70,58 @@ - assert: *false - block: - - name: Configure auto1 - nxos_vpc: &auto_false + # This block is only useful on platforms that nvgen 'no auto-recovery'. + # Note: auto_recovery is mutually_exclusive with auto_recovery_reload_delay. + - set_fact: def_auto_recovery=False + - set_fact: def_auto_recovery=True + when: platform is search("N7K") + + - name: auto-recovery from default to non-default + nxos_vpc: &auto_recovery_1 provider: "{{ connection }}" - state: present domain: 100 - auto_recovery: False + auto_recovery: "{{ not def_auto_recovery }}" register: result - assert: *true - - name: "Conf Idempotence" - nxos_vpc: *auto_false + - name: "Conf Idempotence auto-recovery def-to-non-def" + nxos_vpc: *auto_recovery_1 register: result - assert: *false - - name: Configure auto2 - nxos_vpc: &auto_true + - name: auto-recovery from non-default to default + nxos_vpc: &auto_recovery_2 provider: "{{ connection }}" - state: present domain: 100 - auto_recovery: True + auto_recovery: "{{ def_auto_recovery }}" register: result - assert: *true - - name: "Conf Idempotence" - nxos_vpc: *auto_true + - name: "Conf Idempotence auto-recovery non-def-to-def" + nxos_vpc: *auto_recovery_2 register: result - assert: *false - when: (platform is search("N7K|N9K-F")) + when: platform is search("N35|N7K|N3K-F|N9K-F") - - block: - - name: Configure auto1 - nxos_vpc: &auto_true1 - provider: "{{ connection }}" - state: present - domain: 100 - auto_recovery: True - register: result + - name: Configure auto-recovery reload-delay + nxos_vpc: &auto_reload + provider: "{{ connection }}" + domain: 100 + auto_recovery_reload_delay: 242 + register: result - - assert: *true + - assert: *true - - name: "Conf Idempotence" - nxos_vpc: *auto_true1 - register: result + - name: "Conf Idempotence auto-recovery reload-delay" + nxos_vpc: *auto_reload + register: result - - assert: *false - - - name: Configure auto2 - nxos_vpc: &auto_false1 - provider: "{{ connection }}" - state: present - domain: 100 - auto_recovery: False - register: result - - - assert: *true - - - name: "Conf Idempotence" - nxos_vpc: *auto_false1 - register: result - - - assert: *false - - when: not (platform is search("N7K|N9K-F")) + - assert: *false - name: Configure vpc2 nxos_vpc: &conf_vpc2 @@ -135,6 +132,8 @@ system_priority: default peer_gw: True delay_restore: default + delay_restore_interface_vlan: default + delay_restore_orphan_port: "{{ def_delay_restore_orphan_port|default(omit) }}" register: result - assert: *true