Repository: threefoldtech/zos
Pull request #1054 (2020-11-12 13:04:09)

Check if we have enough space in the pool for a vdisk
Also fixes a precision issue with the counters

LeeSmet: It seems you need to regenerate the stubs though as the interface changed

Patch:
diff --git a/pkg/provision/primitives/counters.go b/pkg/provision/primitives/counters.go
index ec358786..52cb9c10 100644
--- a/pkg/provision/primitives/counters.go
+++ b/pkg/provision/primitives/counters.go
@@ -81,11 +81,12 @@ func (c *Counters) CurrentWorkloads() directory.WorkloadAmount {
 
 // CurrentUnits return the number of each resource units reserved on the system
 func (c *Counters) CurrentUnits() directory.ResourceAmount {
+	gib := float64(gib)
 	return directory.ResourceAmount{
 		Cru: c.CRU.Current(),
-		Mru: float64(c.MRU.Current() / gib),
-		Hru: float64(c.HRU.Current() / gib),
-		Sru: float64(c.SRU.Current() / gib),
+		Mru: float64(c.MRU.Current()) / gib,
+		Hru: float64(c.HRU.Current()) / gib,
+		Sru: float64(c.SRU.Current()) / gib,
 	}
 }
 
diff --git a/pkg/storage.go b/pkg/storage.go
index fc187788..f325abc5 100644
--- a/pkg/storage.go
+++ b/pkg/storage.go
@@ -168,6 +168,11 @@ type VolumeAllocater interface {
 
 	// GetVdiskFS return the filesystem used to store the vdisk file for the VM module
 	GetVdiskFS() (Filesystem, error)
+
+	// CanAllocate checks if the given subvolume name can use this given size
+	// it checks against physical disk space first if there is enough capacity
+	// to support this, and if okay, it checks against the subvolume quota limit if set
+	CanAllocate(name string, size uint64) (bool, error)
 }
 
 // VDisk info returned by a call to inspect
diff --git a/pkg/storage/disk.go b/pkg/storage/disk.go
index a7b97325..20b2aa84 100644
--- a/pkg/storage/disk.go
+++ b/pkg/storage/disk.go
@@ -19,6 +19,7 @@ const (
 )
 
 type vdiskModule struct {
+	v    pkg.VolumeAllocater
 	path string
 }
 
@@ -33,7 +34,7 @@ func NewVDiskModule(v pkg.VolumeAllocater) (pkg.VDiskModule, error) {
 		return nil, err
 	}
 
-	return &vdiskModule{path: filepath.Clean(fs.Path)}, nil
+	return &vdiskModule{v: v, path: filepath.Clean(fs.Path)}, nil
 }
 
 // AllocateDisk with given size, return path to virtual disk (size in MB)
@@ -48,6 +49,16 @@ func (d *vdiskModule) Allocate(id string, size int64) (string, error) {
 		return path, errors.Wrapf(os.ErrExist, "disk with id '%s' already exists", id)
 	}
 
