beenull/moby
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

builder: remove redundant ExecBackend.ContainerKill()

Browse source 
The `ExecBackend.ContainerKill()` function was called before removing a build-
container.

This function is backed by `daemon.ContainerKill()` which, if no signal is passed,
performed a `daemon.Kill()`, using `SIGKILL` as signal. However, the
`ExecBackend.ContainerRm()` (backed by `daemonContainerRm()`), which is called
after this, is executed with the `ForceRemove` option set, which calls
`daemon.cleanupContainer()` with `ForceRemove` set, which also results in
`daemon.Kill()` being called:
1a0c15abbb/daemon/delete.go (L84-L95)

This makes the `ExecBackend.ContainerKill()` redundant, so removing this from
the interface.

While looking at this code, one (possible) race-condition was found in
`daemon.cleanupContainer()`, where `daemon.Kill()` could return a `errdefs.Conflict`
if the container was already stopped. An extra check was added for this case to
prevent `daemon.cleanupContainer()` from terminating early.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2022-06-05 22:27:28 +02:00
parent 0aa96c5512
commit 1f9098e7d0
No known key found for this signature in database
GPG key ID: 76698F39D527CE8C
3 changed files with 1 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
builder/builder.go
View file

@ -62,8 +62,6 @@ type ExecBackend interface {
ContainerCreateIgnoreImagesArgsEscaped(ctx context.Context, config backend.ContainerCreateConfig) (container.CreateResponse, error)
// ContainerRm removes a container specified by `id`.
ContainerRm(name string, config *backend.ContainerRmConfig) error
// ContainerKill stops the container execution abruptly.
ContainerKill(containerID string, sig string) error
// ContainerStart starts a new container
ContainerStart(ctx context.Context, containerID string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error
// ContainerWait stops processing until the given container is stopped.

3
builder/dockerfile/containerbackend.go
View file

@ -61,8 +61,7 @@ func (c *containerManager) Run(ctx context.Context, cID string, stdout, stderr i
go func() {
select {
case <-ctx.Done():
log.G(ctx).Debugln("Build cancelled, killing and removing container:", cID)
c.backend.ContainerKill(cID, "")
log.G(ctx).Debugln("Build cancelled, removing container:", cID)
err = c.backend.ContainerRm(cID, &backend.ContainerRmConfig{ForceRemove: true, RemoveVolume: true})
if err != nil {
_, _ = fmt.Fprintf(stdout, "Removing container %s: %v\n", stringid.TruncateID(cID), err)

4
builder/dockerfile/mockbackend_test.go
View file

@ -45,10 +45,6 @@ func (m *MockBackend) CommitBuildStep(ctx context.Context, c backend.CommitConfi
return "", nil
}
func (m *MockBackend) ContainerKill(containerID string, sig string) error {
return nil
}
func (m *MockBackend) ContainerStart(ctx context.Context, containerID string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error {
return nil
}