-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Sleep at the end of every hack/env
invocation for logs
#16604
Sleep at the end of every hack/env
invocation for logs
#16604
Conversation
@smarterclayton this obviously doesn't work as it gives the args to whatever command you asked -- do we want to run a shell as the entrypoint and have it do this? |
When a container exits, the Docker daemon is not very faithful in collecting all of the logs that the contained processes created before they finished. We need to sleep at the end of every `hack/env` call to ensure that we have enough time to notice what happened and we do not lose logs. Signed-off-by: Steve Kuznetsov <[email protected]>
075c8c7
to
2ff4749
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
/retest |
@smarterclayton why does |
/retest |
Because of changes to hack |
# container exit races with log collection so we | ||
# need to sleep at the end but preserve the exit | ||
# code of whatever the user asked for us to run | ||
cmd=( '/bin/bash' '-c' "${cmd[*]}; return_code=\$?; sleep 1; exit \${return_code}" ) |
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.
Pretty ugly.
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.
You're not wrong
@stevekuznetsov anything holding this up? |
@smarterclayton are you nack or ack here |
Don't have time to care, this looks like it has a better fix, if we have an
open issue and owner to come back and fix later I'm fine with the short hack
On Oct 4, 2017, at 1:05 PM, Ben Parees <[email protected]> wrote:
@stevekuznetsov <https://github.com/stevekuznetsov> anything holding this
up?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#16604 (comment)>,
or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p1T25DBYAp37WN3q3ZxPuoYp3CcTks5so7regaJpZM4PnoJb>
.
|
hack/env
invocation for logshack/env
invocation for logs
/kind bug |
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
/retest |
1 similar comment
/retest |
Automatic merge from submit-queue (batch tested with PRs 16615, 16604). |
When a container exits, the Docker daemon is not very faithful in
collecting all of the logs that the contained processes created before
they finished. We need to sleep at the end of every
hack/env
call toensure that we have enough time to notice what happened and we do not
lose logs.
Signed-off-by: Steve Kuznetsov [email protected]