From 02927a3552304f2f4b1d226b897265d980ca5fcb Mon Sep 17 00:00:00 2001 From: Yusuke Kadowaki Date: Wed, 14 Jun 2023 18:12:28 +0900 Subject: [PATCH] Fix to create index_path when it does not exists (#2060) * stop checking if n.path exists * fix when CoW disabled * fix * add test for the bug condition * remove unused error type * define error * move ErrAgentMigrationFailed to agent.go --- internal/errors/agent.go | 5 +++ internal/errors/errors.go | 5 --- pkg/agent/core/ngt/service/ngt.go | 18 +++++----- pkg/agent/core/ngt/service/ngt_test.go | 47 +++++++++++++++++++++++--- 4 files changed, 58 insertions(+), 17 deletions(-) diff --git a/internal/errors/agent.go b/internal/errors/agent.go index 579d25e761..4f54d1ef04 100644 --- a/internal/errors/agent.go +++ b/internal/errors/agent.go @@ -104,4 +104,9 @@ var ( ErrObjectNotFound = func(err error, uuid string) error { return Wrapf(err, "uuid %s's object not found", uuid) } + + // ErrAgentMigrationFailed represents a function to generate an error that agent migration failed. + ErrAgentMigrationFailed = func(err error) error { + return Wrap(err, "index_path migration failed") + } ) diff --git a/internal/errors/errors.go b/internal/errors/errors.go index ecf39000fa..b9585230c9 100644 --- a/internal/errors/errors.go +++ b/internal/errors/errors.go @@ -80,11 +80,6 @@ var ( return Wrapf(err, "failed to output %s logs", str) } - // ErrIndexPathNotExists represents a function to generate an error that the index path is not exists. - ErrIndexPathNotExists = func(path string) error { - return Errorf("index path %s not exists", path) - } - // New represents a function to generate the new error with a message. // When the message is nil, it will return nil instead of an error. New = func(msg string) error { diff --git a/pkg/agent/core/ngt/service/ngt.go b/pkg/agent/core/ngt/service/ngt.go index becfc22e02..f7be454746 100644 --- a/pkg/agent/core/ngt/service/ngt.go +++ b/pkg/agent/core/ngt/service/ngt.go @@ -171,9 +171,6 @@ func New(cfg *config.NGT, opts ...Option) (nn NGT, err error) { // prepare directories to store index only when it not in-memory mode if !n.inMem { - if !file.Exists(n.path) { - return nil, errors.ErrIndexPathNotExists(n.path) - } ctx := context.Background() err = n.prepareFolders(ctx) if err != nil { @@ -216,12 +213,17 @@ func New(cfg *config.NGT, opts ...Option) (nn NGT, err error) { // which indicates that the user has NOT been using CoW mode and the index directory is not migrated yet. func migrate(ctx context.Context, path string) (err error) { // check if migration is required + if !file.Exists(path) { + log.Infof("the path %v does not exist. no need to migrate since it's probably the initial state", path) + return nil + } files, err := file.ListInDir(path) if err != nil { - return err + return errors.ErrAgentMigrationFailed(err) } if len(files) == 0 { // empty directory doesn't need migration + log.Infof("the path %v is empty. no need to migrate", path) return nil } od := filepath.Join(path, originIndexDirName) @@ -238,23 +240,23 @@ func migrate(ctx context.Context, path string) (err error) { // first move all contents to temporary directory because it's not possible to directly move directory to its subdirectory tp, err := file.MkdirTemp("") if err != nil { - return err + return errors.ErrAgentMigrationFailed(err) } err = file.MoveDir(ctx, path, tp) if err != nil { - return err + return errors.ErrAgentMigrationFailed(err) } // recreate the path again to move contents to `path/origin` lately err = file.MkdirAll(path, fs.ModePerm) if err != nil { - return err + return errors.ErrAgentMigrationFailed(err) } // finally move to `path/origin` directory err = file.MoveDir(ctx, tp, file.Join(path, originIndexDirName)) if err != nil { - return err + return errors.ErrAgentMigrationFailed(err) } return nil diff --git a/pkg/agent/core/ngt/service/ngt_test.go b/pkg/agent/core/ngt/service/ngt_test.go index c6de113167..7d4eb74da8 100644 --- a/pkg/agent/core/ngt/service/ngt_test.go +++ b/pkg/agent/core/ngt/service/ngt_test.go @@ -470,16 +470,55 @@ func TestNew(t *testing.T) { } }(), func() test { + tmpDir := t.TempDir() + indexDir := filepath.Join(tmpDir, "foo") // this does not exists when this test starts + brokenDir := filepath.Join(indexDir, brokenIndexDirName) + config := defaultConfig return test{ - name: "New fails with not existing index path", + name: "New creates `origin` and `backup` directory even when index path does not exist", args: args{ - cfg: &defaultConfig, + cfg: &config, opts: []Option{ - WithIndexPath("/dev/null/ghost"), + WithIndexPath(indexDir), }, }, want: want{ - err: errors.ErrIndexPathNotExists("/dev/null/ghost"), + err: nil, + }, + checkFunc: func(w want, err error) error { + if !errors.Is(err, w.err) { + return errors.Errorf("got_error: \"%#v\",\n\t\t\t\twant: \"%#v\"", err, w.err) + } + dirs, err := file.ListInDir(indexDir) + if err != nil { + return err + } + + // extract folder name from dir path into a map + dirSet := make(map[string]struct{}, len(dirs)) + for _, dir := range dirs { + // extract folder name from dir path + dirSet[filepath.Base(dir)] = struct{}{} + } + + // check if the dirs set contains folder names origin, backup and broken. + if _, ok := dirSet[originIndexDirName]; !ok { + return fmt.Errorf("failed to create origin dir") + } + if _, ok := dirSet[brokenIndexDirName]; !ok { + return fmt.Errorf("failed to create broken dir") + } + + // check if the broken index directory is empty + files, err := file.ListInDir(brokenDir) + if err != nil { + return err + } + if len(files) != 0 { + return fmt.Errorf("broken index directory is not empty") + } + + return nil }, } }(),