From a12a303e754637a356132c2b79f90b531e60190e Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Fri, 7 Nov 2025 16:12:12 -0500 Subject: [PATCH 1/8] Remove unused get_events_by_host method --- .../src/utils/log_analyzer/log_analyzer.py | 9 --------- tests/test_log_analyzer.py | 3 --- 2 files changed, 12 deletions(-) diff --git a/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py b/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py index e35b6a8..9f7bcc9 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py +++ b/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py @@ -4,7 +4,6 @@ import json import logging -from collections import defaultdict from typing import Dict, List, Any, cast import dateutil.parser @@ -123,14 +122,6 @@ def partition_cluster_events( return partitions or [[]] - def get_events_by_host(self) -> Dict[str, List[Dict[str, Any]]]: - """Get events grouped by host ID.""" - events_by_host = defaultdict(list) - for event in self.get_last_install_cluster_events(): - if "host_id" in event: - events_by_host[event["host_id"]].append(event) - return events_by_host - def get_host_log_file(self, host_id: str, filename: str) -> str: """ Get a specific log file for a host. diff --git a/tests/test_log_analyzer.py b/tests/test_log_analyzer.py index 1ef1033..dde7acd 100644 --- a/tests/test_log_analyzer.py +++ b/tests/test_log_analyzer.py @@ -64,9 +64,6 @@ def _get(path: str, **kwargs: Any) -> str: # pylint: disable=unused-argument # last partition should only include post-reset events last_events = la.get_last_install_cluster_events() assert last_events and last_events[0]["name"] == "something_else" - # grouped by host - by_host = la.get_events_by_host() - assert "h2" in by_host and by_host["h2"][0]["name"] == "something_else" def test_log_analyzer_host_log_paths() -> None: From 56a3fb783215a8428896a06868a17ef6f40b8595 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Fri, 7 Nov 2025 16:13:30 -0500 Subject: [PATCH 2/8] Remove unused get_must_gather method --- .../src/utils/log_analyzer/log_analyzer.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py b/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py index 9f7bcc9..581c47d 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py +++ b/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py @@ -196,15 +196,6 @@ def get_controller_logs(self) -> str: ), ) - def get_must_gather(self) -> bytes: - """Get must-gather logs.""" - return cast( - bytes, - self.logs_archive.get( - "controller_logs.tar.gz/must-gather.tar.gz", mode="rb" - ), - ) - @staticmethod def get_hostname(host: Dict[str, Any]) -> str: """Extract hostname from host metadata.""" From 8f7f7730e34c6ad05fe1c86a0379ef598df2f0d1 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Fri, 7 Nov 2025 16:14:21 -0500 Subject: [PATCH 3/8] Remove unused print_results function --- .../src/utils/log_analyzer/main.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/assisted_service_mcp/src/utils/log_analyzer/main.py b/assisted_service_mcp/src/utils/log_analyzer/main.py index 580eafb..c51794c 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/main.py +++ b/assisted_service_mcp/src/utils/log_analyzer/main.py @@ -69,17 +69,3 @@ async def analyze_cluster( except Exception as e: logger.error("Error analyzing cluster %s: %s", cluster_id, e) raise - - -def print_results(results: List[SignatureResult]) -> None: - """Print analysis results to stdout.""" - if not results: - print("No issues found in the cluster logs.") - return - - print("OpenShift Assisted Installer Log Analysis") - print("=" * 50) - print() - - for result in results: - print(result) From 7e1b886fe7472490403800c2467d70d90b4ef3b6 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Fri, 7 Nov 2025 16:15:31 -0500 Subject: [PATCH 4/8] Remove unused _search_patterns_in_string function --- .../utils/log_analyzer/signatures/error_detection.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py b/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py index 8264d4a..119287e 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py +++ b/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py @@ -21,18 +21,6 @@ # pylint: disable=duplicate-code - -def _search_patterns_in_string(string, patterns): - """Utility function to search for patterns in a string.""" - if isinstance(patterns, str): - patterns = [patterns] - - combined_regex = re.compile( - f'({"|".join(fr".*{pattern}.*" for pattern in patterns)})' - ) - return combined_regex.findall(string) - - class SNOHostnameHasEtcd(ErrorSignature): """Looks for etcd in SNO hostname (OCPBUGS-15852).""" From 14b64fa4a9df70a288eb51de3274bd8d4dd7023f Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Fri, 7 Nov 2025 16:16:38 -0500 Subject: [PATCH 5/8] Remove unused format_time function --- .../src/utils/log_analyzer/signatures/base.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/assisted_service_mcp/src/utils/log_analyzer/signatures/base.py b/assisted_service_mcp/src/utils/log_analyzer/signatures/base.py index 04b341e..78ddf20 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/signatures/base.py +++ b/assisted_service_mcp/src/utils/log_analyzer/signatures/base.py @@ -74,14 +74,6 @@ def generate_table(data: Sequence[dict[str, Any]]) -> str: return "No data available" return tabulate(data, headers="keys", tablefmt="grid") - @staticmethod - def format_time(time_str: str) -> str: - """Format time string for display.""" - try: - return dateutil.parser.isoparse(time_str).strftime("%Y-%m-%d %H:%M:%S") - except Exception: - return time_str - @staticmethod def archive_dir_contents(archive_dir): """ From 42a67afff577f62d5a1f99de3234d622e34d51be Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Fri, 7 Nov 2025 16:26:42 -0500 Subject: [PATCH 6/8] Remove fallbacks for the old log path Since this only works against the live assisted-service we don't need to care about the old log path --- .../src/utils/log_analyzer/log_analyzer.py | 21 +--- .../signatures/advanced_analysis.py | 114 +++++++++--------- .../signatures/error_detection.py | 35 +++--- .../log_analyzer/signatures/networking.py | 106 ++++++---------- 4 files changed, 111 insertions(+), 165 deletions(-) diff --git a/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py b/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py index 581c47d..76fa489 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py +++ b/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py @@ -11,11 +11,8 @@ logger = logging.getLogger(__name__) -# Archive path constants for different log bundle formats +# Archive path constant for log bundle format NEW_LOG_BUNDLE_PATH = "*_bootstrap_*.tar/*_bootstrap_*.tar.gz/logs_host_*/log-bundle-*.tar.gz/log-bundle-*" -OLD_LOG_BUNDLE_PATH = ( - "*_bootstrap_*.tar.gz/logs_host_*/log-bundle-*.tar.gz/log-bundle-*" -) class LogAnalyzer: @@ -170,21 +167,11 @@ def get_journal_log(self, host_ip: str, journal_file: str, **kwargs) -> str: Raises: FileNotFoundError: If the journal file cannot be found """ - new_logs_path = ( + logs_path = ( f"{NEW_LOG_BUNDLE_PATH}/control-plane/{host_ip}/journals/{journal_file}" ) - try: - content = self.logs_archive.get(new_logs_path, **kwargs) - logger.debug("Found journal under new location: %s", new_logs_path) - return cast(str, content) - except FileNotFoundError: - pass - - old_logs_path = ( - f"{OLD_LOG_BUNDLE_PATH}/control-plane/{host_ip}/journals/{journal_file}" - ) - content = self.logs_archive.get(old_logs_path, **kwargs) - logger.debug("Found journal under old location: %s", old_logs_path) + content = self.logs_archive.get(logs_path, **kwargs) + logger.debug("Found journal: %s", logs_path) return cast(str, content) def get_controller_logs(self) -> str: diff --git a/assisted_service_mcp/src/utils/log_analyzer/signatures/advanced_analysis.py b/assisted_service_mcp/src/utils/log_analyzer/signatures/advanced_analysis.py index 4469b51..dfa6cbf 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/signatures/advanced_analysis.py +++ b/assisted_service_mcp/src/utils/log_analyzer/signatures/advanced_analysis.py @@ -14,7 +14,6 @@ from assisted_service_mcp.src.utils.log_analyzer.log_analyzer import ( NEW_LOG_BUNDLE_PATH, - OLD_LOG_BUNDLE_PATH, ) from .base import Signature, SignatureResult @@ -228,48 +227,46 @@ def analyze(self, log_analyzer) -> Optional[SignatureResult]: return None if bootstrap[0]["progress"]["current_stage"] != "Waiting for controller": return None - for base in (NEW_LOG_BUNDLE_PATH, OLD_LOG_BUNDLE_PATH): - path = f"{base}/resources/pods.json" - try: - pods_json = log_analyzer.logs_archive.get(path) - except FileNotFoundError: - continue - try: - pods = json.loads(pods_json) - controller_pod = [ - pod - for pod in pods.get("items", []) - if pod.get("metadata", {}).get("namespace") == "assisted-installer" - ][0] - except Exception: - continue - try: - ready = [ - condition.get("status") == "True" - for condition in controller_pod.get("status", {}).get( - "conditions", {} - ) - if condition.get("type") == "Ready" - ][0] - except Exception: - ready = False - conditions_tbl = self.generate_table( - controller_pod.get("status", {}).get("conditions", []) - ) - containers_tbl = self.generate_table( - controller_pod.get("status", {}).get("containerStatuses", []) - ) - content = ( - f"The controller pod {'is' if ready else 'is not'} ready.\n" - f"Conditions:\n{conditions_tbl}\n\nContainer Statuses:\n{containers_tbl}" - ) - return SignatureResult( - signature_name=self.name, - title="Assisted Installer Controller failed to start", - content=content, - severity="warning", - ) - return None + path = f"{NEW_LOG_BUNDLE_PATH}/resources/pods.json" + try: + pods_json = log_analyzer.logs_archive.get(path) + except FileNotFoundError: + return None + try: + pods = json.loads(pods_json) + controller_pod = [ + pod + for pod in pods.get("items", []) + if pod.get("metadata", {}).get("namespace") == "assisted-installer" + ][0] + except Exception: + return None + try: + ready = [ + condition.get("status") == "True" + for condition in controller_pod.get("status", {}).get( + "conditions", {} + ) + if condition.get("type") == "Ready" + ][0] + except Exception: + ready = False + conditions_tbl = self.generate_table( + controller_pod.get("status", {}).get("conditions", []) + ) + containers_tbl = self.generate_table( + controller_pod.get("status", {}).get("containerStatuses", []) + ) + content = ( + f"The controller pod {'is' if ready else 'is not'} ready.\n" + f"Conditions:\n{conditions_tbl}\n\nContainer Statuses:\n{containers_tbl}" + ) + return SignatureResult( + signature_name=self.name, + title="Assisted Installer Controller failed to start", + content=content, + severity="warning", + ) class MachineConfigDaemonErrorExtracting(Signature): @@ -281,23 +278,22 @@ class MachineConfigDaemonErrorExtracting(Signature): ) def analyze(self, log_analyzer) -> Optional[SignatureResult]: - for base in (NEW_LOG_BUNDLE_PATH, OLD_LOG_BUNDLE_PATH): - path = ( - f"{base}/control-plane/*/journals/machine-config-daemon-firstboot.log" + path = ( + f"{NEW_LOG_BUNDLE_PATH}/control-plane/*/journals/machine-config-daemon-firstboot.log" + ) + try: + mcd_logs = log_analyzer.logs_archive.get(path) + except FileNotFoundError: + return None + if self.mco_error.search(mcd_logs): + return SignatureResult( + signature_name=self.name, + title="machine-config-daemon could not extract machine-os-content", + content=( + "machine-config-daemon-firstboot logs indicate a node may be hitting OCPBUGS-5352" + ), + severity="warning", ) - try: - mcd_logs = log_analyzer.logs_archive.get(path) - except FileNotFoundError: - continue - if self.mco_error.search(mcd_logs): - return SignatureResult( - signature_name=self.name, - title="machine-config-daemon could not extract machine-os-content", - content=( - "machine-config-daemon-firstboot logs indicate a node may be hitting OCPBUGS-5352" - ), - severity="warning", - ) return None diff --git a/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py b/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py index 119287e..e501f90 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py +++ b/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py @@ -12,7 +12,6 @@ import yaml from assisted_service_mcp.src.utils.log_analyzer.log_analyzer import ( NEW_LOG_BUNDLE_PATH, - OLD_LOG_BUNDLE_PATH, ) from .base import ErrorSignature, SignatureResult @@ -84,25 +83,23 @@ class ApiExpiredCertificateSignature(ErrorSignature): LOG_PATTERN = re.compile("x509: certificate has expired or is not yet valid.*") def analyze(self, log_analyzer) -> Optional[SignatureResult]: - new_logs_path = f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/bootstrap-control-plane/kube-apiserver.log" - old_logs_path = f"{OLD_LOG_BUNDLE_PATH}/bootstrap/containers/bootstrap-control-plane/kube-apiserver.log" - for path in (new_logs_path, old_logs_path): - try: - logs = log_analyzer.logs_archive.get(path) - except FileNotFoundError: - continue - invalid_api_log_lines = self.LOG_PATTERN.findall(logs) - if invalid_api_log_lines: - content = invalid_api_log_lines[0] - if (num_lines := len(invalid_api_log_lines)) > 1: - content += ( - f"\nadditional {num_lines - 1} similar error log lines found" - ) - return self.create_result( - title="Expired Certificate", - content=content, - severity="error", + path = f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/bootstrap-control-plane/kube-apiserver.log" + try: + logs = log_analyzer.logs_archive.get(path) + except FileNotFoundError: + return None + invalid_api_log_lines = self.LOG_PATTERN.findall(logs) + if invalid_api_log_lines: + content = invalid_api_log_lines[0] + if (num_lines := len(invalid_api_log_lines)) > 1: + content += ( + f"\nadditional {num_lines - 1} similar error log lines found" ) + return self.create_result( + title="Expired Certificate", + content=content, + severity="error", + ) return None diff --git a/assisted_service_mcp/src/utils/log_analyzer/signatures/networking.py b/assisted_service_mcp/src/utils/log_analyzer/signatures/networking.py index 6f937ad..2402c47 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/signatures/networking.py +++ b/assisted_service_mcp/src/utils/log_analyzer/signatures/networking.py @@ -12,7 +12,6 @@ from assisted_service_mcp.src.utils.log_analyzer.log_analyzer import ( NEW_LOG_BUNDLE_PATH, - OLD_LOG_BUNDLE_PATH, ) from assisted_service_mcp.src.utils.log_analyzer.signatures.advanced_analysis import ( operator_statuses_from_controller_logs, @@ -123,36 +122,6 @@ def analyze(self, log_analyzer) -> Optional[SignatureResult]: except FileNotFoundError: pass - # Fallback to OLD path - if not collisions: - try: - control_plane_dir = log_analyzer.logs_archive.get( - f"{OLD_LOG_BUNDLE_PATH}/control-plane/" - ) - for node_dir in self.archive_dir_contents(control_plane_dir): - node_ip = os.path.basename(node_dir) - try: - ip_addr = log_analyzer.logs_archive.get( - f"{OLD_LOG_BUNDLE_PATH}/control-plane/{node_ip}/network/ip-addr.txt" - ) - except FileNotFoundError: - continue - for vip in vips: - if vip in ip_addr: - collisions.append((vip, node_ip)) - except FileNotFoundError: - pass - - try: - bootstrap_ip_addr = log_analyzer.logs_archive.get( - f"{OLD_LOG_BUNDLE_PATH}/bootstrap/network/ip-addr.txt" - ) - for vip in vips: - if vip in bootstrap_ip_addr: - collisions.append((vip, "bootstrap")) - except FileNotFoundError: - pass - # Aggregate per VIP vip_to_nodes = {} for vip, node in collisions: @@ -226,19 +195,18 @@ def analyze(self, log_analyzer) -> Optional[SignatureResult]: return None report_lines = [] - for base in (NEW_LOG_BUNDLE_PATH, OLD_LOG_BUNDLE_PATH): - nameservers = self._get_nameservers(log_analyzer, base) - for ns in nameservers: - for cidr in cidrs: - try: - if ipaddress.ip_address(ns) in ipaddress.ip_network( - cidr, strict=False - ): - report_lines.append( - f"User defined nameserver {ns} overlaps with the cluster network {cidr}" - ) - except ValueError: - continue + nameservers = self._get_nameservers(log_analyzer, NEW_LOG_BUNDLE_PATH) + for ns in nameservers: + for cidr in cidrs: + try: + if ipaddress.ip_address(ns) in ipaddress.ip_network( + cidr, strict=False + ): + report_lines.append( + f"User defined nameserver {ns} overlaps with the cluster network {cidr}" + ) + except ValueError: + continue if report_lines: return self.create_result( title="Nameserver in internal network", @@ -279,18 +247,17 @@ class DualStackBadRoute(ErrorSignature): ) def analyze(self, log_analyzer) -> Optional[SignatureResult]: - for base in (NEW_LOG_BUNDLE_PATH, OLD_LOG_BUNDLE_PATH): - path = f"{base}/control-plane/*/containers/ovnkube-node-*.log" - try: - ovnkube_logs = log_analyzer.logs_archive.get(path) - except FileNotFoundError: - continue - if self.fatal_error_regex.search(ovnkube_logs): - return self.create_result( - title="Bugzilla 2088346", - content="ovnkube-node logs indicate the cluster may be hitting BZ 2088346", - severity="error", - ) + path = f"{NEW_LOG_BUNDLE_PATH}/control-plane/*/containers/ovnkube-node-*.log" + try: + ovnkube_logs = log_analyzer.logs_archive.get(path) + except FileNotFoundError: + return None + if self.fatal_error_regex.search(ovnkube_logs): + return self.create_result( + title="Bugzilla 2088346", + content="ovnkube-node logs indicate the cluster may be hitting BZ 2088346", + severity="error", + ) return None @@ -298,20 +265,19 @@ class DualstackrDNSBug(ErrorSignature): """Detect kube-apiserver 'must match public address family' message (MGMT-11651).""" def analyze(self, log_analyzer) -> Optional[SignatureResult]: - for base in (NEW_LOG_BUNDLE_PATH, OLD_LOG_BUNDLE_PATH): - path = f"{base}/bootstrap/containers/kube-apiserver-*.log" - try: - kubeapiserver_logs = log_analyzer.logs_archive.get(path) - except FileNotFoundError: - continue - if "must match public address family" in kubeapiserver_logs: - return self.create_result( - title="rDNS and DNS entries for IPv4/IPv6 interface - MGMT-11651", - content=( - "kube-apiserver logs contain the message 'must match public address family', this is probably due to MGMT-11651" - ), - severity="warning", - ) + path = f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/kube-apiserver-*.log" + try: + kubeapiserver_logs = log_analyzer.logs_archive.get(path) + except FileNotFoundError: + return None + if "must match public address family" in kubeapiserver_logs: + return self.create_result( + title="rDNS and DNS entries for IPv4/IPv6 interface - MGMT-11651", + content=( + "kube-apiserver logs contain the message 'must match public address family', this is probably due to MGMT-11651" + ), + severity="warning", + ) return None From 66841a5bb17fc072ed33cb5b3b5b65c664b1993c Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Fri, 7 Nov 2025 16:30:12 -0500 Subject: [PATCH 7/8] Rename NEW_LOG_BUNDLE_PATH to just LOG_BUNDLE_PATH --- .../src/utils/log_analyzer/log_analyzer.py | 4 +- .../signatures/advanced_analysis.py | 18 +++--- .../signatures/error_detection.py | 4 +- .../log_analyzer/signatures/networking.py | 14 ++--- tests/test_advanced_analysis.py | 58 +++++++++---------- 5 files changed, 49 insertions(+), 49 deletions(-) diff --git a/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py b/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py index 76fa489..2f08290 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py +++ b/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py @@ -12,7 +12,7 @@ logger = logging.getLogger(__name__) # Archive path constant for log bundle format -NEW_LOG_BUNDLE_PATH = "*_bootstrap_*.tar/*_bootstrap_*.tar.gz/logs_host_*/log-bundle-*.tar.gz/log-bundle-*" +LOG_BUNDLE_PATH = "*_bootstrap_*.tar/*_bootstrap_*.tar.gz/logs_host_*/log-bundle-*.tar.gz/log-bundle-*" class LogAnalyzer: @@ -168,7 +168,7 @@ def get_journal_log(self, host_ip: str, journal_file: str, **kwargs) -> str: FileNotFoundError: If the journal file cannot be found """ logs_path = ( - f"{NEW_LOG_BUNDLE_PATH}/control-plane/{host_ip}/journals/{journal_file}" + f"{LOG_BUNDLE_PATH}/control-plane/{host_ip}/journals/{journal_file}" ) content = self.logs_archive.get(logs_path, **kwargs) logger.debug("Found journal: %s", logs_path) diff --git a/assisted_service_mcp/src/utils/log_analyzer/signatures/advanced_analysis.py b/assisted_service_mcp/src/utils/log_analyzer/signatures/advanced_analysis.py index dfa6cbf..554ae40 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/signatures/advanced_analysis.py +++ b/assisted_service_mcp/src/utils/log_analyzer/signatures/advanced_analysis.py @@ -13,7 +13,7 @@ from typing import Any, Generator, Optional, Callable, List, Dict from assisted_service_mcp.src.utils.log_analyzer.log_analyzer import ( - NEW_LOG_BUNDLE_PATH, + LOG_BUNDLE_PATH, ) from .base import Signature, SignatureResult @@ -227,7 +227,7 @@ def analyze(self, log_analyzer) -> Optional[SignatureResult]: return None if bootstrap[0]["progress"]["current_stage"] != "Waiting for controller": return None - path = f"{NEW_LOG_BUNDLE_PATH}/resources/pods.json" + path = f"{LOG_BUNDLE_PATH}/resources/pods.json" try: pods_json = log_analyzer.logs_archive.get(path) except FileNotFoundError: @@ -279,7 +279,7 @@ class MachineConfigDaemonErrorExtracting(Signature): def analyze(self, log_analyzer) -> Optional[SignatureResult]: path = ( - f"{NEW_LOG_BUNDLE_PATH}/control-plane/*/journals/machine-config-daemon-firstboot.log" + f"{LOG_BUNDLE_PATH}/control-plane/*/journals/machine-config-daemon-firstboot.log" ) try: mcd_logs = log_analyzer.logs_archive.get(path) @@ -368,18 +368,18 @@ def _get_host_directories(self, log_analyzer) -> List[Dict[str, str]]: host_dirs.append( { "host_id": "bootstrap", - "kubelet_path": f"{NEW_LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log", - "containers_path": f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/", + "kubelet_path": f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log", + "containers_path": f"{LOG_BUNDLE_PATH}/bootstrap/containers/", } ) # Add control-plane directories try: control_plane_dir = log_analyzer.logs_archive.get( - f"{NEW_LOG_BUNDLE_PATH}/control-plane/" + f"{LOG_BUNDLE_PATH}/control-plane/" ) logger.debug( - "Found control-plane directory: %s/control-plane/", NEW_LOG_BUNDLE_PATH + "Found control-plane directory: %s/control-plane/", LOG_BUNDLE_PATH ) for node_dir in self.archive_dir_contents(control_plane_dir): @@ -389,8 +389,8 @@ def _get_host_directories(self, log_analyzer) -> List[Dict[str, str]]: host_dirs.append( { "host_id": node_ip, - "kubelet_path": f"{NEW_LOG_BUNDLE_PATH}/control-plane/{node_ip}/journals/kubelet.log", - "containers_path": f"{NEW_LOG_BUNDLE_PATH}/control-plane/{node_ip}/containers/", + "kubelet_path": f"{LOG_BUNDLE_PATH}/control-plane/{node_ip}/journals/kubelet.log", + "containers_path": f"{LOG_BUNDLE_PATH}/control-plane/{node_ip}/containers/", } ) except FileNotFoundError as e: diff --git a/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py b/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py index e501f90..36b2348 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py +++ b/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py @@ -11,7 +11,7 @@ import yaml from assisted_service_mcp.src.utils.log_analyzer.log_analyzer import ( - NEW_LOG_BUNDLE_PATH, + LOG_BUNDLE_PATH, ) from .base import ErrorSignature, SignatureResult @@ -83,7 +83,7 @@ class ApiExpiredCertificateSignature(ErrorSignature): LOG_PATTERN = re.compile("x509: certificate has expired or is not yet valid.*") def analyze(self, log_analyzer) -> Optional[SignatureResult]: - path = f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/bootstrap-control-plane/kube-apiserver.log" + path = f"{LOG_BUNDLE_PATH}/bootstrap/containers/bootstrap-control-plane/kube-apiserver.log" try: logs = log_analyzer.logs_archive.get(path) except FileNotFoundError: diff --git a/assisted_service_mcp/src/utils/log_analyzer/signatures/networking.py b/assisted_service_mcp/src/utils/log_analyzer/signatures/networking.py index 2402c47..805048d 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/signatures/networking.py +++ b/assisted_service_mcp/src/utils/log_analyzer/signatures/networking.py @@ -11,7 +11,7 @@ from typing import Optional from assisted_service_mcp.src.utils.log_analyzer.log_analyzer import ( - NEW_LOG_BUNDLE_PATH, + LOG_BUNDLE_PATH, ) from assisted_service_mcp.src.utils.log_analyzer.signatures.advanced_analysis import ( operator_statuses_from_controller_logs, @@ -95,13 +95,13 @@ def analyze(self, log_analyzer) -> Optional[SignatureResult]: # Check control-plane nodes try: control_plane_dir = log_analyzer.logs_archive.get( - f"{NEW_LOG_BUNDLE_PATH}/control-plane/" + f"{LOG_BUNDLE_PATH}/control-plane/" ) for node_dir in self.archive_dir_contents(control_plane_dir): node_ip = os.path.basename(node_dir) try: ip_addr = log_analyzer.logs_archive.get( - f"{NEW_LOG_BUNDLE_PATH}/control-plane/{node_ip}/network/ip-addr.txt" + f"{LOG_BUNDLE_PATH}/control-plane/{node_ip}/network/ip-addr.txt" ) except FileNotFoundError: continue @@ -114,7 +114,7 @@ def analyze(self, log_analyzer) -> Optional[SignatureResult]: # Check bootstrap try: bootstrap_ip_addr = log_analyzer.logs_archive.get( - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/network/ip-addr.txt" + f"{LOG_BUNDLE_PATH}/bootstrap/network/ip-addr.txt" ) for vip in vips: if vip in bootstrap_ip_addr: @@ -195,7 +195,7 @@ def analyze(self, log_analyzer) -> Optional[SignatureResult]: return None report_lines = [] - nameservers = self._get_nameservers(log_analyzer, NEW_LOG_BUNDLE_PATH) + nameservers = self._get_nameservers(log_analyzer, LOG_BUNDLE_PATH) for ns in nameservers: for cidr in cidrs: try: @@ -247,7 +247,7 @@ class DualStackBadRoute(ErrorSignature): ) def analyze(self, log_analyzer) -> Optional[SignatureResult]: - path = f"{NEW_LOG_BUNDLE_PATH}/control-plane/*/containers/ovnkube-node-*.log" + path = f"{LOG_BUNDLE_PATH}/control-plane/*/containers/ovnkube-node-*.log" try: ovnkube_logs = log_analyzer.logs_archive.get(path) except FileNotFoundError: @@ -265,7 +265,7 @@ class DualstackrDNSBug(ErrorSignature): """Detect kube-apiserver 'must match public address family' message (MGMT-11651).""" def analyze(self, log_analyzer) -> Optional[SignatureResult]: - path = f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/kube-apiserver-*.log" + path = f"{LOG_BUNDLE_PATH}/bootstrap/containers/kube-apiserver-*.log" try: kubeapiserver_logs = log_analyzer.logs_archive.get(path) except FileNotFoundError: diff --git a/tests/test_advanced_analysis.py b/tests/test_advanced_analysis.py index bf70a95..854361e 100644 --- a/tests/test_advanced_analysis.py +++ b/tests/test_advanced_analysis.py @@ -8,7 +8,7 @@ from assisted_service_mcp.src.utils.log_analyzer.log_analyzer import ( LogAnalyzer, - NEW_LOG_BUNDLE_PATH, + LOG_BUNDLE_PATH, ) from assisted_service_mcp.src.utils.log_analyzer.signatures.advanced_analysis import ( ContainerCrashAnalysis, @@ -93,7 +93,7 @@ def test_no_crashes_found(self) -> None: archive = make_archive( { - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, + f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, } ) @@ -114,11 +114,11 @@ def test_single_container_crash_info_severity(self) -> None: ) archive_map = { - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( + f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( [log_file_name] ), - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/{log_file_name}": self._create_container_log_content( + f"{LOG_BUNDLE_PATH}/bootstrap/containers/{log_file_name}": self._create_container_log_content( "etcd" ), } @@ -153,8 +153,8 @@ def test_multiple_crashes_warning_severity(self) -> None: kubelet_log = self._create_kubelet_log_with_crashes(crashes) archive_map = { - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( + f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( [] ), } @@ -188,8 +188,8 @@ def test_many_crashes_error_severity(self) -> None: kubelet_log = self._create_kubelet_log_with_crashes(crashes) archive_map = { - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( + f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( [] ), } @@ -222,15 +222,15 @@ def test_multiple_hosts_analysis(self) -> None: cp_kubelet_log = self._create_kubelet_log_with_crashes(cp_crashes) archive_map = { - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": bootstrap_kubelet_log, - f"{NEW_LOG_BUNDLE_PATH}/control-plane/": self._create_mock_directory( + f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": bootstrap_kubelet_log, + f"{LOG_BUNDLE_PATH}/control-plane/": self._create_mock_directory( ["192.168.1.10"] ), - f"{NEW_LOG_BUNDLE_PATH}/control-plane/192.168.1.10/journals/kubelet.log": cp_kubelet_log, - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( + f"{LOG_BUNDLE_PATH}/control-plane/192.168.1.10/journals/kubelet.log": cp_kubelet_log, + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( [] ), - f"{NEW_LOG_BUNDLE_PATH}/control-plane/192.168.1.10/containers/": self._create_mock_directory( + f"{LOG_BUNDLE_PATH}/control-plane/192.168.1.10/containers/": self._create_mock_directory( [] ), } @@ -260,8 +260,8 @@ def test_time_filtering_excludes_old_crashes(self) -> None: kubelet_log = self._create_kubelet_log_with_crashes(crashes) archive_map = { - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( + f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( [] ), } @@ -307,10 +307,10 @@ def test_invalid_timestamp_parsing(self) -> None: ] archive_map = { - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": "\n".join( + f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": "\n".join( kubelet_log_lines ), - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( [] ), } @@ -333,11 +333,11 @@ def test_container_logs_retrieval(self) -> None: log_file_name = "test-app-abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890.log" archive_map = { - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( + f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( [log_file_name] ), - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/{log_file_name}": self._create_container_log_content( + f"{LOG_BUNDLE_PATH}/bootstrap/containers/{log_file_name}": self._create_container_log_content( "test-app", 25 # More than 20 lines, ), } @@ -363,8 +363,8 @@ def test_container_logs_not_found(self) -> None: kubelet_log = self._create_kubelet_log_with_crashes(crashes) archive_map = { - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( + f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( [] ), # No log files } @@ -385,7 +385,7 @@ def test_containers_directory_not_found(self) -> None: kubelet_log = self._create_kubelet_log_with_crashes(crashes) archive_map = { - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, + f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, # No containers directory } @@ -453,7 +453,7 @@ def test_empty_kubelet_log(self) -> None: signature = ContainerCrashAnalysis() archive_map = { - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": "", # Empty log + f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": "", # Empty log } archive = make_archive(archive_map) @@ -476,12 +476,12 @@ def test_multiple_container_log_files(self) -> None: log_file_name_2 = "multi-log-container-fedcba0987654321fedcba0987654321fedcba0987654321fedcba09abcdef12.log" archive_map = { - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( + f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( [log_file_name_1, log_file_name_2] ), - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/{log_file_name_1}": log_content_1, - f"{NEW_LOG_BUNDLE_PATH}/bootstrap/containers/{log_file_name_2}": log_content_2, + f"{LOG_BUNDLE_PATH}/bootstrap/containers/{log_file_name_1}": log_content_1, + f"{LOG_BUNDLE_PATH}/bootstrap/containers/{log_file_name_2}": log_content_2, } archive = make_archive(archive_map) From f1b8099ab599b8d17328f95d77596eb2f7d63f1d Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Fri, 7 Nov 2025 16:31:43 -0500 Subject: [PATCH 8/8] make format --- .../src/utils/log_analyzer/log_analyzer.py | 4 +--- .../signatures/advanced_analysis.py | 8 ++------ .../src/utils/log_analyzer/signatures/base.py | 1 - .../signatures/error_detection.py | 5 ++--- tests/test_advanced_analysis.py | 20 +++++-------------- 5 files changed, 10 insertions(+), 28 deletions(-) diff --git a/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py b/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py index 2f08290..d0c6850 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py +++ b/assisted_service_mcp/src/utils/log_analyzer/log_analyzer.py @@ -167,9 +167,7 @@ def get_journal_log(self, host_ip: str, journal_file: str, **kwargs) -> str: Raises: FileNotFoundError: If the journal file cannot be found """ - logs_path = ( - f"{LOG_BUNDLE_PATH}/control-plane/{host_ip}/journals/{journal_file}" - ) + logs_path = f"{LOG_BUNDLE_PATH}/control-plane/{host_ip}/journals/{journal_file}" content = self.logs_archive.get(logs_path, **kwargs) logger.debug("Found journal: %s", logs_path) return cast(str, content) diff --git a/assisted_service_mcp/src/utils/log_analyzer/signatures/advanced_analysis.py b/assisted_service_mcp/src/utils/log_analyzer/signatures/advanced_analysis.py index 554ae40..5fea2fd 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/signatures/advanced_analysis.py +++ b/assisted_service_mcp/src/utils/log_analyzer/signatures/advanced_analysis.py @@ -244,9 +244,7 @@ def analyze(self, log_analyzer) -> Optional[SignatureResult]: try: ready = [ condition.get("status") == "True" - for condition in controller_pod.get("status", {}).get( - "conditions", {} - ) + for condition in controller_pod.get("status", {}).get("conditions", {}) if condition.get("type") == "Ready" ][0] except Exception: @@ -278,9 +276,7 @@ class MachineConfigDaemonErrorExtracting(Signature): ) def analyze(self, log_analyzer) -> Optional[SignatureResult]: - path = ( - f"{LOG_BUNDLE_PATH}/control-plane/*/journals/machine-config-daemon-firstboot.log" - ) + path = f"{LOG_BUNDLE_PATH}/control-plane/*/journals/machine-config-daemon-firstboot.log" try: mcd_logs = log_analyzer.logs_archive.get(path) except FileNotFoundError: diff --git a/assisted_service_mcp/src/utils/log_analyzer/signatures/base.py b/assisted_service_mcp/src/utils/log_analyzer/signatures/base.py index 78ddf20..eedb579 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/signatures/base.py +++ b/assisted_service_mcp/src/utils/log_analyzer/signatures/base.py @@ -6,7 +6,6 @@ import logging from typing import Optional, Any, Sequence -import dateutil.parser from tabulate import tabulate logger = logging.getLogger(__name__) diff --git a/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py b/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py index 36b2348..6a1b281 100644 --- a/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py +++ b/assisted_service_mcp/src/utils/log_analyzer/signatures/error_detection.py @@ -20,6 +20,7 @@ # pylint: disable=duplicate-code + class SNOHostnameHasEtcd(ErrorSignature): """Looks for etcd in SNO hostname (OCPBUGS-15852).""" @@ -92,9 +93,7 @@ def analyze(self, log_analyzer) -> Optional[SignatureResult]: if invalid_api_log_lines: content = invalid_api_log_lines[0] if (num_lines := len(invalid_api_log_lines)) > 1: - content += ( - f"\nadditional {num_lines - 1} similar error log lines found" - ) + content += f"\nadditional {num_lines - 1} similar error log lines found" return self.create_result( title="Expired Certificate", content=content, diff --git a/tests/test_advanced_analysis.py b/tests/test_advanced_analysis.py index 854361e..d9e7b45 100644 --- a/tests/test_advanced_analysis.py +++ b/tests/test_advanced_analysis.py @@ -154,9 +154,7 @@ def test_multiple_crashes_warning_severity(self) -> None: archive_map = { f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, - f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( - [] - ), + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory([]), } archive = make_archive(archive_map) @@ -189,9 +187,7 @@ def test_many_crashes_error_severity(self) -> None: archive_map = { f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, - f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( - [] - ), + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory([]), } archive = make_archive(archive_map) @@ -227,9 +223,7 @@ def test_multiple_hosts_analysis(self) -> None: ["192.168.1.10"] ), f"{LOG_BUNDLE_PATH}/control-plane/192.168.1.10/journals/kubelet.log": cp_kubelet_log, - f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( - [] - ), + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory([]), f"{LOG_BUNDLE_PATH}/control-plane/192.168.1.10/containers/": self._create_mock_directory( [] ), @@ -261,9 +255,7 @@ def test_time_filtering_excludes_old_crashes(self) -> None: archive_map = { f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": kubelet_log, - f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( - [] - ), + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory([]), } archive = make_archive(archive_map) @@ -310,9 +302,7 @@ def test_invalid_timestamp_parsing(self) -> None: f"{LOG_BUNDLE_PATH}/bootstrap/journals/kubelet.log": "\n".join( kubelet_log_lines ), - f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory( - [] - ), + f"{LOG_BUNDLE_PATH}/bootstrap/containers/": self._create_mock_directory([]), } archive = make_archive(archive_map)