Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix race condition when looking up reaper (ryuk) container #2508

Merged
merged 8 commits into from
Jun 10, 2024
7 changes: 7 additions & 0 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ type DockerContainer struct {
logProductionTimeout *time.Duration
logger Logging
lifecycleHooks []ContainerLifecycleHooks

healthStatus string // container health status, will default to healthStatusNone if no healthcheck is present
}

// SetLogger sets the logger for the container
Expand Down Expand Up @@ -1590,6 +1592,11 @@ func containerFromDockerResponse(ctx context.Context, response types.Container)
return nil, err
}

// the health status of the container, if any
if health := container.raw.State.Health; health != nil {
mdelapenya marked this conversation as resolved.
Show resolved Hide resolved
container.healthStatus = health.Status
}

return &container, nil
}

Expand Down
2 changes: 1 addition & 1 deletion generic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func TestGenericReusableContainerInSubprocess(t *testing.T) {
output := createReuseContainerInSubprocess(t)

// check is reuse container with WaitingFor work correctly.
require.True(t, strings.Contains(output, "🚧 Waiting for container id"))
require.True(t, strings.Contains(output, " Waiting for container id"))
require.True(t, strings.Contains(output, "🔔 Container is ready"))
}()
}
Expand Down
4 changes: 2 additions & 2 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ func TestReadTCConfig(t *testing.T) {
``,
map[string]string{
"TESTCONTAINERS_RYUK_RECONNECTION_TIMEOUT": "13s",
"TESTCONTAINERS_RYUK_CONNECTION_TIMEOUT": "12s",
"TESTCONTAINERS_RYUK_CONNECTION_TIMEOUT": "12s",
},
Config{
RyukReconnectionTimeout: 13 * time.Second,
Expand All @@ -291,7 +291,7 @@ func TestReadTCConfig(t *testing.T) {
ryuk.reconnection.timeout=23s`,
map[string]string{
"TESTCONTAINERS_RYUK_RECONNECTION_TIMEOUT": "13s",
"TESTCONTAINERS_RYUK_CONNECTION_TIMEOUT": "12s",
"TESTCONTAINERS_RYUK_CONNECTION_TIMEOUT": "12s",
},
Config{
RyukReconnectionTimeout: 13 * time.Second,
Expand Down
2 changes: 1 addition & 1 deletion lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ var defaultReadinessHook = func() ContainerLifecycleHooks {
// if a Wait Strategy has been specified, wait before returning
if dockerContainer.WaitingFor != nil {
dockerContainer.logger.Printf(
"🚧 Waiting for container id %s image: %s. Waiting for: %+v",
" Waiting for container id %s image: %s. Waiting for: %+v",
dockerContainer.ID[:12], dockerContainer.Image, dockerContainer.WaitingFor,
)
if err := dockerContainer.WaitingFor.WaitUntilReady(ctx, c); err != nil {
Expand Down
32 changes: 32 additions & 0 deletions reaper.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

"github.com/cenkalti/backoff/v4"
"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"
Expand Down Expand Up @@ -119,6 +120,15 @@ func lookUpReaperContainer(ctx context.Context, sessionID string) (*DockerContai

reaperContainer = r

if r.healthStatus == types.Healthy || r.healthStatus == types.NoHealthcheck {
return nil
}

// if a health status is present on the container, and the container is healthy, error
if r.healthStatus != "" {
return fmt.Errorf("container %s is not healthy, wanted status=%s, got status=%s", resp[0].ID[:8], types.Healthy, r.healthStatus)
}

return nil
}, backoff.WithContext(exp, ctx))
if err != nil {
Expand Down Expand Up @@ -162,6 +172,7 @@ func reuseOrCreateReaper(ctx context.Context, sessionID string, provider ReaperP
if err != nil {
return nil, err
}

return reaperInstance, nil
}

Expand Down Expand Up @@ -192,6 +203,27 @@ func reuseReaperContainer(ctx context.Context, sessionID string, provider Reaper
if err != nil {
return nil, err
}

Logger.Printf("⏳ Waiting for Reaper port to be ready")

var containerJson *types.ContainerJSON

if containerJson, err = reaperContainer.Inspect(ctx); err != nil {
return nil, fmt.Errorf("failed to inspect reaper container %s: %w", reaperContainer.ID[:8], err)
}

if containerJson != nil && containerJson.NetworkSettings != nil {
for port := range containerJson.NetworkSettings.Ports {
err := wait.ForListeningPort(port).
WithPollInterval(100*time.Millisecond).
WaitUntilReady(ctx, reaperContainer)
if err != nil {
return nil, fmt.Errorf("failed waiting for reaper container %s port %s/%s to be ready: %w",
reaperContainer.ID[:8], port.Proto(), port.Port(), err)
}
}
}

return &Reaper{
Provider: provider,
SessionID: sessionID,
Expand Down