MAAS

Merge ~andreserl/maas:lp1732730 into maas:master

  1. Git
  2. lp:~andreserl/maas
  3. lp1732730
  4. Merge into master
Proposed by Andres Rodriguez
Status: Merged
Approved by: Andres Rodriguez
Approved revision: f8d4e0f989a78b806f39fd5d7c856b859bf275a4
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~andreserl/maas:lp1732730
Merge into: maas:master
Diff against target: 245 lines (+89/-39)
2 files modified
src/metadataserver/builtin_scripts/smartctl.py (+47/-23)
src/metadataserver/builtin_scripts/tests/test_smartctl.py (+42/-16)
Reviewer Review Type Date Requested Status
MAAS Lander Needs Fixing
Lee Trager (community) Approve
Review via email: mp+333839@code.launchpad.net

Commit message

LP: #1732730 - Fix smartctl test to not report SMART is unsupported or loop indefinitely when test fails.

To post a comment you must log in.
Revision history for this message
Lee Trager (ltrager) :
review: Needs Information
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1732730 lp:~andreserl/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/756/console
COMMIT: 527967e81893a3d04cead89f60636a5f74e07161

review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1732730 lp:~andreserl/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/758/console
COMMIT: c4eb1efe82b469cc3dd3419a45bab394c0fec36d

review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1732730 lp:~andreserl/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/763/console
COMMIT: 396073370e681909cebc85e5042940a87066465d

review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1732730 lp:~andreserl/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/764/console
COMMIT: 644268f4ac63713f9b942e7dbe60ce92c1983092

review: Needs Fixing
Revision history for this message
Lee Trager (ltrager) :
review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1732730 lp:~andreserl/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/765/console
COMMIT: 3268c40cf27ddb6ab086b366d8f08aecdbe5d71c

review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1732730 lp:~andreserl/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/766/console
COMMIT: 2144025908dcab351c6eb06a7d94ed18082d2df0

review: Needs Fixing
Revision history for this message
Lee Trager (ltrager) wrote :

Thanks for the fixes! LGTM!

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1732730 lp:~andreserl/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/767/console
COMMIT: 53af6cd8242a9e7171758e765103028baaafca9f

review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

LANDING
-b lp1732730 lp:~andreserl/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/771/consoleText

~andreserl/maas:lp1732730 updated
f8d4e0f... by Andres Rodriguez

