-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
synchronize exec combined output buffer when necessary #972
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/priority critical-urgent |
IPv6 e2e flakes |
nice catch and thanks to @liggitt for keep finding occurrences the ipv6 flakes show errors with some application holding the lock, I'm trying to investigate more |
[iptables, on the node, not the lock we introduced here] |
When we execute things in kind we capture the output in case of an error and provide this when debugging. This is very handy, but had a nasty bug, xref: #971.
When
cmd.Stdout == cmd.Stderr
, go will set up a single fd passed to the underyling process, and singleio.Copy
goroutine will be used for that fd. This makes it safe to share a buffer betweencmd.Stdout
andcmd.Stderr
IFFcmd.Stdout == cmd.Stderr
.When this is not the case, we need to synchronize writes to the combined output buffer.
See:
os/exec: document that non-comparable writers may race
)fixes: #971