Skip to content

Commit

Permalink
Merge pull request #14799 from serathius/fix-client-fileutil-log
Browse files Browse the repository at this point in the history
[3.5] client/pkg/fileutil: add missing logger to {Create,Touch}DirAll
  • Loading branch information
serathius committed Nov 17, 2022
2 parents b282163 + c919780 commit 7ccca08
Show file tree
Hide file tree
Showing 12 changed files with 25 additions and 26 deletions.
10 changes: 3 additions & 7 deletions client/pkg/fileutil/fileutil.go
Expand Up @@ -44,16 +44,12 @@ func IsDirWriteable(dir 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 {
func TouchDirAll(lg *zap.Logger, dir string) error {
// If path is already a directory, MkdirAll does nothing and returns nil, so,
// first check if dir exist with an expected permission mode.
if Exist(dir) {
err := CheckDirPermission(dir, PrivateDirMode)
if err != nil {
lg, _ := zap.NewProduction()
if lg == nil {
lg = zap.NewExample()
}
lg.Warn("check file permission", zap.Error(err))
}
} else {
Expand All @@ -70,8 +66,8 @@ func TouchDirAll(dir string) error {

// CreateDirAll is similar to TouchDirAll but returns error
// if the deepest directory was not empty.
func CreateDirAll(dir string) error {
err := TouchDirAll(dir)
func CreateDirAll(lg *zap.Logger, dir string) error {
err := TouchDirAll(lg, dir)
if err == nil {
var ns []string
ns, err = ReadDir(dir)
Expand Down
6 changes: 3 additions & 3 deletions client/pkg/fileutil/fileutil_test.go
Expand Up @@ -67,15 +67,15 @@ func TestCreateDirAll(t *testing.T) {
defer os.RemoveAll(tmpdir)

tmpdir2 := filepath.Join(tmpdir, "testdir")
if err = CreateDirAll(tmpdir2); err != nil {
if err = CreateDirAll(zaptest.NewLogger(t), 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") {
if err = CreateDirAll(zaptest.NewLogger(t), tmpdir2); err == nil || !strings.Contains(err.Error(), "to be empty, got") {
t.Fatalf("unexpected error %v", err)
}
}
Expand Down Expand Up @@ -186,7 +186,7 @@ func TestDirPermission(t *testing.T) {

tmpdir2 := filepath.Join(tmpdir, "testpermission")
// create a new dir with 0700
if err = CreateDirAll(tmpdir2); err != nil {
if err = CreateDirAll(zaptest.NewLogger(t), tmpdir2); err != nil {
t.Fatal(err)
}
// check dir permission with mode different than created dir
Expand Down
2 changes: 1 addition & 1 deletion client/pkg/transport/listener.go
Expand Up @@ -205,7 +205,7 @@ func SelfCert(lg *zap.Logger, dirpath string, hosts []string, selfSignedCertVali
)
return
}
err = fileutil.TouchDirAll(dirpath)
err = fileutil.TouchDirAll(lg, dirpath)
if err != nil {
if info.Logger != nil {
info.Logger.Warn(
Expand Down
2 changes: 1 addition & 1 deletion etcdutl/etcdutl/backup_command.go
Expand Up @@ -114,7 +114,7 @@ func HandleBackup(withV3 bool, srcDir string, destDir string, srcWAL string, des
destWAL = datadir.ToWalDir(destDir)
}

if err := fileutil.CreateDirAll(destSnap); err != nil {
if err := fileutil.CreateDirAll(lg, destSnap); err != nil {
lg.Fatal("failed creating backup snapshot dir", zap.String("dest-snap", destSnap), zap.Error(err))
}

Expand Down
4 changes: 2 additions & 2 deletions etcdutl/snapshot/v3_snapshot.go
Expand Up @@ -322,7 +322,7 @@ func (s *v3Manager) copyAndVerifyDB() error {
return err
}

if err := fileutil.CreateDirAll(s.snapDir); err != nil {
if err := fileutil.CreateDirAll(s.lg, s.snapDir); err != nil {
return err
}

Expand Down Expand Up @@ -383,7 +383,7 @@ func (s *v3Manager) copyAndVerifyDB() error {
//
// TODO: This code ignores learners !!!
func (s *v3Manager) saveWALAndSnap() (*raftpb.HardState, error) {
if err := fileutil.CreateDirAll(s.walDir); err != nil {
if err := fileutil.CreateDirAll(s.lg, s.walDir); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion server/etcdmain/etcd.go
Expand Up @@ -276,7 +276,7 @@ func startProxy(cfg *config) error {
}

cfg.ec.Dir = filepath.Join(cfg.ec.Dir, "proxy")
err = fileutil.TouchDirAll(cfg.ec.Dir)
err = fileutil.TouchDirAll(lg, cfg.ec.Dir)
if err != nil {
return err
}
Expand Down
6 changes: 3 additions & 3 deletions server/etcdserver/server.go
Expand Up @@ -349,13 +349,13 @@ func NewServer(cfg config.ServerConfig) (srv *EtcdServer, err error) {
)
}

if terr := fileutil.TouchDirAll(cfg.DataDir); terr != nil {
if terr := fileutil.TouchDirAll(cfg.Logger, cfg.DataDir); terr != nil {
return nil, fmt.Errorf("cannot access data directory: %v", terr)
}

haveWAL := wal.Exist(cfg.WALDir())

if err = fileutil.TouchDirAll(cfg.SnapDir()); err != nil {
if err = fileutil.TouchDirAll(cfg.Logger, cfg.SnapDir()); err != nil {
cfg.Logger.Fatal(
"failed to create snapshot directory",
zap.String("path", cfg.SnapDir()),
Expand Down Expand Up @@ -548,7 +548,7 @@ func NewServer(cfg config.ServerConfig) (srv *EtcdServer, err error) {
return nil, fmt.Errorf("unsupported bootstrap config")
}

if terr := fileutil.TouchDirAll(cfg.MemberDir()); terr != nil {
if terr := fileutil.TouchDirAll(cfg.Logger, cfg.MemberDir()); terr != nil {
return nil, fmt.Errorf("cannot access member directory: %v", terr)
}

Expand Down
2 changes: 1 addition & 1 deletion server/wal/wal.go
Expand Up @@ -115,7 +115,7 @@ func Create(lg *zap.Logger, dirpath string, metadata []byte) (*WAL, error) {
}
defer os.RemoveAll(tmpdirpath)

if err := fileutil.CreateDirAll(tmpdirpath); err != nil {
if err := fileutil.CreateDirAll(lg, tmpdirpath); err != nil {
lg.Warn(
"failed to create a temporary WAL directory",
zap.String("tmp-dir-path", tmpdirpath),
Expand Down
7 changes: 4 additions & 3 deletions tests/functional/agent/handler.go
Expand Up @@ -473,7 +473,7 @@ func (srv *Server) handle_INITIAL_START_ETCD(req *rpcpb.Request) (*rpcpb.Respons
}, nil
}

err := fileutil.TouchDirAll(srv.Member.BaseDir)
err := fileutil.TouchDirAll(srv.lg, srv.Member.BaseDir)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -508,7 +508,7 @@ func (srv *Server) handle_INITIAL_START_ETCD(req *rpcpb.Request) (*rpcpb.Respons
func (srv *Server) handle_RESTART_ETCD(req *rpcpb.Request) (*rpcpb.Response, error) {
var err error
if !fileutil.Exist(srv.Member.BaseDir) {
err = fileutil.TouchDirAll(srv.Member.BaseDir)
err = fileutil.TouchDirAll(srv.lg, srv.Member.BaseDir)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -579,7 +579,7 @@ func (srv *Server) handle_SIGQUIT_ETCD_AND_REMOVE_DATA() (*rpcpb.Response, error

// create a new log file for next new member restart
if !fileutil.Exist(srv.Member.BaseDir) {
err = fileutil.TouchDirAll(srv.Member.BaseDir)
err = fileutil.TouchDirAll(srv.lg, srv.Member.BaseDir)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -651,6 +651,7 @@ func (srv *Server) handle_SIGQUIT_ETCD_AND_ARCHIVE_DATA() (*rpcpb.Response, erro

// TODO: support separate WAL directory
if err = archive(
srv.lg,
srv.Member.BaseDir,
srv.Member.Etcd.LogOutputs[0],
srv.Member.Etcd.DataDir,
Expand Down
6 changes: 4 additions & 2 deletions tests/functional/agent/utils.go
Expand Up @@ -25,15 +25,17 @@ import (
"time"

"go.etcd.io/etcd/client/pkg/v3/fileutil"

"go.uber.org/zap"
)

// TODO: support separate WAL directory
func archive(baseDir, etcdLogPath, dataDir string) error {
func archive(lg *zap.Logger, baseDir, etcdLogPath, dataDir string) error {
dir := filepath.Join(baseDir, "etcd-failure-archive", time.Now().Format(time.RFC3339))
if existDir(dir) {
dir = filepath.Join(baseDir, "etcd-failure-archive", time.Now().Add(time.Second).Format(time.RFC3339))
}
if err := fileutil.TouchDirAll(dir); err != nil {
if err := fileutil.TouchDirAll(lg, dir); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion tests/functional/tester/cluster.go
Expand Up @@ -520,7 +520,7 @@ func (clus *Cluster) sendOpWithResp(idx int, op rpcpb.Operation) (*rpcpb.Respons
"fixtures",
"client",
)
if err = fileutil.TouchDirAll(dirClient); err != nil {
if err = fileutil.TouchDirAll(clus.lg, dirClient); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion tests/functional/tester/cluster_run.go
Expand Up @@ -37,7 +37,7 @@ func (clus *Cluster) Run() {
// needs to obtain all the failpoints from the etcd member.
clus.updateCases()

if err := fileutil.TouchDirAll(clus.Tester.DataDir); err != nil {
if err := fileutil.TouchDirAll(clus.lg, clus.Tester.DataDir); err != nil {
clus.lg.Panic(
"failed to create test data directory",
zap.String("dir", clus.Tester.DataDir),
Expand Down

0 comments on commit 7ccca08

Please sign in to comment.