Repository: dragonflyoss/Dragonfly2
Pull request #1446 (2022-07-08 09:52:06)

fix: dfpath creates redundant directories
Signed-off-by: Gaius <gaius.qi@gmail.com>

<!--- Provide a general summary of your changes in the Title above -->

## Description
- Fix dfpath creates redundant directories
<!--- Describe your changes in detail -->

## Related Issue
#1438 
<!--- This project only accepts pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps to reproduce -->
<!--- Please link to the issue here: -->

## Motivation and Context

<!--- Why is this change required? What problem does it solve? -->

## Screenshots (if appropriate)

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Documentation Update (if none of the other choices apply)

## Checklist

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have read the **CONTRIBUTING** document.
- [ ] I have added tests to cover my changes.



Patch:
diff --git a/cmd/dfget/cmd/daemon.go b/cmd/dfget/cmd/daemon.go
index d7adab58..13a0b559 100644
--- a/cmd/dfget/cmd/daemon.go
+++ b/cmd/dfget/cmd/daemon.go
@@ -104,17 +104,21 @@ func initDaemonDfpath(cfg *config.DaemonOption) (dfpath.Dfpath, error) {
 		options = append(options, dfpath.WithWorkHome(cfg.WorkHome))
 	}
 
-	if cfg.CacheDir != "" {
-		options = append(options, dfpath.WithCacheDir(cfg.CacheDir))
-	}
-
 	if cfg.LogDir != "" {
 		options = append(options, dfpath.WithLogDir(cfg.LogDir))
 	}
 
+	cacheDir := dfpath.DefaultCacheDir
+	if cfg.CacheDir != "" {
+		cacheDir = cfg.CacheDir
+	}
+	options = append(options, dfpath.WithCacheDir(cacheDir))
+
+	dataDir := dfpath.DefaultDataDir
 	if cfg.DataDir != "" {
-		options = append(options, dfpath.WithDataDir(cfg.DataDir))
+		dataDir = cfg.DataDir
 	}
+	options = append(options, dfpath.WithDataDir(dataDir))
 
 	return dfpath.New(options...)
 }
diff --git a/cmd/scheduler/cmd/root.go b/cmd/scheduler/cmd/root.go
index b6d90b82..b6fa910d 100644
--- a/cmd/scheduler/cmd/root.go
+++ b/cmd/scheduler/cmd/root.go
@@ -93,9 +93,11 @@ func initDfpath(cfg *config.ServerConfig) (dfpath.Dfpath, error) {
 		options = append(options, dfpath.WithLogDir(cfg.LogDir))
 	}
 
+	cacheDir := dfpath.DefaultCacheDir
 	if cfg.CacheDir != "" {
-		options = append(options, dfpath.WithCacheDir(cfg.CacheDir))
+		cacheDir = cfg.CacheDir
 	}
+	options = append(options, dfpath.WithCacheDir(cacheDir))
 
 	return dfpath.New(options...)
 }
