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

ci: use time param for reboot #591

Merged
merged 1 commit into from
Sep 2, 2021
Merged

Conversation

Itxaka
Copy link
Contributor

@Itxaka Itxaka commented Sep 2, 2021

Currently we had the reboot function on SUT with a hardcoded timeout
which was hitting its limits on the raw disk test

This modifies the reboot function so it acceps a timeout and passes it
to the EventuallyConnects function

Signed-off-by: Itxaka igarcia@suse.com

Currently we had the reboot function on SUT with a hardcoded timeout
which was hitting its limits on the raw disk test

This modifies the reboot function so it acceps a timeout and passes it
to the EventuallyConnects function

Signed-off-by: Itxaka <igarcia@suse.com>
Copy link
Contributor

@mudler mudler left a comment

Choose a reason for hiding this comment

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

ouch, good catch!

Copy link
Contributor

@davidcassany davidcassany left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@davidcassany
Copy link
Contributor

According to the serial logs I checked a while ago on some failed tests I saw that first boot took around 250 seconds and it was failing due to a timeout on reboot. So increasing the reboot time might definitely help to fix the issue, even though more 180s for a single reboot without having to install anything at boot time is close to the unacceptable area, but we can analyze that later on.

@davidcassany
Copy link
Contributor

davidcassany commented Sep 2, 2021

@mudler @Itxaka the raw image deploy test passed. I'd say there is no need to wait for all the tests, if possible I'd force merge this PR as it has potential to unblock others.

@Itxaka Itxaka merged commit 930b096 into rancher:master Sep 2, 2021
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.

3 participants