Skip to content

Commit 57c1414

Browse files
benle7benle7
committed
Resolve CR comments
1 parent b2b0c11 commit 57c1414

File tree

3 files changed

+83
-74
lines changed

3 files changed

+83
-74
lines changed

‎scripts/bmc_techsupport.py‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
88
The usage of this script is divided into two parts:
99
1. Triggering BMC debug log dump Redfish task
10-
* In this case the script, triggers a POST request to BMC to start collecting debug log dump.
10+
* In this case the script triggers a POST request to BMC to start collecting debug log dump.
1111
* In this script we will print the new task-id to the console output
1212
to collect the debug log dump once the task-id has finished.
1313
* This step is non-blocking, task-id is returned immediately.
@@ -16,7 +16,7 @@
1616
1717
2. Collecting BMC debug log dump
1818
* In this step we will wait for the task-id to finish if it has not finished.
19-
* Blocking action till we get the file or having ERROR or Timeout.
19+
* Blocking action until we get the file or encounter an ERROR or Timeout.
2020
* It is invoked with the parameter '--mode collect --task <task-id> --path <path>'
2121
E.g.: /usr/local/bin/bmc_techsupport.py --mode collect --path <path> --task <task-id>
2222
@@ -80,7 +80,7 @@ def extract_debug_dump_file(self, task_id, filepath):
8080
if not log_dump_dir or not log_dump_filename:
8181
raise Exception(f'Invalid given filepath: {filepath}')
8282
if not log_dump_filename.endswith('.tar.xz'):
83-
raise Exception(f'Invalid given filepath extension:, should be .tar.xz: {log_dump_filename}')
83+
raise Exception(f'Invalid given filepath extension, should be .tar.xz: {log_dump_filename}')
8484

8585
start_time = time.time()
8686
log.log_info("Collecting BMC debug log dump")
@@ -117,6 +117,9 @@ def main(mode, task_id, filepath):
117117
if mode == BMCDebugDumpExtractor.TRIGGER_MODE:
118118
extractor.trigger_debug_dump()
119119
elif mode == BMCDebugDumpExtractor.COLLECT_MODE:
120+
if not task_id or not filepath:
121+
log.log_error("Both --task and --path arguments are required for 'collect' mode")
122+
return
120123
extractor.extract_debug_dump_file(task_id, filepath)
121124

122125

‎scripts/generate_dump‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1952,12 +1952,10 @@ save_log_files() {
19521952

19531953
###############################################################################
19541954
# Check BMC presence
1955-
# Globals:
1956-
# TAR, TARFILE, DUMPDIR, BASE, TARDIR, TECHSUPPORT_TIME_INFO
19571955
# Arguments:
19581956
# None
19591957
# Returns:
1960-
# None
1958+
# 0 if BMC is supported, 1 otherwise
19611959
###############################################################################
19621960
is_bmc_supported() {
19631961
local platform=$(python3 -c "from sonic_py_common import device_info; print(device_info.get_platform())")
@@ -1971,8 +1969,6 @@ is_bmc_supported() {
19711969

19721970
###############################################################################
19731971
# Trigger BMC debug log dump task
1974-
# Globals:
1975-
# TAR, TARFILE, DUMPDIR, BASE, TARDIR, TECHSUPPORT_TIME_INFO
19761972
# Arguments:
19771973
# None
19781974
# Returns:
@@ -1986,15 +1982,15 @@ trigger_bmc_debug_log_dump() {
19861982
fi
19871983
# Trigger BMC redfish API to start BMC debug log dump task
19881984
local task_id=$(python3 /usr/local/bin/bmc_techsupport.py -m trigger)
1989-
echo $task_id
1985+
echo "$task_id"
19901986
}
19911987

19921988
###############################################################################
19931989
# Save BMC debug log dump files
19941990
# Globals:
1995-
# TAR, TARFILE, DUMPDIR, BASE, TARDIR, TECHSUPPORT_TIME_INFO
1991+
# MKDIR, CP, TARDIR, TECHSUPPORT_TIME_INFO
19961992
# Arguments:
1997-
# None
1993+
# $1 - BMC debug log dump task ID
19981994
# Returns:
19991995
# None
20001996
###############################################################################
@@ -2012,7 +2008,7 @@ collect_bmc_files() {
20122008
[ -f "$TARBALL_XZ" ] && rm -f "$TARBALL_XZ"
20132009

20142010
# Invoke BMC redfish API to extract BMC debug log dump to "/tmp/bmc_debug_log_dump.tar.xz"
2015-
python3 /usr/local/bin/bmc_techsupport.py -m collect -p "$TARBALL_XZ" -t $bmc_debug_log_dump_task_id
2011+
python3 /usr/local/bin/bmc_techsupport.py -m collect -p "$TARBALL_XZ" -t "$bmc_debug_log_dump_task_id"
20162012
if [ -f "$TARBALL_XZ" ]; then
20172013
$CP $V -rf "$TARBALL_XZ" $TARDIR/bmc
20182014
else

‎tests/show_platform_test.py‎

Lines changed: 72 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,6 @@ def test_bmc_summary_regular_output(self):
213213
)
214214

215215
mock_sonic_platform = mock.MagicMock()
216-
sys.modules['sonic_platform'] = mock_sonic_platform
217-
sys.modules['sonic_platform.platform'] = mock_sonic_platform.platform
218-
219216
mock_platform = mock.MagicMock()
220217
mock_chassis = mock.MagicMock()
221218
mock_bmc = mock.MagicMock()
@@ -226,9 +223,13 @@ def test_bmc_summary_regular_output(self):
226223
mock_bmc.get_version.return_value = self.TEST_BMC_VERSION
227224
mock_sonic_platform.platform.Platform.return_value = mock_platform
228225

229-
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['summary'], [])
230-
assert result.exit_code == 0, result.output
231-
assert result.output == textwrap.dedent(expected_output)
226+
with mock.patch.dict('sys.modules', {
227+
'sonic_platform': mock_sonic_platform,
228+
'sonic_platform.platform': mock_sonic_platform.platform
229+
}):
230+
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['summary'], [])
231+
assert result.exit_code == 0, result.output
232+
assert result.output == textwrap.dedent(expected_output)
232233

233234
def test_bmc_summary_json_output(self):
234235
"""Test 'show platform bmc summary' with JSON output"""
@@ -242,9 +243,6 @@ def test_bmc_summary_json_output(self):
242243
}
243244

