Skip to content

[process-reboot-cause]Address the issue: Incorrect reboot cause returned when warm reboot follows a hardware caused reboot #3880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yxieca merged 7 commits into from
Dec 14, 2019
Merged

[process-reboot-cause]Address the issue: Incorrect reboot cause returned when warm reboot follows a hardware caused reboot #3880

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
66a9a25
[process-reboot-cause]Address the issue: Incorrect reboot cause retur…
Dec 12, 2019
757d82b
[process-reboot-cause]Fix review comments
Dec 12, 2019
70a7df1
[process-reboot-cause]address comments
Dec 12, 2019
0ea7089
[process-reboot-cause]address comments
Dec 13, 2019
8fb11e3
refactor the code flow
yxieca Dec 13, 2019
f37d08d
Remove escape
yxieca Dec 13, 2019
00603a9
Remove extra ':'
yxieca Dec 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 52 additions & 20 deletions files/image_config/process-reboot-cause/process-reboot-cause
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ try:
import pwd
import sys
import syslog
import re
except ImportError as err:
raise ImportError("%s - required module not found" % str(err))

Expand All @@ -22,6 +23,9 @@ REBOOT_CAUSE_DIR = "/host/reboot-cause/"
REBOOT_CAUSE_FILE = REBOOT_CAUSE_DIR + "reboot-cause.txt"
PREVIOUS_REBOOT_CAUSE_FILE = REBOOT_CAUSE_DIR + "previous-reboot-cause.txt"
FIRST_BOOT_PLATFORM_FILE = "/tmp/notify_firstboot_to_platform"
REBOOT_CAUSE_KEXEC = "/proc/cmdline"
# The following SONIC_BOOT_TYPEs come from the warm/fast reboot script which is in sonic-utilities
REBOOT_CAUSE_KEXEC_PATTERN = ".*SONIC_BOOT_TYPE=(fast-reboot|warm|fastfast)$"
Copy link
Copy Markdown
Collaborator

@stepanblyschak stepanblyschak Dec 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to new design for fast reboot - "SONIC_BOOT_TYPE=fast", but also need to check for "fast-reboot" in case reboot was done from 201803

Copy link
Copy Markdown
Collaborator

@stepanblyschak stepanblyschak Dec 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regular expression will match if SONIC_BOOT_TYPE is the last argument, lets have '.*' instead of '$'

Copy link
Copy Markdown
Collaborator

@nazariig nazariig Dec 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephenxs IMHO:

REBOOT_CAUSE_KEXEC_PATTERN = ".*SONIC_BOOT_TYPE=(fast-reboot|warm|fastfast)$" ->
REBOOT_CAUSE_KEXEC_PATTERN = "SONIC_BOOT_TYPE=(fast-reboot|warm|fastfast)"

re.match(REBOOT_CAUSE_KEXEC_PATTERN, cause_file_kexec) ->
re.search(REBOOT_CAUSE_KEXEC_PATTERN, cause_file_kexec)


UNKNOWN_REBOOT_CAUSE = "Unknown"

Expand All @@ -47,6 +51,16 @@ def log_error(msg):


# ============================= Functions =============================
def is_warmfast_reboot_from_proc_cmdline():
if os.path.isfile(REBOOT_CAUSE_KEXEC):
cause_file = open(REBOOT_CAUSE_KEXEC, "r")
Copy link
Copy Markdown
Collaborator

@stepanblyschak stepanblyschak Dec 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

files was not closed, try to use with open(REBOOT_CASE_KEXEC, "r"):

Copy link
Copy Markdown
Collaborator Author

@stephenxs stephenxs Dec 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed. use "with" for all open file cases.

cause_file_kexec = cause_file.readline()
m = re.match(REBOOT_CAUSE_KEXEC_PATTERN, cause_file_kexec)
if m and m.group(1):
# the pattern matched so it's a fast/warm reboot
return True
return False


def main():
log_info("Starting up...")
Expand All @@ -73,31 +87,49 @@ def main():
try:
import sonic_platform

# Check if the previous reboot was caused by hardware
platform = sonic_platform.platform.Platform()

chassis = platform.get_chassis()
proc_cmdline_reboot_cause = None

hardware_reboot_cause, optional_details = chassis.get_reboot_cause()

