diff --git a/docker.go b/docker.go index 1bb0408cef..0b55d3382b 100644 --- a/docker.go +++ b/docker.go @@ -984,7 +984,7 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque // the reaper does not need to start a reaper for itself isReaperContainer := strings.EqualFold(req.Image, reaperImage(reaperOpts.ImageName)) if !req.SkipReaper && !isReaperContainer { - r, err := newReaper(context.WithValue(ctx, testcontainersdocker.DockerHostContextKey, p.host), sessionID.String(), p, req.ReaperOptions...) + r, err := reuseOrCreateReaper(context.WithValue(ctx, testcontainersdocker.DockerHostContextKey, p.host), sessionID.String(), p, req.ReaperOptions...) if err != nil { return nil, fmt.Errorf("%w: creating reaper failed", err) } @@ -1201,7 +1201,7 @@ func (p *DockerProvider) ReuseOrCreateContainer(ctx context.Context, req Contain sessionID := sessionID() var termSignal chan bool if !req.SkipReaper { - r, err := newReaper(context.WithValue(ctx, testcontainersdocker.DockerHostContextKey, p.host), sessionID.String(), p, req.ReaperOptions...) + r, err := reuseOrCreateReaper(context.WithValue(ctx, testcontainersdocker.DockerHostContextKey, p.host), sessionID.String(), p, req.ReaperOptions...) if err != nil { return nil, fmt.Errorf("%w: creating reaper failed", err) } @@ -1356,7 +1356,7 @@ func (p *DockerProvider) CreateNetwork(ctx context.Context, req NetworkRequest) var termSignal chan bool if !req.SkipReaper { sessionID := sessionID() - r, err := newReaper(context.WithValue(ctx, testcontainersdocker.DockerHostContextKey, p.host), sessionID.String(), p, req.ReaperOptions...) + r, err := reuseOrCreateReaper(context.WithValue(ctx, testcontainersdocker.DockerHostContextKey, p.host), sessionID.String(), p, req.ReaperOptions...) if err != nil { return nil, fmt.Errorf("%w: creating network reaper failed", err) } diff --git a/reaper.go b/reaper.go index fffd13cde1..f81df4bd76 100644 --- a/reaper.go +++ b/reaper.go @@ -24,8 +24,8 @@ const ( ) var ( - reaper *Reaper // We would like to create reaper only once - mutex sync.Mutex + reaperInstance *Reaper // We would like to create reaper only once + mutex sync.Mutex ) // ReaperProvider represents a provider for the reaper to run itself with @@ -38,22 +38,39 @@ type ReaperProvider interface { // NewReaper creates a Reaper with a sessionID to identify containers and a provider to use // Deprecated: it's not possible to create a reaper anymore. func NewReaper(ctx context.Context, sessionID string, provider ReaperProvider, reaperImageName string) (*Reaper, error) { - return newReaper(ctx, sessionID, provider, WithImageName(reaperImageName)) + return reuseOrCreateReaper(ctx, sessionID, provider, WithImageName(reaperImageName)) } -// newReaper creates a Reaper with a sessionID to identify containers and a provider to use -func newReaper(ctx context.Context, sessionID string, provider ReaperProvider, opts ...ContainerOption) (*Reaper, error) { +// reuseOrCreateReaper returns an existing Reaper instance if it exists and is running. Otherwise, a new Reaper instance +// will be created with a sessionID to identify containers and a provider to use +func reuseOrCreateReaper(ctx context.Context, sessionID string, provider ReaperProvider, opts ...ContainerOption) (*Reaper, error) { mutex.Lock() defer mutex.Unlock() - // If reaper already exists re-use it - if reaper != nil { - return reaper, nil + // If reaper already exists and healthy, re-use it + if reaperInstance != nil { + // Verify this instance is still running by checking state. + // Can't use Container.IsRunning because the bool is not updated when Reaper is terminated + state, err := reaperInstance.container.State(ctx) + if err == nil && state.Running { + return reaperInstance, nil + } } + r, err := newReaper(ctx, sessionID, provider, opts...) + if err != nil { + return nil, err + } + + reaperInstance = r + return reaperInstance, nil +} + +// newReaper creates a Reaper with a sessionID to identify containers and a provider to use +// Should only be used internally and instead use reuseOrCreateReaper to prefer reusing an existing Reaper instance +func newReaper(ctx context.Context, sessionID string, provider ReaperProvider, opts ...ContainerOption) (*Reaper, error) { dockerHost := testcontainersdocker.ExtractDockerHost(ctx) - // Otherwise create a new one - reaper = &Reaper{ + reaper := &Reaper{ Provider: provider, SessionID: sessionID, } @@ -104,6 +121,7 @@ func newReaper(ctx context.Context, sessionID string, provider ReaperProvider, o if err != nil { return nil, err } + reaper.container = c endpoint, err := c.PortEndpoint(ctx, "8080", "") if err != nil { @@ -119,6 +137,7 @@ type Reaper struct { Provider ReaperProvider SessionID string Endpoint string + container Container } // Connect runs a goroutine which can be terminated by sending true into the returned channel diff --git a/reaper_test.go b/reaper_test.go index db6ebe477e..b44c631a7e 100644 --- a/reaper_test.go +++ b/reaper_test.go @@ -57,8 +57,6 @@ func createContainerRequest(customize func(ContainerRequest) ContainerRequest) C } func Test_NewReaper(t *testing.T) { - defer func() { reaper = nil }() - type cases struct { name string req ContainerRequest @@ -105,8 +103,6 @@ func Test_NewReaper(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - // make sure we re-initialize the singleton - reaper = nil provider := &mockReaperProvider{ config: test.config, } @@ -125,8 +121,6 @@ func Test_NewReaper(t *testing.T) { } func Test_ReaperForNetwork(t *testing.T) { - defer func() { reaper = nil }() - ctx := context.Background() networkName := "test-network-with-custom-reaper" @@ -153,3 +147,28 @@ func Test_ReaperForNetwork(t *testing.T) { assert.Equal(t, "reaperImage", provider.req.Image) assert.Equal(t, "reaperImage", provider.req.ReaperImage) } + +func Test_ReaperReusedIfHealthy(t *testing.T) { + SkipIfProviderIsNotHealthy(&testing.T{}) + + ctx := context.Background() + // As other integration tests run with the (shared) Reaper as well, re-use the instance to not interrupt other tests + wasReaperRunning := reaperInstance != nil + + provider, _ := ProviderDocker.GetProvider() + reaper, err := reuseOrCreateReaper(context.WithValue(ctx, testcontainersdocker.DockerHostContextKey, provider.(*DockerProvider).host), "sessionId", provider) + assert.NoError(t, err, "creating the Reaper should not error") + + reaperReused, _ := reuseOrCreateReaper(context.WithValue(ctx, testcontainersdocker.DockerHostContextKey, provider.(*DockerProvider).host), "sessionId", provider) + assert.Same(t, reaper, reaperReused, "expecting the same reaper instance is returned if running and healthy") + + terminate, err := reaper.Connect() + defer func(term chan bool) { + term <- true + }(terminate) + assert.NoError(t, err, "connecting to Reaper should be successful") + + if !wasReaperRunning { + terminateContainerOnEnd(t, ctx, reaper.container) + } +}