Fix format

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/metadataserver/builtin_scripts/smartctl.py b/src/metadataserver/builtin_scripts/smartctl.py
2index f856eac..5cff6d8 100644
3--- a/src/metadataserver/builtin_scripts/smartctl.py
4+++ b/src/metadataserver/builtin_scripts/smartctl.py
5@@ -59,21 +59,33 @@ def check_SMART_support(storage):
6
7 If SMART support is not available, exit the script.
8 """
9- supported = True
10 smart_support_regex = re.compile('SMART support is:\s+Available')
11+ print(
12+ 'INFO: Veriying SMART support for the following drive: %s' % storage)
13 try:
14- output = check_output(
15- ['sudo', '-n', 'smartctl', '--all', storage], timeout=TIMEOUT)
16- except (TimeoutExpired, CalledProcessError):
17- supported = False
18- else:
19- match = smart_support_regex.search(output.decode('utf-8'))
20- if match is None:
21- supported = False
22+ cmd = ['sudo', '-n', 'smartctl', '--all', storage]
23+ print('INFO: Running command: %s\n' % ' '.join(cmd))
24+ output = check_output(cmd, timeout=TIMEOUT, stderr=STDOUT)
25+ except TimeoutExpired:
26+ print(
27+ "ERROR: Unable to determine if the drive supports SMART. "
28+ "Command timed out after %s seconds." % (storage, TIMEOUT))
29+ sys.exit(1)
30+ except CalledProcessError as e:
31+ output = e.output
32+ if output is None:
33+ print(
34+ "INFO: Unable to determine if the drive supports SMART. "
35+ "Command failed to run and did not return any output. ")
36+ sys.exit(1)
37
38- if not supported:
39- print('The following drive does not support SMART: %s\n' % storage)
40+ match = smart_support_regex.search(output.decode('utf-8'))
41+ if match is None:
42+ print(
43+ 'INFO: Unable to run test. The following drive '
44+ 'does not support SMART: %s\n' % storage)
45 sys.exit()
46+ print('INFO: SMART support is available; continuing...')
47
48
49 def run_smartctl_selftest(storage, test):
50@@ -81,27 +93,32 @@ def run_smartctl_selftest(storage, test):
51 try:
52 # Start testing.
53 cmd = ['sudo', '-n', 'smartctl', '-s', 'on', '-t', test, storage]
54- print('Running command: %s\n' % ' '.join(cmd))
55+ print('INFO: Running command: %s\n' % ' '.join(cmd))
56 check_call(cmd, timeout=TIMEOUT, stdout=DEVNULL, stderr=DEVNULL)
57 except (TimeoutExpired, CalledProcessError):
58- print('Failed to start and wait for smartctl self-test: %s' % test)
59+ print(
60+ 'ERROR: Failed to start and wait for smartctl '
61+ 'self-test: %s' % test)
62 return False
63 else:
64 # Wait for testing to complete.
65 status_regex = re.compile(
66- 'Self-test execution status:\s+\(\s*(?P<status>\d+)\s*\)')
67+ 'Self-test execution status:\s+\(\s*(?P<status>\d+)\s*\)'
68+ '\s+Self-test routine in progress')
69 while True:
70 try:
71 stdout = check_output(
72 ['sudo', '-n', 'smartctl', '-c', storage],
73 timeout=TIMEOUT)
74 except (TimeoutExpired, CalledProcessError):
75- print('Failed to start and wait for smartctl self-test:'
76+ print('ERROR: Failed to start and wait for smartctl self-test:'
77 ' %s' % test)
78 return False
79 else:
80 m = status_regex.search(stdout.decode('utf-8'))
81- if m is not None and int(m.group('status')) == 0:
82+ if m is None:
83+ # The test has finished running because we cannot find
84+ # regex match saying that's in-progress.
85 break
86 else:
87 # This is the time the test waits before checking for
88@@ -120,24 +137,31 @@ def run_smartctl(storage, test=None):
89 if test not in ('validate', None):
90 smartctl_passed = run_smartctl_selftest(storage, test)
91
92+ print('INFO: Verifying and/or validating SMART tests...')
93 cmd = ['sudo', '-n', 'smartctl', '--xall', storage]
94- print('Running command: %s\n' % ' '.join(cmd))
95+ print('INFO: Running command: %s\n' % ' '.join(cmd))
96 # Run smartctl and capture its output.
97 with Popen(cmd, stdout=PIPE, stderr=STDOUT) as proc:
98 try:
99 output, _ = proc.communicate(timeout=TIMEOUT)
100 except TimeoutExpired:
101 proc.kill()
102- print('Running `smartctl --xall %s` timed out!' % storage)
103+ print('INFO: Running `smartctl --xall %s` timed out!' % storage)
104 smartctl_passed = False
105 else:
106- if output is not None:
107- print(output.decode('utf-8'))
108 if proc.returncode != 0 and proc.returncode != 4:
109- print('Error, `smartctl --xall %s` returned %d!' % (
110- storage, proc.returncode))
111- print('See the smartctl man page for return code meaning')
112+ print('FAILURE: SMART tests have FAILED for: %s' % storage)
113+ print('The test exited with return code %s! See the smarctl '
114+ 'manpage for information on the return code meaning. '
115+ 'For more information on the test failures, review the '
116+ 'test output provided below.' % proc.returncode)
117 smartctl_passed = False
118+ else:
119+ print('SUCCESS: SMART tests have PASSED for: %s\n' % storage)
120+
121+ if output is not None:
122+ print('---------------------------------------------------\n')
123+ print(output.decode('utf-8'))
124 return 0 if proc.returncode == 4 else proc.returncode
125
126 return 0 if smartctl_passed else 1
127diff --git a/src/metadataserver/builtin_scripts/tests/test_smartctl.py b/src/metadataserver/builtin_scripts/tests/test_smartctl.py
128index 2ffee25..9781549 100644
129--- a/src/metadataserver/builtin_scripts/tests/test_smartctl.py
130+++ b/src/metadataserver/builtin_scripts/tests/test_smartctl.py
131@@ -10,6 +10,7 @@ from subprocess import (
132 DEVNULL,
133 PIPE,
134 Popen,
135+ STDOUT,
136 TimeoutExpired,
137 )
138 from textwrap import dedent
139@@ -52,6 +53,7 @@ class TestSmartCTL(MAASTestCase):
140
141 self.assertThat(mock_check_output, MockCalledOnceWith(
142 ['sudo', '-n', 'smartctl', '--all', storage],
143+ stderr=STDOUT,
144 timeout=smartctl.TIMEOUT))
145
146 def test_SMART_support_is_not_available(self):
147@@ -63,10 +65,16 @@ class TestSmartCTL(MAASTestCase):
148 self.assertRaises(SystemExit, smartctl.check_SMART_support, storage)
149 self.assertThat(mock_check_output, MockCalledOnceWith(
150 ['sudo', '-n', 'smartctl', '--all', storage],
151+ stderr=STDOUT,
152 timeout=smartctl.TIMEOUT))
153 self.assertThat(
154- mock_print, MockCalledOnceWith(
155- 'The following drive does not support SMART: %s\n' % storage))
156+ mock_print, MockCallsMatch(
157+ call('INFO: Veriying SMART support for the following drive: '
158+ '%s' % storage),
159+ call('INFO: Running command: sudo -n smartctl --all '
160+ '%s\n' % storage),
161+ call('INFO: Unable to determine if the drive supports SMART. '
162+ 'Command failed to run and did not return any output. ')))
163
164 def test_SMART_support_no_match_found(self):
165 storage = factory.make_name('storage')
166@@ -77,10 +85,16 @@ class TestSmartCTL(MAASTestCase):
167 self.assertRaises(SystemExit, smartctl.check_SMART_support, storage)
168 self.assertThat(mock_check_output, MockCalledOnceWith(
169 ['sudo', '-n', 'smartctl', '--all', storage],
170+ stderr=STDOUT,
171 timeout=smartctl.TIMEOUT))
172 self.assertThat(
173- mock_print, MockCalledOnceWith(
174- 'The following drive does not support SMART: %s\n' % storage))
175+ mock_print, MockCallsMatch(
176+ call('INFO: Veriying SMART support for the following '
177+ 'drive: %s' % storage),
178+ call('INFO: Running command: '
179+ 'sudo -n smartctl --all %s\n' % storage),
180+ call('INFO: Unable to run test. The following drive does '
181+ 'not support SMART: %s\n' % storage)))
182
183 def test_run_smartctl_selftest(self):
184 storage = factory.make_name('storage')
185@@ -107,10 +121,14 @@ class TestSmartCTL(MAASTestCase):
186 mock_check_call = self.patch(smartctl, "check_call")
187 mock_check_output = self.patch(smartctl, "check_output")
188 mock_check_output.side_effect = [
189- b'Self-test execution status: ( 249)',
190- b'Self-test execution status: ( 249)',
191- b'Self-test execution status: ( 249)',
192- b'Self-test execution status: ( 0)',
193+ b'Self-test execution status: ( 249) ' +
194+ b'Self-test routine in progress...',
195+ b'Self-test execution status: ( 249) ' +
196+ b'Self-test routine in progress...',
197+ b'Self-test execution status: ( 249) ' +
198+ b'Self-test routine in progress...',
199+ b'Self-test execution status: ( 73) ' +
200+ b'The previous self-test completed having',
201 ]
202
203 self.assertTrue(smartctl.run_smartctl_selftest(storage, test))
204@@ -207,7 +225,10 @@ class TestSmartCTL(MAASTestCase):
205
206 self.assertEquals(0, smartctl.run_smartctl(storage))
207 self.assertThat(mock_print, MockCallsMatch(
208- call('Running command: %s\n' % ' '.join(cmd)),
209+ call('INFO: Verifying and/or validating SMART tests...'),
210+ call('INFO: Running command: %s\n' % ' '.join(cmd)),
211+ call('SUCCESS: SMART tests have PASSED for: %s\n' % storage),
212+ call('---------------------------------------------------\n'),
213 call(output.decode('utf-8'))))
214
215 def test_run_smartctl_with_failure(self):
216@@ -224,11 +245,15 @@ class TestSmartCTL(MAASTestCase):
217
218 self.assertEquals(1, smartctl.run_smartctl(storage))
219 self.assertThat(mock_print, MockCallsMatch(
220- call('Running command: %s\n' % ' '.join(cmd)),
221- call(output.decode('utf-8')),
222- call('Error, `smartctl --xall %s` returned %d!' % (
223- storage, 1)),
224- call('See the smartctl man page for return code meaning')))
225+ call('INFO: Verifying and/or validating SMART tests...'),
226+ call('INFO: Running command: %s\n' % ' '.join(cmd)),
227+ call('FAILURE: SMART tests have FAILED for: %s' % storage),
228+ call('The test exited with return code %d! See the smarctl '
229+ 'manpage for information on the return code meaning. For '
230+ 'more information on the test failures, review the test '
231+ 'output provided below.' % 1),
232+ call('---------------------------------------------------\n'),
233+ call(output.decode('utf-8'))))
234
235 def test_run_smartctl_timedout(self):
236 smartctl.TIMEOUT = 1
237@@ -242,5 +267,6 @@ class TestSmartCTL(MAASTestCase):
238
239 self.assertEquals(1, smartctl.run_smartctl(storage))
240 self.assertThat(mock_print, MockCallsMatch(
241- call('Running command: %s\n' % ' '.join(cmd)),
242- call('Running `smartctl --xall %s` timed out!' % storage)))
243+ call('INFO: Verifying and/or validating SMART tests...'),
244+ call('INFO: Running command: %s\n' % ' '.join(cmd)),
245+ call('INFO: Running `smartctl --xall %s` timed out!' % storage)))

Subscribers

People subscribed via source and target branches