From d6c8dcf6356a63efb4868b94eb3cc007bce81552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Tue, 7 May 2024 13:15:33 +0200 Subject: [PATCH 1/5] feat: expose JSON representation of a container with Inspect --- container.go | 1 + docker.go | 21 ++++++++++++++++++--- docker_test.go | 23 +++++++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/container.go b/container.go index 2f24f4c38f..38a91e5614 100644 --- a/container.go +++ b/container.go @@ -39,6 +39,7 @@ type Container interface { Endpoint(context.Context, string) (string, error) // get proto://ip:port string for the first exposed port PortEndpoint(context.Context, nat.Port, string) (string, error) // get proto://ip:port string for the given exposed port Host(context.Context) (string, error) // get host where the container port is exposed + Inspect(context.Context) (*types.ContainerJSON, error) // get container info MappedPort(context.Context, nat.Port) (nat.Port, error) // get externally mapped port for a container port Ports(context.Context) (nat.PortMap, error) // get all exposed ports SessionID() string // get session id diff --git a/docker.go b/docker.go index e42767cedc..fc99b49517 100644 --- a/docker.go +++ b/docker.go @@ -161,6 +161,20 @@ func (c *DockerContainer) Host(ctx context.Context) (string, error) { return host, nil } +// Inspect gets the raw container info, caching the result for subsequent calls +func (c *DockerContainer) Inspect(ctx context.Context) (*types.ContainerJSON, error) { + if c.raw != nil { + return c.raw, nil + } + + json, err := c.inspectRawContainer(ctx) + if err != nil { + return nil, err + } + + return json, nil +} + // MappedPort gets externally mapped port for a container port func (c *DockerContainer) MappedPort(ctx context.Context, port nat.Port) (nat.Port, error) { inspect, err := c.inspectContainer(ctx) @@ -260,6 +274,7 @@ 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 { @@ -298,18 +313,18 @@ 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...) } // update container raw info func (c *DockerContainer) inspectRawContainer(ctx context.Context) (*types.ContainerJSON, error) { - defer c.provider.Close() - inspect, err := c.provider.client.ContainerInspect(ctx, c.ID) + inspect, err := c.inspectContainer(ctx) if err != nil { return nil, err } - c.raw = &inspect + c.raw = inspect return c.raw, nil } diff --git a/docker_test.go b/docker_test.go index 77b7021b62..dd5318ec7d 100644 --- a/docker_test.go +++ b/docker_test.go @@ -1320,6 +1320,29 @@ func TestContainerWithCustomHostname(t *testing.T) { } } +func TestContainerInspect_RawInspectIsCleanedOnStop(t *testing.T) { + container, err := GenericContainer(context.Background(), GenericContainerRequest{ + ContainerRequest: ContainerRequest{ + Image: nginxImage, + }, + Started: true, + }) + require.NoError(t, err) + terminateContainerOnEnd(t, context.Background(), container) + + inspect, err := container.Inspect(context.Background()) + require.NoError(t, err) + + assert.NotEmpty(t, inspect.ID) + + container.Stop(context.Background(), nil) + + // type assertion to ensure that the container is a DockerContainer + dc := container.(*DockerContainer) + + assert.Nil(t, dc.raw) +} + func readHostname(tb testing.TB, containerId string) string { containerClient, err := NewDockerClientWithOpts(context.Background()) if err != nil { From c690b3431949cc038e664c13de409a4fc8fa775b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Tue, 7 May 2024 13:50:40 +0200 Subject: [PATCH 2/5] chore: deprecated c.Name and c.Ports They can be used simply calling c.Inspect, without any other calculation --- container.go | 4 +- docker.go | 39 +++---- docker_test.go | 19 ++-- modules/localstack/localstack_test.go | 4 +- wait/exec_test.go | 5 + wait/exit_test.go | 5 + wait/health_test.go | 5 + wait/host_port.go | 5 +- wait/host_port_test.go | 36 ++++--- wait/http.go | 7 +- wait/http_test.go | 148 ++++++++++++++++++-------- wait/nop.go | 5 + wait/wait.go | 3 +- wait/wait_test.go | 13 ++- 14 files changed, 203 insertions(+), 95 deletions(-) diff --git a/container.go b/container.go index 38a91e5614..99af536cd6 100644 --- a/container.go +++ b/container.go @@ -41,7 +41,7 @@ type Container interface { Host(context.Context) (string, error) // get host where the container port is exposed Inspect(context.Context) (*types.ContainerJSON, error) // get container info MappedPort(context.Context, nat.Port) (nat.Port, error) // get externally mapped port for a container port - Ports(context.Context) (nat.PortMap, error) // get all exposed ports + Ports(context.Context) (nat.PortMap, error) // Deprecated: Use c.Inspect(ctx).NetworkSettings.Ports instead SessionID() string // get session id IsRunning() bool Start(context.Context) error // start the container @@ -51,7 +51,7 @@ type Container interface { FollowOutput(LogConsumer) // Deprecated: it will be removed in the next major release StartLogProducer(context.Context, ...LogProductionOption) error // Deprecated: Use the ContainerRequest instead StopLogProducer() error // Deprecated: it will be removed in the next major release - Name(context.Context) (string, error) // get container name + Name(context.Context) (string, error) // Deprecated: Use c.Inspect(ctx).Name instead State(context.Context) (*types.ContainerState, error) // returns container's running state Networks(context.Context) ([]string, error) // get container networks NetworkAliases(context.Context) (map[string][]string, error) // get container network aliases for a network diff --git a/docker.go b/docker.go index fc99b49517..75315ff531 100644 --- a/docker.go +++ b/docker.go @@ -114,11 +114,13 @@ func (c *DockerContainer) IsRunning() bool { // Endpoint gets proto://host:port string for the first exposed port // Will returns just host:port if proto is "" func (c *DockerContainer) Endpoint(ctx context.Context, proto string) (string, error) { - ports, err := c.Ports(ctx) + inspect, err := c.Inspect(ctx) if err != nil { return "", err } + ports := inspect.NetworkSettings.Ports + // get first port var firstPort nat.Port for p := range ports { @@ -177,17 +179,15 @@ func (c *DockerContainer) Inspect(ctx context.Context) (*types.ContainerJSON, er // MappedPort gets externally mapped port for a container port func (c *DockerContainer) MappedPort(ctx context.Context, port nat.Port) (nat.Port, error) { - inspect, err := c.inspectContainer(ctx) + inspect, err := c.Inspect(ctx) if err != nil { return "", err } if inspect.ContainerJSONBase.HostConfig.NetworkMode == "host" { return port, nil } - ports, err := c.Ports(ctx) - if err != nil { - return "", err - } + + ports := inspect.NetworkSettings.Ports for k, p := range ports { if k.Port() != port.Port() { @@ -205,9 +205,10 @@ func (c *DockerContainer) MappedPort(ctx context.Context, port nat.Port) (nat.Po return "", errors.New("port not found") } +// Deprecated: use c.Inspect(ctx).NetworkSettings.Ports instead. // Ports gets the exposed ports for the container. func (c *DockerContainer) Ports(ctx context.Context) (nat.PortMap, error) { - inspect, err := c.inspectContainer(ctx) + inspect, err := c.Inspect(ctx) if err != nil { return nil, err } @@ -319,23 +320,14 @@ func (c *DockerContainer) Terminate(ctx context.Context) error { // update container raw info func (c *DockerContainer) inspectRawContainer(ctx context.Context) (*types.ContainerJSON, error) { - inspect, err := c.inspectContainer(ctx) - if err != nil { - return nil, err - } - - c.raw = inspect - return c.raw, nil -} - -func (c *DockerContainer) inspectContainer(ctx context.Context) (*types.ContainerJSON, error) { defer c.provider.Close() inspect, err := c.provider.client.ContainerInspect(ctx, c.ID) if err != nil { return nil, err } - return &inspect, nil + c.raw = &inspect + return c.raw, nil } // Logs will fetch both STDOUT and STDERR from the current container. Returns a @@ -403,9 +395,10 @@ func (c *DockerContainer) followOutput(consumer LogConsumer) { c.consumers = append(c.consumers, consumer) } +// Deprecated: use c.Inspect(ctx).Name instead. // Name gets the name of the container. func (c *DockerContainer) Name(ctx context.Context) (string, error) { - inspect, err := c.inspectContainer(ctx) + inspect, err := c.Inspect(ctx) if err != nil { return "", err } @@ -426,7 +419,7 @@ func (c *DockerContainer) State(ctx context.Context) (*types.ContainerState, err // Networks gets the names of the networks the container is attached to. func (c *DockerContainer) Networks(ctx context.Context) ([]string, error) { - inspect, err := c.inspectContainer(ctx) + inspect, err := c.Inspect(ctx) if err != nil { return []string{}, err } @@ -444,7 +437,7 @@ func (c *DockerContainer) Networks(ctx context.Context) ([]string, error) { // ContainerIP gets the IP address of the primary network within the container. func (c *DockerContainer) ContainerIP(ctx context.Context) (string, error) { - inspect, err := c.inspectContainer(ctx) + inspect, err := c.Inspect(ctx) if err != nil { return "", err } @@ -467,7 +460,7 @@ func (c *DockerContainer) ContainerIP(ctx context.Context) (string, error) { func (c *DockerContainer) ContainerIPs(ctx context.Context) ([]string, error) { ips := make([]string, 0) - inspect, err := c.inspectContainer(ctx) + inspect, err := c.Inspect(ctx) if err != nil { return nil, err } @@ -482,7 +475,7 @@ func (c *DockerContainer) ContainerIPs(ctx context.Context) ([]string, error) { // NetworkAliases gets the aliases of the container for the networks it is attached to. func (c *DockerContainer) NetworkAliases(ctx context.Context) (map[string][]string, error) { - inspect, err := c.inspectContainer(ctx) + inspect, err := c.Inspect(ctx) if err != nil { return map[string][]string{}, err } diff --git a/docker_test.go b/docker_test.go index dd5318ec7d..943e699a5d 100644 --- a/docker_test.go +++ b/docker_test.go @@ -267,8 +267,8 @@ func TestContainerTerminationResetsState(t *testing.T) { if nginxA.SessionID() != "" { t.Fatal("Internal state must be reset.") } - ports, err := nginxA.Ports(ctx) - if err == nil || ports != nil { + inspect, err := nginxA.Inspect(ctx) + if err == nil || inspect != nil { t.Fatal("expected error from container inspect.") } } @@ -539,10 +539,12 @@ func TestContainerCreationWithName(t *testing.T) { require.NoError(t, err) terminateContainerOnEnd(t, ctx, nginxC) - name, err := nginxC.Name(ctx) + inspect, err := nginxC.Inspect(ctx) if err != nil { t.Fatal(err) } + + name := inspect.Name if name != expectedName { t.Errorf("Expected container name '%s'. Got '%s'.", expectedName, name) } @@ -2007,10 +2009,13 @@ func TestDockerProviderFindContainerByName(t *testing.T) { Started: true, }) require.NoError(t, err) - c1Name, err := c1.Name(ctx) + + c1Inspect, err := c1.Inspect(ctx) require.NoError(t, err) terminateContainerOnEnd(t, ctx, c1) + c1Name := c1Inspect.Name + c2, err := GenericContainer(ctx, GenericContainerRequest{ ProviderType: providerType, ContainerRequest: ContainerRequest{ @@ -2059,8 +2064,10 @@ func TestImageBuiltFromDockerfile_KeepBuiltImage(t *testing.T) { require.NoError(t, err, "create container should not fail") defer func() { _ = c.Terminate(context.Background()) }() // Get the image ID. - containerName, err := c.Name(ctx) - require.NoError(t, err, "get container name should not fail") + containerInspect, err := c.Inspect(ctx) + require.NoError(t, err, "container inspect should not fail") + + containerName := containerInspect.Name containerDetails, err := cli.ContainerInspect(ctx, containerName) require.NoError(t, err, "inspect container should not fail") containerImage := containerDetails.Image diff --git a/modules/localstack/localstack_test.go b/modules/localstack/localstack_test.go index b2e3a39ffd..e8bf32484d 100644 --- a/modules/localstack/localstack_test.go +++ b/modules/localstack/localstack_test.go @@ -127,9 +127,11 @@ func TestRunContainer(t *testing.T) { require.NoError(t, err) assert.NotNil(t, container) - rawPorts, err := container.Ports(ctx) + inspect, err := container.Inspect(ctx) require.NoError(t, err) + rawPorts := inspect.NetworkSettings.Ports + ports := 0 // only one port is exposed among all the ports in the container for _, v := range rawPorts { diff --git a/wait/exec_test.go b/wait/exec_test.go index 132933018f..f6723a317c 100644 --- a/wait/exec_test.go +++ b/wait/exec_test.go @@ -62,6 +62,11 @@ func (st mockExecTarget) Host(_ context.Context) (string, error) { return "", errors.New("not implemented") } +func (st mockExecTarget) Inspect(ctx context.Context) (*types.ContainerJSON, error) { + return nil, errors.New("not implemented") +} + +// Deprecated: use Inspect instead func (st mockExecTarget) Ports(ctx context.Context) (nat.PortMap, error) { return nil, errors.New("not implemented") } diff --git a/wait/exit_test.go b/wait/exit_test.go index df6aec1c4a..137c4f276b 100644 --- a/wait/exit_test.go +++ b/wait/exit_test.go @@ -20,6 +20,11 @@ func (st exitStrategyTarget) Host(ctx context.Context) (string, error) { return "", nil } +func (st exitStrategyTarget) Inspect(ctx context.Context) (*types.ContainerJSON, error) { + return nil, nil +} + +// Deprecated: use Inspect instead func (st exitStrategyTarget) Ports(ctx context.Context) (nat.PortMap, error) { return nil, nil } diff --git a/wait/health_test.go b/wait/health_test.go index 9f76fb7492..e1d67809de 100644 --- a/wait/health_test.go +++ b/wait/health_test.go @@ -21,6 +21,11 @@ func (st healthStrategyTarget) Host(ctx context.Context) (string, error) { return "", nil } +func (st healthStrategyTarget) Inspect(ctx context.Context) (*types.ContainerJSON, error) { + return nil, nil +} + +// Deprecated: use Inspect instead func (st healthStrategyTarget) Ports(ctx context.Context) (nat.PortMap, error) { return nil, nil } diff --git a/wait/host_port.go b/wait/host_port.go index c544825831..fc95492a0d 100644 --- a/wait/host_port.go +++ b/wait/host_port.go @@ -90,10 +90,13 @@ func (hp *HostPortStrategy) WaitUntilReady(ctx context.Context, target StrategyT internalPort := hp.Port if internalPort == "" { var ports nat.PortMap - ports, err = target.Ports(ctx) + inspect, err := target.Inspect(ctx) if err != nil { return err } + + ports = inspect.NetworkSettings.Ports + if len(ports) > 0 { for p := range ports { internalPort = p diff --git a/wait/host_port_test.go b/wait/host_port_test.go index 92212989eb..c31c3dabc9 100644 --- a/wait/host_port_test.go +++ b/wait/host_port_test.go @@ -80,12 +80,18 @@ func TestWaitForExposedPortSucceeds(t *testing.T) { HostImpl: func(_ context.Context) (string, error) { return "localhost", nil }, - PortsImpl: func(_ context.Context) (nat.PortMap, error) { - return nat.PortMap{ - "80": []nat.PortBinding{ - { - HostIP: "0.0.0.0", - HostPort: port.Port(), + InspectImpl: func(_ context.Context) (*types.ContainerJSON, error) { + return &types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + NetworkSettingsBase: types.NetworkSettingsBase{ + Ports: nat.PortMap{ + "80": []nat.PortBinding{ + { + HostIP: "0.0.0.0", + HostPort: port.Port(), + }, + }, + }, }, }, }, nil @@ -500,12 +506,18 @@ func TestHostPortStrategySucceedsGivenShellIsNotInstalled(t *testing.T) { HostImpl: func(_ context.Context) (string, error) { return "localhost", nil }, - PortsImpl: func(_ context.Context) (nat.PortMap, error) { - return nat.PortMap{ - "80": []nat.PortBinding{ - { - HostIP: "0.0.0.0", - HostPort: port.Port(), + InspectImpl: func(_ context.Context) (*types.ContainerJSON, error) { + return &types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + NetworkSettingsBase: types.NetworkSettingsBase{ + Ports: nat.PortMap{ + "80": []nat.PortBinding{ + { + HostIP: "0.0.0.0", + HostPort: port.Port(), + }, + }, + }, }, }, }, nil diff --git a/wait/http.go b/wait/http.go index 89e4f092d2..c93cec59c8 100644 --- a/wait/http.go +++ b/wait/http.go @@ -185,7 +185,12 @@ func (ws *HTTPStrategy) WaitUntilReady(ctx context.Context, target StrategyTarge return err } - ports, err = target.Ports(ctx) + inspect, err := target.Inspect(ctx) + if err != nil { + return err + } + + ports = inspect.NetworkSettings.Ports } } diff --git a/wait/http_test.go b/wait/http_test.go index e9e065ab33..54610f4686 100644 --- a/wait/http_test.go +++ b/wait/http_test.go @@ -491,12 +491,18 @@ func TestHttpStrategyFailsWhileGettingPortDueToOOMKilledContainer(t *testing.T) OOMKilled: true, }, nil }, - PortsImpl: func(ctx context.Context) (nat.PortMap, error) { - return nat.PortMap{ - "8080/tcp": []nat.PortBinding{ - { - HostIP: "127.0.0.1", - HostPort: "49152", + InspectImpl: func(_ context.Context) (*types.ContainerJSON, error) { + return &types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + NetworkSettingsBase: types.NetworkSettingsBase{ + Ports: nat.PortMap{ + "8080/tcp": []nat.PortBinding{ + { + HostIP: "127.0.0.1", + HostPort: "49152", + }, + }, + }, }, }, }, nil @@ -539,12 +545,18 @@ func TestHttpStrategyFailsWhileGettingPortDueToExitedContainer(t *testing.T) { ExitCode: 1, }, nil }, - PortsImpl: func(ctx context.Context) (nat.PortMap, error) { - return nat.PortMap{ - "8080/tcp": []nat.PortBinding{ - { - HostIP: "127.0.0.1", - HostPort: "49152", + InspectImpl: func(_ context.Context) (*types.ContainerJSON, error) { + return &types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + NetworkSettingsBase: types.NetworkSettingsBase{ + Ports: nat.PortMap{ + "8080/tcp": []nat.PortBinding{ + { + HostIP: "127.0.0.1", + HostPort: "49152", + }, + }, + }, }, }, }, nil @@ -586,12 +598,18 @@ func TestHttpStrategyFailsWhileGettingPortDueToUnexpectedContainerStatus(t *test Status: "dead", }, nil }, - PortsImpl: func(ctx context.Context) (nat.PortMap, error) { - return nat.PortMap{ - "8080/tcp": []nat.PortBinding{ - { - HostIP: "127.0.0.1", - HostPort: "49152", + InspectImpl: func(_ context.Context) (*types.ContainerJSON, error) { + return &types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + NetworkSettingsBase: types.NetworkSettingsBase{ + Ports: nat.PortMap{ + "8080/tcp": []nat.PortBinding{ + { + HostIP: "127.0.0.1", + HostPort: "49152", + }, + }, + }, }, }, }, nil @@ -628,12 +646,18 @@ func TestHTTPStrategyFailsWhileRequestSendingDueToOOMKilledContainer(t *testing. OOMKilled: true, }, nil }, - PortsImpl: func(ctx context.Context) (nat.PortMap, error) { - return nat.PortMap{ - "8080/tcp": []nat.PortBinding{ - { - HostIP: "127.0.0.1", - HostPort: "49152", + InspectImpl: func(_ context.Context) (*types.ContainerJSON, error) { + return &types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + NetworkSettingsBase: types.NetworkSettingsBase{ + Ports: nat.PortMap{ + "8080/tcp": []nat.PortBinding{ + { + HostIP: "127.0.0.1", + HostPort: "49152", + }, + }, + }, }, }, }, nil @@ -671,12 +695,18 @@ func TestHttpStrategyFailsWhileRequestSendingDueToExitedContainer(t *testing.T) ExitCode: 1, }, nil }, - PortsImpl: func(ctx context.Context) (nat.PortMap, error) { - return nat.PortMap{ - "8080/tcp": []nat.PortBinding{ - { - HostIP: "127.0.0.1", - HostPort: "49152", + InspectImpl: func(_ context.Context) (*types.ContainerJSON, error) { + return &types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + NetworkSettingsBase: types.NetworkSettingsBase{ + Ports: nat.PortMap{ + "8080/tcp": []nat.PortBinding{ + { + HostIP: "127.0.0.1", + HostPort: "49152", + }, + }, + }, }, }, }, nil @@ -713,12 +743,18 @@ func TestHttpStrategyFailsWhileRequestSendingDueToUnexpectedContainerStatus(t *t Status: "dead", }, nil }, - PortsImpl: func(ctx context.Context) (nat.PortMap, error) { - return nat.PortMap{ - "8080/tcp": []nat.PortBinding{ - { - HostIP: "127.0.0.1", - HostPort: "49152", + InspectImpl: func(_ context.Context) (*types.ContainerJSON, error) { + return &types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + NetworkSettingsBase: types.NetworkSettingsBase{ + Ports: nat.PortMap{ + "8080/tcp": []nat.PortBinding{ + { + HostIP: "127.0.0.1", + HostPort: "49152", + }, + }, + }, }, }, }, nil @@ -761,8 +797,14 @@ func TestHttpStrategyFailsWhileGettingPortDueToNoExposedPorts(t *testing.T) { Running: true, }, nil }, - PortsImpl: func(ctx context.Context) (nat.PortMap, error) { - return nat.PortMap{}, nil + InspectImpl: func(_ context.Context) (*types.ContainerJSON, error) { + return &types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + NetworkSettingsBase: types.NetworkSettingsBase{ + Ports: nat.PortMap{}, + }, + }, + }, nil }, } @@ -802,12 +844,18 @@ func TestHttpStrategyFailsWhileGettingPortDueToOnlyUDPPorts(t *testing.T) { Status: "running", }, nil }, - PortsImpl: func(ctx context.Context) (nat.PortMap, error) { - return nat.PortMap{ - "8080/udp": []nat.PortBinding{ - { - HostIP: "127.0.0.1", - HostPort: "49152", + InspectImpl: func(_ context.Context) (*types.ContainerJSON, error) { + return &types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + NetworkSettingsBase: types.NetworkSettingsBase{ + Ports: nat.PortMap{ + "8080/udp": []nat.PortBinding{ + { + HostIP: "127.0.0.1", + HostPort: "49152", + }, + }, + }, }, }, }, nil @@ -850,9 +898,15 @@ func TestHttpStrategyFailsWhileGettingPortDueToExposedPortNoBindings(t *testing. Status: "running", }, nil }, - PortsImpl: func(ctx context.Context) (nat.PortMap, error) { - return nat.PortMap{ - "8080/tcp": []nat.PortBinding{}, + InspectImpl: func(_ context.Context) (*types.ContainerJSON, error) { + return &types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + NetworkSettingsBase: types.NetworkSettingsBase{ + Ports: nat.PortMap{ + "8080/tcp": []nat.PortBinding{}, + }, + }, + }, }, nil }, } diff --git a/wait/nop.go b/wait/nop.go index e7c86bea5e..7b8e918abd 100644 --- a/wait/nop.go +++ b/wait/nop.go @@ -51,6 +51,11 @@ func (st NopStrategyTarget) Host(_ context.Context) (string, error) { return "", nil } +func (st NopStrategyTarget) Inspect(_ context.Context) (*types.ContainerJSON, error) { + return nil, nil +} + +// Deprecated: use Inspect instead func (st NopStrategyTarget) Ports(_ context.Context) (nat.PortMap, error) { return nil, nil } diff --git a/wait/wait.go b/wait/wait.go index 46a66f7dfa..c9bc0606f6 100644 --- a/wait/wait.go +++ b/wait/wait.go @@ -25,7 +25,8 @@ type StrategyTimeout interface { type StrategyTarget interface { Host(context.Context) (string, error) - Ports(ctx context.Context) (nat.PortMap, error) + Inspect(context.Context) (*types.ContainerJSON, error) + Ports(ctx context.Context) (nat.PortMap, error) // Deprecated: use Inspect instead MappedPort(context.Context, nat.Port) (nat.Port, error) Logs(context.Context) (io.ReadCloser, error) Exec(context.Context, []string, ...exec.ProcessOption) (int, io.Reader, error) diff --git a/wait/wait_test.go b/wait/wait_test.go index 0e07ba87b6..079e64c2b3 100644 --- a/wait/wait_test.go +++ b/wait/wait_test.go @@ -15,6 +15,7 @@ var ErrPortNotFound = errors.New("port not found") type MockStrategyTarget struct { HostImpl func(context.Context) (string, error) + InspectImpl func(context.Context) (*types.ContainerJSON, error) PortsImpl func(context.Context) (nat.PortMap, error) MappedPortImpl func(context.Context, nat.Port) (nat.Port, error) LogsImpl func(context.Context) (io.ReadCloser, error) @@ -26,8 +27,18 @@ func (st MockStrategyTarget) Host(ctx context.Context) (string, error) { return st.HostImpl(ctx) } +func (st MockStrategyTarget) Inspect(ctx context.Context) (*types.ContainerJSON, error) { + return st.InspectImpl(ctx) +} + +// Deprecated: use Inspect instead func (st MockStrategyTarget) Ports(ctx context.Context) (nat.PortMap, error) { - return st.PortsImpl(ctx) + inspect, err := st.InspectImpl(ctx) + if err != nil { + return nil, err + } + + return inspect.NetworkSettings.Ports, nil } func (st MockStrategyTarget) MappedPort(ctx context.Context, port nat.Port) (nat.Port, error) { From f70e659d39b43f89b242379682d880a3224362c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Tue, 7 May 2024 14:09:49 +0200 Subject: [PATCH 3/5] chore: simplify state logic --- docker.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/docker.go b/docker.go index 75315ff531..b23dbf336b 100644 --- a/docker.go +++ b/docker.go @@ -407,11 +407,8 @@ func (c *DockerContainer) Name(ctx context.Context) (string, error) { // State returns container's running state func (c *DockerContainer) State(ctx context.Context) (*types.ContainerState, error) { - inspect, err := c.inspectRawContainer(ctx) + inspect, err := c.Inspect(ctx) if err != nil { - if c.raw != nil { - return c.raw.State, err - } return nil, err } return inspect.State, nil From 6541a6be11b7f8de05f04dfce955653052ec01b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Tue, 7 May 2024 14:22:15 +0200 Subject: [PATCH 4/5] fix: update test --- docker_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker_test.go b/docker_test.go index 943e699a5d..e94ab00b6b 100644 --- a/docker_test.go +++ b/docker_test.go @@ -306,7 +306,7 @@ func TestContainerStateAfterTermination(t *testing.T) { assert.Nil(t, state, "expected nil container inspect.") }) - t.Run("Non-nil State after termination if raw as already set", func(t *testing.T) { + t.Run("Nil State after termination if raw as already set", func(t *testing.T) { ctx := context.Background() nginx, err := createContainerFn(ctx) if err != nil { @@ -327,7 +327,7 @@ func TestContainerStateAfterTermination(t *testing.T) { state, err = nginx.State(ctx) require.Error(t, err, "expected error from container inspect after container termination.") - assert.NotNil(t, state, "unexpected nil container inspect after container termination.") + assert.Nil(t, state, "unexpected nil container inspect after container termination.") }) } From 1078bff10ba957392f4bdbd5fb4c34743a71b6ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Tue, 7 May 2024 16:15:43 +0200 Subject: [PATCH 5/5] fix: do not cache result in State --- docker.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docker.go b/docker.go index b23dbf336b..e188c25df2 100644 --- a/docker.go +++ b/docker.go @@ -405,10 +405,14 @@ func (c *DockerContainer) Name(ctx context.Context) (string, error) { return inspect.Name, nil } -// State returns container's running state +// State returns container's running state. This method does not use the cache +// and always fetches the latest state from the Docker daemon. func (c *DockerContainer) State(ctx context.Context) (*types.ContainerState, error) { - inspect, err := c.Inspect(ctx) + inspect, err := c.inspectRawContainer(ctx) if err != nil { + if c.raw != nil { + return c.raw.State, err + } return nil, err } return inspect.State, nil