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

[3006.x] Fix salt-ssh retcode reporting #64576

Merged
merged 5 commits into from Nov 28, 2023

Conversation

lkubb
Copy link
Contributor

@lkubb lkubb commented Jun 29, 2023

What does this PR do?

  • Ensures invalid retcodes reported by the remote do not crash salt-ssh.
  • Also generally ensures salt-ssh does not crash if retcode is a non-integer for whatever reason.
  • Makes sure salt.client.ssh.shell.Shell.exec_cmd calls term.close before querying the exit status and returns an exit status > 128 if a process died as the result of a signal. Both changes avoid returning None as retcode.

While I could fairly reliably reproduce the reported behavior instrumentalizing another PR for running the test suite many times in parallel, the changes to Shell found here completely solved the intermittent test failures in two full runs, where previously one could find 3-4 instances per full run (which usually sorted out themselves during the re-run and thus were not spotted; full run meaning all test runs that are part of the CI pipeline for PRs, not a single full run of the suite on a system).

What issues does this PR fix or reference?

Fixes: #64575
Fixes: #64588

Previous Behavior

Seldomly: TypeError: '>' not supported between instances of 'NoneType' and 'int'

New Behavior

Exits with the correct exit code and returns output as usual.

Merge requirements satisfied?cs

Commits signed with GPG?

Yes

@lkubb lkubb requested a review from a team as a code owner June 29, 2023 12:31
@lkubb lkubb requested review from MKLeb and removed request for a team June 29, 2023 12:31
@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title Fix salt-ssh stacktrace when retcode is not an integer [3006.x] Fix salt-ssh stacktrace when retcode is not an integer Jun 29, 2023
@lkubb lkubb temporarily deployed to ci June 29, 2023 13:21 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 29, 2023 13:21 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 29, 2023 13:21 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 29, 2023 13:21 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 29, 2023 13:38 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 29, 2023 13:41 — with GitHub Actions Inactive
@lkubb lkubb marked this pull request as draft June 30, 2023 08:44
@lkubb lkubb changed the title [3006.x] Fix salt-ssh stacktrace when retcode is not an integer [3006.x] Fix salt-ssh retcode reporting Jun 30, 2023
@lkubb lkubb temporarily deployed to ci June 30, 2023 18:59 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 30, 2023 18:59 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 30, 2023 18:59 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 30, 2023 18:59 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 30, 2023 19:17 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 30, 2023 19:22 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 30, 2023 20:45 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 30, 2023 20:45 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 30, 2023 21:01 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 30, 2023 21:03 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 30, 2023 21:17 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci June 30, 2023 21:19 — with GitHub Actions Inactive
@lkubb lkubb marked this pull request as ready for review June 30, 2023 21:37
@lkubb lkubb temporarily deployed to ci July 1, 2023 07:53 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci July 1, 2023 07:53 — with GitHub Actions Inactive
@lkubb
Copy link
Contributor Author

lkubb commented Nov 26, 2023

@dwoz Friendly ping. :)

General remark: This patch has been carried by QubesOS since June to fix their relatively frequent CI failures caused by the underlying issue.

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.

None yet

3 participants