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

tests to cover locks and parallel execution #2551 #2552

Closed
wants to merge 3 commits into from
Closed

tests to cover locks and parallel execution #2551 #2552

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
53db71e
tests to cover locks and parallel execution #2551
muayyad-alsadi Mar 6, 2019
a67465b
tests to cover locks and parallel execution #2551
muayyad-alsadi Mar 6, 2019
a0f241b
Merge branch 'alsadi-tests-2019036' of github.com:muayyad-alsadi/libp…
muayyad-alsadi Mar 6, 2019
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
46 changes: 46 additions & 0 deletions test/test_podman_baseline.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,52 @@ podman run $image java 2>&1 || echo $?
########
podman rm --all

########
Copy link
Member

Choose a reason for hiding this comment

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

Could you copy lines 83 -> 91 (run java when it's not there and clean out containers) and move that below your new section? I think we should still keep the clean out container lines 88->91 where they are, but then remove the run java when it's not there section, at lines 83->86.

If that's not clear, no worries, I'll tend to it post merge.

# pull and run many containers in parallel, test locks ..etc.
########
podman rmi docker.io/library/busybox:latest > /dev/null || :
for i in `seq 10`
do ( podman run -d --name b$i docker.io/library/busybox:latest busybox httpd -f -p 80 )&
done
echo -e "\nwaiting for creation...\n"
wait
echo -e "\ndone\n"
# assert we have 10 running containers
count=$( podman ps -q | wc -l )
[ "x$count" == "x10" ] || { echo "was expecting 10 running containers"; exit -1; }

for i in `seq 10`; do ( podman stop -t=1 b$i; podman rm b$i )& done
echo -e "\nwaiting for deletion...\n"
wait
echo -e "\ndone\n"
# assert we have 0 running containers
count=$( podman ps -q | wc -l )
[ "x$count" == "x0" ] || { echo "was expecting 0 running containers"; exit -1; }


########
# run many containers in parallel for an existing image, test locks ..etc.
########
podman pull docker.io/library/busybox:latest > /dev/null || :
for i in `seq 10`
do ( podman run -d --name c$i docker.io/library/busybox:latest busybox httpd -f -p 80 )&
done
echo -e "\nwaiting for creation...\n"
wait
echo -e "\ndone\n"
# assert we have 10 running containers
count=$( podman ps -q | wc -l )
[ "x$count" == "x10" ] || { echo "was expecting 10 running containers"; exit -1; }

Copy link
Member

Choose a reason for hiding this comment

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

Well this is interesting. The following removal fails for me with:

Error: unable to find container c1: more than one result for ID or name c1: container already exists
Error: unable to find container c1: more than one result for ID or name c1: container already exists

Unless I put a several second sleep here. I then tried to changed lines 130 -> 133 with podman rmi -a -f, that seems to work, but takes a very long time, 30+ seconds of no feedback.

Adding a '-f' to the rm command on line 130 seems to make the error happen less frequently, but it's still pops from time to time.

@mheon, @vrothberg thoughts? Is this pulling up a lock issue?

Copy link
Member

Choose a reason for hiding this comment

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

Also fwiw, I stopped the script in a few places, and looked at the created containers and we'd no duplicates, so I think at least the error could use some massaging.

@muayyad-alsadi these two comments I don't suspect are an issue with your script, rather they may have found something interesting in Podman.

Copy link
Member

Choose a reason for hiding this comment

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

@TomSweeneyRedHat oh, this is an interesting one - 1 in 256 chance that we get a container ID that starts with C1, in which case we have a container c1xxxxxx... and a container with the name c1 - and rm can't tell which one we wanted to remove.

Need to check Docker for expected behavior here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this is the exact case those tests are made to catch. and I guess #2553 is also related.

it happened once, but with yesterday version of podman it did not report error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

container ID that starts with C1, in which case we have a container c1xxxxxx...

I can change it to c_1 to fix that if you like.

Copy link
Member

Choose a reason for hiding this comment

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

@muayyad-alsadi I think this might well be a bug - Docker handles this by deleting the container named c1 even if there is an ID match as well.

Copy link
Member

Choose a reason for hiding this comment

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

I think I'm going to submit a bugfix PR for this.

Copy link
Member

Choose a reason for hiding this comment

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

Opened #2563 to fix our handling of this

for i in `seq 10`; do ( podman stop -t=1 c$i; podman rm c$i )& done
echo -e "\nwaiting for deletion...\n"
wait
echo -e "\ndone\n"
# assert we have 0 running containers
count=$( podman ps -q | wc -l )
[ "x$count" == "x0" ] || { echo "was expecting 0 running containers"; exit -1; }


########
# Install java onto the container, commit it, then run it showing java usage
########
Expand Down