diff --git a/go.mod b/go.mod
index 6e4c3de3..87e998c6 100644
--- a/go.mod
+++ b/go.mod
@@ -35,6 +35,7 @@ require (
 	github.com/google/uuid v1.3.0
 	github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
 	github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
+	github.com/hashicorp/go-multierror v1.1.1
 	github.com/jarcoal/httpmock v1.0.8
 	github.com/looplab/fsm v0.3.0
 	github.com/mcuadros/go-gin-prometheus v0.1.0
@@ -122,7 +123,6 @@ require (
 	github.com/googleapis/enterprise-certificate-proxy v0.1.0 // indirect
 	github.com/googleapis/gax-go/v2 v2.4.0 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
-	github.com/hashicorp/go-multierror v1.1.1 // indirect
 	github.com/hashicorp/go-uuid v1.0.3 // indirect
 	github.com/hashicorp/hcl v1.0.0 // indirect
 	github.com/inconshreveable/mousetrap v1.0.0 // indirect
diff --git a/pkg/dfpath/dfpath.go b/pkg/dfpath/dfpath.go
index a5548529..f3052ea1 100644
--- a/pkg/dfpath/dfpath.go
+++ b/pkg/dfpath/dfpath.go
@@ -19,14 +19,15 @@
 package dfpath
 
 import (
-	"fmt"
 	"io/fs"
 	"os"
 	"path/filepath"
 	"sync"
+
+	"github.com/hashicorp/go-multierror"
 )
 
-// Dfpath is the interface used for init project path
+// Dfpath is the interface used for init project path.
 type Dfpath interface {
 	WorkHome() string
 	CacheDir() string
@@ -38,7 +39,7 @@ type Dfpath interface {
 	DfgetLockPath() string
 }
 
-// Dfpath provides init project path function
+// Dfpath provides init project path function.
 type dfpath struct {
 	workHome       string
 	cacheDir       string
@@ -50,76 +51,103 @@ type dfpath struct {
 	dfgetLockPath  string
 }
 
-// Cache of the dfpath
+// Cache of the dfpath.
 var cache struct {
 	sync.Once
-	d    *dfpath
-	errs []error
+	d   *dfpath
+	err *multierror.Error
 }
 
-// Option is a functional option for configuring the dfpath
+// Option is a functional option for configuring the dfpath.
 type Option func(d *dfpath)
 
-// WithWorkHome set the workhome directory
+// WithWorkHome set the workhome directory.
 func WithWorkHome(dir string) Option {
 	return func(d *dfpath) {
 		d.workHome = dir
 	}
 }
 
-// WithCacheDir set the cache directory
+// WithCacheDir set the cache directory.
 func WithCacheDir(dir string) Option {
 	return func(d *dfpath) {
 		d.cacheDir = dir
 	}
 }
 
-// WithLogDir set the log directory
+// WithLogDir set the log directory.
 func WithLogDir(dir string) Option {
 	return func(d *dfpath) {
 		d.logDir = dir
 	}
 }
 
-// WithDataDir set download data directory
+// WithDataDir set download data directory.
 func WithDataDir(dir string) Option {
 	return func(d *dfpath) {
 		d.dataDir = dir
 	}
 }
 
-// New returns a new dfpath interface
+// WithPluginDir set plugin directory.
+func WithPluginDir(dir string) Option {
+	return func(d *dfpath) {
+		d.pluginDir = dir
+	}
+}
+
+// New returns a new dfpath interface.
 func New(options ...Option) (Dfpath, error) {
 	cache.Do(func() {
 		d := &dfpath{
-			workHome: DefaultWorkHome,
-			cacheDir: DefaultCacheDir,
-			logDir:   DefaultLogDir,
-			dataDir:  DefaultDataDir,
+			workHome:  DefaultWorkHome,
+			logDir:    DefaultLogDir,
+			pluginDir: DefaultPluginDir,
 		}
 
 		for _, opt := range options {
 			opt(d)
 		}
 
-		d.pluginDir = filepath.Join(d.workHome, "plugins")
+		// Initialize dfdaemon path.
 		d.daemonSockPath = filepath.Join(d.workHome, "daemon.sock")
 		d.daemonLockPath = filepath.Join(d.workHome, "daemon.lock")
 		d.dfgetLockPath = filepath.Join(d.workHome, "dfget.lock")
 
-		// Create directories
-		for name, dir := range map[string]string{"workHome": d.workHome, "cacheDir": d.cacheDir, "logDir": d.logDir, "dataDir": d.dataDir,
-			"pluginDir": d.pluginDir} {
-			if err := os.MkdirAll(dir, fs.FileMode(0755)); err != nil {
-				cache.errs = append(cache.errs, fmt.Errorf("create %s dir %s failed: %v", name, dir, err))
+		// Create workhome directory.
+		if err := os.MkdirAll(d.workHome, fs.FileMode(0755)); err != nil {
+			cache.err = multierror.Append(cache.err, err)
+		}
+
+		// Create log directory.
+		if err := os.MkdirAll(d.logDir, fs.FileMode(0755)); err != nil {
+			cache.err = multierror.Append(cache.err, err)
+		}
+
+		// Create plugin directory.
+		if err := os.MkdirAll(d.pluginDir, fs.FileMode(0755)); err != nil {
+			cache.err = multierror.Append(cache.err, err)
+		}
+
+		// Create cache directory.
+		if d.cacheDir != "" {
+			if err := os.MkdirAll(d.cacheDir, fs.FileMode(0755)); err != nil {
+				cache.err = multierror.Append(cache.err, err)
+			}
+		}
+
+		// Create data directory.
+		if d.dataDir != "" {
+			if err := os.MkdirAll(d.dataDir, fs.FileMode(0755)); err != nil {
+				cache.err = multierror.Append(cache.err, err)
 			}
 		}
 
 		cache.d = d
 	})
 
-	if len(cache.errs) > 0 {
-		return nil, fmt.Errorf("create dfpath failed: %s", cache.errs)
+	if cache.err.ErrorOrNil() != nil {
+		return nil, cache.err
 	}
 
 	d := *cache.d
diff --git a/pkg/dfpath/dfpath_darwin.go b/pkg/dfpath/dfpath_darwin.go
index 41590a72..1e52f44f 100644
--- a/pkg/dfpath/dfpath_darwin.go
+++ b/pkg/dfpath/dfpath_darwin.go
@@ -30,3 +30,4 @@ var DefaultCacheDir = filepath.Join(DefaultWorkHome, "cache")
 var DefaultConfigDir = filepath.Join(DefaultWorkHome, "config")
 var DefaultLogDir = filepath.Join(DefaultWorkHome, "logs")
 var DefaultDataDir = filepath.Join(DefaultWorkHome, "data")
+var DefaultPluginDir = filepath.Join(DefaultWorkHome, "plugins")
diff --git a/pkg/dfpath/dfpath_linux.go b/pkg/dfpath/dfpath_linux.go
index 589f72fc..62350489 100644
--- a/pkg/dfpath/dfpath_linux.go
+++ b/pkg/dfpath/dfpath_linux.go
@@ -24,3 +24,4 @@ var DefaultCacheDir = "/var/cache/dragonfly"
 var DefaultConfigDir = "/etc/dragonfly"
 var DefaultLogDir = "/var/log/dragonfly"
 var DefaultDataDir = "/var/lib/dragonfly"
+var DefaultPluginDir = "/usr/local/dragonfly/plugins"


Test patch:
diff --git a/pkg/dfpath/dfpath_test.go b/pkg/dfpath/dfpath_test.go
index 16975028..c5ac694f 100644
--- a/pkg/dfpath/dfpath_test.go
+++ b/pkg/dfpath/dfpath_test.go
@@ -20,6 +20,7 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -43,13 +44,14 @@ func TestNew(t *testing.T) {
 			expect: func(t *testing.T, options []Option) {
 				assert := assert.New(t)
 				cache.Once = sync.Once{}
-				cache.errs = []error{}
+				cache.err = &multierror.Error{}
 				d, err := New(options...)
 				assert.NoError(err)
 				assert.Equal(d.WorkHome(), DefaultWorkHome)
-				assert.Equal(d.CacheDir(), DefaultCacheDir)
+				assert.Equal(d.CacheDir(), "")
 				assert.Equal(d.LogDir(), DefaultLogDir)
-				assert.Equal(d.DataDir(), DefaultDataDir)
+				assert.Equal(d.DataDir(), "")
+				assert.Equal(d.PluginDir(), DefaultPluginDir)
 			},
 		},
 		{
@@ -58,13 +60,14 @@ func TestNew(t *testing.T) {
 			expect: func(t *testing.T, options []Option) {
 				assert := assert.New(t)
 				cache.Once = sync.Once{}
-				cache.errs = []error{}
+				cache.err = &multierror.Error{}
 				d, err := New(options...)
 				assert.NoError(err)
 				assert.Equal(d.WorkHome(), "foo")
-				assert.Equal(d.CacheDir(), DefaultCacheDir)
+				assert.Equal(d.CacheDir(), "")
 				assert.Equal(d.LogDir(), DefaultLogDir)
-				assert.Equal(d.DataDir(), DefaultDataDir)
+				assert.Equal(d.DataDir(), "")
+				assert.Equal(d.PluginDir(), DefaultPluginDir)
 			},
 		},
 		{
@@ -73,13 +76,14 @@ func TestNew(t *testing.T) {
 			expect: func(t *testing.T, options []Option) {
 				assert := assert.New(t)
 				cache.Once = sync.Once{}
-				cache.errs = []error{}
+				cache.err = &multierror.Error{}
 				d, err := New(options...)
 				assert.NoError(err)
 				assert.Equal(d.WorkHome(), DefaultWorkHome)
 				assert.Equal(d.CacheDir(), "foo")
 				assert.Equal(d.LogDir(), DefaultLogDir)
-				assert.Equal(d.DataDir(), DefaultDataDir)
+				assert.Equal(d.DataDir(), "")
+				assert.Equal(d.PluginDir(), DefaultPluginDir)
 			},
 		},
 		{
@@ -88,13 +92,30 @@ func TestNew(t *testing.T) {
 			expect: func(t *testing.T, options []Option) {
 				assert := assert.New(t)
 				cache.Once = sync.Once{}
-				cache.errs = []error{}
+				cache.err = &multierror.Error{}
 				d, err := New(options...)
 				assert.NoError(err)
 				assert.Equal(d.WorkHome(), DefaultWorkHome)
-				assert.Equal(d.CacheDir(), DefaultCacheDir)
+				assert.Equal(d.CacheDir(), "")
 				assert.Equal(d.LogDir(), "foo")
-				assert.Equal(d.DataDir(), DefaultDataDir)
+				assert.Equal(d.DataDir(), "")
+				assert.Equal(d.PluginDir(), DefaultPluginDir)
+			},
+		},
+		{
+			name:    "new dfpath by pluginDir",
+			options: []Option{WithPluginDir("foo")},
+			expect: func(t *testing.T, options []Option) {
+				assert := assert.New(t)
+				cache.Once = sync.Once{}
+				cache.err = &multierror.Error{}
+				d, err := New(options...)
+				assert.NoError(err)
+				assert.Equal(d.WorkHome(), DefaultWorkHome)
+				assert.Equal(d.CacheDir(), "")
+				assert.Equal(d.LogDir(), DefaultLogDir)
+				assert.Equal(d.DataDir(), "")
+				assert.Equal(d.PluginDir(), "foo")
 			},
 		},
 	}