From 10ae93dd24881dd6741a75b349ab5dc3f96d19df Mon Sep 17 00:00:00 2001 From: BYGX-wcr Date: Wed, 5 Feb 2025 22:08:46 +0000 Subject: [PATCH 1/5] scripts/determine-reboot-cause: add an except branch to handle NotImplementedError that will be thrown on VS Chassis platform --- scripts/determine-reboot-cause | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/scripts/determine-reboot-cause b/scripts/determine-reboot-cause index 67722cfc..e173f5b0 100755 --- a/scripts/determine-reboot-cause +++ b/scripts/determine-reboot-cause @@ -115,6 +115,9 @@ def get_reboot_cause_from_platform(): except (ImportError, FileNotFoundError): sonic_logger.log_warning("sonic_platform package not installed. Unable to detect hardware reboot causes.") hardware_reboot_cause_major, hardware_reboot_cause_minor = REBOOT_CAUSE_NON_HARDWARE, "N/A" + except (NotImplementedError): + sonic_logger.log_warning("sonic_platform package not implemented. Unable to detect hardware reboot causes.") + hardware_reboot_cause_major, hardware_reboot_cause_minor = REBOOT_CAUSE_NON_HARDWARE, "N/A" return hardware_reboot_cause_major, hardware_reboot_cause_minor @@ -142,7 +145,7 @@ def get_reboot_cause_dict(previous_reboot_cause, comment, gen_time): If user issused a command to reboot device, then user, command and time will be stored into a dictionary. - If device was rebooted due to the kernel panic, then the string `Kernel Panic` + If device was rebooted due to the kernel panic, then the string `Kernel Panic` and time will be stored into a dictionary. """ reboot_cause_dict = {} @@ -164,7 +167,7 @@ def get_reboot_cause_dict(previous_reboot_cause, comment, gen_time): if match is not None: reboot_cause_dict['cause'] = "Kernel Panic" reboot_cause_dict['time'] = match.group(1) - + return reboot_cause_dict def determine_reboot_cause(): @@ -184,12 +187,12 @@ def determine_reboot_cause(): software_reboot_cause = find_software_reboot_cause() # The main decision logic of the reboot cause: - # If there is a valid hardware reboot cause indicated by platform API, + # If there is a valid hardware reboot cause indicated by platform API, # check the software reboot cause to add additional rebot cause. # If there is a reboot cause indicated by /proc/cmdline, and/or warmreboot/fastreboot/softreboot # the software_reboot_cause which is the content of /hosts/reboot-cause/reboot-cause.txt # will be treated as the additional reboot cause - # Elif there is a cmdline reboot cause, + # Elif there is a cmdline reboot cause, # the software_reboot_cause will be treated as the reboot cause if it's not unknown # otherwise, the cmdline_reboot_cause will be treated as the reboot cause if it's not none # Else the software_reboot_cause will be treated as the reboot cause @@ -244,7 +247,7 @@ def main(): # Write the previous reboot cause to REBOOT_CAUSE_HISTORY_FILE as a JSON format with open(REBOOT_CAUSE_HISTORY_FILE, "w") as reboot_cause_history_file: json.dump(reboot_cause_dict, reboot_cause_history_file) - + # Remove stale PREVIOUS_REBOOT_CAUSE_FILE if it exists if os.path.exists(PREVIOUS_REBOOT_CAUSE_FILE) or os.path.islink(PREVIOUS_REBOOT_CAUSE_FILE): os.remove(PREVIOUS_REBOOT_CAUSE_FILE) From a54b44802ae8c1ee39ca296ebe17efde061fcd96 Mon Sep 17 00:00:00 2001 From: BYGX-wcr Date: Wed, 5 Feb 2025 22:23:48 +0000 Subject: [PATCH 2/5] tests/determine-reboot-cause_test.py: supplement a new test case to improve coverage --- tests/determine-reboot-cause_test.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/determine-reboot-cause_test.py b/tests/determine-reboot-cause_test.py index 61ca4f2e..21bde8ed 100644 --- a/tests/determine-reboot-cause_test.py +++ b/tests/determine-reboot-cause_test.py @@ -125,6 +125,10 @@ def test_find_hardware_reboot_cause_with_minor(self): result = determine_reboot_cause.find_hardware_reboot_cause() assert result == "Powerloss (under-voltage)" + def test_find_hardware_reboot_cause_not_implemented(self): + result = determine_reboot_cause.find_hardware_reboot_cause() + assert result == REBOOT_CAUSE_NON_HARDWARE + " (N/A)" + def test_get_reboot_cause_dict_watchdog(self): reboot_cause_dict = determine_reboot_cause.get_reboot_cause_dict(REBOOT_CAUSE_WATCHDOG, "", GEN_TIME_WATCHDOG) assert reboot_cause_dict == EXPECTED_WATCHDOG_REBOOT_CAUSE_DICT @@ -205,4 +209,4 @@ def test_determine_reboot_cause_main_with_reboot_cause_dir(self): with mock.patch("os.geteuid", return_value=0): determine_reboot_cause.main() assert os.path.exists("host/reboot-cause/reboot-cause.txt") == True - assert os.path.exists("host/reboot-cause/previous-reboot-cause.json") == True + assert os.path.exists("host/reboot-cause/previous-reboot-cause.json") == True From 838af4a0024a63658483559bc6d3de440ed2c4ae Mon Sep 17 00:00:00 2001 From: BYGX-wcr Date: Mon, 10 Feb 2025 18:07:26 +0000 Subject: [PATCH 3/5] fix a typo --- scripts/determine-reboot-cause | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/determine-reboot-cause b/scripts/determine-reboot-cause index e9a50426..09558fc8 100755 --- a/scripts/determine-reboot-cause +++ b/scripts/determine-reboot-cause @@ -189,7 +189,7 @@ def determine_reboot_cause(): # The main decision logic of the reboot cause: # If there is a valid hardware reboot cause indicated by platform API, - # check the software reboot cause to add additional rebot cause. + # check the software reboot cause to add additional reboot cause. # If there is a reboot cause indicated by /proc/cmdline, and/or warmreboot/fastreboot/softreboot # the software_reboot_cause which is the content of /hosts/reboot-cause/reboot-cause.txt # will be treated as the additional reboot cause From 676c189139df7b0f7a0e23788130299d8747436c Mon Sep 17 00:00:00 2001 From: BYGX-wcr Date: Mon, 10 Feb 2025 19:45:08 +0000 Subject: [PATCH 4/5] make error message more specific --- scripts/determine-reboot-cause | 2 +- tests/determine-reboot-cause_test.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/determine-reboot-cause b/scripts/determine-reboot-cause index 09558fc8..019ba784 100755 --- a/scripts/determine-reboot-cause +++ b/scripts/determine-reboot-cause @@ -118,7 +118,7 @@ def get_reboot_cause_from_platform(): hardware_reboot_cause_major, hardware_reboot_cause_minor = REBOOT_CAUSE_NON_HARDWARE, "N/A" except (NotImplementedError): sonic_logger.log_warning("sonic_platform package not implemented. Unable to detect hardware reboot causes.") - hardware_reboot_cause_major, hardware_reboot_cause_minor = REBOOT_CAUSE_NON_HARDWARE, "N/A" + hardware_reboot_cause_major, hardware_reboot_cause_minor = REBOOT_CAUSE_NON_HARDWARE, "Platform pkg not implemented" return hardware_reboot_cause_major, hardware_reboot_cause_minor diff --git a/tests/determine-reboot-cause_test.py b/tests/determine-reboot-cause_test.py index e23b377f..aff29931 100644 --- a/tests/determine-reboot-cause_test.py +++ b/tests/determine-reboot-cause_test.py @@ -132,7 +132,7 @@ def test_find_hardware_reboot_cause_with_minor(self): def test_find_hardware_reboot_cause_not_implemented(self): result = determine_reboot_cause.find_hardware_reboot_cause() - assert result == REBOOT_CAUSE_NON_HARDWARE + " (N/A)" + assert result == REBOOT_CAUSE_NON_HARDWARE + " (Platform pkg not implemented)" def test_get_reboot_cause_dict_watchdog(self): reboot_cause_dict = determine_reboot_cause.get_reboot_cause_dict(REBOOT_CAUSE_WATCHDOG, "", GEN_TIME_WATCHDOG) @@ -214,7 +214,7 @@ def test_determine_reboot_cause_main_with_reboot_cause_dir(self): with mock.patch("os.geteuid", return_value=0): determine_reboot_cause.main() assert os.path.exists("host/reboot-cause/reboot-cause.txt") == True - assert os.path.exists("host/reboot-cause/previous-reboot-cause.json") == True + assert os.path.exists("host/reboot-cause/previous-reboot-cause.json") == True def create_mock_platform_json(self, dpus): """Helper function to create a mock platform.json file.""" From 143960de71940738df99915c248873e78258d05c Mon Sep 17 00:00:00 2001 From: BYGX-wcr Date: Mon, 10 Feb 2025 20:07:25 +0000 Subject: [PATCH 5/5] consolidate the case where sonic_platform is not implemented or installed --- scripts/determine-reboot-cause | 7 ++----- tests/determine-reboot-cause_test.py | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/scripts/determine-reboot-cause b/scripts/determine-reboot-cause index 019ba784..6ad58dab 100755 --- a/scripts/determine-reboot-cause +++ b/scripts/determine-reboot-cause @@ -113,12 +113,9 @@ def get_reboot_cause_from_platform(): chassis = platform.get_chassis() hardware_reboot_cause_major, hardware_reboot_cause_minor = chassis.get_reboot_cause() sonic_logger.log_info("Platform api returns reboot cause {}, {}".format(hardware_reboot_cause_major, hardware_reboot_cause_minor)) - except (ImportError, FileNotFoundError): - sonic_logger.log_warning("sonic_platform package not installed. Unable to detect hardware reboot causes.") + except (ImportError, FileNotFoundError, NotImplementedError): + sonic_logger.log_warning("sonic_platform package not installed or not implemented. Unable to detect hardware reboot causes.") hardware_reboot_cause_major, hardware_reboot_cause_minor = REBOOT_CAUSE_NON_HARDWARE, "N/A" - except (NotImplementedError): - sonic_logger.log_warning("sonic_platform package not implemented. Unable to detect hardware reboot causes.") - hardware_reboot_cause_major, hardware_reboot_cause_minor = REBOOT_CAUSE_NON_HARDWARE, "Platform pkg not implemented" return hardware_reboot_cause_major, hardware_reboot_cause_minor diff --git a/tests/determine-reboot-cause_test.py b/tests/determine-reboot-cause_test.py index aff29931..bab7798a 100644 --- a/tests/determine-reboot-cause_test.py +++ b/tests/determine-reboot-cause_test.py @@ -130,9 +130,9 @@ def test_find_hardware_reboot_cause_with_minor(self): result = determine_reboot_cause.find_hardware_reboot_cause() assert result == "Powerloss (under-voltage)" - def test_find_hardware_reboot_cause_not_implemented(self): + def test_find_hardware_reboot_cause_not_installed_or_not_implemented(self): result = determine_reboot_cause.find_hardware_reboot_cause() - assert result == REBOOT_CAUSE_NON_HARDWARE + " (Platform pkg not implemented)" + assert result == REBOOT_CAUSE_NON_HARDWARE + " (N/A)" def test_get_reboot_cause_dict_watchdog(self): reboot_cause_dict = determine_reboot_cause.get_reboot_cause_dict(REBOOT_CAUSE_WATCHDOG, "", GEN_TIME_WATCHDOG)