Merge ~newell-jensen/maas:fio-test into maas:master

Proposed by Andres Rodriguez
Status: Superseded
Proposed branch: ~newell-jensen/maas:fio-test
Merge into: maas:master
Diff against target: 325 lines (+302/-0)
3 files modified
src/metadataserver/builtin_scripts/__init__.py (+4/-0)
src/metadataserver/builtin_scripts/fio.py (+161/-0)
src/metadataserver/builtin_scripts/tests/test_fio.py (+137/-0)
Reviewer Review Type Date Requested Status
Lee Trager Pending
Review via email: mp+330234@code.launchpad.net

This proposal supersedes a proposal from 2017-08-07.

This proposal has been superseded by a proposal from 2017-09-05.

Commit message

Add builtin script fio test for hardware testing.

To post a comment you must log in.
Revision history for this message
Lee Trager (ltrager) wrote : Posted in a previous version of this proposal

Thanks for working on this. Currently this doesn't work as fio needs to be installed. I'm also a little concerned we're dropping STDERR. Can you see if fio outputs anything to STDERR?

review: Needs Fixing
Revision history for this message
Newell Jensen (newell-jensen) wrote : Posted in a previous version of this proposal

> Thanks for working on this. Currently this doesn't work as fio needs to be
> installed. I'm also a little concerned we're dropping STDERR. Can you see if
> fio outputs anything to STDERR?

Go ahead and have another look at it. Hopefully changes in the spec will not demand too many changes with this :)

Revision history for this message
Newell Jensen (newell-jensen) wrote : Posted in a previous version of this proposal

Setting to WIP since things have been changed in the spec after meeting.

Revision history for this message
Lee Trager (ltrager) wrote : Posted in a previous version of this proposal

