Skip to content

Changed validation that device was rebooted in case of reboot#7064

Closed
ppikh wants to merge 1 commit intosonic-net:masterfrom
ppikh:added_stability_to_reboot_test
Closed

Changed validation that device was rebooted in case of reboot#7064
ppikh wants to merge 1 commit intosonic-net:masterfrom
ppikh:added_stability_to_reboot_test

Conversation

@ppikh
Copy link
Contributor

@ppikh ppikh commented Dec 19, 2022

Signed-off-by: Petro Pikh [email protected]

Description of PR

Changed validation that device was rebooted in case of reboot

Previously we did check by checking time on DUT before reboot and after reboot by checking "uptime since" time. Then we checked that "uptime since" time bigger than time from DUT before reboot. In some cases after reboot time on DUT may be not synchronized as result "uptime since" after reboot may be less than time before reboot DUT.

Now we do check using next logic: Get DUT uptime in seconds before reboot, get DUT uptime in seconds after reboot - then check that uptime after reboot less than uptime before reboot

Summary: Changed validation that device was rebooted in case of reboot
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 201911
  • 202012
  • 202205

Approach

What is the motivation for this PR?

Previously we did check by checking time on DUT before reboot and after reboot by checking "uptime since" time. Then we checked that "uptime since" time bigger than time from DUT before reboot. In some cases after reboot time on DUT may be not synchronized as result "uptime since" after reboot may be less than time before reboot DUT.

How did you do it?

Now we do check using next logic: Get DUT uptime in seconds before reboot, get DUT uptime in seconds after reboot - then check that uptime after reboot less than uptime before reboot

How did you verify/test it?

Executed reboot tests(fast-reboot, warm-reboot)

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Previously we did check by checking time on DUT before reboot and after reboot by checking "uptime since" time. Then we checked that "uptime since" time bigger than time from DUT before reboot.  In some cases afte reboot time on DUT may be not synchronized as result "uptime since" after reboot may be less than time before reboot DUT.

Now we do check using next logic: Get DUT uptime in seconds before reboot, get DUT uptime in seconds after reboot - then check that uptime after reboot less than uptime before reboot

Signed-off-by: Petro Pikh <[email protected]>
@liat-grozovik
Copy link
Collaborator

@ppikh please check failures

@ppikh
Copy link
Contributor Author

ppikh commented Jan 11, 2023

Hi @liat-grozovik ,

Build not found. Could you please re-trigger tests?

@ppikh
Copy link
Contributor Author

ppikh commented Jan 11, 2023

/azpw

@ppikh
Copy link
Contributor Author

ppikh commented Jan 12, 2023

/azpw run Azure.sonic-mgmt

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-mgmt

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@ppikh can you address comment

@liat-grozovik
Copy link
Collaborator

@prgeor kindly reminder to review this one following your prev comments

@liat-grozovik
Copy link
Collaborator

@prgeor kindly reminder to review following comments handling

@prgeor
Copy link
Contributor

prgeor commented Feb 17, 2023

@prgeor kindly reminder to review following comments handling

@liat-grozovik how is my comment resolved?

image

@ppikh
Copy link
Contributor Author

ppikh commented Feb 17, 2023

Hi @prgeor - seems like this fix does not make sense. It looks like was some some issue in our lab. Now this test passing with original code. Sorry for you time. Thanks

@ppikh ppikh closed this Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants