Skip to content

Fix guest_reboot processing in test commands#941

Merged
kostyanf14 merged 3 commits into
HCK-CI:masterfrom
kostyanf14:command-reboot
May 4, 2026
Merged

Fix guest_reboot processing in test commands#941
kostyanf14 merged 3 commits into
HCK-CI:masterfrom
kostyanf14:command-reboot

Conversation

@kostyanf14
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request moves the reboot logic into the run_command_on_client method and adds a 60-second sleep to wait for machine availability. It also refactors the setup manager to use reconfigure_machine for reboots. However, the changes introduce a NoMethodError because the command parameter in run_command_on_client is passed as a string rather than an object. Additionally, performing reboots sequentially within this method may cause failures in multi-machine tests where machines need to remain active while others are being processed.

Comment thread lib/engines/hcktest/tests.rb Outdated
Comment thread lib/engines/hcktest/tests.rb Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to correct how guest_reboot is handled after running guest-side commands, so that reboot behavior is triggered in the right place without performing redundant restarts.

Changes:

  • Removed a redundant explicit reboot in HCKClient#run_post_start_commands, relying on reconfigure_machine to perform the restart.
  • Moved guest_reboot handling into Tests#run_command_on_client, adding a reboot + fixed delay after a command runs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
lib/setupmanagers/hckclient.rb Removes duplicate reboot call before reconfigure_machine and updates inline comments.
lib/engines/hcktest/tests.rb Attempts to centralize guest_reboot handling in run_command_on_client, including a post-reboot wait.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/engines/hcktest/tests.rb Outdated
Comment thread lib/engines/hcktest/tests.rb Outdated
Comment thread lib/setupmanagers/hckclient.rb Outdated
kostyanf14 added 2 commits May 4, 2026 11:28
Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
Reboot takes some time, so we need to wait until machine
is ready otherwise next commands can fail with strange
WinRMAuthorizationError error because Windows prevents
connections to the machine during reboot.
There is no good way to check if machine is ready,
so just wait some time after reboot. Expect that 60s
will be enougth.

Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
@kostyanf14
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mandatory sleep period after a guest reboot in hcktest/tests.rb to prevent WinRM authorization errors during the reboot process. It refactors run_command_on_client to handle the reboot and sleep logic internally and updates run_guest_test_command accordingly. Additionally, it removes a redundant restart call in hckclient.rb because reconfigure_machine already performs the reboot and waits for the machine to be ready. I have no feedback to provide as there were no review comments.

@kostyanf14 kostyanf14 merged commit 395780d into HCK-CI:master May 4, 2026
9 checks passed
@kostyanf14 kostyanf14 deleted the command-reboot branch May 4, 2026 09:23
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