From a8092a105b493f3c7f9ee063aa2ca035bc7f9be8 Mon Sep 17 00:00:00 2001
From: baude <bbaude@redhat.com>
Date: Tue, 6 Feb 2018 19:52:42 -0600
Subject: Rework port code
Rework port code for generalized clean up and to address
issue #269 where additional portbindings between host
and containers we being introduced by error.
Signed-off-by: baude <bbaude@redhat.com>
Closes: #308
Approved by: mheon
---
cmd/podman/create.go | 123 ++++++++++++++++++++--------------------
cmd/podman/spec.go | 13 +----
test/e2e/run_networking_test.go | 2 +-
3 files changed, 62 insertions(+), 76 deletions(-)
diff --git a/cmd/podman/create.go b/cmd/podman/create.go
index b3aa42b8a..340c036cc 100644
--- a/cmd/podman/create.go
+++ b/cmd/podman/create.go
@@ -311,72 +311,69 @@ func isPortInImagePorts(exposedPorts map[string]struct{}, port string) bool {
return false
}
-func exposedPorts(c *cli.Context, imageExposedPorts map[string]struct{}) (map[nat.Port]struct{}, map[nat.Port][]nat.PortBinding, error) {
- var exposedPorts []string
- var ports map[nat.Port]struct{}
- ports = make(map[nat.Port]struct{})
- _, portBindings, err := nat.ParsePortSpecs(c.StringSlice("publish"))
- if err != nil {
- return nil, nil, err
- }
+func exposedPorts(c *cli.Context, imageExposedPorts map[string]struct{}) (map[nat.Port][]nat.PortBinding, error) {
+ containerPorts := make(map[string]string)
- // Parse the ports from the image itself
- for i := range imageExposedPorts {
- fields := strings.Split(i, "/")
- if len(fields) > 2 {
- return nil, nil, errors.Errorf("invalid exposed port format in image")
- }
- exposedPorts = append(exposedPorts, fields[0])
+ // add expose ports from the image itself
+ for expose := range imageExposedPorts {
+ _, port := nat.SplitProtoPort(expose)
+ containerPorts[port] = ""
}
- // Add the ports from the image to the ports from the user
- exposedPorts = append(exposedPorts, c.StringSlice("expose")...)
- for _, e := range exposedPorts {
- // Merge in exposed ports to the map of published ports
- if strings.Contains(e, ":") {
- return nil, nil, fmt.Errorf("invalid port format for --expose: %s", e)
- }
+ // add the expose ports from the user (--expose)
+ // can be single or a range
+ for _, expose := range c.StringSlice("expose") {
//support two formats for expose, original format <portnum>/[<proto>] or <startport-endport>/[<proto>]
- proto, port := nat.SplitProtoPort(e)
+ _, port := nat.SplitProtoPort(expose)
//parse the start and end port and create a sequence of ports to expose
//if expose a port, the start and end port are the same
start, end, err := nat.ParsePortRange(port)
if err != nil {
- return nil, nil, fmt.Errorf("invalid range format for --expose: %s, error: %s", e, err)
+ return nil, fmt.Errorf("invalid range format for --expose: %s, error: %s", expose, err)
}
for i := start; i <= end; i++ {
- p, err := nat.NewPort(proto, strconv.FormatUint(i, 10))
+ containerPorts[strconv.Itoa(int(i))] = ""
+ }
+ }
+
+ // parse user input'd port bindings
+ pbPorts, portBindings, err := nat.ParsePortSpecs(c.StringSlice("publish"))
+ if err != nil {
+ return nil, err
+ }
+
+ // delete exposed container ports if being used by -p
+ for i := range pbPorts {
+ delete(containerPorts, i.Port())
+ }
+
+ // iterate container ports and make port bindings from them
+ if c.Bool("publish-all") {
+ for e := range containerPorts {
+ //support two formats for expose, original format <portnum>/[<proto>] or <startport-endport>/[<proto>]
+ //proto, port := nat.SplitProtoPort(e)
+ p, err := nat.NewPort("tcp", e)
if err != nil {
- return nil, nil, err
+ return nil, err
}
- // check if the port in question is already being used
- if isPortInPortBindings(portBindings, p) && !isPortInImagePorts(imageExposedPorts, p.Port()) {
- return nil, nil, errors.Errorf("host port %s already used in --publish option", p.Port())
+ l, err := net.Listen("tcp", ":0")
+ if err != nil {
+ return nil, errors.Wrapf(err, "unable to get free port")
}
-
- if c.Bool("publish-all") {
- l, err := net.Listen("tcp", ":0")
- if err != nil {
- return nil, nil, errors.Wrapf(err, "unable to get free port")
- }
- _, randomPort, err := net.SplitHostPort(l.Addr().String())
- if err != nil {
- return nil, nil, errors.Wrapf(err, "unable to determine free port")
- }
- rp, err := strconv.Atoi(randomPort)
- if err != nil {
- return nil, nil, errors.Wrapf(err, "unable to convert random port to int")
- }
- logrus.Debug(fmt.Sprintf("Using random host port %s with container port %d", randomPort, p.Int()))
- portBindings[p] = CreatePortBinding(rp, "")
- continue
+ defer l.Close()
+ _, randomPort, err := net.SplitHostPort(l.Addr().String())
+ if err != nil {
+ return nil, errors.Wrapf(err, "unable to determine free port")
}
- if _, exists := ports[p]; !exists {
- ports[p] = struct{}{}
+ rp, err := strconv.Atoi(randomPort)
+ if err != nil {
+ return nil, errors.Wrapf(err, "unable to convert random port to int")
}
+ logrus.Debug(fmt.Sprintf("Using random host port %s with container port %d", randomPort, p.Int()))
+ portBindings[p] = CreatePortBinding(rp, "")
}
}
- return ports, portBindings, nil
+ return portBindings, nil
}
// imageData pulls down the image if not stored locally and extracts the
@@ -582,7 +579,7 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime, imageName string,
}
// EXPOSED PORTS
- ports, portBindings, err := exposedPorts(c, data.Config.ExposedPorts)
+ portBindings, err := exposedPorts(c, data.Config.ExposedPorts)
if err != nil {
return nil, err
}
@@ -617,19 +614,19 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime, imageName string,
}
config := &createConfig{
- Runtime: runtime,
- CapAdd: c.StringSlice("cap-add"),
- CapDrop: c.StringSlice("cap-drop"),
- CgroupParent: c.String("cgroup-parent"),
- Command: command,
- Detach: c.Bool("detach"),
- Devices: c.StringSlice("device"),
- DNSOpt: c.StringSlice("dns-opt"),
- DNSSearch: c.StringSlice("dns-search"),
- DNSServers: c.StringSlice("dns"),
- Entrypoint: entrypoint,
- Env: env,
- ExposedPorts: ports,
+ Runtime: runtime,
+ CapAdd: c.StringSlice("cap-add"),
+ CapDrop: c.StringSlice("cap-drop"),
+ CgroupParent: c.String("cgroup-parent"),
+ Command: command,
+ Detach: c.Bool("detach"),
+ Devices: c.StringSlice("device"),
+ DNSOpt: c.StringSlice("dns-opt"),
+ DNSSearch: c.StringSlice("dns-search"),
+ DNSServers: c.StringSlice("dns"),
+ Entrypoint: entrypoint,
+ Env: env,
+ //ExposedPorts: ports,
GroupAdd: groupAdd,
Hostname: c.String("hostname"),
HostAdd: c.StringSlice("add-host"),
diff --git a/cmd/podman/spec.go b/cmd/podman/spec.go
index 56e8c8d05..5206a4d36 100644
--- a/cmd/podman/spec.go
+++ b/cmd/podman/spec.go
@@ -570,8 +570,7 @@ func (c *createConfig) GetContainerCreateOptions() ([]libpod.CtrCreateOption, er
options = append(options, libpod.WithName(c.Name))
}
- // TODO deal with ports defined in image metadata
- if len(c.PortBindings) > 0 || len(c.ExposedPorts) > 0 {
+ if len(c.PortBindings) > 0 {
portBindings, err = c.CreatePortBindings()
if err != nil {
return nil, errors.Wrapf(err, "unable to create port bindings")
@@ -679,15 +678,5 @@ func (c *createConfig) CreatePortBindings() ([]ocicni.PortMapping, error) {
portBindings = append(portBindings, pm)
}
}
- for j := range c.ExposedPorts {
- var expose ocicni.PortMapping
- expose.HostPort = int32(j.Int())
- expose.ContainerPort = int32(j.Int())
- // CNI requires us to make both udp and tcp structs
- expose.Protocol = "udp"
- portBindings = append(portBindings, expose)
- expose.Protocol = "tcp"
- portBindings = append(portBindings, expose)
- }
return portBindings, nil
}
diff --git a/test/e2e/run_networking_test.go b/test/e2e/run_networking_test.go
index 68be154f5..b56b11634 100644
--- a/test/e2e/run_networking_test.go
+++ b/test/e2e/run_networking_test.go
@@ -47,7 +47,7 @@ var _ = Describe("Podman rmi", func() {
})
It("podman run network expose port 222", func() {
- session := podmanTest.Podman([]string{"run", "-dt", "--expose", "222-223", ALPINE, "/bin/sh"})
+ session := podmanTest.Podman([]string{"run", "-dt", "--expose", "222-223", "-P", ALPINE, "/bin/sh"})
session.Wait(30)
Expect(session.ExitCode()).To(Equal(0))
results := podmanTest.SystemExec("iptables", []string{"-t", "nat", "-L"})
--
cgit v1.2.3-54-g00ecf