Merge pull request #98 from docker/feat-port-print

Implement printing published ports

Dockerfile

@@ -29,9 +29,6 @@ WORKDIR ${PWD}
 ADD go.* ${PWD}
 ADD . ${PWD}
 
-FROM golang:${GO_VERSION} AS lint-base
-RUN go get github.com/golangci/golangci-lint/cmd/golangci-lint@v1.26.0
-
 FROM protos-base AS make-protos
 RUN make -f builder.Makefile protos
 
@@ -56,6 +53,3 @@ COPY --from=make-cross /api/bin/* .
 
 FROM base as test
 RUN make -f builder.Makefile test
-
-FROM lint-base AS lint
-RUN make -f builder.Makefile lint

Makefile

@@ -25,6 +25,7 @@
 
 GOOS ?= $(shell go env GOOS)
 GOARCH ?= $(shell go env GOARCH)
+PWD = $(shell pwd)
 
 export DOCKER_BUILDKIT=1
 
@@ -61,8 +62,7 @@ cache-clear: ## Clear the builder cache
 	@docker builder prune --force --filter type=exec.cachemount --filter=unused-for=24h
 
 lint: ## run linter(s)
-	@docker build . \
-	--target lint
+	docker run --rm -t -v $(PWD):/app -w /app golangci/golangci-lint:v1.27-alpine golangci-lint run ./...
 
 help: ## Show help
 	@echo Please specify a build target. The choices are:

azure/backend.go

@@ -133,10 +133,12 @@ func (cs *aciContainerService) List(ctx context.Context) ([]containers.Container
 			if container.InstanceView != nil && container.InstanceView.CurrentState != nil {
 				status = *container.InstanceView.CurrentState.State
 			}
+
 			res = append(res, containers.Container{
 				ID:     containerID,
 				Image:  *container.Image,
 				Status: status,
+				Ports:  convert.ToPorts(group.IPAddress, *container.Ports),
 			})
 		}
 	}

azure/convert/ports.go

@@ -0,0 +1,37 @@
+package convert
+
+import (
+	"strings"
+
+	"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance"
+
+	"github.com/docker/api/containers"
+)
+
+// ToPorts converts Azure container ports to api ports
+func ToPorts(ipAddr *containerinstance.IPAddress, ports []containerinstance.ContainerPort) []containers.Port {
+	var result []containers.Port
+
+	for _, port := range ports {
+		if port.Port == nil {
+			continue
+		}
+		protocol := "tcp"
+		if port.Protocol != "" {
+			protocol = string(port.Protocol)
+		}
+		ip := ""
+		if ipAddr != nil {
+			ip = *ipAddr.IP
+		}
+
+		result = append(result, containers.Port{
+			HostPort:      uint32(*port.Port),
+			ContainerPort: uint32(*port.Port),
+			HostIP:        ip,
+			Protocol:      strings.ToLower(protocol),
+		})
+	}
+
+	return result
+}

azure/convert/ports_test.go

@@ -0,0 +1,89 @@
+package convert
+
+import (
+	"testing"
+
+	"github.com/Azure/azure-sdk-for-go/profiles/latest/containerinstance/mgmt/containerinstance"
+	"github.com/Azure/go-autorest/autorest/to"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/docker/api/containers"
+)
+
+func TestPortConvert(t *testing.T) {
+	expectedPorts := []containers.Port{
+		{
+			HostPort:      80,
+			ContainerPort: 80,
+			HostIP:        "10.0.0.1",
+			Protocol:      "tcp",
+		},
+	}
+	testCases := []struct {
+		name     string
+		ip       *containerinstance.IPAddress
+		ports    []containerinstance.ContainerPort
+		expected []containers.Port
+	}{
+		{
+			name: "convert port",
+			ip: &containerinstance.IPAddress{
+				IP: to.StringPtr("10.0.0.1"),
+			},
+			ports: []containerinstance.ContainerPort{
+				{
+					Protocol: "tcp",
+					Port:     to.Int32Ptr(80),
+				},
+			},
+			expected: expectedPorts,
+		},
+		{
+			name: "with nil ip",
+			ip:   nil,
+			ports: []containerinstance.ContainerPort{
+				{
+					Protocol: "tcp",
+					Port:     to.Int32Ptr(80),
+				},
+			},
+			expected: []containers.Port{
+				{
+					HostPort:      80,
+					ContainerPort: 80,
+					HostIP:        "",
+					Protocol:      "tcp",
+				},
+			},
+		},
+		{
+			name: "skip nil ports",
+			ip:   nil,
+			ports: []containerinstance.ContainerPort{
+				{
+					Protocol: "tcp",
+					Port:     to.Int32Ptr(80),
+				},
+				{
+					Protocol: "tcp",
+					Port:     nil,
+				},
+			},
+			expected: []containers.Port{
+				{
+					HostPort:      80,
+					ContainerPort: 80,
+					HostIP:        "",
+					Protocol:      "tcp",
+				},
+			},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			ports := ToPorts(testCase.ip, testCase.ports)
+			assert.Equal(t, testCase.expected, ports)
+		})
+	}
+}

cli/cmd/ps.go

@@ -6,9 +6,11 @@ import (
 	"os"
 	"text/tabwriter"
 
+	"github.com/docker/docker/pkg/stringid"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 
+	"github.com/docker/api/cli/formatter"
 	"github.com/docker/api/client"
 )
 
@@ -50,11 +52,11 @@ func runPs(ctx context.Context, opts psOpts) error {
 		return nil
 	}
 
-	w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0)
-	fmt.Fprintf(w, "NAME\tIMAGE\tSTATUS\tCOMMAND\n")
-	format := "%s\t%s\t%s\t%s\n"
+	w := tabwriter.NewWriter(os.Stdout, 0, 0, 8, ' ', 0)
+	fmt.Fprintf(w, "CONTAINER ID\tIMAGE\tCOMMAND\tSTATUS\tPORTS\n")
+	format := "%s\t%s\t%s\t%s\t%s\n"
 	for _, c := range containers {
-		fmt.Fprintf(w, format, c.ID, c.Image, c.Status, c.Command)
+		fmt.Fprintf(w, format, stringid.TruncateID(c.ID), c.Image, c.Command, c.Status, formatter.PortsString(c.Ports))
 	}
 
 	return w.Flush()

cli/cmd/run/run.go

@@ -35,12 +35,13 @@ import (
 	"github.com/docker/docker/pkg/namesgenerator"
 	"github.com/spf13/cobra"
 
+	"github.com/docker/api/cli/options/run"
 	"github.com/docker/api/client"
 )
 
 // Command runs a container
 func Command() *cobra.Command {
-	var opts runOpts
+	var opts run.Opts
 	cmd := &cobra.Command{
 		Use:   "run",
 		Short: "Run a container",
@@ -50,19 +51,19 @@ func Command() *cobra.Command {
 		},
 	}
 
-	cmd.Flags().StringArrayVarP(&opts.publish, "publish", "p", []string{}, "Publish a container's port(s). [HOST_PORT:]CONTAINER_PORT")
-	cmd.Flags().StringVar(&opts.name, "name", getRandomName(), "Assign a name to the container")
+	cmd.Flags().StringArrayVarP(&opts.Publish, "publish", "p", []string{}, "Publish a container's port(s). [HOST_PORT:]CONTAINER_PORT")
+	cmd.Flags().StringVar(&opts.Name, "name", getRandomName(), "Assign a name to the container")
 
 	return cmd
 }
 
-func runRun(ctx context.Context, image string, opts runOpts) error {
+func runRun(ctx context.Context, image string, opts run.Opts) error {
 	c, err := client.New(ctx)
 	if err != nil {
 		return err
 	}
 
-	project, err := opts.toContainerConfig(image)
+	project, err := opts.ToContainerConfig(image)
 	if err != nil {
 		return err
 	}
@@ -70,7 +71,8 @@ func runRun(ctx context.Context, image string, opts runOpts) error {
 	if err = c.ContainerService().Run(ctx, project); err != nil {
 		return err
 	}
-	fmt.Println(opts.name)
+	fmt.Println(opts.Name)
+
 	return nil
 
 }

cli/cmd/testdata/ps-out.golden

@@ -1,3 +1,3 @@
-NAME  IMAGE   STATUS  COMMAND
-id    nginx           
-1234  alpine          
+CONTAINER ID        IMAGE         COMMAND        STATUS        PORTS
+id                  nginx                                      
+1234                alpine                                     

cli/formatter/container.go

@@ -0,0 +1,110 @@
+package formatter
+
+import (
+	"fmt"
+	"sort"
+	"strconv"
+	"strings"
+
+	"github.com/docker/api/containers"
+)
+
+type portGroup struct {
+	first uint32
+	last  uint32
+}
+
+// PortsString returns a human readable published ports
+func PortsString(ports []containers.Port) string {
+	groupMap := make(map[string]*portGroup)
+	var (
+		result       []string
+		hostMappings []string
+		groupMapKeys []string
+	)
+
+	sort.Slice(ports, func(i int, j int) bool {
+		return comparePorts(ports[i], ports[j])
+	})
+
+	for _, port := range ports {
+		// Simple case: HOST_IP:PORT1:PORT2
+		hostIP := "0.0.0.0"
+		if port.HostIP != "" {
+			hostIP = port.HostIP
+		}
+
+		if port.HostPort != port.ContainerPort {
+			hostMappings = append(hostMappings, fmt.Sprintf("%s:%d->%d/%s", hostIP, port.HostPort, port.ContainerPort, port.Protocol))
+			continue
+		}
+
+		current := port.ContainerPort
+		portKey := fmt.Sprintf("%s/%s", hostIP, port.Protocol)
+		group := groupMap[portKey]
+
+		if group == nil {
+			groupMap[portKey] = &portGroup{first: current, last: current}
+			// record order that groupMap keys are created
+			groupMapKeys = append(groupMapKeys, portKey)
+			continue
+		}
+
+		if current == (group.last + 1) {
+			group.last = current
+			continue
+		}
+
+		result = append(result, formGroup(portKey, group.first, group.last))
+		groupMap[portKey] = &portGroup{first: current, last: current}
+	}
+
+	for _, portKey := range groupMapKeys {
+		g := groupMap[portKey]
+		result = append(result, formGroup(portKey, g.first, g.last))
+	}
+
+	result = append(result, hostMappings...)
+
+	return strings.Join(result, ", ")
+}
+
+func formGroup(key string, start uint32, last uint32) string {
+	parts := strings.Split(key, "/")
+	protocol := parts[0]
+	var ip string
+	if len(parts) > 1 {
+		ip = parts[0]
+		protocol = parts[1]
+	}
+	group := strconv.Itoa(int(start))
+
+	// add range
+	if start != last {
+		group = fmt.Sprintf("%s-%d", group, last)
+	}
+
+	// add host ip
+	if ip != "" {
+		group = fmt.Sprintf("%s:%s->%s", ip, group, group)
+	}
+
+	// add protocol
+	return fmt.Sprintf("%s/%s", group, protocol)
+}
+
+func comparePorts(i containers.Port, j containers.Port) bool {
+	if i.ContainerPort != j.ContainerPort {
+		return i.ContainerPort < j.ContainerPort
+	}
+
+	if i.HostIP != j.HostIP {
+		return i.HostIP < j.HostIP
+	}
+
+	if i.HostPort != j.HostPort {
+		return i.HostPort < j.HostPort
+	}
+
+	return i.Protocol < j.Protocol
+}

cli/formatter/container_test.go

@@ -0,0 +1,62 @@
+package formatter
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gotest.tools/v3/assert"
+
+	"github.com/docker/api/cli/options/run"
+)
+
+func TestDisplayPorts(t *testing.T) {
+	testCases := []struct {
+		name     string
+		in       []string
+		expected string
+	}{
+		{
+			name:     "simple",
+			in:       []string{"80"},
+			expected: "0.0.0.0:80->80/tcp",
+		},
+		{
+			name:     "different ports",
+			in:       []string{"80:90"},
+			expected: "0.0.0.0:80->90/tcp",
+		},
+		{
+			name:     "host ip",
+			in:       []string{"192.168.0.1:80:90"},
+			expected: "192.168.0.1:80->90/tcp",
+		},
+		{
+			name:     "port range",
+			in:       []string{"80-90:80-90"},
+			expected: "0.0.0.0:80-90->80-90/tcp",
+		},
+		{
+			name:     "grouping",
+			in:       []string{"80:80", "81:81"},
+			expected: "0.0.0.0:80-81->80-81/tcp",
+		},
+		{
+			name:     "groups",
+			in:       []string{"80:80", "82:82"},
+			expected: "0.0.0.0:80->80/tcp, 0.0.0.0:82->82/tcp",
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			runOpts := run.Opts{
+				Publish: testCase.in,
+			}
+			containerConfig, err := runOpts.ToContainerConfig("test")
+			require.Nil(t, err)
+
+			out := PortsString(containerConfig.Ports)
+			assert.Equal(t, testCase.expected, out)
+		})
+	}
+}

cli/options/run/opts.go

@@ -8,13 +8,28 @@ import (
 	"github.com/docker/api/containers"
 )
 
-type runOpts struct {
-	name    string
-	publish []string
+// Opts contain run command options
+type Opts struct {
+	Name    string
+	Publish []string
 }
 
-func toPorts(ports []string) ([]containers.Port, error) {
-	_, bindings, err := nat.ParsePortSpecs(ports)
+// ToContainerConfig convert run options to a container configuration
+func (r *Opts) ToContainerConfig(image string) (containers.ContainerConfig, error) {
+	publish, err := r.toPorts()
+	if err != nil {
+		return containers.ContainerConfig{}, err
+	}
+
+	return containers.ContainerConfig{
+		ID:    r.Name,
+		Image: image,
+		Ports: publish,
+	}, nil
+}
+
+func (r *Opts) toPorts() ([]containers.Port, error) {
+	_, bindings, err := nat.ParsePortSpecs(r.Publish)
 	if err != nil {
 		return nil, err
 	}
@@ -44,16 +59,3 @@ func toPorts(ports []string) ([]containers.Port, error) {
 
 	return result, nil
 }
-
-func (r *runOpts) toContainerConfig(image string) (containers.ContainerConfig, error) {
-	publish, err := toPorts(r.publish)
-	if err != nil {
-		return containers.ContainerConfig{}, err
-	}
-
-	return containers.ContainerConfig{
-		ID:    r.name,
-		Image: image,
-		Ports: publish,
-	}, nil
-}

cli/options/run/opts_test.go

@@ -88,7 +88,10 @@ func (s *RunOptsSuite) TestPortParse() {
 	}
 
 	for _, testCase := range testCases {
-		result, err := toPorts([]string{testCase.in})
+		opts := Opts{
+			Publish: []string{testCase.in},
+		}
+		result, err := opts.toPorts()
 		require.Nil(s.T(), err)
 		assert.ElementsMatch(s.T(), testCase.expected, result)
 	}

go.mod

@@ -7,7 +7,6 @@ require (
 	github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78 // indirect
 	github.com/Azure/go-autorest/autorest v0.10.0
 	github.com/Azure/go-autorest/autorest/adal v0.8.2
-	github.com/Azure/go-autorest/autorest/azure/auth v0.4.2
 	github.com/Azure/go-autorest/autorest/azure/cli v0.3.1
 	github.com/Azure/go-autorest/autorest/date v0.2.0
 	github.com/Azure/go-autorest/autorest/to v0.3.0

go.sum

@@ -3,19 +3,13 @@ github.com/Azure/azure-sdk-for-go v42.0.0+incompatible h1:yz6sFf5bHZ+gEOQVuK5JhP
 github.com/Azure/azure-sdk-for-go v42.0.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
 github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78 h1:w+iIsaOQNcT7OZ575w+acHgRric5iCyQh+xv+KJ4HB8=
 github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=
-github.com/Azure/go-autorest v14.1.0+incompatible h1:qROrS0rWxAXGfFdNOI33we8553d7T8v78jXf/8tjLBM=
 github.com/Azure/go-autorest/autorest v0.9.0/go.mod h1:xyHB1BMZT0cuDHU7I0+g046+BFDTQ8rEZB0s4Yfa6bI=
-github.com/Azure/go-autorest/autorest v0.9.3/go.mod h1:GsRuLYvwzLjjjRoWEIyMUaYq8GNUx2nRB378IPt/1p0=
 github.com/Azure/go-autorest/autorest v0.10.0 h1:mvdtztBqcL8se7MdrUweNieTNi4kfNG6GOJuurQJpuY=
 github.com/Azure/go-autorest/autorest v0.10.0/go.mod h1:/FALq9T/kS7b5J5qsQ+RSTUdAmGFqi0vUdVNNx8q630=
-github.com/Azure/go-autorest/autorest v0.10.1 h1:uaB8A32IZU9YKs9v50+/LWIWTDHJk2vlGzbfd7FfESI=
 github.com/Azure/go-autorest/autorest/adal v0.5.0/go.mod h1:8Z9fGy2MpX0PvDjB1pEgQTmVqjGhiHBW7RJJEciWzS0=
 github.com/Azure/go-autorest/autorest/adal v0.8.0/go.mod h1:Z6vX6WXXuyieHAXwMj0S6HY6e6wcHn37qQMBQlvY3lc=
-github.com/Azure/go-autorest/autorest/adal v0.8.1/go.mod h1:ZjhuQClTqx435SRJ2iMlOxPYt3d2C/T/7TiQCVZSn3Q=
 github.com/Azure/go-autorest/autorest/adal v0.8.2 h1:O1X4oexUxnZCaEUGsvMnr8ZGj8HI37tNezwY4npRqA0=
 github.com/Azure/go-autorest/autorest/adal v0.8.2/go.mod h1:ZjhuQClTqx435SRJ2iMlOxPYt3d2C/T/7TiQCVZSn3Q=
-github.com/Azure/go-autorest/autorest/azure/auth v0.4.2 h1:iM6UAvjR97ZIeR93qTcwpKNMpV+/FTWjwEbuPD495Tk=
-github.com/Azure/go-autorest/autorest/azure/auth v0.4.2/go.mod h1:90gmfKdlmKgfjUpnCEpOJzsUEjrWDSLwHIG73tSXddM=
 github.com/Azure/go-autorest/autorest/azure/cli v0.3.1 h1:LXl088ZQlP0SBppGFsRZonW6hSvwgL5gRByMbvUbx8U=
 github.com/Azure/go-autorest/autorest/azure/cli v0.3.1/go.mod h1:ZG5p860J94/0kI9mNJVoIoLgXcirM2gF5i2kWloofxw=
 github.com/Azure/go-autorest/autorest/date v0.1.0/go.mod h1:plvfp3oPSKwf2DNjlBjWF/7vwR+cUD/ELuzDCXwHUVA=

tests/aci-e2e/e2e-aci.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"log"
+	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/resources/mgmt/resources"
 	"github.com/Azure/go-autorest/autorest/to"
@@ -66,8 +67,7 @@ func main() {
 		Expect(len(Lines(output))).To(Equal(1))
 	})
 
-	var nginxID string
-	It("runs nginx on port 80 (PORT NOT CHECKED YET!!! REMOVE THAT WHEN IMPLEMENTED)", func() {
+	It("runs nginx on port 80", func() {
 		output := NewDockerCommand("run", "nginx", "-p", "80:80", "--name", testContainerName).ExecOrDie()
 		Expect(output).To(Equal(testContainerName + "\n"))
 		output = NewDockerCommand("ps").ExecOrDie()
@@ -75,20 +75,19 @@ func main() {
 		Expect(len(lines)).To(Equal(2))
 
 		containerFields := Columns(lines[1])
-		nginxID = containerFields[0]
 		Expect(containerFields[1]).To(Equal("nginx"))
 		Expect(containerFields[2]).To(Equal("Running"))
-		// exposedIP := containerFields[3]
-		// Expect(exposedIP).To(ContainSubstring(":80->80/TCP"))
+		exposedIP := containerFields[3]
+		Expect(exposedIP).To(ContainSubstring(":80->80/tcp"))
 
-		// url := strings.ReplaceAll(exposedIP, "->80/TCP", "")
-		// output = NewCommand("curl", url).ExecOrDie()
-		// Expect(output).To(ContainSubstring("Welcome to nginx!"))
+		url := strings.ReplaceAll(exposedIP, "->80/tcp", "")
+		output = NewCommand("curl", url).ExecOrDie()
+		Expect(output).To(ContainSubstring("Welcome to nginx!"))
 	})
 
 	It("removes container nginx", func() {
-		output := NewDockerCommand("rm", nginxID).ExecOrDie()
-		Expect(Lines(output)[0]).To(Equal(nginxID))
+		output := NewDockerCommand("rm", testContainerName).ExecOrDie()
+		Expect(Lines(output)[0]).To(Equal(testContainerName))
 	})
 
 	It("deploys a compose app", func() {
@@ -97,25 +96,25 @@ func main() {
 		output := NewDockerCommand("ps").ExecOrDie()
 		Lines := Lines(output)
 		Expect(len(Lines)).To(Equal(4))
+		webChecked := false
+
 		for _, line := range Lines[1:] {
 			Expect(line).To(ContainSubstring("Running"))
-		}
-		/*
-				if strings.Contains(line, "acicompose_web") {
-					webChecked = true
-					containerFields := Columns(line)
-					exposedIP := containerFields[3]
-					Expect(exposedIP).To(ContainSubstring(":80->80/TCP"))
+			if strings.Contains(line, "acidemo_web") {
+				webChecked = true
+				containerFields := Columns(line)
+				exposedIP := containerFields[3]
+				Expect(exposedIP).To(ContainSubstring(":80->80/tcp"))
 
-					url := strings.ReplaceAll(exposedIP, "->80/TCP", "")
-					output = NewCommand("curl", url).ExecOrDie()
-					Expect(output).To(ContainSubstring("Docker Compose demo"))
-					output = NewCommand("curl", url+"/words/noun").ExecOrDie()
-					Expect(output).To(ContainSubstring("\"word\":"))
-				}
+				url := strings.ReplaceAll(exposedIP, "->80/tcp", "")
+				output = NewCommand("curl", url).ExecOrDie()
+				Expect(output).To(ContainSubstring("Docker Compose demo"))
+				output = NewCommand("curl", url+"/words/noun").ExecOrDie()
+				Expect(output).To(ContainSubstring("\"word\":"))
 			}
-			Expect(webChecked).To(BeTrue())
-		*/
+		}
+
+		Expect(webChecked).To(BeTrue())
 	})
 
 	It("get logs from web service", func() {

tests/e2e/e2e.go

@@ -65,7 +65,7 @@ func main() {
 		output := NewDockerCommand("ps").ExecOrDie()
 		lines := Lines(output)
 		Expect(len(lines)).To(Equal(3))
-		Expect(lines[2]).To(ContainSubstring("1234  alpine"))
+		Expect(lines[2]).To(ContainSubstring("1234                alpine"))
 	})
 
 	It("can run quiet ps command", func() {