-
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
install sigproxy before start/attach #17489
Conversation
Install the signal proxy before attaching to/starting the container to make sure there's no race-condition as revealed in the failing start/run tests in containers#16901. The tests had the valid expectation that signal forwarding works once the container is running. Further update the tests to account for the attach test where the expectation is that signal forwarding works once Podman has attached to container (or even before). Fixes: containers#16901 Signed-off-by: Valentin Rothberg <[email protected]> Signed-off-by: Ed Santiago <[email protected]>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
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
LGTM, nice catch! |
/cherry-pick v4.4 |
@vrothberg: once the present PR merges, I will cherry-pick it on top of v4.4 in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Green and ready to go :) |
/lgtm |
@vrothberg: new pull request created: #17491 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
One of our oldest most frustrating flakes is containers#16091, "Timed out waiting for BYE". In containers#17489 we added some debug output to see if the problem was a container hang of some sort. It does not seem to be (see containers#17675), and the debug output makes it hard to read failure logs, so let's remove it. Signed-off-by: Ed Santiago <[email protected]>
Install the signal proxy before attaching to/starting the container to make sure there's no race-condition as revealed in the failing start/run tests in #16901. The tests had the valid expectation that signal forwarding works once the container is running.
Further update the tests to account for the attach test where the expectation is that signal forwarding works once Podman has attached to container (or even before).
Fixes: #16901
Signed-off-by: Valentin Rothberg [email protected]
Signed-off-by: Ed Santiago [email protected]
Does this PR introduce a user-facing change?
@edsantiago @containers/podman-maintainers PTAL