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

[v15] Prevent exiting a session prior to output being consumed #45372

Merged
merged 2 commits into from
Aug 13, 2024

Conversation

rosstimothy
Copy link
Contributor

@rosstimothy rosstimothy commented Aug 12, 2024

Backport #45223 and #41434 to branch/v15

@rosstimothy rosstimothy added the no-changelog Indicates that a PR does not require a changelog entry label Aug 12, 2024
@rosstimothy rosstimothy enabled auto-merge August 12, 2024 14:13
@github-actions github-actions bot requested review from strideynet and zmb3 August 12, 2024 14:14
#17687 attempted to fix flakiness of TestIntegrations/AuditOn by
sending an exit-status request _prior_ to consuming all output from
the PTY. While this made the test more reliable, it created a
scenario that allowed for a session to be completed without all of
the data from the PTY being consumed by the client. This condition
was hit by running an ansible playbook that output 1MB to stdout.

The reason TestIntegrations/AuditOn was flaky is because the
exit-status request was not received at times. The mechanism used
to send that request requires sending the result over a channel
and the request to be sent by another goroutine. That provides an
opportunity for the request on the channel to be processed after
the underlying ssh connection has been closed.

To resolve the issue of missing output, the change in order of
operations from #17687 was reverted and the exit-status request
is now being sent directly in the same goroutine that waits
for the session to end instead. This change now causes the exit-status
to be sent later in time, which in the real world should not be
noticed, however, some time dependent tests needed to have their
timeout for sessions completing bumped.
@rosstimothy
Copy link
Contributor Author

Tests are failing due to a missing backport of #41434 to branch/v15

* fix(srv): SSH remote sessions resources not being closed correctly

* refactor(srv): code review suggestions

* test(srv): move t.Helper to the correct function

* chore(srv): typo

* chore(srv): typo
@rosstimothy rosstimothy force-pushed the bot/backport-45223-branch/v15 branch from 70b4603 to e929178 Compare August 13, 2024 13:02
@rosstimothy rosstimothy added this pull request to the merge queue Aug 13, 2024
Merged via the queue into branch/v15 with commit 7216a13 Aug 13, 2024
29 of 30 checks passed
@rosstimothy rosstimothy deleted the bot/backport-45223-branch/v15 branch August 13, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport no-changelog Indicates that a PR does not require a changelog entry size/sm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants