Skip to content

Commit

Permalink
Merge branch 'main' into refactor/ollama-local
Browse files Browse the repository at this point in the history
* main:
  chore(deps): bump sonarsource/sonarcloud-github-action (#2933)
  feat(termination)!: make container termination timeout configurable (#2926)
  chore(deps): bump slackapi/slack-github-action from 1.26.0 to 2.0.0 (#2934)
  chore(deps): bump github/codeql-action from 3.25.15 to 3.28.0 (#2932)
  • Loading branch information
mdelapenya committed Jan 2, 2025
2 parents 52bd20f + 7ca837d commit 7c53c49
Show file tree
Hide file tree
Showing 11 changed files with 195 additions and 59 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ jobs:
merge-multiple: true

- name: Analyze with SonarCloud
uses: sonarsource/sonarcloud-github-action@49e6cd3b187936a73b8280d59ffd9da69df63ec9 # v2.1.1
uses: sonarsource/sonarcloud-github-action@02ef91109b2d589e757aefcfb2854c2783fd7b19 # v4.0.0
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
6 changes: 3 additions & 3 deletions .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ jobs:

# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@afb54ba388a7dca6ecae48f608c4ff05ff4cc77a # v3.25.15
uses: github/codeql-action/init@48ab28a6f5dbc2a99bf1e0131198dd8f1df78169 # v3.28.0
with:
languages: ${{ matrix.language }}
# If you wish to specify custom queries, you can do so here or in a config file.
Expand All @@ -67,7 +67,7 @@ jobs:
# Autobuild attempts to build any compiled languages (C/C++, C#, Go, Java, or Swift).
# If this step fails, then you should remove it and run the build manually (see below)
- name: Autobuild
uses: github/codeql-action/autobuild@afb54ba388a7dca6ecae48f608c4ff05ff4cc77a # v3.25.15
uses: github/codeql-action/autobuild@48ab28a6f5dbc2a99bf1e0131198dd8f1df78169 # v3.28.0

# ℹ️ Command-line programs to run using the OS shell.
# 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun
Expand All @@ -80,6 +80,6 @@ jobs:
# ./location_of_script_within_repo/buildscript.sh

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@afb54ba388a7dca6ecae48f608c4ff05ff4cc77a # v3.25.15
uses: github/codeql-action/analyze@48ab28a6f5dbc2a99bf1e0131198dd8f1df78169 # v3.28.0
with:
category: "/language:${{matrix.language}}"
3 changes: 2 additions & 1 deletion .github/workflows/docker-moby-latest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@ jobs:
- name: Notify to Slack on failures
if: failure()
id: slack
uses: slackapi/slack-github-action@v1.26.0
uses: slackapi/slack-github-action@v2.0.0
with:
payload-templated: true
payload-file-path: "./payload-slack-content.json"
env:
SLACK_WEBHOOK_URL: ${{ secrets.SLACK_DOCKER_LATEST_WEBHOOK }}
2 changes: 1 addition & 1 deletion .github/workflows/scorecards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,6 @@ jobs:

# required for Code scanning alerts
- name: "Upload SARIF results to code scanning"
uses: github/codeql-action/upload-sarif@afb54ba388a7dca6ecae48f608c4ff05ff4cc77a # v3.25.15
uses: github/codeql-action/upload-sarif@48ab28a6f5dbc2a99bf1e0131198dd8f1df78169 # v3.28.0
with:
sarif_file: results.sarif
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 {
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...)
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
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)
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",
FileMode: 700,
},
},
Mounts: Mounts(VolumeMount(volumeName, "/data")),
Expand All @@ -1085,10 +1129,41 @@ func TestContainerCreationWithVolumeAndFileWritingToIt(t *testing.T) {
},
Started: true,
})
require.NoError(t, err)
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.

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 {
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
Loading

0 comments on commit 7c53c49

Please sign in to comment.