diff --git a/daemon/networkdriver/portmapper/mapper.go b/daemon/networkdriver/portmapper/mapper.go index 7e8e266bb..24ca0d892 100644 --- a/daemon/networkdriver/portmapper/mapper.go +++ b/daemon/networkdriver/portmapper/mapper.go @@ -97,16 +97,25 @@ func Map(container net.Addr, hostIP net.IP, hostPort int) (host net.Addr, err er return nil, err } - m.userlandProxy = proxy - currentMappings[key] = m - - if err := proxy.Start(); err != nil { + cleanup := func() error { // need to undo the iptables rules before we return + proxy.Stop() forward(iptables.Delete, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort) + if err := portallocator.ReleasePort(hostIP, m.proto, allocatedHostPort); err != nil { + return err + } - return nil, err + return nil } + if err := proxy.Start(); err != nil { + if err := cleanup(); err != nil { + return nil, fmt.Errorf("Error during port allocation cleanup: %v", err) + } + 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 b24723727..1e8e0c398 100644 --- a/daemon/networkdriver/portmapper/proxy.go +++ b/daemon/networkdriver/portmapper/proxy.go @@ -2,6 +2,8 @@ package portmapper import ( "flag" + "fmt" + "io/ioutil" "log" "net" "os" @@ -9,6 +11,7 @@ import ( "os/signal" "strconv" "syscall" + "time" "github.com/docker/docker/pkg/proxy" "github.com/docker/docker/reexec" @@ -37,10 +40,12 @@ func execProxy() { p, err := proxy.NewProxy(host, container) if err != nil { - log.Fatal(err) + os.Stdout.WriteString("1\n") + fmt.Fprint(os.Stderr, err) + os.Exit(1) } - go handleStopSignals(p) + os.Stdout.WriteString("0\n") // Run will block until the proxy stops p.Run() @@ -96,10 +101,8 @@ func NewProxyCommand(proto string, hostIP net.IP, hostPort int, containerIP net. return &proxyCommand{ cmd: &exec.Cmd{ - Path: reexec.Self(), - Args: args, - Stdout: os.Stdout, - Stderr: os.Stderr, + Path: reexec.Self(), + Args: args, SysProcAttr: &syscall.SysProcAttr{ Pdeathsig: syscall.SIGTERM, // send a sigterm to the proxy if the daemon process dies }, @@ -108,12 +111,47 @@ func NewProxyCommand(proto string, hostIP net.IP, hostPort int, containerIP net. } func (p *proxyCommand) Start() error { - return p.cmd.Start() + stdout, err := p.cmd.StdoutPipe() + 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, 1) + go func() { + buf := make([]byte, 2) + stdout.Read(buf) + + if string(buf) != "0\n" { + errStr, _ := ioutil.ReadAll(stderr) + errchan <- fmt.Errorf("Error starting userland proxy: %s", errStr) + return + } + errchan <- nil + }() + + select { + case err := <-errchan: + return err + case <-time.After(1 * time.Second): + return fmt.Errorf("Timed out proxy starting the userland proxy") + } } func (p *proxyCommand) Stop() error { - err := p.cmd.Process.Signal(os.Interrupt) - p.cmd.Wait() - - return err + if p.cmd.Process != nil { + 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 fad3f6273..21aa383f4 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -4,6 +4,7 @@ import ( "bufio" "fmt" "io/ioutil" + "net" "os" "os/exec" "path" @@ -1895,3 +1896,23 @@ func TestRunDeallocatePortOnMissingIptablesRule(t *testing.T) { deleteAllContainers() logDone("run - port should be deallocated even on iptables error") } + +func TestRunPortInUse(t *testing.T) { + port := "1234" + l, err := net.Listen("tcp", ":"+port) + if err != nil { + t.Fatal(err) + } + defer l.Close() + cmd := exec.Command(dockerBinary, "run", "-p", port+":80", "busybox", "true") + out, _, err := runCommandWithOutput(cmd) + if err == nil { + 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 - fail if port already in use") +}