Skip to content

Commit

Permalink
port: improve error-handling if port not found (#10039)
Browse files Browse the repository at this point in the history
This method looked slightly incomplete. If the port wasn't found,
it'd return `err`, but that was always `nil`, so we'd print out
`:0`.

Now, we construct a nice error message with the targeted port and
the ones we found.

The `--protocol` flag is also now case-insensitive to prevent any
weirdness/confusion there.

Co-authored-by: Nick Sieger <[email protected]>
Signed-off-by: Milas Bowman <[email protected]>
  • Loading branch information
milas and nicksieger authored Dec 5, 2022
1 parent 6ed9a79 commit 053f20e
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 10 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ build-and-e2e-compose-standalone: build e2e-compose-standalone ## Compile the co

.PHONY: mocks
mocks:
mockgen --version >/dev/null 2>&1 || go install github.com/golang/mock/[email protected]
mockgen -destination pkg/mocks/mock_docker_cli.go -package mocks github.com/docker/cli/cli/command Cli
mockgen -destination pkg/mocks/mock_docker_api.go -package mocks github.com/docker/docker/client APIClient
mockgen -destination pkg/mocks/mock_docker_compose_api.go -package mocks -source=./pkg/api/api.go Service
Expand Down
8 changes: 5 additions & 3 deletions cmd/compose/port.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"strconv"
"strings"

"github.com/spf13/cobra"

Expand All @@ -28,7 +29,7 @@ import (

type portOptions struct {
*projectOptions
port int
port uint16
protocol string
index int
}
Expand All @@ -42,11 +43,12 @@ func portCommand(p *projectOptions, backend api.Service) *cobra.Command {
Short: "Print the public port for a port binding.",
Args: cobra.MinimumNArgs(2),
PreRunE: Adapt(func(ctx context.Context, args []string) error {
port, err := strconv.Atoi(args[1])
port, err := strconv.ParseUint(args[1], 10, 16)
if err != nil {
return err
}
opts.port = port
opts.port = uint16(port)
opts.protocol = strings.ToLower(opts.protocol)
return nil
}),
RunE: Adapt(func(ctx context.Context, args []string) error {
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ type Service interface {
// Events executes the equivalent to a `compose events`
Events(ctx context.Context, projectName string, options EventsOptions) error
// Port executes the equivalent to a `compose port`
Port(ctx context.Context, projectName string, service string, port int, options PortOptions) (string, int, error)
Port(ctx context.Context, projectName string, service string, port uint16, options PortOptions) (string, int, error)
// Images executes the equivalent of a `compose images`
Images(ctx context.Context, projectName string, options ImagesOptions) ([]ImageSummary, error)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type ServiceProxy struct {
UnPauseFn func(ctx context.Context, project string, options PauseOptions) error
TopFn func(ctx context.Context, projectName string, services []string) ([]ContainerProcSummary, error)
EventsFn func(ctx context.Context, project string, options EventsOptions) error
PortFn func(ctx context.Context, project string, service string, port int, options PortOptions) (string, int, error)
PortFn func(ctx context.Context, project string, service string, port uint16, options PortOptions) (string, int, error)
ImagesFn func(ctx context.Context, projectName string, options ImagesOptions) ([]ImageSummary, error)
interceptors []Interceptor
}
Expand Down Expand Up @@ -294,7 +294,7 @@ func (s *ServiceProxy) Events(ctx context.Context, projectName string, options E
}

// Port implements Service interface
func (s *ServiceProxy) Port(ctx context.Context, projectName string, service string, port int, options PortOptions) (string, int, error) {
func (s *ServiceProxy) Port(ctx context.Context, projectName string, service string, port uint16, options PortOptions) (string, int, error) {
if s.PortFn == nil {
return "", 0, ErrNotImplemented
}
Expand Down
20 changes: 17 additions & 3 deletions pkg/compose/port.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"github.com/docker/docker/api/types/filters"
)

func (s *composeService) Port(ctx context.Context, projectName string, service string, port int, options api.PortOptions) (string, int, error) {
func (s *composeService) Port(ctx context.Context, projectName string, service string, port uint16, options api.PortOptions) (string, int, error) {
projectName = strings.ToLower(projectName)
list, err := s.apiClient().ContainerList(ctx, moby.ContainerListOptions{
Filters: filters.NewArgs(
Expand All @@ -44,9 +44,23 @@ func (s *composeService) Port(ctx context.Context, projectName string, service s
}
container := list[0]
for _, p := range container.Ports {
if p.PrivatePort == uint16(port) && p.Type == options.Protocol {
if p.PrivatePort == port && p.Type == options.Protocol {
return p.IP, int(p.PublicPort), nil
}
}
return "", 0, err
return "", 0, portNotFoundError(options.Protocol, port, container)
}

func portNotFoundError(protocol string, port uint16, ctr moby.Container) error {
formatPort := func(protocol string, port uint16) string {
return fmt.Sprintf("%d/%s", port, protocol)
}

var containerPorts []string
for _, p := range ctr.Ports {
containerPorts = append(containerPorts, formatPort(p.Type, p.PublicPort))
}

name := strings.TrimPrefix(ctr.Names[0], "/")
return fmt.Errorf("no port %s for container %s: %s", formatPort(protocol, port), name, strings.Join(containerPorts, ", "))
}
2 changes: 1 addition & 1 deletion pkg/mocks/mock_docker_compose_api.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 053f20e

Please sign in to comment.