diff --git a/scripts/route_check.py b/scripts/route_check.py index c6852c5e1..d19f29775 100755 --- a/scripts/route_check.py +++ b/scripts/route_check.py @@ -47,6 +47,7 @@ import traceback import subprocess import concurrent.futures +import ijson from ipaddress import ip_network from swsscommon import swsscommon @@ -385,38 +386,81 @@ def is_suppress_fib_pending_enabled(namespace): return state == 'enabled' -def fetch_routes(cmd): +def fetch_routes(ipv6=False, namespace=multi_asic.DEFAULT_NAMESPACE): """ - Fetch routes using the given command. + Fetch routes using vtysh and parse with ijson for streaming JSON parsing. + This handles large route tables efficiently by processing each prefix entry + as it becomes available instead of loading the entire JSON into memory. + + Returns (missing_routes, failing_routes) tuple. """ - output = subprocess.check_output(cmd, text=True) - return json.loads(output) + missing_routes = [] + failing_routes = [] + + asic_id = [] + if namespace is not multi_asic.DEFAULT_NAMESPACE: + asic_id = ['-n', str(multi_asic.get_asic_id_from_name(namespace))] + + if ipv6: + cmd = ["sudo", "vtysh"] + asic_id + ["-c", "show ipv6 route json"] + else: + cmd = ["sudo", "vtysh"] + asic_id + ["-c", "show ip route json"] + + def process_route_entry(prefix, route_entry): + """Process a single route entry and add to missing/failing lists if needed.""" + if route_entry.get('protocol') in ('connected', 'kernel', 'static'): + return + if route_entry.get('vrfName') != 'default': + return + if not route_entry.get('selected', False): + return + if not route_entry.get('offloaded', False): + missing_routes.append({'prefix': prefix, 'protocol': route_entry.get('protocol', '')}) + if route_entry.get('failed', False): + failing_routes.append(prefix) + + try: + with subprocess.Popen(cmd, stdout=subprocess.PIPE, bufsize=0) as proc: + try: + for prefix, route_entries in ijson.kvitems(proc.stdout, ''): + if isinstance(route_entries, list): + for route_entry in route_entries: + process_route_entry(prefix, route_entry) + else: + print_message(syslog.LOG_WARNING, f"Unexpected route entry format for prefix {prefix}") + + except (ijson.JSONError, UnicodeDecodeError, Exception) as e: + proc.kill() + proc.wait() + raise RuntimeError(f"Failed to parse FRR route JSON: {e}") from e + + return_code = proc.wait() + if return_code != 0: + print_message(syslog.LOG_WARNING, f"vtysh exited with return code {return_code}") + + except FileNotFoundError: + raise RuntimeError(f"Command '{cmd[0]}' not found") + + return missing_routes, failing_routes def get_frr_routes_parallel(namespace): """ - Read routes from zebra through CLI command for IPv4 and IPv6 in parallel - :return combined IPv4 and IPv6 routes dictionary. + Read routes from zebra through CLI command for IPv4 and IPv6 in parallel. + :return (missed_routes, failed_routes) tuple combining IPv4 and IPv6 results. """ - if namespace == multi_asic.DEFAULT_NAMESPACE: - v4_route_cmd = ['show', 'ip', 'route', 'json'] - v6_route_cmd = ['show', 'ipv6', 'route', 'json'] - else: - v4_route_cmd = ['show', 'ip', 'route', '-n', namespace, 'json'] - v6_route_cmd = ['show', 'ipv6', 'route', '-n', namespace, 'json'] - with concurrent.futures.ThreadPoolExecutor() as executor: - future_v4 = executor.submit(fetch_routes, v4_route_cmd) - future_v6 = executor.submit(fetch_routes, v6_route_cmd) + future_v4 = executor.submit(fetch_routes, ipv6=False, namespace=namespace) + future_v6 = executor.submit(fetch_routes, ipv6=True, namespace=namespace) - # Wait for both results to complete - v4_routes = future_v4.result() - v6_routes = future_v6.result() + v4_miss_rt, v4_fail_rt = future_v4.result() + v6_miss_rt, v6_fail_rt = future_v6.result() - # Combine both IPv4 and IPv6 routes - v4_routes.update(v6_routes) - print_message(syslog.LOG_DEBUG, "FRR Routes: namespace={}, routes={}".format(namespace, v4_routes)) - return v4_routes + missed_rt = v4_miss_rt + v6_miss_rt + failed_rt = v4_fail_rt + v6_fail_rt + print_message(syslog.LOG_DEBUG, "FRR Missing Routes: namespace={}, routes={}".format(namespace, missed_rt)) + print_message(syslog.LOG_DEBUG, "FRR Failed Routes: namespace={}, routes={}".format(namespace, failed_rt)) + return missed_rt, failed_rt def get_interfaces(namespace): @@ -669,35 +713,14 @@ def is_feature_bgp_enabled(namespace): def check_frr_pending_routes(namespace): """ Check FRR routes for offload flag presence by executing "show ip route json" - Returns a list of routes that have no offload flag. + Returns a tuple (missed_routes, failed_routes). """ missed_rt = [] + failed_rt = [] retries = FRR_CHECK_RETRIES for i in range(retries): - missed_rt = [] - failed_rt = [] - frr_routes = get_frr_routes_parallel(namespace) - - for route_prefix, entries in frr_routes.items(): - for entry in entries: - if entry['protocol'] in ('connected', 'kernel', 'static'): - continue - - # TODO: Also handle VRF routes. Currently this script does not check for VRF routes so it would be incorrect for us - # to assume they are installed in ASIC_DB, so we don't handle them. - if entry['vrfName'] != 'default': - continue - - # skip if this bgp source prefix is not selected as best - if not entry.get('selected', False): - continue - - if not entry.get('offloaded', False): - missed_rt.append(entry) - - if entry.get('failed', False): - failed_rt.append(route_prefix) + missed_rt, failed_rt = get_frr_routes_parallel(namespace) if not missed_rt and not failed_rt: break diff --git a/setup.py b/setup.py index 23f1c1d73..15644bc6c 100644 --- a/setup.py +++ b/setup.py @@ -245,6 +245,7 @@ 'docker-image-py>=0.1.10', 'filelock>=3.0.12', 'enlighten>=1.8.0', + 'ijson>=3.2.3', 'ipaddress>=1.0.23', 'protobuf', 'jinja2>=2.11.3', diff --git a/tests/fetch_routes_chunk_test.py b/tests/fetch_routes_chunk_test.py new file mode 100755 index 000000000..1403cbac4 --- /dev/null +++ b/tests/fetch_routes_chunk_test.py @@ -0,0 +1,875 @@ +#!/usr/bin/env python3 +""" +Unit tests for the chunk-based reading logic in fetch_routes() function. +Tests various scenarios including: +- Complete JSON in single chunk +- JSON split across multiple chunks +- Incomplete JSON objects at chunk boundaries +- UTF-8 sequences split across chunks +- Empty responses +- Malformed JSON +""" + +import json +from unittest.mock import Mock, patch +from io import BytesIO +import sys +import pytest + +sys.path.append("scripts") +import route_check # noqa: E402 + + +def get_missing_prefixes(result): + """Extract prefix strings from missing_routes dicts for assertion comparison.""" + return {entry['prefix'] for entry in result} + + +class ChunkedBytesIO: + def __init__(self, chunks): + self.chunks = chunks + self.index = 0 + + def read(self, size): + if not size: + return b'' + + if self.index >= len(self.chunks): + return b'' + chunk = self.chunks[self.index] + self.index += 1 + return chunk + + +class TestFetchRoutes: + """Test suite for chunk-based reading in fetch_routes()""" + + def setup_method(self): + """Setup for each test method""" + route_check.UNIT_TESTING = 1 + route_check.FRR_WAIT_TIME = 0 + + def create_mock_process(self, data_bytes): + """ + Create a mock subprocess.Popen object with stdout that returns data + in chunks. + + Args: + data_bytes: bytes to return from stdout + + Returns: + Mock Popen object + """ + mock_proc = Mock() + mock_proc.stdout = BytesIO(data_bytes) + mock_proc.wait = Mock(return_value=0) + mock_proc.kill = Mock() + mock_proc.__enter__ = Mock(return_value=mock_proc) + mock_proc.__exit__ = Mock(return_value=False) + return mock_proc + + def test_complete_json_single_chunk(self): + """Test parsing when complete JSON fits in a single chunk""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert result == ([{"prefix": "192.168.1.0/24", "protocol": "bgp"}], []) + + def test_json_split_across_chunks(self): + """Test parsing when JSON is split across multiple chunks""" + json_data = { + "10.0.0.0/8": [{ + "prefix": "10.0.0.0/8", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }], + "172.16.0.0/12": [{ + "prefix": "172.16.0.0/12", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }] + } + json_str = json.dumps(json_data) + + # Split the JSON string in the middle + split_point = len(json_str) // 2 + chunk1 = json_str[:split_point].encode('utf-8') + chunk2 = json_str[split_point:].encode('utf-8') + + # Create a custom BytesIO that returns data in specific chunk sizes + mock_proc = Mock() + mock_proc.stdout = ChunkedBytesIO([chunk1, chunk2]) + mock_proc.wait = Mock(return_value=0) + mock_proc.__enter__ = Mock(return_value=mock_proc) + mock_proc.__exit__ = Mock(return_value=False) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert get_missing_prefixes(result[0]) == {"10.0.0.0/8", "172.16.0.0/12"} + + def test_routes_with_offloaded_flag(self): + """Test that routes with offloaded=True are not included in missing + routes""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": True # This should NOT be in missing routes + }], + "192.168.2.0/24": [{ + "prefix": "192.168.2.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False # This SHOULD be in missing routes + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert result == ([{"prefix": "192.168.2.0/24", "protocol": "bgp"}], []) + + def test_filter_connected_kernel_static_protocols(self): + """Test that connected, kernel and static protocols are filtered out""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "connected", + "vrfName": "default", + "selected": True, + "offloaded": False + }], + "192.168.2.0/24": [{ + "prefix": "192.168.2.0/24", + "protocol": "kernel", + "vrfName": "default", + "selected": True, + "offloaded": False + }], + "192.168.3.0/24": [{ + "prefix": "192.168.3.0/24", + "protocol": "static", + "vrfName": "default", + "selected": True, + "offloaded": False + }], + "192.168.4.0/24": [{ + "prefix": "192.168.4.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + # Only BGP route should be in the result + assert result == ([{"prefix": "192.168.4.0/24", "protocol": "bgp"}], []) + + def test_filter_non_default_vrf(self): + """Test that routes in non-default VRF are filtered out""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "Vrf_RED", + "selected": True, + "offloaded": False + }], + "192.168.2.0/24": [{ + "prefix": "192.168.2.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }], + "192.168.3.0/24": [{ + "prefix": "192.168.3.0/24", + "protocol": "bgp", + "vrfName": "mgmt", + "selected": True, + "offloaded": False + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + # Only default VRF route should be in the result + assert result == ([{"prefix": "192.168.2.0/24", "protocol": "bgp"}], []) + + def test_filter_not_selected_routes(self): + """Test that routes not selected as best are filtered out""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": False, + "offloaded": False + }], + "192.168.2.0/24": [{ + "prefix": "192.168.2.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + # Only selected route should be in the result + assert result == ([{"prefix": "192.168.2.0/24", "protocol": "bgp"}], []) + + def test_empty_json_response(self): + """Test handling of empty JSON response""" + json_data = {} + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert result == ([], []) + + def test_utf8_split_across_chunks(self): + """Test handling of UTF-8 multibyte characters split across chunks""" + # Create JSON with UTF-8 characters + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False, + "description": "Test route with รฉmojis ๐Ÿš€" + }] + } + json_str = json.dumps(json_data) + json_bytes = json_str.encode('utf-8') + + # Find a multibyte character and split there + # The emoji ๐Ÿš€ is 4 bytes in UTF-8 + emoji_pos = json_bytes.find('๐Ÿš€'.encode('utf-8')) + if emoji_pos > 0: + # Split in the middle of the emoji + split_point = emoji_pos + 2 + chunk1 = json_bytes[:split_point] + chunk2 = json_bytes[split_point:] + + mock_proc = Mock() + mock_proc.stdout = ChunkedBytesIO([chunk1, chunk2]) + mock_proc.wait = Mock(return_value=0) + mock_proc.__enter__ = Mock(return_value=mock_proc) + mock_proc.__exit__ = Mock(return_value=False) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert result == ([{"prefix": "192.168.1.0/24", "protocol": "bgp"}], []) + + def test_very_small_chunks(self): + """Test parsing with very small chunk sizes (byte-by-byte)""" + json_data = { + "10.0.0.0/8": [{ + "prefix": "10.0.0.0/8", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + # Create chunks of 10 bytes each + chunks = [json_bytes[i:i+10] for i in range(0, len(json_bytes), 10)] + + mock_proc = Mock() + mock_proc.stdout = ChunkedBytesIO(chunks) + mock_proc.wait = Mock(return_value=0) + mock_proc.__enter__ = Mock(return_value=mock_proc) + mock_proc.__exit__ = Mock(return_value=False) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert result == ([{"prefix": "10.0.0.0/8", "protocol": "bgp"}], []) + + def test_large_json_with_many_routes(self): + """Test parsing large JSON with many route entries""" + json_data = {} + expected_missing = [] + + # Create 100 routes + for i in range(100): + prefix = f"10.{i}.0.0/16" + json_data[prefix] = [{ + "prefix": prefix, + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }] + expected_missing.append(prefix) + + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert get_missing_prefixes(result[0]) == set(expected_missing) + assert len(result[0]) == 100 + + def test_ipv6_command(self): + """Test that IPv6 command is properly constructed""" + json_data = { + "2001:db8::/32": [{ + "prefix": "2001:db8::/32", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc) \ + as mock_popen: + result = route_check.fetch_routes(ipv6=True) + + # Verify the correct command was called + mock_popen.assert_called_once() + call_args = mock_popen.call_args[0][0] + assert call_args == ["sudo", "vtysh", "-c", "show ipv6 route json"] + + assert result == ([{"prefix": "2001:db8::/32", "protocol": "bgp"}], []) + + def test_subprocess_non_zero_exit_code(self): + """Test handling of subprocess with non-zero exit code""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = Mock() + mock_proc.stdout = BytesIO(json_bytes) + mock_proc.wait = Mock(return_value=1) # Non-zero exit code + mock_proc.__enter__ = Mock(return_value=mock_proc) + mock_proc.__exit__ = Mock(return_value=False) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + # Should still return the parsed routes (non-zero exit is just a warning) + assert get_missing_prefixes(result[0]) == {"192.168.1.0/24"} + + def test_file_not_found_error(self): + """Test handling of FileNotFoundError when vtysh is not found""" + with patch('route_check.subprocess.Popen', + side_effect=FileNotFoundError("vtysh not found")): + with pytest.raises(RuntimeError, match="not found"): + route_check.fetch_routes() + + def test_multiple_route_entries_per_prefix(self): + """Test handling of multiple route entries for the same prefix""" + json_data = { + "192.168.1.0/24": [ + { + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": True + }, + { + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": False, + "offloaded": False + } + ] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + # First entry is offloaded, second is not selected, so no missing + # routes + assert result == ([], []) + + def test_json_with_nested_objects(self): + """Test parsing JSON with deeply nested objects""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False, + "nexthops": [ + { + "ip": "10.0.0.1", + "interfaceName": "Ethernet0", + "active": True + } + ] + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert result == ([{"prefix": "192.168.1.0/24", "protocol": "bgp"}], []) + + def test_variable_chunk_boundary(self): + """Test when chunk boundary falls at variious locations""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }] + } + json_str = json.dumps(json_data) + json_bytes = json_str.encode('utf-8') + + json_len = len(json_str) + for chunk_boundary in range(1, json_len): + # Find the position of the first closing brace + chunk1 = json_bytes[:chunk_boundary+1] + chunk2 = json_bytes[chunk_boundary+1:] + + mock_proc = Mock() + mock_proc.stdout = ChunkedBytesIO([chunk1, chunk2]) + mock_proc.wait = Mock(return_value=0) + mock_proc.__enter__ = Mock(return_value=mock_proc) + mock_proc.__exit__ = Mock(return_value=False) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert result == ([{"prefix": "192.168.1.0/24", "protocol": "bgp"}], []) + + def test_chunk_boundary_in_string_value(self): + """Test when chunk boundary falls in the middle of a string value""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False, + "description": "This is a very long description that will be split across chunks" + }] + } + json_str = json.dumps(json_data) + json_bytes = json_str.encode('utf-8') + + # Find the description and split in the middle of it + desc_start = json_str.find("This is a very long") + split_point = desc_start + 10 + chunk1 = json_bytes[:split_point] + chunk2 = json_bytes[split_point:] + + mock_proc = Mock() + mock_proc.stdout = ChunkedBytesIO([chunk1, chunk2]) + mock_proc.wait = Mock(return_value=0) + mock_proc.__enter__ = Mock(return_value=mock_proc) + mock_proc.__exit__ = Mock(return_value=False) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert result == ([{"prefix": "192.168.1.0/24", "protocol": "bgp"}], []) + + def test_escaped_quotes_in_json(self): + """Test handling of escaped quotes in JSON strings""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False, + "description": "Route with \"escaped\" quotes" + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert result == ([{"prefix": "192.168.1.0/24", "protocol": "bgp"}], []) + + def test_whitespace_handling(self): + """Test handling of JSON with various whitespace""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }] + } + # Add extra whitespace + json_str = json.dumps(json_data, indent=4) + json_str = json_str.replace(',', ' \n,') + json_bytes = json_str.encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert result == ([{"prefix": "192.168.1.0/24", "protocol": "bgp"}], []) + + def test_mixed_ipv4_and_ipv6_routes(self): + """Test parsing JSON with both IPv4 and IPv6 routes""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }], + "2001:db8::/32": [{ + "prefix": "2001:db8::/32", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert get_missing_prefixes(result[0]) == {"192.168.1.0/24", "2001:db8::/32"} + + def test_malformed_json_in_buffer(self): + """Test handling of malformed JSON that cannot be parsed""" + malformed_json = b'{"192.168.1.0/24": [{"prefix": "192.168.1.0/24", "protocol": "bgp"' + + mock_proc = self.create_mock_process(malformed_json) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + with pytest.raises(RuntimeError, match="Failed to parse"): + route_check.fetch_routes() + + def test_incremental_parsing_multiple_prefixes(self): + """Test that multiple prefix entries are parsed one at a time""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }], + "192.168.2.0/24": [{ + "prefix": "192.168.2.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }], + "192.168.3.0/24": [{ + "prefix": "192.168.3.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert get_missing_prefixes(result[0]) == { + "192.168.1.0/24", "192.168.2.0/24", "192.168.3.0/24" + } + + def test_incremental_parsing_prefix_split_across_chunks(self): + """Test that a prefix entry split across chunks is parsed correctly""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False, + "description": "This is a long description to make the entry larger" + }], + "192.168.2.0/24": [{ + "prefix": "192.168.2.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False, + "description": "Another long description for the second prefix" + }] + } + json_str = json.dumps(json_data) + json_bytes = json_str.encode('utf-8') + + # Split in the middle of the first prefix entry + # Find the position after the first prefix key + first_prefix_end = json_str.find(']', json_str.find('"192.168.1.0/24"')) + split_point = first_prefix_end - 20 # Split before the end of first entry + + chunk1 = json_bytes[:split_point] + chunk2 = json_bytes[split_point:] + + mock_proc = Mock() + mock_proc.stdout = ChunkedBytesIO([chunk1, chunk2]) + mock_proc.wait = Mock(return_value=0) + mock_proc.__enter__ = Mock(return_value=mock_proc) + mock_proc.__exit__ = Mock(return_value=False) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert get_missing_prefixes(result[0]) == {"192.168.1.0/24", "192.168.2.0/24"} + + def test_incremental_parsing_across_multiple_chunks(self): + # Create a large JSON with many prefixes + json_data = {} + expected_missing = [] + + for i in range(50): + prefix = f"10.{i}.0.0/16" + json_data[prefix] = [{ + "prefix": prefix, + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False, + "description": f"Route {i} with some description text to make it larger" + }] + expected_missing.append(prefix) + + json_bytes = json.dumps(json_data).encode('utf-8') + + # Split into multiple small chunks to test incremental parsing + chunk_size = 500 + chunks = [json_bytes[i:i+chunk_size] for i in range(0, len(json_bytes), chunk_size)] + + mock_proc = Mock() + mock_proc.stdout = ChunkedBytesIO(chunks) + mock_proc.wait = Mock(return_value=0) + mock_proc.__enter__ = Mock(return_value=mock_proc) + mock_proc.__exit__ = Mock(return_value=False) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert get_missing_prefixes(result[0]) == set(expected_missing) + assert len(result[0]) == 50 + + def test_incremental_parsing_prefix_boundary_at_comma(self): + """Test parsing when chunk boundary falls at comma between prefix entries""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }], + "192.168.2.0/24": [{ + "prefix": "192.168.2.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False + }] + } + json_str = json.dumps(json_data) + json_bytes = json_str.encode('utf-8') + + # Find the comma between the two entries + comma_pos = json_str.find('],') + 2 # Position after "]," + + chunk1 = json_bytes[:comma_pos] + chunk2 = json_bytes[comma_pos:] + + mock_proc = Mock() + mock_proc.stdout = ChunkedBytesIO([chunk1, chunk2]) + mock_proc.wait = Mock(return_value=0) + mock_proc.__enter__ = Mock(return_value=mock_proc) + mock_proc.__exit__ = Mock(return_value=False) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert get_missing_prefixes(result[0]) == {"192.168.1.0/24", "192.168.2.0/24"} + + def test_incremental_parsing_mixed_offloaded_states(self): + """Test incremental parsing with mixed offloaded states across multiple prefixes""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": True # This one is offloaded + }], + "192.168.2.0/24": [{ + "prefix": "192.168.2.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False # This one is missing + }], + "192.168.3.0/24": [{ + "prefix": "192.168.3.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": True # This one is offloaded + }], + "192.168.4.0/24": [{ + "prefix": "192.168.4.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False # This one is missing + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + # Only the non-offloaded routes should be in the result + assert get_missing_prefixes(result[0]) == {"192.168.2.0/24", "192.168.4.0/24"} + + def test_incremental_parsing_with_whitespace_variations(self): + """Test incremental parsing with various whitespace formatting""" + # Create JSON with extra whitespace and newlines + json_str = """{ + "192.168.1.0/24": [ + { + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": true, + "offloaded": false + } + ], + "192.168.2.0/24": [ + { + "prefix": "192.168.2.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": true, + "offloaded": false + } + ] + }""" + json_bytes = json_str.encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert get_missing_prefixes(result[0]) == {"192.168.1.0/24", "192.168.2.0/24"} + + def test_incremental_parsing_prefix_with_special_characters(self): + """Test incremental parsing of prefixes with special characters in descriptions""" + json_data = { + "192.168.1.0/24": [{ + "prefix": "192.168.1.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False, + "description": "Route with special chars: {}, [], \", \\" + }], + "192.168.2.0/24": [{ + "prefix": "192.168.2.0/24", + "protocol": "bgp", + "vrfName": "default", + "selected": True, + "offloaded": False, + "description": "Another route with: commas, colons: and braces {}" + }] + } + json_bytes = json.dumps(json_data).encode('utf-8') + + mock_proc = self.create_mock_process(json_bytes) + + with patch('route_check.subprocess.Popen', return_value=mock_proc): + result = route_check.fetch_routes() + + assert get_missing_prefixes(result[0]) == {"192.168.1.0/24", "192.168.2.0/24"} diff --git a/tests/route_check_test.py b/tests/route_check_test.py index 1f92b3d19..a4c28c7dc 100644 --- a/tests/route_check_test.py +++ b/tests/route_check_test.py @@ -244,12 +244,35 @@ def run_test(self, ct_data): patch('sonic_py_common.multi_asic.get_namespace_list', return_value= ct_data[NAMESPACE]), \ patch('sonic_py_common.multi_asic.is_multi_asic', return_value= ct_data[MULTI_ASIC]), \ patch('route_check.subprocess.check_output', side_effect=lambda *args, **kwargs: self.mock_check_output(ct_data, *args, **kwargs)), \ + patch('route_check.check_frr_pending_routes', + side_effect=lambda *args, **kwargs: self.mock_fetch_routes(ct_data, *args, **kwargs)), \ patch('route_check.mitigate_installed_not_offloaded_frr_routes', side_effect=lambda *args, **kwargs: None), \ patch('route_check.load_db_config', side_effect=lambda: init_db_conns(ct_data[NAMESPACE])): ret, res = route_check.main() self.assert_results(ct_data, ret, res) + def mock_fetch_routes(self, ct_data, *args, **kwargs): + ns = args[0] + routes = ct_data.get(FRR_ROUTES, {}).get(ns, {}) + if not routes: + return [], [] + missed_route_list = [] + failed_route_list = [] + for r, v in routes.items(): + for e in v: + if e.get('protocol') in ('connected', 'kernel', 'static'): + continue + if e.get('vrfName') != 'default': + continue + if not e.get('selected', False): + continue + if not e.get('offloaded', False): + missed_route_list.append({'prefix': r, 'protocol': e.get('protocol', '')}) + if e.get('failed', False): + failed_route_list.append(r) + return missed_route_list, failed_route_list + def mock_check_output(self, ct_data, *args, **kwargs): ns = self.extract_namespace_from_args(args[0]) routes = ct_data.get(FRR_ROUTES, {}).get(ns, {}) diff --git a/tests/route_check_test_data.py b/tests/route_check_test_data.py index fffd928c1..d292337bd 100644 --- a/tests/route_check_test_data.py +++ b/tests/route_check_test_data.py @@ -563,7 +563,7 @@ RESULT: { DEFAULTNS: { "missed_FRR_routes": [ - {"prefix": "10.10.196.12/31", "vrfName": "default", "protocol": "bgp", "selected": True} + {"prefix": "10.10.196.12/31", "protocol": "bgp"} ], }, }, @@ -977,7 +977,7 @@ RESULT: { ASIC1: { "missed_FRR_routes": [ - {"prefix": "10.10.196.12/31", "vrfName": "default", "protocol": "bgp", "selected": True} + {"prefix": "10.10.196.12/31", "protocol": "bgp"} ], }, }, @@ -1505,8 +1505,7 @@ RESULT: { ASIC0: { "missed_FRR_routes": [ - {"prefix": "10.10.196.20/31", "vrfName": "default", "protocol": "bgp", - "selected": True, "offloaded": False, "failed": True} + {"prefix": "10.10.196.20/31", "protocol": "bgp"} ], "failed_FRR_routes": [ "10.10.196.12/31", @@ -1599,10 +1598,8 @@ RESULT: { DEFAULTNS: { "missed_FRR_routes": [ - {"prefix": "10.10.196.12/31", "vrfName": "default", "protocol": "bgp", - "selected": True, "offloaded": False}, - {"prefix": "192.168.1.0/24", "vrfName": "default", "protocol": "bgp", - "selected": True, "offloaded": False, "failed": True} + {"prefix": "10.10.196.12/31", "protocol": "bgp"}, + {"prefix": "192.168.1.0/24", "protocol": "bgp"} ], "failed_FRR_routes": [ "10.10.196.20/31",