Skip to content

[reboot-cause] Use UTC to ensure consistent sorting #293

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[reboot-cause] Use UTC to ensure consistent sorting #293

wants to merge 2 commits into from

Conversation

jianyuewu
Copy link

Previously reboot cause uses local timezone. This could result in the timestamp being in UTC, IDT, or any other timezone depending on the system configuration. When reboot cause history files are sorted, mixing different timezones can lead to incorrect chronological order and cause reboot cause test failure.
Now change the code to use datetime.datetime.utcnow() for reboot_cause_gen_time, ensuring that all timestamps are consistently in UTC.

Details:
Previously reboot-cause history will sort by name, name is using system time, could be UTC or IDT.
2025_07_28_10_48_14 reboot Mon Jul 28 01:45:45 PM IDT 2025 admin N/A
Here left side 10_48_14 is UTC time, while right side 01:45:45 is IDT time.
2025_07_28_13_41_44 reboot Mon Jul 28 01:39:13 PM IDT 2025 admin N/A
Left and right are both IDT time.
We need to make left side time aligned, so sort index will be always correct.

show reboot-cause history cmd:

admin@r-bison-06:~$ show reboot-cause history 
Name                 Cause       Time                             User    Comment                                                                                                                                                                                                                                
-------------------  ----------  -------------------------------  ------  ---------                                                                                                                                                                                                                                      
2025_07_28_13_41_44  reboot      Mon Jul 28 01:39:13 PM IDT 2025  admin   N/A                                                                                                                                                                                                                                           
2025_07_28_13_34_54  reboot      Mon Jul 28 01:32:25 PM IDT 2025  admin   N/A                                                                                                                                                                                                                                            
2025_07_28_13_28_13  Watchdog    N/A                              N/A     Unknown                                                                                                                                                                                                              
2025_07_28_13_22_28  reboot      Mon Jul 28 01:19:58 PM IDT 2025  admin   N/A                                                                                                                                                                                                                                  
2025_07_28_13_17_05  reboot      Mon Jul 28 10:15:33 AM UTC 2025  admin   N/A    // Time should be 01:15:33 AM
2025_07_28_10_48_14  reboot      Mon Jul 28 01:45:45 PM IDT 2025  admin   N/A    // This entry should be after 01:39:13 PM IDT                                                                                                                                                                                                                     
2025_07_28_09_57_42  Power Loss  N/A                              N/A     Unknown                                                                                                           
2025_07_28_09_52_04  Power Loss  N/A                              N/A     Unknown                                                                                                                                                                                                                                        
2025_07_28_09_46_15  Power Loss  N/A                              N/A     Unknown                                                                                                          
2025_07_28_09_40_22  Power Loss  N/A                              N/A     Unknown

Reboot cause history in test:

13:53:45 reboot.check_reboot_cause_history        L0532 INFO   | index:  6, reboot cause: Watchdog, reboot cause from DUT: reboot                                                                                                                                                                                        
13:53:45 reboot.check_reboot_cause_history        L0537 ERROR  | The 6 reboot-cause not match. expected_reboot type=Watchdog, actual_reboot_cause=reboot                                                                                                                                                                 
13:53:45 reboot.check_reboot_cause_history        L0541 INFO   | Current reboot_type_history_queue content:                                                                                                                                                                                                              
13:53:45 reboot.check_reboot_cause_history        L0543 INFO   |   index=0, reboot_type=power off
13:53:45 reboot.check_reboot_cause_history        L0543 INFO   |   index=1, reboot_type=power off
13:53:45 reboot.check_reboot_cause_history        L0543 INFO   |   index=2, reboot_type=power off
13:53:45 reboot.check_reboot_cause_history        L0543 INFO   |   index=3, reboot_type=power off
13:53:45 reboot.check_reboot_cause_history        L0543 INFO   |   index=4, reboot_type=cold
13:53:45 reboot.check_reboot_cause_history        L0543 INFO   |   index=5, reboot_type=cold
13:53:45 reboot.check_reboot_cause_history        L0543 INFO   |   index=6, reboot_type=watchdog
13:53:45 reboot.check_reboot_cause_history        L0543 INFO   |   index=7, reboot_type=cold
13:53:45 reboot.check_reboot_cause_history        L0543 INFO   |   index=8, reboot_type=cold
13:53:45 reboot.check_reboot_cause_history        L0543 INFO   |   index=9, reboot_type=cold

jianyuewu added 2 commits July 29, 2025 04:22
@jianyuewu
Use UTC to ensure consistent sorting
7333294 
Previously reboot cause uses local timezone. This could result in the
timestamp being in UTC, IDT, or any other timezone depending on the
system configuration. When reboot cause history files are sorted,
mixing different timezones can lead to incorrect chronological
order and cause reboot cause test failure.
Now change the code to use datetime.datetime.utcnow() for
reboot_cause_gen_time, ensuring that all timestamps are consistently in
UTC.

Signed-off-by: Jianyue Wu <[email protected]>
@jianyuewu
Add UT
eac82ee 
Signed-off-by: Jianyue Wu <[email protected]>
@mssonicbld
Copy link

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jianyuewu jianyuewu marked this pull request as draft July 29, 2025 10:07
@jianyuewu jianyuewu marked this pull request as ready for review July 30, 2025 05:44
@keboliu keboliu requested a review from judyjoseph July 30, 2025 06:32
@r12f r12f requested a review from vvolam July 31, 2025 15:31
@r12f
Copy link

r12f commented Jul 31, 2025

hi @vvolam , do you mind to help reviewing this PR?

vvolam
vvolam approved these changes Jul 31, 2025
View reviewed changes
Copy link
Contributor

@vvolam vvolam left a comment

Choose a reason for hiding this comment

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

LGTM, same change might be needed in scripts/procdockerstatsd as we are still using systemtime.

sonic-host-services/scripts/procdockerstatsd

Line 248 in 0028383

datetimeobj = datetime.now()

Could you also update the PR description with the reboot history command after the fix?

judyjoseph
judyjoseph reviewed Jul 31, 2025
View reviewed changes
scripts/determine-reboot-cause
@@ -254,7 +254,7 @@ def main():
previous_reboot_cause, additional_reboot_info = determine_reboot_cause()

# Current time
reboot_cause_gen_time = str(datetime.datetime.now().strftime('%Y_%m_%d_%H_%M_%S'))
reboot_cause_gen_time = str(datetime.datetime.utcnow().strftime('%Y_%m_%d_%H_%M_%S'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Please share more details on why we need this change. I have this question because we display two timestamps in each line. The first timestamp is this one which we are trying to change to utcnow(). The second time stamp is put I believe by platform code when system goes down.

2025_07_28_13_34_54 reboot Mon Jul 28 01:32:25 PM IDT 2025 admin N/A

So if we change at one place we should change at both places?

Ideally I feel if the system tz is configured as UTC, both timestamps will be in UTC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@judyjoseph judyjoseph judyjoseph left review comments

@keboliu keboliu keboliu approved these changes

@vvolam vvolam vvolam approved these changes

Assignees
No one assigned
Labels
Request for msft-202412 Branch Request for 202505 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jianyuewu @mssonicbld @r12f @keboliu @judyjoseph @vvolam @dgsudharsan