Skip to content

Commit

Permalink
fix: container.Endpoint and wait.FortHTTP to use lowest internal port
Browse files Browse the repository at this point in the history
Ensure Container.Endpoint returns the lowest numbered aka first port
there is more than one exposed.

Ensure that wait.ForHTTP uses the lowest numbered aka first port
when there is more than one exposed.

Prior to this a random port would be returned leading to unpredictable
results.

Clarify which port is uses by using lowest vs first, which could
otherwise be interpreted as the first one specified.

Fixed: #2640
  • Loading branch information
stevenh committed Jul 17, 2024
1 parent 53b0ddf commit d0b3354
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 45 deletions.
2 changes: 1 addition & 1 deletion container.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type DeprecatedContainer interface {
// Container allows getting info about and controlling a single container instance
type Container interface {
GetContainerID() string // get the container id from the provider
Endpoint(context.Context, string) (string, error) // get proto://ip:port string for the first exposed port
Endpoint(context.Context, string) (string, error) // get proto://ip:port string for the lowest 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
Expand Down
17 changes: 8 additions & 9 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,24 +113,23 @@ func (c *DockerContainer) IsRunning() bool {
return c.isRunning
}

// Endpoint gets proto://host:port string for the first exposed port
// Endpoint gets proto://host:port string for the lowest numbered exposed port
// Will returns just host:port if proto is ""
func (c *DockerContainer) Endpoint(ctx context.Context, proto string) (string, error) {
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 {
firstPort = p
break
// Get lowest numbered bound port.
var lowestPort nat.Port
for port := range inspect.NetworkSettings.Ports {
if lowestPort == "" || port.Int() < lowestPort.Int() {
lowestPort = port
}
}

return c.PortEndpoint(ctx, firstPort, proto)
return c.PortEndpoint(ctx, lowestPort, proto)
}

// PortEndpoint gets proto://host:port string for the given exposed port
Expand Down
10 changes: 5 additions & 5 deletions docs/features/wait/host_port.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
The host-port wait strategy will check if the container is listening to a specific port and allows to set the following conditions:

- a port exposed by the container. The port and protocol to be used, which is represented by a string containing the port number and protocol in the format "80/tcp".
- alternatively, wait for the first exposed port in the container.
- 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.

Expand All @@ -19,9 +19,9 @@ req := ContainerRequest{
}
```

## First exposed port in the container
## Lowest exposed port in the container

The wait strategy will use the first exposed port from the container configuration.
The wait strategy will use the lowest exposed port from the container configuration.

```golang
req := ContainerRequest{
Expand All @@ -30,12 +30,12 @@ req := ContainerRequest{
}
```

Said that, it could be the case that the container request included ports to be exposed. Therefore using `wait.ForExposedPort` will wait for the first exposed port in the request, because the container configuration retrieved from Docker will already include them.
Said that, it could be the case that the container request included ports to be exposed. Therefore using `wait.ForExposedPort` will wait for the lowest exposed port in the request, because the container configuration retrieved from Docker will already include them.

```golang
req := ContainerRequest{
Image: "docker.io/nginx:alpine",
ExposedPorts: []string{"80/tcp", "9080/tcp"},
WaitingFor: wait.ForExposedPort(),
}
```
```
2 changes: 1 addition & 1 deletion docs/features/wait/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

The HTTP wait strategy will check the result of an HTTP(S) request against the container and allows to set the following conditions:

- the port to be used. If no port is passed, it will use the first exposed port in the image.
- the port to be used. If no port is passed, it will use the lowest exposed port in the image.
- the path to be used.
- the HTTP method to be used.
- the HTTP request body to be sent.
Expand Down
10 changes: 3 additions & 7 deletions wait/host_port.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,18 +89,14 @@ func (hp *HostPortStrategy) WaitUntilReady(ctx context.Context, target StrategyT

internalPort := hp.Port
if internalPort == "" {
var ports nat.PortMap
inspect, err := target.Inspect(ctx)
if err != nil {
return err
}

ports = inspect.NetworkSettings.Ports

if len(ports) > 0 {
for p := range ports {
internalPort = p
break
for port := range inspect.NetworkSettings.Ports {
if internalPort == "" || port.Int() < internalPort.Int() {
internalPort = port
}
}
}
Expand Down
51 changes: 29 additions & 22 deletions wait/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ func (ws *HTTPStrategy) WithStartupTimeout(timeout time.Duration) *HTTPStrategy
return ws
}

// WithPort set the port to wait for.
// Default is the lowest numbered port.
func (ws *HTTPStrategy) WithPort(port nat.Port) *HTTPStrategy {
ws.Port = port
return ws
Expand Down Expand Up @@ -173,38 +175,43 @@ func (ws *HTTPStrategy) WaitUntilReady(ctx context.Context, target StrategyTarge

var mappedPort nat.Port
if ws.Port == "" {
var err error
var ports nat.PortMap
// we wait one polling interval before we grab the ports otherwise they might not be bound yet on startup
for err != nil || ports == nil {
select {
case <-ctx.Done():
return fmt.Errorf("%w: %w", ctx.Err(), err)
case <-time.After(ws.PollInterval):
if err := checkTarget(ctx, target); err != nil {
return err
}
// We wait one polling interval before we grab the ports
// otherwise they might not be bound yet on startup.
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(ws.PollInterval):
// Port should now be bound so just continue.
}

inspect, err := target.Inspect(ctx)
if err != nil {
return err
}
if err := checkTarget(ctx, target); err != nil {
return err
}

ports = inspect.NetworkSettings.Ports
}
inspect, err := target.Inspect(ctx)
if err != nil {
return err
}

for k, bindings := range ports {
if len(bindings) == 0 || k.Proto() != "tcp" {
// Find the lowest numbered exposed tcp port.
var lowestPort nat.Port
var hostPort string
for port, bindings := range inspect.NetworkSettings.Ports {
if len(bindings) == 0 || port.Proto() != "tcp" {
continue
}
mappedPort, _ = nat.NewPort(k.Proto(), bindings[0].HostPort)
break

if lowestPort == "" || port.Int() < lowestPort.Int() {
lowestPort = port
hostPort = bindings[0].HostPort
}
}

if mappedPort == "" {
if lowestPort == "" {
return errors.New("No exposed tcp ports or mapped ports - cannot wait for status")
}

mappedPort, _ = nat.NewPort(lowestPort.Proto(), hostPort)
} else {
mappedPort, err = target.MappedPort(ctx, ws.Port)

Expand Down

0 comments on commit d0b3354

Please sign in to comment.