From 9066252215f8e2d9bd1f6f265809c7cb3001cc3a Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Thu, 5 Jun 2025 11:27:56 -0500 Subject: [PATCH 1/7] utest updates --- .../plugins/inband/storage/analyzer_args.py | 5 +- .../inband/storage/storage_analyzer.py | 67 +++++++++++-------- test/unit/plugin/test_storage_analyzer.py | 36 +++++----- 3 files changed, 60 insertions(+), 48 deletions(-) diff --git a/nodescraper/plugins/inband/storage/analyzer_args.py b/nodescraper/plugins/inband/storage/analyzer_args.py index dd65e5c6..5b1a48b9 100644 --- a/nodescraper/plugins/inband/storage/analyzer_args.py +++ b/nodescraper/plugins/inband/storage/analyzer_args.py @@ -23,8 +23,11 @@ # SOFTWARE. # ############################################################################### +from typing import Optional + from pydantic import BaseModel class StorageAnalyzerArgs(BaseModel): - min_required_free_space: str = "50G" + min_required_free_space_abs: Optional[str] = None + min_required_free_space_prct: int = 10 diff --git a/nodescraper/plugins/inband/storage/storage_analyzer.py b/nodescraper/plugins/inband/storage/storage_analyzer.py index 252f2bb3..f3babad0 100644 --- a/nodescraper/plugins/inband/storage/storage_analyzer.py +++ b/nodescraper/plugins/inband/storage/storage_analyzer.py @@ -54,41 +54,52 @@ def analyze_data( if args is None: args = StorageAnalyzerArgs() - min_free = convert_to_bytes(args.min_required_free_space) if not data.storage_data: self.result.message = "No storage data available" self.result.status = ExecutionStatus.NOT_RAN return self.result for device_name, device_data in data.storage_data.items(): - free = convert_to_bytes(str(device_data.free)) - if free > min_free: + condition = False + if args.min_required_free_space_abs: + min_free_abs = convert_to_bytes(args.min_required_free_space_abs) + free_abs = convert_to_bytes(str(device_data.free)) + if free_abs and free_abs > min_free_abs: + condition = True + else: + condition = True + + free_prct = 100 - device_data.percent + condition = condition and (free_prct < args.min_required_free_space_prct) + + if condition: self.result.message = f"'{device_name}' has {bytes_to_human_readable(device_data.free)} available, {device_data.percent}% used" self.result.status = ExecutionStatus.OK break - else: - self.result.message = "Not enough disk storage!" - self.result.status = ExecutionStatus.ERROR - # find the device with the largest total storage, and its free space - largest_device = max( - data.storage_data, - key=lambda x: convert_to_bytes(str(data.storage_data[x].total)), - ) - largest_free = data.storage_data[largest_device].free - largest_percent = data.storage_data[largest_device].percent - event_data = { - "largest_device": { - "device": largest_device, - "total": data.storage_data[largest_device].total, - "free": largest_free, - "percent": largest_percent, - }, - } - self._log_event( - category=EventCategory.STORAGE, - description=f"{self.result.message} {largest_percent}% used on {largest_device}", - data=event_data, - priority=EventPriority.CRITICAL, - console_log=True, - ) + else: + self.result.message = "Not enough disk storage!" + self.result.status = ExecutionStatus.ERROR + # find the device with the largest total storage, and its free space + largest_device = max( + data.storage_data, + key=lambda x: convert_to_bytes(str(data.storage_data[x].total)), + ) + largest_free = data.storage_data[largest_device].free + largest_percent = data.storage_data[largest_device].percent + largest_used = data.storage_data[largest_device].used + event_data = { + "largest_device": { + "device": largest_device, + "total": data.storage_data[largest_device].total, + "free": largest_free, + "percent": largest_percent, + }, + } + self._log_event( + category=EventCategory.STORAGE, + description=f"{self.result.message} {bytes_to_human_readable(largest_used)} and {largest_percent}%, used on {largest_device}", + data=event_data, + priority=EventPriority.CRITICAL, + console_log=True, + ) return self.result diff --git a/test/unit/plugin/test_storage_analyzer.py b/test/unit/plugin/test_storage_analyzer.py index d8be1473..280693ea 100644 --- a/test/unit/plugin/test_storage_analyzer.py +++ b/test/unit/plugin/test_storage_analyzer.py @@ -56,28 +56,20 @@ def analyzer(system_info): return StorageAnalyzer(system_info=system_info) -def test_nominal_with_config(analyzer, model_obj): - args = StorageAnalyzerArgs(min_required_free_space="600G") +def test_only_absolute_threshold_fails(analyzer, model_obj): + args = StorageAnalyzerArgs(min_required_free_space_abs="1TB") result = analyzer.analyze_data(model_obj, args) - assert result.status == ExecutionStatus.OK - assert result.message == "'/dev/nvme0n1p2' has 869.8GB available, 3.0% used" - assert len(result.events) == 0 - - -def test_nominal_no_config(analyzer, model_obj): - result = analyzer.analyze_data(model_obj) - assert result.status == ExecutionStatus.OK - assert result.message == "'/dev/nvme0n1p2' has 869.8GB available, 3.0% used" - assert len(result.events) == 0 + assert result.status == ExecutionStatus.ERROR + assert any(event.category == EventCategory.STORAGE.value for event in result.events) + assert any(event.priority == EventPriority.CRITICAL for event in result.events) -def test_insufficient_free_storage(analyzer, model_obj): - args = StorageAnalyzerArgs(min_required_free_space="1TB") +def test_only_percentage_threshold_fails(analyzer, model_obj): + args = StorageAnalyzerArgs(min_required_free_space_prct=50) result = analyzer.analyze_data(model_obj, args) assert result.status == ExecutionStatus.ERROR - for event in result.events: - assert event.category == EventCategory.STORAGE.value - assert event.priority == EventPriority.CRITICAL + assert any(event.category == EventCategory.STORAGE.value for event in result.events) + assert any(event.priority == EventPriority.CRITICAL for event in result.events) def test_windows_nominal(system_info): @@ -94,9 +86,15 @@ def test_windows_nominal(system_info): ) } ) - args = StorageAnalyzerArgs(min_required_free_space="10GB") + args = StorageAnalyzerArgs(min_required_free_space_abs="10GB", min_required_free_space_prct=50) result = analyzer.analyze_data(model, args) assert result.status == ExecutionStatus.OK - assert result.message == "'C:' has 466.44GB available, 53.97% used" + assert " has 466.44GB available, 53.97% used" in result.message assert len(result.events) == 0 + + args2 = StorageAnalyzerArgs(min_required_free_space_prct=40) + result2 = analyzer.analyze_data(model, args2) + assert result2.status == ExecutionStatus.ERROR + assert any(e.category == EventCategory.STORAGE.value for e in result2.events) + assert any(e.priority == EventPriority.CRITICAL for e in result2.events) From 23fbbc6457098c1df7e8bc5746f5a3fdac0013a7 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Fri, 6 Jun 2025 15:14:58 -0500 Subject: [PATCH 2/7] added filtering for devices, bug fix + updated utest --- .../plugins/inband/storage/analyzer_args.py | 3 +- .../inband/storage/storage_analyzer.py | 38 +++++++++-------- test/unit/plugin/test_storage_analyzer.py | 42 ++++++++++++------- 3 files changed, 51 insertions(+), 32 deletions(-) diff --git a/nodescraper/plugins/inband/storage/analyzer_args.py b/nodescraper/plugins/inband/storage/analyzer_args.py index 5b1a48b9..142a3d66 100644 --- a/nodescraper/plugins/inband/storage/analyzer_args.py +++ b/nodescraper/plugins/inband/storage/analyzer_args.py @@ -30,4 +30,5 @@ class StorageAnalyzerArgs(BaseModel): min_required_free_space_abs: Optional[str] = None - min_required_free_space_prct: int = 10 + min_required_free_space_prct: Optional[int] = None + ignore_devices: Optional[list] = [] diff --git a/nodescraper/plugins/inband/storage/storage_analyzer.py b/nodescraper/plugins/inband/storage/storage_analyzer.py index f3babad0..d76d7cdb 100644 --- a/nodescraper/plugins/inband/storage/storage_analyzer.py +++ b/nodescraper/plugins/inband/storage/storage_analyzer.py @@ -53,6 +53,14 @@ def analyze_data( """ if args is None: args = StorageAnalyzerArgs() + if ( + args.min_required_free_space_abs is None + and args.min_required_free_space_prct is None + ): + args.min_required_free_space_prct = 10 + self.logger.warning( + "No defaults provided for storage analyzer arguments. Setting min_required_free_space_prct=10" + ) if not data.storage_data: self.result.message = "No storage data available" @@ -60,6 +68,8 @@ def analyze_data( return self.result for device_name, device_data in data.storage_data.items(): + if args.ignore_devices and device_name in args.ignore_devices: + continue condition = False if args.min_required_free_space_abs: min_free_abs = convert_to_bytes(args.min_required_free_space_abs) @@ -69,35 +79,29 @@ def analyze_data( else: condition = True - free_prct = 100 - device_data.percent - condition = condition and (free_prct < args.min_required_free_space_prct) + if args.min_required_free_space_prct: + free_prct = 100 - device_data.percent + condition = condition and (free_prct > args.min_required_free_space_prct) if condition: self.result.message = f"'{device_name}' has {bytes_to_human_readable(device_data.free)} available, {device_data.percent}% used" self.result.status = ExecutionStatus.OK - break else: self.result.message = "Not enough disk storage!" self.result.status = ExecutionStatus.ERROR - # find the device with the largest total storage, and its free space - largest_device = max( - data.storage_data, - key=lambda x: convert_to_bytes(str(data.storage_data[x].total)), - ) - largest_free = data.storage_data[largest_device].free - largest_percent = data.storage_data[largest_device].percent - largest_used = data.storage_data[largest_device].used event_data = { - "largest_device": { - "device": largest_device, - "total": data.storage_data[largest_device].total, - "free": largest_free, - "percent": largest_percent, + "offending_device": { + "device": device_name, + "total": device_data.total, + "free": device_data.free, + "percent": device_data.percent, }, } + device = convert_to_bytes(str(device_data.total)) + prct = device_data.percent self._log_event( category=EventCategory.STORAGE, - description=f"{self.result.message} {bytes_to_human_readable(largest_used)} and {largest_percent}%, used on {largest_device}", + description=f"{self.result.message} {bytes_to_human_readable(device)} and {prct}%, used on {device_name}", data=event_data, priority=EventPriority.CRITICAL, console_log=True, diff --git a/test/unit/plugin/test_storage_analyzer.py b/test/unit/plugin/test_storage_analyzer.py index 280693ea..51d42452 100644 --- a/test/unit/plugin/test_storage_analyzer.py +++ b/test/unit/plugin/test_storage_analyzer.py @@ -57,22 +57,21 @@ def analyzer(system_info): def test_only_absolute_threshold_fails(analyzer, model_obj): - args = StorageAnalyzerArgs(min_required_free_space_abs="1TB") + args = StorageAnalyzerArgs(min_required_free_space_abs="800GB") result = analyzer.analyze_data(model_obj, args) - assert result.status == ExecutionStatus.ERROR - assert any(event.category == EventCategory.STORAGE.value for event in result.events) - assert any(event.priority == EventPriority.CRITICAL for event in result.events) + assert result.status == ExecutionStatus.OK + assert "'/dev/nvme0n1p2' has 869.8GB available, 3.0% used" in result.message def test_only_percentage_threshold_fails(analyzer, model_obj): - args = StorageAnalyzerArgs(min_required_free_space_prct=50) + args = StorageAnalyzerArgs(min_required_free_space_prct=99) result = analyzer.analyze_data(model_obj, args) assert result.status == ExecutionStatus.ERROR assert any(event.category == EventCategory.STORAGE.value for event in result.events) assert any(event.priority == EventPriority.CRITICAL for event in result.events) -def test_windows_nominal(system_info): +def test_both_abs_and_prct_fail(system_info): system_info.os_family = OSFamily.WINDOWS analyzer = StorageAnalyzer(system_info=system_info) @@ -87,14 +86,29 @@ def test_windows_nominal(system_info): } ) - args = StorageAnalyzerArgs(min_required_free_space_abs="10GB", min_required_free_space_prct=50) + args = StorageAnalyzerArgs(min_required_free_space_abs="10GB", min_required_free_space_prct=96) result = analyzer.analyze_data(model, args) - assert result.status == ExecutionStatus.OK - assert " has 466.44GB available, 53.97% used" in result.message - assert len(result.events) == 0 + assert result.status == ExecutionStatus.ERROR + assert "Not enough disk storage!" in result.message + assert len(result.events) == 1 + assert any(e.category == EventCategory.STORAGE.value for e in result.events) + assert any(e.priority == EventPriority.CRITICAL for e in result.events) - args2 = StorageAnalyzerArgs(min_required_free_space_prct=40) + args2 = StorageAnalyzerArgs(min_required_free_space_prct=40, min_required_dree_space_abs="1GB") result2 = analyzer.analyze_data(model, args2) - assert result2.status == ExecutionStatus.ERROR - assert any(e.category == EventCategory.STORAGE.value for e in result2.events) - assert any(e.priority == EventPriority.CRITICAL for e in result2.events) + assert result2.status == ExecutionStatus.OK + + +def test_device_filter(analyzer, model_obj): + model_obj.storage_data["some_device"] = DeviceStorageData( + total=1000, free=100, used=900, percent=90 + ) + + args = StorageAnalyzerArgs(min_required_free_space_prct="20") + result = analyzer.analyze_data(model_obj, args) + assert result.status == ExecutionStatus.ERROR + assert len(result.events) == 1 + + args2 = StorageAnalyzerArgs(min_required_free_space_prct="20", ignore_devices=["some_device"]) + result2 = analyzer.analyze_data(model_obj, args2) + assert result2.status == ExecutionStatus.OK From 5b753e4e6e397fe4559ec71af819597e2c855511 Mon Sep 17 00:00:00 2001 From: Luke Andrews Date: Wed, 11 Jun 2025 11:11:00 -0400 Subject: [PATCH 3/7] add support for regex match --- .../plugins/inband/storage/analyzer_args.py | 6 ++- .../inband/storage/storage_analyzer.py | 40 ++++++++++++++++++- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/nodescraper/plugins/inband/storage/analyzer_args.py b/nodescraper/plugins/inband/storage/analyzer_args.py index 142a3d66..1f44d1d3 100644 --- a/nodescraper/plugins/inband/storage/analyzer_args.py +++ b/nodescraper/plugins/inband/storage/analyzer_args.py @@ -25,10 +25,12 @@ ############################################################################### from typing import Optional -from pydantic import BaseModel +from pydantic import BaseModel, Field class StorageAnalyzerArgs(BaseModel): min_required_free_space_abs: Optional[str] = None min_required_free_space_prct: Optional[int] = None - ignore_devices: Optional[list] = [] + ignore_devices: Optional[list[str]] = Field(default_factory=list) + check_devices: Optional[list[str]] = Field(default_factory=list) + regex_match: bool = False diff --git a/nodescraper/plugins/inband/storage/storage_analyzer.py b/nodescraper/plugins/inband/storage/storage_analyzer.py index d76d7cdb..523b89fd 100644 --- a/nodescraper/plugins/inband/storage/storage_analyzer.py +++ b/nodescraper/plugins/inband/storage/storage_analyzer.py @@ -23,6 +23,7 @@ # SOFTWARE. # ############################################################################### +import re from typing import Optional from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus @@ -39,6 +40,36 @@ class StorageAnalyzer(DataAnalyzer[StorageDataModel, StorageAnalyzerArgs]): DATA_MODEL = StorageDataModel + def _matches_device_filter( + self, device_name: str, exp_devices: list[str], regex_match: bool + ) -> bool: + """Check if the device name matches any of the expected devices"" + + Args: + device_name (str): device name to check + exp_devices (list[str]): list of expected devices to match against + regex_match (bool): if True, use regex matching; otherwise, use exact match + + Returns: + bool: True if the device name matches any of the expected devices, False otherwise + """ + for exp_device in exp_devices: + if regex_match: + try: + device_regex = re.compile(exp_device) + except re.error: + self._log_event( + category=EventCategory.STORAGE, + description=f"Invalid regex pattern: {exp_device}", + priority=EventPriority.ERROR, + ) + continue + if device_regex.match(device_name): + return True + elif device_name == exp_device: + return True + return False + def analyze_data( self, data: StorageDataModel, args: Optional[StorageAnalyzerArgs] = None ) -> TaskResult: @@ -68,8 +99,15 @@ def analyze_data( return self.result for device_name, device_data in data.storage_data.items(): - if args.ignore_devices and device_name in args.ignore_devices: + if args.check_devices and not self._matches_device_filter( + device_name, args.check_devices, args.regex_match + ): continue + elif args.ignore_devices and self._matches_device_filter( + device_name, args.ignore_devices, args.regex_match + ): + continue + condition = False if args.min_required_free_space_abs: min_free_abs = convert_to_bytes(args.min_required_free_space_abs) From 845ee002e6c7d16121fa880e1ac00b75e4a2462e Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Wed, 11 Jun 2025 10:20:58 -0500 Subject: [PATCH 4/7] added check_devices --- .../plugins/inband/storage/analyzer_args.py | 1 + .../inband/storage/storage_analyzer.py | 46 ++++++++++++------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/nodescraper/plugins/inband/storage/analyzer_args.py b/nodescraper/plugins/inband/storage/analyzer_args.py index 142a3d66..323cd8a5 100644 --- a/nodescraper/plugins/inband/storage/analyzer_args.py +++ b/nodescraper/plugins/inband/storage/analyzer_args.py @@ -32,3 +32,4 @@ class StorageAnalyzerArgs(BaseModel): min_required_free_space_abs: Optional[str] = None min_required_free_space_prct: Optional[int] = None ignore_devices: Optional[list] = [] + check_devices: Optional[list] = [] diff --git a/nodescraper/plugins/inband/storage/storage_analyzer.py b/nodescraper/plugins/inband/storage/storage_analyzer.py index d76d7cdb..65790264 100644 --- a/nodescraper/plugins/inband/storage/storage_analyzer.py +++ b/nodescraper/plugins/inband/storage/storage_analyzer.py @@ -51,24 +51,30 @@ def analyze_data( Returns: TaskResult: Result of the storage analysis containing the status and message. """ - if args is None: - args = StorageAnalyzerArgs() - if ( - args.min_required_free_space_abs is None - and args.min_required_free_space_prct is None - ): - args.min_required_free_space_prct = 10 - self.logger.warning( - "No defaults provided for storage analyzer arguments. Setting min_required_free_space_prct=10" - ) + if args and ( + args.min_required_free_space_abs is None and args.min_required_free_space_prct is None + ): + args.min_required_free_space_prct = 10 + self.logger.warning( + "No defaults provided for storage analyzer arguments. Setting min_required_free_space_prct=10" + ) + else: + args = StorageAnalyzerArgs(min_required_free_space_prct=10) if not data.storage_data: self.result.message = "No storage data available" self.result.status = ExecutionStatus.NOT_RAN return self.result + self.result.status = ExecutionStatus.OK + fail = False + passing_devices = [] + failing_devices = [] for device_name, device_data in data.storage_data.items(): - if args.ignore_devices and device_name in args.ignore_devices: + if args.check_devices: + if device_name not in args.check_devices: + continue + elif args.ignore_devices and device_name in args.ignore_devices: continue condition = False if args.min_required_free_space_abs: @@ -84,11 +90,14 @@ def analyze_data( condition = condition and (free_prct > args.min_required_free_space_prct) if condition: - self.result.message = f"'{device_name}' has {bytes_to_human_readable(device_data.free)} available, {device_data.percent}% used" - self.result.status = ExecutionStatus.OK + passing_devices.append( + f"'{device_name}' has {bytes_to_human_readable(device_data.free)} available, {device_data.percent}% used" + ) else: - self.result.message = "Not enough disk storage!" - self.result.status = ExecutionStatus.ERROR + fail = True + device = convert_to_bytes(str(device_data.total)) + prct = device_data.percent + failing_devices.append(f"{device_name}") event_data = { "offending_device": { "device": device_name, @@ -97,8 +106,6 @@ def analyze_data( "percent": device_data.percent, }, } - device = convert_to_bytes(str(device_data.total)) - prct = device_data.percent self._log_event( category=EventCategory.STORAGE, description=f"{self.result.message} {bytes_to_human_readable(device)} and {prct}%, used on {device_name}", @@ -106,4 +113,9 @@ def analyze_data( priority=EventPriority.CRITICAL, console_log=True, ) + if fail: + self.result.message = f"Insufficient disk space on " f"[{', '.join(failing_devices)}]" + self.result.status = ExecutionStatus.ERROR + else: + self.result.message = ",".join(passing_devices) return self.result From b089cffd185141b81da42eab5ac2c018ebee2cec Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Wed, 11 Jun 2025 10:48:57 -0500 Subject: [PATCH 5/7] bug fix --- nodescraper/plugins/inband/storage/storage_analyzer.py | 10 ++++------ test/unit/plugin/test_storage_analyzer.py | 6 ++++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/nodescraper/plugins/inband/storage/storage_analyzer.py b/nodescraper/plugins/inband/storage/storage_analyzer.py index 788974ca..1f54e0b0 100644 --- a/nodescraper/plugins/inband/storage/storage_analyzer.py +++ b/nodescraper/plugins/inband/storage/storage_analyzer.py @@ -82,15 +82,13 @@ def analyze_data( Returns: TaskResult: Result of the storage analysis containing the status and message. """ - if args and ( - args.min_required_free_space_abs is None and args.min_required_free_space_prct is None - ): + if args is None: + args = StorageAnalyzerArgs(min_required_free_space_prct=10) + elif args.min_required_free_space_abs is None and args.min_required_free_space_prct is None: args.min_required_free_space_prct = 10 self.logger.warning( - "No defaults provided for storage analyzer arguments. Setting min_required_free_space_prct=10" + "No thresholds provided for storage analyzer arguments; defaulting to 10% free" ) - else: - args = StorageAnalyzerArgs(min_required_free_space_prct=10) if not data.storage_data: self.result.message = "No storage data available" diff --git a/test/unit/plugin/test_storage_analyzer.py b/test/unit/plugin/test_storage_analyzer.py index 51d42452..07ebdfbc 100644 --- a/test/unit/plugin/test_storage_analyzer.py +++ b/test/unit/plugin/test_storage_analyzer.py @@ -89,7 +89,7 @@ def test_both_abs_and_prct_fail(system_info): args = StorageAnalyzerArgs(min_required_free_space_abs="10GB", min_required_free_space_prct=96) result = analyzer.analyze_data(model, args) assert result.status == ExecutionStatus.ERROR - assert "Not enough disk storage!" in result.message + assert "Insufficient disk space" in result.message assert len(result.events) == 1 assert any(e.category == EventCategory.STORAGE.value for e in result.events) assert any(e.priority == EventPriority.CRITICAL for e in result.events) @@ -109,6 +109,8 @@ def test_device_filter(analyzer, model_obj): assert result.status == ExecutionStatus.ERROR assert len(result.events) == 1 - args2 = StorageAnalyzerArgs(min_required_free_space_prct="20", ignore_devices=["some_device"]) + args2 = StorageAnalyzerArgs( + min_required_free_space_prct="20", ignore_devices=["some_device"], regex_match=True + ) result2 = analyzer.analyze_data(model_obj, args2) assert result2.status == ExecutionStatus.OK From ec680139789a553065038f6f5b5aed124fb869d8 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Wed, 11 Jun 2025 12:24:41 -0500 Subject: [PATCH 6/7] bug fix + utest updates --- .../inband/storage/storage_analyzer.py | 16 +++--- test/unit/plugin/test_storage_analyzer.py | 52 +++++++++++++++++++ 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/nodescraper/plugins/inband/storage/storage_analyzer.py b/nodescraper/plugins/inband/storage/storage_analyzer.py index 1f54e0b0..4e92fca4 100644 --- a/nodescraper/plugins/inband/storage/storage_analyzer.py +++ b/nodescraper/plugins/inband/storage/storage_analyzer.py @@ -100,14 +100,14 @@ def analyze_data( passing_devices = [] failing_devices = [] for device_name, device_data in data.storage_data.items(): - if args.check_devices and not self._matches_device_filter( - device_name, args.check_devices, args.regex_match - ): - continue - elif args.ignore_devices and self._matches_device_filter( - device_name, args.ignore_devices, args.regex_match - ): - continue + if args.check_devices: + if not self._matches_device_filter( + device_name, args.check_devices, args.regex_match + ): + continue + elif args.ignore_devices: + if self._matches_device_filter(device_name, args.ignore_devices, args.regex_match): + continue condition = False if args.min_required_free_space_abs: diff --git a/test/unit/plugin/test_storage_analyzer.py b/test/unit/plugin/test_storage_analyzer.py index 07ebdfbc..426f5aff 100644 --- a/test/unit/plugin/test_storage_analyzer.py +++ b/test/unit/plugin/test_storage_analyzer.py @@ -51,11 +51,63 @@ def model_obj(): ) +@pytest.fixture +def multiple_dev(): + return StorageDataModel( + storage_data={ + "dev1": DeviceStorageData(total=100, free=90, used=10, percent=10), + "dev2": DeviceStorageData(total=100, free=5, used=95, percent=95), + } + ) + + @pytest.fixture def analyzer(system_info): return StorageAnalyzer(system_info=system_info) +def test_filter_exact_match(analyzer): + assert analyzer._matches_device_filter("foo", ["foo", "bar"], regex_match=False) + assert not analyzer._matches_device_filter("baz", ["foo", "bar"], regex_match=False) + + +def test_filter_regex_match(analyzer): + assert analyzer._matches_device_filter("disk0", [r"disk\d"], regex_match=True) + assert not analyzer._matches_device_filter("diskA", [r"disk\d"], regex_match=True) + + +def test_filter_invalid_regex(analyzer, monkeypatch): + calls = [] + monkeypatch.setattr(analyzer, "_log_event", lambda **kw: calls.append(kw)) + assert not analyzer._matches_device_filter("somestring", ["[invalid"], regex_match=True) + assert len(calls) == 1 + event = calls[0] + assert event["category"] == EventCategory.STORAGE + assert event["priority"] == EventPriority.ERROR + assert "Invalid regex pattern" in event["description"] + + +def test_check_devices_only(analyzer, multiple_dev): + args = StorageAnalyzerArgs(min_required_free_space_prct=50, check_devices=["dev1"]) + res = analyzer.analyze_data(multiple_dev, args) + assert res.status == ExecutionStatus.OK + + +def test_ignore_devices(analyzer, multiple_dev): + args = StorageAnalyzerArgs(min_required_free_space_prct=50, ignore_devices=["dev2"]) + res = analyzer.analyze_data(multiple_dev, args) + assert res.status == ExecutionStatus.OK + + +def test_check_overrides_ignore(analyzer, multiple_dev): + args = StorageAnalyzerArgs( + min_required_free_space_prct=50, check_devices=["dev2"], ignore_devices=["dev2", "dev1"] + ) + res = analyzer.analyze_data(multiple_dev, args) + assert res.status == ExecutionStatus.ERROR + assert any(e.category == EventCategory.STORAGE.value for e in res.events) + + def test_only_absolute_threshold_fails(analyzer, model_obj): args = StorageAnalyzerArgs(min_required_free_space_abs="800GB") result = analyzer.analyze_data(model_obj, args) From f901380e2f2bc26d84a3829150147710c9322588 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Mon, 16 Jun 2025 10:50:30 -0500 Subject: [PATCH 7/7] addressed reviews --- .../plugins/inband/storage/storage_analyzer.py | 16 +++++++--------- test/unit/plugin/test_storage_analyzer.py | 2 +- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/nodescraper/plugins/inband/storage/storage_analyzer.py b/nodescraper/plugins/inband/storage/storage_analyzer.py index 4e92fca4..6c2f8334 100644 --- a/nodescraper/plugins/inband/storage/storage_analyzer.py +++ b/nodescraper/plugins/inband/storage/storage_analyzer.py @@ -96,7 +96,6 @@ def analyze_data( return self.result self.result.status = ExecutionStatus.OK - fail = False passing_devices = [] failing_devices = [] for device_name, device_data in data.storage_data.items(): @@ -123,14 +122,11 @@ def analyze_data( condition = condition and (free_prct > args.min_required_free_space_prct) if condition: - passing_devices.append( - f"'{device_name}' has {bytes_to_human_readable(device_data.free)} available, {device_data.percent}% used" - ) + passing_devices.append(device_name) else: - fail = True device = convert_to_bytes(str(device_data.total)) prct = device_data.percent - failing_devices.append(f"{device_name}") + failing_devices.append(device_name) event_data = { "offending_device": { "device": device_name, @@ -141,14 +137,16 @@ def analyze_data( } self._log_event( category=EventCategory.STORAGE, - description=f"{self.result.message} {bytes_to_human_readable(device)} and {prct}%, used on {device_name}", + description=f"Insufficient disk space: {bytes_to_human_readable(device)} and {prct}%, used on {device_name}", data=event_data, priority=EventPriority.CRITICAL, console_log=True, ) - if fail: + if failing_devices: self.result.message = f"Insufficient disk space on " f"[{', '.join(failing_devices)}]" self.result.status = ExecutionStatus.ERROR else: - self.result.message = ",".join(passing_devices) + self.result.message = ( + f"Sufficient disk space available on " f"[{', '.join(passing_devices)}]" + ) return self.result diff --git a/test/unit/plugin/test_storage_analyzer.py b/test/unit/plugin/test_storage_analyzer.py index 426f5aff..21a16a8c 100644 --- a/test/unit/plugin/test_storage_analyzer.py +++ b/test/unit/plugin/test_storage_analyzer.py @@ -112,7 +112,7 @@ def test_only_absolute_threshold_fails(analyzer, model_obj): args = StorageAnalyzerArgs(min_required_free_space_abs="800GB") result = analyzer.analyze_data(model_obj, args) assert result.status == ExecutionStatus.OK - assert "'/dev/nvme0n1p2' has 869.8GB available, 3.0% used" in result.message + assert "Sufficient disk space available on [/dev/nvme0n1p2]" in result.message def test_only_percentage_threshold_fails(analyzer, model_obj):