Repository: ceph/go-ceph
Pull request #322 (2020-07-06 14:00:31)

rbd: add functions/constants related to rbd pool stat options
Added constants for rbd pool stat options along with wrappers for following functions:
1. rbd_pool_init()
2. rbd_pool_stats_create()
3. rbd_pool_stats_destroy()
4. rbd_pool_stats_option_add_uint64)
5. rbd_pool_stats_get()

Fixes: https://github.com/ceph/go-ceph/issues/278

Signed-off-by: Mudit Agarwal <muagarwa@redhat.com>

## Checklist
- [ ] Added tests for features and functional changes
- [ ] Public functions and types are documented
- [ ] Standard formatting is applied to Go code


agarwal-mudit: > I didn't dig into the tests much once I realized there was a bit of a mismatch between the Golang-way and the C api ceph uses here. Generally, we try to provide a thin wrapper around the C apis of Ceph, but try to make them feel Go idomatic as much as (reasonably) possible. Sometimes an api is awkward enough that I think we need to make a judgement call and deviate a little from what is happening at the C layer. As long as we don't place extra limits on the power of the API I think this is generally a good thing.
> 
> With generalities out of the way, the specific issue I see here is that the C api expects you to set up the "fetcher" with bunch of rbd_pool_stats_option_add_uint64 calls that take pointers to ints, and then call rbd_pool_stats_get which will fetch the stats and fill in pointer values. This is slightly similar to what I was also working on in PR #315 but a bit simpler and more homogeneous. Because the return types are the same I propose a simplified call, partially inspired by the how this function is wrapped by ceph's python code (in https://github.com/ceph/ceph/blob/octopus/src/pybind/rbd/rbd.pyx#L2556 ).
> 
> ```go
> // GetPoolStats returns a map of requested PoolStatOptions to their values. 
> func GetPoolStats(ioctx *rados.IOContext, o []PoolStatOption) (map[PoolStatOption]uint64, error) {
>     // .... implementation ....
> }
> ```
> 
> Unlike the python code, the user specifies what options they want, which is more similar to the C api. If we wanted to, perhaps in the future, we could even do a convenience version of the function that gets all known PoolStatOptions, just like the python code.
> 
> Now, I'm not dead set on this suggested API but I do think we need to pause and consider the API design for this subset of librados. We do want to keep our layer thin, but not too thin. Let's discuss. :-)

So, AFAICT we have two options:

1. We don't expose AddOption() API and set all the options during GetPoolStats() only as done in the python code.

2. We expose AddOption() but ask user to only set the option not the pointer to the option value but internally define a map (of options and valeues) and based on that map passess the address of the respective pointer to the option value inside AddOption(). 

Something like:

```
createPooStatMap() {  // this can be created and destroyed during CreatePoolStats() and Destroy() if required.
  m[PoolStatOptionImages] = cImages                                                                     
  m[PoolStatOptionImageProvisionedBytes] = cImageProvisionBytes 
  m[PoolStatOptionImageMaxProvisionedBytes = cImageProvisionMaxBytes
}

AddOption(PoolStatOption option) {
      optionVal = m[option]
      C.rbd_pool_stats_option_add_uint64(poolStata.stats, C.in(option), &optionVal)
}

GetOption() {
// define another struct for GetOption, not the same map as in AddOption()
return &poolStats {
                cImages: cImages
                cImageProvisionBytes: cImageProvisionBytes
		cImageProvisionMaxBytes: cImageProvisionMaxBytes
	}

```
PS: Just a thought, haven't looked from implementation point of view.

In both the cases, GetPoolStats() will return the map of all the values as done in the python code. 
phlogistonjohn: I certainly lean toward option 1, as my suggested api might indicate. I feel that this api is the way it is due to some of C's quirks and since Go is much more oriented toward more & richer data types, and I generally assume people using the API want something simple to just get the stats they need, there isn't a great impetus to make the API in go-ceph as granular as the one exposed in C.
agarwal-mudit: I have un exported AddOption() and added two APIs:

