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: expose functions for resource clean up in tests and examples #2738

Merged
merged 1 commit into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,7 @@ TEST-*.xml

tcvenv

**/go.work
**/go.work

# VS Code settings
.vscode
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ linters:
- nonamedreturns
- testifylint
- errcheck
- nolintlint

linters-settings:
errorlint:
Expand Down
107 changes: 107 additions & 0 deletions cleanup.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
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
}

// TerminateOption is a type that represents an option for terminating a container.
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
}
}

// 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
}
}

// RemoveVolumes returns a TerminateOption that sets additional volumes to remove.
// This is useful when the container creates named volumes that should be removed
// which are not removed by default.
// Default: nil.
func RemoveVolumes(volumes ...string) TerminateOption {
return func(c *terminateOptions) {
c.volumes = volumes
}
}

// TerminateContainer calls [Container.Terminate] on the container if it is not nil.
//
// This should be called as a defer directly after [GenericContainer](...)
// or a modules Run(...) to ensure the container is terminated when the
// function ends.
func TerminateContainer(container Container, options ...TerminateOption) error {
if isNil(container) {
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)
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...)
}

// isNil returns true if val is nil or an nil instance false otherwise.
func isNil(val any) bool {
if val == nil {
return true
}

valueOf := reflect.ValueOf(val)
switch valueOf.Kind() {
case reflect.Chan, reflect.Func, reflect.Map, reflect.Ptr, reflect.UnsafePointer, reflect.Interface, reflect.Slice:
return valueOf.IsNil()
default:
return false
}
}
53 changes: 22 additions & 31 deletions container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,7 @@ func Test_BuildImageWithContexts(t *testing.T) {
ContainerRequest: req,
Started: true,
})

defer terminateContainerOnEnd(t, ctx, c)
testcontainers.CleanupContainer(t, c)

if testCase.ExpectedError != "" {
require.EqualError(t, err, testCase.ExpectedError)
Expand All @@ -317,7 +316,7 @@ func Test_GetLogsFromFailedContainer(t *testing.T) {
ContainerRequest: req,
Started: true,
})
terminateContainerOnEnd(t, ctx, c)
testcontainers.CleanupContainer(t, c)
require.Error(t, err)
require.Contains(t, err.Error(), "container exited with code 0")

Expand Down Expand Up @@ -417,25 +416,21 @@ func TestImageSubstitutors(t *testing.T) {
ImageSubstitutors: test.substitutors,
}

container, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{
ctr, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{
ContainerRequest: req,
Started: true,
})
testcontainers.CleanupContainer(t, ctr)
if test.expectedError != nil {
require.ErrorIs(t, err, test.expectedError)
return
}

if err != nil {
t.Fatal(err)
}
defer func() {
terminateContainerOnEnd(t, ctx, container)
}()
require.NoError(t, err)

// enforce the concrete type, as GenericContainer returns an interface,
// which will be changed in future implementations of the library
dockerContainer := container.(*testcontainers.DockerContainer)
dockerContainer := ctr.(*testcontainers.DockerContainer)
assert.Equal(t, test.expectedImage, dockerContainer.Image)
})
}
Expand All @@ -455,21 +450,17 @@ func TestShouldStartContainersInParallel(t *testing.T) {
ExposedPorts: []string{nginxDefaultPort},
WaitingFor: wait.ForHTTP("/").WithStartupTimeout(10 * time.Second),
}
container, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{
ctr, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{
ContainerRequest: req,
Started: true,
})
if err != nil {
t.Fatalf("could not start container: %v", err)
}
testcontainers.CleanupContainer(t, ctr)
require.NoError(t, err)

// mappedPort {
port, err := container.MappedPort(ctx, nginxDefaultPort)
port, err := ctr.MappedPort(ctx, nginxDefaultPort)
// }
if err != nil {
t.Fatalf("could not get mapped port: %v", err)
}

terminateContainerOnEnd(t, ctx, container)
require.NoError(t, err)

