Support sig-proxy for podman-remote attach and start #16985
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashley-cui 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 |
openshift-ci
bot
added
the
approved
|
@rhatdan PTAL :) |
ashley-cui
force-pushed
the
attachsig
branch
2 times, most recently
from
9055919 to
a201e20
Compare
test/system/032-sig-proxy.bats
Outdated
| @@ -40,4 +40,99 @@ load helpers | |||
| run_podman rm -f -t0 foo | |||
| } | |||
|
|
|||
| @test "podman start sigkill" { | |||
|
|
|||
| $PODMAN create --name foo $IMAGE sh -c 'trap "echo BYE;exit 0" INT;echo READY;while :;do sleep 0.1;done' | |||
There was a problem hiding this comment.
Please don't use $PODMAN unless it's absolutely an emergency. (The next line, 46, might count as such. I'll look into it closer tomorrow. This line, 45, should use run_podman.
There was a problem hiding this comment.
Never mind, I see what happened. It's copypasta nastiness. Fix in progress, I will report back later this morning.
There was a problem hiding this comment.
@ashley-cui try this 032-sigproxy.bats. I've confirmed that it passes (including remote) on your branch, and the two new tests fail on main.
Suggestion: I encourage you to form a new habit. Any time you find yourself copypasting more than a handful of lines, or if you see someone doing so, take a step back to evaluate what common elements you can refactor. Over time you will find your brain screaming at these aberrations, and you'll take time to fix them, and you will become a much better programmer. (You already are, but we all have room for improvement!)
Signals were not proxied for attach and start for podman-remote. Now they are. Signed-off-by: Ashley Cui <[email protected]>
|
LGTM |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Signals were not proxied for attach and start for podman-remote. Now they are.
Signed-off-by: Ashley Cui [email protected]
Fixes: #16662
Does this PR introduce a user-facing change?