Fix racy test

[runcom]

Fixes: #11966 (just the first two, working on the ps ones now)

@jfrazelle

Signed-off-by: Antonio Murdaca me@runcom.ninja

[jessfraz]

$ TESTFLAGS='-race -v -run TestRunContainerWithRmFlagExitCodeNotEqualToZero' make test-integration-cli

---> Making bundle: test-integration-cli (in bundles/1.5.0-dev/test-integration-cli)
+++ exec docker --daemon --debug --host unix:///go/src/github.com/docker/docker/bundles/1.5.0-dev/test-integration-cli/docker.sock --storage-driver vfs --exec-driver native --pidfile /go/src/github.com/docker/docker/bundles/1.5.0-dev/test-integration-cli/docker.pid
+++ tar -cC /docker-frozen-images .
+++ docker load
+++ tar -cC /go/src/github.com/docker/docker/bundles/1.5.0-dev/test-integration-cli/emptyfs .
+++ docker load
+ go test -race -v -run TestRunContainerWithRmFlagExitCodeNotEqualToZero -test.timeout=30m github.com/docker/docker/integration-cli
=== RUN TestRunContainerWithRmFlagExitCodeNotEqualToZero
--- FAIL: TestRunContainerWithRmFlagExitCodeNotEqualToZero (0.42s)
    docker_cli_run_test.go:3410: error executing docker inspect: exit status 1
FAIL
coverage: 6.8% of statements
exit status 1
FAIL    github.com/docker/docker/integration-cli    0.430s
++++ cat /go/src/github.com/docker/docker/bundles/1.5.0-dev/test-integration-cli/docker.pid
+++ kill 689
Makefile:74: recipe for target 'test-integration-cli' failed
make: *** [test-integration-cli] Error 1

:(

[runcom]

@jfrazelle I'll keep on trying then :)

[runcom]

---> Making bundle: test-integration-cli (in bundles/1.5.0-dev/test-integration-cli)
+++ exec docker --daemon --debug --host unix:///go/src/github.com/docker/docker/bundles/1.5.0-dev/test-integration-cli/docker.sock --storage-driver vfs --exec-driver native --pidfile /go/src/github.com/docker/docker/bundles/1.5.0-dev/test-integration-cli/docker.pid
+++ tar -cC /docker-frozen-images .
+++ docker load
+++ tar -cC /go/src/github.com/docker/docker/bundles/1.5.0-dev/test-integration-cli/emptyfs .
+++ docker load
+ go test -race -v -run TestRunContainerWithRmFlagExitCodeNotEqualToZero -test.timeout=30m github.com/docker/docker/integration-cli
=== RUN TestRunContainerWithRmFlagExitCodeNotEqualToZero
[PASSED]: run - container is removed if run with --rm and exit code != 0
--- PASS: TestRunContainerWithRmFlagExitCodeNotEqualToZero (0.45s)
PASS
coverage: 6.1% of statements
ok      github.com/docker/docker/integration-cli    0.467s
++++ cat /go/src/github.com/docker/docker/bundles/1.5.0-dev/test-integration-cli/docker.pid
+++ kill 695

:)

[jessfraz]

yayyy!

[jessfraz]

windows fail is just TestPullVerified LGTM

[LK4D4]

I don't understand how it fixes races. You only added check that run is exits within 10 seconds.

[estesp]

I was going to ask the same question as @LK4D4. The removal has to be done by the time docker run exits, so unless there is some other inconsistency in daemon state, I'm unclear what is "racing" and whether this just hides it?

[LK4D4]

@estesp I can't reproduce that race on my machine, nor with master, neither with this PR.
But it failing pretty often on jenkins, with leaving containers after docker run --rm

[runcom]

@LK4D4 @estesp I was using TESTFLAGS='-race -v -run TestRunContainerWithRmFlagExitCodeNotEqualToZero' make test-integration-cli as @jfrazelle suggested to reproduce and indeed I can always reproduce it inside dev container

however, seems that when running a container with a nonexistent command or when a command gives error the cli wait a bit before exiting, so I added that. It was failing even with waitInspect from utils

[LK4D4]

@runcom I don't understand motivation of done channel still. It is like voodoo or orcs magic from warhammer.

[runcom]

@LK4D4 it's just to sync. To me, it seems that run doesn't fully block and with that -race above it doesn't wait to finish and move over. Putting that in a go routine and wait it to finish seemed the way to go to properly wait to finish. I don't know of course if there is some inconsistency with deamon state

[LK4D4]

@runcom No, if we don't take GC/scheduler stuff in account, then your code totally equivalent to previous except for timeout check.
We can merge it as is, but I want to be sure, that all understand that code is same.

[runcom]

@LK4D4 no, I'm more into understanding why these tests fails randomly or are racy than merging this

[runcom]

I don't know what's up here but I can't reproduce this anymore with
TESTFLAGS='-race -v -run TestRunContainerWithRmFlagExitCodeNotEqualToZero' make test-integration-cli
😕

[LK4D4]

@runcom Yup, I'm too.

[jessfraz]

hmmm ya the overlay tests I am running to fix the issue are still apparent with this patch :(

[jessfraz]

I'm going to close this sorry I think I have a different fix :(

[jessfraz]

btw it was not these tests, it was specific to running overlay in overlay and containers failing to be removed from build tests

[runcom]

good to know!