t.Logf("Parallel container [iteration_%d] listening on %d\n", i, port.Int())
})
Expand All @@ -480,28 +471,28 @@ func ExampleGenericContainer_withSubstitutors() {
ctx := context.Background()

// applyImageSubstitutors {
container, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{
ctr, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{
ContainerRequest: testcontainers.ContainerRequest{
Image: "alpine:latest",
ImageSubstitutors: []testcontainers.ImageSubstitutor{dockerImageSubstitutor{}},
},
Started: true,
})
// }
if err != nil {
log.Fatalf("could not start container: %v", err)
}

defer func() {
err := container.Terminate(ctx)
if err != nil {
log.Fatalf("could not terminate container: %v", err)
if err := testcontainers.TerminateContainer(ctr); err != nil {
log.Printf("failed to terminate container: %s", err)
}
}()

// }
if err != nil {
log.Printf("could not start container: %v", err)
return
}

// enforce the concrete type, as GenericContainer returns an interface,
// which will be changed in future implementations of the library
dockerContainer := container.(*testcontainers.DockerContainer)
dockerContainer := ctr.(*testcontainers.DockerContainer)

fmt.Println(dockerContainer.Image)

Expand Down
31 changes: 27 additions & 4 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,14 @@ func (c *DockerContainer) Start(ctx context.Context) error {
//
// If the container is already stopped, the method is a no-op.
func (c *DockerContainer) Stop(ctx context.Context, timeout *time.Duration) error {
// Note we can't check isRunning here because we allow external creation
// without exposing the ability to fully initialize the container state.
// See: https://github.com/testcontainers/testcontainers-go/issues/2667
// TODO: Add a check for isRunning when the above issue is resolved.

err := c.stoppingHook(ctx)
if err != nil {
return err
return fmt.Errorf("stopping hook: %w", err)
}

var options container.StopOptions
Expand All @@ -272,22 +277,38 @@ func (c *DockerContainer) Stop(ctx context.Context, timeout *time.Duration) erro
}

if err := c.provider.client.ContainerStop(ctx, c.ID, options); err != nil {
return err
return fmt.Errorf("container stop: %w", err)
}

defer c.provider.Close()

c.isRunning = false

err = c.stoppedHook(ctx)
if err != nil {
return err
return fmt.Errorf("stopped hook: %w", err)
}

return nil
}

// Terminate is used to kill the container. It is usually triggered by as defer function.
// 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)
if err != nil && !isCleanupSafe(err) {
return fmt.Errorf("stop: %w", err)
}

select {
// close reaper if it was created
case c.terminationSignal <- true:
Expand All @@ -296,6 +317,8 @@ func (c *DockerContainer) Terminate(ctx context.Context) error {

defer c.provider.client.Close()

// TODO: Handle errors from ContainerRemove more correctly, e.g. should we
// run the terminated hook?
errs := []error{
c.terminatingHook(ctx),
c.provider.client.ContainerRemove(ctx, c.GetContainerID(), container.RemoveOptions{
Expand Down
15 changes: 6 additions & 9 deletions docker_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ func TestBuildContainerFromDockerfile(t *testing.T) {
}

redisC, err := prepareRedisImage(ctx, req)
CleanupContainer(t, redisC)
require.NoError(t, err)
terminateContainerOnEnd(t, ctx, redisC)
}

// removeImageFromLocalCache removes the image from the local cache
Expand Down Expand Up @@ -202,16 +202,15 @@ func TestBuildContainerFromDockerfileWithDockerAuthConfig(t *testing.T) {
BuildArgs: map[string]*string{
"REGISTRY_HOST": &registryHost,
},
Repo: "localhost",
PrintBuildLog: true,
mdelapenya marked this conversation as resolved.
Show resolved Hide resolved
Repo: "localhost",
},
AlwaysPullImage: true, // make sure the authentication takes place
ExposedPorts: []string{"6379/tcp"},
WaitingFor: wait.ForLog("Ready to accept connections"),
}

redisC, err := prepareRedisImage(ctx, req)
terminateContainerOnEnd(t, ctx, redisC)
CleanupContainer(t, redisC)
require.NoError(t, err)
}

Expand All @@ -237,7 +236,7 @@ func TestBuildContainerFromDockerfileShouldFailWithWrongDockerAuthConfig(t *test
}

redisC, err := prepareRedisImage(ctx, req)
terminateContainerOnEnd(t, ctx, redisC)
CleanupContainer(t, redisC)
require.Error(t, err)
}

Expand All @@ -259,7 +258,7 @@ func TestCreateContainerFromPrivateRegistry(t *testing.T) {
ContainerRequest: req,
Started: true,
})
terminateContainerOnEnd(t, ctx, redisContainer)
CleanupContainer(t, redisContainer)
require.NoError(t, err)
}

Expand Down Expand Up @@ -298,6 +297,7 @@ func prepareLocalRegistryWithAuth(t *testing.T) string {
}

registryC, err := GenericContainer(ctx, genContainerReq)
CleanupContainer(t, registryC)
require.NoError(t, err)

mappedPort, err := registryC.MappedPort(ctx, "5000/tcp")
Expand All @@ -310,9 +310,6 @@ func prepareLocalRegistryWithAuth(t *testing.T) string {
t.Cleanup(func() {
removeImageFromLocalCache(t, addr+"/redis:5.0-alpine")
})
t.Cleanup(func() {
require.NoError(t, registryC.Terminate(context.Background()))
})

_, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
Expand Down
Loading