From f63bbca4dabbedbd7a808600b38f4e2ae55a6c4a Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Wed, 7 Oct 2020 17:00:15 +0200 Subject: Fix ps port output When defining multiple ports (same src/dst) like `-p 80:80 -p 443:443` then podman will not show the complete output on `podman ps` (only `0.0.0.0:80->80/tcp` in the example). This also applies to port ranges. This patch refactors the port loop by pre-checking for ranges and displaying them correctly to the end user. Signed-off-by: Sascha Grunert --- cmd/podman/containers/ps.go | 160 +++++++++++++++++++++++++++++++++----------- test/e2e/ps_test.go | 33 +++++++-- 2 files changed, 150 insertions(+), 43 deletions(-) diff --git a/cmd/podman/containers/ps.go b/cmd/podman/containers/ps.go index 41d309f51..446b46471 100644 --- a/cmd/podman/containers/ps.go +++ b/cmd/podman/containers/ps.go @@ -371,12 +371,6 @@ func (l psReporter) CreatedHuman() string { // portsToString converts the ports used to a string of the from "port1, port2" // and also groups a continuous list of ports into a readable format. func portsToString(ports []ocicni.PortMapping) string { - type portGroup struct { - first int32 - last int32 - } - portDisplay := []string{} - if len(ports) == 0 { return "" } @@ -385,41 +379,124 @@ func portsToString(ports []ocicni.PortMapping) string { return comparePorts(ports[i], ports[j]) }) - // portGroupMap is used for grouping continuous ports. - portGroupMap := make(map[string]*portGroup) - var groupKeyList []string + portGroups := [][]ocicni.PortMapping{} + currentGroup := []ocicni.PortMapping{} + for i, v := range ports { + var prevPort, nextPort *int32 + if i > 0 { + prevPort = &ports[i-1].ContainerPort + } + if i+1 < len(ports) { + nextPort = &ports[i+1].ContainerPort + } - for _, v := range ports { + port := v.ContainerPort - hostIP := v.HostIP - if hostIP == "" { - hostIP = "0.0.0.0" + // Helper functions + addToCurrentGroup := func(x ocicni.PortMapping) { + currentGroup = append(currentGroup, x) } - // If hostPort and containerPort are not same, consider as individual port. - if v.ContainerPort != v.HostPort { - portDisplay = append(portDisplay, fmt.Sprintf("%s:%d->%d/%s", hostIP, v.HostPort, v.ContainerPort, v.Protocol)) - continue + + addToPortGroup := func(x ocicni.PortMapping) { + portGroups = append(portGroups, []ocicni.PortMapping{x}) + } + + finishCurrentGroup := func() { + portGroups = append(portGroups, currentGroup) + currentGroup = []ocicni.PortMapping{} } - portMapKey := fmt.Sprintf("%s/%s", hostIP, v.Protocol) + // Single entry slice + if prevPort == nil && nextPort == nil { + addToPortGroup(v) + } + + // Start of the slice with len > 0 + if prevPort == nil && nextPort != nil { + isGroup := *nextPort-1 == port + + if isGroup { + // Start with a group + addToCurrentGroup(v) + } else { + // Start with single item + addToPortGroup(v) + } - portgroup, ok := portGroupMap[portMapKey] - if !ok { - portGroupMap[portMapKey] = &portGroup{first: v.ContainerPort, last: v.ContainerPort} - // This list is required to traverse portGroupMap. - groupKeyList = append(groupKeyList, portMapKey) continue } - if portgroup.last == (v.ContainerPort - 1) { - portgroup.last = v.ContainerPort + // Middle of the slice with len > 0 + if prevPort != nil && nextPort != nil { + currentIsGroup := *prevPort+1 == port + nextIsGroup := *nextPort-1 == port + + if currentIsGroup { + // Maybe in the middle of a group + addToCurrentGroup(v) + + if !nextIsGroup { + // End of a group + finishCurrentGroup() + } + } else if nextIsGroup { + // Start of a new group + addToCurrentGroup(v) + } else { + // No group at all + addToPortGroup(v) + } + continue } + + // End of the slice with len > 0 + if prevPort != nil && nextPort == nil { + isGroup := *prevPort+1 == port + + if isGroup { + // End group + addToCurrentGroup(v) + finishCurrentGroup() + } else { + // End single item + addToPortGroup(v) + } + } } - // For each portMapKey, format group list and append to output string. - for _, portKey := range groupKeyList { - group := portGroupMap[portKey] - portDisplay = append(portDisplay, formatGroup(portKey, group.first, group.last)) + + portDisplay := []string{} + for _, group := range portGroups { + if len(group) == 0 { + // Usually should not happen, but better do not crash. + continue + } + + first := group[0] + + hostIP := first.HostIP + if hostIP == "" { + hostIP = "0.0.0.0" + } + + // Single mappings + if len(group) == 1 { + portDisplay = append(portDisplay, + fmt.Sprintf( + "%s:%d->%d/%s", + hostIP, first.HostPort, first.ContainerPort, first.Protocol, + ), + ) + continue + } + + // Group mappings + last := group[len(group)-1] + portDisplay = append(portDisplay, formatGroup( + fmt.Sprintf("%s/%s", hostIP, first.Protocol), + first.HostPort, last.HostPort, + first.ContainerPort, last.ContainerPort, + )) } return strings.Join(portDisplay, ", ") } @@ -440,9 +517,10 @@ func comparePorts(i, j ocicni.PortMapping) bool { return i.Protocol < j.Protocol } -// formatGroup returns the group as startPort:lastPort/Proto> -// e.g 0.0.0.0:1000-1006->1000-1006/tcp. -func formatGroup(key string, start, last int32) string { +// formatGroup returns the group in the format: +// firstCtr:lastCtr/Proto> +// e.g 0.0.0.0:1000-1006->2000-2006/tcp. +func formatGroup(key string, firstHost, lastHost, firstCtr, lastCtr int32) string { parts := strings.Split(key, "/") groupType := parts[0] var ip string @@ -450,12 +528,16 @@ func formatGroup(key string, start, last int32) string { ip = parts[0] groupType = parts[1] } - group := strconv.Itoa(int(start)) - if start != last { - group = fmt.Sprintf("%s-%d", group, last) - } - if ip != "" { - group = fmt.Sprintf("%s:%s->%s", ip, group, group) + + group := func(first, last int32) string { + group := strconv.Itoa(int(first)) + if first != last { + group = fmt.Sprintf("%s-%d", group, last) + } + return group } - return fmt.Sprintf("%s/%s", group, groupType) + hostGroup := group(firstHost, lastHost) + ctrGroup := group(firstCtr, lastCtr) + + return fmt.Sprintf("%s:%s->%s/%s", ip, hostGroup, ctrGroup, groupType) } diff --git a/test/e2e/ps_test.go b/test/e2e/ps_test.go index 48ef566ce..c65738993 100644 --- a/test/e2e/ps_test.go +++ b/test/e2e/ps_test.go @@ -411,18 +411,43 @@ var _ = Describe("Podman ps", func() { Expect(output).To(ContainSubstring(podName)) }) - It("podman ps test with port range", func() { - session := podmanTest.RunTopContainer("") + It("podman ps test with single port range", func() { + session := podmanTest.Podman([]string{"run", "-dt", "-p", "2000-2006:2000-2006", ALPINE, "top"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) - session = podmanTest.Podman([]string{"run", "-dt", "-p", "2000-2006:2000-2006", ALPINE, "top"}) + session = podmanTest.Podman([]string{"ps", "--format", "{{.Ports}}"}) + session.WaitWithDefaultTimeout() + Expect(session.OutputToString()).To(ContainSubstring("0.0.0.0:2000-2006")) + }) + + It("podman ps test with invalid port range", func() { + session := podmanTest.Podman([]string{ + "run", "-p", "1000-2000:2000-3000", "-p", "1999-2999:3001-4001", ALPINE, + }) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(125)) + Expect(session.ErrorToString()).To(ContainSubstring("conflicting port mappings for host port 1999")) + }) + + It("podman ps test with multiple port range", func() { + session := podmanTest.Podman([]string{ + "run", "-dt", + "-p", "3000-3001:3000-3001", + "-p", "3100-3102:4000-4002", + "-p", "30080:30080", + "-p", "30443:30443", + "-p", "8000:8080", + ALPINE, "top"}, + ) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) session = podmanTest.Podman([]string{"ps", "--format", "{{.Ports}}"}) session.WaitWithDefaultTimeout() - Expect(session.OutputToString()).To(ContainSubstring("0.0.0.0:2000-2006")) + Expect(session.OutputToString()).To(ContainSubstring( + "0.0.0.0:3000-3001->3000-3001/tcp, 0.0.0.0:3100-3102->4000-4002/tcp, 0.0.0.0:8000->8080/tcp, 0.0.0.0:30080->30080/tcp, 0.0.0.0:30443->30443/tcp", + )) }) It("podman ps sync flag", func() { -- cgit v1.2.3-54-g00ecf