+	supported, err := d.v.CanAllocate(vdiskVolumeName, uint64(size))
+	if err != nil {
+		return path, errors.Wrap(err, "failed to check capacity for this disk allocation")
+	}
+
+	if !supported {
+		// TODO: we need to find another disk on this node if possible
+		return path, fmt.Errorf("not enough space available for this disk size")
+	}
+
 	file, err := os.Create(path)
 	if err != nil {
 		return "", err
diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go
index f63ca6d1..169e6d68 100644
--- a/pkg/storage/storage.go
+++ b/pkg/storage/storage.go
@@ -465,22 +465,29 @@ func (s *storageModule) ListFilesystems() ([]pkg.Filesystem, error) {
 // Path return the path of the mountpoint of the named filesystem
 // if no volume with name exists, an empty path and an error is returned
 func (s *storageModule) Path(name string) (pkg.Filesystem, error) {
+	_, fs, err := s.path(name)
+	return fs, err
+}
+
+// Path return the path of the mountpoint of the named filesystem
+// if no volume with name exists, an empty path and an error is returned
+func (s *storageModule) path(name string) (filesystem.Pool, pkg.Filesystem, error) {
 	for _, pool := range s.pools {
 		if _, mounted := pool.Mounted(); !mounted {
 			continue
 		}
 		filesystems, err := pool.Volumes()
 		if err != nil {
-			return pkg.Filesystem{}, err
+			return nil, pkg.Filesystem{}, err
 		}
 		for _, fs := range filesystems {
 			if fs.Name() == name {
 				usage, err := fs.Usage()
 				if err != nil {
-					return pkg.Filesystem{}, err
+					return nil, pkg.Filesystem{}, err
 				}
 
-				return pkg.Filesystem{
+				return pool, pkg.Filesystem{
 					ID:     fs.ID(),
 					FsType: fs.FsType(),
 					Name:   fs.Name(),
@@ -495,7 +502,47 @@ func (s *storageModule) Path(name string) (pkg.Filesystem, error) {
 		}
 	}
 
-	return pkg.Filesystem{}, errors.Wrapf(os.ErrNotExist, "subvolume '%s' not found", name)
+	return nil, pkg.Filesystem{}, errors.Wrapf(os.ErrNotExist, "subvolume '%s' not found", name)
+}
+
+func (s *storageModule) CanAllocate(name string, size uint64) (bool, error) {
+	pool, fs, err := s.path(name)
+	if err != nil {
+		return false, err
+	}
+
+	usage, err := pool.Usage()
+	if err != nil {
+		return false, err
+	}
+
+	reserved, err := pool.Reserved()
+	if err != nil {
+		return false, err
+	}
+
+	if usage.Used+size > usage.Size {
+		// disk does not have enough space for this size
+		return false, nil
+	}
+
+	if reserved+size > usage.Size {
+		// check that we won't go over reserved capacity as well
+		return false, nil
+	}
+
+	// so disk have enough capacity, now we need to check if the
+	// subvolume limit actually supports this
+	if fs.Usage.Size > 0 {
+		// we only validate the quota limit if and only if
+		// quota is set
+		if fs.Usage.Used+size > fs.Usage.Size {
+			return false, nil
+		}
+	}
+
+	// otherwise it's okay
+	return true, nil
 }
 
 // GetCacheFS return the special filesystem used by 0-OS to store internal state and flist cache
diff --git a/pkg/stubs/storage_stub.go b/pkg/stubs/storage_stub.go
index 4b52f7e2..435b36c6 100644
--- a/pkg/stubs/storage_stub.go
+++ b/pkg/stubs/storage_stub.go
@@ -63,6 +63,22 @@ func (s *StorageModuleStub) BrokenPools() (ret0 []pkg.BrokenPool) {
 	return
 }
 
+func (s *StorageModuleStub) CanAllocate(arg0 string, arg1 uint64) (ret0 bool, ret1 error) {
+	args := []interface{}{arg0, arg1}
+	result, err := s.client.Request(s.module, s.object, "CanAllocate", args...)
+	if err != nil {
+		panic(err)
+	}
+	if err := result.Unmarshal(0, &ret0); err != nil {
+		panic(err)
+	}
+	ret1 = new(zbus.RemoteError)
+	if err := result.Unmarshal(1, &ret1); err != nil {
+		panic(err)
+	}
+	return
+}
+
 func (s *StorageModuleStub) CreateFilesystem(arg0 string, arg1 uint64, arg2 pkg.DeviceType) (ret0 pkg.Filesystem, ret1 error) {
 	args := []interface{}{arg0, arg1, arg2}
 	result, err := s.client.Request(s.module, s.object, "CreateFilesystem", args...)


Test patch:
diff --git a/pkg/flist/flist_test.go b/pkg/flist/flist_test.go
index e4a65753..22af3cc9 100644
--- a/pkg/flist/flist_test.go
+++ b/pkg/flist/flist_test.go
@@ -58,6 +58,11 @@ func (s *StorageMock) Path(name string) (pkg.Filesystem, error) {
 	}, args.Error(1)
 }
 
+func (s *StorageMock) CanAllocate(name string, size uint64) (bool, error) {
+	args := s.Called(name, size)
+	return args.Bool(0), args.Error(1)
+}
+
 // GetCacheFS return the special filesystem used by 0-OS to store internal state and flist cache
 func (s *StorageMock) GetCacheFS() (pkg.Filesystem, error) {
 	return pkg.Filesystem{}, nil
diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go
index cd002a11..659ce753 100644
--- a/pkg/storage/storage_test.go
+++ b/pkg/storage/storage_test.go
@@ -13,7 +13,8 @@ import (
 
 type testVolume struct {
 	mock.Mock
-	name string
+	name  string
+	usage filesystem.Usage
 }
 
 func (p *testVolume) ID() int {
@@ -25,7 +26,7 @@ func (p *testVolume) Path() string {
 }
 
 func (p *testVolume) Usage() (filesystem.Usage, error) {
-	return filesystem.Usage{}, fmt.Errorf("not implemented")
+	return p.usage, nil
 }
 
 func (p *testVolume) Limit(size uint64) error {
@@ -80,23 +81,23 @@ func (p *testPool) Mounted() (string, bool) {
 }
 
 func (p *testPool) Mount() (string, error) {
-	return "", fmt.Errorf("not implemented")
+	return "", fmt.Errorf("Mount not implemented")
 }
 
 func (p *testPool) MountWithoutScan() (string, error) {
-	return "", fmt.Errorf("not implemented")
+	return "", fmt.Errorf("MountWithoutScan not implemented")
 }
 
 func (p *testPool) UnMount() error {
-	return fmt.Errorf("not implemented")
+	return fmt.Errorf("UnMount not implemented")
 }
 
 func (p *testPool) AddDevice(_ *filesystem.Device) error {
-	return fmt.Errorf("not implemented")
+	return fmt.Errorf("AddDevice not implemented")
 }
 
 func (p *testPool) RemoveDevice(_ *filesystem.Device) error {
-	return fmt.Errorf("not implemented")
+	return fmt.Errorf("RemoveDevice not implemented")
 }
 
 func (p *testPool) Type() pkg.DeviceType {
@@ -286,3 +287,120 @@ func TestCreateSubvolNoSpaceLeft(t *testing.T) {
 
 	require.EqualError(err, "Not enough space left in pools of this type ssd")
 }
+
+func TestCanAllocateUsageNoReservation(t *testing.T) {
+	require := require.New(t)
+
+	pool1 := &testPool{
+		name:     "pool-1",
+		reserved: 0,
+		usage: filesystem.Usage{
+			Size: 10000,
+			Used: 100,
+		},
+		ptype: pkg.SSDDevice,
+	}
+
+	mod := &storageModule{
+		pools: []filesystem.Pool{
+			pool1,
+		},
+	}
+
+	pool1.On("Volumes").Return([]filesystem.Volume{
+		&testVolume{
+			name: "vdisk",
+		},
+	}, nil)
+
+	can, err := mod.CanAllocate("vdisk", 100)
+	require.NoError(err)
+	require.True(can)
+
+	can, err = mod.CanAllocate("vdisk", 1900)
+	require.NoError(err)
+	require.True(can)
+
+	can, err = mod.CanAllocate("vdisk", 10000)
+	require.NoError(err)
+	require.False(can)
+}
+
+func TestCanAllocateReservationNoUsage(t *testing.T) {
+	require := require.New(t)
+
+	pool1 := &testPool{
+		name:     "pool-1",
+		reserved: 5000,
+		usage: filesystem.Usage{
+			Size: 10000,
+			Used: 0,
+		},
+		ptype: pkg.SSDDevice,
+	}
+
+	mod := &storageModule{
+		pools: []filesystem.Pool{
+			pool1,
+		},
+	}
+
+	pool1.On("Volumes").Return([]filesystem.Volume{
+		&testVolume{
+			name: "vdisk",
+		},
+	}, nil)
+
+	can, err := mod.CanAllocate("vdisk", 100)
+	require.NoError(err)
+	require.True(can)
+
+	can, err = mod.CanAllocate("vdisk", 5000)
+	require.NoError(err)
+	require.True(can)
+
+	can, err = mod.CanAllocate("vdisk", 6000)
+	require.NoError(err)
+	require.False(can)
+}
+
+func TestCanAllocateLimit(t *testing.T) {
+	require := require.New(t)
+
+	pool1 := &testPool{
+		name:     "pool-1",
+		reserved: 5000,
+		usage: filesystem.Usage{
+			Size: 10000,
+			Used: 0,
+		},
+		ptype: pkg.SSDDevice,
+	}
+
+	mod := &storageModule{
+		pools: []filesystem.Pool{
+			pool1,
+		},
+	}
+
+	pool1.On("Volumes").Return([]filesystem.Volume{
+		&testVolume{
+			name: "vdisk",
+			usage: filesystem.Usage{
+				Size: 2000,
+			},
+		},
+	}, nil)
+
+	can, err := mod.CanAllocate("vdisk", 100)
+	require.NoError(err)
+	require.True(can)
+
+	can, err = mod.CanAllocate("vdisk", 1500)
+	require.NoError(err)
+	require.True(can)
+
+	can, err = mod.CanAllocate("vdisk", 2100)
+	require.NoError(err)
+	require.False(can)
+}