Skip to content

Commit 3fd2a02

Browse files
yroblataskbotCopilot
authored
feat: select random host port for containers (#865)
Co-authored-by: taskbot <[email protected]> Co-authored-by: Copilot <[email protected]>
1 parent 18956ca commit 3fd2a02

File tree

4 files changed

+64
-35
lines changed

4 files changed

+64
-35
lines changed

pkg/container/docker/client.go

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,40 @@ func (c *Client) createIngressContainer(ctx context.Context, containerName strin
362362

363363
}
364364

365+
func extractFirstPort(options *runtime.DeployWorkloadOptions) (int, error) {
366+
var firstPort string
367+
if len(options.ExposedPorts) == 0 {
368+
return 0, fmt.Errorf("no exposed ports specified in options.ExposedPorts")
369+
}
370+
for port := range options.ExposedPorts {
371+
firstPort = port
372+
373+
// need to strip the protocol
374+
firstPort = strings.Split(firstPort, "/")[0]
375+
break // take only the first one
376+
}
377+
firstPortInt, err := strconv.Atoi(firstPort)
378+
if err != nil {
379+
return 0, fmt.Errorf("failed to convert port %s to int: %v", firstPort, err)
380+
}
381+
return firstPortInt, nil
382+
}
383+
384+
func extractFirstPortBinding(portBindings map[string][]runtime.PortBinding) (int, error) {
385+
var firstHostPort string
386+
for _, bindings := range portBindings {
387+
if len(bindings) > 0 {
388+
firstHostPort = bindings[0].HostPort
389+
break // we only want the first item in the map
390+
}
391+
}
392+
hostPortInt, err := strconv.Atoi(firstHostPort)
393+
if err != nil {
394+
return 0, fmt.Errorf("failed to convert host port %s to int: %v", firstHostPort, err)
395+
}
396+
return hostPortInt, nil
397+
}
398+
365399
// DeployWorkload creates and starts a workload.
366400
// It configures the workload based on the provided permission profile and transport type.
367401
// If options is nil, default options will be used.
@@ -465,38 +499,24 @@ func (c *Client) DeployWorkload(
465499
return containerId, 0, nil
466500
}
467501

468-
firstPortInt, err := extractFirstPort(options)
502+
// extract the first port binding
503+
firstHostPort, err := extractFirstPortBinding(options.PortBindings)
469504
if err != nil {
470-
return "", 0, err // extractFirstPort already wraps the error with context.
505+
return "", 0, err
471506
}
472-
473507
if isolateNetwork {
474-
firstPortInt, err = c.createIngressContainer(ctx, name, firstPortInt, attachStdio, externalEndpointsConfig)
508+
// just extract the first exposed port
509+
firstPortInt, err := extractFirstPort(options)
510+
if err != nil {
511+
return "", 0, err // extractFirstPort already wraps the error with context.
512+
}
513+
firstHostPort, err = c.createIngressContainer(ctx, name, firstPortInt, attachStdio, externalEndpointsConfig)
475514
if err != nil {
476515
return "", 0, fmt.Errorf("failed to create ingress container: %v", err)
477516
}
478517
}
479518

480-
return containerId, firstPortInt, nil
481-
}
482-
483-
func extractFirstPort(options *runtime.DeployWorkloadOptions) (int, error) {
484-
var firstPort string
485-
if len(options.ExposedPorts) == 0 {
486-
return 0, fmt.Errorf("no exposed ports specified in options.ExposedPorts")
487-
}
488-
for port := range options.ExposedPorts {
489-
firstPort = port
490-
491-
// need to strip the protocol
492-
firstPort = strings.Split(firstPort, "/")[0]
493-
break // take only the first one
494-
}
495-
firstPortInt, err := strconv.Atoi(firstPort)
496-
if err != nil {
497-
return 0, fmt.Errorf("failed to convert port %s to int: %v", firstPort, err)
498-
}
499-
return firstPortInt, nil
519+
return containerId, firstHostPort, nil
500520
}
501521

