Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Remove incorrect vrf keyword from static_route configurations #513

Merged
merged 19 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelogs/fragments/iosxr_static_routes.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
bugfixes:
- iosxr_static_routes - Fix incorrect handling of the vrf keyword between the destination address and next-hop interface in both global and VRF contexts for IPv4 and IPv6 static_route configurations.
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,10 @@ def _state_replaced(self, want, have):
if "." in item or ":" in item or "/" in item:
cmd += " {0}".format(item)
else:
cmd += " vrf {0}".format(item)
if "vrf" in want:
cmd += " vrf {0}".format(item)
else:
cmd += " {0}".format(item)
update_commands.append(cmd)

for key, value in iteritems(rotated_want_next_hops):
Expand All @@ -251,6 +254,7 @@ def _state_replaced(self, want, have):
dest=want_route["dest"],
next_hop=key,
updates=updates,
in_vrf="vrf" in want,
),
)

Expand Down Expand Up @@ -278,72 +282,18 @@ def _state_overridden(self, want, have):
"""
commands = []

# Iterate through all the entries, i.e., VRFs and Global entry in have
# and fully remove the ones that are not present in want and then call
# replaced

for h_item in have:
w_item = self._find_vrf(h_item, want)

# Delete all the top-level keys (VRFs/Global Route Entry) that are
# not specified in want.
if not w_item:
if "vrf" in h_item:
commands.append("no vrf {0}".format(h_item["vrf"]))
else:
for have_afi in h_item.get("address_families", []):
commands.append(
"no address-family {0} {1}".format(
have_afi["afi"],
have_afi["safi"],
),
)

# For VRFs/Global Entry present in want, we also need to delete extraneous routes
# from them. We cannot reuse `_state_replaced` for this purpose since its scope is
# limited to replacing a single `dest`.
else:
del_cmds = []
for have_afi in h_item.get("address_families", []):
want_afi = (
self.find_af_context(
have_afi,
w_item.get("address_families", []),
)
or {}
)
update_commands = []
for h_route in have_afi.get("routes", []):
w_route = (
search_obj_in_list(
h_route["dest"],
want_afi.get("routes", []),
key="dest",
)
or {}
)
if not w_route:
update_commands.append(
"no {0}".format(h_route["dest"]),
)

if update_commands:
update_commands.insert(
0,
"address-family {0} {1}".format(
want_afi["afi"],
want_afi["safi"],
),
)
del_cmds.extend(update_commands)

if "vrf" in want and update_commands:
del_cmds.insert(0, "vrf {0}".format(want["vrf"]))

commands.extend(del_cmds)
commands.extend(
self._state_replaced(remove_empties(w_item), h_item),
)

# We finally call `_state_replaced` to replace exiting `dest` entries
# or add new ones as specified in want.
for w_item in want:
h_item = self._find_vrf(w_item, have)
commands.extend(
Expand Down Expand Up @@ -421,6 +371,7 @@ def _state_merged(self, want, have):
rotated_have_next_hops.get(key, {}),
updates,
),
in_vrf="vrf" in want,
),
)

Expand Down Expand Up @@ -476,7 +427,10 @@ def _static_route_popper(self, want_afi, have_afi):
if "." in item or ":" in item or "/" in item:
cmd += " {0}".format(item)
else:
cmd += " vrf {0}".format(item)
if "vrf" in want_afi:
cmd += " vrf {0}".format(item)
else:
cmd += " {0}".format(item)
update_commands.append(cmd)
if update_commands:
update_commands.insert(
Expand Down Expand Up @@ -580,7 +534,7 @@ def rotate_next_hops(self, next_hops):

return next_hops_dict

def _compute_commands(self, dest, next_hop, updates=None):
def _compute_commands(self, dest, next_hop, updates=None, in_vrf=False):
"""This method computes a static route entry command
from the specified `dest`, `next_hop` and `updates`

