-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce FS API #669
Introduce FS API #669
Changes from 13 commits
744c226
cbcdaec
b5385b6
11a6759
f7c3673
46bb3fb
8636fb6
6604ef5
2b1962b
9dced9a
ee36938
78d0361
4345fc9
e7b1424
2184795
12f44a2
91869fb
778c5d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,6 +38,12 @@ type AllocDir struct { | |
mounted []string | ||
} | ||
|
||
type AllocFileInfo struct { | ||
Name string | ||
IsDir bool | ||
Size int64 | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would add an interfacet:
|
||
func NewAllocDir(allocDir string) *AllocDir { | ||
d := &AllocDir{AllocDir: allocDir, TaskDirs: make(map[string]string)} | ||
d.SharedDir = filepath.Join(d.AllocDir, SharedAllocName) | ||
|
@@ -217,6 +223,48 @@ func (d *AllocDir) MountSharedDir(task string) error { | |
return nil | ||
} | ||
|
||
func (d *AllocDir) List(path string) ([]*AllocFileInfo, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All your public methods should have a comment. |
||
p := filepath.Join(d.AllocDir, path) | ||
finfos, err := ioutil.ReadDir(p) | ||
if err != nil { | ||
return []*AllocFileInfo{}, nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be an error? If you |
||
} | ||
files := make([]*AllocFileInfo, len(finfos)) | ||
for idx, info := range finfos { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
files[idx] = &AllocFileInfo{ | ||
Name: info.Name(), | ||
IsDir: info.IsDir(), | ||
Size: info.Size(), | ||
} | ||
} | ||
return files, err | ||
} | ||
|
||
func (d *AllocDir) Stat(path string) (*AllocFileInfo, error) { | ||
p := filepath.Join(d.AllocDir, path) | ||
info, err := os.Stat(p) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return &AllocFileInfo{ | ||
Size: info.Size(), | ||
Name: info.Name(), | ||
IsDir: info.IsDir(), | ||
}, nil | ||
} | ||
|
||
func (d *AllocDir) ReadAt(allocID string, path string, offset int64, limit int64, w io.Writer) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't need the allocID and you should return the LimitReader rather than take a writer. The caller can do the io.Copy if they want. |
||
p := filepath.Join(d.AllocDir, path) | ||
f, err := os.Open(p) | ||
if err != nil { | ||
return err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since these errors will be visible to end users, it may be worth wrapping them to make them more readable. |
||
} | ||
defer f.Close() | ||
io.Copy(w, io.LimitReader(f, limit)) | ||
return nil | ||
} | ||
|
||
func fileCopy(src, dst string, perm os.FileMode) error { | ||
// Do a simple copy. | ||
srcFile, err := os.Open(src) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ package client | |
|
||
import ( | ||
"fmt" | ||
"io" | ||
"io/ioutil" | ||
"log" | ||
"net" | ||
|
@@ -12,6 +13,7 @@ import ( | |
"time" | ||
|
||
"github.com/hashicorp/go-multierror" | ||
"github.com/hashicorp/nomad/client/allocdir" | ||
"github.com/hashicorp/nomad/client/config" | ||
"github.com/hashicorp/nomad/client/driver" | ||
"github.com/hashicorp/nomad/client/fingerprint" | ||
|
@@ -353,6 +355,31 @@ func (c *Client) Node() *structs.Node { | |
return c.config.Node | ||
} | ||
|
||
func (c *Client) FSList(allocID string, path string) ([]*allocdir.AllocFileInfo, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then the client can have a method called |
||
ar, ok := c.allocs[allocID] | ||
if !ok { | ||
return nil, fmt.Errorf("alloc not present") | ||
} | ||
|
||
return ar.FSList(path) | ||
} | ||
|
||
func (c *Client) FSStat(allocID string, path string) (*allocdir.AllocFileInfo, error) { | ||
ar, ok := c.allocs[allocID] | ||
if !ok { | ||
return nil, fmt.Errorf("alloc not found") | ||
} | ||
return ar.FSStat(path) | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spacing |
||
func (c *Client) FSReadAt(allocID string, path string, offset int64, limit int64, w io.Writer) error { | ||
ar, ok := c.allocs[allocID] | ||
if !ok { | ||
return fmt.Errorf("alloc not found") | ||
} | ||
return ar.FSReadAt(allocID, path, offset, limit, w) | ||
} | ||
|
||
// restoreState is used to restore our state from the data dir | ||
func (c *Client) restoreState() error { | ||
if c.config.DevMode { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
package agent | ||
|
||
import ( | ||
"fmt" | ||
"net/http" | ||
"strconv" | ||
"strings" | ||
) | ||
|
||
var ( | ||
allocIDNotPresentErr = fmt.Errorf("must provide a valid alloc id") | ||
fileNameNotPresentErr = fmt.Errorf("must provide a file name") | ||
) | ||
|
||
func (s *HTTPServer) DirectoryListRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { | ||
var allocID, path string | ||
|
||
if allocID = strings.TrimPrefix(req.URL.Path, "/v1/client/fs/ls/"); allocID == "" { | ||
return nil, allocIDNotPresentErr | ||
} | ||
if path = req.URL.Query().Get("path"); path == "" { | ||
path = "/" | ||
} | ||
return s.agent.client.FSList(allocID, path) | ||
} | ||
|
||
func (s *HTTPServer) FileStatRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { | ||
var allocID, path string | ||
if allocID = strings.TrimPrefix(req.URL.Path, "/v1/client/fs/stat/"); allocID == "" { | ||
return nil, allocIDNotPresentErr | ||
} | ||
if path = req.URL.Query().Get("path"); path == "" { | ||
return nil, fileNameNotPresentErr | ||
} | ||
return s.agent.client.FSStat(allocID, path) | ||
} | ||
|
||
func (s *HTTPServer) FileReadAtRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { | ||
var allocID, path string | ||
var offset, limit int64 | ||
var err error | ||
|
||
q := req.URL.Query() | ||
|
||
if allocID = strings.TrimPrefix(req.URL.Path, "/v1/client/fs/readat/"); allocID == "" { | ||
return nil, allocIDNotPresentErr | ||
} | ||
if path = q.Get("path"); path == "" { | ||
return nil, fileNameNotPresentErr | ||
} | ||
|
||
if offset, err = strconv.ParseInt(q.Get("offset"), 10, 64); err != nil { | ||
return nil, fmt.Errorf("error parsing offset: %v", err) | ||
} | ||
if limit, err = strconv.ParseInt(q.Get("limit"), 10, 64); err != nil { | ||
return nil, fmt.Errorf("error parsing limit: %v", err) | ||
} | ||
if err = s.agent.client.FSReadAt(allocID, path, offset, limit, resp); err != nil { | ||
return nil, err | ||
} | ||
return nil, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
package agent | ||
|
||
import ( | ||
"net/http" | ||
"net/http/httptest" | ||
"testing" | ||
) | ||
|
||
func TestHTTP_FSDirectoryList(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Append "_NoAllocID" to all them, because that is really what you are testing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dadgar Some tests are testing for other errors too. |
||
httpTest(t, nil, func(s *TestServer) { | ||
req, err := http.NewRequest("GET", "/v1/client/fs/ls/", nil) | ||
if err != nil { | ||
t.Fatalf("err: %v", err) | ||
} | ||
respW := httptest.NewRecorder() | ||
|
||
_, err = s.Server.DirectoryListRequest(respW, req) | ||
if err != allocIDNotPresentErr { | ||
t.Fatalf("expected err: %v, actual: %v", allocIDNotPresentErr, err) | ||
} | ||
}) | ||
} | ||
|
||
func TestHTTP_FSStat(t *testing.T) { | ||
httpTest(t, nil, func(s *TestServer) { | ||
req, err := http.NewRequest("GET", "/v1/client/fs/stat/", nil) | ||
if err != nil { | ||
t.Fatalf("err: %v", err) | ||
} | ||
respW := httptest.NewRecorder() | ||
|
||
_, err = s.Server.FileStatRequest(respW, req) | ||
if err != allocIDNotPresentErr { | ||
t.Fatalf("expected err: %v, actual: %v", allocIDNotPresentErr, err) | ||
} | ||
|
||
req, err = http.NewRequest("GET", "/v1/client/fs/stat/foo", nil) | ||
if err != nil { | ||
t.Fatalf("err: %v", err) | ||
} | ||
respW = httptest.NewRecorder() | ||
|
||
_, err = s.Server.FileStatRequest(respW, req) | ||
if err != fileNameNotPresentErr { | ||
t.Fatalf("expected err: %v, actual: %v", allocIDNotPresentErr, err) | ||
} | ||
|
||
}) | ||
} | ||
|
||
func TestHTTP_FSReadAt(t *testing.T) { | ||
httpTest(t, nil, func(s *TestServer) { | ||
req, err := http.NewRequest("GET", "/v1/client/fs/readat/", nil) | ||
if err != nil { | ||
t.Fatalf("err: %v", err) | ||
} | ||
respW := httptest.NewRecorder() | ||
|
||
_, err = s.Server.FileReadAtRequest(respW, req) | ||
if err == nil { | ||
t.Fatal("expected error") | ||
} | ||
|
||
req, err = http.NewRequest("GET", "/v1/client/fs/readat/foo", nil) | ||
if err != nil { | ||
t.Fatalf("err: %v", err) | ||
} | ||
respW = httptest.NewRecorder() | ||
|
||
_, err = s.Server.FileReadAtRequest(respW, req) | ||
if err == nil { | ||
t.Fatal("expected error") | ||
} | ||
|
||
req, err = http.NewRequest("GET", "/v1/client/fs/readat/foo?path=/path/to/file", nil) | ||
if err != nil { | ||
t.Fatalf("err: %v", err) | ||
} | ||
respW = httptest.NewRecorder() | ||
|
||
_, err = s.Server.FileReadAtRequest(respW, req) | ||
if err == nil { | ||
t.Fatal("expected error") | ||
} | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put a comment on the top of the struct