Skip to content
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

[3.5] client/pkg/fileutil: add missing logger to {Create,Touch}DirAll #14799

Merged
merged 1 commit into from Nov 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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