Skip to content

Collect log using SerialConsole when SshShell fails to connect #3845

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

adityagesh
Copy link
Collaborator

@adityagesh adityagesh commented Jun 5, 2025

ssh may not succeed due to multiple reasons, at present if ssh fails it is either very difficult or impossible to triage the issue without reproducing it.
This change uses NonSshExecutor to run commands for log collection.

ssh may not succeed due to many reasons, at present if ssh fails
it is either very difficult or impossible to triage the issue without
reproducing it.
This change uses SerialConsole to run commands for log collection.
@adityagesh
Copy link
Collaborator Author

@squirrelsc @LiliDeng , this PR is for early design review.

This will enhance triaging of failures for both engineers and AI


# Prepare the RunCommandInput for Azure
command = RunCommandInput(
command_id="RunShellScript",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this Linux specific? Consider whether you can add support for Windows. If not possible, think about how to exclude the Feature for non-Posix systems.

@adityagesh adityagesh force-pushed the aditya/serial_log_after_test_failure branch from 5a994dd to ff40ee5 Compare July 7, 2025 10:15
@adityagesh adityagesh force-pushed the aditya/serial_log_after_test_failure branch from ff40ee5 to bc92811 Compare July 7, 2025 10:15
@adityagesh adityagesh force-pushed the aditya/serial_log_after_test_failure branch from df59804 to a69b551 Compare July 8, 2025 09:17
@adityagesh adityagesh marked this pull request as draft July 8, 2025 09:23
Introduce NonSshExecutor.
It Internally uses SerialConsole/RunCommand
@adityagesh adityagesh force-pushed the aditya/serial_log_after_test_failure branch from a69b551 to 84d4f4a Compare July 8, 2025 09:50
:return: A string containing the output of the executed commands.
"""
out = []
serial_console = self._node.features[SerialConsole]
Copy link
Member

Choose a reason for hiding this comment

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

Check if the feature is not available, raise an exception.

self.log.debug(
f"Failed to collect logs using non-ssh executor: {log_error}"
)
raise e
Copy link
Member

Choose a reason for hiding this comment

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

raise

if self.features.is_supported(NonSshExecutor):
non_ssh_executor = self.features[NonSshExecutor]
out = non_ssh_executor.execute()
return "\n".join(out)
Copy link
Member

Choose a reason for hiding this comment

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

Log could be output in this method directly, instead of return and output in upper level.

execution method for scenarios where SSH connectivity is not possible or desired.
"""

COMMANDS_TO_EXECUTE = [
Copy link
Member

Choose a reason for hiding this comment

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

The commands are not related to NonSshExecutor, they should be moved to node level.

password = self._connection_info.password
if not password:
raise RequireUserPasswordException(
"The password is not set, and generate is False."
Copy link
Member

Choose a reason for hiding this comment

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

The password has not been set and generated.

@@ -190,6 +191,34 @@ def check_initramfs(
f"{initramfs_logs} {filesystem_exception_logs}"
)

def login(self) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

ensure_login

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants