From 62c0ae9202393f73d4de7dac47e0c07e1843d461 Mon Sep 17 00:00:00 2001 From: Yong Zhao Date: Tue, 9 Mar 2021 16:48:24 -0800 Subject: [PATCH 01/10] [reboot] Write the reboot reason into `reboot-cause.txt` if kernel was panicked. Signed-off-by: Yong Zhao --- scripts/reboot | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/reboot b/scripts/reboot index 8b1f9d60c3..5a89066948 100755 --- a/scripts/reboot +++ b/scripts/reboot @@ -9,7 +9,7 @@ REBOOT_TIME=$(date) VMCORE_FILE=/proc/vmcore if [ -e $VMCORE_FILE -a -s $VMCORE_FILE ]; then echo "We have a /proc/vmcore, then we just kdump'ed" - echo "User issued 'kdump' command [User: kdump, Time: ${REBOOT_TIME}]" > ${REBOOT_CAUSE_FILE} + echo "Kernel Panic (Time: ${REBOOT_TIME})" > ${REBOOT_CAUSE_FILE} sync PLATFORM=$(grep -oP 'sonic_platform=\K\S+' /proc/cmdline) if [ ! -z "${PLATFORM}" -a -x ${DEVPATH}/${PLATFORM}/${PLAT_REBOOT} ]; then From f160eb464dfe8f1be8dd467af72cea193d96a6d0 Mon Sep 17 00:00:00 2001 From: Yong Zhao Date: Thu, 25 Mar 2021 17:14:08 +0000 Subject: [PATCH 02/10] [reboot] Show the reason and time of reboot if kernel was panicked. Signed-off-by: Yong Zhao --- show/reboot_cause.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/show/reboot_cause.py b/show/reboot_cause.py index d1424d8676..09fb4dec4f 100755 --- a/show/reboot_cause.py +++ b/show/reboot_cause.py @@ -30,7 +30,10 @@ def reboot_cause(ctx): # Read the previous reboot cause data = read_reboot_cause_file() if data['user'] == "N/A": - reboot_cause = "{}".format(data['cause']) + if data['cause'] == "kernel Panic": + reboot_cause = "{} (Time: {})".format(data['cause'], data['time']) + else: + reboot_cause = "{}".format(data['cause']) else: reboot_cause = USER_ISSUED_REBOOT_CAUSE_REGEX.format(data['cause'], data['user'], data['time']) From e2f904de5ff8d4abca46b776ed1e2cf9294f109e Mon Sep 17 00:00:00 2001 From: Yong Zhao Date: Thu, 25 Mar 2021 19:47:32 +0000 Subject: [PATCH 03/10] [reboot] Change the "kernel" ---> "Kernel". Signed-off-by: Yong Zhao --- show/reboot_cause.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/show/reboot_cause.py b/show/reboot_cause.py index 09fb4dec4f..d2f81d8038 100755 --- a/show/reboot_cause.py +++ b/show/reboot_cause.py @@ -30,7 +30,7 @@ def reboot_cause(ctx): # Read the previous reboot cause data = read_reboot_cause_file() if data['user'] == "N/A": - if data['cause'] == "kernel Panic": + if data['cause'] == "Kernel Panic": reboot_cause = "{} (Time: {})".format(data['cause'], data['time']) else: reboot_cause = "{}".format(data['cause']) From 2c64b897927e315bef330316abb5754984a0911a Mon Sep 17 00:00:00 2001 From: Yong Zhao Date: Fri, 26 Mar 2021 00:53:44 +0000 Subject: [PATCH 04/10] [reboot] Reorganize the function `reboot_cause()` and `read_reboot_cause_file()`. Signed-off-by: Yong Zhao --- show/reboot_cause.py | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/show/reboot_cause.py b/show/reboot_cause.py index d2f81d8038..573f94a600 100755 --- a/show/reboot_cause.py +++ b/show/reboot_cause.py @@ -9,14 +9,18 @@ PREVIOUS_REBOOT_CAUSE_FILE = "/host/reboot-cause/previous-reboot-cause.json" -USER_ISSUED_REBOOT_CAUSE_REGEX ="User issued \'{}\' command [User: {}, Time: {}]" def read_reboot_cause_file(): - result = "" + reboot_cause = None + if os.path.exists(PREVIOUS_REBOOT_CAUSE_FILE): - with open(PREVIOUS_REBOOT_CAUSE_FILE) as f: - result = json.load(f) - return result + with open(PREVIOUS_REBOOT_CAUSE_FILE) as file_handler: + try: + reboot_cause = json.load(file_handler) + except json.JSONDecodeError as err: + click.echo("Failed to load JSON file '{}'!".format(PREVIOUS_REBOOT_CAUSE_FILE)) + + return reboot_cause # # 'reboot-cause' group ("show reboot-cause") @@ -26,18 +30,21 @@ def read_reboot_cause_file(): def reboot_cause(ctx): """Show cause of most recent reboot""" if ctx.invoked_subcommand is None: - reboot_cause = "" - # Read the previous reboot cause - data = read_reboot_cause_file() - if data['user'] == "N/A": - if data['cause'] == "Kernel Panic": - reboot_cause = "{} (Time: {})".format(data['cause'], data['time']) - else: - reboot_cause = "{}".format(data['cause']) - else: - reboot_cause = USER_ISSUED_REBOOT_CAUSE_REGEX.format(data['cause'], data['user'], data['time']) + reboot_cause_str = "" + + # Read the previous reboot reason + reboot_cause = read_reboot_cause_file() + + if reboot_cause and "cause" in reboot_cause.keys(): + reboot_cause_str = "Cause: {}".format(reboot_cause["cause"]) + + if "user" in reboot_cause.keys() and reboot_cause["user"] != "N/A": + reboot_cause_str += ", User: {}".format(reboot_cause["user"]) + + if "time" in reboot_cause.keys() and reboot_cause["time"] != "N/A": + reboot_cause_str += ", Time: {}".format(reboot_cause["time"]) - click.echo(reboot_cause) + click.echo(reboot_cause_str) # 'history' subcommand ("show reboot-cause history") @reboot_cause.command() From e4bf6b094c06e533bf258b836f99dd25ca454762 Mon Sep 17 00:00:00 2001 From: Yong Zhao Date: Fri, 26 Mar 2021 00:58:36 +0000 Subject: [PATCH 05/10] [reboot] Remove the extra blank lines. Signed-off-by: Yong Zhao --- show/reboot_cause.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/show/reboot_cause.py b/show/reboot_cause.py index 573f94a600..0c808ce369 100755 --- a/show/reboot_cause.py +++ b/show/reboot_cause.py @@ -32,15 +32,13 @@ def reboot_cause(ctx): if ctx.invoked_subcommand is None: reboot_cause_str = "" - # Read the previous reboot reason + # Read the previous reboot cause reboot_cause = read_reboot_cause_file() if reboot_cause and "cause" in reboot_cause.keys(): reboot_cause_str = "Cause: {}".format(reboot_cause["cause"]) - if "user" in reboot_cause.keys() and reboot_cause["user"] != "N/A": reboot_cause_str += ", User: {}".format(reboot_cause["user"]) - if "time" in reboot_cause.keys() and reboot_cause["time"] != "N/A": reboot_cause_str += ", Time: {}".format(reboot_cause["time"]) From 595898530d83eb347a12a8209c94826d610b91f6 Mon Sep 17 00:00:00 2001 From: Yong Zhao Date: Fri, 26 Mar 2021 18:11:05 +0000 Subject: [PATCH 06/10] [reboot] Reorganize the function `reboot_cause()` to follow the initial format of message. Signed-off-by: Yong Zhao --- show/reboot_cause.py | 45 +++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/show/reboot_cause.py b/show/reboot_cause.py index 0c808ce369..ae5af682c2 100755 --- a/show/reboot_cause.py +++ b/show/reboot_cause.py @@ -8,19 +8,19 @@ import utilities_common.cli as clicommon -PREVIOUS_REBOOT_CAUSE_FILE = "/host/reboot-cause/previous-reboot-cause.json" +PREVIOUS_REBOOT_CAUSE_FILE_PATH = "/host/reboot-cause/previous-reboot-cause.json" def read_reboot_cause_file(): - reboot_cause = None + reboot_cause_dict = {} - if os.path.exists(PREVIOUS_REBOOT_CAUSE_FILE): - with open(PREVIOUS_REBOOT_CAUSE_FILE) as file_handler: + if os.path.exists(PREVIOUS_REBOOT_CAUSE_FILE_PATH): + with open(PREVIOUS_REBOOT_CAUSE_FILE_PATH) as prev_reboot_cause_file: try: - reboot_cause = json.load(file_handler) + reboot_cause_dict = json.load(prev_reboot_cause_file) except json.JSONDecodeError as err: - click.echo("Failed to load JSON file '{}'!".format(PREVIOUS_REBOOT_CAUSE_FILE)) + click.echo("Failed to load JSON file '{}'!".format(PREVIOUS_REBOOT_CAUSE_FILE_PATH)) - return reboot_cause + return reboot_cause_dict # # 'reboot-cause' group ("show reboot-cause") @@ -33,16 +33,31 @@ def reboot_cause(ctx): reboot_cause_str = "" # Read the previous reboot cause - reboot_cause = read_reboot_cause_file() + reboot_cause_dict = read_reboot_cause_file() + + reboot_cause = reboot_cause_dict.get("cause", "Unknown") + reboot_user = reboot_cause_dict.get("user", "N/A") + reboot_time = reboot_cause_dict.get("time", "N/A") - if reboot_cause and "cause" in reboot_cause.keys(): - reboot_cause_str = "Cause: {}".format(reboot_cause["cause"]) - if "user" in reboot_cause.keys() and reboot_cause["user"] != "N/A": - reboot_cause_str += ", User: {}".format(reboot_cause["user"]) - if "time" in reboot_cause.keys() and reboot_cause["time"] != "N/A": - reboot_cause_str += ", Time: {}".format(reboot_cause["time"]) + if reboot_user != "N/A": + reboot_cause_str = "User issued '{}' command".format(reboot_cause) + else: + reboot_cuase_str = reboot_cause - click.echo(reboot_cause_str) + if reboot_user != "N/A" or reboot_time != "N/A": + reboot_cause_str += "[" + + if reboot_user != "N/A": + reboot_cause_str += "User: {}".format(reboot_user) + if reboot_time != "N/A": + reboot_cause_str += ", " + + if reboot_time != "N/A": + reboot_cause_str += "Time: {}".format(reboot_time) + + reboot_cause_str += "]" + + click.echo(reboot_cause_str) # 'history' subcommand ("show reboot-cause history") @reboot_cause.command() From e72e23c2274cc6db0bb6b6678bc59d3ec6cbdcf6 Mon Sep 17 00:00:00 2001 From: Yong Zhao Date: Fri, 26 Mar 2021 18:26:11 +0000 Subject: [PATCH 07/10] [reboot] Fix a typo and remove whitespaces. Signed-off-by: Yong Zhao --- show/reboot_cause.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/show/reboot_cause.py b/show/reboot_cause.py index ae5af682c2..e53698f869 100755 --- a/show/reboot_cause.py +++ b/show/reboot_cause.py @@ -34,7 +34,7 @@ def reboot_cause(ctx): # Read the previous reboot cause reboot_cause_dict = read_reboot_cause_file() - + reboot_cause = reboot_cause_dict.get("cause", "Unknown") reboot_user = reboot_cause_dict.get("user", "N/A") reboot_time = reboot_cause_dict.get("time", "N/A") @@ -42,7 +42,7 @@ def reboot_cause(ctx): if reboot_user != "N/A": reboot_cause_str = "User issued '{}' command".format(reboot_cause) else: - reboot_cuase_str = reboot_cause + reboot_cause_str = reboot_cause if reboot_user != "N/A" or reboot_time != "N/A": reboot_cause_str += "[" @@ -56,7 +56,7 @@ def reboot_cause(ctx): reboot_cause_str += "Time: {}".format(reboot_time) reboot_cause_str += "]" - + click.echo(reboot_cause_str) # 'history' subcommand ("show reboot-cause history") @@ -77,7 +77,7 @@ def history(): for tk in table_keys: entry = db.get_all(db.STATE_DB, tk) r = [] - r.append(tk.replace(prefix,"")) + r.append(tk.replace(prefix, "")) r.append(entry['cause'] if 'cause' in entry else "") r.append(entry['time'] if 'time' in entry else "") r.append(entry['user'] if 'user' in entry else "") From b53f5c447f07e256434a587befbe8d3078222244 Mon Sep 17 00:00:00 2001 From: Yong Zhao Date: Fri, 26 Mar 2021 18:30:38 +0000 Subject: [PATCH 08/10] [reboot] Use the '[]' to replace '()' in the message of kernel panic. Signed-off-by: Yong Zhao --- scripts/reboot | 2 +- show/reboot_cause.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/reboot b/scripts/reboot index 5a89066948..e79493c04c 100755 --- a/scripts/reboot +++ b/scripts/reboot @@ -9,7 +9,7 @@ REBOOT_TIME=$(date) VMCORE_FILE=/proc/vmcore if [ -e $VMCORE_FILE -a -s $VMCORE_FILE ]; then echo "We have a /proc/vmcore, then we just kdump'ed" - echo "Kernel Panic (Time: ${REBOOT_TIME})" > ${REBOOT_CAUSE_FILE} + echo "Kernel Panic [Time: ${REBOOT_TIME}]" > ${REBOOT_CAUSE_FILE} sync PLATFORM=$(grep -oP 'sonic_platform=\K\S+' /proc/cmdline) if [ ! -z "${PLATFORM}" -a -x ${DEVPATH}/${PLATFORM}/${PLAT_REBOOT} ]; then diff --git a/show/reboot_cause.py b/show/reboot_cause.py index e53698f869..2b34c88343 100755 --- a/show/reboot_cause.py +++ b/show/reboot_cause.py @@ -45,7 +45,7 @@ def reboot_cause(ctx): reboot_cause_str = reboot_cause if reboot_user != "N/A" or reboot_time != "N/A": - reboot_cause_str += "[" + reboot_cause_str += " [" if reboot_user != "N/A": reboot_cause_str += "User: {}".format(reboot_user) From 3253dc62e09fdb257ccd6e03034763c5d4b5643a Mon Sep 17 00:00:00 2001 From: Yong Zhao Date: Fri, 26 Mar 2021 22:34:14 +0000 Subject: [PATCH 09/10] [reboot] Update the unit test and send the error message to `stderr` when previous reboot file can not be read successfully. Signed-off-by: Yong Zhao --- show/reboot_cause.py | 5 ++++- tests/reboot_cause_test.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/show/reboot_cause.py b/show/reboot_cause.py index 2b34c88343..57bd15e863 100755 --- a/show/reboot_cause.py +++ b/show/reboot_cause.py @@ -10,6 +10,7 @@ PREVIOUS_REBOOT_CAUSE_FILE_PATH = "/host/reboot-cause/previous-reboot-cause.json" + def read_reboot_cause_file(): reboot_cause_dict = {} @@ -18,10 +19,11 @@ def read_reboot_cause_file(): try: reboot_cause_dict = json.load(prev_reboot_cause_file) except json.JSONDecodeError as err: - click.echo("Failed to load JSON file '{}'!".format(PREVIOUS_REBOOT_CAUSE_FILE_PATH)) + click.echo("Failed to load JSON file '{}'!".format(PREVIOUS_REBOOT_CAUSE_FILE_PATH), err=True) return reboot_cause_dict + # # 'reboot-cause' group ("show reboot-cause") # @@ -59,6 +61,7 @@ def reboot_cause(ctx): click.echo(reboot_cause_str) + # 'history' subcommand ("show reboot-cause history") @reboot_cause.command() def history(): diff --git a/tests/reboot_cause_test.py b/tests/reboot_cause_test.py index 6ecc248dd5..44de4f1181 100644 --- a/tests/reboot_cause_test.py +++ b/tests/reboot_cause_test.py @@ -31,7 +31,7 @@ def setup_class(cls): # Test 'show reboot-cause' without previous-reboot-cause.json def test_reboot_cause_no_history_file(self): - expected_output = "" + expected_output = "Unknown" runner = CliRunner() result = runner.invoke(show.cli.commands["reboot-cause"], []) assert result.output == expected_output From 81ea797179b05f87fe865bb7aa601398e375bd36 Mon Sep 17 00:00:00 2001 From: Yong Zhao Date: Fri, 26 Mar 2021 22:48:48 +0000 Subject: [PATCH 10/10] [reboot] Add newline in the expected output in unit test. Signed-off-by: Yong Zhao --- tests/reboot_cause_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/reboot_cause_test.py b/tests/reboot_cause_test.py index 44de4f1181..f3372c3eb1 100644 --- a/tests/reboot_cause_test.py +++ b/tests/reboot_cause_test.py @@ -31,7 +31,7 @@ def setup_class(cls): # Test 'show reboot-cause' without previous-reboot-cause.json def test_reboot_cause_no_history_file(self): - expected_output = "Unknown" + expected_output = "Unknown\n" runner = CliRunner() result = runner.invoke(show.cli.commands["reboot-cause"], []) assert result.output == expected_output