From 606c71882c9443dcbd9236b23a5ded7002bfc4d8 Mon Sep 17 00:00:00 2001 From: Rudy Zhang Date: Tue, 30 Oct 2018 12:59:55 +0800 Subject: [PATCH] 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 --- daemon/mgr/volume.go | 8 ++----- pkg/utils/utils.go | 16 ++++++++++++++ pkg/utils/utils_test.go | 26 ++++++++++++++++++++++ storage/volume/core.go | 9 ++------ storage/volume/modules/local/local.go | 22 +++++++++++++------ storage/volume/modules/tmpfs/tmpfs.go | 31 ++++++++++++++++++++++++--- storage/volume/types/volume.go | 9 +++++--- test/cli_volume_test.go | 9 ++++---- 8 files changed, 99 insertions(+), 31 deletions(-) diff --git a/daemon/mgr/volume.go b/daemon/mgr/volume.go index 2f3fd433a9..038999b3fe 100644 --- 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 index 9378b928f3..bfd6fd7d39 100644 --- 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 index 78d3499d9e..33f8bc6a83 100644 --- 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 index 2d8418daad..39257c5d53 100644 --- 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 index fd373bcbaf..a0a6b9e9d2 100644 --- 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 index 5887dbd891..cec15d43a2 100644 --- 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 index 8da4dbe1cd..f2c3c11f52 100644 --- 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 index 2badecc9e3..97d6999d2f 100644 --- 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