Skip to content
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.

Sign up for GitHub

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

kill test: clean up warnings; document better #5334

Merged
merged 1 commit into from
Feb 27, 2020
Merged

kill test: clean up warnings; document better #5334

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions test/system/130-kill.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,23 @@
load helpers

@test "podman kill - test signal handling in containers" {
# podman-remote and crun interact poorly in f31: crun seems to gobble up
# some signals.
# Workaround: run 'env --default-signal sh' instead of just 'sh' in
# the container. Since env on our regular alpine image doesn't support
# that flag, we need to pull fedora-minimal. See:
# https://github.com/containers/libpod/issues/5004
# FIXME: remove this kludge once we get rid of podman-remote
local _image=$IMAGE
local _sh_cmd="sh"
if is_remote; then
_image=quay.io/libpod/fedora-minimal:latest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW and/or maybe a helpful addition to this comment. This image is built automatically from ./contrib/fedora-minimal/ (since the fedoraproject registry is so often flaky).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - I think the quay.io is enough to clue someone in that this is a special image; but in this test my goal is to minimize all pulls, even from quay. Every pull is a high-risk chance for a flake.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I agree with the methodology.

Actually, something that occurred to me: When the ginkgo tests run, we cache image tarballs in /tmp with some very stable/well-known filenames. If those files exist, pulling could be completely avoided...just import them directly.

_sh_cmd="env --default-signal sh"
fi

# Start a container that will handle all signals by emitting 'got: N'
local -a signals=(1 2 3 4 5 6 8 10 12 13 14 15 16 20 21 22 23 24 25 26 64)
# The --default-signal option not available in busybox implementation of 'env' in $IMAGE
# needed here to ensure handling of SIGINT inside container uses the default handler
_IMAGE=quay.io/libpod/fedora-minimal:latest
run_podman run -d $_IMAGE env --default-signal sh -c \
run_podman run -d $_image $_sh_cmd -c \
"for i in ${signals[*]}; do trap \"echo got: \$i\" \$i; done;
echo READY;
while ! test -e /stop; do sleep 0.05; done;
Expand Down Expand Up @@ -70,6 +81,10 @@ load helpers
run_podman wait $cid
run_podman rm $cid
wait $podman_log_pid

if [[ $_image != $IMAGE ]]; then
run_podman rmi $_image
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure why you need to conditionalize this, couldn't you just always do the rmi?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that would be un-nice. The entire rationale behind this test suite is to minimize pulling. In order to do so, all tests are guaranteed to have one and exactly one $IMAGE loaded at start-time; so it is courteous for each test to (absent any failures) leave state as they found it.

}

@test "podman kill - rejects invalid args" {
Expand Down