Merge pull request #5752 from gyuho/mkdir

Make mkdir consistent

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)

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)
 	}
 

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"}

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

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 {

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
 	}
 

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
 	}