502522
// ListWorkloads lists workloads

pkg/networking/port.go

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const (
2323
// IsAvailable checks if a port is available
2424
func IsAvailable(port int) bool {
2525
// Check TCP
26-
tcpAddr, err := net.ResolveTCPAddr("tcp", fmt.Sprintf(":%d", port))
26+
tcpAddr, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("127.0.0.1:%d", port))
2727
if err != nil {
2828
return false
2929
}
@@ -38,7 +38,7 @@ func IsAvailable(port int) bool {
3838
}
3939

4040
// Check UDP
41-
udpAddr, err := net.ResolveUDPAddr("udp", fmt.Sprintf(":%d", port))
41+
udpAddr, err := net.ResolveUDPAddr("udp", fmt.Sprintf("127.0.0.1:%d", port))
4242
if err != nil {
4343
return false
4444
}
@@ -128,11 +128,16 @@ func FindOrUsePort(port int) (int, error) {
128128
return 0, fmt.Errorf("could not find an available port")
129129
}
130130
return port, nil
131-
} else if port > 0 && !IsAvailable(port) {
132-
// Check if the provided port is available
133-
return 0, fmt.Errorf("port %d is already in use", port)
134131
}
135132

136-
// Port is available
137-
return port, nil
133+
if IsAvailable(port) {
134+
return port, nil
135+
}
136+
137+
// Requested port is busy — find an alternative
138+
alt := FindAvailable()
139+
if alt == 0 {
140+
return 0, fmt.Errorf("failed to find an alternative port after requested port %d was unavailable", port)
141+
}
142+
return alt, nil
138143
}

pkg/runner/config.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,6 @@ func (c *RunConfig) WithPorts(port, targetPort int) (*RunConfig, error) {
321321
if err != nil {
322322
return c, err
323323
}
324-
logger.Infof("Using host port: %d", selectedPort)
325324
c.Port = selectedPort
326325

327326
// Select a target port for the container if using SSE or Streamable HTTP transport

pkg/transport/http.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/stacklok/toolhive/pkg/container"
1010
rt "github.com/stacklok/toolhive/pkg/container/runtime"
1111
"github.com/stacklok/toolhive/pkg/logger"
12+
"github.com/stacklok/toolhive/pkg/networking"
1213
"github.com/stacklok/toolhive/pkg/permissions"
1314
"github.com/stacklok/toolhive/pkg/transport/errors"
1415
"github.com/stacklok/toolhive/pkg/transport/proxy/transparent"
@@ -129,11 +130,17 @@ func (t *HTTPTransport) Setup(ctx context.Context, runtime rt.Runtime, container
129130
containerPortStr := fmt.Sprintf("%d/tcp", t.targetPort)
130131
containerOptions.ExposedPorts[containerPortStr] = struct{}{}
131132

133+
// bind to a random host port
132134
// Create host port bindings (configurable through the --host flag)
135+
hostPort := networking.FindAvailable()
136+
if hostPort == 0 {
137+
return fmt.Errorf("could not find an available port")
138+
}
139+
133140
portBindings := []rt.PortBinding{
134141
{
135142
HostIP: t.host,
136-
HostPort: fmt.Sprintf("%d", t.targetPort),
143+
HostPort: fmt.Sprintf("%d", hostPort),
137144
},
138145
}
139146

@@ -148,8 +155,6 @@ func (t *HTTPTransport) Setup(ctx context.Context, runtime rt.Runtime, container
148155
// Set the port bindings
149156
containerOptions.PortBindings[containerPortStr] = portBindings
150157

151-
logger.Infof("Exposing container port %d", t.targetPort)
152-
153158
// For SSE transport, we don't need to attach stdio
154159
containerOptions.AttachStdio = false
155160

0 commit comments

Comments
 (0)