Browse code

Merge pull request #7934 from LK4D4/fix_double_allocation

Fix error propagation from userland-proxy

Michael Crosby authored on 2014/09/13 03:39:10
Showing 3 changed files
... ...
@@ -97,16 +97,25 @@ 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
-	if err := proxy.Start(); err != nil {
100
+	cleanup := func() error {
104 101
 		// need to undo the iptables rules before we return
102
+		proxy.Stop()
105 103
 		forward(iptables.Delete, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort)
104
+		if err := portallocator.ReleasePort(hostIP, m.proto, allocatedHostPort); err != nil {
105
+			return err
106
+		}
106 107
 
107
-		return nil, err
108
+		return nil
108 109
 	}
109 110
 
111
+	if err := proxy.Start(); err != nil {
112
+		if err := cleanup(); err != nil {
113
+			return nil, fmt.Errorf("Error during port allocation cleanup: %v", err)
114
+		}
115
+		return nil, err
116
+	}
117
+	m.userlandProxy = proxy
118
+	currentMappings[key] = m
110 119
 	return m.host, nil
111 120
 }
112 121
 
... ...
@@ -2,6 +2,8 @@ package portmapper
2 2
 
3 3
 import (
4 4
 	"flag"
5
+	"fmt"
6
+	"io/ioutil"
5 7
 	"log"
6 8
 	"net"
7 9
 	"os"
... ...
@@ -9,6 +11,7 @@ import (
9 9
 	"os/signal"
10 10
 	"strconv"
11 11
 	"syscall"
12
+	"time"
12 13
 
13 14
 	"github.com/docker/docker/pkg/proxy"
14 15
 	"github.com/docker/docker/reexec"
... ...
@@ -37,10 +40,12 @@ func execProxy() {
37 37
 
38 38
 	p, err := proxy.NewProxy(host, container)
39 39
 	if err != nil {
40
-		log.Fatal(err)
40
+		os.Stdout.WriteString("1\n")
41
+		fmt.Fprint(os.Stderr, err)
42
+		os.Exit(1)
41 43
 	}
42
-
43 44
 	go handleStopSignals(p)
45
+	os.Stdout.WriteString("0\n")
44 46
 
45 47
 	// Run will block until the proxy stops
46 48
 	p.Run()
... ...
@@ -96,10 +101,8 @@ func NewProxyCommand(proto string, hostIP net.IP, hostPort int, containerIP net.
96 96
 
97 97
 	return &proxyCommand{
98 98
 		cmd: &exec.Cmd{
99
-			Path:   reexec.Self(),
100
-			Args:   args,
101
-			Stdout: os.Stdout,
102
-			Stderr: os.Stderr,
99
+			Path: reexec.Self(),
100
+			Args: args,
103 101
 			SysProcAttr: &syscall.SysProcAttr{
104 102
 				Pdeathsig: syscall.SIGTERM, // send a sigterm to the proxy if the daemon process dies
105 103
 			},
... ...
@@ -108,12 +111,47 @@ func NewProxyCommand(proto string, hostIP net.IP, hostPort int, containerIP net.
108 108
 }
109 109
 
110 110
 func (p *proxyCommand) Start() error {
111
-	return p.cmd.Start()
111
+	stdout, err := p.cmd.StdoutPipe()
112
+	if err != nil {
113
+		return err
114
+	}
115
+	defer stdout.Close()
116
+	stderr, err := p.cmd.StderrPipe()
117
+	if err != nil {
118
+		return err
119
+	}
120
+	defer stderr.Close()
121
+	if err := p.cmd.Start(); err != nil {
122
+		return err
123
+	}
124
+
125
+	errchan := make(chan error, 1)
126
+	go func() {
127
+		buf := make([]byte, 2)
128
+		stdout.Read(buf)
129
+
130
+		if string(buf) != "0\n" {
131
+			errStr, _ := ioutil.ReadAll(stderr)
132
+			errchan <- fmt.Errorf("Error starting userland proxy: %s", errStr)
133
+			return
134
+		}
135
+		errchan <- nil
136
+	}()
137
+
138
+	select {
139
+	case err := <-errchan:
140
+		return err
141
+	case <-time.After(1 * time.Second):
142
+		return fmt.Errorf("Timed out proxy starting the userland proxy")
143
+	}
112 144
 }
113 145
 
114 146
 func (p *proxyCommand) Stop() error {
115
-	err := p.cmd.Process.Signal(os.Interrupt)
116
-	p.cmd.Wait()
117
-
118
-	return err
147
+	if p.cmd.Process != nil {
148
+		if err := p.cmd.Process.Signal(os.Interrupt); err != nil {
149
+			return err
150
+		}
151
+		return p.cmd.Wait()
152
+	}
153
+	return nil
119 154
 }
... ...
@@ -4,6 +4,7 @@ import (
4 4
 	"bufio"
5 5
 	"fmt"
6 6
 	"io/ioutil"
7
+	"net"
7 8
 	"os"
8 9
 	"os/exec"
9 10
 	"path"
... ...
@@ -1895,3 +1896,23 @@ func TestRunDeallocatePortOnMissingIptablesRule(t *testing.T) {
1895 1895
 	deleteAllContainers()
1896 1896
 	logDone("run - port should be deallocated even on iptables error")
1897 1897
 }
1898
+
1899
+func TestRunPortInUse(t *testing.T) {
1900
+	port := "1234"
1901
+	l, err := net.Listen("tcp", ":"+port)
1902
+	if err != nil {
1903
+		t.Fatal(err)
1904
+	}
1905
+	defer l.Close()
1906
+	cmd := exec.Command(dockerBinary, "run", "-p", port+":80", "busybox", "true")
1907
+	out, _, err := runCommandWithOutput(cmd)
1908
+	if err == nil {
1909
+		t.Fatalf("Binding on used port must fail")
1910
+	}
1911
+	if !strings.Contains(out, "address already in use") {
1912
+		t.Fatalf("Out must be about \"address already in use\", got %s", out)
1913
+	}
1914
+
1915
+	deleteAllContainers()
1916
+	logDone("run - fail if port already in use")
1917
+}