if hardware_reboot_cause == chassis.REBOOT_CAUSE_NON_HARDWARE:
# The reboot was not caused by hardware. If there is a REBOOT_CAUSE_FILE, it will
# contain any software-related reboot info. We will use it as the previous cause.
# 1. Check if the previous reboot was warm/fast reboot by testing whether there is "fast|fastfast|warm" in /proc/cmdline
# If yes, the content of /hosts/reboot-cause/reboot-cause.txt will be treated as the reboot cause
if is_warmfast_reboot_from_proc_cmdline():
if os.path.isfile(REBOOT_CAUSE_FILE):
cause_file = open(REBOOT_CAUSE_FILE, "r")
previous_reboot_cause = cause_file.readline().rstrip('\n')
proc_cmdline_reboot_cause = cause_file.readline().rstrip('\n')
cause_file.close()
# If it is FirstTime Boot and previous_reboot_cause is unknown
# and hardware_reboot cause is non_hardware then
# Update the reboot cause as required
if os.path.isfile(FIRST_BOOT_PLATFORM_FILE):
if (previous_reboot_cause == UNKNOWN_REBOOT_CAUSE):
previous_reboot_cause = UNKNOWN_REBOOT_CAUSE
os.remove(FIRST_BOOT_PLATFORM_FILE)
elif hardware_reboot_cause == chassis.REBOOT_CAUSE_HARDWARE_OTHER:
previous_reboot_cause = "{} ({})".format(hardware_reboot_cause, optional_details)
else:
# /proc/cmdline says it's a warm/fast reboot but /host/reboot-cause.txt doesn't exist.
# report an error.
log_error("/proc/cmdline indicates a fast/warm reboot but {} doesn't exist".format(REBOOT_CAUSE_DIR))
Copy link
Copy Markdown
Collaborator

@lguohan lguohan Dec 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other than report error,

I think we should still assign the

proc_cmdline_reboot_cause = warm or fast

we should treat proc cmdline as truth.

Copy link
Copy Markdown
Contributor

@yxieca yxieca Dec 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


if proc_cmdline_reboot_cause is not None:
previous_reboot_cause = proc_cmdline_reboot_cause
else:
previous_reboot_cause = hardware_reboot_cause
# 2. Check if the previous reboot was caused by hardware
# If yes, the hardware reboot cause will be treated as teh reboot cause
Copy link
Copy Markdown
Collaborator

@lguohan lguohan Dec 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

teh [](start = 70, length = 3)

-> the

Copy link
Copy Markdown
Contributor

@yxieca yxieca Dec 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

platform = sonic_platform.platform.Platform()

chassis = platform.get_chassis()

hardware_reboot_cause, optional_details = chassis.get_reboot_cause()

if hardware_reboot_cause == chassis.REBOOT_CAUSE_NON_HARDWARE:
# The reboot was not caused by hardware. If there is a REBOOT_CAUSE_FILE, it will
# contain any software-related reboot info. We will use it as the previous cause.
if os.path.isfile(REBOOT_CAUSE_FILE):
cause_file = open(REBOOT_CAUSE_FILE, "r")
Copy link
Copy Markdown
Collaborator

@nazariig nazariig Dec 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephenxs please use with statement

previous_reboot_cause = cause_file.readline().rstrip('\n')
cause_file.close()
# If it is FirstTime Boot and previous_reboot_cause is unknown
# and hardware_reboot cause is non_hardware then
# Update the reboot cause as required
if os.path.isfile(FIRST_BOOT_PLATFORM_FILE):
if (previous_reboot_cause == UNKNOWN_REBOOT_CAUSE):
previous_reboot_cause = UNKNOWN_REBOOT_CAUSE
os.remove(FIRST_BOOT_PLATFORM_FILE)
Copy link
Copy Markdown
Collaborator

@lguohan lguohan Dec 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 121 to line 132 are same as line 140 to line 151.

suggest to consolidate them as a single function.

previous_reboot_cause = find_software_reboot_cause()

Copy link
Copy Markdown
Contributor

@yxieca yxieca Dec 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

elif hardware_reboot_cause == chassis.REBOOT_CAUSE_HARDWARE_OTHER:
previous_reboot_cause = "{} ({})".format(hardware_reboot_cause, optional_details)
else:
previous_reboot_cause = hardware_reboot_cause
except ImportError as err:
log_warning("sonic_platform package not installed. Unable to detect hardware reboot causes.")

Expand Down