MAAS

Merge ~ltrager/maas:lp1733923_2.3 into maas:2.3

  1. Git
  2. lp:~ltrager/maas
  3. lp1733923_2.3
  4. Merge into 2.3
Proposed by Lee Trager
Status: Merged
Approved by: Lee Trager
Approved revision: 0e2b5655f741736b26850a336a6f9d331fc48ac9
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~ltrager/maas:lp1733923_2.3
Merge into: maas:2.3
Diff against target: 201 lines (+62/-50)
2 files modified
src/metadataserver/builtin_scripts/badblocks.py (+49/-18)
src/metadataserver/builtin_scripts/tests/test_badblocks.py (+13/-32)
Reviewer Review Type Date Requested Status
Lee Trager (community) Approve
MAAS Lander unittests Pending
Andres Rodriguez Pending
Review via email: mp+334324@code.launchpad.net

Commit message

Backport a03af41 from master - LP: #1733923 - Fail badblocks if any error is detected

To post a comment you must log in.
Revision history for this message
Lee Trager (ltrager) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/src/metadataserver/builtin_scripts/badblocks.py b/src/metadataserver/builtin_scripts/badblocks.py
index 6dfeca9..292b7fd 100644
--- a/src/metadataserver/builtin_scripts/badblocks.py
+++ b/src/metadataserver/builtin_scripts/badblocks.py
@@ -31,6 +31,18 @@
31# badblocks:31# badblocks:
32# title: Bad blocks32# title: Bad blocks
33# description: The number of bad blocks found on the storage device.33# description: The number of bad blocks found on the storage device.
34# read_errors:
35# title: Bad blocks read errors
36# description: >
37# The number of bad blocks read errors found on the storage device.
38# write_errors:
39# title: Bad blocks write errors
40# description: >
41# The number of bad blocks write errors found on the storage device.
42# comparison_errors:
43# title: Bad blocks comparison errors
44# description: >
45# The number of bad blocks comparison errors found on the storage device.
34# parameters:46# parameters:
35# storage: {type: storage}47# storage: {type: storage}
36# destructive: {{if 'destructive' in name}}True{{else}}False{{endif}}48# destructive: {{if 'destructive' in name}}True{{else}}False{{endif}}
@@ -113,37 +125,56 @@ def run_badblocks(storage, destructive=False):
113 if destructive:125 if destructive:
114 cmd = [126 cmd = [
115 'sudo', '-n', 'badblocks', '-b', str(blocksize),127 'sudo', '-n', 'badblocks', '-b', str(blocksize),
116 '-c', str(parallel_blocks), '-v', '-f', '-w', storage128 '-c', str(parallel_blocks), '-v', '-f', '-s', '-w', storage
117 ]129 ]
118 else:130 else:
119 cmd = [131 cmd = [
120 'sudo', '-n', 'badblocks', '-b', str(blocksize),132 'sudo', '-n', 'badblocks', '-b', str(blocksize),
121 '-c', str(parallel_blocks), '-v', '-f', '-n', storage133 '-c', str(parallel_blocks), '-v', '-f', '-s', '-n', storage
122 ]134 ]
123135
124 print('INFO: Running command: %s\n' % ' '.join(cmd))136 print('INFO: Running command: %s\n' % ' '.join(cmd))
125 proc = Popen(cmd, stdout=PIPE, stderr=STDOUT)137 proc = Popen(cmd, stdout=PIPE, stderr=STDOUT)
126 stdout, _ = proc.communicate()138 stdout, _ = proc.communicate()
139 stdout = stdout.decode()
127140
128 # Print stdout to the console.141 # Print stdout to the console.
129 if stdout is not None:142 print(stdout)
130 print(stdout.decode())143
131144 m = re.search(
132 result_path = os.environ.get("RESULT_PATH")145 '^Pass completed, (?P<badblocks>\d+) bad blocks found. '
146 '\((?P<read>\d+)\/(?P<write>\d+)\/(?P<comparison>\d+) errors\)$',
147 stdout, re.M)
148 badblocks = int(m.group('badblocks'))
149 read_errors = int(m.group('read'))
150 write_errors = int(m.group('write'))
151 comparison_errors = int(m.group('comparison'))
152 result_path = os.environ.get('RESULT_PATH')
133 if result_path is not None:153 if result_path is not None:
134 # Parse the results for the desired information and154 results = {
135 # then wrtie this to the results file.155 'results': {
136 match = re.search(b', ([0-9]+) bad blocks found', stdout)156 'badblocks': badblocks,
137 if match is not None:157 'read_errors': read_errors,
138 results = {158 'write_errors': write_errors,
139 'results': {159 'comparison_errors': comparison_errors,
140 'badblocks': int(match.group(1).decode()),
141 }
142 }160 }
143 with open(result_path, 'w') as results_file:161 }
144 yaml.safe_dump(results, results_file)162 with open(result_path, 'w') as results_file:
145163 yaml.safe_dump(results, results_file)
146 return proc.returncode164
165 # LP: #1733923 - Badblocks normally returns 0 no matter the result. If any
166 # errors are found fail the test.
167 if (proc.returncode + badblocks + read_errors + write_errors +
168 comparison_errors) != 0:
169 print('FAILURE: Test FAILED!')
170 print('INFO: %s badblocks found' % badblocks)
171 print('INFO: %s read errors found' % read_errors)
172 print('INFO: %s write errors found' % write_errors)
173 print('INFO: %s comparison errors found' % comparison_errors)
174 return 1
175 else:
176 print('SUCCESS: Test PASSED!')
177 return 0
147178
148179
149if __name__ == '__main__':180if __name__ == '__main__':
diff --git a/src/metadataserver/builtin_scripts/tests/test_badblocks.py b/src/metadataserver/builtin_scripts/tests/test_badblocks.py
index a04c33c..66f6b25 100644
--- a/src/metadataserver/builtin_scripts/tests/test_badblocks.py
+++ b/src/metadataserver/builtin_scripts/tests/test_badblocks.py
@@ -9,7 +9,6 @@ __all__ = []
9import io9import io
10import os10import os
11import random11import random
12import re
13from subprocess import (12from subprocess import (
14 PIPE,13 PIPE,
15 STDOUT,14 STDOUT,
@@ -18,22 +17,22 @@ from textwrap import dedent
18from unittest.mock import ANY17from unittest.mock import ANY
1918
20from maastesting.factory import factory19from maastesting.factory import factory
21from maastesting.matchers import (20from maastesting.matchers import MockCalledOnceWith
22 MockCalledOnce,
23 MockCalledOnceWith,
24)
25from maastesting.testcase import MAASTestCase21from maastesting.testcase import MAASTestCase
26from metadataserver.builtin_scripts import badblocks22from metadataserver.builtin_scripts import badblocks
27import yaml23import yaml
2824
2925
30BADBLOCKS = random.randint(0, 1000)26BADBLOCKS = random.randint(0, 1000)
27READ_ERRORS = random.randint(0, 1000)
28WRITE_ERRORS = random.randint(0, 1000)
29COMPARISON_ERRORS = random.randint(0, 1000)
31BADBLOCKS_OUTPUT = dedent("""30BADBLOCKS_OUTPUT = dedent("""
32 Checking for bad blocks in non-destructive read-write mode31 Checking for bad blocks in non-destructive read-write mode
33 From block 0 to 524287932 From block 0 to 5242879
34 Testing with random pattern:33 Testing with random pattern:
35 Pass completed, %s bad blocks found. (0/0/%s errors)34 Pass completed, %s bad blocks found. (%s/%s/%s errors)
36 """ % (BADBLOCKS, BADBLOCKS))35 """ % (BADBLOCKS, READ_ERRORS, WRITE_ERRORS, COMPARISON_ERRORS))
3736
3837
39class TestRunBadBlocks(MAASTestCase):38class TestRunBadBlocks(MAASTestCase):
@@ -75,7 +74,7 @@ class TestRunBadBlocks(MAASTestCase):
75 })74 })
76 cmd = [75 cmd = [
77 'sudo', '-n', 'badblocks', '-b', str(blocksize),76 'sudo', '-n', 'badblocks', '-b', str(blocksize),
78 '-c', str(parallel_blocks), '-v', '-f', '-n', storage77 '-c', str(parallel_blocks), '-v', '-f', '-s', '-n', storage
79 ]78 ]
80 mock_popen = self.patch(badblocks, "Popen")79 mock_popen = self.patch(badblocks, "Popen")
81 proc = mock_popen.return_value80 proc = mock_popen.return_value
@@ -88,10 +87,13 @@ class TestRunBadBlocks(MAASTestCase):
88 results = {87 results = {
89 'results': {88 'results': {
90 'badblocks': BADBLOCKS,89 'badblocks': BADBLOCKS,
90 'read_errors': READ_ERRORS,
91 'write_errors': WRITE_ERRORS,
92 'comparison_errors': COMPARISON_ERRORS,
91 }93 }
92 }94 }
9395
94 self.assertEquals(0, badblocks.run_badblocks(storage))96 self.assertEquals(1, badblocks.run_badblocks(storage))
95 self.assertThat(mock_popen, MockCalledOnceWith(97 self.assertThat(mock_popen, MockCalledOnceWith(
96 cmd, stdout=PIPE, stderr=STDOUT))98 cmd, stdout=PIPE, stderr=STDOUT))
97 self.assertThat(mock_open, MockCalledOnceWith(ANY, "w"))99 self.assertThat(mock_open, MockCalledOnceWith(ANY, "w"))
@@ -107,7 +109,7 @@ class TestRunBadBlocks(MAASTestCase):
107 parallel_blocks)109 parallel_blocks)
108 cmd = [110 cmd = [
109 'sudo', '-n', 'badblocks', '-b', str(blocksize),111 'sudo', '-n', 'badblocks', '-b', str(blocksize),
110 '-c', str(parallel_blocks), '-v', '-f', '-w', storage,112 '-c', str(parallel_blocks), '-v', '-f', '-s', '-w', storage,
111 ]113 ]
112 mock_popen = self.patch(badblocks, "Popen")114 mock_popen = self.patch(badblocks, "Popen")
113 proc = mock_popen.return_value115 proc = mock_popen.return_value
@@ -116,27 +118,6 @@ class TestRunBadBlocks(MAASTestCase):
116 proc.returncode = 0118 proc.returncode = 0
117119
118 self.assertEquals(120 self.assertEquals(
119 0, badblocks.run_badblocks(storage, destructive=True))121 1, badblocks.run_badblocks(storage, destructive=True))
120 self.assertThat(mock_popen, MockCalledOnceWith(122 self.assertThat(mock_popen, MockCalledOnceWith(
121 cmd, stdout=PIPE, stderr=STDOUT))123 cmd, stdout=PIPE, stderr=STDOUT))
122
123 def test_run_badblocks_exits_if_no_regex_match_found(self):
124 storage = factory.make_name('storage')
125 blocksize = random.randint(512, 4096)
126 self.patch(badblocks, 'get_block_size').return_value = blocksize
127 parallel_blocks = random.randint(1, 50000)
128 self.patch(badblocks, 'get_parallel_blocks').return_value = (
129 parallel_blocks)
130 self.patch(os, "environ", {
131 "RESULT_PATH": factory.make_name()
132 })
133 mock_popen = self.patch(badblocks, "Popen")
134 proc = mock_popen.return_value
135 proc.communicate.return_value = (
136 BADBLOCKS_OUTPUT.encode('utf-8'), None)
137 proc.returncode = 0
138 mock_re_search = self.patch(re, "search")
139 mock_re_search.return_value = None
140
141 self.assertEquals(0, badblocks.run_badblocks(storage))
142 self.assertThat(mock_re_search, MockCalledOnce())

Subscribers

People subscribed via source and target branches