Skip to content

Conversation

m-dati
Copy link
Contributor

@m-dati m-dati commented Aug 29, 2025

This PR's primary scope, in sync with the PR 23141, is to address ssh pubkey issue, fixing some points, improving the routines to address eventual errors detected during the elaboration and adding ssh debugging.

The basic points here managed in wait_for_ssh are:
   a) exclude user’s known_hosts when missing;
  b) merge exit_code and exit_ssh results, for the final check;
  c) add ssh debugging on error;
  d) fix successful result for wait_stop case too;
  e) simplify in last record_info the ‘result’ ok/fail.

More VRs coming
~~
NOTE: exit error in L491 is for testing purpose and will be removed soon.

  a) user’s known_hosts excluded when missing
  b) exit results merged, for the final check
  c) ssh debug added on error
  d) return ok fixed also for wait_stop case:
  e) simplify in last record_info the ‘result’ ok/fail
@m-dati m-dati self-assigned this Aug 29, 2025
Copy link

Great PR! Please pay attention to the following items before merging:

Files matching lib/publiccloud/**.pm:

  • Provide VRs for both QE-C as well as QE-SAP (check Confluence for more info)

This is an automatically generated QA checklist based on modified files.

Comment on lines +495 to +496
my $known_hosts_2 = "/home/$testapi::username/.ssh/known_hosts";
$known_hosts_2 = "" unless (-f $known_hosts_2);
Copy link
Member

Choose a reason for hiding this comment

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

Does this work? Does this really check for the file on the SUT?

Suggested change
my $known_hosts_2 = "/home/$testapi::username/.ssh/known_hosts";
$known_hosts_2 = "" unless (-f $known_hosts_2);
my $known_hosts_2 = (script_run("file -f /home/$testapi::username/.ssh/known_hosts") eq 0) ? "/home/$testapi::username/.ssh/known_hosts" : "";

Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

Please phrase the git commit message in the common imperative mood. I honestly don't understand what it says right now

What you put into the description of the pull request could also be in the commit message details. I don't know how you created your pull request but your git commit messages has only the subject line while the pull request description has more details. Please keep in mind that the github pull request description is only visible on github, the commit can be considered permanent information storage.

I can recommend the tool hub (zypper in rubygem-hub) for easier PR creation. Also, I myself use a script git-pr-last to create a PR with proper description for these simple one-commit PRs:

$ cat $(which git-pr-last )
#!/bin/sh -e
target="${target:-"$USER"}"
git push $target && git show --no-patch --format=%B | hub pull-request -F -

You might want to use the more up-to-date tool https://github.com/cli/cli

See https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/CONTRIBUTING.md#coding-style for more details

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