From 5c25323157250ffb4b9745561d26760927f58d21 Mon Sep 17 00:00:00 2001 From: Jmnote Date: Tue, 18 Jul 2023 12:11:26 +0900 Subject: [PATCH] clean bugfix (#95) * clean bugfix * minor edit * minor edit on testcases * minor edit * add comment & parameter name for driver interface * . --- etc/{leth_sample.yaml => lethe.example.yaml} | 0 storage/driver/driver.go | 23 +++++----- storage/driver/error.go | 2 +- storage/driver/error_test.go | 32 ++++++++++++-- storage/driver/filesystem/driver.go | 9 ++-- storage/driver/filesystem/driver_test.go | 20 ++++----- storage/fileservice/clean.go | 18 ++++---- storage/fileservice/clean_test.go | 44 ++++++++++++++++++++ storage/fileservice/list_test.go | 12 +++--- 9 files changed, 117 insertions(+), 43 deletions(-) rename etc/{leth_sample.yaml => lethe.example.yaml} (100%) diff --git a/etc/leth_sample.yaml b/etc/lethe.example.yaml similarity index 100% rename from etc/leth_sample.yaml rename to etc/lethe.example.yaml diff --git a/storage/driver/driver.go b/storage/driver/driver.go index 2ed7a14..5825683 100644 --- a/storage/driver/driver.go +++ b/storage/driver/driver.go @@ -4,18 +4,19 @@ import ( "io" ) +// https://github.com/distribution/distribution/blob/v2.8.2/registry/storage/driver/storagedriver.go#L41 type Driver interface { Name() string - GetContent(string) ([]byte, error) - PutContent(string, []byte) error - Reader(string) (io.ReadCloser, error) - Writer(string) (FileWriter, error) - Stat(string) (FileInfo, error) - List(string) ([]string, error) - Move(string, string) error - Delete(string) error - Walk(string) ([]FileInfo, error) - WalkDir(string) ([]string, error) - Mkdir(string) error + GetContent(path string) ([]byte, error) + PutContent(path string, content []byte) error + Reader(path string) (io.ReadCloser, error) + Writer(path string) (FileWriter, error) + Stat(path string) (FileInfo, error) + List(path string) ([]string, error) + Move(sourcePath string, destPath string) error + Delete(path string) error + Walk(path string) ([]FileInfo, error) + WalkDir(path string) ([]string, error) + Mkdir(path string) error RootDirectory() string } diff --git a/storage/driver/error.go b/storage/driver/error.go index 2d8bbad..dfebc9c 100644 --- a/storage/driver/error.go +++ b/storage/driver/error.go @@ -8,5 +8,5 @@ type PathNotFoundError struct { } func (err PathNotFoundError) Error() string { - return fmt.Sprintf("Path not found: %s", err.Path) + return fmt.Sprintf("Path not found: %s, err: %s", err.Path, err.Err) } diff --git a/storage/driver/error_test.go b/storage/driver/error_test.go index f048a57..31341da 100644 --- a/storage/driver/error_test.go +++ b/storage/driver/error_test.go @@ -1,13 +1,39 @@ package driver import ( + "errors" "testing" "github.com/stretchr/testify/assert" ) func TestError(t *testing.T) { - err := PathNotFoundError{Path: "/tmp"} - got := err.Error() - assert.Equal(t, "Path not found: /tmp", got) + testCases := []struct { + path string + err error + wantError string + }{ + { + "", nil, + "Path not found: , err: %!s()", + }, + { + "", errors.New("hello"), + "Path not found: , err: hello", + }, + { + "/tmp", nil, + "Path not found: /tmp, err: %!s()", + }, + { + "/tmp", errors.New("hello"), + "Path not found: /tmp, err: hello", + }, + } + for _, tc := range testCases { + t.Run("", func(t *testing.T) { + err := PathNotFoundError{Path: tc.path, Err: tc.err} + assert.EqualError(t, err, tc.wantError) + }) + } } diff --git a/storage/driver/filesystem/driver.go b/storage/driver/filesystem/driver.go index 8f4b943..9879e59 100644 --- a/storage/driver/filesystem/driver.go +++ b/storage/driver/filesystem/driver.go @@ -159,14 +159,15 @@ func (d *driver) Move(sourcePath, targetPath string) error { } func (d *driver) Delete(subpath string) error { - if len(strings.Split(subpath, string(os.PathSeparator))) < 2 { - return fmt.Errorf("deleting 0-1 depth directory is not allowed") - } fullpath := d.fullPath(subpath) - _, err := os.Stat(fullpath) + fileInfo, err := os.Stat(fullpath) if err != nil { return storagedriver.PathNotFoundError{Path: subpath, Err: fmt.Errorf("stat err: %w", err)} } + depth := len(strings.Split(subpath, string(os.PathSeparator))) + if depth < 2 && fileInfo.IsDir() { + return fmt.Errorf("deleting 0-1 depth directory is not allowed") + } err = os.RemoveAll(fullpath) if err != nil { return fmt.Errorf("removeAll err: %w", err) diff --git a/storage/driver/filesystem/driver_test.go b/storage/driver/filesystem/driver_test.go index cdddfa5..050eae1 100644 --- a/storage/driver/filesystem/driver_test.go +++ b/storage/driver/filesystem/driver_test.go @@ -230,7 +230,7 @@ func TestStat(t *testing.T) { { "hello", "", - "Path not found: hello", + "Path not found: hello, err: stat err: stat tmp/storage_driver_filesystem_driver_test/hello: no such file or directory", }, { "node", @@ -250,7 +250,7 @@ func TestStat(t *testing.T) { { "pod/namespace01/hello.log", "", - "Path not found: pod/namespace01/hello.log", + "Path not found: pod/namespace01/hello.log, err: stat err: stat tmp/storage_driver_filesystem_driver_test/pod/namespace01/hello.log: no such file or directory", }, { "pod/namespace01/2009-11-10_21.log", @@ -291,7 +291,7 @@ func TestList(t *testing.T) { { "hello", nil, - "Path not found: hello", + "Path not found: hello, err: open err: open tmp/storage_driver_filesystem_driver_test/hello: no such file or directory", }, { "pod", @@ -330,7 +330,7 @@ func TestMove(t *testing.T) { }, { "hello", "", - "Path not found: hello", + "Path not found: hello, err: stat err: stat tmp/storage_driver_filesystem_driver_test/hello: no such file or directory", }, { "", "hello", @@ -338,11 +338,11 @@ func TestMove(t *testing.T) { }, { "hello", "hello", - "Path not found: hello", + "Path not found: hello, err: stat err: stat tmp/storage_driver_filesystem_driver_test/hello: no such file or directory", }, { "pod/namespace01/hello.log", "pod/namespace01/hello.log", - "Path not found: pod/namespace01/hello.log", + "Path not found: pod/namespace01/hello.log, err: stat err: stat tmp/storage_driver_filesystem_driver_test/pod/namespace01/hello.log: no such file or directory", }, { "pod/namespace01/2009-11-10_21.log", "pod/namespace01/2009-11-10_00.log", // move @@ -350,11 +350,11 @@ func TestMove(t *testing.T) { }, { "pod/namespace01/2009-11-10_21.log", "pod/namespace01/2009-11-10_00.log", // duplicate - "Path not found: pod/namespace01/2009-11-10_21.log", + "Path not found: pod/namespace01/2009-11-10_21.log, err: stat err: stat tmp/storage_driver_filesystem_driver_test/pod/namespace01/2009-11-10_21.log: no such file or directory", }, } - for i, tc := range testCases { - t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) { + for _, tc := range testCases { + t.Run("", func(t *testing.T) { err := driver1.Move(tc.a, tc.b) if tc.wantError == "" { assert.NoError(t, err) @@ -389,7 +389,7 @@ func TestDelete(t *testing.T) { }, { "pod/namespace01/2009-11-10_21.log", // duplicate - "Path not found: pod/namespace01/2009-11-10_21.log", + "Path not found: pod/namespace01/2009-11-10_21.log, err: stat err: stat tmp/storage_driver_filesystem_driver_test/pod/namespace01/2009-11-10_21.log: no such file or directory", }, } for i, tc := range testCases { diff --git a/storage/fileservice/clean.go b/storage/fileservice/clean.go index 8d62d58..2575da4 100644 --- a/storage/fileservice/clean.go +++ b/storage/fileservice/clean.go @@ -1,7 +1,7 @@ package fileservice import ( - "fmt" + "strings" "github.com/kuoss/common/logger" ) @@ -12,20 +12,22 @@ func (s *FileService) Clean() { } func (s *FileService) removeFilesWithPrefix(prefix string) { - files, err := s.driver.Walk(fmt.Sprintf("%s/%s.*", s.config.LogDataPath(), prefix)) + files, err := s.driver.List("") if err != nil { - logger.Warnf("glob err: %s, prefix: %s", err.Error(), prefix) + logger.Warnf("list err: %s, prefix: %s", err.Error(), prefix) return } if len(files) < 1 { return } - logger.Warnf("cleansing files prefix: %s", prefix) + logger.Warnf("cleanning files prefix: %s", prefix) for _, file := range files { - logger.Infof("remove file: %s", file) - err := s.driver.Delete(file.Fullpath()) - if err != nil { - logger.Warnf("remove err: %s, file: %s", err.Error(), file) + if strings.HasPrefix(file, prefix) { + logger.Infof("remove file: %s", file) + err := s.driver.Delete(file) + if err != nil { + logger.Warnf("remove err: %s, file: %s", err.Error(), file) + } } } } diff --git a/storage/fileservice/clean_test.go b/storage/fileservice/clean_test.go index 15c46f2..ba1c5fb 100644 --- a/storage/fileservice/clean_test.go +++ b/storage/fileservice/clean_test.go @@ -1 +1,45 @@ package fileservice + +import ( + "os" + "testing" + + "github.com/kuoss/lethe/config" + "github.com/kuoss/lethe/util/testutil" + "github.com/stretchr/testify/require" +) + +var ( + fileService_clean *FileService + logDataPath_clean string = "tmp/storage_fileservice_clean_test" +) + +func init() { + testutil.ChdirRoot() + testutil.ResetLogData() + + cfg, err := config.New("test") + if err != nil { + panic(err) + } + cfg.SetLogDataPath(logDataPath_clean) + fileService_clean, err = New(cfg) + if err != nil { + panic(err) + } +} + +func TestClean(t *testing.T) { + // setup + var err error + err = os.WriteFile("tmp/storage_fileservice_clean_test/kube.1", []byte("hello"), 0644) + require.NoError(t, err) + err = os.WriteFile("tmp/storage_fileservice_clean_test/host.1", []byte("hello"), 0644) + require.NoError(t, err) + + require.FileExists(t, "tmp/storage_fileservice_clean_test/kube.1") + require.FileExists(t, "tmp/storage_fileservice_clean_test/host.1") + fileService_clean.Clean() + require.NoFileExists(t, "tmp/storage_fileservice_clean_test/kube.1") + require.NoFileExists(t, "tmp/storage_fileservice_clean_test/host.1") +} diff --git a/storage/fileservice/list_test.go b/storage/fileservice/list_test.go index 2ee7acc..cbff686 100644 --- a/storage/fileservice/list_test.go +++ b/storage/fileservice/list_test.go @@ -36,7 +36,7 @@ func TestDirSize(t *testing.T) { wantError string }{ {"", 0, ""}, - {"hello", 0, "Path not found: hello"}, + {"hello", 0, "Path not found: hello, err: open err: open tmp/init/hello: no such file or directory"}, {"node", 0, ""}, {"pod", 0, ""}, {"node/node01", 1234, ""}, @@ -44,8 +44,8 @@ func TestDirSize(t *testing.T) { {"pod/namespace01", 2620, ""}, {"pod/namespace02", 1137, ""}, } - for i, tc := range testCases { - t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) { + for _, tc := range testCases { + t.Run("", func(t *testing.T) { got, err := fileService.dirSize(tc.path) if tc.wantError == "" { assert.NoError(t, err) @@ -76,7 +76,7 @@ func TestList(t *testing.T) { { "hello", nil, - "list err: Path not found: hello", + "list err: Path not found: hello, err: open err: open tmp/init/hello: no such file or directory", }, { "node", @@ -99,8 +99,8 @@ func TestList(t *testing.T) { "list err: readdirnames err: readdirent tmp/init/pod/namespace01/2029-11-10_23.log: not a directory", }, } - for i, tc := range testCases { - t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + for _, tc := range testCases { + t.Run("", func(t *testing.T) { got, err := fileService.List(tc.subpath) if tc.wantError == "" { assert.NoError(t, err)