Skip to content

Commit

Permalink
[bgp_global] Fix shutdown attribute to negate on boolean false (#1038)
Browse files Browse the repository at this point in the history 
* Fix shutdown attribute to negate on boolean false

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add a changelog

* fix sanity

* added extra test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* update test comments

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove print statements

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
@KB-perByte @pre-commit-ci
KB-perByte and pre-commit-ci[bot] authored Mar 8, 2024
1 parent fa25415 commit 53a071a
Show file tree
Hide file tree
Showing 7 changed files with 179 additions and 51 deletions.
3 changes: 3 additions & 0 deletions changelogs/fragments/bgp_global_shutdown.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
bugfixes:
- ios_bgp_global - Shutdown attributes generates negate command on set as false.
18 changes: 16 additions & 2 deletions plugins/module_utils/network/ios/config/bgp_global/bgp_global.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,15 +237,18 @@ def _compare(self, want, have):
have.get("redistribute", {}),
)

# add as_number in the begining of commands set if commands generated
# add as_number in the beginning of commands set if commands generated
if len(self.commands) != cmd_len or (not have and want):
self.commands.insert(
0,
self._tmplt.render(want or have, "as_number", False),
)

def _set_bgp_defaults(self, bgp_dict):
bgp_dict.setdefault("bgp", {}).setdefault("default", {}).setdefault("ipv4_unicast", True)
bgp_dict.setdefault("bgp", {}).setdefault("default", {}).setdefault(
"ipv4_unicast",
True,
)
bgp_dict.setdefault("bgp", {}).setdefault("default", {}).setdefault(
"route_target",
{},
Expand Down Expand Up @@ -357,10 +360,21 @@ def _compare_neighbor_lists(self, want, have):
]

for name, w_neighbor in want.items():
handle_shutdown_default = False
have_nbr = have.pop(name, {})
want_route = w_neighbor.pop("route_maps", {})
have_route = have_nbr.pop("route_maps", {})
if (
not w_neighbor.get("shutdown", {}).get("set")
and have_nbr.get("shutdown", {}).get("set")
and self.state in ["merged", "replaced", "overridden"]
):
neig_parses.remove("shutdown")
handle_shutdown_default = True
self.compare(parsers=neig_parses, want=w_neighbor, have=have_nbr)
if handle_shutdown_default:
self.addcmd(have_nbr, "shutdown", True)

if want_route:
for k_rmps, w_rmps in want_route.items():
have_rmps = have_route.pop(k_rmps, {})
Expand Down
5 changes: 4 additions & 1 deletion plugins/module_utils/network/ios/rm_templates/bgp_global.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,10 @@ def __init__(self, lines=None, module=None):
},
{
"name": "bgp.default.route_target.filter",
"getval": re.compile(r"""\sno\sbgp\sdefault\sroute\-target\sfilter""", re.VERBOSE),
"getval": re.compile(
r"""\sno\sbgp\sdefault\sroute\-target\sfilter""",
re.VERBOSE,
),
"setval": "bgp default route-target filter",
"result": {"bgp": {"default": {"route_target": {"filter": False}}}},
},
Expand Down
1 change: 1 addition & 0 deletions tests/integration/targets/ios_bgp_global/tests/cli/deleted.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- name: Delete provided BGP global (idempotent)
register: result
cisco.ios.ios_bgp_global: *id001

- name: Assert that the previous task was idempotent
ansible.builtin.assert:
that:
Expand Down
20 changes: 0 additions & 20 deletions tests/integration/targets/ios_bgp_global/tests/cli/merged.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,8 @@
config:
as_number: 65000
bgp:
advertise_best_external: true
bestpath:
- compare_routerid: true
dampening:
penalty_half_time: 1
reuse_route_val: 1
suppress_route_val: 1
max_suppress: 1
default:
ipv4_unicast: false
route_target:
Expand All @@ -37,20 +31,6 @@
remote_as: 100
shutdown:
set: false
aigp:
send:
cost_community:
id: 100
poi:
igp_cost: true
transitive: true
route_map:
name: test-route
out: true
redistribute:
- connected:
metric: 10
set: true
timers:
keepalive: 100
holdtime: 200
Expand Down
45 changes: 18 additions & 27 deletions tests/integration/targets/ios_bgp_global/vars/main.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,20 @@ merged:
commands:
- router bgp 65000
- timers bgp 100 200 150
- bgp advertise-best-external
- bgp bestpath compare-routerid
- bgp dampening 1 1 1 1
- no bgp default ipv4-unicast
- no bgp default route-target filter
- bgp advertise-best-external
- bgp graceful-shutdown all neighbors 50 local-preference 100 community 100
- bgp log-neighbor-changes
- bgp nopeerup-delay post-boot 10
- neighbor 198.0.2.1 remote-as 100
- neighbor 198.0.2.1 description merge neighbor
- neighbor 198.0.2.1 aigp send cost-community 100 poi igp-cost transitive
- neighbor 198.0.2.1 route-map test-route out
- redistribute connected metric 10

