-
Notifications
You must be signed in to change notification settings - Fork 2k
container/run: Fix stdout/err truncation after container exit #5957
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5957 +/- ##
==========================================
- Coverage 59.47% 59.46% -0.01%
==========================================
Files 357 357
Lines 29744 29750 +6
==========================================
+ Hits 17690 17691 +1
- Misses 11090 11091 +1
- Partials 964 968 +4 🚀 New features to boost your workflow:
|
Benehiko
reviewed
Mar 24, 2025
Benehiko
approved these changes
Mar 24, 2025
laurazard
reviewed
Mar 24, 2025
laurazard
approved these changes
Mar 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Fix a regression introduced by 30c4637 which made the `docker run` command produce potentially truncated stdout/stderr output. Previous implementation stopped the content streaming as soon as the container exited which would potentially truncate a long outputs. This change fixes the issue by only canceling the IO stream immediately if neither stdout nor stderr is attached. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Restore part of the code removed by 966b441 that closed the stream. It's required now because the Run command won't finish before the output stream was processed by the caller. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
related to: run: correctly handle only STDIN attached #5662
fix: Docker Desktop truncates the standard output for-mac#7632
fix: docker run truncates stdout when output is large moby/moby#49655
Fix a regression introduced by 30c4637
which made the
docker run
command produce potentially truncatedstdout/stderr output.
Previous implementation stopped the content streaming as soon as the
container exited which would potentially truncate a long outputs.
This change fixes the issue by only canceling the IO stream immediately
if neither stdout nor stderr is attached.