-
Notifications
You must be signed in to change notification settings - Fork 180
add-go-fuse-to-inject-filesystem-error #583
base: master
Are you sure you want to change the base?
Changes from 74 commits
b97c06f
29b7209
394b39a
2472c78
ec0a5e0
75d7e04
c44cf44
a9ed232
81f7dd5
8ce9574
13374dd
62ae86f
5037567
406adcc
37157e9
b8478d4
d469dbf
261d9c5
f3808fe
b00d5bd
c1a3061
e11678e
4ac7384
bb5f22d
c8b6c6f
097610a
b634c3e
ccf974d
6aa7043
7ba8847
0795eb8
5de9085
ca04b2b
c607310
ddcc50d
9753d96
54774f1
5f15701
0417ed6
f3f984d
4bda5c0
8d09cc0
ac9068f
8c485d6
d9c5506
90f8077
80edb85
797257b
de99273
26b3fb8
0c0a19d
e16e3a7
ad6151f
59c387b
687fd7f
20534be
42b7859
9f79aa9
6d56002
9b0e24f
0e390ac
33e8785
0ac1b40
a545d05
43c2c03
f1cf68f
5859805
f7be8b1
0ebd0a7
9408f0d
8833d11
f9af261
0e6f527
e670888
f7c0590
719257b
213d516
bbc3ee7
33b31b8
6320020
0680777
e59acfa
998ef76
429f7b0
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 |
---|---|---|
@@ -1,20 +1,37 @@ | ||
dist: trusty | ||
language: go | ||
os: | ||
- windows | ||
- linux | ||
- osx | ||
|
||
go: | ||
- 1.12.x | ||
|
||
go_import_path: github.com/prometheus/tsdb | ||
matrix: | ||
include: | ||
- os: linux | ||
dist: trusty | ||
sudo: required | ||
go: 1.12.x | ||
install: | ||
- make deps | ||
script: | ||
- sudo -E apt-get -yq --no-install-suggests --no-install-recommends --force-yes install fuse | ||
- sudo modprobe fuse | ||
- sudo chmod a+rw /etc/fuse.conf | ||
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. why do we need these?
I think just 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. We need to write I remove following lines, but the
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. are you sure that 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. are you sure that sudo echo user_allow_other >> /etc/fuse.conf doesn't work without sudo chmod a+rw /etc/fuse.conf ? |
||
- sudo echo user_allow_other >> /etc/fuse.conf | ||
- make all | ||
- os: osx | ||
go: 1.12.x | ||
sudo: required | ||
osx_image: xcode8.3 | ||
script: | ||
- brew update | ||
- brew tap homebrew/cask | ||
- brew cask install osxfuse | ||
- sudo /Library/Filesystems/osxfuse.fs/Contents/Resources/load_osxfuse | ||
- sudo sysctl -w vfs.generic.osxfuse.tunables.allow_other=1 | ||
- make all | ||
- os: windows | ||
go: 1.12.x | ||
before_install: | ||
- choco install make | ||
install: | ||
- make deps | ||
script: | ||
- make test | ||
|
||
before_install: | ||
- if [[ "$TRAVIS_OS_NAME" == "windows" ]]; then choco install make; fi | ||
|
||
install: | ||
- make deps | ||
|
||
script: | ||
- if [[ "$TRAVIS_OS_NAME" == "windows" ]]; then make test; else make all; fi |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,130 @@ | ||
// +build linux darwin | ||
|
||
// Copyright 2019 The qiffang Authors | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package fuse | ||
|
||
import ( | ||
"fmt" | ||
"github.com/prometheus/tsdb/testutil" | ||
"os" | ||
"path/filepath" | ||
"testing" | ||
"time" | ||
|
||
"github.com/hanwen/go-fuse/fuse" | ||
"github.com/hanwen/go-fuse/fuse/nodefs" | ||
"github.com/hanwen/go-fuse/fuse/pathfs" | ||
) | ||
|
||
type loopBackFs struct { | ||
pathfs.FileSystem | ||
} | ||
|
||
// A FUSE filesystem that shunts all request to an underlying file system. | ||
// Its main purpose is to provide test coverage. | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
type hookFs struct { | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
original string | ||
mountpoint string | ||
fsName string | ||
loopBackFs | ||
hook Hook | ||
} | ||
|
||
func newHookFs(original string, mountpoint string, hook Hook) (*hookFs, error) { | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
hookfs := &hookFs{ | ||
original: original, | ||
mountpoint: mountpoint, | ||
fsName: "hookfs", | ||
loopBackFs: loopBackFs{pathfs.NewLoopbackFileSystem(original)}, | ||
hook: hook, | ||
} | ||
return hookfs, nil | ||
} | ||
|
||
// String implements hanwen/go-fuse/fuse/pathfs.FileSystem. You are not expected to call h manually. | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func (h *hookFs) String() string { | ||
return fmt.Sprintf("HookFs{Original=%s, Mountpoint=%s, FsName=%s, Underlying fs=%s, hook=%s}", | ||
h.original, h.mountpoint, h.fsName, h.loopBackFs.String(), h.hook) | ||
} | ||
|
||
// Rename implements hanwen/go-fuse/fuse/pathfs.FileSystem. You are not expected to call h manually. | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func (h *hookFs) Rename(oldName string, newName string, context *fuse.Context) fuse.Status { | ||
preHooked, err := h.hook.PreRename(oldName, newName) | ||
if preHooked { | ||
if err != nil { | ||
|
||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return fuse.ToStatus(err) | ||
} | ||
} | ||
|
||
status := h.loopBackFs.Rename(oldName, newName, context) | ||
|
||
postHooked, err := h.hook.PostRename(oldName, newName) | ||
if postHooked { | ||
if err != nil { | ||
return fuse.ToStatus(err) | ||
} | ||
} | ||
return status | ||
} | ||
|
||
func (h *hookFs) newServer() (*fuse.Server, error) { | ||
opts := &nodefs.Options{ | ||
NegativeTimeout: time.Second, | ||
AttrTimeout: time.Second, | ||
EntryTimeout: time.Second, | ||
} | ||
pathFsOpts := &pathfs.PathNodeFsOptions{ClientInodes: true} | ||
pathFs := pathfs.NewPathNodeFs(h, pathFsOpts) | ||
conn := nodefs.NewFileSystemConnector(pathFs.Root(), opts) | ||
originalAbs, _ := filepath.Abs(h.original) | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
mOpts := &fuse.MountOptions{ | ||
AllowOther: true, | ||
Name: h.fsName, | ||
FsName: originalAbs, | ||
} | ||
server, err := fuse.NewServer(conn.RawFS(), h.mountpoint, mOpts) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return server, nil | ||
} | ||
|
||
// NewServer creates a fuse server and attaches it to the given `mountpoint` directory. | ||
// It returns a function to `unmount` the given `mountpoint` directory. | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func NewServer(t *testing.T, original, mountpoint string, hook Hook) (clean func()) { | ||
fs, err := newHookFs(original, mountpoint, hook) | ||
testutil.Ok(t, err) | ||
|
||
server, err := fs.newServer() | ||
testutil.Ok(t, err) | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Async start fuse server, and it will be stopped when calling `fuse.Unmount()` method. | ||
go func() { | ||
server.Serve() | ||
}() | ||
|
||
testutil.Ok(t, server.WaitMount()) | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return func() { | ||
if err = server.Unmount(); err != nil { | ||
testutil.Ok(t, err) | ||
} | ||
|
||
testutil.Ok(t, os.RemoveAll(mountpoint)) | ||
testutil.Ok(t, os.RemoveAll(original)) | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
// Copyright 2019 The qiffang Authors | ||
krasi-georgiev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
krasi-georgiev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
package fuse | ||
|
||
import "testing" | ||
|
||
// NewServer is a mock method because fuse does not support Windows platform. | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func NewServer(t *testing.T, original, mountpoint string, hook Hook) (clean func()) { | ||
t.Skip("Skip windows platform") | ||
return func() {} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
// Copyright 2019 The qiffang Authors | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package fuse | ||
|
||
import ( | ||
"syscall" | ||
) | ||
|
||
// Hook is the base interface for user-written hooks. | ||
// You have to implement XXXHooK(e.g. `TestRenameHook`, ..). | ||
type Hook interface { | ||
krasi-georgiev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// If hooked is true, the real `rename()` would not be called. | ||
PreRename(oldPatgh string, newPath string) (hooked bool, err error) | ||
|
||
// If hooked is true, it will triggered after the real `rename()`. | ||
PostRename(oldPatgh string, newPath string) (hooked bool, err error) | ||
} | ||
|
||
// TestRenameHook is called on rename. | ||
type TestRenameHook struct { | ||
qiffang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Add the Empty hook so the that this struct implements the hook interface. | ||
EmptyHook | ||
} | ||
|
||
// These will take precedence over the `EmptyHook.PreRename()`. | ||
func (h TestRenameHook) PreRename(oldPatgh string, newPath string) (hooked bool, err error) { | ||
return true, syscall.EIO | ||
} | ||
|
||
// These will take precedence over the `EmptyHook.PostRename()`. | ||
func (h TestRenameHook) PostRename(oldPatgh string, newPath string) (hooked bool, err error) { | ||
return false, nil | ||
} | ||
|
||
// EmptyHook implements a Hook that returns false for every operation. | ||
type EmptyHook struct{} | ||
|
||
// PreRename is called on before real `rename()` method. | ||
func (h EmptyHook) PreRename(oldPatgh string, newPath string) (hooked bool, err error) { | ||
return false, nil | ||
} | ||
|
||
// PostRename is called on after real `rename()` method. | ||
func (h EmptyHook) PostRename(oldPatgh string, newPath string) (hooked bool, err error) { | ||
return false, nil | ||
} |
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.
Sorry maybe I wasn't clear. We still need to run all but the file system tests for Windows.
For example maybe put all unix OS specific tests in
block_unix_test.go
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.
@krasi-georgiev
Currently, go fuse library do not support windows platform, so there is a compile error in windows platform
I am trying to solve it and do you have any suggestions?
Thanks
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.
why do you think the library is loaded when running the tests under windows?
When you have something like
block_unix_test.go
the file should be loaded only on unix OS-esThere 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.
I think you should rename all fuse utils files to
fuse_..._unix.go
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.
For linux and macos, i add the build flag in golang files
Then if the platform is linux or osx, these go files will be automatic applied.
And for windows platform, the fuse_utils_windows.go will be automatic applied.
The advantage is that other developers can write fuse cases in everywhere and do not need to put fuse cases in a special directory.
From the build flag, i see that there is only linux、darwin、windows、freebsd suffix to support. I am not sure the file which suffix is unix can work.
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.
Good point, but looking at the code seems that these tests will fail under windows anyway as they will not use the fuse server mounts so can't inject errors and when the tests expects an error, no error will be returned.
yes inside you can put:
// +build !windows
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.
@krasi-georgiev
They do not fail under windows platform because fuse related cases will automatic skipped in windows platform(Other developer who want to inject file system error do not care it because it is automatic skipped in windows platform). In windows platform, it only run other cases which do not include fuse related cases.
For linux and macosx platform, it will run all cases. They works well.
I think the tests which need to inject filesystem errors only run in linux and macosx platform.
And in windows platform, it only run other cases, the fuse related cases will automatic skipped in windows platform. Because windows platform do not support fuse.
Every thing looks well, do you think it is ok?