bugfix: add more option's to set volume's size

add more options to set volume's size, you also can use "-o size=10g" or "-o Size=10g" or "-o opt.Size=10g" to set volume's size. Signed-off-by: Rudy Zhang <[email protected]>
AliyunContainerService · Nov 2, 2018 · 606c718 · 606c718
1 parent 1d51e23
commit 606c718
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 31 deletions.
diff --git a/daemon/mgr/volume.go b/daemon/mgr/volume.go
@@ -6,6 +6,7 @@ import (
 
 	"github.com/alibaba/pouch/daemon/events"
 	"github.com/alibaba/pouch/pkg/errtypes"
+	"github.com/alibaba/pouch/pkg/utils"
 	"github.com/alibaba/pouch/storage/volume"
 	volerr "github.com/alibaba/pouch/storage/volume/error"
 	"github.com/alibaba/pouch/storage/volume/types"
@@ -201,12 +202,7 @@ func (vm *VolumeManager) Detach(ctx context.Context, name string, options map[st
 
 		if ref != "" {
 			ids := strings.Split(ref, ",")
-			for i, id := range ids {
-				if id == cid {
-					ids = append(ids[:i], ids[i+1:]...)
-				}
-			}
-
+			ids = utils.StringSliceDelete(ids, cid)
 			if len(ids) > 0 {
 				options[types.OptionRef] = strings.Join(ids, ",")
 			} else {

diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
@@ -376,3 +376,19 @@ func ToStringMap(in map[string]interface{}) map[string]string {
 	}
 	return out
 }
+
+// StringSliceDelete deletes the `del` string in string slice.
+func StringSliceDelete(in []string, del string) []string {
+	if in == nil {
+		return nil
+	}
+
+	out := make([]string, 0)
+	for _, value := range in {
+		if value != del {
+			out = append(out, value)
+		}
+	}
+
+	return out
+}
diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go
@@ -602,6 +602,32 @@ func TestToStringMap(t *testing.T) {
 		if m2[test.expect.key] != test.expect.value {
 			t.Fatalf("ToStringMap(%v) expected: %v, but got %v", test.m1, test.expect.value, m2[test.expect.key])
 		}
+	}
+}
+
+func TestStringSliceDelete(t *testing.T) {
+	type Expect struct {
+		index int
+		value string
+	}
+	tests := []struct {
+		s1     []string
+		del    string
+		expect *Expect
+	}{
+		{nil, "", nil},
+		{[]string{"a", "b", "a"}, "a", &Expect{0, "b"}},
+		{[]string{"a", "b", "a"}, "b", &Expect{0, "a"}},
+		{[]string{"a", "b", "a", "c"}, "a", &Expect{1, "c"}},
+	}
 
+	for _, test := range tests {
+		s2 := StringSliceDelete(test.s1, test.del)
+		if test.expect == nil && s2 != nil {
+			t.Fatalf("StringSliceDelete(%v) expected: nil, but got %v", test.s1, test.expect.value, s2)
+		}
+		if s2[test.expect.index] != test.expect.value {
+			t.Fatalf("ToStringMap(%v) expected: %v, but got %v", test.s1, test.expect.value, s2[test.expect.index])
+		}
 	}
 }
diff --git a/storage/volume/core.go b/storage/volume/core.go
@@ -229,11 +229,8 @@ func (c *Core) ListVolumes(labels map[string]string) ([]*types.Volume, error) {
 	ctx := driver.Contexts()
 
 	var realVolumes = map[string]*types.Volume{}
-	var volumeDrivers = map[string]driver.Driver{}
 
 	for _, dv := range drivers {
-		volumeDrivers[dv.Name(ctx)] = dv
-
 		d, ok := dv.(driver.Lister)
 		if !ok {
 			// not Lister, ignore it.
@@ -256,13 +253,11 @@ func (c *Core) ListVolumes(labels map[string]string) ([]*types.Volume, error) {
 			continue
 		}
 
-		d, ok := volumeDrivers[v.Spec.Backend]
-		if !ok {
-			// driver not exist, ignore it
+		d, err := driver.Get(v.Spec.Backend)
+		if err != nil || d == nil {
 			continue
 		}
 
-		// the local driver and tmpfs driver
 		if d.StoreMode(ctx).IsLocal() {
 			retVolumes = append(retVolumes, v)
 			continue

diff --git a/storage/volume/modules/local/local.go b/storage/volume/modules/local/local.go
@@ -55,16 +55,24 @@ func (p *Local) Create(ctx driver.Context, id types.VolumeContext) (*types.Volum
 
 	// parse the mount path.
 	if dir, ok := id.Options["mount"]; ok {
-		mountPath = path.Join(dir, id.Name)
+		mountPath = dir
 	}
 
 	// parse the size.
-	if value, ok := id.Options["opt.size"]; ok {
-		sizeInt, err := bytefmt.ToMegabytes(value)
+	s := ""
+	for _, k := range []string{"size", "opt.size", "Size", "opt.Size"} {
+		var ok bool
+		s, ok = id.Options[k]
+		if ok {
+			break
+		}
+	}
+	if s != "" {
+		sizeInt, err := bytefmt.ToBytes(s)
 		if err != nil {
 			return nil, err
 		}
-		size = strconv.Itoa(int(sizeInt)) + "M"
+		size = strconv.Itoa(int(sizeInt))
 	}
 
 	// create the volume path
@@ -97,13 +105,13 @@ func (p *Local) Path(ctx driver.Context, v *types.Volume) (string, error) {
 
 	mountPath := v.Option("mount")
 	if mountPath == "" {
-		mountPath = defaultDataPath
+		mountPath = path.Join(defaultDataPath, v.Name)
 		if p.DataPath != "" {
-			mountPath = p.DataPath
+			mountPath = path.Join(p.DataPath, v.Name)
 		}
 	}
 
-	return path.Join(mountPath, v.Name), nil
+	return mountPath, nil
 }
 
 // Options returns local volume's options.

diff --git a/storage/volume/modules/tmpfs/tmpfs.go b/storage/volume/modules/tmpfs/tmpfs.go
@@ -11,6 +11,7 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/alibaba/pouch/pkg/bytefmt"
 	"github.com/alibaba/pouch/pkg/utils"
 	"github.com/alibaba/pouch/storage/volume/driver"
 	"github.com/alibaba/pouch/storage/volume/types"
@@ -47,7 +48,25 @@ func (p *Tmpfs) Create(ctx driver.Context, id types.VolumeContext) (*types.Volum
 	// parse the mount path
 	mountPath := path.Join(dataDir, id.Name)
 
-	return types.NewVolumeFromContext(mountPath, "", id), nil
+	// parse the size, default size is 64M * 1024 * 1024
+	size := "67108864"
+	s := ""
+	for _, k := range []string{"size", "opt.size", "Size", "opt.Size"} {
+		var ok bool
+		s, ok = id.Options[k]
+		if ok {
+			break
+		}
+	}
+	if s != "" {
+		sizeInt, err := bytefmt.ToBytes(s)
+		if err != nil {
+			return nil, err
+		}
+		size = strconv.Itoa(int(sizeInt))
+	}
+
+	return types.NewVolumeFromContext(mountPath, size, id), nil
 }
 
 // Remove a tmpfs volume.
@@ -76,10 +95,16 @@ func (p *Tmpfs) Options() map[string]types.Option {
 func (p *Tmpfs) Attach(ctx driver.Context, v *types.Volume) error {
 	ctx.Log.Debugf("Tmpfs attach volume: %s", v.Name)
 	mountPath := v.Path()
-	size := v.Size()
 	reqID := v.Option("reqID")
-	ids := v.Option("ids")
 
+	size := v.Size()
+	sizeInt, err := bytefmt.ToKilobytes(size)
+	if err != nil {
+		return err
+	}
+	size = strconv.Itoa(int(sizeInt))
+
+	ids := v.Option("ids")
 	if ids != "" {
 		if !strings.Contains(ids, reqID) {
 			ids = ids + "," + reqID

diff --git a/storage/volume/types/volume.go b/storage/volume/types/volume.go
@@ -155,15 +155,18 @@ func (v *Volume) SetLabel(label, value string) {
 	v.Labels[label] = value
 }
 
-// Size returns volume's size(MB).
+// Size returns volume's size(bytes).
 func (v *Volume) Size() string {
 	if v.Spec.Size != "" {
 		return v.Spec.Size
 	}
 
-	if s, ok := v.Spec.Extra["Size"]; ok {
-		return s
+	for _, k := range []string{"size", "Size", "opt.size", "opt.Size"} {
+		if s, ok := v.Spec.Extra[k]; ok {
+			return s
+		}
 	}
+
 	return ""
 }
 

diff --git a/test/cli_volume_test.go b/test/cli_volume_test.go
@@ -60,7 +60,7 @@ func (suite *PouchVolumeSuite) TestVolumeCreateLocalDriverAndSpecifyMountPoint(c
 		funcname = tmpname[i]
 	}
 
-	res := command.PouchRun("volume", "create", "--name", funcname, "--driver", "local", "-o", "mount=/tmp")
+	res := command.PouchRun("volume", "create", "--name", funcname, "--driver", "local", "-o", "mount=/tmp/"+funcname)
 	res.Assert(c, icmd.Success)
 
 	res = command.PouchRun("volume", "inspect", funcname)
@@ -71,7 +71,7 @@ func (suite *PouchVolumeSuite) TestVolumeCreateLocalDriverAndSpecifyMountPoint(c
 	}
 
 	if !strings.Contains(output, "/tmp/"+funcname) {
-		c.Errorf("failed to get the mountpoint, expect:/tmp/%s, acturally: %s", funcname, output)
+		c.Errorf("failed to get the mountpoint, expect:/tmp, acturally: %s", output)
 	}
 
 	command.PouchRun("volume", "remove", funcname).Assert(c, icmd.Success)
@@ -94,7 +94,7 @@ func (suite *PouchVolumeSuite) TestVolumeCreateWithMountPointExitsFile(c *check.
 	icmd.RunCommand("touch", "/tmp/"+funcname)
 
 	err := command.PouchRun("volume", "create", "--name", funcname,
-		"--driver", "local", "-o", "mount=/tmp").Compare(expct)
+		"--driver", "local", "-o", "mount=/tmp/"+funcname).Compare(expct)
 	defer command.PouchRun("volume", "remove", funcname)
 
 	c.Assert(err, check.IsNil)
@@ -291,7 +291,7 @@ func (suite *PouchVolumeSuite) TestVolumeList(c *check.C) {
 	}
 }
 
-// TestVolumeList tests the volume list with options: size and mountpoint.
+// TestVolumeListOptions tests the volume list with options: size and mountpoint.
 func (suite *PouchVolumeSuite) TestVolumeListOptions(c *check.C) {
 	pc, _, _, _ := runtime.Caller(0)
 	tmpname := strings.Split(runtime.FuncForPC(pc).Name(), ".")
@@ -319,7 +319,6 @@ func (suite *PouchVolumeSuite) TestVolumeListOptions(c *check.C) {
 	for _, line := range strings.Split(ret.Stdout(), "\n") {
 		if strings.Contains(line, volumeName) {
 			if !strings.Contains(line, "local") ||
-				!strings.Contains(line, "M") ||
 				!strings.Contains(line, DefaultVolumeMountPath) {
 				c.Errorf("list result have no driver or name or size or mountpoint, line: %s", line)
 				break