Skip to content

Commit

Permalink
fix: Recreate Ryuk container if terminated (#2084)
Browse files Browse the repository at this point in the history
* fix: recreate terminated repear container when needed.

* chore: rename variable name in test

* chore: run make lint

* chore: simplify with else if

---------

Co-authored-by: Manuel de la Peña <[email protected]>
  • Loading branch information
Mathew-Estafanous and mdelapenya authored Jan 12, 2024
1 parent 7b06a62 commit 12cce18
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 4 deletions.
10 changes: 6 additions & 4 deletions reaper.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/filters"
"github.com/docker/docker/errdefs"
"github.com/docker/go-connections/nat"

"github.com/testcontainers/testcontainers-go/internal/config"
Expand Down Expand Up @@ -141,13 +142,14 @@ func reuseOrCreateReaper(ctx context.Context, sessionID string, provider ReaperP
// Can't use Container.IsRunning because the bool is not updated when Reaper is terminated
state, err := reaperInstance.container.State(ctx)
if err != nil {
return nil, err
}

if state.Running {
if !errdefs.IsNotFound(err) {
return nil, err
}
} else if state.Running {
return reaperInstance, nil
}
// else: the reaper instance has been terminated, so we need to create a new one
reaperOnce = sync.Once{}
}

// 2. because the reaper instance has not been created yet, look for it in the Docker daemon, which
Expand Down
36 changes: 36 additions & 0 deletions reaper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,42 @@ func Test_ReaperReusedIfHealthy(t *testing.T) {
}
}

func Test_RecreateReaperIfTerminated(t *testing.T) {
config.Reset() // reset the config using the internal method to avoid the sync.Once
tcConfig := config.Read()
if tcConfig.RyukDisabled {
t.Skip("Ryuk is disabled, skipping test")
}

mockProvider := newMockReaperProvider(t)
t.Cleanup(mockProvider.RestoreReaperState)

SkipIfProviderIsNotHealthy(&testing.T{})

provider, _ := ProviderDocker.GetProvider()
ctx := context.Background()
reaper, err := reuseOrCreateReaper(context.WithValue(ctx, core.DockerHostContextKey, provider.(*DockerProvider).host), testSessionID, provider)
assert.NoError(t, err, "creating the Reaper should not error")

terminate, err := reaper.Connect()
assert.NoError(t, err, "connecting to Reaper should be successful")
terminate <- true

// Wait for ryuk's default timeout (10s) + 1s to allow for a graceful shutdown/cleanup of the container.
time.Sleep(11 * time.Second)

recreatedReaper, err := reuseOrCreateReaper(context.WithValue(ctx, core.DockerHostContextKey, provider.(*DockerProvider).host), testSessionID, provider)
assert.NoError(t, err, "creating the Reaper should not error")
assert.NotEqual(t, reaper.container.GetContainerID(), recreatedReaper.container.GetContainerID(), "expected different container ID")

terminate, err = recreatedReaper.Connect()
defer func(term chan bool) {
term <- true
}(terminate)
assert.NoError(t, err, "connecting to Reaper should be successful")
terminateContainerOnEnd(t, ctx, recreatedReaper.container)
}

func TestReaper_reuseItFromOtherTestProgramUsingDocker(t *testing.T) {
config.Reset() // reset the config using the internal method to avoid the sync.Once
tcConfig := config.Read()
Expand Down

0 comments on commit 12cce18

Please sign in to comment.