-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
fix podman system connection - tcp
flake
#16929
Conversation
LGTM |
🤣 |
I guess if this fails over 10 seconds, we might be looking at a different problem. |
I copy-pasted the loop but should have altered the break-condition. Will update. |
The test was only waiting for the port to be ready but that doesn't imply the server being ready to serve requests. Hence, add a loop waiting for the `info` call to succeed. Fixes: containers#16916 Signed-off-by: Valentin Rothberg <[email protected]>
@containers/podman-maintainers PTAL |
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 with one change request.
For extra credit: if you copypasted this loop from elsewhere, and that elsewhere also has an unquoted question mark, could you also fix that one? I see two instances in 255-auto-update, but those can't possibly be the ones you copied, so I'll take care of fixing those. TIA.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, vrothberg 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 |
/lgtm Changed my mind. I'll fix the questionmark in #16972 instead, so we can get this merged |
The test was only waiting for the port to be ready but that doesn't imply the server being ready to serve requests. Hence, add a loop waiting for the
info
call to succeed.Fixes: #16916
Signed-off-by: Valentin Rothberg [email protected]
Does this PR introduce a user-facing change?
@containers/podman-maintainers PTAL