Browse code

cleanup attach api calls

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Brian Goff authored on 2016/01/06 06:23:24
Showing 6 changed files
... ...
@@ -59,8 +59,7 @@ type monitorBackend interface {
59 59
 
60 60
 // attachBackend includes function to implement to provide container attaching functionality.
61 61
 type attachBackend interface {
62
-	ContainerAttachWithLogs(name string, c *backend.ContainerAttachWithLogsConfig) error
63
-	ContainerWsAttachWithLogs(name string, c *backend.ContainerWsAttachWithLogsConfig) error
62
+	ContainerAttach(name string, c *backend.ContainerAttachConfig) error
64 63
 }
65 64
 
66 65
 // Backend is all the methods that need to be implemented to provide container specific functionality.
... ...
@@ -3,6 +3,7 @@ package container
3 3
 import (
4 4
 	"encoding/json"
5 5
 	"fmt"
6
+	"io"
6 7
 	"net/http"
7 8
 	"strconv"
8 9
 	"strings"
... ...
@@ -14,6 +15,7 @@ import (
14 14
 	"github.com/docker/docker/api/server/httputils"
15 15
 	"github.com/docker/docker/api/types/backend"
16 16
 	derr "github.com/docker/docker/errors"
17
+	"github.com/docker/docker/pkg/ioutils"
17 18
 	"github.com/docker/docker/pkg/signal"
18 19
 	"github.com/docker/docker/pkg/term"
19 20
 	"github.com/docker/docker/runconfig"
... ...
@@ -425,18 +427,45 @@ func (s *containerRouter) postContainersAttach(ctx context.Context, w http.Respo
425 425
 		}
426 426
 	}
427 427
 
428
-	attachWithLogsConfig := &backend.ContainerAttachWithLogsConfig{
429
-		Hijacker:   w.(http.Hijacker),
430
-		Upgrade:    upgrade,
428
+	hijacker, ok := w.(http.Hijacker)
429
+	if !ok {
430
+		return derr.ErrorCodeNoHijackConnection.WithArgs(containerName)
431
+	}
432
+
433
+	setupStreams := func() (io.ReadCloser, io.Writer, io.Writer, error) {
434
+		conn, _, err := hijacker.Hijack()
435
+		if err != nil {
436
+			return nil, nil, nil, err
437
+		}
438
+
439
+		// set raw mode
440
+		conn.Write([]byte{})
441
+
442
+		if upgrade {
443
+			fmt.Fprintf(conn, "HTTP/1.1 101 UPGRADED\r\nContent-Type: application/vnd.docker.raw-stream\r\nConnection: Upgrade\r\nUpgrade: tcp\r\n\r\n")
444
+		} else {
445
+			fmt.Fprintf(conn, "HTTP/1.1 200 OK\r\nContent-Type: application/vnd.docker.raw-stream\r\n\r\n")
446
+		}
447
+
448
+		closer := func() error {
449
+			httputils.CloseStreams(conn)
450
+			return nil
451
+		}
452
+		return ioutils.NewReadCloserWrapper(conn, closer), conn, conn, nil
453
+	}
454
+
455
+	attachConfig := &backend.ContainerAttachConfig{
456
+		GetStreams: setupStreams,
431 457
 		UseStdin:   httputils.BoolValue(r, "stdin"),
432 458
 		UseStdout:  httputils.BoolValue(r, "stdout"),
433 459
 		UseStderr:  httputils.BoolValue(r, "stderr"),
434 460
 		Logs:       httputils.BoolValue(r, "logs"),
435 461
 		Stream:     httputils.BoolValue(r, "stream"),
436 462
 		DetachKeys: keys,
463
+		MuxStreams: true,
437 464
 	}
438 465
 
439
-	return s.backend.ContainerAttachWithLogs(containerName, attachWithLogsConfig)
466
+	return s.backend.ContainerAttach(containerName, attachConfig)
440 467
 }
441 468
 
442 469
 func (s *containerRouter) wsContainersAttach(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
... ...
@@ -455,24 +484,44 @@ func (s *containerRouter) wsContainersAttach(ctx context.Context, w http.Respons
455 455
 		}
456 456
 	}
457 457
 
458
-	h := websocket.Handler(func(ws *websocket.Conn) {
459
-		defer ws.Close()
458
+	done := make(chan struct{})
459
+	started := make(chan struct{})
460 460
 
461
-		wsAttachWithLogsConfig := &backend.ContainerWsAttachWithLogsConfig{
462
-			InStream:   ws,
463
-			OutStream:  ws,
464
-			ErrStream:  ws,
465
-			Logs:       httputils.BoolValue(r, "logs"),
466
-			Stream:     httputils.BoolValue(r, "stream"),
467
-			DetachKeys: keys,
461
+	setupStreams := func() (io.ReadCloser, io.Writer, io.Writer, error) {
462
+		wsChan := make(chan *websocket.Conn)
463
+		h := func(conn *websocket.Conn) {
464
+			wsChan <- conn
465
+			<-done
468 466
 		}
469 467
 
470
-		if err := s.backend.ContainerWsAttachWithLogs(containerName, wsAttachWithLogsConfig); err != nil {
471
-			logrus.Errorf("Error attaching websocket: %s", utils.GetErrorMessage(err))
472
-		}
473
-	})
474
-	ws := websocket.Server{Handler: h, Handshake: nil}
475
-	ws.ServeHTTP(w, r)
468
+		srv := websocket.Server{Handler: h, Handshake: nil}
469
+		go func() {
470
+			close(started)
471
+			srv.ServeHTTP(w, r)
472
+		}()
476 473
 
477
-	return nil
474
+		conn := <-wsChan
475
+		return conn, conn, conn, nil
476
+	}
477
+
478
+	attachConfig := &backend.ContainerAttachConfig{
479
+		GetStreams: setupStreams,
480
+		Logs:       httputils.BoolValue(r, "logs"),
481
+		Stream:     httputils.BoolValue(r, "stream"),
482
+		DetachKeys: keys,
483
+		UseStdin:   true,
484
+		UseStdout:  true,
485
+		UseStderr:  true,
486
+		MuxStreams: false, // TODO: this should be true since it's a single stream for both stdout and stderr
487
+	}
488
+
489
+	err = s.backend.ContainerAttach(containerName, attachConfig)
490
+	close(done)
491
+	select {
492
+	case <-started:
493
+		logrus.Errorf("Error attaching websocket: %s", err)
494
+		return nil
495
+	default:
496
+	}
497
+	return err
478 498
 }
... ...
@@ -5,32 +5,25 @@ package backend
5 5
 
6 6
 import (
7 7
 	"io"
8
-	"net/http"
9 8
 
10 9
 	"github.com/docker/engine-api/types"
11 10
 )
12 11
 
13
-// ContainerAttachWithLogsConfig holds the streams to use when connecting to a container to view logs.
14
-type ContainerAttachWithLogsConfig struct {
15
-	Hijacker   http.Hijacker
16
-	Upgrade    bool
12
+// ContainerAttachConfig holds the streams to use when connecting to a container to view logs.
13
+type ContainerAttachConfig struct {
14
+	GetStreams func() (io.ReadCloser, io.Writer, io.Writer, error)
17 15
 	UseStdin   bool
18 16
 	UseStdout  bool
19 17
 	UseStderr  bool
20 18
 	Logs       bool
21 19
 	Stream     bool
22 20
 	DetachKeys []byte
23
-}
24 21
 
25
-// ContainerWsAttachWithLogsConfig attach with websockets, since all
26
-// stream data is delegated to the websocket to handle there.
27
-type ContainerWsAttachWithLogsConfig struct {
28
-	InStream   io.ReadCloser // Reader to attach to stdin of container
29
-	OutStream  io.Writer     // Writer to attach to stdout of container
30
-	ErrStream  io.Writer     // Writer to attach to stderr of container
31
-	Logs       bool          // If true return log output
32
-	Stream     bool          // If true return stream output
33
-	DetachKeys []byte
22
+	// Used to signify that streams are multiplexed and therefore need a StdWriter to encode stdout/sderr messages accordingly.
23
+	// TODO @cpuguy83: This shouldn't be needed. It was only added so that http and websocket endpoints can use the same function, and the websocket function was not using a stdwriter prior to this change...
24
+	// HOWEVER, the websocket endpoint is using a single stream and SHOULD be encoded with stdout/stderr as is done for HTTP since it is still just a single stream.
25
+	// Since such a change is an API change unrelated to the current changeset we'll keep it as is here and change separately.
26
+	MuxStreams bool
34 27
 }
35 28
 
36 29
 // ContainerLogsConfig holds configs for logging operations. Exists
... ...
@@ -106,7 +106,7 @@ type Backend interface {
106 106
 	// Pull tells Docker to pull image referenced by `name`.
107 107
 	PullOnBuild(name string, authConfigs map[string]types.AuthConfig, output io.Writer) (Image, error)
108 108
 	// ContainerAttach attaches to container.
109
-	ContainerAttachOnBuild(cID string, stdin io.ReadCloser, stdout, stderr io.Writer, stream bool) error
109
+	ContainerAttachRaw(cID string, stdin io.ReadCloser, stdout, stderr io.Writer, stream bool) error
110 110
 	// ContainerCreate creates a new Docker container and returns potential warnings
111 111
 	ContainerCreate(types.ContainerCreateConfig) (types.ContainerCreateResponse, error)
112 112
 	// ContainerRm removes a container specified by `id`.
... ...
@@ -541,7 +541,7 @@ func (b *Builder) create() (string, error) {
541 541
 func (b *Builder) run(cID string) (err error) {
542 542
 	errCh := make(chan error)
543 543
 	go func() {
544
-		errCh <- b.docker.ContainerAttachOnBuild(cID, nil, b.Stdout, b.Stderr, true)
544
+		errCh <- b.docker.ContainerAttachRaw(cID, nil, b.Stdout, b.Stderr, true)
545 545
 	}()
546 546
 
547 547
 	finished := make(chan struct{})
... ...
@@ -13,11 +13,8 @@ import (
13 13
 	"github.com/docker/docker/pkg/stdcopy"
14 14
 )
15 15
 
16
-// ContainerAttachWithLogs attaches to logs according to the config passed in. See ContainerAttachWithLogsConfig.
17
-func (daemon *Daemon) ContainerAttachWithLogs(prefixOrName string, c *backend.ContainerAttachWithLogsConfig) error {
18
-	if c.Hijacker == nil {
19
-		return derr.ErrorCodeNoHijackConnection.WithArgs(prefixOrName)
20
-	}
16
+// ContainerAttach attaches to logs according to the config passed in. See ContainerAttachConfig.
17
+func (daemon *Daemon) ContainerAttach(prefixOrName string, c *backend.ContainerAttachConfig) error {
21 18
 	container, err := daemon.GetContainer(prefixOrName)
22 19
 	if err != nil {
23 20
 		return derr.ErrorCodeNoSuchContainer.WithArgs(prefixOrName)
... ...
@@ -26,29 +23,15 @@ func (daemon *Daemon) ContainerAttachWithLogs(prefixOrName string, c *backend.Co
26 26
 		return derr.ErrorCodePausedContainer.WithArgs(prefixOrName)
27 27
 	}
28 28
 
29
-	conn, _, err := c.Hijacker.Hijack()
29
+	inStream, outStream, errStream, err := c.GetStreams()
30 30
 	if err != nil {
31 31
 		return err
32 32
 	}
33
-	defer conn.Close()
34
-	// Flush the options to make sure the client sets the raw mode
35
-	conn.Write([]byte{})
36
-	inStream := conn.(io.ReadCloser)
37
-	outStream := conn.(io.Writer)
38
-
39
-	if c.Upgrade {
40
-		fmt.Fprintf(outStream, "HTTP/1.1 101 UPGRADED\r\nContent-Type: application/vnd.docker.raw-stream\r\nConnection: Upgrade\r\nUpgrade: tcp\r\n\r\n")
41
-	} else {
42
-		fmt.Fprintf(outStream, "HTTP/1.1 200 OK\r\nContent-Type: application/vnd.docker.raw-stream\r\n\r\n")
43
-	}
33
+	defer inStream.Close()
44 34
 
45
-	var errStream io.Writer
46
-
47
-	if !container.Config.Tty {
48
-		errStream = stdcopy.NewStdWriter(outStream, stdcopy.Stderr)
35
+	if !container.Config.Tty && c.MuxStreams {
36
+		errStream = stdcopy.NewStdWriter(errStream, stdcopy.Stderr)
49 37
 		outStream = stdcopy.NewStdWriter(outStream, stdcopy.Stdout)
50
-	} else {
51
-		errStream = outStream
52 38
 	}
53 39
 
54 40
 	var stdin io.ReadCloser
... ...
@@ -64,32 +47,22 @@ func (daemon *Daemon) ContainerAttachWithLogs(prefixOrName string, c *backend.Co
64 64
 		stderr = errStream
65 65
 	}
66 66
 
67
-	if err := daemon.attachWithLogs(container, stdin, stdout, stderr, c.Logs, c.Stream, c.DetachKeys); err != nil {
67
+	if err := daemon.containerAttach(container, stdin, stdout, stderr, c.Logs, c.Stream, c.DetachKeys); err != nil {
68 68
 		fmt.Fprintf(outStream, "Error attaching: %s\n", err)
69 69
 	}
70 70
 	return nil
71 71
 }
72 72
 
73
-// ContainerWsAttachWithLogs websocket connection
74
-func (daemon *Daemon) ContainerWsAttachWithLogs(prefixOrName string, c *backend.ContainerWsAttachWithLogsConfig) error {
73
+// ContainerAttachRaw attaches the provided streams to the container's stdio
74
+func (daemon *Daemon) ContainerAttachRaw(prefixOrName string, stdin io.ReadCloser, stdout, stderr io.Writer, stream bool) error {
75 75
 	container, err := daemon.GetContainer(prefixOrName)
76 76
 	if err != nil {
77 77
 		return err
78 78
 	}
79
-	return daemon.attachWithLogs(container, c.InStream, c.OutStream, c.ErrStream, c.Logs, c.Stream, c.DetachKeys)
80
-}
81
-
82
-// ContainerAttachOnBuild attaches streams to the container cID. If stream is true, it streams the output.
83
-func (daemon *Daemon) ContainerAttachOnBuild(cID string, stdin io.ReadCloser, stdout, stderr io.Writer, stream bool) error {
84
-	return daemon.ContainerWsAttachWithLogs(cID, &backend.ContainerWsAttachWithLogsConfig{
85
-		InStream:  stdin,
86
-		OutStream: stdout,
87
-		ErrStream: stderr,
88
-		Stream:    stream,
89
-	})
79
+	return daemon.containerAttach(container, stdin, stdout, stderr, false, stream, nil)
90 80
 }
91 81
 
92
-func (daemon *Daemon) attachWithLogs(container *container.Container, stdin io.ReadCloser, stdout, stderr io.Writer, logs, stream bool, keys []byte) error {
82
+func (daemon *Daemon) containerAttach(container *container.Container, stdin io.ReadCloser, stdout, stderr io.Writer, logs, stream bool, keys []byte) error {
93 83
 	if logs {
94 84
 		logDriver, err := daemon.getLogger(container)
95 85
 		if err != nil {