Merge pull request #5706 from vieux/remove_add_string

api/server/server.go

@@ -122,17 +122,17 @@ func postAuth(eng *engine.Engine, version version.Version, w http.ResponseWriter
 	var (
 		authConfig, err = ioutil.ReadAll(r.Body)
 		job             = eng.Job("auth")
-		status          string
+		stdoutBuffer    = bytes.NewBuffer(nil)
 	)
 	if err != nil {
 		return err
 	}
 	job.Setenv("authConfig", string(authConfig))
-	job.Stdout.AddString(&status)
+	job.Stdout.Add(stdoutBuffer)
 	if err = job.Run(); err != nil {
 		return err
 	}
-	if status != "" {
+	if status := engine.Tail(stdoutBuffer, 1); status != "" {
 		var env engine.Env
 		env.Set("Status", status)
 		return writeJSON(w, http.StatusOK, env)
@@ -393,9 +393,10 @@ func postCommit(eng *engine.Engine, version version.Version, w http.ResponseWrit
 		return err
 	}
 	var (
-		config engine.Env
-		env    engine.Env
-		job    = eng.Job("commit", r.Form.Get("container"))
+		config       engine.Env
+		env          engine.Env
+		job          = eng.Job("commit", r.Form.Get("container"))
+		stdoutBuffer = bytes.NewBuffer(nil)
 	)
 	if err := config.Decode(r.Body); err != nil {
 		utils.Errorf("%s", err)
@@ -407,12 +408,11 @@ func postCommit(eng *engine.Engine, version version.Version, w http.ResponseWrit
 	job.Setenv("comment", r.Form.Get("comment"))
 	job.SetenvSubEnv("config", &config)
 
-	var id string
-	job.Stdout.AddString(&id)
+	job.Stdout.Add(stdoutBuffer)
 	if err := job.Run(); err != nil {
 		return err
 	}
-	env.Set("Id", id)
+	env.Set("Id", engine.Tail(stdoutBuffer, 1))
 	return writeJSON(w, http.StatusCreated, env)
 }
 
@@ -603,17 +603,17 @@ func postContainersCreate(eng *engine.Engine, version version.Version, w http.Re
 		return nil
 	}
 	var (
-		out         engine.Env
-		job         = eng.Job("create", r.Form.Get("name"))
-		outWarnings []string
-		outId       string
-		warnings    = bytes.NewBuffer(nil)
+		out          engine.Env
+		job          = eng.Job("create", r.Form.Get("name"))
+		outWarnings  []string
+		stdoutBuffer = bytes.NewBuffer(nil)
+		warnings     = bytes.NewBuffer(nil)
 	)
 	if err := job.DecodeEnv(r.Body); err != nil {
 		return err
 	}
 	// Read container ID from the first line of stdout
-	job.Stdout.AddString(&outId)
+	job.Stdout.Add(stdoutBuffer)
 	// Read warnings from stderr
 	job.Stderr.Add(warnings)
 	if err := job.Run(); err != nil {
@@ -624,7 +624,7 @@ func postContainersCreate(eng *engine.Engine, version version.Version, w http.Re
 	for scanner.Scan() {
 		outWarnings = append(outWarnings, scanner.Text())
 	}
-	out.Set("Id", outId)
+	out.Set("Id", engine.Tail(stdoutBuffer, 1))
 	out.SetList("Warnings", outWarnings)
 	return writeJSON(w, http.StatusCreated, out)
 }
@@ -720,20 +720,16 @@ func postContainersWait(eng *engine.Engine, version version.Version, w http.Resp
 		return fmt.Errorf("Missing parameter")
 	}
 	var (
-		env    engine.Env
-		status string
-		job    = eng.Job("wait", vars["name"])
+		env          engine.Env
+		stdoutBuffer = bytes.NewBuffer(nil)
+		job          = eng.Job("wait", vars["name"])
 	)
-	job.Stdout.AddString(&status)
+	job.Stdout.Add(stdoutBuffer)
 	if err := job.Run(); err != nil {
 		return err
 	}
-	// Parse a 16-bit encoded integer to map typical unix exit status.
-	_, err := strconv.ParseInt(status, 10, 16)
-	if err != nil {
-		return err
-	}
-	env.Set("StatusCode", status)
+
+	env.Set("StatusCode", engine.Tail(stdoutBuffer, 1))
 	return writeJSON(w, http.StatusOK, env)
 }
 

engine/engine.go

@@ -3,11 +3,12 @@ package engine
 import (
 	"bufio"
 	"fmt"
-	"github.com/dotcloud/docker/utils"
 	"io"
 	"os"
 	"sort"
 	"strings"
+
+	"github.com/dotcloud/docker/utils"
 )
 
 // Installer is a standard interface for objects which can "install" themselves

engine/job.go

@@ -1,6 +1,7 @@
 package engine
 
 import (
+	"bytes"
 	"fmt"
 	"io"
 	"strings"
@@ -56,8 +57,8 @@ func (job *Job) Run() error {
 	defer func() {
 		job.Eng.Logf("-job %s%s", job.CallString(), job.StatusString())
 	}()
-	var errorMessage string
-	job.Stderr.AddString(&errorMessage)
+	var errorMessage = bytes.NewBuffer(nil)
+	job.Stderr.Add(errorMessage)
 	if job.handler == nil {
 		job.Errorf("%s: command not found", job.Name)
 		job.status = 127
@@ -76,7 +77,7 @@ func (job *Job) Run() error {
 		return err
 	}
 	if job.status != 0 {
-		return fmt.Errorf("%s", errorMessage)
+		return fmt.Errorf("%s", Tail(errorMessage, 1))
 	}
 	return nil
 }

engine/job_test.go

@@ -1,6 +1,8 @@
 package engine
 
 import (
+	"bytes"
+	"fmt"
 	"testing"
 )
 
@@ -40,13 +42,13 @@ func TestJobStdoutString(t *testing.T) {
 	})
 
 	job := eng.Job("say_something_in_stdout")
-	var output string
-	if err := job.Stdout.AddString(&output); err != nil {
-		t.Fatal(err)
-	}
+	var outputBuffer = bytes.NewBuffer(nil)
+	job.Stdout.Add(outputBuffer)
 	if err := job.Run(); err != nil {
 		t.Fatal(err)
 	}
+	fmt.Println(outputBuffer)
+	var output = Tail(outputBuffer, 1)
 	if expectedOutput := "Hello world"; output != expectedOutput {
 		t.Fatalf("Stdout last line:\nExpected: %v\nReceived: %v", expectedOutput, output)
 	}
@@ -61,13 +63,12 @@ func TestJobStderrString(t *testing.T) {
 	})
 
 	job := eng.Job("say_something_in_stderr")
-	var output string
-	if err := job.Stderr.AddString(&output); err != nil {
-		t.Fatal(err)
-	}
+	var outputBuffer = bytes.NewBuffer(nil)
+	job.Stderr.Add(outputBuffer)
 	if err := job.Run(); err != nil {
 		t.Fatal(err)
 	}
+	var output = Tail(outputBuffer, 1)
 	if expectedOutput := "Something happened"; output != expectedOutput {
 		t.Fatalf("Stderr last line:\nExpected: %v\nReceived: %v", expectedOutput, output)
 	}

engine/streams.go

@@ -1,8 +1,7 @@
 package engine
 
 import (
-	"bufio"
-	"container/ring"
+	"bytes"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -16,6 +15,28 @@ type Output struct {
 	used  bool
 }
 
+// Tail returns the n last lines of a buffer
+// stripped out of the last \n, if any
+// if n <= 0, returns an empty string
+func Tail(buffer *bytes.Buffer, n int) string {
+	if n <= 0 {
+		return ""
+	}
+	bytes := buffer.Bytes()
+	if len(bytes) > 0 && bytes[len(bytes)-1] == '\n' {
+		bytes = bytes[:len(bytes)-1]
+	}
+	for i := buffer.Len() - 2; i >= 0; i-- {
+		if bytes[i] == '\n' {
+			n--
+			if n == 0 {
+				return string(bytes[i+1:])
+			}
+		}
+	}
+	return string(bytes)
+}
+
 // NewOutput returns a new Output object with no destinations attached.
 // Writing to an empty Output will cause the written data to be discarded.
 func NewOutput() *Output {
@@ -58,42 +79,6 @@ func (o *Output) AddPipe() (io.Reader, error) {
 	return r, nil
 }
 
-// AddTail starts a new goroutine which will read all subsequent data written to the output,
-// line by line, and append the last `n` lines to `dst`.
-func (o *Output) AddTail(dst *[]string, n int) error {
-	src, err := o.AddPipe()
-	if err != nil {
-		return err
-	}
-	o.tasks.Add(1)
-	go func() {
-		defer o.tasks.Done()
-		Tail(src, n, dst)
-	}()
-	return nil
-}
-
-// AddString starts a new goroutine which will read all subsequent data written to the output,
-// line by line, and store the last line into `dst`.
-func (o *Output) AddString(dst *string) error {
-	src, err := o.AddPipe()
-	if err != nil {
-		return err
-	}
-	o.tasks.Add(1)
-	go func() {
-		defer o.tasks.Done()
-		lines := make([]string, 0, 1)
-		Tail(src, 1, &lines)
-		if len(lines) == 0 {
-			*dst = ""
-		} else {
-			*dst = lines[0]
-		}
-	}()
-	return nil
-}
-
 // Write writes the same data to all registered destinations.
 // This method is thread-safe.
 func (o *Output) Write(p []byte) (n int, err error) {
@@ -174,26 +159,6 @@ func (i *Input) Add(src io.Reader) error {
 	return nil
 }
 
-// Tail reads from `src` line per line, and returns the last `n` lines as an array.
-// A ring buffer is used to only store `n` lines at any time.
-func Tail(src io.Reader, n int, dst *[]string) {
-	scanner := bufio.NewScanner(src)
-	r := ring.New(n)
-	for scanner.Scan() {
-		if n == 0 {
-			continue
-		}
-		r.Value = scanner.Text()
-		r = r.Next()
-	}
-	r.Do(func(v interface{}) {
-		if v == nil {
-			return
-		}
-		*dst = append(*dst, v.(string))
-	})
-}
-
 // AddEnv starts a new goroutine which will decode all subsequent data
 // as a stream of json-encoded objects, and point `dst` to the last
 // decoded object.

engine/streams_test.go

@@ -10,53 +10,6 @@ import (
 	"testing"
 )
 
-func TestOutputAddString(t *testing.T) {
-	var testInputs = [][2]string{
-		{
-			"hello, world!",
-			"hello, world!",
-		},
-
-		{
-			"One\nTwo\nThree",
-			"Three",
-		},
-
-		{
-			"",
-			"",
-		},
-
-		{
-			"A line\nThen another nl-terminated line\n",
-			"Then another nl-terminated line",
-		},
-
-		{
-			"A line followed by an empty line\n\n",
-			"",
-		},
-	}
-	for _, testData := range testInputs {
-		input := testData[0]
-		expectedOutput := testData[1]
-		o := NewOutput()
-		var output string
-		if err := o.AddString(&output); err != nil {
-			t.Error(err)
-		}
-		if n, err := o.Write([]byte(input)); err != nil {
-			t.Error(err)
-		} else if n != len(input) {
-			t.Errorf("Expected %d, got %d", len(input), n)
-		}
-		o.Close()
-		if output != expectedOutput {
-			t.Errorf("Last line is not stored as return string.\nInput:   '%s'\nExpected: '%s'\nGot:       '%s'", input, expectedOutput, output)
-		}
-	}
-}
-
 type sentinelWriteCloser struct {
 	calledWrite bool
 	calledClose bool
@@ -145,59 +98,24 @@ func TestOutputAddPipe(t *testing.T) {
 }
 
 func TestTail(t *testing.T) {
-	var tests = make(map[string][][]string)
-	tests["hello, world!"] = [][]string{
-		{},
-		{"hello, world!"},
-		{"hello, world!"},
-		{"hello, world!"},
-	}
-	tests["One\nTwo\nThree"] = [][]string{
-		{},
-		{"Three"},
-		{"Two", "Three"},
-		{"One", "Two", "Three"},
-	}
-	for input, outputs := range tests {
-		for n, expectedOutput := range outputs {
-			var output []string
-			Tail(strings.NewReader(input), n, &output)
-			if fmt.Sprintf("%v", output) != fmt.Sprintf("%v", expectedOutput) {
-				t.Errorf("Tail n=%d returned wrong result.\nExpected: '%s'\nGot     : '%s'", expectedOutput, output)
-			}
-		}
-	}
-}
-
-func TestOutputAddTail(t *testing.T) {
-	var tests = make(map[string][][]string)
-	tests["hello, world!"] = [][]string{
-		{},
-		{"hello, world!"},
-		{"hello, world!"},
-		{"hello, world!"},
+	var tests = make(map[string][]string)
+	tests["hello, world!"] = []string{
+		"",
+		"hello, world!",
+		"hello, world!",
+		"hello, world!",
 	}
-	tests["One\nTwo\nThree"] = [][]string{
-		{},
-		{"Three"},
-		{"Two", "Three"},
-		{"One", "Two", "Three"},
+	tests["One\nTwo\nThree"] = []string{
+		"",
+		"Three",
+		"Two\nThree",
+		"One\nTwo\nThree",
 	}
 	for input, outputs := range tests {
 		for n, expectedOutput := range outputs {
-			o := NewOutput()
-			var output []string
-			if err := o.AddTail(&output, n); err != nil {
-				t.Error(err)
-			}
-			if n, err := o.Write([]byte(input)); err != nil {
-				t.Error(err)
-			} else if n != len(input) {
-				t.Errorf("Expected %d, got %d", len(input), n)
-			}
-			o.Close()
-			if fmt.Sprintf("%v", output) != fmt.Sprintf("%v", expectedOutput) {
-				t.Errorf("Tail(%d) returned wrong result.\nExpected: %v\nGot:      %v", n, expectedOutput, output)
+			output := Tail(bytes.NewBufferString(input), n)
+			if output != expectedOutput {
+				t.Errorf("Tail n=%d returned wrong result.\nExpected: '%s'\nGot     : '%s'", n, expectedOutput, output)
 			}
 		}
 	}

integration/runtime_test.go

@@ -3,13 +3,6 @@ package docker
 import (
 	"bytes"
 	"fmt"
-	"github.com/dotcloud/docker/daemon"
-	"github.com/dotcloud/docker/engine"
-	"github.com/dotcloud/docker/image"
-	"github.com/dotcloud/docker/nat"
-	"github.com/dotcloud/docker/runconfig"
-	"github.com/dotcloud/docker/sysinit"
-	"github.com/dotcloud/docker/utils"
 	"io"
 	"log"
 	"net"
@@ -22,6 +15,14 @@ import (
 	"syscall"
 	"testing"
 	"time"
+
+	"github.com/dotcloud/docker/daemon"
+	"github.com/dotcloud/docker/engine"
+	"github.com/dotcloud/docker/image"
+	"github.com/dotcloud/docker/nat"
+	"github.com/dotcloud/docker/runconfig"
+	"github.com/dotcloud/docker/sysinit"
+	"github.com/dotcloud/docker/utils"
 )
 
 const (
@@ -421,13 +422,14 @@ func TestGet(t *testing.T) {
 
 func startEchoServerContainer(t *testing.T, proto string) (*daemon.Daemon, *daemon.Container, string) {
 	var (
-		err     error
-		id      string
-		strPort string
-		eng     = NewTestEngine(t)
-		daemon  = mkDaemonFromEngine(eng, t)
-		port    = 5554
-		p       nat.Port
+		err          error
+		id           string
+		outputBuffer = bytes.NewBuffer(nil)
+		strPort      string
+		eng          = NewTestEngine(t)
+		daemon       = mkDaemonFromEngine(eng, t)
+		port         = 5554
+		p            nat.Port
 	)
 	defer func() {
 		if err != nil {
@@ -455,10 +457,11 @@ func startEchoServerContainer(t *testing.T, proto string) (*daemon.Daemon, *daem
 		jobCreate.SetenvList("Cmd", []string{"sh", "-c", cmd})
 		jobCreate.SetenvList("PortSpecs", []string{fmt.Sprintf("%s/%s", strPort, proto)})
 		jobCreate.SetenvJson("ExposedPorts", ep)
-		jobCreate.Stdout.AddString(&id)
+		jobCreate.Stdout.Add(outputBuffer)
 		if err := jobCreate.Run(); err != nil {
 			t.Fatal(err)
 		}
+		id = engine.Tail(outputBuffer, 1)
 		// FIXME: this relies on the undocumented behavior of daemon.Create
 		// which will return a nil error AND container if the exposed ports
 		// are invalid. That behavior should be fixed!
@@ -720,12 +723,12 @@ func TestContainerNameValidation(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		var shortID string
+		var outputBuffer = bytes.NewBuffer(nil)
 		job := eng.Job("create", test.Name)
 		if err := job.ImportEnv(config); err != nil {
 			t.Fatal(err)
 		}
-		job.Stdout.AddString(&shortID)
+		job.Stdout.Add(outputBuffer)
 		if err := job.Run(); err != nil {
 			if !test.Valid {
 				continue
@@ -733,7 +736,7 @@ func TestContainerNameValidation(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		container := daemon.Get(shortID)
+		container := daemon.Get(engine.Tail(outputBuffer, 1))
 
 		if container.Name != "/"+test.Name {
 			t.Fatalf("Expect /%s got %s", test.Name, container.Name)

integration/server_test.go

@@ -1,12 +1,14 @@
 package docker
 
 import (
-	"github.com/dotcloud/docker/engine"
-	"github.com/dotcloud/docker/runconfig"
-	"github.com/dotcloud/docker/server"
+	"bytes"
 	"strings"
 	"testing"
 	"time"
+
+	"github.com/dotcloud/docker/engine"
+	"github.com/dotcloud/docker/runconfig"
+	"github.com/dotcloud/docker/server"
 )
 
 func TestCreateNumberHostname(t *testing.T) {
@@ -70,13 +72,13 @@ func TestMergeConfigOnCommit(t *testing.T) {
 	job.Setenv("repo", "testrepo")
 	job.Setenv("tag", "testtag")
 	job.SetenvJson("config", config)
-	var newId string
-	job.Stdout.AddString(&newId)
+	var outputBuffer = bytes.NewBuffer(nil)
+	job.Stdout.Add(outputBuffer)
 	if err := job.Run(); err != nil {
 		t.Error(err)
 	}
 
-	container2, _, _ := mkContainer(runtime, []string{newId}, t)
+	container2, _, _ := mkContainer(runtime, []string{engine.Tail(outputBuffer, 1)}, t)
 	defer runtime.Destroy(container2)
 
 	job = eng.Job("inspect", container1.Name, "container")
@@ -168,8 +170,6 @@ func TestRestartKillWait(t *testing.T) {
 
 	setTimeout(t, "Waiting on stopped container timedout", 5*time.Second, func() {
 		job = srv.Eng.Job("wait", outs.Data[0].Get("Id"))
-		var statusStr string
-		job.Stdout.AddString(&statusStr)
 		if err := job.Run(); err != nil {
 			t.Fatal(err)
 		}
@@ -266,8 +266,6 @@ func TestRunWithTooLowMemoryLimit(t *testing.T) {
 	job.Setenv("Memory", "524287")
 	job.Setenv("CpuShares", "1000")
 	job.SetenvList("Cmd", []string{"/bin/cat"})
-	var id string
-	job.Stdout.AddString(&id)
 	if err := job.Run(); err == nil {
 		t.Errorf("Memory limit is smaller than the allowed limit. Container creation should've failed!")
 	}
@@ -302,13 +300,13 @@ func TestRmi(t *testing.T) {
 
 	job = eng.Job("commit", containerID)
 	job.Setenv("repo", "test")
-	var imageID string
-	job.Stdout.AddString(&imageID)
+	var outputBuffer = bytes.NewBuffer(nil)
+	job.Stdout.Add(outputBuffer)
 	if err := job.Run(); err != nil {
 		t.Fatal(err)
 	}
 
-	if err := eng.Job("tag", imageID, "test", "0.1").Run(); err != nil {
+	if err := eng.Job("tag", engine.Tail(outputBuffer, 1), "test", "0.1").Run(); err != nil {
 		t.Fatal(err)
 	}
 
@@ -339,7 +337,7 @@ func TestRmi(t *testing.T) {
 		t.Fatalf("Expected 2 new images, found %d.", images.Len()-initialImages.Len())
 	}
 
-	if err = srv.DeleteImage(imageID, engine.NewTable("", 0), true, false, false); err != nil {
+	if err = srv.DeleteImage(engine.Tail(outputBuffer, 1), engine.NewTable("", 0), true, false, false); err != nil {
 		t.Fatal(err)
 	}
 

integration/utils_test.go

@@ -3,7 +3,6 @@ package docker
 import (
 	"bytes"
 	"fmt"
-	"github.com/dotcloud/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar"
 	"io"
 	"io/ioutil"
 	"net/http"
@@ -14,6 +13,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/dotcloud/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar"
+
 	"github.com/dotcloud/docker/builtins"
 	"github.com/dotcloud/docker/daemon"
 	"github.com/dotcloud/docker/engine"
@@ -42,11 +43,12 @@ func createNamedTestContainer(eng *engine.Engine, config *runconfig.Config, f ut
 	if err := job.ImportEnv(config); err != nil {
 		f.Fatal(err)
 	}
-	job.Stdout.AddString(&shortId)
+	var outputBuffer = bytes.NewBuffer(nil)
+	job.Stdout.Add(outputBuffer)
 	if err := job.Run(); err != nil {
 		f.Fatal(err)
 	}
-	return
+	return engine.Tail(outputBuffer, 1)
 }
 
 func createTestContainer(eng *engine.Engine, config *runconfig.Config, f utils.Fataler) (shortId string) {