diff --git a/api/server/server.go b/api/server/server.go index 3c93a3478d..e2a7f651a8 100644 --- a/api/server/server.go +++ b/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) } diff --git a/engine/engine.go b/engine/engine.go index 58b43eca04..5c3228d5d3 100644 --- a/engine/engine.go +++ b/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 diff --git a/engine/job.go b/engine/job.go index 7e655e1b12..ab8120dd44 100644 --- a/engine/job.go +++ b/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 } diff --git a/engine/job_test.go b/engine/job_test.go index 1f927cbafc..67e723988e 100644 --- a/engine/job_test.go +++ b/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) } diff --git a/engine/streams.go b/engine/streams.go index 6cc10d346d..99e876e17b 100644 --- a/engine/streams.go +++ b/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. diff --git a/engine/streams_test.go b/engine/streams_test.go index 30d31d2952..83dd05c6f4 100644 --- a/engine/streams_test.go +++ b/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!"}, + 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 { - 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!"}, - } - tests["One\nTwo\nThree"] = [][]string{ - {}, - {"Three"}, - {"Two", "Three"}, - {"One", "Two", "Three"}, - } - 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) } } } diff --git a/integration/runtime_test.go b/integration/runtime_test.go index 07207c9b53..9c59d38e01 100644 --- a/integration/runtime_test.go +++ b/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) diff --git a/integration/server_test.go b/integration/server_test.go index 226247556d..4da3e6e368 100644 --- a/integration/server_test.go +++ b/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) } diff --git a/integration/utils_test.go b/integration/utils_test.go index 6901662ce6..d8101dfb1d 100644 --- a/integration/utils_test.go +++ b/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) {