Repository: redhat-cip/hardware

Issue:
hardware-detect disk benchmark fails to parse modern fio output
The output of `fio` parsed by the disk benchmark has changed around 2017: https://github.com/axboe/fio/commit/d694a6a7c02f577b2bb5d0ad24331b775acf6869

The disk benchmark code in `hardware-detect` fails to parse its output.

Discussion:

Patch:
diff --git a/hardware/benchmark/disk.py b/hardware/benchmark/disk.py
index 6392f0c5..10de669b 100644
--- a/hardware/benchmark/disk.py
+++ b/hardware/benchmark/disk.py
@@ -18,8 +18,8 @@
 Benchmark Disk functions.
 """
 
+import json
 import os
-import re
 import subprocess
 import sys
 
@@ -70,7 +70,7 @@ def run_fio(hw_lst, disks_list, mode, io_size, time, rampup_time):
     for myfile in filelist:
         os.remove(myfile)
     fio = ("fio --ioengine=libaio --invalidate=1 --ramp_time=%d --iodepth=32 "
-           "--runtime=%d --time_based --direct=1 "
+           "--runtime=%d --time_based --direct=1 --output-format=json "
            "--bs=%s --rw=%s" % (rampup_time, time, io_size, mode))
 
     global_disk_list = ''
@@ -87,54 +87,21 @@ def run_fio(hw_lst, disks_list, mode, io_size, time, rampup_time):
         'Benchmarking storage %s for %s seconds in '
         '%s mode with blocksize=%s\n' %
         (global_disk_list, time, mode, io_size))
-    fio_cmd = subprocess.Popen(fio,
-                               shell=True, stdout=subprocess.PIPE)
-    current_disk = ''
-    for line in fio_cmd.stdout:
-        line = line.decode(errors='ignore')
-        if ('MYJOB-' in line) and ('pid=' in line):
-            # MYJOB-sda: (groupid=0, jobs=1): err= 0: pid=23652: Mon Sep  9
-            # 16:21:42 2013
-            current_disk = re.search(r"MYJOB-(.*): \(groupid", line).group(1)
-            continue
-        if ("read : io=" in line) or ("write: io=" in line):
-            # read : io=169756KB, bw=16947KB/s, iops=4230, runt= 10017msec
-            if len(disks_list) > 1:
-                mode_str = "simultaneous_%s_%s" % (mode, io_size)
-            else:
-                mode_str = "standalone_%s_%s" % (mode, io_size)
-
-            try:
-                perf = re.search('bw=(.*?B/s),', line).group(1)
-            except Exception:
-                sys.stderr.write('Failed at detecting '
-                                 'bwps pattern with %s\n' % line)
-            else:
-                multiply = 1
-                divide = 1
-                if "MB/s" in perf:
-                    multiply = 1024
-                elif "KB/s" in perf:
-                    multiply = 1
-                elif "B/s" in perf:
-                    divide = 1024
-                try:
-                    iperf = perf.replace(
-                        'KB/s', '').replace('MB/s', '').replace('B/s', '')
-                except Exception:
-                    True
-
-                value = str(int(float(float(iperf) * multiply / divide)))
+    fio_cmd = subprocess.check_output(fio, shell=True)
+    data = json.loads(fio_cmd)
+    for job in data['jobs']:
+        current_disk = job['jobname'].replace("MYJOB-", "")
+        if len(disks_list) > 1:
+            mode_str = "simultaneous_%s_%s" % (mode, io_size)
+        else:
+            mode_str = "standalone_%s_%s" % (mode, io_size)
+
+        for item in ['read', 'write']:
+            if job[item]['runtime'] > 0:
                 hw_lst.append(('disk', current_disk, mode_str + '_KBps',
-                               value))
-
-            try:
-                value = re.search('iops=(.*),', line).group(1).strip(' ')
+                               str(job[item]['bw'])))
                 hw_lst.append(('disk', current_disk, mode_str + '_IOps',
-                               value))
-            except Exception:
-                sys.stderr.write('Failed at detecting iops '
-                                 'pattern with %s\n' % line)
+                               str(job[item]['iops'])))
 
 
 def disk_perf(hw_lst, destructive=False, running_time=10):


Test patch:
diff --git a/hardware/tests/test_benchmark_disk.py b/hardware/tests/test_benchmark_disk.py
index 7186f6c1..5c6fde81 100644
--- a/hardware/tests/test_benchmark_disk.py
+++ b/hardware/tests/test_benchmark_disk.py
@@ -22,8 +22,31 @@ from unittest import mock
 from hardware.benchmark import disk
 
 
-FIO_OUTPUT_READ = """MYJOB-fake-disk: (groupid=0, jobs=1): err= 0: pid=5427:
-  read : io=123456KB, bw=123456KB/s, iops=123, runt= 10304msec"""
+FIO_OUTPUT_READ = """{
+    "jobs": [
+        {
+        "jobname": "MYJOB-fake-disk",
+        "groupid": 0,
+        "error": 0,
+        "read": {
+            "io_bytes": 126418944,
+            "io_kbytes": 123456,
+            "bw_bytes": 126418944,
+            "bw": 123456,
+            "iops": 123,
+            "runtime": 10304
+            },
+        "write": {
+            "io_bytes": 0,
+            "io_kbytes": 0,
+            "bw_bytes": 0,
+            "bw": 0,
+            "iops": 0,
+            "runtime": 0
+            }
+        }
+        ]
+    }"""
 
 DISK_PERF_EXPECTED = [
     ('disk', 'fake-disk', 'size', '10'),
@@ -43,7 +66,7 @@ DISK_PERF_EXPECTED = [
 ]
 
 
-@mock.patch.object(subprocess, 'Popen')
+@mock.patch.object(subprocess, 'check_output')
 class TestBenchmarkDisk(unittest.TestCase):
 
     def setUp(self):
@@ -51,19 +74,17 @@ class TestBenchmarkDisk(unittest.TestCase):
         self.hw_data = [('disk', 'fake-disk', 'size', '10'),
                         ('disk', 'fake-disk2', 'size', '15')]
 
-    def test_disk_perf_bytes(self, mock_popen):
-        mock_popen.return_value = mock.Mock(
-            stdout=FIO_OUTPUT_READ.encode().splitlines())
+    def test_disk_perf_bytes(self, mock_check_output):
+        mock_check_output.return_value = FIO_OUTPUT_READ.encode('utf-8')
         disk.disk_perf(self.hw_data)
         self.assertEqual(sorted(DISK_PERF_EXPECTED), sorted(self.hw_data))
 
-    def test_get_disks_name(self, mock_popen):
+    def test_get_disks_name(self, mock_check_output):
         result = disk.get_disks_name(self.hw_data)
         self.assertEqual(sorted(['fake-disk', 'fake-disk2']), sorted(result))
 
-    def test_run_fio(self, mock_popen):
-        mock_popen.return_value = mock.Mock(
-            stdout=FIO_OUTPUT_READ.encode().splitlines())
+    def test_run_fio(self, mock_check_output):
+        mock_check_output.return_value = FIO_OUTPUT_READ.encode('utf-8')
         hw_data = []
         disks_list = ['fake-disk', 'fake-disk2']
         disk.run_fio(hw_data, disks_list, "read", 123, 10, 5)