From 41e9e93e27ccd637d9490412622529bdc7d7b8ff Mon Sep 17 00:00:00 2001
From: Alexandr Morozov <lk4d4math@gmail.com>
Date: Mon, 8 Sep 2014 14:22:50 +0400
Subject: [PATCH] Fix my own comments from #7927

Signed-off-by: Alexandr Morozov <lk4d4math@gmail.com>
---
 daemon/networkdriver/portmapper/mapper.go | 14 ++-----
 daemon/networkdriver/portmapper/proxy.go  | 45 ++++++++++++-----------
 integration-cli/docker_cli_run_test.go    |  8 ++--
 3 files changed, 31 insertions(+), 36 deletions(-)

diff --git a/daemon/networkdriver/portmapper/mapper.go b/daemon/networkdriver/portmapper/mapper.go
index 774b58059..24ca0d892 100644
--- a/daemon/networkdriver/portmapper/mapper.go
+++ b/daemon/networkdriver/portmapper/mapper.go
@@ -97,16 +97,10 @@ func Map(container net.Addr, hostIP net.IP, hostPort int) (host net.Addr, err er
 		return nil, err
 	}
 
-	m.userlandProxy = proxy
-	currentMappings[key] = m
-
 	cleanup := func() error {
 		// need to undo the iptables rules before we return
-		forward(iptables.Delete, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort)
 		proxy.Stop()
 		forward(iptables.Delete, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort)
-		m.userlandProxy = nil
-		delete(currentMappings, key)
 		if err := portallocator.ReleasePort(hostIP, m.proto, allocatedHostPort); err != nil {
 			return err
 		}
@@ -118,12 +112,10 @@ func Map(container net.Addr, hostIP net.IP, hostPort int) (host net.Addr, err er
 		if err := cleanup(); err != nil {
 			return nil, fmt.Errorf("Error during port allocation cleanup: %v", err)
 		}
-
-		if err == ErrPortMappingFailure {
-			return nil, portallocator.NewErrPortAlreadyAllocated(hostIP.String(), allocatedHostPort)
-		}
+		return nil, err
 	}
-
+	m.userlandProxy = proxy
+	currentMappings[key] = m
 	return m.host, nil
 }
 
diff --git a/daemon/networkdriver/portmapper/proxy.go b/daemon/networkdriver/portmapper/proxy.go
index 67cd9710b..1e8e0c398 100644
--- a/daemon/networkdriver/portmapper/proxy.go
+++ b/daemon/networkdriver/portmapper/proxy.go
@@ -1,8 +1,9 @@
 package portmapper
 
 import (
-	"errors"
 	"flag"
+	"fmt"
+	"io/ioutil"
 	"log"
 	"net"
 	"os"
@@ -16,8 +17,6 @@ import (
 	"github.com/docker/docker/reexec"
 )
 
-var ErrPortMappingFailure = errors.New("Failure Mapping Port")
-
 const userlandProxyCommandName = "docker-proxy"
 
 func init() {
@@ -42,12 +41,11 @@ func execProxy() {
 	p, err := proxy.NewProxy(host, container)
 	if err != nil {
 		os.Stdout.WriteString("1\n")
+		fmt.Fprint(os.Stderr, err)
 		os.Exit(1)
 	}
-
-	os.Stdout.WriteString("0\n")
-
 	go handleStopSignals(p)
+	os.Stdout.WriteString("0\n")
 
 	// Run will block until the proxy stops
 	p.Run()
@@ -117,40 +115,43 @@ func (p *proxyCommand) Start() error {
 	if err != nil {
 		return err
 	}
+	defer stdout.Close()
+	stderr, err := p.cmd.StderrPipe()
+	if err != nil {
+		return err
+	}
+	defer stderr.Close()
 	if err := p.cmd.Start(); err != nil {
 		return err
 	}
 
-	errchan := make(chan error)
-	after := time.After(1 * time.Second)
+	errchan := make(chan error, 1)
 	go func() {
 		buf := make([]byte, 2)
 		stdout.Read(buf)
 
 		if string(buf) != "0\n" {
-			errchan <- ErrPortMappingFailure
-		} else {
-			errchan <- nil
+			errStr, _ := ioutil.ReadAll(stderr)
+			errchan <- fmt.Errorf("Error starting userland proxy: %s", errStr)
+			return
 		}
+		errchan <- nil
 	}()
 
-	var readErr error
-
 	select {
-	case readErr = <-errchan:
-	case <-after:
-		readErr = ErrPortMappingFailure
+	case err := <-errchan:
+		return err
+	case <-time.After(1 * time.Second):
+		return fmt.Errorf("Timed out proxy starting the userland proxy")
 	}
-
-	return readErr
 }
 
 func (p *proxyCommand) Stop() error {
 	if p.cmd.Process != nil {
-		err := p.cmd.Process.Signal(os.Interrupt)
-		p.cmd.Wait()
-		return err
+		if err := p.cmd.Process.Signal(os.Interrupt); err != nil {
+			return err
+		}
+		return p.cmd.Wait()
 	}
-
 	return nil
 }
diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go
index 1dc0e41b0..a8632a6f8 100644
--- a/integration-cli/docker_cli_run_test.go
+++ b/integration-cli/docker_cli_run_test.go
@@ -1887,10 +1887,12 @@ func TestRunPortInUse(t *testing.T) {
 	cmd := exec.Command(dockerBinary, "run", "-p", port+":80", "busybox", "true")
 	out, _, err := runCommandWithOutput(cmd)
 	if err == nil {
-		t.Fatalf("Host port %s already in use, has been allocated by docker: %q", port, out)
+		t.Fatalf("Binding on used port must fail")
+	}
+	if !strings.Contains(out, "address already in use") {
+		t.Fatalf("Out must be about \"address already in use\", got %s", out)
 	}
 
 	deleteAllContainers()
-
-	logDone("run - port in use")
+	logDone("run - fail if port already in use")
 }