Terminating the exec process when the context is canceled has been
broken since Docker v17.11 so nobody has been able to depend upon that
behaviour in five years of releases. We are thus free from backwards-
compatibility constraints.
conflicts:
- minor conflict in daemon/exec.go, as 2ec2b65e45
is not in the 20.10 branch, so had to cast the signal to an int.
- minor conflict in daemon/health.go, where a comment was updated, which was
added in bdc6473d2d, which is not in the
20.10 branch
- remove the skip.If() from TestHealthCheckProcessKilled, as the 20.10 branch
is not testing on Windows with containerd (and the RuntimeIsWindowsContainerd
does not exist), but kept a "FIXME" comment.
Co-authored-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Cory Snider <csnider@mirantis.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 4b84a33217)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
(cherry picked from commit 27ba755f70)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
Cory Snider