[PATCH] Adding stressor validation for stress-ng

[PATCH] Adding stressor validation for stress-ng

[Sana Sharma]

Previously stressor names were unvalidated.
This has been changed so that now if the user inputs an invalid
stressor name, they get an error message telling them to check
"stress-ng --help". The list of stressors is created dynamically
during "WorkloadPrepare()". The helper functions are currently a
part of the class but we could also make them static methods,
depending on opinions.

Signed-off-by: Sana Sharma <sansshar@redhat.com>
---
 rteval/modules/loads/stressng.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/rteval/modules/loads/stressng.py b/rteval/modules/loads/stressng.py
index 32e2dbd..a72ab9d 100644
--- a/rteval/modules/loads/stressng.py
+++ b/rteval/modules/loads/stressng.py
@@ -31,6 +31,25 @@ class Stressng(CommandLineLoad):
         # When this module runs, other load modules should not
         self.set_exclusive()
 
+    def get_valid_stressors(self):
+        """Query stress-ng for list of valid stressor names."""
+        try:
+            result = subprocess.run(['stress-ng', '--stressors'],
+                                  capture_output=True, text=True, check=True)
+            return result.stdout.strip().split()
+        except (subprocess.CalledProcessError, FileNotFoundError):
+            return []
+
+    def validate_stressor(self,stressor_name):
+        """Validate a single stressor name against stress-ng's available stressors."""
+        valid = self.get_valid_stressors()
+        if not valid:
+            return
+
+        if stressor_name not in valid:
+            raise ValueError(f"Invalid stress-ng stressor: '{stressor_name}'. "
+                            f"Run 'stress-ng --stressors' to see valid options.")
+
     def _WorkloadSetup(self):
         " Since there is nothing to build, we don't need to do anything here "
         return
@@ -51,6 +70,7 @@ class Stressng(CommandLineLoad):
 
         # stress-ng is only run if the user specifies an stressor
         self.args = ['stress-ng']
+        self.validate_stressor(self.cfg.stressor)
         self.args.append(f'--{str(self.cfg.stressor)}')
         if self.cfg.workers is not None:
             self.args.append(self.cfg.workers) #default is 0
-- 
2.54.0

Re: [PATCH] Adding stressor validation for stress-ng

[John Kacur]

Hi Sana,

Thanks for the patch! The validation logic looks good and the error messages are helpful.

One request: could you move `get_valid_stressors()` and `validate_stressor()`
out of the class and make them module-level functions? They don't need access to
the class instance, and module-level functions are easier to unit test and can be
imported directly.

Place them after the imports and before the class definition:

def get_valid_stressors():
    """Query stress-ng for list of valid stressor names."""
    try:
        result = subprocess.run(['stress-ng', '--stressors'],
                              capture_output=True, text=True, check=True)
        return result.stdout.strip().split()
    except (subprocess.CalledProcessError, FileNotFoundError):
        return []

def validate_stressor(stressor_name):
    """Validate a single stressor name against stress-ng's available stressors."""
    valid = get_valid_stressors()
    if not valid:
        return

    if stressor_name not in valid:
        raise ValueError(f"Invalid stress-ng stressor: '{stressor_name}'. "
                        f"Run 'stress-ng --stressors' to see valid options.")

Then call it in _WorkloadPrepare() as: validate_stressor(self.cfg.stressor)

Thanks!
John

[PATCH v2] Adding stressor validation for stress-ng

[Sana Sharma]

Previously stressor names were unvalidated.
This has been changed so that now if the user inputs an invalid
stressor name, they get an error message telling them to check
"stress-ng --help". The list of stressors is created dynamically
during "WorkloadPrepare()"

Signed-off-by: Sana Sharma <sansshar@redhat.com>
---
 rteval/modules/loads/stressng.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/rteval/modules/loads/stressng.py b/rteval/modules/loads/stressng.py
index 32e2dbd..ed93646 100644
--- a/rteval/modules/loads/stressng.py
+++ b/rteval/modules/loads/stressng.py
@@ -10,6 +10,25 @@ from rteval.Log import Log
 from rteval.systopology import SysTopology
 from rteval.cpulist_utils import CpuList
 
