Skip to content

Commit

Permalink
Merge branch 'main' into catinapoke/main
Browse files Browse the repository at this point in the history
* main:
  fix: resource clean up for tests and examples (testcontainers#2738)
  ci: add generate for mocks (testcontainers#2774)
  fix: docker config error handling when config file does not exist (testcontainers#2772)
  • Loading branch information
mdelapenya committed Sep 18, 2024
2 parents b51be12 + b60497e commit 5283d99
Show file tree
Hide file tree
Showing 263 changed files with 4,174 additions and 4,326 deletions.
14 changes: 13 additions & 1 deletion .github/workflows/ci-test-go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,25 @@ jobs:
# takes precedence over all other caching options.
skip-cache: true

- name: generate
if: ${{ inputs.platform == 'ubuntu-latest' }}
working-directory: ./${{ inputs.project-directory }}
shell: bash
run: |
make generate
git --no-pager diff && [[ 0 -eq $(git status --porcelain | wc -l) ]]
- name: modVerify
working-directory: ./${{ inputs.project-directory }}
run: go mod verify

- name: modTidy
if: ${{ inputs.platform == 'ubuntu-latest' }}
working-directory: ./${{ inputs.project-directory }}
run: make tidy
shell: bash
run: |
make tidy
git --no-pager diff && [[ 0 -eq $(git status --porcelain | wc -l) ]]
- name: ensure compilation
working-directory: ./${{ inputs.project-directory }}
Expand Down
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
}
}
15 changes: 13 additions & 2 deletions commons-test.mk
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@ $(GOBIN)/golangci-lint:
$(GOBIN)/gotestsum:
$(call go_install,gotest.tools/gotestsum@latest)

$(GOBIN)/mockery:
$(call go_install,github.com/vektra/mockery/[email protected])

.PHONY: install
install: $(GOBIN)/golangci-lint $(GOBIN)/gotestsum
install: $(GOBIN)/golangci-lint $(GOBIN)/gotestsum $(GOBIN)/mockery

.PHONY: clean
clean:
rm $(GOBIN)/golangci-lint
rm $(GOBIN)/gotestsum
rm $(GOBIN)/mockery

.PHONY: dependencies-scan
dependencies-scan:
Expand All @@ -26,7 +30,11 @@ dependencies-scan:

.PHONY: lint
lint: $(GOBIN)/golangci-lint
golangci-lint run --out-format=github-actions --path-prefix=. --verbose -c $(ROOT_DIR)/.golangci.yml --fix
golangci-lint run --out-format=colored-line-number --path-prefix=. --verbose -c $(ROOT_DIR)/.golangci.yml --fix

.PHONY: generate
generate: $(GOBIN)/mockery
go generate ./...

.PHONY: test-%
test-%: $(GOBIN)/gotestsum
Expand All @@ -51,3 +59,6 @@ test-tools: $(GOBIN)/gotestsum
.PHONY: tidy
tidy:
go mod tidy

.PHONY: pre-commit
pre-commit: generate tidy lint
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
Loading

0 comments on commit 5283d99

Please sign in to comment.