Skip to content

Commit

Permalink
address more comments
Browse files Browse the repository at this point in the history
  • Loading branch information
moogacs committed Dec 29, 2024
1 parent 31203a9 commit 12269a4
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 38 deletions.
22 changes: 11 additions & 11 deletions cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,20 @@ import (
// DefaultTimeout for termination
var DefaultTimeout = 10 * time.Second

// terminateOptions is a type that holds the options for terminating a container.
type terminateOptions struct {
// 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 {
options := &terminateOptions{
func NewTerminateOptions(ctx context.Context, opts ...TerminateOption) *TerminateOptions {
options := &TerminateOptions{
stopTimeout: &DefaultTimeout,
ctx: ctx,
}
Expand All @@ -35,17 +35,17 @@ func NewTerminateOptions(ctx context.Context, opts ...TerminateOption) *terminat
}

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

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

// Cleanup performs any clean up needed
func (o *terminateOptions) Cleanup() error {
func (o *TerminateOptions) Cleanup() error {
// TODO: simplify this when when perform the client refactor.
if len(o.volumes) == 0 {
return nil
Expand All @@ -68,15 +68,15 @@ func (o *terminateOptions) Cleanup() error {
// 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) {
return func(c *TerminateOptions) {
c.stopTimeout = &timeout
}
}
Expand All @@ -86,7 +86,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 Down
16 changes: 1 addition & 15 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,20 +296,6 @@ func (c *DockerContainer) Stop(ctx context.Context, timeout *time.Duration) erro
return nil
}

// WithStopTimeout is a functional option that sets the timeout for the container termination.
func WithStopTimeout(timeout time.Duration) TerminateOption {
return func(c *terminateOptions) {
c.stopTimeout = &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.
Expand Down Expand Up @@ -356,7 +342,7 @@ func (c *DockerContainer) Terminate(ctx context.Context, opts ...TerminateOption
c.sessionID = ""
c.isRunning = false

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

Expand Down
24 changes: 12 additions & 12 deletions docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ func TestContainerStateAfterTermination(t *testing.T) {
err = nginx.Start(ctx)
require.NoError(t, err, "expected no error from container start.")

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

Expand Down Expand Up @@ -1127,34 +1127,34 @@ func TestContainerCreationWithVolumeCleaning(t *testing.T) {
Started: true,
})
require.NoError(t, err)
err = bashC.Terminate(ctx, WithTerminateVolumes(volumeName))
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
WithTerminateVolumes("vol1", "vol2")(&options)
require.Equal(t, terminateOptions{
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
var options TerminateOptions
timeout := 11 * time.Second
WithStopTimeout(timeout)(&options)
require.Equal(t, terminateOptions{
StopTimeout(timeout)(&options)
require.Equal(t, TerminateOptions{
stopTimeout: &timeout,
}, options)
})

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

0 comments on commit 12269a4

Please sign in to comment.