diff --git a/changelogs/fragments/fix_neighbor_deletion.yaml b/changelogs/fragments/fix_neighbor_deletion.yaml new file mode 100644 index 000000000..633815692 --- /dev/null +++ b/changelogs/fragments/fix_neighbor_deletion.yaml @@ -0,0 +1,3 @@ +--- +bugfixes: + - Fixed deletion of all neighbors unintentionally getting deleted instead of only the specified ones in iosxr_bgp_global module. diff --git a/plugins/module_utils/network/iosxr/config/bgp_global/bgp_global.py b/plugins/module_utils/network/iosxr/config/bgp_global/bgp_global.py index 4cb4083c6..fffcff67e 100644 --- a/plugins/module_utils/network/iosxr/config/bgp_global/bgp_global.py +++ b/plugins/module_utils/network/iosxr/config/bgp_global/bgp_global.py @@ -137,13 +137,13 @@ def generate_commands(self): for entry in self.want, self.have: self._bgp_list_to_dict(entry) - # if state is deleted, clean up global params if self.state == "deleted": if not self.want or (self.have.get("as_number") == self.want.get("as_number")): self._compare( - want={"as_number": self.want.get("as_number")}, + want=self.want, have=self.have, ) + elif self.state == "purged": if not self.want or (self.have.get("as_number") == self.want.get("as_number")): self.addcmd(self.have or {}, "router", True) @@ -319,20 +319,38 @@ def _compare_neighbors(self, want, have, vrf=None): want_nbr = want.get("neighbors", {}) have_nbr = have.get("neighbors", {}) - for name, entry in iteritems(want_nbr): - have = have_nbr.pop(name, {}) - begin = len(self.commands) - self.compare(parsers=neighbor_parsers, want=entry, have=have) - neighbor_address = entry.get("neighbor_address", "") - if len(self.commands) != begin: - self.commands.insert( - begin, - self._tmplt.render( - {"neighbor_address": neighbor_address}, - "neighbor_address", - False, - ), - ) + + if self.state == "deleted": + # For each explicitly given neighbor in 'want', always delete it + for name, entry in iteritems(want_nbr): + if self._check_af("neighbor_address", name): + self._module.fail_json( + msg="Neighbor {0} has address-family configurations. " + "Please use the iosxr_bgp_neighbor_address_family module to remove those first.".format( + name, + ), + ) + else: + self.addcmd(entry, "neighbor_address", True) + return + + else: + # Existing logic for merged, replaced, etc. + for name, entry in iteritems(want_nbr): + have_entry = have_nbr.pop(name, {}) + begin = len(self.commands) + self.compare(parsers=neighbor_parsers, want=entry, have=have_entry) + neighbor_address = entry.get("neighbor_address", "") + if len(self.commands) != begin: + self.commands.insert( + begin, + self._tmplt.render( + {"neighbor_address": neighbor_address}, + "neighbor_address", + False, + ), + ) + for name, entry in iteritems(have_nbr): if self._check_af("neighbor_address", name): self._module.fail_json( diff --git a/tests/integration/targets/iosxr_bgp_global/tests/common/deleted_neighbor.yaml b/tests/integration/targets/iosxr_bgp_global/tests/common/deleted_neighbor.yaml new file mode 100644 index 000000000..4fbc4d924 --- /dev/null +++ b/tests/integration/targets/iosxr_bgp_global/tests/common/deleted_neighbor.yaml @@ -0,0 +1,58 @@ +--- +- ansible.builtin.debug: + msg: Start nxos_bgp_global deleted integration tests connection={{ ansible_connection}} + +- ansible.builtin.include_tasks: _remove_config.yaml + +- ansible.builtin.include_tasks: _populate_config.yaml + +- block: + - name: Push multiple neighbors + become: true + cisco.iosxr.iosxr_bgp_global: + config: + as_number: 65137 + neighbors: + - neighbor_address: 192.168.253.1 + - neighbor_address: 192.168.253.2 + - neighbor_address: 192.168.253.3 + - neighbor_address: 192.168.253.4 + state: merged + + - name: Delete one neighbor only + become: true + register: result + cisco.iosxr.iosxr_bgp_global: + config: + as_number: 65137 + neighbors: + - neighbor_address: 192.168.253.4 + state: deleted + + - name: Validate single neighbor was removed + ansible.builtin.assert: + that: + - "'no neighbor 192.168.253.4' in result.commands" + - "'no neighbor 192.168.253.1' not in result.commands" + - "'no neighbor 192.168.253.2' not in result.commands" + - "'no neighbor 192.168.253.3' not in result.commands" + - result.changed == true + + - name: Confirm idempotency + become: true + register: result + cisco.iosxr.iosxr_bgp_global: + config: + as_number: 65137 + neighbors: + - neighbor_address: 192.168.253.4 + state: deleted + + - name: Validate no further change + ansible.builtin.assert: + that: + - result.commands|length == 0 + - result.changed == false + + always: + - ansible.builtin.include_tasks: _remove_config.yaml diff --git a/tests/unit/modules/network/iosxr/test_iosxr_bgp_global.py b/tests/unit/modules/network/iosxr/test_iosxr_bgp_global.py index 6527fb48d..5aea43d5c 100644 --- a/tests/unit/modules/network/iosxr/test_iosxr_bgp_global.py +++ b/tests/unit/modules/network/iosxr/test_iosxr_bgp_global.py @@ -331,6 +331,35 @@ def test_iosxr_bgp_global_replaced_idempotent(self): self.execute_module(changed=False, commands=[]) + def test_iosxr_bgp_global_delete_single_neighbor(self): + run_cfg = dedent( + """\ + router bgp 65137 + neighbor 192.168.253.1 + neighbor 192.168.253.2 + neighbor 192.168.253.3 + neighbor 192.168.253.4 + """, + ) + self.get_config.return_value = run_cfg + set_module_args( + dict( + config=dict( + as_number="65137", + neighbors=[ + dict(neighbor_address="192.168.253.4"), + ], + ), + state="deleted", + ), + ) + commands = [ + "router bgp 65137", + "no neighbor 192.168.253.4", + ] + result = self.execute_module(changed=True) + self.assertEqual(sorted(result["commands"]), sorted(commands)) + def test_iosxr_bgp_global_deleted(self): run_cfg = dedent( """\ @@ -366,8 +395,6 @@ def test_iosxr_bgp_global_deleted(self): "no default-metric 4", "no socket receive-buffer-size 514", "no socket send-buffer-size 4098", - "no neighbor 192.0.2.11", - "no neighbor 192.0.2.14", "no vrf vrf1", ] result = self.execute_module(changed=True)