+
+def get_valid_stressors():
+    """Query stress-ng for list of valid stressor names."""
+    try:
+        result = subprocess.run(['stress-ng', '--stressors'],
+                                capture_output=True, text=True, check=True)
+        return result.stdout.strip().split()
+    except (subprocess.CalledProcessError, FileNotFoundError):
+        return []
+
+def validate_stressor(stressor_name):
+    """Validate a single stressor name against stress-ng's available stressors."""
+    valid = get_valid_stressors()
+    if not valid:
+        return
+    if stressor_name not in valid:
+        raise ValueError(f"Invalid stress-ng stressor: '{stressor_name}'. "
+                        f"Run 'stress-ng --stressors' to see valid options.")
+
 class Stressng(CommandLineLoad):
     " This class creates a load module that runs stress-ng "
     def __init__(self, config, logger):
@@ -51,6 +70,7 @@ class Stressng(CommandLineLoad):
 
         # stress-ng is only run if the user specifies an stressor
         self.args = ['stress-ng']
+        validate_stressor(self.cfg.stressor)
         self.args.append(f'--{str(self.cfg.stressor)}')
         if self.cfg.workers is not None:
             self.args.append(self.cfg.workers) #default is 0
-- 
2.54.0

Re: [PATCH v2] Adding stressor validation for stress-ng

[John Kacur]

Hi Sana,

I have one suggestion about error handling:

**Issue: Silent failure when stress-ng is not installed**

In `get_valid_stressors()`, when stress-ng is not found, the function returns an empty list:

```python
except (subprocess.CalledProcessError, FileNotFoundError):
    return []
```

Then in `validate_stressor()`, this empty list causes validation to silently pass:

```python
if not valid:
    return  # Allows any stressor name through
```

**Suggested fix:**

```python
def get_valid_stressors():
    """Query stress-ng for list of valid stressor names."""
    try:
        result = subprocess.run(['stress-ng', '--stressors'],
                                capture_output=True, text=True, check=True)
        return result.stdout.strip().split()
    except FileNotFoundError:
        print("stress-ng is not installed. Please install the stress-ng package.")
        sys.exit(1)
    except subprocess.CalledProcessError as e:
        print(f"Failed to query stress-ng stressors: {e}")
        sys.exit(1)
```

This follows the pattern used in rteval/cpupower.py and gives users a clear error message during `_WorkloadPrepare()` instead of a cryptic failure later.

Thanks
John Kacur

[PATCH] Adding stressor validation for stress-ng

[Sana Sharma]

Previously stressor names were unvalidated.
This has been changed so that now if the user inputs an invalid
stressor name, they get an error message telling them to check
"stress-ng --help". The list of stressors is created dynamically
during "WorkloadPrepare()". The helper functions are currently a
part of the class but we could also make them static methods,
depending on opinions.

Signed-off-by: Sana Sharma <sansshar@redhat.com>
---
 rteval/modules/loads/stressng.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/rteval/modules/loads/stressng.py b/rteval/modules/loads/stressng.py
index 32e2dbd..a72ab9d 100644
--- a/rteval/modules/loads/stressng.py
+++ b/rteval/modules/loads/stressng.py
@@ -31,6 +31,25 @@ class Stressng(CommandLineLoad):
         # When this module runs, other load modules should not
         self.set_exclusive()
 
+    def get_valid_stressors(self):
+        """Query stress-ng for list of valid stressor names."""
+        try:
+            result = subprocess.run(['stress-ng', '--stressors'],
+                                  capture_output=True, text=True, check=True)
+            return result.stdout.strip().split()
+        except (subprocess.CalledProcessError, FileNotFoundError):
+            return []
+
+    def validate_stressor(self,stressor_name):
+        """Validate a single stressor name against stress-ng's available stressors."""
+        valid = self.get_valid_stressors()
+        if not valid:
+            return
+
+        if stressor_name not in valid:
+            raise ValueError(f"Invalid stress-ng stressor: '{stressor_name}'. "
+                            f"Run 'stress-ng --stressors' to see valid options.")
+
     def _WorkloadSetup(self):
         " Since there is nothing to build, we don't need to do anything here "
         return
@@ -51,6 +70,7 @@ class Stressng(CommandLineLoad):
 
         # stress-ng is only run if the user specifies an stressor
         self.args = ['stress-ng']
+        self.validate_stressor(self.cfg.stressor)
         self.args.append(f'--{str(self.cfg.stressor)}')
         if self.cfg.workers is not None:
             self.args.append(self.cfg.workers) #default is 0
-- 
2.54.0