Skip to content
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

(QENG-7422) Add wait_time and max_connection_tries to exec.reboot #1625

Merged
merged 2 commits into from
Mar 11, 2020

Conversation

highb
Copy link
Contributor

@highb highb commented Feb 21, 2020

Prior to this commit there was no way to customize the behavior of the
Host::Unix::Exec.reboot method. This meant that if you had a host that
took a particularly long time to come back from a reboot, your tests
would fail.

This commit adds two parameters, wait_time and max_connection_tries,
which will modify how long to wait before checking on the host after
rebooting it and how many times to attempt connecting to the host after
the reboot. As noted in the method comment, the max_connection_tries
uses a fibbonacci backoff so increasing it will not increase the time
spent waiting for reconnection in a linear way.

Additionally, I have added specs for these parameters and mocked out
Kernel.sleep in the specs so that they run faster.

Prior to this commit there was no way to customize the behavior of the
Host::Unix::Exec.reboot method. This meant that if you had a host that
took a particularly long time to come back from a reboot, your tests
would fail.

This commit adds two parameters, `wait_time` and `max_connection_tries`,
which will modify how long to wait before checking on the host after
rebooting it and how many times to attempt connecting to the host after
the reboot. As noted in the method comment, the `max_connection_tries`
uses a fibbonacci backoff so increasing it will not increase the time
spent waiting for reconnection in a linear way.

Additionally, I have added specs for these parameters and mocked out
`Kernel.sleep` in the specs so that they run faster.
@highb highb requested a review from sbeaulie February 21, 2020 20:23
@trevor-vaughan
Copy link
Contributor

@highb You can run our full test suites with any changes, if you

  1. clone https://github.com/simp/rubygem-simp-beaker-helpers
  2. Update the Gemfile
  3. Run bundle update
  4. Run rake beaker:suites

@highb
Copy link
Contributor Author

highb commented Feb 25, 2020

@trevor-vaughan That is super helpful, thank you!

@highb
Copy link
Contributor Author

highb commented Feb 25, 2020

Found a small issue with the default nodeset for running the tests, put up a PR to fix it. simp/rubygem-simp-beaker-helpers#119 edit: Determined a fix for the issue in beaker itself by setting a sensible packaging_platform default #1628

@highb
Copy link
Contributor Author

highb commented Feb 25, 2020

I got the https://github.com/simp/rubygem-simp-beaker-helpers tests to pass using a Beaker branch containing this PR and #1628 so I think once #1628 is reviewed/merged and the acceptance tests are happy again, this is ready to review/merge.

@highb
Copy link
Contributor Author

highb commented Feb 25, 2020

Jenkins, test this please

Copy link
Contributor

@genebean genebean left a comment

Choose a reason for hiding this comment

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

Just need to make the comment and code match then GTG as far as I can tell.

spec/beaker/host/unix/exec_spec.rb Outdated Show resolved Hide resolved
Prior to this commit if the `uptime` command did not reset after the
provided `wait_time`, we would assume that the host wasn't going to
reboot.
This commit adds an `uptime_retries` parameter to the
`Unix::Exec.reboot` method that specifies how many times Beaker should
attempt to check the result of the `uptime` command before giving up on
the host.
@highb highb force-pushed the QENG-7422_configurable_reboot_wait branch from ffd92b6 to b6f5ea4 Compare March 5, 2020 21:50
@highb highb requested a review from a team as a code owner March 5, 2020 21:50
@highb
Copy link
Contributor Author

highb commented Mar 5, 2020

Jenkins, test this please

2 similar comments
@highb
Copy link
Contributor Author

highb commented Mar 6, 2020

Jenkins, test this please

@highb
Copy link
Contributor Author

highb commented Mar 9, 2020

Jenkins, test this please

@highb
Copy link
Contributor Author

highb commented Mar 9, 2020

@genebean Could you review again, please?

@mattkirby mattkirby merged commit 9a2df1c into voxpupuli:master Mar 11, 2020
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.

5 participants