bugfix: fix some volume bugs

don't return error when create volume is exist. Refect the `VolumeID` to `VolumeContext`. remove unuse field `Selectors` in `VolumeContext`. set directory's quota id after check the limit of device. get volume's size in `Extra` field. Signed-off-by: Rudy Zhang <[email protected]>
AliyunContainerService · Oct 30, 2018 · bdc5f46 · bdc5f46
1 parent 17eb976
commit bdc5f46
Show file tree

Hide file tree

Showing 22 changed files with 230 additions and 284 deletions.
diff --git a/daemon/mgr/container_storage.go b/daemon/mgr/container_storage.go
@@ -173,22 +173,29 @@ func (mgr *ContainerManager) getMountPointFromBinds(ctx context.Context, c *Cont
 			// volume bind.
 			name := mp.Source
 			if _, exist := volumeSet[name]; !exist {
-				mp.Name = name
-				mp.Source, mp.Driver, err = mgr.attachVolume(ctx, name, c)
+				_, mp.Driver, err = mgr.attachVolume(ctx, name, c)
 				if err != nil {
 					logrus.Errorf("failed to bind volume(%s): %v", name, err)
 					return errors.Wrap(err, "failed to bind volume")
 				}
 
-				volumeSet[mp.Name] = struct{}{}
+				volumeSet[name] = struct{}{}
 			}
 
-			if mp.Replace != "" {
-				mp.Source, err = mgr.VolumeMgr.Path(ctx, name)
-				if err != nil {
-					return err
-				}
+			volume, err := mgr.VolumeMgr.Get(ctx, name)
+			if err != nil || volume == nil {
+				logrus.Errorf("failed to get volume: (%s), err: (%v)", name, err)
+				return errors.Wrapf(err, "failed to get volume: (%s)", name)
+			}
+			mp.Driver = volume.Driver()
+			mp.Name = name
 
+			mp.Source, err = mgr.VolumeMgr.Path(ctx, name)
+			if err != nil {
+				return err
+			}
+
+			if mp.Replace != "" {
 				switch mp.Replace {
 				case "dr":
 					mp.Source = path.Join(mp.Source, mp.Destination)

diff --git a/daemon/mgr/volume.go b/daemon/mgr/volume.go
@@ -7,6 +7,7 @@ import (
 	"github.com/alibaba/pouch/daemon/events"
 	"github.com/alibaba/pouch/pkg/errtypes"
 	"github.com/alibaba/pouch/storage/volume"
+	volerr "github.com/alibaba/pouch/storage/volume/error"
 	"github.com/alibaba/pouch/storage/volume/types"
 
 	"github.com/pkg/errors"
@@ -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{
+		Name:    name,
+		Driver:  driver,
+		Options: map[string]string{},
+		Labels:  map[string]string{},
 	}
 
 	if labels != nil {
@@ -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 e, ok := err.(*volerr.CoreError); ok && e.IsVolumeExisted() {
+			return v, nil
+		}
 		return nil, err
 	}
 
@@ -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)
@@ -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 {
@@ -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,
 	}
 
@@ -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,
 	}
 

diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
@@ -361,3 +361,18 @@ 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 {
+	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
+}
diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go
@@ -576,3 +576,32 @@ 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])
+		}
+
+	}
+}
diff --git a/storage/quota/grpquota.go b/storage/quota/grpquota.go
@@ -233,14 +233,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 {
@@ -251,6 +243,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)
 }
 

diff --git a/storage/quota/prjquota.go b/storage/quota/prjquota.go
@@ -136,14 +136,6 @@ func (quota *PrjQuotaDriver) 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 {
@@ -154,6 +146,14 @@ func (quota *PrjQuotaDriver) SetDiskQuota(dir string, size string, quotaID uint3
 		return errors.Wrapf(err, "failed to check device limit, dir: (%s), limit: (%d)kb", dir, limit)
 	}
 
+	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)
 }
 

