Repository: ceph/go-ceph
Pull request #342 (2020-08-05 18:04:56)

Add a CephFS Administrative sub-package
Fixes: #341 (it's a beginning at least)
Fixes: #202 

This is a RFC / design review PR for a new package, named "cephfs/admin", that aims to provide a simple API for administrating CephFS volumes. The focus is on functions needed to create/remove/update/view subvolumes and subvoulme groups. As noted in the linked issue, the first consumer is expected to be ceph-csi but the aim to have a general API that can be consumed by any Go code that wants to manage cephfs volumes. Ideally, this module should be both more efficient and more convenient than shelling out to the `ceph` cli tool.

Functions currently covered:
* CreateSubVolume
* CreateSubVolumeGroup
* ListSubVolumeGroups
* ListSubVolumes
* ListVolumes
* RemoveSubVolume
* RemoveSubVolumeGroup
* ResizeSubVolume
* VolumeStatus

I'll try and resist the temptation to add more functions at this point and wait for reviews/discussion. I'm happy to hear any suggestions around the API style, naming, etc. as this API is more under go-ceph's direct control rather than needing to hew closely to the ceph C apis, like our other packages.

There are still a few small gaps in the tests but most of them can be improved in follow on PRs and as we add additional functions to the module. These are not all the functions ceph-csi will need, but mainly just a start and demonstration of the approach I'd like to go with.

## Checklist
- [x] Added tests for features and functional changes
- [x] Public functions and types are documented
- [x] Standard formatting is applied to Go code


phlogistonjohn: Of course, when I was running tests locally everything looked fine. At least one test failure seems to be due to versions prior to "octopus" not honoring the "json" format request. I'll need to investigate the others.
phlogistonjohn: Looks like mimic doesn't even support the command set I'm using WRT the manager. Since we're planning on deprecating mimic and luminous, I am wondering if we should declare that this new package will only support octopus (and probably) nautilus [1].

[1] - I'm OK with not supporting some functions on nautilus with build tags but it appears mimic (and I'm assuming luminous) are very different.
nixpanic: > Looks like mimic doesn't even support the command set I'm using WRT the manager. Since we're planning on deprecating mimic and luminous, I am wondering if we should declare that this new package will only support octopus (and probably) nautilus [1].

Support for Nautilus would be nice, older versions should really not be needed. Start with Octopus, and see if it works on Nautilus too?
phlogistonjohn: Nautilus works modulo one call `VolumeStatus` that does not return JSON even if JSON is requested. In theory we could scrape the plain-ish-text output but I would rather not.
We could simply use build tags to disable that one function on nautilus. At this moment the function is implemented but the test is disabled.
I've already used build tags to disable the majority of the package on luminous and mimic.
nixpanic: > Nautilus works modulo one call `VolumeStatus` that does not return JSON even if JSON is requested. In theory we could scrape the plain-ish-text output but I would rather not.
> We could simply use build tags to disable that one function on nautilus. At this moment the function is implemented but the test is disabled.

This works for me, thanks.
phlogistonjohn: I've kinda-sorta determined why the tests were so flaky. Apologies if this gets esoteric. I need a place to note some of what I learned today down and this is better than nowhere :-)

Apparently, when ceph wants to send a message to a mgr it will use the MgrClient class which has a flag mgr_optional and if its sets it can return -EACCES which is the errno I was seeing. Code looks like:
```
  if (map.epoch == 0 && mgr_optional) {
    ldout(cct,20) << " no MgrMap, assuming EACCES" << dendl;
    return -EACCES;
  }
```
Now I have yet to figure out why mgr_optional is set or how to handle it on our side, if that's even possible. But I did confirm this was the case by changing the 'mgrc' log level and sure enough I got the log line above. Oddly, I was seeing it a lot when running the tests.

This was due to another bug, this time on our side where the global I meant to use for caching the FSAdmin pointer was not getting set poperly so every test function was creating a *new* rados connection, and thus needing to constantly fetch the mgr map.

Once I fix this I think things will improve. One minor annoying thing is that it won't eliminate the problem because there's still a possibility that the MgrCommand function, and the ceph code it calls will execute before the mgr is available. Thus we'd still get spurious test failures. For now, I'll probably just drop a sleep in there as a hack as it only seems to be on the order of milliseconds and once the caching issue is fixed, it would only be done once per-execution of the test suite.

That said I want to understand what ceph is doing here a bit better and see if we can either make mgr_optional false or use the ceph C api to block until we know that the mgr map is loaded.

phlogistonjohn: I've removed the RFC tag. Please review with an eye toward the general flow of the interfaces and functions first, as this will be a new sub-package in v0.6.0 we can still tweak quite a bit in the implementation in follow up PRs if we have to, IMO.
phlogistonjohn: Based on a chat I had with @nixpanic I'm going to opt to merge this w/o his formal approval (on this PR), because this is a new package and it is effectively in a preliminary state anyway. I'll leave it unmerged for another couple of days in case @ansiwen or @agarwal-mudit have an objection or request specific changes. Merging us will make it easier for more of us to add functions to this package. Because it's so new we're not going to be afraid of changes the apis until closer to release time.