1. func (poolStats *PoolStats) GetPoolStats(ioctx *rados.IOContext, option PoolStatOption) (uint64, error)
>> Returns value for a single Pool Stat Option

2. func GetAllPoolStats(ioctx *rados.IOContext) (map[PoolStatOption]uint64, error)
>> Returns a map of all Pool Stat Options to their respective values.

Have pushed the changes to  https://github.com/agarwal-mudit/go-ceph/tree/pool but somehow that is not updating this PR automatically :(

agarwal-mudit: @nixpanic @phlogistonjohn PTAL

Patch:
diff --git a/rbd/pool_nautilus.go b/rbd/pool_nautilus.go
index d436064..8fd18ac 100644
--- a/rbd/pool_nautilus.go
+++ b/rbd/pool_nautilus.go
@@ -84,3 +84,134 @@ func RemovePoolMetadata(ioctx *rados.IOContext, key string) error {
 	ret := C.rbd_pool_metadata_remove(cephIoctx(ioctx), cKey)
 	return getError(ret)
 }
+
+// PoolInit initializes a pool for use by rbd.
+// This function does not create new pools, rather it prepares the pool
+// to host rbd images.
+//
+// Implements:
+//  int rbd_pool_init(rados_ioctx_t io, bool force)
+func PoolInit(ioctx *rados.IOContext, force bool) error {
+	if ioctx == nil {
+		return ErrNoIOContext
+	}
+
+	ret := C.rbd_pool_init(cephIoctx(ioctx), C.bool(force))
+	return getError(ret)
+}
+
+// poolStats represents RBD pool stats variable.
+type poolStats struct {
+	stats C.rbd_pool_stats_t
+}
+
+// poolStatsCreate creates a new poolStats struct.
+//
+// Implements:
+//  void rbd_pool_stats_create(rbd_pool_stats_t *stats)
+func poolStatsCreate() *poolStats {
+	poolstats := &poolStats{}
+	C.rbd_pool_stats_create(&poolstats.stats)
+	return poolstats
+}
+
+// destroy a poolStats struct and free the associated resources.
+//
+// Implements:
+//  void rbd_pool_stats_destroy(rbd_pool_stats_t stats)
+func (poolstats *poolStats) destroy() {
+	C.rbd_pool_stats_destroy(poolstats.stats)
+
+	if poolstats.stats != nil {
+		poolstats.stats = nil
+	}
+}
+
+// PoolStatOption represents a group of configurable pool stat options.
+type PoolStatOption C.rbd_pool_stat_option_t
+
+const (
+	// PoolStatOptionImages is the representation of
+	// RBD_POOL_STAT_OPTION_IMAGES from librbd.
+	PoolStatOptionImages = PoolStatOption(C.RBD_POOL_STAT_OPTION_IMAGES)
+	// PoolStatOptionImageProvisionedBytes is the representation of
+	// RBD_POOL_STAT_OPTION_IMAGE_PROVISIONED_BYTES from librbd.
+	PoolStatOptionImageProvisionedBytes = PoolStatOption(C.RBD_POOL_STAT_OPTION_IMAGE_PROVISIONED_BYTES)
+	// PoolStatOptionImageMaxProvisionedBytes is the representation of
+	// RBD_POOL_STAT_OPTION_IMAGE_MAX_PROVISIONED_BYTES from librbd.
+	PoolStatOptionImageMaxProvisionedBytes = PoolStatOption(C.RBD_POOL_STAT_OPTION_IMAGE_MAX_PROVISIONED_BYTES)
+	// PoolStatOptionImageSnapshots is the representation of
+	// RBD_POOL_STAT_OPTION_IMAGE_SNAPSHOTS from librbd.
+	PoolStatOptionImageSnapshots = PoolStatOption(C.RBD_POOL_STAT_OPTION_IMAGE_SNAPSHOTS)
+	// PoolStatOptionTrashImages is the representation of
+	// RBD_POOL_STAT_OPTION_TRASH_IMAGES from librbd.
+	PoolStatOptionTrashImages = PoolStatOption(C.RBD_POOL_STAT_OPTION_TRASH_IMAGES)
+	// PoolStatOptionTrashProvisionedBytes is the representation of
+	// RBD_POOL_STAT_OPTION_TRASH_PROVISIONED_BYTES from librbd.
+	PoolStatOptionTrashProvisionedBytes = PoolStatOption(C.RBD_POOL_STAT_OPTION_TRASH_PROVISIONED_BYTES)
+	// PoolStatOptionTrashMaxProvisionedBytes is the representation of
+	// RBD_POOL_STAT_OPTION_TRASH_MAX_PROVISIONED_BYTES from librbd.
+	PoolStatOptionTrashMaxProvisionedBytes = PoolStatOption(C.RBD_POOL_STAT_OPTION_TRASH_MAX_PROVISIONED_BYTES)
+	// PoolStatOptionTrashSnapshots is the representation of
+	// RBD_POOL_STAT_OPTION_TRASH_SNAPSHOTS from librbd.
+	PoolStatOptionTrashSnapshots = PoolStatOption(C.RBD_POOL_STAT_OPTION_TRASH_SNAPSHOTS)
+)
+
+// addPoolStatOption adds the given PoolStatOption to PoolStats.
+//
+// Implements:
+//  int rbd_pool_stats_option_add_uint64(rbd_pool_stats_t stats, int stat_option, uint64_t* stat_val)
+func (poolstats *poolStats) addPoolStatOption(option PoolStatOption, val *uint64) error {
+	ret := C.rbd_pool_stats_option_add_uint64(
+		poolstats.stats,
+		C.int(option),
+		(*C.uint64_t)(val))
+	return getError(ret)
+}
+
+// GetAllPoolStats returns a map of all PoolStatOption(s) to their respective values.
+//
+// Implements:
+//  int rbd_pool_stats_get(rados_ioctx_t io, rbd_pool_stats_t stats);
+func GetAllPoolStats(ioctx *rados.IOContext) (map[PoolStatOption]uint64, error) {
+	var omap = make(map[PoolStatOption]uint64)
+	if ioctx == nil {
+		return omap, ErrNoIOContext
+	}
+
+	poolstats := poolStatsCreate()
+	defer func() {
+		poolstats.destroy()
+	}()
+
+	var keys [8]PoolStatOption
+
+	keys[0] = PoolStatOptionImages
+	keys[1] = PoolStatOptionImageProvisionedBytes
+	keys[2] = PoolStatOptionImageMaxProvisionedBytes
+	keys[3] = PoolStatOptionImageSnapshots
+	keys[4] = PoolStatOptionTrashImages
+	keys[5] = PoolStatOptionTrashProvisionedBytes
+	keys[6] = PoolStatOptionTrashMaxProvisionedBytes
+	keys[7] = PoolStatOptionTrashSnapshots
+
+	ovalArray := make([]uint64, len(keys))
+
+	// add option with the address where the respective value would be stored.
+	for i, key := range keys {
+		err := poolstats.addPoolStatOption(key, &ovalArray[i])
+		if err != nil {
+			return omap, err
+		}
+	}
+
+	ret := C.rbd_pool_stats_get(cephIoctx(ioctx), poolstats.stats)
+	if ret < 0 {
+		return omap, getError(ret)
+	}
+
+	for j, key := range keys {
+		omap[key] = ovalArray[j]
+	}
+	return omap, nil
+}


