Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refector: refect the pouch's volume module #2379

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 91 additions & 72 deletions daemon/mgr/container_storage.go

Large diffs are not rendered by default.

32 changes: 15 additions & 17 deletions daemon/mgr/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
"github.com/alibaba/pouch/storage/volume/types"

Expand Down Expand Up @@ -65,12 +66,11 @@ func (vm *VolumeManager) Create(ctx context.Context, name, driver string, option
driver = types.DefaultBackend
}

id := types.VolumeID{
Name: name,
Driver: driver,
Options: map[string]string{},
Selectors: map[string]string{},
Labels: map[string]string{},
id := types.VolumeContext{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change VolumeID to VolumeContext, feel better.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐶

Name: name,
Driver: driver,
Options: map[string]string{},
Labels: map[string]string{},
}

if labels != nil {
Expand All @@ -83,6 +83,9 @@ func (vm *VolumeManager) Create(ctx context.Context, name, driver string, option

v, err := vm.core.CreateVolume(id)
if err != nil {
if errtypes.IsVolumeExisted(err) {
return v, nil
}
return nil, err
}

Expand All @@ -93,7 +96,7 @@ func (vm *VolumeManager) Create(ctx context.Context, name, driver string, option

// Get returns the information of volume that specified name/id.
func (vm *VolumeManager) Get(ctx context.Context, name string) (*types.Volume, error) {
id := types.VolumeID{
id := types.VolumeContext{
Name: name,
}
vol, err := vm.core.GetVolume(id)
Expand Down Expand Up @@ -123,7 +126,7 @@ func (vm *VolumeManager) Remove(ctx context.Context, name string) error {
return errors.Wrapf(errtypes.ErrVolumeInUse, "failed to remove volume(%s)", name)
}

id := types.VolumeID{
id := types.VolumeContext{
Name: name,
}
if err := vm.core.RemoveVolume(id); err != nil {
Expand All @@ -140,15 +143,15 @@ func (vm *VolumeManager) Remove(ctx context.Context, name string) error {

// Path returns the mount path of volume.
func (vm *VolumeManager) Path(ctx context.Context, name string) (string, error) {
id := types.VolumeID{
id := types.VolumeContext{
Name: name,
}
return vm.core.VolumePath(id)
}

// Attach is used to bind a volume to container.
func (vm *VolumeManager) Attach(ctx context.Context, name string, options map[string]string) (*types.Volume, error) {
id := types.VolumeID{
id := types.VolumeContext{
Name: name,
}

Expand Down Expand Up @@ -176,7 +179,7 @@ func (vm *VolumeManager) Attach(ctx context.Context, name string, options map[st

// Detach is used to unbind a volume from container.
func (vm *VolumeManager) Detach(ctx context.Context, name string, options map[string]string) (*types.Volume, error) {
id := types.VolumeID{
id := types.VolumeContext{
Name: name,
}

Expand All @@ -198,12 +201,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 {
Expand Down
12 changes: 7 additions & 5 deletions pkg/errtypes/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,8 @@ var (
// ErrNotImplemented represents that the function is not implemented.
ErrNotImplemented = errorType{codeNotImplemented, "not implemented"}

// ErrVolumeInUse represents that volume in use.
ErrVolumeInUse = errorType{codeInUse, "volume is in use"}

// ErrVolumeNotFound represents that no such volume.
ErrVolumeNotFound = errorType{codeNotFound, "no such volume"}
// ErrInUse represents that object is using.
ErrInUse = errorType{codeInUse, "in use"}

// ErrNotModified represents that the resource is not modified
ErrNotModified = errorType{codeNotModified, "not modified"}
Expand All @@ -50,6 +47,11 @@ const (
codeNotImplemented
codeInUse
codeNotModified

// volume error code
codeVolumeExisted
codeVolumeDriverNotFound
codeVolumeMetaNotFound
)

type errorType struct {
Expand Down
43 changes: 43 additions & 0 deletions pkg/errtypes/volume_errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package errtypes

var (
// ErrVolumeInUse represents that volume in use.
ErrVolumeInUse = errorType{codeInUse, "volume is in use"}

// ErrVolumeNotFound represents that no such volume.
ErrVolumeNotFound = errorType{codeNotFound, "no such volume"}

// ErrVolumeExisted represents error is "volume exist"
ErrVolumeExisted = errorType{codeVolumeExisted, "volume exist"}

// ErrVolumeDriverNotFound represents error is "driver not found"
ErrVolumeDriverNotFound = errorType{codeVolumeDriverNotFound, "driver not found"}

// ErrVolumeMetaNotFound represents error is "local meta not found"
ErrVolumeMetaNotFound = errorType{codeVolumeMetaNotFound, "local meta not found"}
)

// IsVolumeInUse is used to check error is volume in use.
func IsVolumeInUse(err error) bool {
return checkError(err, codeInUse)
}

// IsVolumeNotFound is used to check error is volumeNotFound or not.
func IsVolumeNotFound(err error) bool {
return checkError(err, codeNotFound)
}

// IsVolumeExisted is used to check error is volumeExisted or not.
func IsVolumeExisted(err error) bool {
return checkError(err, codeVolumeExisted)
}

// IsVolumeDriverNotFound is used to check error is driverNotFound or not.
func IsVolumeDriverNotFound(err error) bool {
return checkError(err, codeVolumeDriverNotFound)
}

// IsVolumeMetaNotFound is used to check error is localMetaNotFound or not.
func IsVolumeMetaNotFound(err error) bool {
return checkError(err, codeVolumeMetaNotFound)
}
19 changes: 0 additions & 19 deletions pkg/serializer/interfaces.go

This file was deleted.

63 changes: 0 additions & 63 deletions pkg/serializer/serialize.go

This file was deleted.

31 changes: 31 additions & 0 deletions pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,3 +361,34 @@ func StringDefault(s string, val string) string {
}
return val
}

// ToStringMap changes the map[string]interface{} to map[string]string,
// If the interface is not string, it will be ignore.
func ToStringMap(in map[string]interface{}) map[string]string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feel weird, enough though I can not figure out a more proper solution.

if in == nil {
return nil
}
out := make(map[string]string, 0)
for k, v := range in {
if s, ok := v.(string); ok {
out[k] = s
}
}
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
}
56 changes: 56 additions & 0 deletions pkg/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,3 +576,59 @@ func TestMergeMap(t *testing.T) {
}
}
}

func TestToStringMap(t *testing.T) {
type Expect struct {
isNil bool
key string
value string
}
tests := []struct {
m1 map[string]interface{}
expect Expect
}{
{nil, Expect{true, "", ""}},
{map[string]interface{}{"a": "a", "b": "b"}, Expect{false, "a", "a"}},
{map[string]interface{}{"a": "a", "b": "b"}, Expect{false, "b", "b"}},
{map[string]interface{}{"a": map[string]string{"aa": "aa"}, "b": "b"}, Expect{false, "a", ""}},
{map[string]interface{}{"a": map[string]string{"aa": "aa"}, "b": "b"}, Expect{false, "b", "b"}},
}

for _, test := range tests {
m2 := ToStringMap(test.m1)
if (test.expect.isNil && m2 != nil) || (!test.expect.isNil && m2 == nil) {
t.Fatalf("ToStringMap(%v) expected: %v, but got %v", test.m1, test.expect, m2)
}
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, s2)
}

if s2 != nil && s2[test.expect.index] != test.expect.value {
t.Fatalf("StringSliceDelete(%v) expected: %v, but got %v", test.s1, test.expect.value, s2[test.expect.index])
}
}
}
16 changes: 8 additions & 8 deletions storage/quota/grpquota.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,14 +229,6 @@ func (quota *GrpQuotaDriver) SetDiskQuota(dir string, size string, quotaID uint3
return errors.Errorf("failed to find mountpoint, dir: (%s)", dir)
}

id, err := quota.SetSubtree(dir, quotaID)
if err != nil {
return errors.Wrapf(err, "failed to set subtree, dir: (%s), quota id: (%d)", dir, quotaID)
}
if id == 0 {
return errors.Errorf("failed to find quota id to set subtree")
}

// transfer limit from kbyte to byte
limit, err := bytefmt.ToKilobytes(size)
if err != nil {
Expand All @@ -247,6 +239,14 @@ func (quota *GrpQuotaDriver) SetDiskQuota(dir string, size string, quotaID uint3
return err
}

id, err := quota.SetSubtree(dir, quotaID)
if err != nil {
return errors.Wrapf(err, "failed to set subtree, dir: (%s), quota id: (%d)", dir, quotaID)
}
if id == 0 {
return errors.Errorf("failed to find quota id to set subtree")
}

return quota.setQuota(id, limit, mountPoint)
}

Expand Down
Loading