Merge pull request #419 from threefoldtech/storaged_qgroups

deletes qgroups when deleting sub volumes

Author: zaibon

pkg/storage/filesystem/btrfs.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"syscall"
 
+	"github.com/pkg/errors"
 	"github.com/rs/zerolog/log"
 
 	"github.com/threefoldtech/zos/pkg"
@@ -174,6 +175,10 @@ func (p *btrfsPool) mounted(fs *Btrfs) (string, bool) {
 	return "", false
 }
 
+func (p *btrfsPool) ID() int {
+	return 0
+}
+
 func (p *btrfsPool) Name() string {
 	return p.name
 }
@@ -182,6 +187,16 @@ func (p *btrfsPool) Path() string {
 	return filepath.Join("/mnt", p.name)
 }
 
+// Limit on a pool is not supported yet
+func (p *btrfsPool) Limit(size uint64) error {
+	return fmt.Errorf("not implemented")
+}
+
+// FsType of the filesystem of this volume
+func (p *btrfsPool) FsType() string {
+	return "btrfs"
+}
+
 // Mount mounts the pool in it's default mount location under /mnt/name
 func (p *btrfsPool) Mount() (string, error) {
 	ctx := context.Background()
@@ -287,6 +302,7 @@ func (p *btrfsPool) Volumes() ([]Volume, error) {
 
 	for _, sub := range subs {
 		volumes = append(volumes, newBtrfsVolume(
+			sub.ID,
 			filepath.Join(mnt, sub.Path),
 			p.utils,
 		))
@@ -296,11 +312,17 @@ func (p *btrfsPool) Volumes() ([]Volume, error) {
 }
 
 func (p *btrfsPool) addVolume(root string) (*btrfsVolume, error) {
-	if err := p.utils.SubvolumeAdd(context.Background(), root); err != nil {
+	ctx := context.Background()
+	if err := p.utils.SubvolumeAdd(ctx, root); err != nil {
+		return nil, err
+	}
+
+	volume, err := p.utils.SubvolumeInfo(ctx, root)
+	if err != nil {
 		return nil, err
 	}
 
-	return newBtrfsVolume(root, p.utils), nil
+	return newBtrfsVolume(volume.ID, root, p.utils), nil
 }
 
 func (p *btrfsPool) AddVolume(name string) (Volume, error) {
@@ -314,7 +336,23 @@ func (p *btrfsPool) AddVolume(name string) (Volume, error) {
 }
 
 func (p *btrfsPool) removeVolume(root string) error {
-	return p.utils.SubvolumeRemove(context.Background(), root)
+	ctx := context.Background()
+
+	info, err := p.utils.SubvolumeInfo(ctx, root)
+	if err != nil {
+		return err
+	}
+
+	if err := p.utils.SubvolumeRemove(ctx, root); err != nil {
+		return err
+	}
+
+	qgroupID := fmt.Sprintf("0/%d", info.ID)
+	if err := p.utils.QGroupDestroy(ctx, qgroupID, p.Path()); err != nil {
+		return errors.Wrapf(err, "failed to delete qgroup %s", qgroupID)
+	}
+
+	return nil
 }
 
 func (p *btrfsPool) RemoveVolume(name string) error {
@@ -357,16 +395,6 @@ func (p *btrfsPool) Usage() (usage Usage, err error) {
 	return Usage{Size: totalSize / raidSizeDivisor[du.Data.Profile], Used: uint64(fsi[0].Used)}, nil
 }
 
-// Limit on a pool is not supported yet
-func (p *btrfsPool) Limit(size uint64) error {
-	return fmt.Errorf("not implemented")
-}
-
-// FsType of the filesystem of this volume
-func (p *btrfsPool) FsType() string {
-	return "btrfs"
-}
-
 // Type of the physical storage used for this pool
 func (p *btrfsPool) Type() pkg.DeviceType {
 	// We only create heterogenous pools for now
@@ -393,49 +421,71 @@ func (p *btrfsPool) Reserved() (uint64, error) {
 	return total, nil
 }
 
+func (p *btrfsPool) Maintenance() error {
+	// this method cleans up all the unused
+	// qgroups that could exists on a filesystem
+
+	volumes, err := p.Volumes()
+	if err != nil {
+		return err
+	}
+	subVolsIDs := map[string]struct{}{}
+	for _, volume := range volumes {
+		// use the 0/X notation to match the qgroup IDs format
+		subVolsIDs[fmt.Sprintf("0/%d", volume.ID())] = struct{}{}
+	}
+
+	ctx := context.Background()
+	qgroups, err := p.utils.QGroupList(ctx, p.Path())
+	if err != nil {
+		return err
+	}
+
+	for qgroupID := range qgroups {
+		// for all qgroup that doesn't have an linked
+		// volume, delete the qgroup
+		_, ok := subVolsIDs[qgroupID]
+		if !ok {
+			log.Debug().Str("id", qgroupID).Msg("destroy qgroup")
+			if err := p.utils.QGroupDestroy(ctx, qgroupID, p.Path()); err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 type btrfsVolume struct {
+	id    int
 	path  string
 	utils *BtrfsUtil
 }
 
-func newBtrfsVolume(path string, utils *BtrfsUtil) *btrfsVolume {
+func newBtrfsVolume(ID int, path string, utils *BtrfsUtil) *btrfsVolume {
 	return &btrfsVolume{
+		id:    ID,
 		path:  path,
 		utils: utils,
 	}
 }
 
-func (v *btrfsVolume) Path() string {
-	return v.path
+func (v *btrfsVolume) ID() int {
+	return v.id
 }
 
-func (v *btrfsVolume) Volumes() ([]Volume, error) {
-	var volumes []Volume
-
-	subs, err := v.utils.SubvolumeList(context.Background(), v.Path())
-	if err != nil {
-		return nil, err
-	}
-
-	for _, sub := range subs {
-		volumes = append(volumes, newBtrfsVolume(filepath.Join(v.Path(), sub.Path), v.utils))
-	}
-
-	return volumes, nil
+func (v *btrfsVolume) Path() string {
+	return v.path
 }
 
-func (v *btrfsVolume) AddVolume(name string) (Volume, error) {
-	mnt := filepath.Join(v.Path(), name)
-	if err := v.utils.SubvolumeAdd(context.Background(), mnt); err != nil {
-		return nil, err
-	}
-
-	return newBtrfsVolume(mnt, v.utils), nil
+// Name of the filesystem
+func (v *btrfsVolume) Name() string {
+	return filepath.Base(v.Path())
 }
 
-func (v *btrfsVolume) RemoveVolume(name string) error {
-	mnt := filepath.Join(v.Path(), name)
-	return v.utils.SubvolumeRemove(context.Background(), mnt)
+// FsType of the filesystem
+func (v *btrfsVolume) FsType() string {
+	return "btrfs"
 }
 
 // Usage return the volume usage
@@ -470,13 +520,3 @@ func (v *btrfsVolume) Limit(size uint64) error {
 
 	return v.utils.QGroupLimit(ctx, size, v.Path())
 }
-
-// Name of the filesystem
-func (v *btrfsVolume) Name() string {
-	return filepath.Base(v.Path())
-}
-
-// FsType of the filesystem
-func (v *btrfsVolume) FsType() string {
-	return "btrfs"
-}

pkg/storage/filesystem/btrfs_ci_test.go

@@ -162,13 +162,6 @@ func basePoolTest(t *testing.T, pool Pool) {
 		assert.Equal(t, uint64(1024*1024*1024), usage.Size)
 	})
 
-	t.Run("test subvolume list no subvolumes", func(t *testing.T) {
-		volumes, err := volume.Volumes()
-		require.NoError(t, err)
-
-		assert.Empty(t, volumes)
-	})
-
 	t.Run("test limit subvolume", func(t *testing.T) {
 		usage, err := volume.Usage()
 		require.NoError(t, err)
@@ -301,3 +294,60 @@ func TestBtrfsListCI(t *testing.T) {
 	ok := assert.Len(t, names, 0)
 	assert.True(t, ok, "not all pools were listed")
 }
+
+func TestCLeanUpQgroupsCI(t *testing.T) {
+	if SkipCITests {
+		t.Skip("test requires ability to create loop devices")
+	}
+
+	devices, err := SetupDevices(1)
+	require.NoError(t, err, "failed to initialize devices")
+	defer devices.Destroy()
+
+	loops := devices.Loops()
+	fs := NewBtrfs(&TestDeviceManager{loops})
+
+	names := make(map[string]struct{})
+	for idx := range loops {
+		loop := &loops[idx]
+		name := fmt.Sprintf("test-list-%d", idx)
+		names[name] = struct{}{}
+		_, err := fs.Create(context.Background(), name, pkg.Single, loop)
+		require.NoError(t, err)
+	}
+	pools, err := fs.List(context.Background(), func(p Pool) bool {
+		return strings.HasPrefix(p.Name(), "test-")
+	})
+	pool := pools[0]
+
+	_, err = pool.Mount()
+	require.NoError(t, err)
+	defer pool.UnMount()
+
+	volume, err := pool.AddVolume("vol1")
+	require.NoError(t, err)
+	t.Logf("volume ID: %v\n", volume.ID())
+
+	err = volume.Limit(256 * 1024 * 1024)
+	require.NoError(t, err)
+
+	btrfsVol, ok := volume.(*btrfsVolume)
+	require.True(t, ok, "volume should be a btrfsVolume")
+
+	qgroups, err := btrfsVol.utils.QGroupList(context.TODO(), pool.Path())
+	require.NoError(t, err)
+	assert.Equal(t, 2, len(qgroups))
+	t.Logf("qgroups before delete: %v", qgroups)
+
+	_, ok = qgroups[fmt.Sprintf("0/%d", btrfsVol.id)]
+	assert.True(t, ok, "qgroups should contains a qgroup linked to the subvolume")
+
+	err = pool.RemoveVolume("vol1")
+	require.NoError(t, err)
+
+	qgroups, err = btrfsVol.utils.QGroupList(context.TODO(), pool.Path())
+	require.NoError(t, err)
+
+	t.Logf("remaining qgroups: %+v", qgroups)
+	assert.Equal(t, 1, len(qgroups), "qgroups should have been deleted with the subvolume")
+}

pkg/storage/filesystem/btrfs_utils.go

@@ -174,6 +174,13 @@ func (u *BtrfsUtil) QGroupLimit(ctx context.Context, size uint64, path string) e
 	return err
 }
 
+// QGroupDestroy deletes a qgroup on a subvol
+func (u *BtrfsUtil) QGroupDestroy(ctx context.Context, id, path string) error {
+	_, err := u.run(ctx, "btrfs", "qgroup", "destroy", id, path)
+
+	return err
+}
+
 // GetDiskUsage get btrfs usage
 func (u *BtrfsUtil) GetDiskUsage(ctx context.Context, path string) (usage BtrfsDiskUsage, err error) {
 	output, err := u.run(ctx, "btrfs", "filesystem", "df", "--raw", path)

pkg/storage/filesystem/filesystem.go

@@ -14,14 +14,10 @@ type Usage struct {
 
 // Volume represents a logical volume in the pool. Volumes can be nested
 type Volume interface {
+	// Volume ID
+	ID() int
 	// Path of the volume
 	Path() string
-	// Volumes are all subvolumes of this volume
-	Volumes() ([]Volume, error)
-	// AddVolume adds a new subvolume with the given name
-	AddVolume(name string) (Volume, error)
-	// RemoveVolume removes a subvolume with the given name
-	RemoveVolume(name string) error
 	// Usage reports the current usage of the volume
 	Usage() (Usage, error)
 	// Limit the maximum size of the volume
@@ -50,8 +46,19 @@ type Pool interface {
 	Type() pkg.DeviceType
 	// Reserved is reserved size of the devices in bytes
 	Reserved() (uint64, error)
+	// Maintenance is a routine that is called at boot
+	// and that all implementer can use to do some clean up and
+	// other maintenance on the pool
+	Maintenance() error
 
 	// Health() ?
+
+	// Volumes are all subvolumes of this volume
+	Volumes() ([]Volume, error)
+	// AddVolume adds a new subvolume with the given name
+	AddVolume(name string) (Volume, error)
+	// RemoveVolume removes a subvolume with the given name
+	RemoveVolume(name string) error
 }
 
 // Filter closure for Filesystem list

pkg/storage/storage.go

@@ -60,6 +60,10 @@ func New() (pkg.StorageModule, error) {
 		log.Info().Msgf("Finished initializing storage module")
 	}
 
+	if err := s.Maintenance(); err != nil {
+		log.Error().Err(err).Msg("storage devices maintenance failed")
+	}
+
 	return s, err
 }
 
@@ -211,6 +215,26 @@ func (s *storageModule) initialize(policy pkg.StoragePolicy) error {
 	return s.ensureCache()
 }
 
+func (s *storageModule) Maintenance() error {
+
+	for _, pool := range s.volumes {
+		log.Info().
+			Str("pool", pool.Name()).
+			Msg("start storage pool maintained")
+		if err := pool.Maintenance(); err != nil {
+			log.Error().
+				Err(err).
+				Str("pool", pool.Name()).
+				Msg("error during maintainace")
+			return err
+		}
+		log.Info().
+			Str("pool", pool.Name()).
+			Msg("finished storage pool maintained")
+	}
+	return nil
+}
+
 // CreateFilesystem with the given size in a storage pool.
 func (s *storageModule) CreateFilesystem(name string, size uint64, poolType pkg.DeviceType) (string, error) {
 	log.Info().Msgf("Creating new volume with size %d", size)