Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

darion: Use http.ServeContent for serving log data #102

Merged
merged 1 commit into from
May 14, 2018
Merged

darion: Use http.ServeContent for serving log data #102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 15 additions & 27 deletions darion/api/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"
"regexp"
"strings"
"time"

"github.com/Netflix/titus-executor/darion/conf"
"github.com/Netflix/titus-executor/filesystems"
Expand Down Expand Up @@ -90,7 +91,7 @@ func buildVirtualFileMapping(containerID, uriFileName string) map[string]virtual
return virtualFilemapping
}

func maybeVirtualFileStdioLogHandler(w io.Writer, r *http.Request, containerID, uriFileName string) error {
func maybeVirtualFileStdioLogHandler(w http.ResponseWriter, r *http.Request, containerID, uriFileName string) error {
virtualFilemapping := buildVirtualFileMapping(containerID, uriFileName)

mapping, ok := virtualFilemapping[path.Base(uriFileName)]
Expand All @@ -112,50 +113,37 @@ func maybeVirtualFileStdioLogHandler(w io.Writer, r *http.Request, containerID,
return err
}

_, err = fout.Seek(offset, io.SeekStart)
if err != nil {
log.Errorf("Error seeking in %s because: %v", fout.Name(), err)
return err
}

_, err = io.CopyN(w, fout, length)
if err != nil {
log.Errorf("Error writing %s because: %v", fout.Name(), err)
return err
}

csr := io.NewSectionReader(fout, offset, length)
http.ServeContent(w, r, fout.Name(), time.Now(), csr)
return nil
}

func logHandlerWithFile(w io.Writer, r *http.Request, fout *os.File) error {
func logHandlerWithFile(w http.ResponseWriter, r *http.Request, fout *os.File) error {

if filesystems.CheckFDForStdio(fout) {
return stdioLogHandlerWithFile(w, r, fout)
}
_, err := io.Copy(w, fout)
if err != nil {
log.Errorf("Error writing %s because: %v", fout.Name(), err)
}

http.ServeContent(w, r, fout.Name(), time.Now(), fout)
return nil
}

func stdioLogHandlerWithFile(w io.Writer, r *http.Request, fout *os.File) error {
func stdioLogHandlerWithFile(w http.ResponseWriter, r *http.Request, fout *os.File) error {
// Do stdio handler path
currentOffset, err := filesystems.GetCurrentOffset(fout)
if err != nil {
log.Errorf("Error getting current offest for %s because %v", fout.Name(), err)
return err
}
_, err = fout.Seek(currentOffset, io.SeekStart)
if err != nil {
log.Errorf("Error seeking in %s because: %v", fout.Name(), err)

size, e := fout.Seek(0, io.SeekEnd)
if e != nil {
log.Errorf("Error getting size for %s because %v", fout.Name(), e)
return err
}
_, err = io.Copy(w, fout)
if err != nil {
log.Errorf("Error writing %s because: %v", fout.Name(), err)
}

size = size - currentOffset
csr := io.NewSectionReader(fout, currentOffset, size)
http.ServeContent(w, r, fout.Name(), time.Now(), csr)
return nil
}

Expand Down
77 changes: 72 additions & 5 deletions darion/api/handlers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/Netflix/titus-executor/filesystems"
"github.com/Netflix/titus-executor/filesystems/xattr"
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
)

func init() {
Expand Down Expand Up @@ -99,6 +100,23 @@ func TestReadBaseVirtualFileLogs(t *testing.T) {
testReadLogs(verifyFunc, "/logs/Titus-fake-container?f=subdir/otherlogfile", t)
}

func TestReadBaseVirtualFileLogsRange(t *testing.T) {
verifyFunc := func(resp *http.Response, t *testing.T) {
dataStr := verifyHelper(resp, t)
if !strings.HasSuffix(dataStr, "\n") {
t.Fatal("Output truncated")
}
if len(dataStr) < 800 {
t.Fatal("Output truncated")
}
if strings.Count(dataStr, "z") != 800 {
t.Fatalf("Unexpected number of zs found: %s", dataStr)
}
}

testReadLogsRange(verifyFunc, "/logs/Titus-fake-container?f=subdir/otherlogfile", "bytes=200-", t)
}

func TestReadVirtualFileLogs(t *testing.T) {
verifyFunc := func(resp *http.Response, t *testing.T) {
dataStr := verifyHelper(resp, t)
Expand All @@ -116,6 +134,23 @@ func TestReadVirtualFileLogs(t *testing.T) {
testReadLogs(verifyFunc, "/logs/Titus-fake-container?f=subdir/otherlogfile.testsuffix", t)
}

func TestReadVirtualFileLogsRange(t *testing.T) {
verifyFunc := func(resp *http.Response, t *testing.T) {
dataStr := verifyHelper(resp, t)
if !strings.HasSuffix(dataStr, "\n") {
t.Fatal("Output truncated")
}
if len(dataStr) < 437 {
t.Fatal("Output truncated")
}
if strings.Count(dataStr, "a") != 437 {
t.Fatal("Unexpected number of as found")
}
}

testReadLogsRange(verifyFunc, "/logs/Titus-fake-container?f=subdir/otherlogfile.testsuffix", "bytes=200-", t)
}

func TestReadMissingVirtualFileLogs(t *testing.T) {
verifyFunc := func(resp *http.Response, t *testing.T) {
data, err := ioutil.ReadAll(resp.Body)
Expand All @@ -140,6 +175,28 @@ func TestReadLogs(t *testing.T) {
testReadLogs(verifyStdout, "/logs/Titus-fake-container?f=stdout", t)
}

func TestReadLogsRange(t *testing.T) {
verifyFunc := func(resp *http.Response, t *testing.T) {
dataStr := verifyHelper(resp, t)
if !strings.HasSuffix(dataStr, "\n") {
t.Fatal("Output truncated")
}
if len(dataStr) != 15 {
t.Fatalf("Output wrong length")
}
}
testReadLogsRange(verifyFunc, "/logs/Titus-fake-container?f=stdout", "bytes=5-", t)
}

func TestReadLogsOutOfRange(t *testing.T) {
verifyFunc := func(resp *http.Response, t *testing.T) {
if resp.StatusCode != 416 {
t.Fatal("Received non-416 return code: ", resp.StatusCode)
}
}
testReadLogsRange(verifyFunc, "/logs/Titus-fake-container?f=stdout", "bytes=100000-", t)
}

func TestReadLogsDefaultFile(t *testing.T) {
testReadLogs(verifyStdout, "/logs/Titus-fake-container", t)
}
Expand Down Expand Up @@ -168,7 +225,7 @@ func verifyHelper(resp *http.Response, t *testing.T) string {
t.Fatal("Could not read response body: ", err)
}
dataStr := string(data)
if resp.StatusCode != 200 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When will we receive not 200? When it's range request not satisfiable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that's the case, we should probably explicitly check for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a range is requested http specs say the server should respond with a 206 "Partial Content".

In general though, any code between 200 and 399 is considered a success. And success checks should allow for anything in that range. For example 304s would be pretty common. But I can change the test to only allow 200 or 206.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW the code for unsatisfiable range requests is 416.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want add a specific test checking for the 416 case, and for the 206 case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, added. Though at that point we are testing the http lib not our code.

if resp.StatusCode != 200 && resp.StatusCode != 206 {
t.Fatalf("Received non-200 return code (%d), with error: %s", resp.StatusCode, dataStr)
}
return string(data)
Expand All @@ -184,22 +241,32 @@ func verifyStdout(resp *http.Response, t *testing.T) {
}
}

func testReadLogs(verifyFunc func(resp *http.Response, t *testing.T), path string, t *testing.T) {
func testReadLogsRange(verifyFunc func(resp *http.Response, t *testing.T), path string, rangeHeader string, t *testing.T) {
conf.ContainersHome = "testdata"
r := http.NewServeMux()
r.HandleFunc("/logs/", LogHandler)
server := httptest.NewServer(r)
defer server.Close()

resp, err := http.Get(server.URL + path)
if err != nil {
t.Fatal("Unexpected error")
client := &http.Client{}
req, err := http.NewRequest("GET", server.URL+path, nil)
assert.NoError(t, err)

if rangeHeader != "" {
req.Header.Add("Range", rangeHeader)
}

resp, e := client.Do(req)
assert.NoError(t, e)
defer mustClose(resp.Body)

verifyFunc(resp, t)
}

func testReadLogs(verifyFunc func(resp *http.Response, t *testing.T), path string, t *testing.T) {
testReadLogsRange(verifyFunc, path, "", t)
}

func mustClose(f io.Closer) {
if err := f.Close(); err != nil {
panic(err)
Expand Down