Skip to content

Commit

Permalink
Merge branch 'main' into chore/improve-error-wrapping
Browse files Browse the repository at this point in the history
  • Loading branch information
mdelapenya authored Aug 14, 2024
2 parents d8ae013 + 4545c29 commit ed06f7c
Show file tree
Hide file tree
Showing 32 changed files with 287 additions and 353 deletions.
7 changes: 5 additions & 2 deletions container.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,13 @@ type Container interface {
MappedPort(context.Context, nat.Port) (nat.Port, error) // get externally mapped port for a container port
Ports(context.Context) (nat.PortMap, error) // Deprecated: Use c.Inspect(ctx).NetworkSettings.Ports instead
SessionID() string // get session id
IsRunning() bool
IsRunning() bool // IsRunning returns true if the container is running, false otherwise.
Start(context.Context) error // start the container
Stop(context.Context, *time.Duration) error // stop the container
Terminate(context.Context) error // terminate 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

Logs(context.Context) (io.ReadCloser, error) // Get logs of the container
FollowOutput(LogConsumer) // Deprecated: it will be removed in the next major release
StartLogProducer(context.Context, ...LogProductionOption) error // Deprecated: Use the ContainerRequest instead
Expand Down
23 changes: 6 additions & 17 deletions container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"fmt"
"io"
"log"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -319,28 +318,18 @@ func Test_GetLogsFromFailedContainer(t *testing.T) {
ContainerRequest: req,
Started: true,
})

if err != nil && err.Error() != "failed to start container: container exited with code 0" {
t.Fatal(err)
} else if err == nil {
terminateContainerOnEnd(t, ctx, c)
t.Fatal("was expecting error starting container")
}
terminateContainerOnEnd(t, ctx, c)
require.Error(t, err)
require.Contains(t, err.Error(), "container exited with code 0")

logs, logErr := c.Logs(ctx)
if logErr != nil {
t.Fatal(logErr)
}
require.NoError(t, logErr)

b, err := io.ReadAll(logs)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)

log := string(b)
if strings.Contains(log, "I was not expecting this") == false {
t.Fatalf("could not find expected log in %s", log)
}
require.Contains(t, log, "I was not expecting this")
}

