diff --git a/dockers/docker-fpm-frr/frr/bgpd/templates/dynamic/update.conf.j2 b/dockers/docker-fpm-frr/frr/bgpd/templates/dynamic/update.conf.j2 new file mode 100644 index 00000000000..343129dcfaf --- /dev/null +++ b/dockers/docker-fpm-frr/frr/bgpd/templates/dynamic/update.conf.j2 @@ -0,0 +1,12 @@ +! +! template: bgpd/templates/dynamic/update.conf.j2 +! +{% for ip_range in delete_ranges %} + no bgp listen range {{ ip_range }} peer-group {{ bgp_session['name'] }} +{% endfor %} +{% for ip_range in add_ranges %} + bgp listen range {{ ip_range }} peer-group {{ bgp_session['name'] }} +{% endfor %} +! +! end of template: bgpd/templates/dynamic/update.conf.j2 +! diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py index 3af049d63fb..fcaab583f79 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py @@ -3,6 +3,7 @@ import jinja2 import netaddr +import os from .log import log_warn, log_err, log_info, log_debug, log_crit from .manager import Manager @@ -105,6 +106,9 @@ def __init__(self, common_objs, db_name, table_name, peer_type, check_neig_meta) "no shutdown": self.fabric.from_string('no neighbor {{ neighbor_addr }} shutdown'), } + if (os.path.exists(self.fabric.env.loader.searchpath[0] + "/" + base_template + "update.conf.j2")): + self.templates["update"] = self.fabric.from_file(base_template + "update.conf.j2") + deps = [ ("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"), ("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/type"), @@ -227,10 +231,46 @@ def add_peer(self, vrf, nbr, data): self.apply_op(cmd, vrf) key = (vrf, nbr) self.peers.add(key) + self.update_state_db(vrf, nbr, data, "SET") log_info("Peer '(%s|%s)' has been scheduled to be added with attributes '%s'" % print_data) return True + def update_state_db(self, vrf, nbr, data, op): + """ + Update the database with the new data + :param vrf: vrf name. Name is equal "default" for the global vrf + :param nbr: neighbor ip address (name for dynamic peer type) + :param data: associated data (will be empty for deletion case) + :param op: operation type. It can be "SET" or "DEL" + :return: True if this adding was successful, False otherwise + """ + if (vrf == "default"): + key = nbr + else: + key = vrf + "|" + nbr + # Update the peer in the STATE_DB table + try: + state_db = swsscommon.DBConnector("STATE_DB", 0) + state_peer_table = swsscommon.Table(state_db, swsscommon.STATE_BGP_PEER_CONFIGURED_TABLE_NAME) + if (op == "SET"): + state_peer_table.set(key, list(sorted(data.items()))) + log_info("Peer '(%s)' has been added to BGP_PEER_CONFIGURED_TABLE with attributes '%s'" % (key, data)) + elif (op == "DEL"): + (status, fvs) = state_peer_table.get(key) + if status == True: + state_peer_table.delete(key) + log_info("Peer '(%s)' has been deleted from BGP_PEER_CONFIGURED_TABLE" % (key)) + else: + log_warn("Peer '(%s)' not found in BGP_PEER_CONFIGURED_TABLE" % (key)) + else: + log_err("Update state DB called for Peer '(%s)' with unkown operation '%s'" % (key, op)) + return False + return True + except Exception as e: + log_err("Update of state db failed for peer '(%s)' with error: %s" % (key, str(e))) + return False + def update_peer(self, vrf, nbr, data): """ Update a peer. This is used when the peer is already in the FRR @@ -242,6 +282,8 @@ def update_peer(self, vrf, nbr, data): """ if "admin_status" in data: self.change_admin_status(vrf, nbr, data) + elif "update" in self.templates and "ip_range" in data and self.peer_type == 'dynamic': + self.change_ip_range(vrf, nbr, data) else: log_err("Peer '(%s|%s)': Can't update the peer. Only 'admin_status' attribute is supported" % (vrf, nbr)) @@ -256,14 +298,14 @@ def change_admin_status(self, vrf, nbr, data): :return: True if this adding was successful, False otherwise """ if data['admin_status'] == 'up': - self.apply_admin_status(vrf, nbr, "no shutdown", "up") + self.apply_admin_status(vrf, nbr, "no shutdown", "up", data) elif data['admin_status'] == 'down': - self.apply_admin_status(vrf, nbr, "shutdown", "down") + self.apply_admin_status(vrf, nbr, "shutdown", "down", data) else: print_data = vrf, nbr, data['admin_status'] log_err("Peer '%s|%s': Can't update the peer. It has wrong attribute value attr['admin_status'] = '%s'" % print_data) - def apply_admin_status(self, vrf, nbr, template_name, admin_state): + def apply_admin_status(self, vrf, nbr, template_name, admin_state, data): """ Render admin state template and apply the command to the FRR :param vrf: vrf name. Name is equal "default" for the global vrf @@ -275,10 +317,99 @@ def apply_admin_status(self, vrf, nbr, template_name, admin_state): print_data = vrf, nbr, admin_state ret_code = self.apply_op(self.templates[template_name].render(neighbor_addr=nbr), vrf) if ret_code: + self.update_state_db(vrf, nbr, data, "SET") log_info("Peer '%s|%s' admin state is set to '%s'" % print_data) else: log_err("Can't set peer '%s|%s' admin state to '%s'." % print_data) + def change_ip_range(self, vrf, nbr, data): + """ + Change ip range of a peer + :param vrf: vrf name. Name is equal "default" for the global vrf + :param nbr: neighbor ip address (name for dynamic peer type) + :param data: associated data + :return: True if this adding was successful, False otherwise + """ + if data['ip_range']: + log_info("Peer '(%s|%s)' ip range is going to be updated with range: %s" % (vrf, nbr, data['ip_range'])) + new_ip_range = data["ip_range"].split(",") + ip_ranges_to_add = new_ip_range + ip_ranges_to_del = [] + existing_ipv4_range, existing_ipv6_range = self.get_existing_ip_ranges(vrf, nbr) + if existing_ipv4_range: + for ipv4_range in existing_ipv4_range: + if ipv4_range not in new_ip_range: + ip_ranges_to_del.append(ipv4_range) + else: + ip_ranges_to_add.remove(ipv4_range) + if existing_ipv6_range: + for ipv6_range in existing_ipv6_range: + if ipv6_range not in new_ip_range: + ip_ranges_to_del.append(ipv6_range) + else: + ip_ranges_to_add.remove(ipv6_range) + if ip_ranges_to_del or ip_ranges_to_add: + log_info("Peer '(%s|%s)' ip range is going to be updated. Ranges to delete: %s Ranges to add: %s" % (vrf, nbr, ip_ranges_to_del, ip_ranges_to_add)) + self.apply_range_changes(vrf, nbr, ip_ranges_to_add, ip_ranges_to_del, data) + + def get_existing_ip_ranges(self, vrf, nbr): + """ + Get existing ip range of a peer + :param vrf: vrf name. Name is equal "default" for the global vrf + :param nbr: neighbor ip address (name for dynamic peer type) + :return: existing ipv4 and ipv6 ranges of a peer if they exist. + """ + ipv4_ranges = [] + ipv6_ranges = [] + if vrf == 'default': + command = ["vtysh", "-c", "show bgp peer-group %s json" % (nbr)] + else: + command = ["vtysh", "-c", "show bgp vrf %s peer-group %s json" % (vrf, nbr)] + try: + ret_code, out, err = run_command(command) + if ret_code == 0: + js_bgp = json.loads(out) + if nbr in js_bgp and 'dynamicRanges' in js_bgp[nbr] and 'IPv4' in js_bgp[nbr]['dynamicRanges'] and 'ranges' in js_bgp[nbr]['dynamicRanges']['IPv4']: + ipv4_ranges = js_bgp[nbr]['dynamicRanges']['IPv4']['ranges'] + log_info("Peer '(%s|%s)' already has ipV4 range: %s" % (vrf, nbr, ipv4_ranges)) + if nbr in js_bgp and 'dynamicRanges' in js_bgp[nbr] and 'IPv6' in js_bgp[nbr]['dynamicRanges'] and 'ranges' in js_bgp[nbr]['dynamicRanges']['IPv6']: + ipv6_ranges = js_bgp[nbr]['dynamicRanges']['IPv6']['ranges'] + log_info("Peer '(%s|%s)' already has ipV6 range: %s" % (vrf, nbr, ipv6_ranges)) + return ipv4_ranges, ipv6_ranges + else: + log_err("Can't read ip range of peer '%s|%s': %s" % (vrf, nbr, str(err))) + return ipv4_ranges, ipv6_ranges + except Exception as e: + log_err("Error in parsing ip range: %s" % str(e)) + return ipv4_ranges, ipv6_ranges + + def apply_range_changes(self, vrf, nbr, new_ip_range, ip_ranges_to_del, data): + """ + Apply changes of ip range of a peer + :param vrf: vrf name. Name is equal "default" for the global vrf + :param nbr: neighbor ip address (name for dynamic peer type) + :param new_ip_range: new ip range + :param ip_ranges_to_del: ip ranges to delete + """ + print_data = vrf, nbr, data + kwargs = { + 'CONFIG_DB__DEVICE_METADATA': self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME), + 'vrf': vrf, + 'bgp_session': data, + 'delete_ranges': ip_ranges_to_del, + 'add_ranges': new_ip_range + } + try: + cmd = self.templates["update"].render(**kwargs) + except jinja2.TemplateError as e: + msg = "Peer '(%s|%s)'. Error in rendering the template for 'SET' command '%s'" % print_data + log_err("%s: %s" % (msg, str(e))) + return True + if cmd is not None: + self.apply_op(cmd, vrf) + self.update_state_db(vrf, nbr, data, "SET") + log_info("Peer '(%s|%s)' ip range has been scheduled to be updated with range '%s'" % (vrf, nbr, data['ip_range'])) + def del_handler(self, key): """ 'DEL' handler for the BGP PEER tables @@ -292,6 +423,7 @@ def del_handler(self, key): cmd = self.templates["delete"].render(neighbor_addr=nbr) ret_code = self.apply_op(cmd, vrf) if ret_code: + self.update_state_db(vrf, nbr, {}, "DEL") log_info("Peer '(%s|%s)' has been removed" % (vrf, nbr)) self.peers.remove(peer_key) else: diff --git a/src/sonic-bgpcfgd/tests/test_bgp.py b/src/sonic-bgpcfgd/tests/test_bgp.py index f52c63494a9..9c2cd417657 100644 --- a/src/sonic-bgpcfgd/tests/test_bgp.py +++ b/src/sonic-bgpcfgd/tests/test_bgp.py @@ -33,7 +33,9 @@ def constructor(constants_path, bgp_router_id="", peer_type="general", with_lo0_ return_value_map = { "['vtysh', '-H', '/dev/null', '-c', 'show bgp vrfs json']": (0, "{\"vrfs\": {\"default\": {}}}", ""), - "['vtysh', '-c', 'show bgp vrf default neighbors json']": (0, "{\"10.10.10.1\": {}, \"20.20.20.1\": {}, \"fc00:10::1\": {}}", "") + "['vtysh', '-c', 'show bgp vrf default neighbors json']": (0, "{\"10.10.10.1\": {}, \"20.20.20.1\": {}, \"fc00:10::1\": {}, \"DynNbr1\": {}, \"DynNbr2\": {}}", ""), + "['vtysh', '-c', 'show bgp peer-group DynNbr1 json']": (0, "{\"DynNbr1\":{\"dynamicRanges\":{\"IPv4\":{\"count\":1,\"ranges\":[\"10.255.0.0/24\"]}}}}", ""), + "['vtysh', '-c', 'show bgp peer-group DynNbr2 json']": (0, "{\"DynNbr2\":{\"dynamicRanges\":{\"IPv4\":{\"count\":1,\"ranges\":[\"192.168.0.0/24\",\"192.168.1.0/24\"]}}}}", "") } bgpcfgd.managers_bgp.run_command = lambda cmd: return_value_map[str(cmd)] @@ -162,7 +164,66 @@ def test_add_peer_ipv6_in_vnet(): for constant in load_constant_files(): m = constructor(constant) res = m.set_handler("Vnet-10|fc00:20::1", {'asn': '65200', 'holdtime': '180', 'keepalive': '60', 'local_addr': 'fc00:20::20', 'name': 'TOR', 'nhopself': '0', 'rrclient': '0'}) + +@patch('bgpcfgd.managers_bgp.log_info') +def test_add_dynamic_peer(mocked_log_info): + for constant in load_constant_files(): + m = constructor(constant, peer_type="dynamic") + m.check_neig_meta = False + res = m.set_handler("BGPSLBPassive", {"peer_asn": "65200", "ip_range": "10.250.0.0/27", "name": "BGPSLBPassive", "src_address": "10.250.0.1"}) + mocked_log_info.assert_called_with("Peer '(default|BGPSLBPassive)' has been scheduled to be added with attributes '{'peer_asn': '65200', 'ip_range': '10.250.0.0/27', 'name': 'BGPSLBPassive', 'src_address': '10.250.0.1'}'") + assert res, "Expect True return value" + +@patch('bgpcfgd.managers_bgp.log_info') +def test_add_dynamic_peer_ipv6(mocked_log_info): + for constant in load_constant_files(): + m = constructor(constant, peer_type="dynamic") + m.check_neig_meta = False + res = m.set_handler("BGPSLBPassive", {"peer_asn": "65200", "ip_range": "fc00:20::/64", "name": "BGPSLBPassive", "src_address": "fc00:20::1"}) + mocked_log_info.assert_called_with("Peer '(default|BGPSLBPassive)' has been scheduled to be added with attributes '{'peer_asn': '65200', 'ip_range': 'fc00:20::/64', 'name': 'BGPSLBPassive', 'src_address': 'fc00:20::1'}'") + assert res, "Expect True return value" + +@patch('bgpcfgd.managers_bgp.log_info') +@patch('bgpcfgd.managers_bgp.swsscommon.Table') +@patch('bgpcfgd.managers_bgp.swsscommon.DBConnector') +def modify_dynamic_peer_common(mock_db_conn, mock_table, mocked_log_info, peer, data, expected_cmds, expected_log): + for constant in load_constant_files(): + m = constructor(constant, peer_type="dynamic") + m.cfg_mgr.push = MagicMock(return_value = None) + m.check_neig_meta = False + swsscommon.STATE_BGP_PEER_CONFIGURED_TABLE_NAME = "BGP_PEER_CONFIGURED_TABLE" + mock_state_db_table = MagicMock() + mock_table.return_value = mock_state_db_table + res = m.set_handler(peer, data) assert res, "Expect True return value" + mock_state_db_table.set.assert_called_once_with(peer, list(sorted(data.items()))) + mocked_log_info.assert_called_with(expected_log) + m.cfg_mgr.push.assert_called_once_with(expected_cmds) + +def test_add_dynamic_peer_range(): + expected_cmds = 'router bgp 65100\n bgp suppress-fib-pending\n!\n! template: bgpd/templates/dynamic/update.conf.j2\n!\n\n\n' \ + ' bgp listen range 10.255.1.0/24 peer-group DynNbr1\n\n!\n! end of template: bgpd/templates/dynamic/update.conf.j2\n!\nexit' + data = {"peer_asn": "65200", "ip_range": "10.255.0.0/24,10.255.1.0/24", "name": "DynNbr1"} + peer = "DynNbr1" + expected_log = "Peer '(default|DynNbr1)' ip range has been scheduled to be updated with range '10.255.0.0/24,10.255.1.0/24'" + modify_dynamic_peer_common(peer=peer, data=data, expected_cmds=expected_cmds, expected_log=expected_log) + +def test_modify_dynamic_peer_range(): + expected_cmds = 'router bgp 65100\n bgp suppress-fib-pending\n!\n! template: bgpd/templates/dynamic/update.conf.j2\n!\n\n' \ + ' no bgp listen range 10.255.0.0/24 peer-group DynNbr1\n\n\n bgp listen range 10.255.0.0/26 peer-group DynNbr1\n\n!\n!' \ + ' end of template: bgpd/templates/dynamic/update.conf.j2\n!\nexit' + data = {"peer_asn": "65200", "ip_range": "10.255.0.0/26", "name": "DynNbr1"} + peer = "DynNbr1" + expected_log = "Peer '(default|DynNbr1)' ip range has been scheduled to be updated with range '10.255.0.0/26'" + modify_dynamic_peer_common(peer=peer, data=data, expected_cmds=expected_cmds, expected_log=expected_log) + +def test_delete_dynamic_peer_range(): + expected_cmds = 'router bgp 65100\n bgp suppress-fib-pending\n!\n! template: bgpd/templates/dynamic/update.conf.j2\n!\n\n' \ + ' no bgp listen range 192.168.1.0/24 peer-group DynNbr2\n\n\n!\n! end of template: bgpd/templates/dynamic/update.conf.j2\n!\nexit' + data = {"peer_asn": "65200", "ip_range": "192.168.0.0/24", "name": "DynNbr2"} + peer = "DynNbr2" + expected_log = "Peer '(default|DynNbr2)' ip range has been scheduled to be updated with range '192.168.0.0/24'" + modify_dynamic_peer_common(peer=peer, data=data, expected_cmds=expected_cmds, expected_log=expected_log) @patch('bgpcfgd.managers_bgp.log_warn') def test_add_peer_no_local_addr(mocked_log_warn):