diff --git a/cleanup.go b/cleanup.go index e2d52440b9..4de5247a8f 100644 --- a/cleanup.go +++ b/cleanup.go @@ -2,35 +2,37 @@ package testcontainers import ( "context" - "errors" "fmt" "reflect" "time" ) -// terminateOptions is a type that holds the options for terminating a container. -type terminateOptions struct { - ctx context.Context - timeout *time.Duration - volumes []string +// DefaultTimeout for termination +var DefaultTimeout = 10 * time.Second + +// TerminateOptions is a type that holds the options for terminating a container. +type TerminateOptions struct { + Context context.Context + Timeout *time.Duration + Volumes []string } // TerminateOption is a type that represents an option for terminating a container. -type TerminateOption func(*terminateOptions) +type TerminateOption func(*TerminateOptions) // 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 *TerminateOptions) { + c.Context = 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.Timeout = &timeout } } @@ -39,8 +41,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 *TerminateOptions) { + c.Volumes = volumes } } @@ -54,41 +56,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. diff --git a/container.go b/container.go index 35be60fb81..50fc656e7e 100644 --- a/container.go +++ b/container.go @@ -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 diff --git a/docker.go b/docker.go index b10b14b7ff..77634ee4a1 100644 --- a/docker.go +++ b/docker.go @@ -296,6 +296,20 @@ 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 *TerminateOptions) { + c.Timeout = &timeout + } +} + +// WithTerminateVolumes is a functional option that sets the volumes for the container termination. +func WithTerminateVolumes(volumes ...string) TerminateOption { + return func(opts *TerminateOptions) { + opts.Volumes = volumes + } +} + // 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. @@ -303,12 +317,19 @@ 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 { + defaultOptions := &TerminateOptions{ + Timeout: &DefaultTimeout, + Context: ctx, + } + + for _, opt := range opts { + opt(defaultOptions) + } + + err := c.Stop(defaultOptions.Context, defaultOptions.Timeout) if err != nil && !isCleanupSafe(err) { return fmt.Errorf("stop: %w", err) } @@ -343,6 +364,26 @@ func (c *DockerContainer) Terminate(ctx context.Context) error { c.sessionID = "" c.isRunning = false + // Remove additional volumes if any. + // TODO: simplify this when when perform the client refactor. + if len(defaultOptions.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 defaultOptions.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...) } diff --git a/docker_test.go b/docker_test.go index eb92e15060..bdcd272e92 100644 --- a/docker_test.go +++ b/docker_test.go @@ -279,6 +279,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, WithTerminateTimeout(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) @@ -1066,6 +1078,37 @@ func TestContainerCreationWithVolumeAndFileWritingToIt(t *testing.T) { // 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", + 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) + 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, @@ -1083,10 +1126,31 @@ func TestContainerCreationWithVolumeAndFileWritingToIt(t *testing.T) { }, Started: true, }) + require.NoError(t, err) + err = bashC.Terminate(ctx, WithTerminateVolumes(volumeName)) CleanupContainer(t, bashC, RemoveVolumes(volumeName)) require.NoError(t, err) } +func TestContainerTerminationOptions(t *testing.T) { + volumeName := "volumeName" + definedVolumeOpt := &TerminateOptions{} + volumeOpt := WithTerminateVolumes(volumeName) + volumeOpt(definedVolumeOpt) + require.Equal(t, definedVolumeOpt.Volumes, []string{volumeName}) + + defaultTimeout := 10 * time.Second + definedTimeoutOpt := &TerminateOptions{ + Timeout: &defaultTimeout, + } + + configuredTimeout := 1 * time.Second + + timeoutOpt := WithTerminateTimeout(1 * time.Second) + timeoutOpt(definedTimeoutOpt) + require.Equal(t, *definedTimeoutOpt.Timeout, configuredTimeout) +} + func TestContainerWithTmpFs(t *testing.T) { ctx := context.Background() req := ContainerRequest{ diff --git a/modules/etcd/etcd.go b/modules/etcd/etcd.go index 7ea78b4385..a715150bf1 100644 --- a/modules/etcd/etcd.go +++ b/modules/etcd/etcd.go @@ -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 { 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)) } } diff --git a/port_forwarding.go b/port_forwarding.go index bb6bae2393..3411ff0c1f 100644 --- a/port_forwarding.go +++ b/port_forwarding.go @@ -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...), ) }