Updated review, we still need to add the MAAS metadata embedded YAML and modify MAAS to load the script in the database(I'm reworking that code now).

Revision history for this message
Lee Trager (ltrager) : Posted in a previous version of this proposal
Revision history for this message
Lee Trager (ltrager) : Posted in a previous version of this proposal
Revision history for this message
Lee Trager (ltrager) wrote : Posted in a previous version of this proposal

Looks good! One small thing inline and you have a merge conflict but I won't block you.

review: Approve
Revision history for this message
Andres Rodriguez (andreserl) wrote : Posted in a previous version of this proposal

This branch has been reverted because it is dependent on https://code.launchpad.net/~ltrager/maas/+git/maas/+merge/329983 , which has also been reverted.

Unmerged commits

9d6cb80... by Newell Jensen

Review fixes.

c9b16c7... by Newell Jensen

Fix lint.

7fcca1f... by Newell Jensen

Decode bytes from regex matching.

390f153... by Newell Jensen

Update embedded YAML and add fio to builtin scripts.

1a27095... by Newell Jensen

Update results dictionary in embedded YAML and change dictionary keys used in results file to match.

1db6b27... by Newell Jensen

Update embedded YAML.

4e46174... by Newell Jensen

Delete YAML metadata title's curly brackets to make it more readable.

667f9d0... by Newell Jensen

Add YAML metadata to fio test.

027849c... by Newell Jensen

Only print stdout if it is not None.

86eaea0... by Newell Jensen

Review fixes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/metadataserver/builtin_scripts/__init__.py b/src/metadataserver/builtin_scripts/__init__.py
2index c47f1a2..bfade7a 100644
3--- a/src/metadataserver/builtin_scripts/__init__.py
4+++ b/src/metadataserver/builtin_scripts/__init__.py
5@@ -131,6 +131,10 @@ BUILTIN_SCRIPTS = [
6 name='7z',
7 filename='seven_z.py',
8 ),
9+ BuiltinScript(
10+ name='fio',
11+ filename='fio.py',
12+ ),
13 ]
14
15
16diff --git a/src/metadataserver/builtin_scripts/fio.py b/src/metadataserver/builtin_scripts/fio.py
17new file mode 100644
18index 0000000..0cb5204
19--- /dev/null
20+++ b/src/metadataserver/builtin_scripts/fio.py
21@@ -0,0 +1,161 @@
22+#!/usr/bin/env python3
23+#
24+# fio - Run fio on supplied drive.
25+#
26+# Author: Newell Jensen <newell.jensen@canonical.com>
27+#
28+# Copyright (C) 2017 Canonical
29+#
30+# This program is free software: you can redistribute it and/or modify
31+# it under the terms of the GNU Affero General Public License as
32+# published by the Free Software Foundation, either version 3 of the
33+# License, or (at your option) any later version.
34+#
35+# This program is distributed in the hope that it will be useful,
36+# but WITHOUT ANY WARRANTY; without even the implied warranty of
37+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
38+# GNU Affero General Public License for more details.
39+#
40+# You should have received a copy of the GNU Affero General Public License
41+# along with this program. If not, see <http://www.gnu.org/licenses/>.
42+#
43+# --- Start MAAS 1.0 script metadata ---
44+# name: fio
45+# title: Storage benchmark
46+# description: Run Fio benchmarking against selected storage devices.
47+# tags: storage
48+# script_type: testing
49+# hardware_type: storage
50+# parallel: instance
51+# results:
52+# random_read:
53+# title: Random read
54+# description: Read speed when reading randomly from the disk.
55+# random_read_iops:
56+# title: Random read IOPS
57+# description: IOPS when reading randomly from the disk.
58+# sequential_read:
59+# title: Sequential read
60+# description: Read speed when reading sequentialy from the disk.
61+# sequential_read_iops:
62+# title: Sequential read IOPS
63+# description: IOPS when reading sequentialy from the disk.
64+# random_write:
65+# title: Random write
66+# description: Write speed when reading randomly from the disk.
67+# random_write_iops:
68+# title: Random write IOPS
69+# description: IOPS when reading randomly from the disk.
70+# sequential_write:
71+# title: Sequential write
72+# description: Write speed when reading sequentialy from the disk.
73+# sequential_write_iops:
74+# title: Sequential write IOPS
75+# description: IOPS when reading sequentialy from the disk.
76+# parameters:
77+# storage: {type: storage}
78+# packages: {apt: fio}
79+# destructive: true
80+# --- End MAAS 1.0 script metadata ---
81+
82+
83+import argparse
84+from copy import deepcopy
85+import os
86+import re
87+from subprocess import (
88+ PIPE,
89+ Popen,
90+ STDOUT,
91+)
92+import sys
93+
94+import yaml
95+
96+
97+CMD = [
98+ 'sudo', '-n', 'fio', '--randrepeat=1', '--ioengine=libaio',
99+ '--direct=1', '--gtod_reduce=1', '--name=fio_test', '--bs=4k',
100+ '--iodepth=64', '--size=4G'
101+]
102+
103+REGEX = b"bw=([0-9]+[a-zA-Z]+/s), iops=([0-9]+)"
104+
105+
106+def run_cmd(cmd):
107+ """Execute `cmd` and return output or exit if error."""
108+ proc = Popen(cmd, stdout=PIPE, stderr=STDOUT)
109+ # Currently, we are piping stderr to STDOUT.
110+ stdout, _ = proc.communicate()
111+
112+ # Print stdout to the console.
113+ if stdout is not None:
114+ print('Running command: %s\n' % ' '.join(cmd))
115+ print(stdout.decode())
116+ print('-' * 80)
117+ if proc.returncode == 0:
118+ return stdout, proc.returncode
119+ sys.exit(proc.returncode)
120+
121+
122+def run_fio_test(readwrite, result_path):
123+ """Run fio for the given type of test specified by `cmd`."""
124+ cmd = deepcopy(CMD)
125+ cmd.append('--readwrite=%s' % readwrite)
126+ stdout, returncode = run_cmd(cmd)
127+ if result_path is not None:
128+ # Parse the results for the desired information and
129+ # then wrtie this to the results file.
130+ match = re.search(REGEX, stdout)
131+ if match is None:
132+ print("Warning: results could not be found.")
133+ return match
134+
135+
136+def run_fio(storage):
137+ """Execute fio tests for supplied storage device.
138+
139+ Performs random and sequential read and write tests.
140+ """
141+ result_path = os.environ.get("RESULT_PATH")
142+ CMD.append('--filename=%s' % storage)
143+ random_read_match = run_fio_test("randread", result_path)
144+ sequential_read_match = run_fio_test("read", result_path)
145+ random_write_match = run_fio_test("randwrite", result_path)
146+ sequential_write_match = run_fio_test("write", result_path)
147+
148+ # Write out YAML file if RESULT_PATH is set.
149+ # The status is hardcoded at the moment because there isn't a
150+ # degraded state yet for fio. This most likely will change in
151+ # the future when there is an agreed upon crtteria for fio to
152+ # mark a storage device in the degraded state based on one of
153+ # its tests.
154+ if all(var is not None for var in [
155+ result_path, random_read_match, sequential_read_match,
156+ random_write_match, sequential_write_match]):
157+ results = {
158+ 'status': "passed",
159+ 'results': {
160+ 'random_read': random_read_match.group(1).decode(),
161+ 'random_read_iops': random_read_match.group(2).decode(),
162+ 'sequential_read': sequential_read_match.group(1).decode(),
163+ 'sequential_read_iops': (
164+ sequential_read_match.group(2).decode()),
165+ 'random_write': random_write_match.group(1).decode(),
166+ 'random_write_iops': random_write_match.group(2).decode(),
167+ 'sequential_write': sequential_write_match.group(1).decode(),
168+ 'sequential_write_iops': (
169+ sequential_write_match.group(2).decode()),
170+ }
171+ }
172+ with open(result_path, 'w') as results_file:
173+ yaml.safe_dump(results, results_file)
174+
175+
176+if __name__ == '__main__':
177+ parser = argparse.ArgumentParser(description='Fio Hardware Testing.')
178+ parser.add_argument(
179+ '--storage', dest='storage',
180+ help='path to storage device you want to test. e.g. /dev/sda')
181+ args = parser.parse_args()
182+ sys.exit(run_fio(args.storage))
183diff --git a/src/metadataserver/builtin_scripts/tests/test_fio.py b/src/metadataserver/builtin_scripts/tests/test_fio.py
184new file mode 100644
185index 0000000..1e39a9b
186--- /dev/null
187+++ b/src/metadataserver/builtin_scripts/tests/test_fio.py
188@@ -0,0 +1,137 @@
189+# Copyright 2017 Canonical Ltd. This software is licensed under the
190+# GNU Affero General Public License version 3 (see the file LICENSE).
191+
192+"""Test builtin_script fio."""
193+
194+__all__ = []
195+
196+from copy import deepcopy
197+import io
198+import os
199+import re
200+from subprocess import (
201+ PIPE,
202+ Popen,
203+ STDOUT,
204+)
205+from textwrap import dedent
206+from unittest.mock import ANY
207+
208+from maastesting.factory import factory
209+from maastesting.matchers import MockCalledOnceWith
210+from maastesting.testcase import MAASTestCase
211+from metadataserver.builtin_scripts import fio
212+import yaml
213+
214+
215+FIO_READ_OUTPUT = dedent("""
216+ ...
217+ Starting 1 process
218+ Jobs: 1 (f=1): [r] [100.0% done] [62135K/0K /s] [15.6K/0 iops]
219+ test: (groupid=0, jobs=1): err= 0: pid=31181: Fri May 9 15:38:57 2014
220+ read : io=1024.0MB, bw=62748KB/s, iops=15686 , runt= 16711msec
221+ ...
222+ """)
223+
224+FIO_WRITE_OUTPUT = dedent("""
225+ ...
226+ Starting 1 process
227+ Jobs: 1 (f=1): [w] [100.0% done] [0K/26326K /s] [0 /6581 iops]
228+ test: (groupid=0, jobs=1): err= 0: pid=31235: Fri May 9 16:16:21 2014
229+ write: io=1024.0MB, bw=29195KB/s, iops=7298 , runt= 35916msec
230+ ...
231+ """)
232+
233+
234+class TestFioTest(MAASTestCase):
235+
236+ def test_run_cmd_runs_cmd_and_returns_output(self):
237+ cmd = factory.make_string()
238+ output = factory.make_string()
239+ mock_popen = self.patch(fio, "Popen")
240+ mock_popen.return_value = Popen(
241+ ['echo', '-n', output], stdout=PIPE, stderr=PIPE)
242+
243+ cmd_output = fio.run_cmd(cmd)
244+
245+ self.assertEquals((output.encode(), 0), cmd_output)
246+ self.assertThat(mock_popen, MockCalledOnceWith(
247+ cmd, stdout=PIPE, stderr=STDOUT))
248+
249+ def test_run_cmd_runs_cmd_and_exits_on_error(self):
250+ cmd = factory.make_string()
251+ mock_popen = self.patch(fio, "Popen")
252+ proc = mock_popen.return_value
253+ proc.communicate.return_value = (b"Output", None)
254+ proc.returncode = 1
255+
256+ self.assertRaises(SystemExit, fio.run_cmd, cmd)
257+ self.assertThat(mock_popen, MockCalledOnceWith(
258+ cmd, stdout=PIPE, stderr=STDOUT))
259+
260+ def test_run_fio_test_runs_test(self):
261+ result_path = factory.make_string()
262+ readwrite = factory.make_string()
263+ cmd = deepcopy(fio.CMD)
264+ cmd.append('--readwrite=%s' % readwrite)
265+ returncode = 0
266+ mock_run_cmd = self.patch(fio, "run_cmd")
267+ mock_run_cmd.return_value = (
268+ FIO_READ_OUTPUT.encode('utf-8'), returncode)
269+ mock_re_search = self.patch(re, "search")
270+ fio.run_fio_test(readwrite, result_path)
271+
272+ self.assertThat(mock_run_cmd, MockCalledOnceWith(cmd))
273+ self.assertThat(mock_re_search, MockCalledOnceWith(
274+ fio.REGEX, FIO_READ_OUTPUT.encode('utf-8')))
275+
276+ def test_run_fio_test_exits_if_no_match_found(self):
277+ result_path = factory.make_string()
278+ readwrite = factory.make_string()
279+ cmd = deepcopy(fio.CMD)
280+ cmd.append('--readwrite=%s' % readwrite)
281+ returncode = 0
282+ mock_run_cmd = self.patch(fio, "run_cmd")
283+ mock_run_cmd.return_value = (
284+ FIO_WRITE_OUTPUT.encode('utf-8'), returncode)
285+ mock_re_search = self.patch(re, "search")
286+ mock_re_search.return_value = None
287+ fio.run_fio_test(readwrite, result_path)
288+
289+ self.assertThat(mock_run_cmd, MockCalledOnceWith(cmd))
290+ self.assertThat(mock_re_search, MockCalledOnceWith(
291+ fio.REGEX, FIO_WRITE_OUTPUT.encode('utf-8')))
292+
293+ def test_run_fio_writes_yaml_file(self):
294+ self.patch(os, 'environ', {
295+ "RESULT_PATH": factory.make_name()
296+ })
297+ disk = factory.make_name('disk')
298+ read_match = re.search(fio.REGEX, FIO_READ_OUTPUT.encode('utf-8'))
299+ write_match = re.search(fio.REGEX, FIO_WRITE_OUTPUT.encode('utf-8'))
300+ mock_run_fio_test = self.patch(fio, "run_fio_test")
301+ mock_run_fio_test.side_effect = [
302+ read_match, read_match, write_match, write_match]
303+ mock_open = self.patch(fio, "open")
304+ mock_open.return_value = io.StringIO()
305+ mock_yaml_safe_dump = self.patch(yaml, "safe_dump")
306+ # For the test, we will just use the same fio result for both
307+ # random and sequential.
308+ results = {
309+ 'status': "passed",
310+ 'results': {
311+ 'random_read': read_match.group(1).decode(),
312+ 'random_read_iops': read_match.group(2).decode(),
313+ 'sequential_read': read_match.group(1).decode(),
314+ 'sequential_read_iops': read_match.group(2).decode(),
315+ 'random_write': write_match.group(1).decode(),
316+ 'random_write_iops': write_match.group(2).decode(),
317+ 'sequential_write': write_match.group(1).decode(),
318+ 'sequential_write_iops': write_match.group(2).decode(),
319+ }
320+ }
321+ fio.run_fio(disk)
322+
323+ self.assertThat(mock_open, MockCalledOnceWith(ANY, "w"))
324+ self.assertThat(mock_yaml_safe_dump, MockCalledOnceWith(
325+ results, mock_open.return_value))

Subscribers

People subscribed via source and target branches