From 80d19e6af3f06dc5dc5bc844459cbf6c872c20e6 Mon Sep 17 00:00:00 2001 From: saichint Date: Mon, 19 Feb 2018 20:38:17 -0800 Subject: [PATCH] fix nxos_bgp_neighbor issues (#36318) --- .../modules/network/nxos/nxos_bgp_neighbor.py | 25 +- .../nxos_bgp_neighbor/defaults/main.yaml | 3 + .../tests/common/sanity.yaml | 226 ++++++++++++++---- 3 files changed, 189 insertions(+), 65 deletions(-) diff --git a/lib/ansible/modules/network/nxos/nxos_bgp_neighbor.py b/lib/ansible/modules/network/nxos/nxos_bgp_neighbor.py index 310734232d4..62767c2beff 100644 --- a/lib/ansible/modules/network/nxos/nxos_bgp_neighbor.py +++ b/lib/ansible/modules/network/nxos/nxos_bgp_neighbor.py @@ -107,7 +107,8 @@ options: description: - Specify Maximum number of peers for this neighbor prefix Valid values are between 1 and 1000, or 'default', which does - not impose the limit. + not impose the limit. Note that this parameter is accepted + only on neighbors with address/prefix. required: false default: null pwd: @@ -118,9 +119,9 @@ options: pwd_type: description: - Specify the encryption type the password will use. Valid values - are '3des' or 'cisco_type_7' encryption. + are '3des' or 'cisco_type_7' encryption or keyword 'default'. required: false - choices: ['3des', 'cisco_type_7'] + choices: ['3des', 'cisco_type_7', 'default'] default: null remote_as: description: @@ -170,8 +171,6 @@ options: Valid values are 'true', 'false', and 'default', which defaults to 'false'. This property can only be configured when the neighbor is in 'ip' address format without prefix length. - This property and the transport_passive_mode property are - mutually exclusive. required: false choices: ['true','false'] default: null @@ -388,11 +387,13 @@ def state_present(module, existing, proposed, candidate): command = '{0} {1}'.format(key, value) commands.append(command) elif key == 'timers': - command = 'timers {0} {1}'.format( - proposed['timers_keepalive'], - proposed['timers_holdtime']) - if command not in commands: - commands.append(command) + if (proposed['timers_keepalive'] != PARAM_TO_DEFAULT_KEYMAP.get('timers_keepalive') or + proposed['timers_holdtime'] != PARAM_TO_DEFAULT_KEYMAP.get('timers_holdtime')): + command = 'timers {0} {1}'.format( + proposed['timers_keepalive'], + proposed['timers_holdtime']) + if command not in commands: + commands.append(command) else: command = '{0} {1}'.format(key, value) commands.append(command) @@ -437,7 +438,7 @@ def main(): low_memory_exempt=dict(required=False, type='bool'), maximum_peers=dict(required=False, type='str'), pwd=dict(required=False, type='str'), - pwd_type=dict(required=False, type='str', choices=['cleartext', '3des', 'cisco_type_7', 'default']), + pwd_type=dict(required=False, type='str', choices=['3des', 'cisco_type_7', 'default']), remote_as=dict(required=False, type='str'), remove_private_as=dict(required=False, type='str', choices=['enable', 'disable', 'all', 'replace-as']), shutdown=dict(required=False, type='bool'), @@ -452,7 +453,7 @@ def main(): module = AnsibleModule( argument_spec=argument_spec, - required_together=[['timers_holdtime', 'timers_keepalive']], + required_together=[['timers_holdtime', 'timers_keepalive'], ['pwd', 'pwd_type']], supports_check_mode=True, ) diff --git a/test/integration/targets/nxos_bgp_neighbor/defaults/main.yaml b/test/integration/targets/nxos_bgp_neighbor/defaults/main.yaml index 5f709c5aac1..525b7aab903 100644 --- a/test/integration/targets/nxos_bgp_neighbor/defaults/main.yaml +++ b/test/integration/targets/nxos_bgp_neighbor/defaults/main.yaml @@ -1,2 +1,5 @@ --- testcase: "*" +vrfs: + - default + - myvrf diff --git a/test/integration/targets/nxos_bgp_neighbor/tests/common/sanity.yaml b/test/integration/targets/nxos_bgp_neighbor/tests/common/sanity.yaml index 988bd179696..a899cbfec75 100644 --- a/test/integration/targets/nxos_bgp_neighbor/tests/common/sanity.yaml +++ b/test/integration/targets/nxos_bgp_neighbor/tests/common/sanity.yaml @@ -5,10 +5,14 @@ - set_fact: intname="{{ nxos_int1 }}" -- set_fact: log_neighbor_changes="enable" - when: not titanium +- set_fact: log_neighbor_changese="enable" + when: (imagetag and (imagetag is version_compare('D1', 'ne')) and (imagetag is version_compare('N1', 'ne'))) +- set_fact: log_neighbor_changesd="disable" + when: (imagetag and (imagetag is version_compare('D1', 'ne')) and (imagetag is version_compare('N1', 'ne'))) -- set_fact: remove_private_as="all" +- set_fact: remove_private_asa="all" + when: not titanium +- set_fact: remove_private_asr="replace-as" when: not titanium - name: "Enable feature BGP" @@ -19,23 +23,49 @@ ignore_errors: yes - name: "Setup" - nxos_bgp: &remove + nxos_bgp_neighbor: &removenp asn: 65535 + neighbor: 3.3.3.3 + vrf: "{{ item }}" state: absent provider: "{{ connection }}" + with_items: "{{ vrfs }}" + ignore_errors: yes + +- name: "Setup" + nxos_bgp_neighbor: &remove + asn: 65535 + neighbor: 3.3.3.3/32 + vrf: "{{ item }}" + state: absent + provider: "{{ connection }}" + with_items: "{{ vrfs }}" ignore_errors: yes - block: - - name: "Configure BGP neighbor defaults" - nxos_bgp_neighbor: &configure_default + - name: "Configure BGP neighbor1" + nxos_bgp_neighbor: &configure1 asn: 65535 - neighbor: 3.3.3.3 - local_as: 20 - remote_as: 30 + neighbor: 3.3.3.3/32 + vrf: "{{ item }}" + connected_check: true + capability_negotiation: true + dynamic_capability: true + ebgp_multihop: 2 + low_memory_exempt: true + maximum_peers: 10 + suppress_4_byte_as: true + timers_keepalive: 90 + timers_holdtime: 270 + log_neighbor_changes: "{{log_neighbor_changese|default(omit)}}" + local_as: 22.33 + remote_as: 33.22 description: "just a description" update_source: "{{ intname.capitalize() }}" + shutdown: true state: present provider: "{{ connection }}" + with_items: "{{ vrfs }}" register: result - assert: &true @@ -43,57 +73,107 @@ - "result.changed == true" - name: "Check Idempotence" - nxos_bgp_neighbor: *configure_default + nxos_bgp_neighbor: *configure1 + with_items: "{{ vrfs }}" register: result - assert: &false that: - "result.changed == false" - - name: "Remove BGP" - nxos_bgp: *remove - register: result - - - assert: *true - - - name: "Check Idempotence" - nxos_bgp: *remove - register: result - - - assert: *false - - - name: "Configure BGP neighbor non-defaults" - nxos_bgp_neighbor: &configure_non_default + - name: "Configure BGP neighbor2" + nxos_bgp_neighbor: &configure2 asn: 65535 - neighbor: 3.3.3.3 - description: "tested by ansible" - connected_check: true - capability_negotiation: true - dynamic_capability: true - ebgp_multihop: 2 - log_neighbor_changes: "{{log_neighbor_changes|default(omit)}}" - low_memory_exempt: true - remote_as: 12.1 - remove_private_as: "{{remove_private_as|default(omit)}}" - shutdown: true - suppress_4_byte_as: true - timers_keepalive: 90 - timers_holdtime: 270 - update_source: loopback151 + neighbor: 3.3.3.3/32 + vrf: "{{ item }}" + connected_check: False + capability_negotiation: False + dynamic_capability: False + ebgp_multihop: default + low_memory_exempt: False + maximum_peers: default + suppress_4_byte_as: False + timers_keepalive: default + timers_holdtime: default + log_neighbor_changes: "{{log_neighbor_changesd|default(omit)}}" + local_as: default + remote_as: default + description: default + update_source: default + shutdown: False state: present provider: "{{ connection }}" + with_items: "{{ vrfs }}" register: result - assert: *true - name: "Check Idempotence" - nxos_bgp_neighbor: *configure_non_default + nxos_bgp_neighbor: *configure2 + with_items: "{{ vrfs }}" register: result - assert: *false - name: "Remove BGP" - nxos_bgp: *remove + nxos_bgp_neighbor: *remove + with_items: "{{ vrfs }}" + register: result + + - assert: *true + + - name: "Check Idempotence" + nxos_bgp_neighbor: *remove + with_items: "{{ vrfs }}" + register: result + + - assert: *false + + - name: "Configure BGP neighbor3" + nxos_bgp_neighbor: &configure3 + asn: 65535 + neighbor: 3.3.3.3/32 + vrf: "{{ item }}" + description: "tested by ansible" + remove_private_as: "{{remove_private_asa|default(omit)}}" + state: present + provider: "{{ connection }}" + with_items: "{{ vrfs }}" + register: result + + - assert: *true + + - name: "Check Idempotence" + nxos_bgp_neighbor: *configure3 + with_items: "{{ vrfs }}" + register: result + + - assert: *false + + - name: "Configure BGP neighbor4" + nxos_bgp_neighbor: &configure4 + asn: 65535 + neighbor: 3.3.3.3/32 + vrf: "{{ item }}" + description: "tested by ansible" + remove_private_as: "{{remove_private_asr|default(omit)}}" + state: present + provider: "{{ connection }}" + with_items: "{{ vrfs }}" + register: result + + - assert: *true + + - name: "Check Idempotence" + nxos_bgp_neighbor: *configure4 + with_items: "{{ vrfs }}" + register: result + + - assert: *false + + - name: "Remove BGP" + nxos_bgp_neighbor: *remove + with_items: "{{ vrfs }}" register: result - assert: *true @@ -101,23 +181,27 @@ - name: "Configure BGP neighbor 3des password" nxos_bgp_neighbor: &configure_3des_password asn: 65535 - neighbor: 3.3.3.3 + neighbor: 3.3.3.3/32 + vrf: "{{ item }}" remote_as: 30 pwd: '386c0565965f89de' pwd_type: 3des provider: "{{ connection }}" + with_items: "{{ vrfs }}" register: result - assert: *true - name: "Check Idempotence" nxos_bgp_neighbor: *configure_3des_password + with_items: "{{ vrfs }}" register: result - assert: *false - name: "Remove BGP" - nxos_bgp: *remove + nxos_bgp_neighbor: *remove + with_items: "{{ vrfs }}" register: result - assert: *true @@ -125,23 +209,48 @@ - name: "Configure BGP neighbor type 7 password" nxos_bgp_neighbor: &configure_type7_password asn: 65535 - neighbor: 3.3.3.3 + neighbor: 3.3.3.3/32 + vrf: "{{ item }}" remote_as: 30 pwd: '386c0565965f89de' pwd_type: cisco_type_7 provider: "{{ connection }}" + with_items: "{{ vrfs }}" register: result - assert: *true - name: "Check Idempotence" nxos_bgp_neighbor: *configure_type7_password + with_items: "{{ vrfs }}" + register: result + + - assert: *false + + - name: "Remove BGP neighbor password" + nxos_bgp_neighbor: &remove_password + asn: 65535 + neighbor: 3.3.3.3/32 + vrf: "{{ item }}" + remote_as: 30 + pwd: default + pwd_type: default + provider: "{{ connection }}" + with_items: "{{ vrfs }}" + register: result + + - assert: *true + + - name: "Check Idempotence" + nxos_bgp_neighbor: *remove_password + with_items: "{{ vrfs }}" register: result - assert: *false - name: "Remove BGP" - nxos_bgp: *remove + nxos_bgp_neighbor: *remove + with_items: "{{ vrfs }}" register: result - assert: *true @@ -150,49 +259,60 @@ nxos_bgp_neighbor: &configure_transport_passive asn: 65535 neighbor: 3.3.3.3 + vrf: "{{ item }}" remote_as: 30 transport_passive_only: true provider: "{{ connection }}" + with_items: "{{ vrfs }}" register: result + - assert: *true - name: "Check Idempotence" nxos_bgp_neighbor: *configure_transport_passive + with_items: "{{ vrfs }}" register: result - assert: *false - - name: "Remove BGP" - nxos_bgp: *remove - register: result - - - assert: *true - - name: "Configure BGP neighbor transport type default" nxos_bgp_neighbor: &configure_transport_default asn: 65535 neighbor: 3.3.3.3 + vrf: "{{ item }}" remote_as: 30 transport_passive_only: false provider: "{{ connection }}" + with_items: "{{ vrfs }}" register: result + - assert: *true - name: "Check Idempotence" nxos_bgp_neighbor: *configure_transport_default + with_items: "{{ vrfs }}" register: result - assert: *false - name: "Remove BGP" - nxos_bgp: *remove + nxos_bgp_neighbor: *removenp + with_items: "{{ vrfs }}" register: result - assert: *true + - name: "Check Idempotence" + nxos_bgp_neighbor: *removenp + with_items: "{{ vrfs }}" + register: result + + - assert: *false + rescue: - name: "Cleanup BGP" - nxos_bgp: *remove + nxos_bgp_neighbor: *remove + with_items: "{{ vrfs }}" ignore_errors: yes always: