chore(e2e): cleanup e2e framework#1268
Merged
Merged
Conversation
Signed-off-by: Calum Murray <cmurray@redhat.com>
Send kubectl port-forward stdout to a temp file instead of an undrained PIPE and poll it with a bounded deadline for the "Forwarding from" line. kubectl writes "Handling connection for <port>" to stdout on every forwarded connection, so the single readline() left the pipe undrained for the life of the process; once the ~64 KB OS buffer fills kubectl blocks on write and the forward stalls -- the exact deadlock the stderr-to-file handling already avoids. The readline() was also an unbounded blocking read that could hang the suite if kubectl wedged during setup. A temp file never blocks the writer and the 30s deadline bounds the wait. Scanning the whole output also makes port parsing order- independent on dual-stack hosts, and both stream files are closed on teardown. Also wait() after every kill() so terminated port-forward processes are reaped and returncode is populated in error messages. Signed-off-by: Marc Nuri <marc@marcnuri.com>
manusa
approved these changes
Jul 7, 2026
manusa
left a comment
manusa
left a comment
Member
There was a problem hiding this comment.
LGTM, thx!
I also pushed a small fix directly to this branch (rather than a follow-up PR) to expedite merging, these are the changes (feel free to rollback if deemed unnecessary):
- Port-forward stdout now goes to a temp file polled with a 30s deadline, instead of an undrained PIPE read with a single readline().
kubectl port-forward writes Handling connection for to stdout on every connection, so the old approach could deadlock once the ~64 KB pipe buffer filled (and readline() could hang unbounded if kubectl wedged).
It also makes local-port parsing dual-stack-safe. - proc.wait() after each kill() so the terminated port-forward is reaped (also fixes the "exit code None" diagnostic).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Follow up on #1228
This addresses some final comments left by @manusa :