244245
mock_sonic_platform = mock.MagicMock()
245-
sys.modules['sonic_platform'] = mock_sonic_platform
246-
sys.modules['sonic_platform.platform'] = mock_sonic_platform.platform
247-
248246
mock_platform = mock.MagicMock()
249247
mock_chassis = mock.MagicMock()
250248
mock_bmc = mock.MagicMock()
@@ -255,10 +253,14 @@ def test_bmc_summary_json_output(self):
255253
mock_bmc.get_version.return_value = self.TEST_BMC_VERSION
256254
mock_sonic_platform.platform.Platform.return_value = mock_platform
257255

258-
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['summary'], ['--json'])
259-
assert result.exit_code == 0, result.output
260-
output_json = json.loads(result.output)
261-
assert output_json == expected_json
256+
with mock.patch.dict('sys.modules', {
257+
'sonic_platform': mock_sonic_platform,
258+
'sonic_platform.platform': mock_sonic_platform.platform
259+
}):
260+
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['summary'], ['--json'])
261+
assert result.exit_code == 0, result.output
262+
output_json = json.loads(result.output)
263+
assert output_json == expected_json
262264

263265
def test_bmc_eeprom_regular_output(self):
264266
"""Test 'show platform bmc eeprom' with regular output"""
@@ -277,9 +279,6 @@ def test_bmc_eeprom_regular_output(self):
277279
)
278280

279281
mock_sonic_platform = mock.MagicMock()
280-
sys.modules['sonic_platform'] = mock_sonic_platform
281-
sys.modules['sonic_platform.platform'] = mock_sonic_platform.platform
282-
283282
mock_platform = mock.MagicMock()
284283
mock_chassis = mock.MagicMock()
285284
mock_bmc = mock.MagicMock()
@@ -289,9 +288,13 @@ def test_bmc_eeprom_regular_output(self):
289288
mock_bmc.get_eeprom.return_value = self.TEST_BMC_EEPROM_INFO
290289
mock_sonic_platform.platform.Platform.return_value = mock_platform
291290

292-
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['eeprom'], [])
293-
assert result.exit_code == 0, result.output
294-
assert result.output == textwrap.dedent(expected_output)
291+
with mock.patch.dict('sys.modules', {
292+
'sonic_platform': mock_sonic_platform,
293+
'sonic_platform.platform': mock_sonic_platform.platform
294+
}):
295+
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['eeprom'], [])
296+
assert result.exit_code == 0, result.output
297+
assert result.output == textwrap.dedent(expected_output)
295298