Patch:
diff --git a/Makefile b/Makefile
index ff5a003..f879a98 100644
--- a/Makefile
+++ b/Makefile
@@ -68,6 +68,7 @@ check-revive:
 # library code as well.
 test-binaries: \
 	cephfs.test \
+	cephfs/admin.test \
 	internal/callbacks.test \
 	internal/cutil.test \
 	internal/errutil.test \
diff --git a/cephfs/admin/bytecount.go b/cephfs/admin/bytecount.go
new file mode 100644
index 0000000..a1b60cb
--- /dev/null
+++ b/cephfs/admin/bytecount.go
@@ -0,0 +1,37 @@
+// +build !luminous,!mimic
+
+package admin
+
+// ByteCount represents the size of a volume in bytes.
+type ByteCount uint64
+
+// SI byte size constants. keep these private for now.
+const (
+	kibiByte ByteCount = 1024
+	mebiByte           = 1024 * kibiByte
+	gibiByte           = 1024 * mebiByte
+	tebiByte           = 1024 * gibiByte
+)
+
+// newSizeValue returns a size value as a string, as needed by the subvolume
+// resize command json.
+func (bc ByteCount) newSizeValue() string {
+	return uint64String(uint64(bc))
+}
+
+// NewSize interface values can be used to change the size of a volume.
+type NewSize interface {
+	newSizeValue() string
+}
+
+// specialSize is a custom non-numeric new size value.
+type specialSize string
+
+// newSizeValue for a specialSize returns the literal string.
+func (s specialSize) newSizeValue() string {
+	return string(s)
+}
+
+// Infinite is a special NewSize value that can be used to clear size limits on
+// a subvolume.
+const Infinite = specialSize("infinite")
diff --git a/cephfs/admin/doc.go b/cephfs/admin/doc.go
new file mode 100644
index 0000000..3ff5398
--- /dev/null
+++ b/cephfs/admin/doc.go
@@ -0,0 +1,11 @@
+/*
+Package admin is a convenience layer to support the administration of
+CephFS volumes, subvolumes, etc.
+
+Unlike the cephfs package this API does not map to APIs provided by
+ceph libraries themselves. This API is not yet stable and is subject
+to change.
+
+This package only supports ceph "nautilus" and "octopus" at this time.
+*/
+package admin
diff --git a/cephfs/admin/fsadmin.go b/cephfs/admin/fsadmin.go
new file mode 100644
index 0000000..606759d
--- /dev/null
+++ b/cephfs/admin/fsadmin.go
@@ -0,0 +1,135 @@
+// +build !luminous,!mimic
+
+package admin
+
+import (
+	"encoding/json"
+	"fmt"
+	"strconv"
+
+	"github.com/ceph/go-ceph/rados"
+)
+
+// RadosCommander provides an interface to execute JSON-formatted commands that
+// allow the cephfs administrative functions to interact with the Ceph cluster.
+type RadosCommander interface {
+	MgrCommand(buf [][]byte) ([]byte, string, error)
+}
+
+// FSAdmin is used to administrate CephFS within a ceph cluster.
+type FSAdmin struct {
+	conn RadosCommander
+}
+
+// New creates an FSAdmin automatically based on the default ceph
+// configuration file. If more customization is needed, create a
+// *rados.Conn as you see fit and use NewFromConn to use that
+// connection with these administrative functions.
+func New() (*FSAdmin, error) {
+	conn, err := rados.NewConn()
+	if err != nil {
+		return nil, err
+	}
+	err = conn.ReadDefaultConfigFile()
+	if err != nil {
+		return nil, err
+	}
+	err = conn.Connect()
+	if err != nil {
+		return nil, err
+	}
+	return NewFromConn(conn), nil
+}
+
+// NewFromConn creates an FSAdmin management object from a preexisting
+// rados connection. The existing connection can be rados.Conn or any
+// type implementing the RadosCommander interface. This may be useful
+// if the calling layer needs to inject additional logging, error handling,
+// fault injection, etc.
+func NewFromConn(conn RadosCommander) *FSAdmin {
+	return &FSAdmin{conn}
+}
+
+func (fsa *FSAdmin) validate() error {
+	if fsa.conn == nil {
+		return rados.ErrNotConnected
+	}
+	return nil
+}
+
+// rawMgrCommand takes a byte buffer and sends it to the MGR as a command.
+// The buffer is expected to contain preformatted JSON.
+func (fsa *FSAdmin) rawMgrCommand(buf []byte) ([]byte, string, error) {
+	if err := fsa.validate(); err != nil {
+		return nil, "", err
+	}
+	return fsa.conn.MgrCommand([][]byte{buf})
+}
+
+// marshalMgrCommand takes an generic interface{} value, converts it to JSON and
+// sends the json to the MGR as a command.
+func (fsa *FSAdmin) marshalMgrCommand(v interface{}) ([]byte, string, error) {
+	b, err := json.Marshal(v)
+	if err != nil {
+		return nil, "", err
+	}
+	return fsa.rawMgrCommand(b)
+}
+
+type listNamedResult struct {
+	Name string `json:"name"`
+}
+
+func parseListNames(res []byte, status string, err error) ([]string, error) {
+	var r []listNamedResult
+	if err := unmarshalResponseJSON(res, status, err, &r); err != nil {
+		return nil, err
+	}
+	vl := make([]string, len(r))
+	for i := range r {
+		vl[i] = r[i].Name
+	}
+	return vl, nil
+}
+
+// checkEmptyResponseExpected returns an error if the result or status
+// are non-empty.
+func checkEmptyResponseExpected(res []byte, status string, err error) error {
+	if err != nil {
+		return err
+	}
+	if len(res) != 0 {
+		return fmt.Errorf("unexpected response: %s", string(res))
+	}
+	if status != "" {
+		return fmt.Errorf("error status: %s", status)
+	}
+	return nil
+}
+
+func unmarshalResponseJSON(res []byte, status string, err error, v interface{}) error {
+	if err != nil {
+		return err
+	}
+	if status != "" {
+		return fmt.Errorf("error status: %s", status)
+	}
+	return json.Unmarshal(res, v)
+}
+
+// modeString converts a unix-style mode value to a string-ified version in an
+// octal representation (e.g. "777", "700", etc). This format is expected by
+// some of the ceph JSON command inputs.
+func modeString(m int, force bool) string {
+	if force || m != 0 {
+		return strconv.FormatInt(int64(m), 8)
+	}
+	return ""
+}
+
+// uint64String converts a uint64 to a string. Some of the ceph json commands
+// can take a string or "int" (as a string). This is a common function for
+// doing that conversion.
+func uint64String(v uint64) string {
+	return strconv.FormatUint(uint64(v), 10)
+}
diff --git a/cephfs/admin/subvolume.go b/cephfs/admin/subvolume.go
new file mode 100644
index 0000000..90109c9
--- /dev/null
+++ b/cephfs/admin/subvolume.go
@@ -0,0 +1,145 @@
+// +build !luminous,!mimic
+
+package admin
+
+// this is the internal type used to create JSON for ceph.
+// See SubVolumeOptions for the type that users of the library
+// interact with.
+// note that the ceph json takes mode as a string.
+type subVolumeFields struct {
+	Prefix            string    `json:"prefix"`
+	Format            string    `json:"format"`
+	VolName           string    `json:"vol_name"`
+	GroupName         string    `json:"group_name,omitempty"`
+	SubName           string    `json:"sub_name"`
+	Size              ByteCount `json:"size,omitempty"`
+	Uid               int       `json:"uid,omitempty"`
+	Gid               int       `json:"gid,omitempty"`
+	Mode              string    `json:"mode,omitempty"`
+	PoolLayout        string    `json:"pool_layout,omitempty"`
+	NamespaceIsolated bool      `json:"namespace_isolated"`
+}
+
+// SubVolumeOptions are used to specify optional, non-identifying, values
+// to be used when creating a new subvolume.
+type SubVolumeOptions struct {
+	Size              ByteCount
+	Uid               int
+	Gid               int
+	Mode              int
+	PoolLayout        string
+	NamespaceIsolated bool
+}
+
+func (s *SubVolumeOptions) toFields(v, g, n string) *subVolumeFields {
+	return &subVolumeFields{
+		Prefix:            "fs subvolume create",
+		Format:            "json",
+		VolName:           v,
+		GroupName:         g,
+		SubName:           n,
+		Size:              s.Size,
+		Uid:               s.Uid,
+		Gid:               s.Gid,
+		Mode:              modeString(s.Mode, false),
+		PoolLayout:        s.PoolLayout,
+		NamespaceIsolated: s.NamespaceIsolated,
+	}
+}
+
+// NoGroup should be used when an optional subvolume group name is not
+// specified.
+const NoGroup = ""
+
+// CreateSubVolume sends a request to create a CephFS subvolume in a volume,
+// belonging to an optional subvolume group.
+//
+// Similar To:
+//  ceph fs subvolume create <volume> --group-name=<group> <name> ...
+func (fsa *FSAdmin) CreateSubVolume(volume, group, name string, o *SubVolumeOptions) error {
+	if o == nil {
+		o = &SubVolumeOptions{}
+	}
+	f := o.toFields(volume, group, name)
+	return checkEmptyResponseExpected(fsa.marshalMgrCommand(f))
+}
+
+// ListSubVolumes returns a list of subvolumes belonging to the volume and
+// optional subvolume group.
+//
+// Similar To:
+//  ceph fs subvolume ls <volume> --group-name=<group>
+func (fsa *FSAdmin) ListSubVolumes(volume, group string) ([]string, error) {
+	m := map[string]string{
+		"prefix":   "fs subvolume ls",
+		"vol_name": volume,
+		"format":   "json",
+	}
+	if group != NoGroup {
+		m["group_name"] = group
+	}
+	return parseListNames(fsa.marshalMgrCommand(m))
+}
+
+// RemoveSubVolume will delete a CephFS subvolume in a volume and optional
+// subvolume group.
+//
+// Similar To:
+//  ceph fs subvolume rm <volume> --group-name=<group> <name> ...
+func (fsa *FSAdmin) RemoveSubVolume(volume, group, name string) error {
+	m := map[string]string{
+		"prefix":   "fs subvolume rm",
+		"vol_name": volume,
+		"sub_name": name,
+		"format":   "json",
+	}
+	if group != NoGroup {
+		m["group_name"] = group
+	}
+	return checkEmptyResponseExpected(fsa.marshalMgrCommand(m))
+}
+
+type subVolumeResizeFields struct {
+	Prefix    string `json:"prefix"`
+	Format    string `json:"format"`
+	VolName   string `json:"vol_name"`
+	GroupName string `json:"group_name,omitempty"`
+	SubName   string `json:"sub_name"`
+	NewSize   string `json:"new_size"`
+	NoShrink  bool   `json:"no_shrink"`
+}
+
+// SubVolumeResizeResult reports the size values returned by the
+// ResizeSubVolume function, as reported by Ceph.
+type SubVolumeResizeResult struct {
+	BytesUsed    ByteCount `json:"bytes_used"`
+	BytesQuota   ByteCount `json:"bytes_quota"`
+	BytesPercent string    `json:"bytes_pcent"`
+}
+
+// ResizeSubVolume will resize a CephFS subvolume. The newSize value may be a
+// ByteCount or the special Infinite constant. Setting noShrink to true will
+// prevent reducing the size of the volume below the current used size.
+//
+// Similar To:
+//  ceph fs subvolume resize <volume> --group-name=<group> <name> ...
+func (fsa *FSAdmin) ResizeSubVolume(
+	volume, group, name string,
+	newSize NewSize, noShrink bool) (*SubVolumeResizeResult, error) {
+
+	f := &subVolumeResizeFields{
+		Prefix:    "fs subvolume resize",
+		Format:    "json",
+		VolName:   volume,
+		GroupName: group,
+		SubName:   name,
+		NewSize:   newSize.newSizeValue(),
+		NoShrink:  noShrink,
+	}
+	var result []*SubVolumeResizeResult
+	r, s, err := fsa.marshalMgrCommand(f)
+	if err := unmarshalResponseJSON(r, s, err, &result); err != nil {
+		return nil, err
+	}
+	return result[0], nil
+}
diff --git a/cephfs/admin/subvolumegroup.go b/cephfs/admin/subvolumegroup.go
new file mode 100644
index 0000000..709b02b
--- /dev/null
+++ b/cephfs/admin/subvolumegroup.go
@@ -0,0 +1,79 @@
+// +build !luminous,!mimic
+
+package admin
+
+// this is the internal type used to create JSON for ceph.
+// See SubVolumeGroupOptions for the type that users of the library
+// interact with.
+// note that the ceph json takes mode as a string.
+type subVolumeGroupFields struct {
+	Prefix     string `json:"prefix"`
+	Format     string `json:"format"`
+	VolName    string `json:"vol_name"`
+	GroupName  string `json:"group_name"`
+	Uid        int    `json:"uid,omitempty"`
+	Gid        int    `json:"gid,omitempty"`
+	Mode       string `json:"mode,omitempty"`
+	PoolLayout string `json:"pool_layout,omitempty"`
+}
+
+// SubVolumeGroupOptions are used to specify optional, non-identifying, values
+// to be used when creating a new subvolume group.
+type SubVolumeGroupOptions struct {
+	Uid        int
+	Gid        int
+	Mode       int
+	PoolLayout string
+}
+
+func (s *SubVolumeGroupOptions) toFields(v, g string) *subVolumeGroupFields {
+	return &subVolumeGroupFields{
+		Prefix:     "fs subvolumegroup create",
+		Format:     "json",
+		VolName:    v,
+		GroupName:  g,
+		Uid:        s.Uid,
+		Gid:        s.Gid,
+		Mode:       modeString(s.Mode, false),
+		PoolLayout: s.PoolLayout,
+	}
+}
+
+// CreateSubVolumeGroup sends a request to create a subvolume group in a volume.
+//
+// Similar To:
+//  ceph fs subvolumegroup create <volume> <group_name>  ...
+func (fsa *FSAdmin) CreateSubVolumeGroup(volume, name string, o *SubVolumeGroupOptions) error {
+	if o == nil {
+		o = &SubVolumeGroupOptions{}
+	}
+	r, s, err := fsa.marshalMgrCommand(o.toFields(volume, name))
+	return checkEmptyResponseExpected(r, s, err)
+}
+
+// ListSubVolumeGroups returns a list of subvolume groups belonging to the
+// specified volume.
+//
+// Similar To:
+//  ceph fs subvolumegroup ls cephfs <volume>
+func (fsa *FSAdmin) ListSubVolumeGroups(volume string) ([]string, error) {
+	r, s, err := fsa.marshalMgrCommand(map[string]string{
+		"prefix":   "fs subvolumegroup ls",
+		"vol_name": volume,
+		"format":   "json",
+	})
+	return parseListNames(r, s, err)
+}
+
+// RemoveSubVolumeGroup will delete a subvolume group in a volume.
+// Similar To:
+//  ceph fs subvolumegroup rm <volume> <group_name>
+func (fsa *FSAdmin) RemoveSubVolumeGroup(volume, name string) error {
+	r, s, err := fsa.marshalMgrCommand(map[string]string{
+		"prefix":     "fs subvolumegroup rm",
+		"vol_name":   volume,
+		"group_name": name,
+		"format":     "json",
+	})
+	return checkEmptyResponseExpected(r, s, err)
+}
diff --git a/cephfs/admin/volume.go b/cephfs/admin/volume.go
new file mode 100644
index 0000000..226fef9
--- /dev/null
+++ b/cephfs/admin/volume.go
@@ -0,0 +1,14 @@
+// +build !luminous,!mimic
+
+package admin
+
+var listVolumesCmd = []byte(`{"prefix":"fs volume ls"}`)
+
+// ListVolumes return a list of volumes in this Ceph cluster.
+//
+// Similar To:
+//  ceph fs volume ls
+func (fsa *FSAdmin) ListVolumes() ([]string, error) {
+	r, s, err := fsa.rawMgrCommand(listVolumesCmd)
+	return parseListNames(r, s, err)
+}
diff --git a/cephfs/admin/volume_octopus.go b/cephfs/admin/volume_octopus.go
new file mode 100644
index 0000000..b75c9ca
--- /dev/null
+++ b/cephfs/admin/volume_octopus.go
@@ -0,0 +1,38 @@
+// +build octopus
+
+package admin
+
+// VolumePool reports on the pool status for a CephFS volume.
+type VolumePool struct {
+	ID        int    `json:"id"`
+	Name      string `json:"name"`
+	Type      string `json:"type"`
+	Available uint64 `json:"avail"`
+	Used      uint64 `json:"used"`
+}
+
+// VolumeStatus reports various properties of a CephFS volume.
+// TODO: Fill in.
+type VolumeStatus struct {
+	MDSVersion string       `json:"mds_version"`
+	Pools      []VolumePool `json:"pools"`
+}
+
+func parseVolumeStatus(res []byte, status string, err error) (*VolumeStatus, error) {
+	var vs VolumeStatus
+	err = unmarshalResponseJSON(res, status, err, &vs)
+	return &vs, err
+}
+
+// VolumeStatus returns a VolumeStatus object for the given volume name.
+//
+// Similar To:
+//  ceph fs status cephfs <name>
+func (fsa *FSAdmin) VolumeStatus(name string) (*VolumeStatus, error) {
+	r, s, err := fsa.marshalMgrCommand(map[string]string{
+		"fs":     name,
+		"prefix": "fs status",
+		"format": "json",
+	})
+	return parseVolumeStatus(r, s, err)
+}
diff --git a/entrypoint.sh b/entrypoint.sh
index e86e522..b9b42c1 100755
--- a/entrypoint.sh
+++ b/entrypoint.sh
@@ -184,6 +184,7 @@ test_go_ceph() {
     P=github.com/ceph/go-ceph
     pkgs=(\
         "cephfs" \
+        "cephfs/admin" \
         "internal/callbacks" \
         "internal/cutil" \
         "internal/errutil" \


Test patch:
diff --git a/cephfs/admin/fsadmin_test.go b/cephfs/admin/fsadmin_test.go
new file mode 100644
index 0000000..928eacf
--- /dev/null
+++ b/cephfs/admin/fsadmin_test.go
@@ -0,0 +1,140 @@
+// +build !luminous,!mimic
+
+package admin
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"strconv"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+var (
+	cachedFSAdmin *FSAdmin
+
+	// set debugTrace to true to use tracing in tests
+	debugTrace = false
+)
+
+func init() {
+	dt := os.Getenv("GO_CEPH_TEST_DEBUG_TRACE")
+	if ok, err := strconv.ParseBool(dt); ok && err == nil {
+		debugTrace = true
+	}
+}
+
+// tracingCommander serves two purposes: first, it allows one to trace the
+// input and output json when running the tests. It can help with actually
+// debugging the tests. Second, it demonstrates the rationale for using an
+// interface in FSAdmin. You can layer any sort of debugging, error injection,
+// or whatnot between the FSAdmin layer and the RADOS layer.
+type tracingCommander struct {
+	conn RadosCommander
+}
+
+func tracer(c RadosCommander) RadosCommander {
+	return &tracingCommander{c}
+}
+
+func (t *tracingCommander) MgrCommand(buf [][]byte) ([]byte, string, error) {
+	for i := range buf {
+		fmt.Println("IN:", string(buf[i]))
+	}
+	r, s, err := t.conn.MgrCommand(buf)
+	fmt.Println("OUT(result):", string(r))
+	if s != "" {
+		fmt.Println("OUT(status):", s)
+	}
+	if err != nil {
+		fmt.Println("OUT(error):", err.Error())
+	}
+	return r, s, err
+}
+
+func getFSAdmin(t *testing.T) *FSAdmin {
+	if cachedFSAdmin != nil {
+		return cachedFSAdmin
+	}
+	fsa, err := New()
+	require.NoError(t, err)
+	require.NotNil(t, fsa)
+	// We steal the connection set up by the New() method and wrap it in an
+	// optional tracer.
+	c := fsa.conn
+	if debugTrace {
+		c = tracer(c)
+	}
+	cachedFSAdmin = NewFromConn(c)
+	// We sleep briefly before returning in order to ensure we have a mgr map
+	// before we start executing the tests.
+	time.Sleep(50 * time.Millisecond)
+	return cachedFSAdmin
+}
+
+func TestInvalidFSAdmin(t *testing.T) {
+	fsa := &FSAdmin{}
+	_, _, err := fsa.rawMgrCommand([]byte("FOOBAR!"))
+	assert.Error(t, err)
+}
+
+type badMarshalType bool
+
+func (badMarshalType) MarshalJSON() ([]byte, error) {
+	return nil, errors.New("Zowie! wow")
+}
+
+func TestBadMarshal(t *testing.T) {
+	fsa := getFSAdmin(t)
+
+	var bad badMarshalType
+	_, _, err := fsa.marshalMgrCommand(bad)
+	assert.Error(t, err)
+}
+
+func TestParseListNames(t *testing.T) {
+	t.Run("error", func(t *testing.T) {
+		_, err := parseListNames(nil, "", errors.New("bonk"))
+		assert.Error(t, err)
+		assert.Equal(t, "bonk", err.Error())
+	})
+	t.Run("statusSet", func(t *testing.T) {
+		_, err := parseListNames(nil, "unexpected!", nil)
+		assert.Error(t, err)
+	})
+	t.Run("badJSON", func(t *testing.T) {
+		_, err := parseListNames([]byte("Foo[[["), "", nil)
+		assert.Error(t, err)
+	})
+	t.Run("ok", func(t *testing.T) {
+		l, err := parseListNames([]byte(`[{"name":"bob"}]`), "", nil)
+		assert.NoError(t, err)
+		if assert.Len(t, l, 1) {
+			assert.Equal(t, "bob", l[0])
+		}
+	})
+}
+
+func TestCheckEmptyResponseExpected(t *testing.T) {
+	t.Run("error", func(t *testing.T) {
+		err := checkEmptyResponseExpected(nil, "", errors.New("bonk"))
+		assert.Error(t, err)
+		assert.Equal(t, "bonk", err.Error())
+	})
+	t.Run("statusSet", func(t *testing.T) {
+		err := checkEmptyResponseExpected(nil, "unexpected!", nil)
+		assert.Error(t, err)
+	})
+	t.Run("someJSON", func(t *testing.T) {
+		err := checkEmptyResponseExpected([]byte(`{"trouble": true}`), "", nil)
+		assert.Error(t, err)
+	})
+	t.Run("ok", func(t *testing.T) {
+		err := checkEmptyResponseExpected([]byte{}, "", nil)
+		assert.NoError(t, err)
+	})
+}
diff --git a/cephfs/admin/subvolume_test.go b/cephfs/admin/subvolume_test.go
new file mode 100644
index 0000000..e3a4a9a
--- /dev/null
+++ b/cephfs/admin/subvolume_test.go
@@ -0,0 +1,175 @@
+// +build !luminous,!mimic
+
+package admin
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func delay() {
+	// ceph seems to do this (partly?) async. So for now, we cheat
+	// and sleep a little to make subsequent tests more reliable
+	time.Sleep(50 * time.Millisecond)
+}
+
+func TestCreateSubVolume(t *testing.T) {
+	fsa := getFSAdmin(t)
+	volume := "cephfs"
+	type gn struct {
+		group string
+		name  string
+	}
+	created := []gn{}
+	defer func() {
+		for _, c := range created {
+			err := fsa.RemoveSubVolume(volume, c.group, c.name)
+			assert.NoError(t, err)
+			delay()
+			if c.group != NoGroup {
+				err := fsa.RemoveSubVolumeGroup(volume, c.group)
+				assert.NoError(t, err)
+			}
+		}
+	}()
+
+	t.Run("simple", func(t *testing.T) {
+		subname := "SubVol1"
+		err := fsa.CreateSubVolume(volume, NoGroup, subname, nil)
+		assert.NoError(t, err)
+		created = append(created, gn{NoGroup, subname})
+
+		lsv, err := fsa.ListSubVolumes(volume, NoGroup)
+		assert.NoError(t, err)
+		assert.GreaterOrEqual(t, len(lsv), 1)
+		assert.Contains(t, lsv, subname)
+	})
+
+	t.Run("options", func(t *testing.T) {
+		subname := "SubVol2"
+		o := &SubVolumeOptions{
+			Mode: 0777,
+			Uid:  200,
+			Gid:  200,
+		}
+		err := fsa.CreateSubVolume(volume, NoGroup, subname, o)
+		assert.NoError(t, err)
+		created = append(created, gn{NoGroup, subname})
+
+		lsv, err := fsa.ListSubVolumes(volume, NoGroup)
+		assert.NoError(t, err)
+		assert.GreaterOrEqual(t, len(lsv), 1)
+		assert.Contains(t, lsv, subname)
+	})
+
+	t.Run("withGroup", func(t *testing.T) {
+		group := "withGroup1"
+		subname := "SubVol3"
+
+		err := fsa.CreateSubVolumeGroup(volume, group, nil)
+		assert.NoError(t, err)
+
+		err = fsa.CreateSubVolume(volume, group, subname, nil)
+		assert.NoError(t, err)
+		created = append(created, gn{group, subname})
+
+		lsv, err := fsa.ListSubVolumes(volume, group)
+		assert.NoError(t, err)
+		assert.GreaterOrEqual(t, len(lsv), 1)
+		assert.Contains(t, lsv, subname)
+	})
+
+	t.Run("groupAndOptions", func(t *testing.T) {
+		group := "withGroup2"
+		subname := "SubVol4"
+		err := fsa.CreateSubVolumeGroup(volume, group, nil)
+		assert.NoError(t, err)
+
+		o := &SubVolumeOptions{
+			Size: 5 * gibiByte,
+			Mode: 0777,
+			Uid:  200,
+			Gid:  200,
+		}
+		err = fsa.CreateSubVolume(volume, group, subname, o)
+		assert.NoError(t, err)
+		created = append(created, gn{group, subname})
+
+		lsv, err := fsa.ListSubVolumes(volume, group)
+		assert.NoError(t, err)
+		assert.GreaterOrEqual(t, len(lsv), 1)
+		assert.Contains(t, lsv, subname)
+	})
+}
+
+func TestRemoveSubVolume(t *testing.T) {
+	fsa := getFSAdmin(t)
+	volume := "cephfs"
+
+	lsv, err := fsa.ListSubVolumes(volume, NoGroup)
+	assert.NoError(t, err)
+	beforeCount := len(lsv)
+
+	err = fsa.CreateSubVolume(volume, NoGroup, "deletemev1", nil)
+	assert.NoError(t, err)
+
+	lsv, err = fsa.ListSubVolumes(volume, NoGroup)
+	assert.NoError(t, err)
+	afterCount := len(lsv)
+	assert.Equal(t, beforeCount, afterCount-1)
+
+	err = fsa.RemoveSubVolume(volume, NoGroup, "deletemev1")
+	assert.NoError(t, err)
+
+	delay()
+	lsv, err = fsa.ListSubVolumes(volume, NoGroup)
+	assert.NoError(t, err)
+	nowCount := len(lsv)
+	if !assert.Equal(t, beforeCount, nowCount) {
+		// this is a hack for debugging a flapping test
+		assert.Equal(t, []string{}, lsv)
+	}
+}
+
+func TestResizeSubVolume(t *testing.T) {
+	fsa := getFSAdmin(t)
+	volume := "cephfs"
+	group := "sizedGroup"
+	subname := "sizeMe1"
+
+	err := fsa.CreateSubVolumeGroup(volume, group, nil)
+	assert.NoError(t, err)
+	defer func() {
+		err := fsa.RemoveSubVolumeGroup(volume, group)
+		assert.NoError(t, err)
+	}()
+
+	svopts := &SubVolumeOptions{
+		Mode: 0777,
+		Size: 20 * gibiByte,
+	}
+	err = fsa.CreateSubVolume(volume, group, subname, svopts)
+	assert.NoError(t, err)
+	defer func() {
+		err := fsa.RemoveSubVolume(volume, group, subname)
+		assert.NoError(t, err)
+	}()
+
+	lsv, err := fsa.ListSubVolumes(volume, group)
+	assert.NoError(t, err)
+	assert.Contains(t, lsv, subname)
+
+	rr, err := fsa.ResizeSubVolume(volume, group, subname, 30*gibiByte, false)
+	assert.NoError(t, err)
+	assert.NotNil(t, rr)
+
+	rr, err = fsa.ResizeSubVolume(volume, group, subname, 10*gibiByte, true)
+	assert.NoError(t, err)
+	assert.NotNil(t, rr)
+
+	rr, err = fsa.ResizeSubVolume(volume, group, subname, Infinite, true)
+	assert.NoError(t, err)
+	assert.NotNil(t, rr)
+}
diff --git a/cephfs/admin/subvolumegroup_test.go b/cephfs/admin/subvolumegroup_test.go
new file mode 100644
index 0000000..79a0d57
--- /dev/null
+++ b/cephfs/admin/subvolumegroup_test.go
@@ -0,0 +1,89 @@
+// +build !luminous,!mimic
+
+package admin
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCreateSubVolumeGroup(t *testing.T) {
+	fsa := getFSAdmin(t)
+	volume := "cephfs"
+	created := []string{}
+	defer func() {
+		for _, name := range created {
+			err := fsa.RemoveSubVolumeGroup(volume, name)
+			assert.NoError(t, err)
+		}
+	}()
+
+	t.Run("simple", func(t *testing.T) {
+		svgroup := "svg1"
+		err := fsa.CreateSubVolumeGroup(volume, svgroup, nil)
+		assert.NoError(t, err)
+		created = append(created, svgroup)
+
+		lsvg, err := fsa.ListSubVolumeGroups(volume)
+		assert.NoError(t, err)
+		assert.GreaterOrEqual(t, len(lsvg), 1)
+		assert.Contains(t, lsvg, svgroup)
+	})
+
+	t.Run("options1", func(t *testing.T) {
+		svgroup := "svg2"
+		err := fsa.CreateSubVolumeGroup(volume, svgroup, &SubVolumeGroupOptions{
+			Mode: 0777,
+		})
+		assert.NoError(t, err)
+		created = append(created, svgroup)
+
+		lsvg, err := fsa.ListSubVolumeGroups(volume)
+		assert.NoError(t, err)
+		assert.GreaterOrEqual(t, len(lsvg), 1)
+		assert.Contains(t, lsvg, svgroup)
+	})
+
+	t.Run("options2", func(t *testing.T) {
+		svgroup := "anotherSVG"
+		err := fsa.CreateSubVolumeGroup(volume, svgroup, &SubVolumeGroupOptions{
+			Uid:  200,
+			Gid:  200,
+			Mode: 0771,
+			// TODO: test pool_layout... I think its a pool name
+		})
+		assert.NoError(t, err)
+		created = append(created, svgroup)
+
+		lsvg, err := fsa.ListSubVolumeGroups(volume)
+		assert.NoError(t, err)
+		assert.GreaterOrEqual(t, len(lsvg), 1)
+		assert.Contains(t, lsvg, svgroup)
+	})
+}
+
+func TestRemoveSubVolumeGroup(t *testing.T) {
+	fsa := getFSAdmin(t)
+	volume := "cephfs"
+
+	lsvg, err := fsa.ListSubVolumeGroups(volume)
+	assert.NoError(t, err)
+	beforeCount := len(lsvg)
+
+	err = fsa.CreateSubVolumeGroup(volume, "deleteme1", nil)
+	assert.NoError(t, err)
+
+	lsvg, err = fsa.ListSubVolumeGroups(volume)
+	assert.NoError(t, err)
+	afterCount := len(lsvg)
+	assert.Equal(t, beforeCount, afterCount-1)
+
+	err = fsa.RemoveSubVolumeGroup(volume, "deleteme1")
+	assert.NoError(t, err)
+
+	lsvg, err = fsa.ListSubVolumeGroups(volume)
+	assert.NoError(t, err)
+	nowCount := len(lsvg)
+	assert.Equal(t, beforeCount, nowCount)
+}
diff --git a/cephfs/admin/volume_octopus_test.go b/cephfs/admin/volume_octopus_test.go
new file mode 100644
index 0000000..bf8b87b
--- /dev/null
+++ b/cephfs/admin/volume_octopus_test.go
@@ -0,0 +1,51 @@
+// +build octopus
+
+package admin
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestVolumeStatus(t *testing.T) {
+	fsa := getFSAdmin(t)
+
+	vs, err := fsa.VolumeStatus("cephfs")
+	assert.NoError(t, err)
+	assert.Contains(t, vs.MDSVersion, "version")
+}
+
+var sampleVolumeStatus1 = []byte(`
+{
+"clients": [{"clients": 1, "fs": "cephfs"}],
+"mds_version": "ceph version 15.2.4 (7447c15c6ff58d7fce91843b705a268a1917325c) octopus (stable)",
+"mdsmap": [{"dns": 76, "inos": 19, "name": "Z", "rank": 0, "rate": 0.0, "state": "active"}],
+"pools": [{"avail": 1017799872, "id": 2, "name": "cephfs_metadata", "type": "metadata", "used": 2204126}, {"avail": 1017799872, "id": 1, "name": "cephfs_data", "type": "data", "used": 0}]
+}
+`)
+
+func TestParseVolumeStatus(t *testing.T) {
+	t.Run("error", func(t *testing.T) {
+		_, err := parseVolumeStatus(nil, "", errors.New("bonk"))
+		assert.Error(t, err)
+		assert.Equal(t, "bonk", err.Error())
+	})
+	t.Run("statusSet", func(t *testing.T) {
+		_, err := parseVolumeStatus(nil, "unexpected!", nil)
+		assert.Error(t, err)
+	})
+	t.Run("badJSON", func(t *testing.T) {
+		_, err := parseVolumeStatus([]byte("_XxXxX"), "", nil)
+		assert.Error(t, err)
+	})
+	t.Run("ok", func(t *testing.T) {
+		s, err := parseVolumeStatus(sampleVolumeStatus1, "", nil)
+		assert.NoError(t, err)
+		if assert.NotNil(t, s) {
+			assert.Contains(t, s.MDSVersion, "ceph version 15.2.4")
+			assert.Contains(t, s.MDSVersion, "octopus")
+		}
+	})
+}
diff --git a/cephfs/admin/volume_test.go b/cephfs/admin/volume_test.go
new file mode 100644
index 0000000..98aebf4
--- /dev/null
+++ b/cephfs/admin/volume_test.go
@@ -0,0 +1,18 @@
+// +build !luminous,!mimic
+
+package admin
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestListVolumes(t *testing.T) {
+	fsa := getFSAdmin(t)
+
+	vl, err := fsa.ListVolumes()
+	assert.NoError(t, err)
+	assert.Len(t, vl, 1)
+	assert.Equal(t, "cephfs", vl[0])
+}