From 7b44bc1ac9a8bb1b223755a7c71854cbd6601ec7 Mon Sep 17 00:00:00 2001 From: Chris Van Heuveln Date: Thu, 21 Mar 2019 09:22:46 -0400 Subject: [PATCH] nxos_static_route: reconcile_candidate fails to remove 'track' routes (#53806) * * `reconcile_candidate()` * old code searched the ip route configs for a given prefix+nexthop and then tried to remove the route based on prefix+nexthop only; this would fail when a static route was configured with `track` values. * new code still looks for prefix+nexthop but uses the route config it finds on the device to remove it; e.g. * search for: `ip route 192.168.20.64/24 192.0.2.3` * find: `ip route 192.168.20.64/24 192.0.2.3 track 1 10` * remove: `no ip route 192.168.20.64/24 192.0.2.3 track 1 10` * logic cleanups: * old code did a `show run` for every prefix. This can be a lot of data when there are large configs. * new code uses filters to only return the static route configs. * The filters now allow a common code path so no need for default vs vrf code paths * `sanity` test: 100% Pass rate on N9K,N7K,N6K,N3K - Bugfix Pull Request `nxos_static_route` * filter() does not return a list with python3 `filter()` was breaking pytest when it ran with python3, since it returns an iterable instead of a list with python3. Found that I didn't really need `filter()` anyway so just removed it * restore var names /w/want/ --- .../modules/network/nxos/nxos_static_route.py | 63 +++++++------------ .../tests/common/sanity.yaml | 43 +++++++------ 2 files changed, 47 insertions(+), 59 deletions(-) diff --git a/lib/ansible/modules/network/nxos/nxos_static_route.py b/lib/ansible/modules/network/nxos/nxos_static_route.py index fec86743228..2fab19c9c57 100644 --- a/lib/ansible/modules/network/nxos/nxos_static_route.py +++ b/lib/ansible/modules/network/nxos/nxos_static_route.py @@ -100,54 +100,37 @@ from ansible.module_utils.network.common.config import CustomNetworkConfig from ansible.module_utils.network.common.utils import remove_default_spec -def reconcile_candidate(module, candidate, prefix, w): - netcfg = CustomNetworkConfig(indent=2, contents=get_config(module)) - state = w['state'] +def reconcile_candidate(module, candidate, prefix, want): + state, vrf = want['state'], want['vrf'] + if vrf == 'default': + parents = [] + flags = " | include '^ip route'" + else: + parents = ['vrf context {0}'.format(vrf)] + flags = " | section '{0}' | include '^ ip route'".format(parents[0]) + + # Find existing routes in this vrf/default + netcfg = CustomNetworkConfig(indent=2, contents=get_config(module, flags=[flags])) + routes = str(netcfg).split('\n') + # strip whitespace from route strings + routes = [i.strip() for i in routes] - set_command = set_route_command(prefix, w, module) - remove_command = remove_route_command(prefix, w) + prefix_and_nh = 'ip route {0} {1}'.format(prefix, want['next_hop']) + existing = [i for i in routes if i.startswith(prefix_and_nh)] + proposed = set_route_command(prefix, want, module) - parents = [] commands = [] - yrc = remove_command.replace('no ', '') - if w['vrf'] == 'default': - netcfg = str(netcfg).split('\n') - ncfg = [] - for line in netcfg: - # remove ip route commands of non-default vrfs from - # the running config just in case the same commands - # exist in default and non-default vrfs - if ' ip route' not in line: - ncfg.append(line) - if any(yrc in s for s in ncfg) and state == 'absent': - commands = [remove_command] - elif set_command not in ncfg and state == 'present': - if any(yrc in s for s in ncfg): - commands = [remove_command, set_command] - else: - commands = [set_command] - else: - parents = ['vrf context {0}'.format(w['vrf'])] - config = netcfg.get_section(parents) - if not isinstance(config, list): - config = config.split('\n') - config = [line.strip() for line in config] - if any(yrc in s for s in config) and state == 'absent': - commands = [remove_command] - elif set_command not in config and state == 'present': - if any(yrc in s for s in config): - commands = [remove_command, set_command] - else: - commands = [set_command] + if state == 'absent' and existing: + commands = ['no ' + existing[0]] + elif state == 'present' and proposed not in routes: + if existing: + commands = ['no ' + existing[0]] + commands.append(proposed) if commands: candidate.add(commands, parents=parents) -def remove_route_command(prefix, w): - return 'no ip route {0} {1}'.format(prefix, w['next_hop']) - - def get_configured_track(module, ctrack): check_track = '{0}'.format(ctrack) track_exists = False diff --git a/test/integration/targets/nxos_static_route/tests/common/sanity.yaml b/test/integration/targets/nxos_static_route/tests/common/sanity.yaml index 1b746c541a5..10d3c7cb373 100644 --- a/test/integration/targets/nxos_static_route/tests/common/sanity.yaml +++ b/test/integration/targets/nxos_static_route/tests/common/sanity.yaml @@ -22,6 +22,28 @@ - debug: msg="Test Track Feature {{ test_track_feature }}" +- name: Setup and teardown, remove test routes if present + nxos_static_route: &setup_teardown + aggregate: + - { prefix: "192.168.1.164/32", next_hop: "192.0.2.3" } + - { prefix: "192.168.20.64/24", next_hop: "192.0.2.3" } + - { prefix: "192.168.22.64/24", next_hop: "192.0.2.3" } + - { prefix: "192.168.24.64/24", next_hop: "192.0.2.3" } + vrf: "{{ item }}" + provider: "{{ connection }}" + state: absent + with_items: "{{ vrfs }}" + ignore_errors: yes + +- name: Setup noise routes to ensure testing while non-test routes present + nxos_static_route: + prefix: "192.168.1.164/32" + next_hop: "192.0.2.3" + vrf: "{{ item }}" + provider: "{{ connection }}" + state: present + with_items: "{{ vrfs }}" + - block: - name: create static route nxos_static_route: &configure_static @@ -178,26 +200,9 @@ ignore_errors: yes when: test_track_feature - - name: remove static route - nxos_static_route: - prefix: "192.168.20.64/24" - next_hop: "192.0.2.3" - route_name: testing - pref: 100 - tag: 5500 - vrf: "{{ item }}" - provider: "{{ connection }}" - state: absent + - name: teardown test routes + nxos_static_route: *setup_teardown with_items: "{{ vrfs }}" ignore_errors: yes - - name: remove static route aggregate - nxos_static_route: - aggregate: - - { prefix: "192.168.22.64/24", next_hop: "192.0.2.3" } - - { prefix: "192.168.24.64/24", next_hop: "192.0.2.3" } - provider: "{{ connection }}" - state: absent - ignore_errors: yes - - debug: msg="END connection={{ ansible_connection }} nxos_static_route sanity test"