Test patch:
diff --git a/rbd/pool_nautilus_test.go b/rbd/pool_nautilus_test.go
index 30e7d1f..d59af27 100644
--- a/rbd/pool_nautilus_test.go
+++ b/rbd/pool_nautilus_test.go
@@ -5,6 +5,7 @@ package rbd
 import (
 	"math/rand"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -99,3 +100,112 @@ func TestPoolMetadata(t *testing.T) {
 		assert.Error(t, err)
 	})
 }
+
+func TestPoolInit(t *testing.T) {
+	conn := radosConnect(t)
+	poolName := GetUUID()
+	err := conn.MakePool(poolName)
+	assert.NoError(t, err)
+
+	ioctx, err := conn.OpenIOContext(poolName)
+	assert.NoError(t, err)
+	assert.NotNil(t, ioctx)
+
+	defer func() {
+		ioctx.Destroy()
+		conn.DeletePool(poolName)
+		conn.Shutdown()
+	}()
+
+	t.Run("NullIOContext", func(t *testing.T) {
+		err := PoolInit(nil, true)
+		assert.Error(t, err)
+	})
+
+	t.Run("PoolInitWithForce", func(t *testing.T) {
+		err := PoolInit(ioctx, true)
+		assert.NoError(t, err)
+	})
+
+	t.Run("PoolInitWithoutForce", func(t *testing.T) {
+		err := PoolInit(ioctx, false)
+		assert.NoError(t, err)
+	})
+}
+
+func TestGetAllPoolStat(t *testing.T) {
+	conn := radosConnect(t)
+	poolName := GetUUID()
+	err := conn.MakePool(poolName)
+	assert.NoError(t, err)
+
+	ioctx, err := conn.OpenIOContext(poolName)
+	assert.NoError(t, err)
+	assert.NotNil(t, ioctx)
+
+	defer func() {
+		ioctx.Destroy()
+		conn.DeletePool(poolName)
+		conn.Shutdown()
+	}()
+
+	poolstats := poolStatsCreate()
+	defer func() {
+		poolstats.destroy()
+	}()
+
+	var imageName string
+	size := uint64(2 << 20)
+	var expectedSize uint64
+
+	for idx := 0; idx < 3; idx++ {
+		imageName = GetUUID()
+		image, err := Create(ioctx, imageName, size, testImageOrder)
+		assert.NoError(t, err)
+		defer func() {
+			assert.NoError(t, image.Remove())
+		}()
+		expectedSize += size
+	}
+
+	imageName = GetUUID()
+	_, err = Create(ioctx, imageName, size, testImageOrder)
+	assert.NoError(t, err)
+	expectedSize += size
+	img, err := OpenImage(ioctx, imageName, NoSnapshot)
+	assert.NoError(t, err)
+
+	mySnap, err := img.CreateSnapshot("mySnap")
+	assert.NoError(t, err)
+
+	t.Run("NullIOContext", func(t *testing.T) {
+		_, err = GetAllPoolStats(nil)
+		assert.Error(t, err)
+	})
+
+	t.Run("CheckPoolStatOption", func(t *testing.T) {
+		err := img.Resize(size)
+		assert.NoError(t, err)
+		assert.NoError(t, mySnap.Remove())
+		assert.NoError(t, img.Close())
+		err = img.Trash(time.Hour)
+		assert.NoError(t, err)
+		defer func() {
+			trashList, err := GetTrashList(ioctx)
+			assert.NoError(t, err)
+			assert.NoError(t, TrashRemove(ioctx, trashList[0].Id, true))
+		}()
+
+		omap, err := GetAllPoolStats(ioctx)
+		assert.NoError(t, err)
+
+		assert.Equal(t, uint64(3), omap[PoolStatOptionImages])
+		assert.Equal(t, (expectedSize - size), omap[PoolStatOptionImageProvisionedBytes])
+		assert.Equal(t, expectedSize-size, omap[PoolStatOptionImageMaxProvisionedBytes])
+		assert.Equal(t, uint64(0), omap[PoolStatOptionImageSnapshots])
+		assert.Equal(t, uint64(0), omap[PoolStatOptionTrashSnapshots])
+		assert.Equal(t, uint64(1), omap[PoolStatOptionTrashImages])
+		assert.Equal(t, size, omap[PoolStatOptionTrashProvisionedBytes])
+		assert.Equal(t, size, omap[PoolStatOptionTrashMaxProvisionedBytes])
+	})
+}