From 929ff7c5fd9bc723ac0201445e39b19210f1f0e5 Mon Sep 17 00:00:00 2001 From: judyjoseph <53951155+judyjoseph@users.noreply.github.com> Date: Fri, 13 Nov 2020 10:35:11 -0800 Subject: [PATCH] Updates to bgp config and show commands with BGP_INTERNAL_NEIGHBOR table (#1224) - What I did Updates to bgp config and show commands with the movement of internal bgp sessions to a new table BGP_INTERNAL_NEIGHBOR table. With the introduction of BGP_INERNAL_NEIGHBOR table, the user cannot start/stop/remove internal neighbors which I feel is not needed as these internal bgp sessions which are supposed to be UP always. - How I did it Updated the show/bgp util so that it gets the internal bgp sessions from BGP_INTERNAL_NEIGHBOR table. Removed the API's to get the local_as in config/bgp which is not required now. - How to verify it Made sure that the output remain same with the new internal bgp neighbor table. --- config/main.py | 21 ++++----------------- show/main.py | 13 ++++++++----- tests/bgp_commands_test.py | 8 ++++---- tests/mock_tables/config_db.json | 16 ++++++++-------- utilities_common/bgp_util.py | 2 ++ 5 files changed, 26 insertions(+), 34 deletions(-) diff --git a/config/main.py b/config/main.py index 5f3fa24c861b..c3ddaf9db9f4 100755 --- a/config/main.py +++ b/config/main.py @@ -491,28 +491,19 @@ def set_interface_naming_mode(mode): click.echo("Please logout and log back in for changes take effect.") -# Get the local BGP ASN from DEVICE_METADATA -def get_local_bgp_asn(config_db): - metadata = config_db.get_table('DEVICE_METADATA') - return metadata['localhost']['bgp_asn'] - def _is_neighbor_ipaddress(config_db, ipaddress): """Returns True if a neighbor has the IP address , False if not """ entry = config_db.get_entry('BGP_NEIGHBOR', ipaddress) return True if entry else False -def _get_all_neighbor_ipaddresses(config_db, ignore_local_hosts=False): +def _get_all_neighbor_ipaddresses(config_db): """Returns list of strings containing IP addresses of all BGP neighbors - if the flag ignore_local_hosts is set to True, additional check to see if - if the BGP neighbor AS number is same as local BGP AS number, if so ignore that neigbor. """ addrs = [] bgp_sessions = config_db.get_table('BGP_NEIGHBOR') - local_as = get_local_bgp_asn(config_db) for addr, session in bgp_sessions.iteritems(): - if not ignore_local_hosts or (ignore_local_hosts and local_as != session['asn']): - addrs.append(addr) + addrs.append(addr) return addrs def _get_neighbor_ipaddress_list_by_hostname(config_db, hostname): @@ -1961,19 +1952,17 @@ def all(verbose): """ log.log_info("'bgp shutdown all' executing...") namespaces = [DEFAULT_NAMESPACE] - ignore_local_hosts = False if multi_asic.is_multi_asic(): ns_list = multi_asic.get_all_namespaces() namespaces = ns_list['front_ns'] - ignore_local_hosts = True # Connect to CONFIG_DB in linux host (in case of single ASIC) or CONFIG_DB in all the # namespaces (in case of multi ASIC) and do the sepcified "action" on the BGP neighbor(s) for namespace in namespaces: config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace) config_db.connect() - bgp_neighbor_ip_list = _get_all_neighbor_ipaddresses(config_db, ignore_local_hosts) + bgp_neighbor_ip_list = _get_all_neighbor_ipaddresses(config_db) for ipaddress in bgp_neighbor_ip_list: _change_bgp_session_status_by_addr(config_db, ipaddress, 'down', verbose) @@ -2018,19 +2007,17 @@ def all(verbose): """ log.log_info("'bgp startup all' executing...") namespaces = [DEFAULT_NAMESPACE] - ignore_local_hosts = False if multi_asic.is_multi_asic(): ns_list = multi_asic.get_all_namespaces() namespaces = ns_list['front_ns'] - ignore_local_hosts = True # Connect to CONFIG_DB in linux host (in case of single ASIC) or CONFIG_DB in all the # namespaces (in case of multi ASIC) and do the sepcified "action" on the BGP neighbor(s) for namespace in namespaces: config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace) config_db.connect() - bgp_neighbor_ip_list = _get_all_neighbor_ipaddresses(config_db, ignore_local_hosts) + bgp_neighbor_ip_list = _get_all_neighbor_ipaddresses(config_db) for ipaddress in bgp_neighbor_ip_list: _change_bgp_session_status_by_addr(config_db, ipaddress, 'up', verbose) diff --git a/show/main.py b/show/main.py index 797655529965..cff194dc423a 100755 --- a/show/main.py +++ b/show/main.py @@ -774,13 +774,16 @@ def get_bgp_peer(): """ config_db = ConfigDBConnector() config_db.connect() - data = config_db.get_table('BGP_NEIGHBOR') bgp_peer = {} + bgp_neighbor_tables = ['BGP_NEIGHBOR', 'BGP_INTERNAL_NEIGHBOR'] + + for table in bgp_neighbor_tables: + data = config_db.get_table(table) + for neighbor_ip in data.keys(): + local_addr = data[neighbor_ip]['local_addr'] + neighbor_name = data[neighbor_ip]['name'] + bgp_peer.setdefault(local_addr, [neighbor_name, neighbor_ip]) - for neighbor_ip in data.keys(): - local_addr = data[neighbor_ip]['local_addr'] - neighbor_name = data[neighbor_ip]['name'] - bgp_peer.setdefault(local_addr, [neighbor_name, neighbor_ip]) return bgp_peer # diff --git a/tests/bgp_commands_test.py b/tests/bgp_commands_test.py index 2b9f4610043f..9f8c4fb21c54 100644 --- a/tests/bgp_commands_test.py +++ b/tests/bgp_commands_test.py @@ -38,8 +38,8 @@ 10.0.0.55 4 64012 0 0 0 0 0 never Active ARISTA12T0 10.0.0.57 4 64013 0 0 0 0 0 never Active ARISTA13T0 10.0.0.59 4 64014 0 0 0 0 0 never Active ARISTA14T0 -10.0.0.61 4 64015 0 0 0 0 0 never Active ARISTA15T0 -10.0.0.63 4 64016 0 0 0 0 0 never Active ARISTA16T0 +10.0.0.61 4 64015 0 0 0 0 0 never Active INT_NEIGH0 +10.0.0.63 4 64016 0 0 0 0 0 never Active INT_NEIGH1 Total number of neighbors 24 """ @@ -78,8 +78,8 @@ fc00::62 4 64009 0 0 0 0 0 never Active ARISTA09T0 fc00::66 4 64010 0 0 0 0 0 never Active ARISTA10T0 fc00::72 4 64013 0 0 0 0 0 never Active ARISTA13T0 -fc00::76 4 64014 0 0 0 0 0 never Active ARISTA14T0 -fc00::a 4 65200 6665 6671 0 0 0 2d09h38m 6402 ARISTA03T2 +fc00::76 4 64014 0 0 0 0 0 never Active INT_NEIGH0 +fc00::a 4 65200 6665 6671 0 0 0 2d09h38m 6402 INT_NEIGH1 Total number of neighbors 24 """ diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index 3ee150ff7cca..557890a30cdf 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -988,21 +988,21 @@ "nhopself": "0", "rrclient": "0" }, - "BGP_NEIGHBOR|10.0.0.61": { + "BGP_INTERNAL_NEIGHBOR|10.0.0.61": { "asn": "64015", "holdtime": "10", "keepalive": "3", "local_addr": "10.0.0.60", - "name": "ARISTA15T0", + "name": "INT_NEIGH0", "nhopself": "0", "rrclient": "0" }, - "BGP_NEIGHBOR|10.0.0.63": { + "BGP_INTERNAL_NEIGHBOR|10.0.0.63": { "asn": "64016", "holdtime": "10", "keepalive": "3", "local_addr": "10.0.0.62", - "name": "ARISTA16T0", + "name": "INT_NEIGH1", "nhopself": "0", "rrclient": "0" }, @@ -1204,21 +1204,21 @@ "nhopself": "0", "rrclient": "0" }, - "BGP_NEIGHBOR|fc00::76": { + "BGP_INTERNAL_NEIGHBOR|fc00::76": { "asn": "64014", "holdtime": "10", "keepalive": "3", "local_addr": "fc00::75", - "name": "ARISTA14T0", + "name": "INT_NEIGH0", "nhopself": "0", "rrclient": "0" }, - "BGP_NEIGHBOR|fc00::a": { + "BGP_INTERNAL_NEIGHBOR|fc00::a": { "asn": "65200", "holdtime": "10", "keepalive": "3", "local_addr": "fc00::9", - "name": "ARISTA03T2", + "name": "INT_NEIGH1", "nhopself": "0", "rrclient": "0" }, diff --git a/utilities_common/bgp_util.py b/utilities_common/bgp_util.py index 691390b79812..39c0f7d047b7 100644 --- a/utilities_common/bgp_util.py +++ b/utilities_common/bgp_util.py @@ -70,6 +70,8 @@ def get_bgp_neighbors_dict(namespace=multi_asic.DEFAULT_NAMESPACE): dynamic_neighbors = {} config_db = multi_asic.connect_config_db_for_ns(namespace) static_neighbors = get_neighbor_dict_from_table(config_db, 'BGP_NEIGHBOR') + static_internal_neighbors = get_neighbor_dict_from_table(config_db, 'BGP_INTERNAL_NEIGHBOR') + static_neighbors.update(static_internal_neighbors) bgp_monitors = get_neighbor_dict_from_table(config_db, 'BGP_MONITORS') static_neighbors.update(bgp_monitors) dynamic_neighbors = get_dynamic_neighbor_subnet(config_db)