Browse code

Fix my own comments from #7927

Signed-off-by: Alexandr Morozov <lk4d4math@gmail.com>

Alexandr Morozov authored on 2014/09/08 19:22:50
Showing 3 changed files
... ...
@@ -97,16 +97,10 @@ func Map(container net.Addr, hostIP net.IP, hostPort int) (host net.Addr, err er
97 97
 		return nil, err
98 98
 	}
99 99
 
100
-	m.userlandProxy = proxy
101
-	currentMappings[key] = m
102
-
103 100
 	cleanup := func() error {
104 101
 		// need to undo the iptables rules before we return
105
-		forward(iptables.Delete, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort)
106 102
 		proxy.Stop()
107 103
 		forward(iptables.Delete, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort)
108
-		m.userlandProxy = nil
109
-		delete(currentMappings, key)
110 104
 		if err := portallocator.ReleasePort(hostIP, m.proto, allocatedHostPort); err != nil {
111 105
 			return err
112 106
 		}
... ...
@@ -118,12 +112,10 @@ func Map(container net.Addr, hostIP net.IP, hostPort int) (host net.Addr, err er
118 118
 		if err := cleanup(); err != nil {
119 119
 			return nil, fmt.Errorf("Error during port allocation cleanup: %v", err)
120 120
 		}
121
-
122
-		if err == ErrPortMappingFailure {
123
-			return nil, portallocator.NewErrPortAlreadyAllocated(hostIP.String(), allocatedHostPort)
124
-		}
121
+		return nil, err
125 122
 	}
126
-
123
+	m.userlandProxy = proxy
124
+	currentMappings[key] = m
127 125
 	return m.host, nil
128 126
 }
129 127
 
... ...
@@ -1,8 +1,9 @@
1 1
 package portmapper
2 2
 
3 3
 import (
4
-	"errors"
5 4
 	"flag"
5
+	"fmt"
6
+	"io/ioutil"
6 7
 	"log"
7 8
 	"net"
8 9
 	"os"
... ...
@@ -16,8 +17,6 @@ import (
16 16
 	"github.com/docker/docker/reexec"
17 17
 )
18 18
 
19
-var ErrPortMappingFailure = errors.New("Failure Mapping Port")
20
-
21 19
 const userlandProxyCommandName = "docker-proxy"
22 20
 
23 21
 func init() {
... ...
@@ -42,12 +41,11 @@ func execProxy() {
42 42
 	p, err := proxy.NewProxy(host, container)
43 43
 	if err != nil {
44 44
 		os.Stdout.WriteString("1\n")
45
+		fmt.Fprint(os.Stderr, err)
45 46
 		os.Exit(1)
46 47
 	}
47
-
48
-	os.Stdout.WriteString("0\n")
49
-
50 48
 	go handleStopSignals(p)
49
+	os.Stdout.WriteString("0\n")
51 50
 
52 51
 	// Run will block until the proxy stops
53 52
 	p.Run()
... ...
@@ -117,40 +115,43 @@ func (p *proxyCommand) Start() error {
117 117
 	if err != nil {
118 118
 		return err
119 119
 	}
120
+	defer stdout.Close()
121
+	stderr, err := p.cmd.StderrPipe()
122
+	if err != nil {
123
+		return err
124
+	}
125
+	defer stderr.Close()
120 126
 	if err := p.cmd.Start(); err != nil {
121 127
 		return err
122 128
 	}
123 129
 
124
-	errchan := make(chan error)
125
-	after := time.After(1 * time.Second)
130
+	errchan := make(chan error, 1)
126 131
 	go func() {
127 132
 		buf := make([]byte, 2)
128 133
 		stdout.Read(buf)
129 134
 
130 135
 		if string(buf) != "0\n" {
131
-			errchan <- ErrPortMappingFailure
132
-		} else {
133
-			errchan <- nil
136
+			errStr, _ := ioutil.ReadAll(stderr)
137
+			errchan <- fmt.Errorf("Error starting userland proxy: %s", errStr)
138
+			return
134 139
 		}
140
+		errchan <- nil
135 141
 	}()
136 142
 
137
-	var readErr error
138
-
139 143
 	select {
140
-	case readErr = <-errchan:
141
-	case <-after:
142
-		readErr = ErrPortMappingFailure
144
+	case err := <-errchan:
145
+		return err
146
+	case <-time.After(1 * time.Second):
147
+		return fmt.Errorf("Timed out proxy starting the userland proxy")
143 148
 	}
144
-
145
-	return readErr
146 149
 }
147 150
 
148 151
 func (p *proxyCommand) Stop() error {
149 152
 	if p.cmd.Process != nil {
150
-		err := p.cmd.Process.Signal(os.Interrupt)
151
-		p.cmd.Wait()
152
-		return err
153
+		if err := p.cmd.Process.Signal(os.Interrupt); err != nil {
154
+			return err
155
+		}
156
+		return p.cmd.Wait()
153 157
 	}
154
-
155 158
 	return nil
156 159
 }
... ...
@@ -1887,10 +1887,12 @@ func TestRunPortInUse(t *testing.T) {
1887 1887
 	cmd := exec.Command(dockerBinary, "run", "-p", port+":80", "busybox", "true")
1888 1888
 	out, _, err := runCommandWithOutput(cmd)
1889 1889
 	if err == nil {
1890
-		t.Fatalf("Host port %s already in use, has been allocated by docker: %q", port, out)
1890
+		t.Fatalf("Binding on used port must fail")
1891
+	}
1892
+	if !strings.Contains(out, "address already in use") {
1893
+		t.Fatalf("Out must be about \"address already in use\", got %s", out)
1891 1894
 	}
1892 1895
 
1893 1896
 	deleteAllContainers()
1894
-
1895
-	logDone("run - port in use")
1897
+	logDone("run - fail if port already in use")
1896 1898
 }