Handling modification of ip range for dynamic BGP peers

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
+!

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:

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):