// dockerImageSubstitutor {
Expand Down
34 changes: 24 additions & 10 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,38 +221,43 @@ func (c *DockerContainer) SessionID() string {
func (c *DockerContainer) Start(ctx context.Context) error {
err := c.startingHook(ctx)
if err != nil {
return err
return fmt.Errorf("starting hook: %w", err)
}

if err := c.provider.client.ContainerStart(ctx, c.ID, container.StartOptions{}); err != nil {
return err
return fmt.Errorf("container start: %w", err)
}
defer c.provider.Close()

err = c.startedHook(ctx)
if err != nil {
return err
return fmt.Errorf("started hook: %w", err)
}

c.isRunning = true

err = c.readiedHook(ctx)
if err != nil {
return err
return fmt.Errorf("readied hook: %w", err)
}

return nil
}

// Stop will stop an already started container
// Stop stops the container.
//
// In case the container fails to stop
// gracefully within a time frame specified by the timeout argument,
// it is forcefully terminated (killed).
// In case the container fails to stop gracefully within a time frame specified
// by the timeout argument, it is forcefully terminated (killed).
//
// If the timeout is nil, the container's StopTimeout value is used, if set,
// otherwise the engine default. A negative timeout value can be specified,
// meaning no timeout, i.e. no forceful termination is performed.
//
// All hooks are called in the following order:
// - [ContainerLifecycleHooks.PreStops]
// - [ContainerLifecycleHooks.PostStops]
//
// If the container is already stopped, the method is a no-op.
func (c *DockerContainer) Stop(ctx context.Context, timeout *time.Duration) error {
err := c.stoppingHook(ctx)
if err != nil {
Expand Down Expand Up @@ -814,7 +819,16 @@ func (c *DockerContainer) stopLogProduction() error {

c.logProductionWaitGroup.Wait()

return <-c.logProductionError
if err := <-c.logProductionError; err != nil {
if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) {
// Returning context errors is not useful for the consumer.
return nil
}

return err
}

return nil
}

// GetLogProductionErrorChannel exposes the only way for the consumer
Expand Down Expand Up @@ -1086,7 +1100,7 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque
// default hooks include logger hook and pre-create hook
defaultHooks := []ContainerLifecycleHooks{
DefaultLoggingHook(p.Logger),
defaultPreCreateHook(ctx, p, req, dockerInput, hostConfig, networkingConfig),
defaultPreCreateHook(p, dockerInput, hostConfig, networkingConfig),
defaultCopyFileToContainerHook(req.Files),
defaultLogConsumersHook(req.LogConsumerCfg),
defaultReadinessHook(),
Expand Down
13 changes: 7 additions & 6 deletions docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestContainerWithHostNetworkOptions(t *testing.T) {
nginxHighPort,
},
Privileged: true,
WaitingFor: wait.ForListeningPort(nginxHighPort),
WaitingFor: wait.ForHTTP("/").WithPort(nginxHighPort),
HostConfigModifier: func(hc *container.HostConfig) {
hc.NetworkMode = "host"
},
Expand Down Expand Up @@ -182,7 +182,7 @@ func TestContainerWithHostNetwork(t *testing.T) {
ProviderType: providerType,
ContainerRequest: ContainerRequest{
Image: nginxAlpineImage,
WaitingFor: wait.ForListeningPort(nginxHighPort),
WaitingFor: wait.ForHTTP("/").WithPort(nginxHighPort),
Files: []ContainerFile{
{
HostFilePath: absPath,
Expand Down Expand Up @@ -414,6 +414,7 @@ func TestTwoContainersExposingTheSamePort(t *testing.T) {
ExposedPorts: []string{
nginxDefaultPort,
},
WaitingFor: wait.ForHTTP("/").WithPort(nginxDefaultPort),
},
Started: true,
})
Expand All @@ -428,7 +429,7 @@ func TestTwoContainersExposingTheSamePort(t *testing.T) {
ExposedPorts: []string{
nginxDefaultPort,
},
WaitingFor: wait.ForListeningPort(nginxDefaultPort),
WaitingFor: wait.ForHTTP("/").WithPort(nginxDefaultPort),
},
Started: true,
})
Expand Down Expand Up @@ -475,7 +476,7 @@ func TestContainerCreation(t *testing.T) {
ExposedPorts: []string{
nginxDefaultPort,
},
WaitingFor: wait.ForListeningPort(nginxDefaultPort),
WaitingFor: wait.ForHTTP("/").WithPort(nginxDefaultPort),
},
Started: true,
})
Expand Down Expand Up @@ -529,7 +530,7 @@ func TestContainerCreationWithName(t *testing.T) {
ExposedPorts: []string{
nginxDefaultPort,
},
WaitingFor: wait.ForListeningPort(nginxDefaultPort),
WaitingFor: wait.ForHTTP("/").WithPort(nginxDefaultPort),
Name: creationName,
Networks: []string{"bridge"},
},
Expand Down Expand Up @@ -592,7 +593,7 @@ func TestContainerCreationAndWaitForListeningPortLongEnough(t *testing.T) {
ExposedPorts: []string{
nginxDefaultPort,
},
WaitingFor: wait.ForListeningPort(nginxDefaultPort), // default startupTimeout is 60s
WaitingFor: wait.ForHTTP("/").WithPort(nginxDefaultPort), // default startupTimeout is 60s
},
Started: true,
})
Expand Down
21 changes: 21 additions & 0 deletions docs/features/wait/host_port.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ The host-port wait strategy will check if the container is listening to a specif
- alternatively, wait for the lowest exposed port in the container.
- the startup timeout to be used, default is 60 seconds.
- the poll interval to be used, default is 100 milliseconds.
- skip the internal check.

Variations on the HostPort wait strategy are supported, including:

