Merge pull request #353 from jgreat/smarter-strip-log-headers

revert dropping header,  for only setting RawTerminal when container tty

Author: michaelshobbs

glide.lock

@@ -7,4 +7,3 @@ import:
 - package: golang.org/x/net
   subpackages:
   - websocket
-- package: github.com/hashicorp/go-version

glide.yaml

@@ -11,7 +11,6 @@ import (
 	"time"
 
 	"github.com/fsouza/go-dockerclient"
-	"github.com/hashicorp/go-version"
 )
 
 var allowTTY bool
@@ -181,10 +180,6 @@ func (p *LogsPump) Run() error {
 
 func (p *LogsPump) pumpLogs(event *docker.APIEvents, backlog bool, inactivityTimeout time.Duration) {
 	id := normalID(event.ID)
-
-	dockerVersion, err := p.client.Version()
-	assert(err, "Could not get Docker Version endpoint")
-
 	container, err := p.client.InspectContainer(id)
 	assert(err, "pump")
 	if ignoreContainerTTY(container) {
@@ -214,9 +209,16 @@ func (p *LogsPump) pumpLogs(event *docker.APIEvents, backlog bool, inactivityTim
 		debug("pump.pumpLogs():", id, "pump exists")
 		return
 	}
+
+	// RawTerminal with container Tty=false injects binary headers into
+	// the log stream that show up as garbage unicode characters
+	rawTerminal := false 
+	if allowTTY && container.Config.Tty {
+		rawTerminal = true
+	}
 	outrd, outwr := io.Pipe()
 	errrd, errwr := io.Pipe()
-	p.pumps[id] = newContainerPump(container, dockerVersion, outrd, errrd)
+	p.pumps[id] = newContainerPump(container, outrd, errrd)
 	p.mu.Unlock()
 	p.update(event)
 	go func() {
@@ -232,7 +234,7 @@ func (p *LogsPump) pumpLogs(event *docker.APIEvents, backlog bool, inactivityTim
 				Tail:              tail,
 				Since:             sinceTime.Unix(),
 				InactivityTimeout: inactivityTimeout,
-				RawTerminal:       allowTTY,
+				RawTerminal:       rawTerminal,
 			})
 			if err != nil {
 				debug("pump.pumpLogs():", id, "stopped with error:", err)
@@ -344,32 +346,14 @@ type containerPump struct {
 	logstreams map[chan *Message]*Route
 }
 
-func newContainerPump(container *docker.Container, dockerVersion *docker.Env, stdout, stderr io.Reader) *containerPump {
-	apiVersion := dockerVersion.Get("ApiVersion")
-	debug("API Version: ", apiVersion)
-	debug("TTY is: ", container.Config.Tty)
-	earliestAPI, _ := version.NewVersion("1.13")
-	currentAPI, err := version.NewVersion(apiVersion)
-	assert(err, "current docker api version is not valid version string")
-	hasHeaders := false
-	if container.Config.Tty == false {
-		if currentAPI.Compare(earliestAPI) >= 0 {
-			hasHeaders = true
-		}
-	}
-	debug("hasHeaders set to: ", hasHeaders)
-
+func newContainerPump(container *docker.Container, stdout, stderr io.Reader) *containerPump {
 	cp := &containerPump{
 		container:  container,
 		logstreams: make(map[chan *Message]*Route),
 	}
-	pump := func(source string, hasHeaders bool, input io.Reader) {
+	pump := func(source string, input io.Reader) {
 		buf := bufio.NewReader(input)
 		for {
-			// Discard header bytes from log message if TTY is false and api > 1.13.
-			if hasHeaders {
-				buf.Discard(8)
-			}
 			line, err := buf.ReadString('\n')
 			if err != nil {
 				if err != io.EOF {
@@ -385,8 +369,8 @@ func newContainerPump(container *docker.Container, dockerVersion *docker.Env, st
 			})
 		}
 	}
-	go pump("stdout", hasHeaders, stdout)
-	go pump("stderr", hasHeaders, stderr)
+	go pump("stdout", stdout)
+	go pump("stderr", stderr)
 	return cp
 }
 

router/pump.go

@@ -135,10 +135,7 @@ func TestPumpContainerRename(t *testing.T) {
 		Name:   "foo",
 		Config: config,
 	}
-	version := &docker.Env{
-		"ApiVersion=1.33",
-	}
-	p.pumps["8dfafdbc3a40"] = newContainerPump(container, version, os.Stdout, os.Stderr)
+	p.pumps["8dfafdbc3a40"] = newContainerPump(container, os.Stdout, os.Stderr)
 	if name := p.pumps["8dfafdbc3a40"].container.Name; name != "foo" {
 		t.Errorf("containerPump should have name: 'foo' got name: '%s'", name)
 	}
@@ -156,10 +153,7 @@ func TestPumpNewContainerPump(t *testing.T) {
 		ID:     "8dfafdbc3a40",
 		Config: config,
 	}
-	version := &docker.Env{
-		"ApiVersion=1.33",
-	}
-	pump := newContainerPump(container, version, os.Stdout, os.Stderr)
+	pump := newContainerPump(container, os.Stdout, os.Stderr)
 	if pump == nil {
 		t.Error("pump nil")
 		return
@@ -174,10 +168,7 @@ func TestPumpContainerPump(t *testing.T) {
 		ID:     "8dfafdbc3a40",
 		Config: config,
 	}
-	version := &docker.Env{
-		"ApiVersion=1.33",
-	}
-	pump := newContainerPump(container, version, os.Stdout, os.Stderr)
+	pump := newContainerPump(container, os.Stdout, os.Stderr)
 	logstream, route := make(chan *Message), &Route{}
 	go func() {
 		for msg := range logstream {