Merge pull request #5544 from vieux/cleanup_api_client

Cleanup api client

api/client/cli.go

@@ -65,8 +65,13 @@ func NewDockerCli(in io.ReadCloser, out, err io.Writer, proto, addr string, tlsC
 	var (
 		isTerminal = false
 		terminalFd uintptr
+		scheme     = "http"
 	)
 
+	if tlsConfig != nil {
+		scheme = "https"
+	}
+
 	if in != nil {
 		if file, ok := in.(*os.File); ok {
 			terminalFd = file.Fd()
@@ -86,6 +91,7 @@ func NewDockerCli(in io.ReadCloser, out, err io.Writer, proto, addr string, tlsC
 		isTerminal: isTerminal,
 		terminalFd: terminalFd,
 		tlsConfig:  tlsConfig,
+		scheme:     scheme,
 	}
 }
 
@@ -99,4 +105,5 @@ type DockerCli struct {
 	isTerminal bool
 	terminalFd uintptr
 	tlsConfig  *tls.Config
+	scheme     string
 }

api/client/hijack.go

@@ -0,0 +1,133 @@
+package client
+
+import (
+	"crypto/tls"
+	"fmt"
+	"io"
+	"net"
+	"net/http"
+	"net/http/httputil"
+	"os"
+	"runtime"
+	"strings"
+
+	"github.com/dotcloud/docker/api"
+	"github.com/dotcloud/docker/dockerversion"
+	"github.com/dotcloud/docker/pkg/term"
+	"github.com/dotcloud/docker/utils"
+)
+
+func (cli *DockerCli) dial() (net.Conn, error) {
+	if cli.tlsConfig != nil && cli.proto != "unix" {
+		return tls.Dial(cli.proto, cli.addr, cli.tlsConfig)
+	}
+	return net.Dial(cli.proto, cli.addr)
+}
+
+func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.ReadCloser, stdout, stderr io.Writer, started chan io.Closer) error {
+	defer func() {
+		if started != nil {
+			close(started)
+		}
+	}()
+
+	req, err := http.NewRequest(method, fmt.Sprintf("/v%s%s", api.APIVERSION, path), nil)
+	if err != nil {
+		return err
+	}
+	req.Header.Set("User-Agent", "Docker-Client/"+dockerversion.VERSION)
+	req.Header.Set("Content-Type", "plain/text")
+	req.Host = cli.addr
+
+	dial, err := cli.dial()
+	if err != nil {
+		if strings.Contains(err.Error(), "connection refused") {
+			return fmt.Errorf("Cannot connect to the Docker daemon. Is 'docker -d' running on this host?")
+		}
+		return err
+	}
+	clientconn := httputil.NewClientConn(dial, nil)
+	defer clientconn.Close()
+
+	// Server hijacks the connection, error 'connection closed' expected
+	clientconn.Do(req)
+
+	rwc, br := clientconn.Hijack()
+	defer rwc.Close()
+
+	if started != nil {
+		started <- rwc
+	}
+
+	var receiveStdout chan error
+
+	var oldState *term.State
+
+	if in != nil && setRawTerminal && cli.isTerminal && os.Getenv("NORAW") == "" {
+		oldState, err = term.SetRawTerminal(cli.terminalFd)
+		if err != nil {
+			return err
+		}
+		defer term.RestoreTerminal(cli.terminalFd, oldState)
+	}
+
+	if stdout != nil || stderr != nil {
+		receiveStdout = utils.Go(func() (err error) {
+			defer func() {
+				if in != nil {
+					if setRawTerminal && cli.isTerminal {
+						term.RestoreTerminal(cli.terminalFd, oldState)
+					}
+					// For some reason this Close call blocks on darwin..
+					// As the client exists right after, simply discard the close
+					// until we find a better solution.
+					if runtime.GOOS != "darwin" {
+						in.Close()
+					}
+				}
+			}()
+
+			// When TTY is ON, use regular copy
+			if setRawTerminal {
+				_, err = io.Copy(stdout, br)
+			} else {
+				_, err = utils.StdCopy(stdout, stderr, br)
+			}
+			utils.Debugf("[hijack] End of stdout")
+			return err
+		})
+	}
+
+	sendStdin := utils.Go(func() error {
+		if in != nil {
+			io.Copy(rwc, in)
+			utils.Debugf("[hijack] End of stdin")
+		}
+		if tcpc, ok := rwc.(*net.TCPConn); ok {
+			if err := tcpc.CloseWrite(); err != nil {
+				utils.Debugf("Couldn't send EOF: %s\n", err)
+			}
+		} else if unixc, ok := rwc.(*net.UnixConn); ok {
+			if err := unixc.CloseWrite(); err != nil {
+				utils.Debugf("Couldn't send EOF: %s\n", err)
+			}
+		}
+		// Discard errors due to pipe interruption
+		return nil
+	})
+
+	if stdout != nil || stderr != nil {
+		if err := <-receiveStdout; err != nil {
+			utils.Debugf("Error receiveStdout: %s", err)
+			return err
+		}
+	}
+
+	if !cli.isTerminal {
+		if err := <-sendStdin; err != nil {
+			utils.Debugf("Error sendStdin: %s", err)
+			return err
+		}
+	}
+	return nil
+}

api/client/utils.go

@@ -2,7 +2,6 @@ package client
 
 import (
 	"bytes"
-	"crypto/tls"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
@@ -11,12 +10,9 @@ import (
 	"io/ioutil"
 	"net"
 	"net/http"
-	"net/http/httputil"
 	"net/url"
 	"os"
 	gosignal "os/signal"
-	"regexp"
-	goruntime "runtime"
 	"strconv"
 	"strings"
 	"syscall"
@@ -33,11 +29,14 @@ var (
 	ErrConnectionRefused = errors.New("Cannot connect to the Docker daemon. Is 'docker -d' running on this host?")
 )
 
-func (cli *DockerCli) dial() (net.Conn, error) {
-	if cli.tlsConfig != nil && cli.proto != "unix" {
-		return tls.Dial(cli.proto, cli.addr, cli.tlsConfig)
+func (cli *DockerCli) HTTPClient() *http.Client {
+	tr := &http.Transport{
+		TLSClientConfig: cli.tlsConfig,
+		Dial: func(network, addr string) (net.Conn, error) {
+			return net.Dial(cli.proto, cli.addr)
+		},
 	}
-	return net.Dial(cli.proto, cli.addr)
+	return &http.Client{Transport: tr}
 }
 
 func (cli *DockerCli) call(method, path string, data interface{}, passAuthInfo bool) (io.ReadCloser, int, error) {
@@ -57,9 +56,6 @@ func (cli *DockerCli) call(method, path string, data interface{}, passAuthInfo b
 			}
 		}
 	}
-	// fixme: refactor client to support redirect
-	re := regexp.MustCompile("/+")
-	path = re.ReplaceAllString(path, "/")
 
 	req, err := http.NewRequest(method, fmt.Sprintf("/v%s%s", api.APIVERSION, path), params)
 	if err != nil {
@@ -86,28 +82,20 @@ func (cli *DockerCli) call(method, path string, data interface{}, passAuthInfo b
 		}
 	}
 	req.Header.Set("User-Agent", "Docker-Client/"+dockerversion.VERSION)
-	req.Host = cli.addr
+	req.URL.Host = cli.addr
+	req.URL.Scheme = cli.scheme
 	if data != nil {
 		req.Header.Set("Content-Type", "application/json")
 	} else if method == "POST" {
 		req.Header.Set("Content-Type", "plain/text")
 	}
-	dial, err := cli.dial()
+	resp, err := cli.HTTPClient().Do(req)
 	if err != nil {
 		if strings.Contains(err.Error(), "connection refused") {
 			return nil, -1, ErrConnectionRefused
 		}
 		return nil, -1, err
 	}
-	clientconn := httputil.NewClientConn(dial, nil)
-	resp, err := clientconn.Do(req)
-	if err != nil {
-		clientconn.Close()
-		if strings.Contains(err.Error(), "connection refused") {
-			return nil, -1, ErrConnectionRefused
-		}
-		return nil, -1, err
-	}
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 400 {
 		body, err := ioutil.ReadAll(resp.Body)
@@ -119,14 +107,7 @@ func (cli *DockerCli) call(method, path string, data interface{}, passAuthInfo b
 		}
 		return nil, resp.StatusCode, fmt.Errorf("Error: %s", bytes.TrimSpace(body))
 	}
-
-	wrapper := utils.NewReadCloserWrapper(resp.Body, func() error {
-		if resp != nil && resp.Body != nil {
-			resp.Body.Close()
-		}
-		return clientconn.Close()
-	})
-	return wrapper, resp.StatusCode, nil
+	return resp.Body, resp.StatusCode, nil
 }
 
 func (cli *DockerCli) stream(method, path string, in io.Reader, out io.Writer, headers map[string][]string) error {
@@ -138,16 +119,13 @@ func (cli *DockerCli) streamHelper(method, path string, setRawTerminal bool, in
 		in = bytes.NewReader([]byte{})
 	}
 
-	// fixme: refactor client to support redirect
-	re := regexp.MustCompile("/+")
-	path = re.ReplaceAllString(path, "/")
-
-	req, err := http.NewRequest(method, fmt.Sprintf("/v%s%s", api.APIVERSION, path), in)
+	req, err := http.NewRequest(method, fmt.Sprintf("http://v%s%s", api.APIVERSION, path), in)
 	if err != nil {
 		return err
 	}
 	req.Header.Set("User-Agent", "Docker-Client/"+dockerversion.VERSION)
-	req.Host = cli.addr
+	req.URL.Host = cli.addr
+	req.URL.Scheme = cli.scheme
 	if method == "POST" {
 		req.Header.Set("Content-Type", "plain/text")
 	}
@@ -157,17 +135,7 @@ func (cli *DockerCli) streamHelper(method, path string, setRawTerminal bool, in
 			req.Header[k] = v
 		}
 	}
-
-	dial, err := cli.dial()
-	if err != nil {
-		if strings.Contains(err.Error(), "connection refused") {
-			return fmt.Errorf("Cannot connect to the Docker daemon. Is 'docker -d' running on this host?")
-		}
-		return err
-	}
-	clientconn := httputil.NewClientConn(dial, nil)
-	resp, err := clientconn.Do(req)
-	defer clientconn.Close()
+	resp, err := cli.HTTPClient().Do(req)
 	if err != nil {
 		if strings.Contains(err.Error(), "connection refused") {
 			return fmt.Errorf("Cannot connect to the Docker daemon. Is 'docker -d' running on this host?")
@@ -203,118 +171,6 @@ func (cli *DockerCli) streamHelper(method, path string, setRawTerminal bool, in
 	return nil
 }
 
-func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.ReadCloser, stdout, stderr io.Writer, started chan io.Closer) error {
-	defer func() {
-		if started != nil {
-			close(started)
-		}
-	}()
-	// fixme: refactor client to support redirect
-	re := regexp.MustCompile("/+")
-	path = re.ReplaceAllString(path, "/")
-
-	req, err := http.NewRequest(method, fmt.Sprintf("/v%s%s", api.APIVERSION, path), nil)
-	if err != nil {
-		return err
-	}
-	req.Header.Set("User-Agent", "Docker-Client/"+dockerversion.VERSION)
-	req.Header.Set("Content-Type", "plain/text")
-	req.Host = cli.addr
-
-	dial, err := cli.dial()
-	if err != nil {
-		if strings.Contains(err.Error(), "connection refused") {
-			return fmt.Errorf("Cannot connect to the Docker daemon. Is 'docker -d' running on this host?")
-		}
-		return err
-	}
-	clientconn := httputil.NewClientConn(dial, nil)
-	defer clientconn.Close()
-
-	// Server hijacks the connection, error 'connection closed' expected
-	clientconn.Do(req)
-
-	rwc, br := clientconn.Hijack()
-	defer rwc.Close()
-
-	if started != nil {
-		started <- rwc
-	}
-
-	var receiveStdout chan error
-
-	var oldState *term.State
-
-	if in != nil && setRawTerminal && cli.isTerminal && os.Getenv("NORAW") == "" {
-		oldState, err = term.SetRawTerminal(cli.terminalFd)
-		if err != nil {
-			return err
-		}
-		defer term.RestoreTerminal(cli.terminalFd, oldState)
-	}
-
-	if stdout != nil || stderr != nil {
-		receiveStdout = utils.Go(func() (err error) {
-			defer func() {
-				if in != nil {
-					if setRawTerminal && cli.isTerminal {
-						term.RestoreTerminal(cli.terminalFd, oldState)
-					}
-					// For some reason this Close call blocks on darwin..
-					// As the client exists right after, simply discard the close
-					// until we find a better solution.
-					if goruntime.GOOS != "darwin" {
-						in.Close()
-					}
-				}
-			}()
-
-			// When TTY is ON, use regular copy
-			if setRawTerminal {
-				_, err = io.Copy(stdout, br)
-			} else {
-				_, err = utils.StdCopy(stdout, stderr, br)
-			}
-			utils.Debugf("[hijack] End of stdout")
-			return err
-		})
-	}
-
-	sendStdin := utils.Go(func() error {
-		if in != nil {
-			io.Copy(rwc, in)
-			utils.Debugf("[hijack] End of stdin")
-		}
-		if tcpc, ok := rwc.(*net.TCPConn); ok {
-			if err := tcpc.CloseWrite(); err != nil {
-				utils.Debugf("Couldn't send EOF: %s\n", err)
-			}
-		} else if unixc, ok := rwc.(*net.UnixConn); ok {
-			if err := unixc.CloseWrite(); err != nil {
-				utils.Debugf("Couldn't send EOF: %s\n", err)
-			}
-		}
-		// Discard errors due to pipe interruption
-		return nil
-	})
-
-	if stdout != nil || stderr != nil {
-		if err := <-receiveStdout; err != nil {
-			utils.Debugf("Error receiveStdout: %s", err)
-			return err
-		}
-	}
-
-	if !cli.isTerminal {
-		if err := <-sendStdin; err != nil {
-			utils.Debugf("Error sendStdin: %s", err)
-			return err
-		}
-	}
-	return nil
-
-}
-
 func (cli *DockerCli) resizeTty(id string) {
 	height, width := cli.getTtySize()
 	if height == 0 && width == 0 {

integration/https_test.go

@@ -3,10 +3,12 @@ package docker
 import (
 	"crypto/tls"
 	"crypto/x509"
-	"github.com/dotcloud/docker/api/client"
 	"io/ioutil"
+	"strings"
 	"testing"
 	"time"
+
+	"github.com/dotcloud/docker/api/client"
 )
 
 const (
@@ -56,7 +58,7 @@ func TestHttpsInfoRogueCert(t *testing.T) {
 		if err == nil {
 			t.Fatal("Expected error but got nil")
 		}
-		if err.Error() != errBadCertificate {
+		if !strings.Contains(err.Error(), errBadCertificate) {
 			t.Fatalf("Expected error: %s, got instead: %s", errBadCertificate, err)
 		}
 	})
@@ -74,7 +76,7 @@ func TestHttpsInfoRogueServerCert(t *testing.T) {
 			t.Fatal("Expected error but got nil")
 		}
 
-		if err.Error() != errCaUnknown {
+		if !strings.Contains(err.Error(), errCaUnknown) {
 			t.Fatalf("Expected error: %s, got instead: %s", errBadCertificate, err)
 		}