Expand Down Expand Up @@ -39,3 +40,23 @@ req := ContainerRequest{
WaitingFor: wait.ForExposedPort(),
}
```

## Skipping the internal check

_Testcontainers for Go_ checks if the container is listening to the port internally before returning the control to the caller. For that it uses a shell command to check the port status:

<!--codeinclude-->
[Internal check](../../../wait/host_port.go) inside_block:buildInternalCheckCommand
<!--/codeinclude-->

But there are cases where this internal check is not needed, for example when a shell is not available in the container or
when the container doesn't bind the port internally until additional conditions are met.
In this case, the `wait.ForExposedPort.SkipInternalCheck` can be used to skip the internal check.

```golang
req := ContainerRequest{
Image: "docker.io/nginx:alpine",
ExposedPorts: []string{"80/tcp", "9080/tcp"},
WaitingFor: wait.ForExposedPort().SkipInternalCheck(),
}
```
4 changes: 2 additions & 2 deletions internal/core/docker_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func extractDockerSocketFromClient(ctx context.Context, cli client.APIClient) st
return checkDockerSocketFn(tcHost)
}

testcontainersDockerSocket, err := dockerSocketOverridePath(ctx)
testcontainersDockerSocket, err := dockerSocketOverridePath()
if err == nil {
return checkDockerSocketFn(testcontainersDockerSocket)
}
Expand Down Expand Up @@ -220,7 +220,7 @@ func dockerHostFromProperties(ctx context.Context) (string, error) {

// dockerSocketOverridePath returns the docker socket from the TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE environment variable,
// if it's not empty
func dockerSocketOverridePath(ctx context.Context) (string, error) {
func dockerSocketOverridePath() (string, error) {
if dockerHostPath, exists := os.LookupEnv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE"); exists {
return dockerHostPath, nil
}
Expand Down
4 changes: 2 additions & 2 deletions internal/core/docker_host_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func TestExtractDockerHost(t *testing.T) {
err := createTmpDockerSocket(tmpDir)
require.NoError(t, err)

socket, err := dockerSocketOverridePath(context.Background())
socket, err := dockerSocketOverridePath()
require.NoError(t, err)
assert.Equal(t, tmpSocket, socket)
})
Expand All @@ -199,7 +199,7 @@ func TestExtractDockerHost(t *testing.T) {

os.Unsetenv("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE")

socket, err := dockerSocketOverridePath(context.Background())
socket, err := dockerSocketOverridePath()
require.ErrorIs(t, err, ErrDockerSocketOverrideNotSet)
assert.Empty(t, socket)
})
Expand Down
12 changes: 6 additions & 6 deletions lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ var DefaultLoggingHook = func(logger Logging) ContainerLifecycleHooks {
}

// defaultPreCreateHook is a hook that will apply the default configuration to the container
var defaultPreCreateHook = func(ctx context.Context, p *DockerProvider, req ContainerRequest, dockerInput *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig) ContainerLifecycleHooks {
var defaultPreCreateHook = func(p *DockerProvider, dockerInput *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig) ContainerLifecycleHooks {
return ContainerLifecycleHooks{
PreCreates: []ContainerRequestHook{
func(ctx context.Context, req ContainerRequest) error {
Expand Down Expand Up @@ -233,15 +233,15 @@ var defaultReadinessHook = func() ContainerLifecycleHooks {
func(ctx context.Context, c Container) error {
// wait until all the exposed ports are mapped:
// it will be ready when all the exposed ports are mapped,
// checking every 50ms, up to 5s, and failing if all the
// exposed ports are not mapped in that time.
// checking every 50ms, up to 1s, and failing if all the
// exposed ports are not mapped in 5s.
dockerContainer := c.(*DockerContainer)

b := backoff.NewExponentialBackOff()

b.InitialInterval = 50 * time.Millisecond
b.MaxElapsedTime = 1 * time.Second
b.MaxInterval = 5 * time.Second
b.MaxElapsedTime = 5 * time.Second
b.MaxInterval = time.Duration(float64(time.Second) * backoff.DefaultRandomizationFactor)

err := backoff.RetryNotify(
func() error {
Expand Down Expand Up @@ -274,7 +274,7 @@ var defaultReadinessHook = func() ContainerLifecycleHooks {
dockerContainer.ID[:12], dockerContainer.Image, dockerContainer.WaitingFor,
)
if err := dockerContainer.WaitingFor.WaitUntilReady(ctx, c); err != nil {
return err
return fmt.Errorf("wait until ready: %w", err)
}
}

Expand Down
2 changes: 1 addition & 1 deletion logconsumer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ func TestContainerLogWithErrClosed(t *testing.T) {
if errors.Is(err, context.DeadlineExceeded) {
break
}
time.Sleep(100 * time.Microsecond)
time.Sleep(10 * time.Millisecond)
t.Log("retrying get endpoint")
}
if err != nil {
Expand Down
Loading

0 comments on commit ed06f7c

Please sign in to comment.