Before this change if you assume that things work the way the test
expects them to (it does not, but lets assume for now) we aren't really
testing anything because we are testing that a container is healthy
before and after we send a signal. This will give false positives even
if there is a bug in the underlying code. Sending a signal can take any
amount of time to cause a container to exit or to trigger healthchecks
to stop or whatever.
Now lets remove the assumption that things are working as expected,
because they are not.
In this case, `top` (which is what is running in the container) is
actually exiting when it receives `USR1`.
This totally invalidates the test.
We need more control and knowledge as to what is happening in the
container to properly test this.
This change introduces a custom script which traps `USR1` and flips the
health status each time the signal is received.
We then send the signal twice so that we know the change has occurred
and check that the value has flipped so that we know the change has
actually occurred.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This test is failing on Windows currently:
```
11:59:47 --- FAIL: TestHealthKillContainer (8.12s)
11:59:47 health_test.go:57: assertion failed: error is not nil: Error response from daemon: Invalid signal: SIGUSR1
``
That test was added recently in https://github.com/moby/moby/pull/39454, but
rewritten in a commit in the same PR:
f8aef6a92f
In that rewrite, there were some changes:
- originally it was skipped on Windows, but the rewritten test doesn't have that skip:
```go
testRequires(c, DaemonIsLinux) // busybox doesn't work on Windows
```
- the original test used `SIGINT`, but the new one uses `SIGUSR1`
Analysis:
- The Error bubbles up from: 8e610b2b55/pkg/signal/signal.go (L29-L44)
- Interestingly; `ContainerKill` should validate if a signal is valid for the given platform, but somehow we don't hit that part; f1b5612f20/daemon/kill.go (L40-L48)
- Windows only looks to support 2 signals currently 8e610b2b55/pkg/signal/signal_windows.go (L17-L26)
- Upstream Golang looks to define `SIGINT` as well; 77f9b2728e/src/runtime/defs_windows.go (L44)
- This looks like the current list of Signals upstream in Go; 3b58ed4ad3/windows/types_windows.go (L52-L67)
```go
const (
// More invented values for signals
SIGHUP = Signal(0x1)
SIGINT = Signal(0x2)
SIGQUIT = Signal(0x3)
SIGILL = Signal(0x4)
SIGTRAP = Signal(0x5)
SIGABRT = Signal(0x6)
SIGBUS = Signal(0x7)
SIGFPE = Signal(0x8)
SIGKILL = Signal(0x9)
SIGSEGV = Signal(0xb)
SIGPIPE = Signal(0xd)
SIGALRM = Signal(0xe)
SIGTERM = Signal(0xf)
)
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
A client is already created in testenv.New(), so we can just
as well use that one, instead of creating a new client.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Add windows CI entrypoint script.
Signed-off-by: John Howard <jhoward@microsoft.com>
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
This fix is a sync up with 36266 so that relevant api tests
use the newly added container.Run/Create in helper package
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Both names have no real sense, but one allows to make sure these packages
aren't used outside of `integration`.
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
