From 187faba3e02060ffd613885f5bf4b74c5eba983b Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Wed, 22 Jun 2016 14:58:53 -0700 Subject: [PATCH 1/5] pkg/fileutil: fix TouchDirAll, add CreateDirAll os.MkdirAll never returns os.ErrExist. And add another function to ensure deepest directory is empty. --- pkg/fileutil/fileutil.go | 24 +++++++++++++++++++++++- pkg/fileutil/fileutil_test.go | 22 ++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/pkg/fileutil/fileutil.go b/pkg/fileutil/fileutil.go index ee3acad96..c963a7903 100644 --- a/pkg/fileutil/fileutil.go +++ b/pkg/fileutil/fileutil.go @@ -16,6 +16,7 @@ package fileutil import ( + "fmt" "io/ioutil" "os" "path" @@ -63,13 +64,34 @@ func ReadDir(dirpath string) ([]string, error) { // TouchDirAll is similar to os.MkdirAll. It creates directories with 0700 permission if any directory // does not exists. TouchDirAll also ensures the given directory is writable. func TouchDirAll(dir string) error { + // If path is already a directory, MkdirAll does nothing + // and returns nil. err := os.MkdirAll(dir, PrivateDirMode) - if err != nil && err != os.ErrExist { + if err != nil { + // if mkdirAll("a/text") and "text" is not + // a directory, this will return syscall.ENOTDIR return err } return IsDirWriteable(dir) } +// CreateDirAll is similar to TouchDirAll but returns error +// if the deepest directory was not empty. +func CreateDirAll(dir string) error { + err := TouchDirAll(dir) + if err == nil { + var ns []string + ns, err = ReadDir(dir) + if err != nil { + return err + } + if len(ns) != 0 { + err = fmt.Errorf("expected %q to be empty, got %q", dir, ns) + } + } + return err +} + func Exist(name string) bool { _, err := os.Stat(name) return err == nil diff --git a/pkg/fileutil/fileutil_test.go b/pkg/fileutil/fileutil_test.go index f0674cd9d..43644311c 100644 --- a/pkg/fileutil/fileutil_test.go +++ b/pkg/fileutil/fileutil_test.go @@ -21,6 +21,7 @@ import ( "path/filepath" "reflect" "runtime" + "strings" "testing" ) @@ -80,6 +81,27 @@ func TestReadDir(t *testing.T) { } } +func TestCreateDirAll(t *testing.T) { + tmpdir, err := ioutil.TempDir(os.TempDir(), "foo") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpdir) + + tmpdir2 := filepath.Join(tmpdir, "testdir") + if err = CreateDirAll(tmpdir2); err != nil { + t.Fatal(err) + } + + if err = ioutil.WriteFile(filepath.Join(tmpdir2, "text.txt"), []byte("test text"), PrivateFileMode); err != nil { + t.Fatal(err) + } + + if err = CreateDirAll(tmpdir2); err == nil || !strings.Contains(err.Error(), "to be empty, got") { + t.Fatalf("unexpected error %v", err) + } +} + func TestExist(t *testing.T) { f, err := ioutil.TempFile(os.TempDir(), "fileutil") if err != nil { From 5720fe812ebcc4f07697c69828a3ef820b2a0145 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Wed, 22 Jun 2016 15:01:27 -0700 Subject: [PATCH 2/5] etcdctl: use CreateDirAll --- etcdctl/ctlv2/command/backup_command.go | 4 ++-- etcdctl/ctlv3/command/snapshot_command.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/etcdctl/ctlv2/command/backup_command.go b/etcdctl/ctlv2/command/backup_command.go index c83761a15..9c7e2ffbf 100644 --- a/etcdctl/ctlv2/command/backup_command.go +++ b/etcdctl/ctlv2/command/backup_command.go @@ -17,11 +17,11 @@ package command import ( "fmt" "log" - "os" "path" "time" "github.com/coreos/etcd/etcdserver/etcdserverpb" + "github.com/coreos/etcd/pkg/fileutil" "github.com/coreos/etcd/pkg/idutil" "github.com/coreos/etcd/pkg/pbutil" "github.com/coreos/etcd/snap" @@ -65,7 +65,7 @@ func handleBackup(c *cli.Context) error { destWAL = path.Join(c.String("backup-dir"), "member", "wal") } - if err := os.MkdirAll(destSnap, 0700); err != nil { + if err := fileutil.CreateDirAll(destSnap); err != nil { log.Fatalf("failed creating backup snapshot dir %v: %v", destSnap, err) } ss := snap.New(srcSnap) diff --git a/etcdctl/ctlv3/command/snapshot_command.go b/etcdctl/ctlv3/command/snapshot_command.go index 3dc73189b..56b6e1f97 100644 --- a/etcdctl/ctlv3/command/snapshot_command.go +++ b/etcdctl/ctlv3/command/snapshot_command.go @@ -200,7 +200,7 @@ func initialClusterFromName(name string) string { // makeWAL creates a WAL for the initial cluster func makeWAL(waldir string, cl *membership.RaftCluster) { - if err := os.MkdirAll(waldir, 0755); err != nil { + if err := fileutil.CreateDirAll(waldir); err != nil { ExitWithError(ExitIO, err) } @@ -277,7 +277,7 @@ func makeDB(snapdir, dbfile string) { ExitWithError(ExitIO, err) } - if err := os.MkdirAll(snapdir, 0755); err != nil { + if err := fileutil.CreateDirAll(snapdir); err != nil { ExitWithError(ExitIO, err) } From c363fd288bf46033e0e42712d2534046353e6b94 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Wed, 22 Jun 2016 15:05:29 -0700 Subject: [PATCH 3/5] etcdserver: use CreateDirAll --- etcdserver/storage.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/etcdserver/storage.go b/etcdserver/storage.go index 41176ebdb..6453a1794 100644 --- a/etcdserver/storage.go +++ b/etcdserver/storage.go @@ -20,6 +20,7 @@ import ( "path" pb "github.com/coreos/etcd/etcdserver/etcdserverpb" + "github.com/coreos/etcd/pkg/fileutil" "github.com/coreos/etcd/pkg/pbutil" "github.com/coreos/etcd/pkg/types" "github.com/coreos/etcd/raft/raftpb" @@ -129,7 +130,7 @@ func makeMemberDir(dir string) error { case !os.IsNotExist(err): return err } - if err := os.MkdirAll(membdir, 0700); err != nil { + if err := fileutil.CreateDirAll(membdir); err != nil { return err } names := []string{"snap", "wal"} From 6cfc03a5f9f2f83b7259c3f74b62461839c9ba85 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Wed, 22 Jun 2016 15:07:04 -0700 Subject: [PATCH 4/5] wal: use CreateDirAll --- wal/wal.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wal/wal.go b/wal/wal.go index 5cfbc777c..d924a458f 100644 --- a/wal/wal.go +++ b/wal/wal.go @@ -97,7 +97,7 @@ func Create(dirpath string, metadata []byte) (*WAL, error) { return nil, err } } - if err := os.MkdirAll(tmpdirpath, fileutil.PrivateDirMode); err != nil { + if err := fileutil.CreateDirAll(tmpdirpath); err != nil { return nil, err } From 4a0f922a6cbd140e13796ced1ce69ab993aad506 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Wed, 22 Jun 2016 15:09:47 -0700 Subject: [PATCH 5/5] pkg/transport: use TouchDirAll --- pkg/transport/listener.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/transport/listener.go b/pkg/transport/listener.go index 0a745adfb..4e38bf95f 100644 --- a/pkg/transport/listener.go +++ b/pkg/transport/listener.go @@ -31,6 +31,7 @@ import ( "strings" "time" + "github.com/coreos/etcd/pkg/fileutil" "github.com/coreos/etcd/pkg/tlsutil" ) @@ -101,7 +102,7 @@ func (info TLSInfo) Empty() bool { } func SelfCert(dirpath string, hosts []string) (info TLSInfo, err error) { - if err = os.MkdirAll(dirpath, 0700); err != nil { + if err = fileutil.TouchDirAll(dirpath); err != nil { return }