Skip to content

Commit

Permalink
feat(termination): make container termination timeout configurable
Browse files Browse the repository at this point in the history
  • Loading branch information
moogacs committed Dec 19, 2024
1 parent ea4feea commit 4c3c5b5
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 50 deletions.
48 changes: 9 additions & 39 deletions cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package testcontainers

import (
"context"
"errors"
"fmt"
"reflect"
"time"
Expand All @@ -16,21 +15,21 @@ type terminateOptions struct {
}

// TerminateOption is a type that represents an option for terminating a container.
type TerminateOption func(*terminateOptions)
type TerminateOption func(*DockerContainer)

// StopContext returns a TerminateOption that sets the context.
// Default: context.Background().
func StopContext(ctx context.Context) TerminateOption {
return func(c *terminateOptions) {
c.ctx = ctx
return func(c *DockerContainer) {
c.terminationOptions.ctx = ctx
}
}

// StopTimeout returns a TerminateOption that sets the timeout.
// Default: See [Container.Stop].
func StopTimeout(timeout time.Duration) TerminateOption {
return func(c *terminateOptions) {
c.timeout = &timeout
return func(c *DockerContainer) {
c.terminationOptions.timeout = &timeout
}
}

Expand All @@ -39,8 +38,8 @@ func StopTimeout(timeout time.Duration) TerminateOption {
// which are not removed by default.
// Default: nil.
func RemoveVolumes(volumes ...string) TerminateOption {
return func(c *terminateOptions) {
c.volumes = volumes
return func(c *DockerContainer) {
c.terminationOptions.volumes = volumes
}
}

Expand All @@ -54,41 +53,12 @@ func TerminateContainer(container Container, options ...TerminateOption) error {
return nil
}

c := &terminateOptions{
ctx: context.Background(),
}

for _, opt := range options {
opt(c)
}

// TODO: Add a timeout when terminate supports it.
err := container.Terminate(c.ctx)
err := container.Terminate(context.Background(), options...)
if !isCleanupSafe(err) {
return fmt.Errorf("terminate: %w", err)
}

// Remove additional volumes if any.
if len(c.volumes) == 0 {
return nil
}

client, err := NewDockerClientWithOpts(c.ctx)
if err != nil {
return fmt.Errorf("docker client: %w", err)
}

defer client.Close()

// Best effort to remove all volumes.
var errs []error
for _, volume := range c.volumes {
if errRemove := client.VolumeRemove(c.ctx, volume, true); errRemove != nil {
errs = append(errs, fmt.Errorf("volume remove %q: %w", volume, errRemove))
}
}

return errors.Join(errs...)
return nil
}

// isNil returns true if val is nil or an nil instance false otherwise.
Expand Down
2 changes: 1 addition & 1 deletion container.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type Container interface {
Stop(context.Context, *time.Duration) error // stop the container

// Terminate stops and removes the container and its image if it was built and not flagged as kept.
Terminate(ctx context.Context) error
Terminate(ctx context.Context, opts ...TerminateOption) error

Logs(context.Context) (io.ReadCloser, error) // Get logs of the container
FollowOutput(LogConsumer) // Deprecated: it will be removed in the next major release
Expand Down
46 changes: 40 additions & 6 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ type DockerContainer struct {
logProductionCtx context.Context

logProductionTimeout *time.Duration
terminationOptions terminateOptions
logger Logging
lifecycleHooks []ContainerLifecycleHooks

Expand Down Expand Up @@ -296,19 +297,33 @@ func (c *DockerContainer) Stop(ctx context.Context, timeout *time.Duration) erro
return nil
}

// WithTerminateTimeout is a functional option that sets the timeout for the container termination.
func WithTerminateTimeout(timeout time.Duration) TerminateOption {
return func(c *DockerContainer) {
c.terminationOptions.timeout = &timeout
}
}

// Terminate calls stops and then removes the container including its volumes.
// If its image was built it and all child images are also removed unless
// the [FromDockerfile.KeepImage] on the [ContainerRequest] was set to true.
//
// The following hooks are called in order:
// - [ContainerLifecycleHooks.PreTerminates]
// - [ContainerLifecycleHooks.PostTerminates]
func (c *DockerContainer) Terminate(ctx context.Context) error {
// ContainerRemove hardcodes stop timeout to 3 seconds which is too short
// to ensure that child containers are stopped so we manually call stop.
// TODO: make this configurable via a functional option.
timeout := 10 * time.Second
err := c.Stop(ctx, &timeout)
//
// Default: timeout is 10 seconds.
func (c *DockerContainer) Terminate(ctx context.Context, opts ...TerminateOption) error {
if len(opts) == 0 {
d := 10 * time.Second
c.terminationOptions.timeout = &d
}

for _, opt := range opts {
opt(c)
}

err := c.Stop(ctx, c.terminationOptions.timeout)
if err != nil && !isCleanupSafe(err) {
return fmt.Errorf("stop: %w", err)
}
Expand Down Expand Up @@ -343,6 +358,25 @@ func (c *DockerContainer) Terminate(ctx context.Context) error {
c.sessionID = ""
c.isRunning = false

// Remove additional volumes if any.
if len(c.terminationOptions.volumes) == 0 {
return nil
}

client, err := NewDockerClientWithOpts(ctx)
if err != nil {
return fmt.Errorf("docker client: %w", err)
}

defer client.Close()

// Best effort to remove all volumes.
for _, volume := range c.terminationOptions.volumes {
if errRemove := client.VolumeRemove(ctx, volume, true); errRemove != nil {
errs = append(errs, fmt.Errorf("volume remove %q: %w", volume, errRemove))
}
}

return errors.Join(errs...)
}

Expand Down
21 changes: 21 additions & 0 deletions docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,27 @@ func TestContainerStateAfterTermination(t *testing.T) {
require.Nil(t, state, "expected nil container inspect.")
})

t.Run("Nil State after termination with timeout", func(t *testing.T) {
ctx := context.Background()
nginx, err := createContainerFn(ctx)
require.NoError(t, err)

err = nginx.Start(ctx)
require.NoError(t, err, "expected no error from container start.")

state, err := nginx.State(ctx)
require.NoError(t, err, "expected no error from container inspect.")
require.NotNil(t, state, "expected non nil container inspect.")
require.True(t, state.Running, "expected container state to be running.")

err = nginx.Terminate(ctx, WithTerminateTimeout(5*time.Microsecond))
require.NoError(t, err)

state, err = nginx.State(ctx)
require.Error(t, err, "expected error from container inspect.")
require.Nil(t, state, "expected nil container inspect.")
})

t.Run("Nil State after termination if raw as already set", func(t *testing.T) {
ctx := context.Background()
nginx, err := createContainerFn(ctx)
Expand Down
4 changes: 2 additions & 2 deletions modules/etcd/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ type EtcdContainer struct {

// Terminate terminates the etcd container, its child nodes, and the network in which the cluster is running
// to communicate between the nodes.
func (c *EtcdContainer) Terminate(ctx context.Context) error {
func (c *EtcdContainer) Terminate(ctx context.Context, opts ...testcontainers.TerminateOption) error {
var errs []error

// child nodes has no other children
for i, child := range c.childNodes {
if err := child.Terminate(ctx); err != nil {
if err := child.Terminate(ctx, opts...); err != nil {
errs = append(errs, fmt.Errorf("terminate child node(%d): %w", i, err))
}
}
Expand Down
4 changes: 2 additions & 2 deletions port_forwarding.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,10 +225,10 @@ type sshdContainer struct {
}

// Terminate stops the container and closes the SSH session
func (sshdC *sshdContainer) Terminate(ctx context.Context) error {
func (sshdC *sshdContainer) Terminate(ctx context.Context, opts ...TerminateOption) error {
return errors.Join(
sshdC.closePorts(),
sshdC.Container.Terminate(ctx),
sshdC.Container.Terminate(ctx, opts...),
)
}

Expand Down

0 comments on commit 4c3c5b5

Please sign in to comment.