From a8092a105b493f3c7f9ee063aa2ca035bc7f9be8 Mon Sep 17 00:00:00 2001 From: baude 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 Closes: #308 Approved by: mheon --- cmd/podman/create.go | 123 +++++++++++++++++++++++++-------------------------- cmd/podman/spec.go | 13 +----- 2 files changed, 61 insertions(+), 75 deletions(-) (limited to 'cmd/podman') 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 /[] or /[] - 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 /[] or /[] + //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 } -- cgit v1.2.3-54-g00ecf