diff --git a/storage/quota/quota.go b/storage/quota/quota.go
@@ -127,7 +127,7 @@ func SetSubtree(dir string, qid uint32) (uint32, error) {
 
 // SetDiskQuota is used to set quota for directory.
 func SetDiskQuota(dir string, size string, quotaID uint32) error {
-	logrus.Infof("set disk quota, dir: (%s), size: (%s), quotaID: (%d)", dir, size, quotaID)
+	logrus.Infof("set disk quota, dir(%s), size(%s), quotaID(%d)", dir, size, quotaID)
 	return GQuotaDriver.SetDiskQuota(dir, size, quotaID)
 }
 
@@ -156,6 +156,21 @@ func GetNextQuotaID() (uint32, error) {
 	return GQuotaDriver.GetNextQuotaID()
 }
 
+// GetQuotaID returns the quota id of directory,
+// if no quota id, it will alloc the next available quota id.
+func GetQuotaID(dir string) (uint32, error) {
+	id := GetQuotaIDInFileAttr(dir)
+	if id > 0 {
+		return id, nil
+	}
+	id, err := GetNextQuotaID()
+	if err != nil {
+		return 0, errors.Wrapf(err, "failed to get file(%s) quota id", dir)
+	}
+
+	return id, nil
+}
+
 //GetDefaultQuota returns the default quota size.
 func GetDefaultQuota(quotas map[string]string) string {
 	if quotas == nil {
@@ -181,22 +196,24 @@ func GetDefaultQuota(quotas map[string]string) string {
 func SetRootfsDiskQuota(basefs, size string, quotaID uint32) (uint32, error) {
 	overlayMountInfo, err := getOverlayMountInfo(basefs)
 	if err != nil {
-		return 0, fmt.Errorf("failed to get overlay mount info: %v", err)
+		return 0, errors.Wrapf(err, "failed to get overlay(%s) mount info", basefs)
 	}
 
 	for _, dir := range []string{overlayMountInfo.Upper, overlayMountInfo.Work} {
 		_, err = StartQuotaDriver(dir)
 		if err != nil {
-			return 0, fmt.Errorf("failed to start quota driver: %v", err)
+			return 0, errors.Wrapf(err, "failed to start dir(%s) quota driver", dir)
 		}
 
-		quotaID, err = SetSubtree(dir, quotaID)
-		if err != nil {
-			return 0, fmt.Errorf("failed to set subtree: %v", err)
+		if quotaID == 0 {
+			quotaID, err = GetQuotaID(dir)
+			if err != nil {
+				return 0, errors.Wrapf(err, "failed to get dir(%s) quota id", dir)
+			}
 		}
 
 		if err := SetDiskQuota(dir, size, quotaID); err != nil {
-			return 0, fmt.Errorf("failed to set disk quota: %v", err)
+			return 0, errors.Wrapf(err, "failed to set dir(%s) disk quota", dir)
 		}
 	}
 
@@ -222,7 +239,7 @@ func SetQuotaForDir(src string, quotaID uint32) error {
 func getOverlayMountInfo(basefs string) (*OverlayMount, error) {
 	output, err := ioutil.ReadFile(procMountFile)
 	if err != nil {
-		logrus.Warnf("failed to ReadFile %s: %v", procMountFile, err)
+		logrus.Warnf("failed to read file(%s), err(%v)", procMountFile, err)
 		return nil, err
 	}
 
@@ -323,7 +340,7 @@ func loadQuotaIDs(repquotaOpt string) (map[uint32]struct{}, uint32, error) {
 			}
 		}
 	}
-	logrus.Infof("Load repquota ids: %d, list: %v", len(quotaIDs), quotaIDs)
+	logrus.Infof("Load repquota ids(%d), list(%v)", len(quotaIDs), quotaIDs)
 	return quotaIDs, minID, nil
 }
 
@@ -334,15 +351,15 @@ func getMountpoint(dir string) (string, error) {
 
 	output, err := ioutil.ReadFile(procMountFile)
 	if err != nil {
-		logrus.Warnf("failed to read file: (%s), err: (%v)", procMountFile, err)
-		return "", errors.Wrapf(err, "failed to read file: (%s)", procMountFile)
+		logrus.Warnf("failed to read file(%s), err(%v)", procMountFile, err)
+		return "", errors.Wrapf(err, "failed to read file(%s)", procMountFile)
 	}
 
 	devID, err := system.GetDevID(dir)
 	if err != nil {
-		return "", errors.Wrapf(err, "failed to get device id for dir: (%s)", dir)
+		return "", errors.Wrapf(err, "failed to get device id for dir(%s)", dir)
 	}
-	logrus.Debugf("get directory(%s) device id: (%d)", dir, devID)
+	logrus.Debugf("get dir(%s) device id(%d)", dir, devID)
 
 	// /dev/sdb1 /home/pouch ext4 rw,relatime,prjquota,data=ordered 0 0
 	for _, line := range strings.Split(string(output), "\n") {
@@ -372,10 +389,10 @@ func getMountpoint(dir string) (string, error) {
 	}
 
 	if mountPoint == "" {
-		return "", errors.Errorf("failed to get mount point of directory: (%s)", dir)
+		return "", errors.Errorf("failed to get mount point of dir(%s)", dir)
 	}
 
-	logrus.Debugf("get the directory: (%s) mountpoint: (%s)", dir, mountPoint)
+	logrus.Debugf("get the dir(%s)'s mountpoint(%s)", dir, mountPoint)
 
 	return mountPoint, nil
 }
@@ -391,36 +408,36 @@ func setDevLimit(dir string, devID uint64) (uint64, error) {
 
 	mp, err := getMountpoint(dir)
 	if err != nil {
-		return 0, errors.Wrapf(err, "failed to set device limit, dir: (%s), devID: (%d)", dir, devID)
+		return 0, errors.Wrapf(err, "failed to set device limit, dir(%s), devID(%d)", dir, devID)
 	}
 
 	newDevID, _ := system.GetDevID(mp)
 	if newDevID != devID {
-		return 0, errors.Errorf("failed to set device limit, no such device id: (%d), checked id: (%d)",
+		return 0, errors.Errorf("failed to set device limit, no such device id(%d), checked id(%d)",
 			devID, newDevID)
 	}
 
 	// get storage upper limit of the device which the dir is on.
 	var stfs syscall.Statfs_t
 	if err := syscall.Statfs(mp, &stfs); err != nil {
-		logrus.Errorf("failed to get path: (%s) limit, err: (%v)", mp, err)
-		return 0, errors.Wrapf(err, "failed to get path: (%s) limit", mp)
+		logrus.Errorf("failed to get path(%s) limit, err(%v)", mp, err)
+		return 0, errors.Wrapf(err, "failed to get path(%s) limit", mp)
 	}
 	limit = stfs.Blocks * uint64(stfs.Bsize)
 
 	lock.Lock()
 	devLimits[devID] = limit
 	lock.Unlock()
 
-	logrus.Debugf("SetDevLimit: dir: (%s), mountpoint: (%s), limit: (%v) B", dir, mp, limit)
+	logrus.Debugf("SetDevLimit: dir(%s), mountpoint(%s), limit(%v) B", dir, mp, limit)
 	return limit, nil
 }
 
 // checkDevLimit checks if the device on which the input dir lies has already been recorded in driver.
 func checkDevLimit(dir string, size uint64) error {
 	devID, err := system.GetDevID(dir)
 	if err != nil {
-		return errors.Wrapf(err, "failed to get device id, dir: (%s)", dir)
+		return errors.Wrapf(err, "failed to get device id, dir(%s)", dir)
 	}
 
 	lock.Lock()
@@ -429,7 +446,7 @@ func checkDevLimit(dir string, size uint64) error {
 	if !exist {
 		// if has not recorded, just add (dir, device, limit) to driver.
 		if limit, err = setDevLimit(dir, devID); err != nil {
-			return errors.Wrapf(err, "failed to set device limit, dir: (%s), devID: (%d)", dir, devID)
+			return errors.Wrapf(err, "failed to set device limit, dir(%s), devID: (%d)", dir, devID)
 		}
 	}