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.

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

integration: Fix race in TestFreeze #901

Closed
wants to merge 1 commit into from

Conversation

mrunalp
Copy link
Contributor

@mrunalp mrunalp commented Jun 9, 2016

Without the check to ensure that the container is running
the test times out and panics.

I can reproduce this consistently by running make test locally.

@crosbymichael PTAL.

This was brought to my attention by @hmeng-19

=== RUN   TestFreeze
panic: test timed out after 3m0s


goroutine 16 [running]:
panic(0x721200, 0xc820398a10)
    /usr/local/go/src/runtime/panic.go:481 +0x3e6
testing.startAlarm.func1()
    /usr/local/go/src/testing/testing.go:725 +0x14b
created by time.goFunc
    /usr/local/go/src/time/sleep.go:129 +0x3a

goroutine 1 [chan receive, 2 minutes]:
testing.RunTests(0x8dcba8, 0xc15460, 0x2f, 0x2f, 0x1)
    /usr/local/go/src/testing/testing.go:583 +0x8d2
testing.(*M).Run(0xc820035f08, 0x1)
    /usr/local/go/src/testing/testing.go:515 +0x81
_/go/src/github.com/opencontainers/runc/libcontainer/integration.TestMain(0xc820035f08)
    /go/src/github.com/opencontainers/runc/libcontainer/integration/init_test.go:58 +0x3d7
main.main()
    _/go/src/github.com/opencontainers/runc/libcontainer/integration/_test/_testmain.go:144 +0x114

goroutine 17 [syscall, 2 minutes, locked to thread]:
runtime.goexit()
    /usr/local/go/src/runtime/asm_amd64.s:1998 +0x1

goroutine 5 [syscall, 2 minutes]:
os/signal.signal_recv(0x0)
    /usr/local/go/src/runtime/sigqueue.go:116 +0x132
os/signal.loop()
    /usr/local/go/src/os/signal/signal_unix.go:22 +0x18
created by os/signal.init.1
    /usr/local/go/src/os/signal/signal_unix.go:28 +0x37

goroutine 101 [runnable]:
time.Sleep(0xf4240)
    /usr/local/go/src/runtime/time.go:59 +0xf9
github.com/opencontainers/runc/libcontainer/cgroups/fs.(*FreezerGroup).Set(0xc43748, 0xc82036ac30, 0x27, 0xc820384050, 0x0, 0x0)
    /home/hmeng/go/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/opencontainers/runc/libcontainer/cgroups/fs/freezer.go:44 +0x289
github.com/opencontainers/runc/libcontainer/cgroups/fs.(*Manager).Freeze(0xc820396040, 0x829348, 0x6, 0x0, 0x0)
    /home/hmeng/go/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/opencontainers/runc/libcontainer/cgroups/fs/apply_raw.go:235 +0x207
github.com/opencontainers/runc/libcontainer.(*linuxContainer).Pause(0xc82014a000, 0x0, 0x0)
    /home/hmeng/go/src/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/opencontainers/runc/libcontainer/container_linux.go:402 +0x139
_/go/src/github.com/opencontainers/runc/libcontainer/integration.testFreeze(0xc8204ff5f0, 0xc8118fa400)
    /go/src/github.com/opencontainers/runc/libcontainer/integration/exec_test.go:516 +0x600
_/go/src/github.com/opencontainers/runc/libcontainer/integration.TestFreeze(0xc8204ff5f0)
    /go/src/github.com/opencontainers/runc/libcontainer/integration/exec_test.go:470 +0x26
testing.tRunner(0xc8204ff5f0, 0xc15598)
    /usr/local/go/src/testing/testing.go:473 +0x98
created by testing.RunTests
    /usr/local/go/src/testing/testing.go:582 +0x892
FAIL    _/go/src/github.com/opencontainers/runc/libcontainer/integration    180.013s

Signed-off-by: Mrunal Patel mrunalp@gmail.com

Without the check to ensure that the container is running
the test times out and panics.

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
@mrunalp
Copy link
Contributor Author

mrunalp commented Jun 9, 2016

The janky failure here is the one fixed in #900 ;)

@cyphar
Copy link
Member

cyphar commented Jun 10, 2016

Triggered rebuild since #900 is merged.

@haiyanmeng
Copy link
Contributor

@mrunalp , I tested this PR, and it fixed the problem.

@cyphar
Copy link
Member

cyphar commented Jun 10, 2016

LGTM. Although, I'm always a bit worried when we have to deal with race conditions this way.

Approved with PullApprove

@crosbymichael
Copy link
Member

This type of race should be fixed by #886

@crosbymichael
Copy link
Member

@mrunalp can you verify that this is fixed now that the fifo pr was merged?

@mrunalp
Copy link
Contributor Author

mrunalp commented Jun 13, 2016

@crosbymichael Yep, it is fixed. Thanks!

@mrunalp mrunalp closed this Jun 13, 2016
stefanberger pushed a commit to stefanberger/runc that referenced this pull request Sep 8, 2017
config: Simplify title to "Configuration"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants