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: never cache JSON representation of a container #2606

Merged
merged 7 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -1154,6 +1144,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 @@ -1257,6 +1248,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 @@ -1575,13 +1567,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