Skip to content

Commit

Permalink
fix: never cache JSON representation of a container (#2606)
Browse files Browse the repository at this point in the history
* chore: wait for the exposed ports before wait strategies

* chore: do not cache raw representation of a container while inspecting it

* fix: lint

* fix: make test deterministic

Not using fixed ports is good

* fix: consider exposed port has no protocol

* fix: lint
  • Loading branch information
mdelapenya authored Jul 2, 2024
1 parent 79d8da1 commit 59cf064
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 40 deletions.
32 changes: 12 additions & 20 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,10 @@ var createContainerFailDueToNameConflictRegex = regexp.MustCompile("Conflict. Th
// DockerContainer represents a container started using Docker
type DockerContainer struct {
// Container ID from Docker
ID string
WaitingFor wait.Strategy
Image string
ID string
WaitingFor wait.Strategy
Image string
exposedPorts []string // a reference to the container's requested exposed ports. It allows checking they are ready before any wait strategy

isRunning bool
imageWasBuilt bool
Expand All @@ -69,7 +70,6 @@ type DockerContainer struct {
sessionID string
terminationSignal chan bool
consumers []LogConsumer
raw *types.ContainerJSON
logProductionError chan error

// TODO: Remove locking and wait group once the deprecated StartLogProducer and
Expand Down Expand Up @@ -165,12 +165,8 @@ func (c *DockerContainer) Host(ctx context.Context) (string, error) {
return host, nil
}

// Inspect gets the raw container info, caching the result for subsequent calls
// Inspect gets the raw container info
func (c *DockerContainer) Inspect(ctx context.Context) (*types.ContainerJSON, error) {
if c.raw != nil {
return c.raw, nil
}

jsonRaw, err := c.inspectRawContainer(ctx)
if err != nil {
return nil, err
Expand Down Expand Up @@ -277,7 +273,6 @@ func (c *DockerContainer) Stop(ctx context.Context, timeout *time.Duration) erro
defer c.provider.Close()

c.isRunning = false
c.raw = nil // invalidate the cache, as the container representation will change after stopping

err = c.stoppedHook(ctx)
if err != nil {
Expand Down Expand Up @@ -316,7 +311,7 @@ func (c *DockerContainer) Terminate(ctx context.Context) error {

c.sessionID = ""
c.isRunning = false
c.raw = nil // invalidate the cache here too

return errors.Join(errs...)
}

Expand All @@ -328,8 +323,7 @@ func (c *DockerContainer) inspectRawContainer(ctx context.Context) (*types.Conta
return nil, err
}

c.raw = &inspect
return c.raw, nil
return &inspect, nil
}

// Logs will fetch both STDOUT and STDERR from the current container. Returns a
Expand Down Expand Up @@ -407,14 +401,10 @@ func (c *DockerContainer) Name(ctx context.Context) (string, error) {
return inspect.Name, nil
}

// State returns container's running state. This method does not use the cache
// and always fetches the latest state from the Docker daemon.
// State returns container's running state.
func (c *DockerContainer) State(ctx context.Context) (*types.ContainerState, error) {
inspect, err := c.inspectRawContainer(ctx)
if err != nil {
if c.raw != nil {
return c.raw.State, err
}
return nil, err
}
return inspect.State, nil
Expand Down Expand Up @@ -1152,6 +1142,7 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque
imageWasBuilt: req.ShouldBuildImage(),
keepBuiltImage: req.ShouldKeepBuiltImage(),
sessionID: core.SessionID(),
exposedPorts: req.ExposedPorts,
provider: p,
terminationSignal: termSignal,
logger: p.Logger,
Expand Down Expand Up @@ -1253,6 +1244,7 @@ func (p *DockerProvider) ReuseOrCreateContainer(ctx context.Context, req Contain
WaitingFor: req.WaitingFor,
Image: c.Image,
sessionID: sessionID,
exposedPorts: req.ExposedPorts,
provider: p,
terminationSignal: termSignal,
logger: p.Logger,
Expand Down Expand Up @@ -1576,13 +1568,13 @@ func containerFromDockerResponse(ctx context.Context, response types.Container)
ctr.terminationSignal = nil

// populate the raw representation of the container
_, err = ctr.inspectRawContainer(ctx)
jsonRaw, err := ctr.inspectRawContainer(ctx)
if err != nil {
return nil, err
}

// the health status of the container, if any
if health := ctr.raw.State.Health; health != nil {
if health := jsonRaw.State.Health; health != nil {
ctr.healthStatus = health.Status
}

Expand Down
39 changes: 26 additions & 13 deletions docker_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,23 +223,26 @@ func removeImageFromLocalCache(t *testing.T, img string) {
}

func TestBuildContainerFromDockerfileWithDockerAuthConfig(t *testing.T) {
mappedPort := prepareLocalRegistryWithAuth(t)

// using the same credentials as in the Docker Registry
base64 := "dGVzdHVzZXI6dGVzdHBhc3N3b3Jk" // testuser:testpassword
t.Setenv("DOCKER_AUTH_CONFIG", `{
"auths": {
"localhost:5001": { "username": "testuser", "password": "testpassword", "auth": "`+base64+`" }
"localhost:`+mappedPort+`": { "username": "testuser", "password": "testpassword", "auth": "`+base64+`" }
},
"credsStore": "desktop"
}`)

prepareLocalRegistryWithAuth(t)

ctx := context.Background()

req := ContainerRequest{
FromDockerfile: FromDockerfile{
Context: "./testdata",
Dockerfile: "auth.Dockerfile",
BuildArgs: map[string]*string{
"REGISTRY_PORT": &mappedPort,
},
},
AlwaysPullImage: true, // make sure the authentication takes place
ExposedPorts: []string{"6379/tcp"},
Expand All @@ -252,23 +255,26 @@ func TestBuildContainerFromDockerfileWithDockerAuthConfig(t *testing.T) {
}

func TestBuildContainerFromDockerfileShouldFailWithWrongDockerAuthConfig(t *testing.T) {
mappedPort := prepareLocalRegistryWithAuth(t)

// using different credentials than in the Docker Registry
base64 := "Zm9vOmJhcg==" // foo:bar
t.Setenv("DOCKER_AUTH_CONFIG", `{
"auths": {
"localhost:5001": { "username": "foo", "password": "bar", "auth": "`+base64+`" }
"localhost:`+mappedPort+`": { "username": "foo", "password": "bar", "auth": "`+base64+`" }
},
"credsStore": "desktop"
}`)

prepareLocalRegistryWithAuth(t)

ctx := context.Background()

req := ContainerRequest{
FromDockerfile: FromDockerfile{
Context: "./testdata",
Dockerfile: "auth.Dockerfile",
BuildArgs: map[string]*string{
"REGISTRY_PORT": &mappedPort,
},
},
AlwaysPullImage: true, // make sure the authentication takes place
ExposedPorts: []string{"6379/tcp"},
Expand All @@ -281,20 +287,20 @@ func TestBuildContainerFromDockerfileShouldFailWithWrongDockerAuthConfig(t *test
}

func TestCreateContainerFromPrivateRegistry(t *testing.T) {
mappedPort := prepareLocalRegistryWithAuth(t)

// using the same credentials as in the Docker Registry
base64 := "dGVzdHVzZXI6dGVzdHBhc3N3b3Jk" // testuser:testpassword
t.Setenv("DOCKER_AUTH_CONFIG", `{
"auths": {
"localhost:5001": { "username": "testuser", "password": "testpassword", "auth": "`+base64+`" }
"localhost:`+mappedPort+`": { "username": "testuser", "password": "testpassword", "auth": "`+base64+`" }
},
"credsStore": "desktop"
}`)

prepareLocalRegistryWithAuth(t)

ctx := context.Background()
req := ContainerRequest{
Image: "localhost:5001/redis:5.0-alpine",
Image: "localhost:" + mappedPort + "/redis:5.0-alpine",
AlwaysPullImage: true, // make sure the authentication takes place
ExposedPorts: []string{"6379/tcp"},
WaitingFor: wait.ForLog("Ready to accept connections"),
Expand All @@ -308,14 +314,14 @@ func TestCreateContainerFromPrivateRegistry(t *testing.T) {
terminateContainerOnEnd(t, ctx, redisContainer)
}

func prepareLocalRegistryWithAuth(t *testing.T) {
func prepareLocalRegistryWithAuth(t *testing.T) string {
ctx := context.Background()
wd, err := os.Getwd()
require.NoError(t, err)
// copyDirectoryToContainer {
req := ContainerRequest{
Image: "registry:2",
ExposedPorts: []string{"5001:5000/tcp"},
ExposedPorts: []string{"5000/tcp"},
Env: map[string]string{
"REGISTRY_AUTH": "htpasswd",
"REGISTRY_AUTH_HTPASSWD_REALM": "Registry",
Expand Down Expand Up @@ -345,15 +351,22 @@ func prepareLocalRegistryWithAuth(t *testing.T) {
registryC, err := GenericContainer(ctx, genContainerReq)
require.NoError(t, err)

mappedPort, err := registryC.MappedPort(ctx, "5000/tcp")
require.NoError(t, err)

mp := mappedPort.Port()

t.Cleanup(func() {
removeImageFromLocalCache(t, "localhost:5001/redis:5.0-alpine")
removeImageFromLocalCache(t, "localhost:"+mp+"/redis:5.0-alpine")
})
t.Cleanup(func() {
require.NoError(t, registryC.Terminate(context.Background()))
})

_, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)

return mp
}

func prepareRedisImage(ctx context.Context, req ContainerRequest, t *testing.T) (Container, error) {
Expand Down
5 changes: 0 additions & 5 deletions docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1338,11 +1338,6 @@ func TestContainerInspect_RawInspectIsCleanedOnStop(t *testing.T) {
assert.NotEmpty(t, inspect.ID)

require.NoError(t, ctr.Stop(context.Background(), nil))

// type assertion to ensure that the container is a DockerContainer
dc := ctr.(*DockerContainer)

assert.Nil(t, dc.raw)
}

func readHostname(tb testing.TB, containerId string) string {
Expand Down
48 changes: 48 additions & 0 deletions lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (
"fmt"
"io"
"strings"
"time"

"github.com/cenkalti/backoff/v4"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/network"
"github.com/docker/go-connections/nat"
Expand Down Expand Up @@ -200,6 +202,52 @@ var defaultLogConsumersHook = func(cfg *LogConsumerConfig) ContainerLifecycleHoo
var defaultReadinessHook = func() ContainerLifecycleHooks {
return ContainerLifecycleHooks{
PostStarts: []ContainerHook{
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.
dockerContainer := c.(*DockerContainer)

b := backoff.NewExponentialBackOff()

b.InitialInterval = 50 * time.Millisecond
b.MaxElapsedTime = 1 * time.Second
b.MaxInterval = 5 * time.Second

err := backoff.Retry(func() error {
jsonRaw, err := dockerContainer.inspectRawContainer(ctx)
if err != nil {
return err
}

exposedAndMappedPorts := jsonRaw.NetworkSettings.Ports

for _, exposedPort := range dockerContainer.exposedPorts {
portMap := nat.Port(exposedPort)
// having entries in exposedAndMappedPorts, where the key is the exposed port,
// and the value is the mapped port, means that the port has been already mapped.
if _, ok := exposedAndMappedPorts[portMap]; !ok {
// check if the port is mapped with the protocol (default is TCP)
if !strings.Contains(exposedPort, "/") {
portMap = nat.Port(fmt.Sprintf("%s/tcp", exposedPort))
if _, ok := exposedAndMappedPorts[portMap]; !ok {
return fmt.Errorf("port %s is not mapped yet", exposedPort)
}
} else {
return fmt.Errorf("port %s is not mapped yet", exposedPort)
}
}
}

return nil
}, b)
if err != nil {
return fmt.Errorf("all exposed ports, %s, were not mapped in 5s: %w", dockerContainer.exposedPorts, err)
}

return nil
},
// wait for the container to be ready
func(ctx context.Context, c Container) error {
dockerContainer := c.(*DockerContainer)
Expand Down
2 changes: 1 addition & 1 deletion port_forwarding.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const (
// using the SSHD container as a bridge.
HostInternal string = "host.testcontainers.internal"
user string = "root"
sshPort = "22"
sshPort = "22/tcp"
)

// sshPassword is a random password generated for the SSHD container.
Expand Down
4 changes: 3 additions & 1 deletion testdata/auth.Dockerfile
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
FROM localhost:5001/redis:5.0-alpine
ARG REGISTRY_PORT=5001

FROM localhost:${REGISTRY_PORT}/redis:5.0-alpine

0 comments on commit 59cf064

Please sign in to comment.