296299
def test_bmc_eeprom_json_output(self):
297300
"""Test 'show platform bmc eeprom' with JSON output"""
@@ -304,9 +307,6 @@ def test_bmc_eeprom_json_output(self):
304307
}
305308

306309
mock_sonic_platform = mock.MagicMock()
307-
sys.modules['sonic_platform'] = mock_sonic_platform
308-
sys.modules['sonic_platform.platform'] = mock_sonic_platform.platform
309-
310310
mock_platform = mock.MagicMock()
311311
mock_chassis = mock.MagicMock()
312312
mock_bmc = mock.MagicMock()
@@ -316,34 +316,36 @@ def test_bmc_eeprom_json_output(self):
316316
mock_bmc.get_eeprom.return_value = self.TEST_BMC_EEPROM_INFO
317317
mock_sonic_platform.platform.Platform.return_value = mock_platform
318318

319-
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['eeprom'], ['--json'])
320-
assert result.exit_code == 0, result.output
321-
output_json = json.loads(result.output)
322-
assert output_json == expected_json
319+
with mock.patch.dict('sys.modules', {
320+
'sonic_platform': mock_sonic_platform,
321+
'sonic_platform.platform': mock_sonic_platform.platform
322+
}):
323+
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['eeprom'], ['--json'])
324+
assert result.exit_code == 0, result.output
325+
output_json = json.loads(result.output)
326+
assert output_json == expected_json
323327

324328
def test_bmc_summary_bmc_not_available(self):
325329
"""Test 'show platform bmc summary' when BMC is not available"""
326330
mock_sonic_platform = mock.MagicMock()
327-
sys.modules['sonic_platform'] = mock_sonic_platform
328-
sys.modules['sonic_platform.platform'] = mock_sonic_platform.platform
329-
330331
mock_platform = mock.MagicMock()
331332
mock_chassis = mock.MagicMock()
332333

333334
mock_platform.get_chassis.return_value = mock_chassis
334335
mock_chassis.get_bmc.return_value = None
335336
mock_sonic_platform.platform.Platform.return_value = mock_platform
336337

337-
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['summary'], [])
338-
assert result.exit_code == 0, result.output
339-
assert "BMC is not available on this platform" in result.output
338+
with mock.patch.dict('sys.modules', {
339+
'sonic_platform': mock_sonic_platform,
340+
'sonic_platform.platform': mock_sonic_platform.platform
341+
}):
342+
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['summary'], [])
343+
assert result.exit_code == 0, result.output
344+
assert "BMC is not available on this platform" in result.output
340345

341346
def test_bmc_summary_eeprom_info_empty(self):
342347
"""Test 'show platform bmc summary' when EEPROM info is empty"""
343348
mock_sonic_platform = mock.MagicMock()
344-
sys.modules['sonic_platform'] = mock_sonic_platform
345-
sys.modules['sonic_platform.platform'] = mock_sonic_platform.platform
346-
347349
mock_platform = mock.MagicMock()
348350
mock_chassis = mock.MagicMock()
349351
mock_bmc = mock.MagicMock()
@@ -353,50 +355,53 @@ def test_bmc_summary_eeprom_info_empty(self):
353355
mock_bmc.get_eeprom.return_value = None
354356
mock_sonic_platform.platform.Platform.return_value = mock_platform
355357

356-
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['summary'], [])
357-
assert result.exit_code == 0, result.output
358-
assert "Failed to retrieve BMC EEPROM information" in result.output
358+
with mock.patch.dict('sys.modules', {
359+
'sonic_platform': mock_sonic_platform,
360+
'sonic_platform.platform': mock_sonic_platform.platform
361+
}):
362+
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['summary'], [])
363+
assert result.exit_code == 0, result.output
364+
assert "Failed to retrieve BMC EEPROM information" in result.output
359365

360366
def test_bmc_summary_exception(self):
361367
"""Test 'show platform bmc summary' when an exception occurs"""
362368
mock_sonic_platform = mock.MagicMock()
363-
sys.modules['sonic_platform'] = mock_sonic_platform
364-
sys.modules['sonic_platform.platform'] = mock_sonic_platform.platform
365-
366369
mock_platform = mock.MagicMock()
367370
mock_chassis = mock.MagicMock()
368371