after:
as_number: "65000"
bgp:
advertise_best_external: true
bestpath_options:
compare_routerid: true
dampening:
max_suppress: 1
penalty_half_time: 1
reuse_route_val: 1
suppress_route_val: 1
default:
ipv4_unicast: false
route_target:
Expand All @@ -44,23 +32,9 @@ merged:
nopeerup_delay_options:
post_boot: 10
neighbors:
- aigp:
send:
cost_community:
id: 100
poi:
igp_cost: true
transitive: true
description: merge neighbor
- description: merge neighbor
neighbor_address: 198.0.2.1
remote_as: "100"
route_maps:
- name: test-route
out: true
redistribute:
- connected:
metric: 10
set: true
timers:
holdtime: 200
keepalive: 100
Expand All @@ -70,6 +44,10 @@ gathered:
after:
as_number: "65000"
bgp:
default:
ipv4_unicast: true
route_target:
filter: true
advertise_best_external: true
bestpath_options:
compare_routerid: true
Expand Down Expand Up @@ -126,6 +104,10 @@ replaced:
after:
as_number: "65000"
bgp:
default:
ipv4_unicast: true
route_target:
filter: true
advertise_best_external: true
bestpath_options:
med:
Expand All @@ -150,6 +132,10 @@ parsed:
after:
as_number: "65000"
bgp:
default:
ipv4_unicast: true
route_target:
filter: true
advertise_best_external: true
bestpath_options:
compare_routerid: true
Expand Down Expand Up @@ -220,6 +206,11 @@ deleted:

after:
as_number: "65000"
bgp:
default:
ipv4_unicast: true
route_target:
filter: true

rtt:
commands:
Expand Down
138 changes: 137 additions & 1 deletion tests/unit/modules/network/ios/test_ios_bgp_global.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,10 @@ def test_ios_bgp_global_replaced(self):
bgp=dict(
advertise_best_external=True,
bestpath_options=dict(compare_routerid=True),
default=dict(ipv4_unicast=False, route_target=dict(filter=True)),
default=dict(
ipv4_unicast=False,
route_target=dict(filter=True),
),
log_neighbor_changes=True,
nopeerup_delay_options=dict(cold_boot=20, post_boot=10),
),
Expand Down Expand Up @@ -863,3 +866,136 @@ def test_ios_bgp_global_parsed(self):
],
}
self.assertEqual(parsed_list, result["parsed"])

def test_ios_bgp_global_action_states_specific_default(self):
self.execute_show_command.return_value = dedent(
"""\
router bgp 6500
bgp log-neighbor-changes
no bgp default ipv4-unicast
no bgp default route-target filter
neighbor 192.0.2.1 remote-as 100
neighbor 192.0.2.1 description Test description
neighbor 192.0.2.2 remote-as 200
neighbor 192.0.2.2 description Test description 2
neighbor 192.0.2.2 shutdown
!
address-family ipv4
exit-address-family
""",
)
for stt in ["merged", "replaced", "overridden"]:
set_module_args(
{
"config": {
"as_number": "6500",
"bgp": {
"default": {
"ipv4_unicast": False,
"route_target": {
"filter": False,
},
},
"log_neighbor_changes": True,
},
"neighbors": [
{
"neighbor_address": "192.0.2.1",
"remote_as": "100",
"description": "Test description",
"shutdown": { # Don't have in config, adding
"set": True,
},
},
{
"neighbor_address": "192.0.2.2",
"remote_as": "200",
"description": "Test description 2",
"shutdown": { # Have in config negating with false
"set": False,
},
},
],
},
"state": stt,
},
)
commands = [
"router bgp 6500",
"neighbor 192.0.2.1 shutdown",
"no neighbor 192.0.2.2 shutdown",
]
result = self.execute_module(changed=True)
self.assertEqual(sorted(result["commands"]), sorted(commands))

def test_ios_bgp_global_action_states_no_default(self):
self.execute_show_command.return_value = dedent(
"""\
router bgp 6500
bgp log-neighbor-changes
no bgp default ipv4-unicast
no bgp default route-target filter
neighbor 192.0.2.1 remote-as 100
neighbor 192.0.2.1 description Test description
neighbor 192.0.2.1 shutdown
neighbor 192.0.2.2 remote-as 200
neighbor 192.0.2.2 description Test description 2
neighbor 192.0.2.2 shutdown
neighbor 192.0.2.3 remote-as 300
neighbor 192.0.2.3 description Test description 3
neighbor 192.0.2.4 remote-as 400
neighbor 192.0.2.4 description Test description 4
!
address-family ipv4
exit-address-family
""",
)
for stt in ["replaced", "overridden"]:
set_module_args(
{
"config": {
"as_number": "6500",
"bgp": {
"default": {
"ipv4_unicast": False,
"route_target": {
"filter": False,
},
},
"log_neighbor_changes": True,
},
"neighbors": [
{
"neighbor_address": "192.0.2.1",
"remote_as": "100",
"description": "Test description",
"shutdown": { # Have in config not adding again (idempotent)
"set": True,
},
},
{
"neighbor_address": "192.0.2.2",
"remote_as": "200",
"description": "Test description 2", # Have in config but don't want (to be removed)
},
{
"neighbor_address": "192.0.2.3",
"remote_as": "300",
"description": "Test description 3", # Don't have in config don't want
},
{
"neighbor_address": "192.0.2.4",
"remote_as": "400",
"description": "Test description 4",
"shutdown": { # Don't have in config, explicitly don't want
"set": False,
},
},
],
},
"state": stt,
},
)
commands = ["router bgp 6500", "no neighbor 192.0.2.2 shutdown"]
result = self.execute_module(changed=True)
self.assertEqual(sorted(result["commands"]), sorted(commands))

0 comments on commit 53a071a

Please sign in to comment.