-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
[DELL] S6100 Support PowerCycle in Last Reboot Reason #3403
[DELL] S6100 Support PowerCycle in Last Reboot Reason #3403
Conversation
I tested this with the build on Jenkins using S6000 (which should be supported the reboot-cause at this point, I think) but the reboot reason never got properly updated after once hardware reboot happened with PDU control. |
S6000_UT_Logs.txt |
what about S6100? |
retest vs please |
1 similar comment
retest vs please |
@sridhar-ravindran, I just checked the latest Jenkins build on this PR. It seems S6100 has a problem. "show reboot-cause" always returns "Unable to determine cause of previous reboot". |
Hi @sujinmkang For internal unit testing, i reverted that change and tested. Thanks |
# 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need to set previous_reboot_cause = UNKNOWN_REBOOT_CAUSE in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @sujinmkang
Ideally the code can be
# Update the reboot cause as required
if os.path.isfile(FIRST_BOOT_PLATFORM_FILE):
if (previous_reboot_cause == UNKNOWN_REBOOT_CAUSE):
previous_reboot_cause = "First Boot"
When the device is booted with sonic first time, the first boot file will be present.
Now when we install Sonic the first time (lets say FTOS to sonic conversion),
the show reboot-cause will return "Unable to determine the previous reboot cause".
Instead it will be good if we return clear data like "First Time Boot" or "FirstDeployment"
However this has to be decided by MSFT. Hence i set it as previous value itself.
MSFT, if needed can change to relevant return string.
Thanks
Sridhar.R
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sridhar-ravindran I got it. Let's keep it as is for now. We can revisit if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please resolve the merge conflicts ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @sujinmkang
Synced to latest and resolved conflicts
retest vs please |
- What I did
Added support to fix power-cycle last reboot cause in S6100
Known Caveat :- If we upgrade old_image (old_image without this fix) to the new_image(with this fix)
If we do a power-cycle instead of fast/warm/cold reset, show reboot-cause will return "Unknown" instead of "power-cycle"
- How I did it
Changes Done
modified: files/image_config/process-reboot-cause/process-reboot-cause
modified: files/image_config/platform/rc.local
modified: platform/broadcom/sonic-platform-modules-dell/common/dell_pmc.c
modified: platform/broadcom/sonic-platform-modules-dell/s6000/sonic_platform/chassis.py
modified: platform/broadcom/sonic-platform-modules-dell/s6100/scripts/s6100_platform.sh
modified: platform/broadcom/sonic-platform-modules-dell/s6100/sonic_platform/chassis.py
rc.local will inform the platform about first boot. Platform Init Script will use the info and update the
logical to return non-hardware in case of first time onie boot/image upgrade.
process-reboot-cause will use the first_boot notification and use it to return Valid last-reset-reason
Currently it will show unkwown which is not correct.
- How to verify it
Unit Test Results attached
a) power-cycle the box and issue show reboot-cause
b) Onie install the box and check return value of get_reboot_cause api
c) Upgrade the image and check return value of get_reboot_Cause api
- Description for the changelog
UnitTesting_Final_With_PowerCycle_and_FirstBoot (1).txt
[DELL] S6100 Support PowerCycle in Last Reboot Reason
- A picture of a cute animal (not mandatory but encouraged)