369372
mock_platform.get_chassis.return_value = mock_chassis
370373
mock_chassis.get_bmc.side_effect = Exception("Test exception")
371374
mock_sonic_platform.platform.Platform.return_value = mock_platform
372375

373-
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['summary'], [])
374-
assert result.exit_code == 0, result.output
375-
assert "Error retrieving BMC information: Test exception" in result.output
376+
with mock.patch.dict('sys.modules', {
377+
'sonic_platform': mock_sonic_platform,
378+
'sonic_platform.platform': mock_sonic_platform.platform
379+
}):
380+
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['summary'], [])
381+
assert result.exit_code == 0, result.output
382+
assert "Error retrieving BMC information: Test exception" in result.output
376383

377384
def test_bmc_eeprom_bmc_not_available(self):
378385
"""Test 'show platform bmc eeprom' when BMC is not available"""
379386
mock_sonic_platform = mock.MagicMock()
380-
sys.modules['sonic_platform'] = mock_sonic_platform
381-
sys.modules['sonic_platform.platform'] = mock_sonic_platform.platform
382-
383387
mock_platform = mock.MagicMock()
384388
mock_chassis = mock.MagicMock()
385389

386390
mock_platform.get_chassis.return_value = mock_chassis
387391
mock_chassis.get_bmc.return_value = None
388392
mock_sonic_platform.platform.Platform.return_value = mock_platform
389393

390-
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['eeprom'], [])
391-
assert result.exit_code == 0, result.output
392-
assert "BMC is not available on this platform" in result.output
394+
with mock.patch.dict('sys.modules', {
395+
'sonic_platform': mock_sonic_platform,
396+
'sonic_platform.platform': mock_sonic_platform.platform
397+
}):
398+
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['eeprom'], [])
399+
assert result.exit_code == 0, result.output
400+
assert "BMC is not available on this platform" in result.output
393401

394402
def test_bmc_eeprom_info_empty(self):
395403
"""Test 'show platform bmc eeprom' when EEPROM info is empty"""
396404
mock_sonic_platform = mock.MagicMock()
397-
sys.modules['sonic_platform'] = mock_sonic_platform
398-
sys.modules['sonic_platform.platform'] = mock_sonic_platform.platform
399-
400405
mock_platform = mock.MagicMock()
401406
mock_chassis = mock.MagicMock()
402407
mock_bmc = mock.MagicMock()
@@ -406,23 +411,28 @@ def test_bmc_eeprom_info_empty(self):
406411
mock_bmc.get_eeprom.return_value = None
407412
mock_sonic_platform.platform.Platform.return_value = mock_platform
408413

409-
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['eeprom'], [])
410-
assert result.exit_code == 0, result.output
411-
assert "Failed to retrieve BMC EEPROM information" in result.output
414+
with mock.patch.dict('sys.modules', {
415+
'sonic_platform': mock_sonic_platform,
416+
'sonic_platform.platform': mock_sonic_platform.platform
417+
}):
418+
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['eeprom'], [])
419+
assert result.exit_code == 0, result.output
420+
assert "Failed to retrieve BMC EEPROM information" in result.output
412421

413422
def test_bmc_eeprom_exception(self):
414423
"""Test 'show platform bmc eeprom' when an exception occurs"""
415424
mock_sonic_platform = mock.MagicMock()
416-
sys.modules['sonic_platform'] = mock_sonic_platform
417-
sys.modules['sonic_platform.platform'] = mock_sonic_platform.platform
418-
419425
mock_platform = mock.MagicMock()
420426
mock_chassis = mock.MagicMock()
421427

422428
mock_platform.get_chassis.return_value = mock_chassis
423429
mock_chassis.get_bmc.side_effect = Exception("Test exception")
424430
mock_sonic_platform.platform.Platform.return_value = mock_platform
425431

426-
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['eeprom'], [])
427-
assert result.exit_code == 0, result.output
428-
assert "Error retrieving BMC EEPROM information: Test exception" in result.output
432+
with mock.patch.dict('sys.modules', {
433+
'sonic_platform': mock_sonic_platform,
434+
'sonic_platform.platform': mock_sonic_platform.platform
435+
}):
436+
result = CliRunner().invoke(show.cli.commands['platform'].commands['bmc'].commands['eeprom'], [])
437+
assert result.exit_code == 0, result.output
438+
assert "Error retrieving BMC EEPROM information: Test exception" in result.output

0 commit comments

Comments
 (0)