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

feat(termination)!: make container termination timeout configurable #2926

Merged
merged 13 commits into from
Jan 2, 2025
98 changes: 57 additions & 41 deletions cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,74 @@ import (
"time"
)

// terminateOptions is a type that holds the options for terminating a container.
type terminateOptions struct {
ctx context.Context
timeout *time.Duration
volumes []string
// TerminateOptions is a type that holds the options for terminating a container.
type TerminateOptions struct {
ctx context.Context
stopTimeout *time.Duration
volumes []string
}

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

// NewTerminateOptions returns a fully initialised TerminateOptions.
// Defaults: StopTimeout: 10 seconds.
func NewTerminateOptions(ctx context.Context, opts ...TerminateOption) *TerminateOptions {
stevenh marked this conversation as resolved.
Show resolved Hide resolved
timeout := time.Second * 10
options := &TerminateOptions{
stopTimeout: &timeout,
ctx: ctx,
}
for _, opt := range opts {
opt(options)
}
return options
}

// Context returns the context to use during a Terminate.
func (o *TerminateOptions) Context() context.Context {
return o.ctx
}

// StopTimeout returns the stop timeout to use during a Terminate.
func (o *TerminateOptions) StopTimeout() *time.Duration {
return o.stopTimeout
}

// Cleanup performs any clean up needed
func (o *TerminateOptions) Cleanup() error {
// TODO: simplify this when when perform the client refactor.
if len(o.volumes) == 0 {
return nil
}
client, err := NewDockerClientWithOpts(o.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 o.volumes {
if errRemove := client.VolumeRemove(o.ctx, volume, true); errRemove != nil {
errs = append(errs, fmt.Errorf("volume remove %q: %w", volume, errRemove))
}
}
return errors.Join(errs...)
}

// StopContext returns a TerminateOption that sets the context.
// Default: context.Background().
func StopContext(ctx context.Context) TerminateOption {
return func(c *terminateOptions) {
return func(c *TerminateOptions) {
c.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 *TerminateOptions) {
c.stopTimeout = &timeout
}
}

Expand All @@ -39,7 +84,7 @@ func StopTimeout(timeout time.Duration) TerminateOption {
// which are not removed by default.
// Default: nil.
func RemoveVolumes(volumes ...string) TerminateOption {
return func(c *terminateOptions) {
return func(c *TerminateOptions) {
c.volumes = volumes
}
}
Expand All @@ -54,41 +99,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...)
stevenh marked this conversation as resolved.
Show resolved Hide resolved
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
moogacs marked this conversation as resolved.
Show resolved Hide resolved

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
15 changes: 9 additions & 6 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,12 +303,11 @@ func (c *DockerContainer) Stop(ctx context.Context, timeout *time.Duration) erro
// 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 {
options := NewTerminateOptions(ctx, opts...)
err := c.Stop(options.Context(), options.StopTimeout())
if err != nil && !isCleanupSafe(err) {
return fmt.Errorf("stop: %w", err)
}
Expand Down Expand Up @@ -343,6 +342,10 @@ func (c *DockerContainer) Terminate(ctx context.Context) error {
c.sessionID = ""
c.isRunning = false

if err = options.Cleanup(); err != nil {
errs = append(errs, err)
}

return errors.Join(errs...)
}

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

t.Run("termination-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.")

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

t.Run("Nil State after termination if raw as already set", func(t *testing.T) {
ctx := context.Background()
nginx, err := createContainerFn(ctx)
Expand Down Expand Up @@ -1077,6 +1089,38 @@ func TestContainerCreationWithVolumeAndFileWritingToIt(t *testing.T) {
{
HostFilePath: absPath,
ContainerFilePath: "/hello.sh",
FileMode: 700,
},
},
Mounts: Mounts(VolumeMount(volumeName, "/data")),
Cmd: []string{"bash", "/hello.sh"},
WaitingFor: wait.ForLog("done"),
},
Started: true,
})
CleanupContainer(t, bashC, RemoveVolumes(volumeName))
require.NoError(t, err)
}

func TestContainerCreationWithVolumeCleaning(t *testing.T) {
absPath, err := filepath.Abs(filepath.Join(".", "testdata", "hello.sh"))
require.NoError(t, err)
stevenh marked this conversation as resolved.
Show resolved Hide resolved
ctx, cnl := context.WithTimeout(context.Background(), 30*time.Second)
defer cnl()

// Create the volume.
volumeName := "volumeName"

// Create the container that writes into the mounted volume.
bashC, err := GenericContainer(ctx, GenericContainerRequest{
ProviderType: providerType,
ContainerRequest: ContainerRequest{
Image: "bash:5.2.26",
Files: []ContainerFile{
{
HostFilePath: absPath,
ContainerFilePath: "/hello.sh",
moogacs marked this conversation as resolved.
Show resolved Hide resolved
FileMode: 700,
},
},
Mounts: Mounts(VolumeMount(volumeName, "/data")),
Expand All @@ -1085,10 +1129,41 @@ func TestContainerCreationWithVolumeAndFileWritingToIt(t *testing.T) {
},
Started: true,
})
require.NoError(t, err)
moogacs marked this conversation as resolved.
Show resolved Hide resolved
err = bashC.Terminate(ctx, RemoveVolumes(volumeName))
CleanupContainer(t, bashC, RemoveVolumes(volumeName))
require.NoError(t, err)
}

func TestContainerTerminationOptions(t *testing.T) {
t.Run("volumes", func(t *testing.T) {
var options TerminateOptions
RemoveVolumes("vol1", "vol2")(&options)
require.Equal(t, TerminateOptions{
volumes: []string{"vol1", "vol2"},
}, options)
})
t.Run("stop-timeout", func(t *testing.T) {
var options TerminateOptions
timeout := 11 * time.Second
StopTimeout(timeout)(&options)
require.Equal(t, TerminateOptions{
stopTimeout: &timeout,
}, options)
})

t.Run("all", func(t *testing.T) {
var options TerminateOptions
timeout := 9 * time.Second
StopTimeout(timeout)(&options)
RemoveVolumes("vol1", "vol2")(&options)
require.Equal(t, TerminateOptions{
stopTimeout: &timeout,
volumes: []string{"vol1", "vol2"},
}, options)
})
}

func TestContainerWithTmpFs(t *testing.T) {
ctx := context.Background()
req := ContainerRequest{
Expand Down
41 changes: 41 additions & 0 deletions docs/features/garbage_collector.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,47 @@ The primary method is to use the `Terminate(context.Context)` function that is
available when a container is created. Use `defer` to ensure that it is called
on test completion.

moogacs marked this conversation as resolved.
Show resolved Hide resolved
The `Terminate` function can be customised with termination options to determine how a container is removed: termination timeout, and the ability to remove container volumes are supported at the moment. You can build the default options using the `testcontainers.NewTerminationOptions` function.

#### NewTerminateOptions

- Not available until the next release of testcontainers-go <a href="https://github.com/testcontainers/testcontainers-go"><span class="tc-version">:material-tag: main</span></a>

If you want to attach option to container termination, you can use the `testcontainers.NewTerminateOptions(ctx context.Context, opts ...TerminateOption) *TerminateOptions` option, which receives a TerminateOption as parameter, creating custom termination options to be passed on the container termination.

##### Terminate Options

###### [StopContext](../../cleanup.go)
Sets the context for the Container termination.

- **Function**: `StopContext(ctx context.Context) TerminateOption`
- **Default**: The context passed in `Terminate()`
- **Usage**:
```go
err := container.Terminate(ctx,StopContext(context.Background()))
```

###### [StopTimeout](../../cleanup.go)
Sets the timeout for stopping the Container.

- **Function**: ` StopTimeout(timeout time.Duration) TerminateOption`
- **Default**: 10 seconds
- **Usage**:
```go
err := container.Terminate(ctx, StopTimeout(20 * time.Second))
```

###### [RemoveVolumes](../../cleanup.go)
Sets the volumes to be removed during Container termination.

- **Function**: ` RemoveVolumes(volumes ...string) TerminateOption`
- **Default**: Empty (no volumes removed)
- **Usage**:
```go
err := container.Terminate(ctx, RemoveVolumes("vol1", "vol2"))
```


!!!tip

Remember to `defer` as soon as possible so you won't forget. The best time
Expand Down
6 changes: 3 additions & 3 deletions modules/etcd/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,18 @@ 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 {
moogacs marked this conversation as resolved.
Show resolved Hide resolved
errs = append(errs, fmt.Errorf("terminate child node(%d): %w", i, err))
}
}

if c.Container != nil {
if err := c.Container.Terminate(ctx); err != nil {
if err := c.Container.Terminate(ctx, opts...); err != nil {
errs = append(errs, fmt.Errorf("terminate cluster node: %w", 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
Loading