diff --git a/cmd/podman/containers/ps.go b/cmd/podman/containers/ps.go index 084c84fbcd..92e3bc725f 100644 --- a/cmd/podman/containers/ps.go +++ b/cmd/podman/containers/ps.go @@ -1,6 +1,7 @@ package containers import ( + "cmp" "errors" "fmt" "os" @@ -491,6 +492,8 @@ func portsToString(ports []types.PortMapping, exposedPorts map[uint16][]string) if len(ports) == 0 && len(exposedPorts) == 0 { return "" } + portMap := make(map[string]struct{}) + sb := &strings.Builder{} for _, port := range ports { hostIP := port.HostIP @@ -501,27 +504,72 @@ func portsToString(ports []types.PortMapping, exposedPorts map[uint16][]string) fmt.Fprintf(sb, "%s:%d-%d->%d-%d/%s, ", hostIP, port.HostPort, port.HostPort+port.Range-1, port.ContainerPort, port.ContainerPort+port.Range-1, port.Protocol) + for i := range port.Range { + portMap[fmt.Sprintf("%d/%s", port.ContainerPort+i, port.Protocol)] = struct{}{} + } } else { fmt.Fprintf(sb, "%s:%d->%d/%s, ", hostIP, port.HostPort, port.ContainerPort, port.Protocol) + portMap[fmt.Sprintf("%d/%s", port.ContainerPort, port.Protocol)] = struct{}{} } } - // iterating a map is not deterministic so let's convert slice first and sort by port to make it deterministic - sortedPorts := make([]uint16, 0, len(exposedPorts)) - for port := range exposedPorts { - sortedPorts = append(sortedPorts, port) + // iterating a map is not deterministic so let's convert slice first and sort by protocol and port to make it deterministic + sortedPorts := make([]exposedPort, 0, len(exposedPorts)) + for port, protocols := range exposedPorts { + for _, proto := range protocols { + sortedPorts = append(sortedPorts, exposedPort{num: port, protocol: proto}) + } } - slices.Sort(sortedPorts) + slices.SortFunc(sortedPorts, func(a, b exposedPort) int { + protoCmp := cmp.Compare(a.protocol, b.protocol) + if protoCmp != 0 { + return protoCmp + } + return cmp.Compare(a.num, b.num) + }) + + var prevPort *exposedPort for _, port := range sortedPorts { - for _, protocol := range exposedPorts[port] { - // exposed ports do not have a host part and are just written as "NUM/PROTO" - fmt.Fprintf(sb, "%d/%s, ", port, protocol) + // only if it was not published already so we do not have duplicates + if _, ok := portMap[fmt.Sprintf("%d/%s", port.num, port.protocol)]; ok { + continue + } + + if prevPort != nil { + // if the prevPort is one below us we know it is a range, do not print it and just increase the range by one + if prevPort.protocol == port.protocol && prevPort.num == port.num-prevPort.portRange-1 { + prevPort.portRange++ + continue + } + // the new port is not a range with the previous one so print it + printExposedPort(prevPort, sb) } + prevPort = &port + } + // do not forget to print the last port + if prevPort != nil { + printExposedPort(prevPort, sb) } display := sb.String() // make sure to trim the last ", " of the string return display[:len(display)-2] } + +type exposedPort struct { + num uint16 + protocol string + // portRange is 0 indexed + portRange uint16 +} + +func printExposedPort(port *exposedPort, sb *strings.Builder) { + // exposed ports do not have a host part and are just written as "NUM[-RANGE]/PROTO" + if port.portRange > 0 { + fmt.Fprintf(sb, "%d-%d/%s, ", port.num, port.num+port.portRange, port.protocol) + } else { + fmt.Fprintf(sb, "%d/%s, ", port.num, port.protocol) + } +} diff --git a/cmd/podman/containers/ps_test.go b/cmd/podman/containers/ps_test.go new file mode 100644 index 0000000000..ad2d1ab663 --- /dev/null +++ b/cmd/podman/containers/ps_test.go @@ -0,0 +1,196 @@ +package containers + +import ( + "testing" + + "github.com/containers/common/libnetwork/types" + "github.com/stretchr/testify/assert" +) + +func Test_portsToString(t *testing.T) { + tests := []struct { + name string + ports []types.PortMapping + exposedPorts map[uint16][]string + want string + }{ + { + name: "no ports", + want: "", + }, + { + name: "no ports empty map/slice", + ports: []types.PortMapping{}, + exposedPorts: map[uint16][]string{}, + want: "", + }, + { + name: "single published port", + ports: []types.PortMapping{ + { + ContainerPort: 80, + HostPort: 8080, + Protocol: "tcp", + Range: 1, + }, + }, + want: "0.0.0.0:8080->80/tcp", + }, + { + name: "published port with host ip", + ports: []types.PortMapping{ + { + ContainerPort: 80, + HostPort: 8080, + HostIP: "127.0.0.1", + Protocol: "tcp", + Range: 1, + }, + }, + want: "127.0.0.1:8080->80/tcp", + }, + { + name: "published port with same exposed port", + ports: []types.PortMapping{ + { + ContainerPort: 80, + HostPort: 8080, + Protocol: "tcp", + Range: 1, + }, + }, + exposedPorts: map[uint16][]string{ + 80: {"tcp"}, + }, + want: "0.0.0.0:8080->80/tcp", + }, + { + name: "published port and exposed port with different protocol", + ports: []types.PortMapping{ + { + ContainerPort: 80, + HostPort: 8080, + Protocol: "tcp", + Range: 1, + }, + }, + exposedPorts: map[uint16][]string{ + 80: {"udp"}, + }, + want: "0.0.0.0:8080->80/tcp, 80/udp", + }, + { + name: "published port range", + ports: []types.PortMapping{ + { + ContainerPort: 80, + HostPort: 8080, + Protocol: "tcp", + Range: 3, + }, + }, + want: "0.0.0.0:8080-8082->80-82/tcp", + }, + { + name: "published port range and exposed port in that range", + ports: []types.PortMapping{ + { + ContainerPort: 80, + HostPort: 8080, + Protocol: "tcp", + Range: 3, + }, + }, + exposedPorts: map[uint16][]string{ + 81: {"tcp"}, + }, + want: "0.0.0.0:8080-8082->80-82/tcp", + }, + { + name: "two published ports", + ports: []types.PortMapping{ + { + ContainerPort: 80, + HostPort: 8080, + Protocol: "tcp", + Range: 3, + }, + { + ContainerPort: 80, + HostPort: 8080, + Protocol: "udp", + Range: 1, + }, + }, + want: "0.0.0.0:8080-8082->80-82/tcp, 0.0.0.0:8080->80/udp", + }, + { + name: "exposed port", + exposedPorts: map[uint16][]string{ + 80: {"tcp"}, + }, + want: "80/tcp", + }, + { + name: "exposed port multiple protocols", + exposedPorts: map[uint16][]string{ + 80: {"tcp", "udp"}, + }, + want: "80/tcp, 80/udp", + }, + { + name: "exposed port range", + exposedPorts: map[uint16][]string{ + 80: {"tcp"}, + 81: {"tcp"}, + 82: {"tcp"}, + }, + want: "80-82/tcp", + }, + { + name: "exposed port range with different protocols", + exposedPorts: map[uint16][]string{ + 80: {"tcp", "udp"}, + 81: {"tcp", "sctp"}, + 82: {"tcp", "udp"}, + }, + want: "81/sctp, 80-82/tcp, 80/udp, 82/udp", + }, + { + name: "multiple exposed port ranges", + exposedPorts: map[uint16][]string{ + 80: {"tcp"}, + 81: {"tcp"}, + 82: {"tcp"}, + // 83 missing to split the range + 84: {"tcp"}, + 85: {"tcp"}, + 86: {"tcp"}, + }, + want: "80-82/tcp, 84-86/tcp", + }, + { + name: "published port range partially overlaps with exposed port range", + ports: []types.PortMapping{ + { + ContainerPort: 80, + HostPort: 8080, + Protocol: "tcp", + Range: 3, + }, + }, + exposedPorts: map[uint16][]string{ + 82: {"tcp"}, + 83: {"tcp"}, + 84: {"tcp"}, + }, + want: "0.0.0.0:8080-8082->80-82/tcp, 83-84/tcp", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := portsToString(tt.ports, tt.exposedPorts) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/test/e2e/run_networking_test.go b/test/e2e/run_networking_test.go index 882f92b8ce..e63f9ad0d5 100644 --- a/test/e2e/run_networking_test.go +++ b/test/e2e/run_networking_test.go @@ -396,12 +396,20 @@ var _ = Describe("Podman run networking", func() { name := "testctr" session := podmanTest.Podman([]string{"create", "-t", "--expose", "80", "-p", "80", "--name", name, ALPINE, "/bin/sh"}) session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) inspectOut := podmanTest.InspectContainer(name) Expect(inspectOut).To(HaveLen(1)) Expect(inspectOut[0].NetworkSettings.Ports).To(HaveLen(1)) Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"]).To(HaveLen(1)) - Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostPort).To(Not(Equal("80"))) + hostPort := inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostPort + Expect(hostPort).To(Not(Equal("80"))) Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0]).To(HaveField("HostIP", "0.0.0.0")) + + session = podmanTest.Podman([]string{"ps", "-a", "--format", "{{.Ports}}"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + // This must use Equal() to ensure we do not see the extra ", 80/tcp" from the exposed port + Expect(session.OutputToString()).To(Equal("0.0.0.0:" + hostPort + "->80/tcp")) }) It("podman run --publish-all with EXPOSE port ranges in Dockerfile", func() {