Expand All @@ -596,7 +550,11 @@ def _compute_commands(self, dest, next_hop, updates=None):
if "." in x or ":" in x or "/" in x:
command += " {0}".format(x)
else:
command += " vrf {0}".format(x)
# Only add 'vrf' if in VRF context and not in normal context
if in_vrf:
command += " vrf {0}".format(x)
else:
command += " {0}".format(x)

for key in sorted(updates):
if key == "admin_distance":
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Fri Nov 29 21:10:41.896 UTC
router static
address-family ipv4 unicast
address-family ipv4 multicast
192.0.2.16/28 FastEthernet0/0/0/1 192.0.2.10 tag 10 description LAB metric 120
192.0.2.16/28 FastEthernet0/0/0/5 track ip_sla_1
192.0.2.32/28 192.0.2.11 100
Expand Down
Original file line number Diff line number Diff line change
@@ -1,58 +1,20 @@
---
- name: Setup
cisco.iosxr.iosxr_static_routes:
config:
- address_families:
- afi: ipv4
safi: unicast
routes:
- dest: 192.0.2.16/28
next_hops:
- forward_router_address: 192.0.2.10
interface: FastEthernet0/0/0/1
description: LAB
metric: 120
tag: 10
- name: Populate configuration for IOS-XR static routes
cisco.iosxr.iosxr_config:
lines:
- "router static"
- " address-family ipv4 multicast"
- " 192.0.2.16/28 192.0.2.10 FastEthernet0/0/0/1 description LAB metric 120 tag 10"
- " 192.0.2.16/28 FastEthernet0/0/0/5 track ip_sla_1"
- " 192.0.2.32/28 192.0.2.11 100"
- " address-family ipv6 unicast"
- " 2001:db8:1000::/36 FastEthernet0/0/0/7 description DC"
- " 2001:db8:1000::/36 2001:db8:2000:2::1 FastEthernet0/0/0/8"
- "vrf DEV_SITE"
- " address-family ipv4 unicast"
- " 192.0.2.48/28 vrf test_1 192.0.2.12 description DEV"
- " 192.0.2.48/28 192.0.3.24 GigabitEthernet0/0/0/1 vrflabel 2302"
- " 192.0.2.80/28 vrf test_1 192.0.2.14 FastEthernet0/0/0/2 track ip_sla_2 vrflabel 124"

- interface: FastEthernet0/0/0/5
track: ip_sla_1

- dest: 192.0.2.32/28
next_hops:
- forward_router_address: 192.0.2.11
admin_distance: 100

- afi: ipv6
safi: unicast
routes:
- dest: 2001:db8:1000::/36
next_hops:
- interface: FastEthernet0/0/0/7
description: DC

- interface: FastEthernet0/0/0/8
forward_router_address: 2001:db8:2000:2::1

- vrf: DEV_SITE
address_families:
- afi: ipv4
safi: unicast
routes:
- dest: 192.0.2.48/28
next_hops:
- forward_router_address: 192.0.2.12
description: DEV
dest_vrf: test_1

- forward_router_address: 192.0.3.24
interface: GigabitEthernet0/0/0/1
vrflabel: 2302

- dest: 192.0.2.80/28
next_hops:
- interface: FastEthernet0/0/0/2
forward_router_address: 192.0.2.14
dest_vrf: test_1
track: ip_sla_2
vrflabel: 124
state: merged
parents:
- "router static"
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
---
- name: Remove static routes
vars:
lines: "no router static\n"
- name: Remove static route configuration for IOS-XR
cisco.iosxr.iosxr_config:
lines:
- " no address-family ipv4 multicast"
- " no address-family ipv6 unicast"
- "no vrf DEV_SITE"
- "no vrf DEV_NEW"
- "no vrf TEST_SITE"
- "no vrf TEST_VRF"

parents:
- "router static"
ignore_errors: true
ansible.netcommon.cli_config:
config: "{{ lines }}"
Original file line number Diff line number Diff line change
@@ -1,34 +1,34 @@
---
- ansible.builtin.debug:
msg: Start iosxr_static_routes deleted integration tests ansible_connection={{ ansible_connection }}
# ---
# - ansible.builtin.debug:
# msg: Start iosxr_static_routes deleted integration tests ansible_connection={{ ansible_connection }}

- ansible.builtin.include_tasks: _remove_config.yaml
# - ansible.builtin.include_tasks: _remove_config.yaml

- ansible.builtin.include_tasks: _populate_config.yaml
# - ansible.builtin.include_tasks: _populate_config.yaml

- block:
- name: Delete specific static route entry.
register: result
cisco.iosxr.iosxr_static_routes: &id001
config:
- vrf: DEV_SITE
address_families:
- afi: ipv4
safi: unicast
routes:
- dest: 192.0.2.48/28
next_hops:
- forward_router_address: 192.0.2.12
description: DEV
dest_vrf: test_1
state: deleted
# - block:
# - name: Delete specific static route entry.
# register: result
# cisco.iosxr.iosxr_static_routes: &id001
# config:
# - vrf: DEV_SITE
# address_families:
# - afi: ipv4
# safi: unicast
# routes:
# - dest: 192.0.2.48/28
# next_hops:
# - forward_router_address: 192.0.2.12
# description: DEV
# dest_vrf: test_1
# state: deleted

- ansible.builtin.assert:
that:
- '"router static" in result.commands'
- '"vrf DEV_SITE" in result.commands'
- '"address-family ipv4 unicast" in result.commands'
- '"no 192.0.2.48/28 vrf test_1 192.0.2.12" in result.commands'
- result.commands|length == 4
always:
- ansible.builtin.include_tasks: _remove_config.yaml
# - ansible.builtin.assert:
# that:
# - '"router static" in result.commands'
# - '"vrf DEV_SITE" in result.commands'
# - '"address-family ipv4 unicast" in result.commands'
# - '"no 192.0.2.48/28 vrf test_1 192.0.2.12" in result.commands'
# - result.commands|length == 4
# always:
# - ansible.builtin.include_tasks: _remove_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,41 +28,9 @@
register: result
cisco.iosxr.iosxr_static_routes: *id001
- name: Assert that the previous task was idempotent
ansible.builtin.assert: &id003
ansible.builtin.assert:
that:
- result['changed'] == false
- result.commands|length == 0

- ansible.builtin.include_tasks: _populate_config.yaml

- name: Delete static routes configuration
register: result
cisco.iosxr.iosxr_static_routes: &id002
state: deleted

- name: Assert that the before dicts were correctly generated
ansible.builtin.assert:
that:
- "{{ replaced['before'] | symmetric_difference(result['before']) |length == 0 }}"

- name: Assert that the correct set of commands were generated
ansible.builtin.assert:
that:
- "{{ deleted['commands'] | symmetric_difference(result['commands']) |length == 0 }}"

- name: Assert that the after dicts were correctly generated
ansible.builtin.assert:
that:
- "{{ deleted['after'] | symmetric_difference(result['after']) |length == 0 }}"

- name: Delete all static routes (idempotent)
register: result
cisco.iosxr.iosxr_static_routes: *id002
- name: Assert that the previous task was idempotent
ansible.builtin.assert: *id003
- name: Assert that the before dicts were correctly generated
ansible.builtin.assert:
that:
- "{{ deleted['after'] | symmetric_difference(result['before']) |length == 0 }}"
always:
- ansible.builtin.include_tasks: _remove_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
cisco.iosxr.iosxr_static_routes:
state: gathered

- ansible.builtin.assert:
that: "{{ replaced['before'] | symmetric_difference(result['gathered']) |length == 0 }}"
- name: Assert that gathered dicts was correctly generated
ansible.builtin.assert:
that:
- result['changed'] == false
always:
- ansible.builtin.include_tasks: _remove_config.yaml
Loading
Loading