Repository: ceph/go-ceph
Pull request #463 (2021-03-03 20:05:32)

Add a new internal commands package for functions common to running mon/mgr commands
Soon, I will be submitting patches to add a new 'rbd/admin' subpackage. As some of this code is simlar to what we are doing in cephfs/admin, this PR extracts some of the most common code into a new internal subpackage called 'commands'.

## 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: @mergifyio rebase
mergify[bot]: **Command `rebase`: success**
> **Branch has been successfully rebased**
> 

phlogistonjohn: @Mergifyio rebase

mergify[bot]: **Command `rebase`: success**
> **Branch has been successfully rebased**
> 

phlogistonjohn: @agarwal-mudit or @ansiwen PTAL when you have some time, thanks!
phlogistonjohn: @Mergifyio rebase
mergify[bot]: **Command `rebase`: success**
> **Branch has been successfully rebased**
> 

phlogistonjohn: @Mergifyio rebase
mergify[bot]: **Command `rebase`: success**
> **Branch has been successfully rebased**
> 


Patch:
diff --git a/cephfs/admin/clone.go b/cephfs/admin/clone.go
index d314310..c904792 100644
--- a/cephfs/admin/clone.go
+++ b/cephfs/admin/clone.go
@@ -53,10 +53,10 @@ func (fsa *FSAdmin) CloneSubVolumeSnapshot(volume, group, subvolume, snapshot, n
 }
 
 func checkCloneResponse(res response) error {
-	if strings.HasSuffix(res.status, notProtectedSuffix) {
+	if strings.HasSuffix(res.Status(), notProtectedSuffix) {
 		return NotProtectedError{response: res}
 	}
-	return res.noData().End()
+	return res.NoData().End()
 }
 
 // CloneState is used to define constant values used to determine the state of
@@ -94,7 +94,7 @@ type cloneStatusWrapper struct {
 
 func parseCloneStatus(res response) (*CloneStatus, error) {
 	var status cloneStatusWrapper
-	if err := res.noStatus().unmarshal(&status).End(); err != nil {
+	if err := res.NoStatus().Unmarshal(&status).End(); err != nil {
 		return nil, err
 	}
 	return &status.Status, nil
@@ -132,5 +132,5 @@ func (fsa *FSAdmin) CancelClone(volume, group, clone string) error {
 	if group != NoGroup {
 		m["group_name"] = group
 	}
-	return fsa.marshalMgrCommand(m).noData().End()
+	return fsa.marshalMgrCommand(m).NoData().End()
 }
diff --git a/cephfs/admin/fsadmin.go b/cephfs/admin/fsadmin.go
index e7bed2d..3555c3e 100644
--- a/cephfs/admin/fsadmin.go
+++ b/cephfs/admin/fsadmin.go
@@ -3,18 +3,15 @@
 package admin
 
 import (
-	"encoding/json"
 	"strconv"
 
+	"github.com/ceph/go-ceph/internal/commands"
 	"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)
-	MonCommand(buf []byte) ([]byte, string, error)
-}
+type RadosCommander = commands.RadosCommander
 
 // FSAdmin is used to administrate CephFS within a ceph cluster.
 type FSAdmin struct {
@@ -60,39 +57,25 @@ func (fsa *FSAdmin) validate() error {
 // 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) response {
-	if err := fsa.validate(); err != nil {
-		return response{err: err}
-	}
-	return newResponse(fsa.conn.MgrCommand([][]byte{buf}))
+	return commands.RawMgrCommand(fsa.conn, 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{}) response {
-	b, err := json.Marshal(v)
-	if err != nil {
-		return response{err: err}
-	}
-	return fsa.rawMgrCommand(b)
+	return commands.MarshalMgrCommand(fsa.conn, v)
 }
 
 // rawMonCommand takes a byte buffer and sends it to the MON as a command.
 // The buffer is expected to contain preformatted JSON.
 func (fsa *FSAdmin) rawMonCommand(buf []byte) response {
-	if err := fsa.validate(); err != nil {
-		return response{err: err}
-	}
-	return newResponse(fsa.conn.MonCommand(buf))
+	return commands.RawMonCommand(fsa.conn, buf)
 }
 
 // marshalMonCommand takes an generic interface{} value, converts it to JSON and
 // sends the json to the MGR as a command.
 func (fsa *FSAdmin) marshalMonCommand(v interface{}) response {
-	b, err := json.Marshal(v)
-	if err != nil {
-		return response{err: err}
-	}
-	return fsa.rawMonCommand(b)
+	return commands.MarshalMonCommand(fsa.conn, v)
 }
 
 type listNamedResult struct {
@@ -101,7 +84,7 @@ type listNamedResult struct {
 
 func parseListNames(res response) ([]string, error) {
 	var r []listNamedResult
-	if err := res.noStatus().unmarshal(&r).End(); err != nil {
+	if err := res.NoStatus().Unmarshal(&r).End(); err != nil {
 		return nil, err
 	}
 	vl := make([]string, len(r))
@@ -114,10 +97,10 @@ func parseListNames(res response) ([]string, error) {
 // parsePathResponse returns a cleaned up path from requests that get a path
 // unless an error is encountered, then an error is returned.
 func parsePathResponse(res response) (string, error) {
-	if res2 := res.noStatus(); !res2.Ok() {
+	if res2 := res.NoStatus(); !res2.Ok() {
 		return "", res.End()
 	}
-	b := res.body
+	b := res.Body()
 	// if there's a trailing newline in the buffer strip it.
 	// ceph assumes a CLI wants the output of the buffer and there's
 	// no format=json mode available currently.
diff --git a/cephfs/admin/response.go b/cephfs/admin/response.go
index a1b6bf3..29e2df4 100644
--- a/cephfs/admin/response.go
+++ b/cephfs/admin/response.go
@@ -3,140 +3,22 @@
 package admin
 
 import (
-	"encoding/json"
-	"errors"
-	"fmt"
-	"strings"
+	"github.com/ceph/go-ceph/internal/commands"
 )
 
 var (
-	// ErrStatusNotEmpty may be returned if a call should not have a status
-	// string set but one is.
-	ErrStatusNotEmpty = errors.New("response status not empty")
-	// ErrBodyNotEmpty may be returned if a call should have an empty body but
-	// a body value is present.
-	ErrBodyNotEmpty = errors.New("response body not empty")
+	// ErrStatusNotEmpty is an alias for commands.ErrStatusNotEmpty
+	ErrStatusNotEmpty = commands.ErrStatusNotEmpty
+	// ErrBodyNotEmpty is an alias for commands.ErrBodyNotEmpty
+	ErrBodyNotEmpty = commands.ErrBodyNotEmpty
 )
 
-const (
-	deprecatedSuffix = "call is deprecated and will be removed in a future release"
-	missingPrefix    = "No handler found"
-	einval           = -22
-)
-
-type cephError interface {
-	ErrorCode() int
-}
-
-// NotImplementedError error values will be returned in the case that an API
-// call is not available in the version of Ceph that is running in the target
-// cluster.
-type NotImplementedError struct {
-	response
-}
-
-// Error implements the error interface.
-func (e NotImplementedError) Error() string {
-	return fmt.Sprintf("API call not implemented server-side: %s", e.status)
-}
-
-// response encapsulates the data returned by ceph and supports easy processing
-// pipelines.
-type response struct {
-	body   []byte
-	status string
-	err    error
-}
-
-// Ok returns true if the response contains no error.
-func (r response) Ok() bool {
-	return r.err == nil
-}
-
-// Error implements the error interface.
-func (r response) Error() string {
-	if r.status == "" {
-		return r.err.Error()
-	}
-	return fmt.Sprintf("%s: %q", r.err, r.status)
-}
-
-// Unwrap returns the error this response contains.
-func (r response) Unwrap() error {
-	return r.err
-}
-
-// Status returns the status string value.
-func (r response) Status() string {
-	return r.status
-}
-
-// End returns an error if the response contains an error or nil, indicating
-// that response is no longer needed for processing.
-func (r response) End() error {
-	if !r.Ok() {
-		if ce, ok := r.err.(cephError); ok {
-			if ce.ErrorCode() == einval && strings.HasPrefix(r.status, missingPrefix) {
-				return NotImplementedError{response: r}
-			}
-		}
-		return r
-	}
-	return nil
-}
-
-// noStatus asserts that the input response has no status value.
-func (r response) noStatus() response {
-	if !r.Ok() {
-		return r
-	}
-	if r.status != "" {
-		return response{r.body, r.status, ErrStatusNotEmpty}
-	}
-	return r
-}
+type response = commands.Response
 
-// noBody asserts that the input response has no body value.
-func (r response) noBody() response {
-	if !r.Ok() {
-		return r
-	}
-	if len(r.body) != 0 {
-		return response{r.body, r.status, ErrBodyNotEmpty}
-	}
-	return r
-}
-
-// noData asserts that the input response has no status or body values.
-func (r response) noData() response {
-	return r.noStatus().noBody()
-}
-
-// filterDeprecated removes deprecation warnings from the response status.
-// Use it when checking the response from calls that may be deprecated in ceph
-// if you want those calls to continue working if the warning is present.
-func (r response) filterDeprecated() response {
-	if !r.Ok() {
-		return r
-	}
-	if strings.HasSuffix(r.status, deprecatedSuffix) {
-		return response{r.body, "", r.err}
-	}
-	return r
-}
-
-// unmarshal data from the response body into v.
-func (r response) unmarshal(v interface{}) response {
-	if !r.Ok() {
-		return r
-	}
-	if err := json.Unmarshal(r.body, v); err != nil {
-		return response{body: r.body, err: err}
-	}
-	return r
-}
+// NotImplementedError is an alias for commands.NotImplementedError.
+type NotImplementedError = commands.NotImplementedError
 
 // newResponse returns a response.
 func newResponse(b []byte, s string, e error) response {
-	return response{b, s, e}
+	return commands.NewResponse(b, s, e)
 }
diff --git a/cephfs/admin/subvolume.go b/cephfs/admin/subvolume.go
index 69fb4f9..dec96d4 100644
--- a/cephfs/admin/subvolume.go
+++ b/cephfs/admin/subvolume.go
@@ -61,7 +61,7 @@ func (fsa *FSAdmin) CreateSubVolume(volume, group, name string, o *SubVolumeOpti
 		o = &SubVolumeOptions{}
 	}
 	f := o.toFields(volume, group, name)
-	return fsa.marshalMgrCommand(f).noData().End()
+	return fsa.marshalMgrCommand(f).NoData().End()
 }
 
 // ListSubVolumes returns a list of subvolumes belonging to the volume and
@@ -117,7 +117,7 @@ func (fsa *FSAdmin) RemoveSubVolumeWithFlags(volume, group, name string, o SubVo
 	if group != NoGroup {
 		m["group_name"] = group
 	}
-	return fsa.marshalMgrCommand(mergeFlags(m, o)).noData().End()
+	return fsa.marshalMgrCommand(mergeFlags(m, o)).NoData().End()
 }
 
 type subVolumeResizeFields struct {
@@ -159,7 +159,7 @@ func (fsa *FSAdmin) ResizeSubVolume(
 	}
 	var result []*SubVolumeResizeResult
 	res := fsa.marshalMgrCommand(f)
-	if err := res.noStatus().unmarshal(&result).End(); err != nil {
+	if err := res.NoStatus().Unmarshal(&result).End(); err != nil {
 		return nil, err
 	}
 	return result[0], nil
@@ -248,7 +248,7 @@ type subVolumeInfoWrapper struct {
 
 func parseSubVolumeInfo(res response) (*SubVolumeInfo, error) {
 	var info subVolumeInfoWrapper
-	if err := res.noStatus().unmarshal(&info).End(); err != nil {
+	if err := res.NoStatus().Unmarshal(&info).End(); err != nil {
 		return nil, err
 	}
 	if info.VBytesQuota != nil {
@@ -289,7 +289,7 @@ func (fsa *FSAdmin) CreateSubVolumeSnapshot(volume, group, source, name string)
 	if group != NoGroup {
 		m["group_name"] = group
 	}
-	return fsa.marshalMgrCommand(m).noData().End()
+	return fsa.marshalMgrCommand(m).NoData().End()
 }
 
 // RemoveSubVolumeSnapshot removes the specified snapshot from the subvolume.
@@ -320,7 +320,7 @@ func (fsa *FSAdmin) rmSubVolumeSnapshot(volume, group, subvolume, name string, o
 	if group != NoGroup {
 		m["group_name"] = group
 	}
-	return fsa.marshalMgrCommand(mergeFlags(m, o)).noData().End()
+	return fsa.marshalMgrCommand(mergeFlags(m, o)).NoData().End()
 }
 
 // ListSubVolumeSnapshots returns a listing of snapshots for a given subvolume.
@@ -351,7 +351,7 @@ type SubVolumeSnapshotInfo struct {
 
 func parseSubVolumeSnapshotInfo(res response) (*SubVolumeSnapshotInfo, error) {
 	var info SubVolumeSnapshotInfo
-	if err := res.noStatus().unmarshal(&info).End(); err != nil {
+	if err := res.NoStatus().Unmarshal(&info).End(); err != nil {
 		return nil, err
 	}
 	return &info, nil
@@ -390,7 +390,7 @@ func (fsa *FSAdmin) ProtectSubVolumeSnapshot(volume, group, subvolume, name stri
 	if group != NoGroup {
 		m["group_name"] = group
 	}
-	return fsa.marshalMgrCommand(m).filterDeprecated().noData().End()
+	return fsa.marshalMgrCommand(m).FilterDeprecated().NoData().End()
 }
 
 // UnprotectSubVolumeSnapshot removes protection from the specified snapshot.
@@ -408,5 +408,5 @@ func (fsa *FSAdmin) UnprotectSubVolumeSnapshot(volume, group, subvolume, name st
 	if group != NoGroup {
 		m["group_name"] = group
 	}
-	return fsa.marshalMgrCommand(m).filterDeprecated().noData().End()
+	return fsa.marshalMgrCommand(m).FilterDeprecated().NoData().End()
 }
diff --git a/cephfs/admin/subvolumegroup.go b/cephfs/admin/subvolumegroup.go
index 573c8c1..455ec49 100644
--- a/cephfs/admin/subvolumegroup.go
+++ b/cephfs/admin/subvolumegroup.go
@@ -48,7 +48,7 @@ func (fsa *FSAdmin) CreateSubVolumeGroup(volume, name string, o *SubVolumeGroupO
 		o = &SubVolumeGroupOptions{}
 	}
 	res := fsa.marshalMgrCommand(o.toFields(volume, name))
-	return res.noData().End()
+	return res.NoData().End()
 }
 
 // ListSubVolumeGroups returns a list of subvolume groups belonging to the
@@ -86,7 +86,7 @@ func (fsa *FSAdmin) rmSubVolumeGroup(volume, name string, o commonRmFlags) error
 		"group_name": name,
 		"format":     "json",
 	}, o))
-	return res.noData().End()
+	return res.NoData().End()
 }
 
 // SubVolumeGroupPath returns the path to the subvolume from the root of the
diff --git a/cephfs/admin/volume.go b/cephfs/admin/volume.go
index 082fa4d..cbca3a8 100644
--- a/cephfs/admin/volume.go
+++ b/cephfs/admin/volume.go
@@ -43,7 +43,7 @@ func (fsa *FSAdmin) ListFileSystems() ([]FSPoolInfo, error) {
 
 func parseFsList(res response) ([]FSPoolInfo, error) {
 	var listing []FSPoolInfo
-	if err := res.noStatus().unmarshal(&listing).End(); err != nil {
+	if err := res.NoStatus().Unmarshal(&listing).End(); err != nil {
 		return nil, err
 	}
 	return listing, nil
@@ -78,13 +78,8 @@ func parseDumpToIdents(res response) ([]VolumeIdent, error) {
 	if !res.Ok() {
 		return nil, res.End()
 	}
-	if len(res.status) >= dumpOkLen && res.status[:dumpOkLen] == dumpOkPrefix {
-		// Unhelpfully, ceph drops a status string on success responses for this
-		// call. this hacks around that by ignoring its typical prefix
-		res.status = ""
-	}
 	var dump fsDump
-	if err := res.noStatus().unmarshal(&dump).End(); err != nil {
+	if err := res.FilterPrefix(dumpOkPrefix).NoStatus().Unmarshal(&dump).End(); err != nil {
 		return nil, err
 	}
 	// copy the dump json into the simpler enumeration list
@@ -123,15 +118,16 @@ type VolumeStatus struct {
 
 func parseVolumeStatus(res response) (*VolumeStatus, error) {
 	var vs VolumeStatus
-	res = res.noStatus()
+	res = res.NoStatus()
 	if !res.Ok() {
 		return nil, res.End()
 	}
-	res = res.unmarshal(&vs)
+	res = res.Unmarshal(&vs)
 	if !res.Ok() {
-		if bytes.HasPrefix(res.body, []byte("ceph")) {
-			res.status = invalidTextualResponse
-			return nil, NotImplementedError{response: res}
+		if bytes.HasPrefix(res.Body(), []byte("ceph")) {
+			return nil, NotImplementedError{
+				Response: newResponse(res.Body(), invalidTextualResponse, res.Unwrap()),
+			}
 		}
 		return nil, res.End()
 	}
diff --git a/internal/commands/commands.go b/internal/commands/commands.go
new file mode 100644
index 0000000..095140f
--- /dev/null
+++ b/internal/commands/commands.go
@@ -0,0 +1,71 @@
+package commands
+
+import (
+	"encoding/json"
+
+	"github.com/ceph/go-ceph/rados"
+)
+
+// MgrCommander in an interface for the API needed to execute JSON formatted
+// commands on the ceph mgr.
+type MgrCommander interface {
+	MgrCommand(buf [][]byte) ([]byte, string, error)
+}
+
+// MonCommander is an interface for the API needed to execute JSON formatted
+// commands on the ceph mon(s).
+type MonCommander interface {
+	MonCommand(buf []byte) ([]byte, string, error)
+}
+
+// RadosCommander provides an interface for APIs needed to execute JSON
+// formatted commands on the Ceph cluster.
+type RadosCommander interface {
+	MgrCommander
+	MonCommander
+}
+
+func validate(m interface{}) error {
+	if m == 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 RawMgrCommand(m MgrCommander, buf []byte) Response {
+	if err := validate(m); err != nil {
+		return Response{err: err}
+	}
+	return NewResponse(m.MgrCommand([][]byte{buf}))
+}
+
+// MarshalMgrCommand takes an generic interface{} value, converts it to JSON
+// and sends the json to the MGR as a command.
+func MarshalMgrCommand(m MgrCommander, v interface{}) Response {
+	b, err := json.Marshal(v)
+	if err != nil {
+		return Response{err: err}
+	}
+	return RawMgrCommand(m, b)
+}
+
+// RawMonCommand takes a byte buffer and sends it to the MON as a command.
+// The buffer is expected to contain preformatted JSON.
+func RawMonCommand(m MonCommander, buf []byte) Response {
+	if err := validate(m); err != nil {
+		return Response{err: err}
+	}
+	return NewResponse(m.MonCommand(buf))
+}
+
+// MarshalMonCommand takes an generic interface{} value, converts it to JSON
+// and sends the json to the MGR as a command.
+func MarshalMonCommand(m MonCommander, v interface{}) Response {
+	b, err := json.Marshal(v)
+	if err != nil {
+		return Response{err: err}
+	}
+	return RawMonCommand(m, b)
+}
diff --git a/internal/commands/response.go b/internal/commands/response.go
new file mode 100644
index 0000000..86a56d3
--- /dev/null
+++ b/internal/commands/response.go
@@ -0,0 +1,163 @@
+package commands
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"strings"
+)
+
+var (
+	// ErrStatusNotEmpty may be returned if a call should not have a status
+	// string set but one is.
+	ErrStatusNotEmpty = errors.New("response status not empty")
+	// ErrBodyNotEmpty may be returned if a call should have an empty body but
+	// a body value is present.
+	ErrBodyNotEmpty = errors.New("response body not empty")
+)
+
+const (
+	deprecatedSuffix = "call is deprecated and will be removed in a future release"
+	missingPrefix    = "No handler found"
+	einval           = -22
+)
+
+type cephError interface {
+	ErrorCode() int
+}
+
+// NotImplementedError error values will be returned in the case that an API
+// call is not available in the version of Ceph that is running in the target
+// cluster.
+type NotImplementedError struct {
+	Response
+}
+
+// Error implements the error interface.
+func (e NotImplementedError) Error() string {
+	return fmt.Sprintf("API call not implemented server-side: %s", e.status)
+}
+
+// Response encapsulates the data returned by ceph and supports easy processing
+// pipelines.
+type Response struct {
+	body   []byte
+	status string
+	err    error
+}
+
+// Ok returns true if the response contains no error.
+func (r Response) Ok() bool {
+	return r.err == nil
+}
+
+// Error implements the error interface.
+func (r Response) Error() string {
+	if r.status == "" {
+		return r.err.Error()
+	}
+	return fmt.Sprintf("%s: %q", r.err, r.status)
+}
+
+// Unwrap returns the error this response contains.
+func (r Response) Unwrap() error {
+	return r.err
+}
+
+// Status returns the status string value.
+func (r Response) Status() string {
+	return r.status
+}
+
+// Body returns the response body as a raw byte-slice.
+func (r Response) Body() []byte {
+	return r.body
+}
+
+// End returns an error if the response contains an error or nil, indicating
+// that response is no longer needed for processing.
+func (r Response) End() error {
+	if !r.Ok() {
+		if ce, ok := r.err.(cephError); ok {
+			if ce.ErrorCode() == einval && strings.HasPrefix(r.status, missingPrefix) {
+				return NotImplementedError{Response: r}
+			}
+		}
+		return r
+	}
+	return nil
+}
+
+// NoStatus asserts that the input response has no status value.
+func (r Response) NoStatus() Response {
+	if !r.Ok() {
+		return r
+	}
+	if r.status != "" {
+		return Response{r.body, r.status, ErrStatusNotEmpty}
+	}
+	return r
+}
+
+// NoBody asserts that the input response has no body value.
+func (r Response) NoBody() Response {
+	if !r.Ok() {
+		return r
+	}
+	if len(r.body) != 0 {
+		return Response{r.body, r.status, ErrBodyNotEmpty}
+	}
+	return r
+}
+
+// NoData asserts that the input response has no status or body values.
+func (r Response) NoData() Response {
+	return r.NoStatus().NoBody()
+}
+
+// FilterPrefix sets the status value to an empty string if the status
+// value contains the given prefix string.
+func (r Response) FilterPrefix(p string) Response {
+	if !r.Ok() {
+		return r
+	}
+	if strings.HasPrefix(r.status, p) {
+		return Response{r.body, "", r.err}
+	}
+	return r
+}
+
+// FilterSuffix sets the status value to an empty string if the status
+// value contains the given suffix string.
+func (r Response) FilterSuffix(s string) Response {
+	if !r.Ok() {
+		return r
+	}
+	if strings.HasSuffix(r.status, s) {
+		return Response{r.body, "", r.err}
+	}
+	return r
+}
+
+// FilterDeprecated removes deprecation warnings from the response status.
+// Use it when checking the response from calls that may be deprecated in ceph
+// if you want those calls to continue working if the warning is present.
+func (r Response) FilterDeprecated() Response {
+	return r.FilterSuffix(deprecatedSuffix)
+}
+
+// Unmarshal data from the response body into v.
+func (r Response) Unmarshal(v interface{}) Response {
+	if !r.Ok() {
+		return r
+	}
+	if err := json.Unmarshal(r.body, v); err != nil {
+		return Response{body: r.body, err: err}
+	}
+	return r
+}
+
+// NewResponse returns a response.
+func NewResponse(b []byte, s string, e error) Response {
+	return Response{b, s, e}
+}
diff --git a/internal/commands/trace.go b/internal/commands/trace.go
new file mode 100644
index 0000000..687249d
--- /dev/null
+++ b/internal/commands/trace.go
@@ -0,0 +1,51 @@
+package commands
+
+import (
+	"fmt"
+)
+
+// NewTraceCommander is a RadosCommander that wraps a given RadosCommander
+// and when commands are executes prints debug level "traces" to the
+// standard output.
+func NewTraceCommander(c RadosCommander) RadosCommander {
+	return &tracingCommander{c}
+}
+
+// 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 (t *tracingCommander) MgrCommand(buf [][]byte) ([]byte, string, error) {
+	fmt.Println("(MGR Command)")
+	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 (t *tracingCommander) MonCommand(buf []byte) ([]byte, string, error) {
+	fmt.Println("(MON Command)")
+	fmt.Println("IN:", string(buf))
+	r, s, err := t.conn.MonCommand(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
+}


Test patch:
diff --git a/cephfs/admin/fsadmin_test.go b/cephfs/admin/fsadmin_test.go
index dfc1294..517398a 100644
--- a/cephfs/admin/fsadmin_test.go
+++ b/cephfs/admin/fsadmin_test.go
@@ -4,7 +4,6 @@ package admin
 
 import (
 	"errors"
-	"fmt"
 	"os"
 	"strconv"
 	"testing"
@@ -12,6 +11,8 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/ceph/go-ceph/internal/commands"
 )
 
 var (
@@ -53,49 +54,6 @@ func TestServerSentinel(t *testing.T) {
 	}
 }
 
-// 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) {
-	fmt.Println("(MGR Command)")
-	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 (t *tracingCommander) MonCommand(buf []byte) ([]byte, string, error) {
-	fmt.Println("(MON Command)")
-	fmt.Println("IN:", string(buf))
-	r, s, err := t.conn.MonCommand(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
@@ -107,7 +65,7 @@ func getFSAdmin(t *testing.T) *FSAdmin {
 	// optional tracer.
 	c := fsa.conn
 	if debugTrace {
-		c = tracer(c)
+		c = commands.NewTraceCommander(c)
 	}
 	cachedFSAdmin = NewFromConn(c)
 	// We sleep briefly before returning in order to ensure we have a mgr map
@@ -163,20 +121,20 @@ func TestParseListNames(t *testing.T) {
 func TestCheckEmptyResponseExpected(t *testing.T) {
 	R := newResponse
 	t.Run("error", func(t *testing.T) {
-		err := R(nil, "", errors.New("bonk")).noData().End()
+		err := R(nil, "", errors.New("bonk")).NoData().End()
 		assert.Error(t, err)
 		assert.Equal(t, "bonk", err.Error())
 	})
 	t.Run("statusSet", func(t *testing.T) {
-		err := R(nil, "unexpected!", nil).noData().End()
+		err := R(nil, "unexpected!", nil).NoData().End()
 		assert.Error(t, err)
 	})
 	t.Run("someJSON", func(t *testing.T) {
-		err := R([]byte(`{"trouble": true}`), "", nil).noData().End()
+		err := R([]byte(`{"trouble": true}`), "", nil).NoData().End()
 		assert.Error(t, err)
 	})
 	t.Run("ok", func(t *testing.T) {
-		err := R([]byte{}, "", nil).noData().End()
+		err := R([]byte{}, "", nil).NoData().End()
 		assert.NoError(t, err)
 	})
 }
diff --git a/cephfs/admin/volume_test.go b/cephfs/admin/volume_test.go
index 3e1edf9..2c6dc30 100644
--- a/cephfs/admin/volume_test.go
+++ b/cephfs/admin/volume_test.go
@@ -310,20 +310,24 @@ var sampleFsLs2 = []byte(`
 
 func TestParseFsList(t *testing.T) {
 	t.Run("error", func(t *testing.T) {
-		_, err := parseFsList(response{err: errors.New("eek")})
+		_, err := parseFsList(
+			newResponse(nil, "", errors.New("eek")))
 		assert.Error(t, err)
 		assert.Equal(t, "eek", err.Error())
 	})
 	t.Run("statusSet", func(t *testing.T) {
-		_, err := parseFsList(response{status: "oof"})
+		_, err := parseFsList(
+			newResponse(nil, "oof", nil))
 		assert.Error(t, err)
 	})
 	t.Run("badJSON", func(t *testing.T) {
-		_, err := parseFsList(response{body: []byte("______")})
+		_, err := parseFsList(
+			newResponse([]byte("______"), "", nil))
 		assert.Error(t, err)
 	})
 	t.Run("ok1", func(t *testing.T) {
-		l, err := parseFsList(response{body: sampleFsLs1})
+		l, err := parseFsList(
+			newResponse(sampleFsLs1, "", nil))
 		assert.NoError(t, err)
 		if assert.NotNil(t, l) && assert.Len(t, l, 1) {
 			fs := l[0]
@@ -337,7 +341,8 @@ func TestParseFsList(t *testing.T) {
 		}
 	})
 	t.Run("ok2", func(t *testing.T) {
-		l, err := parseFsList(response{body: sampleFsLs2})
+		l, err := parseFsList(
+			newResponse(sampleFsLs2, "", nil))
 		assert.NoError(t, err)
 		if assert.NotNil(t, l) && assert.Len(t, l, 2) {
 			fs := l[0]
diff --git a/cephfs/admin/response_test.go b/internal/commands/response_test.go
similarity index 89%
rename from cephfs/admin/response_test.go
rename to internal/commands/response_test.go
index ee00541..00baf20 100644
--- a/cephfs/admin/response_test.go
+++ b/internal/commands/response_test.go
@@ -1,6 +1,4 @@
-// +build !luminous,!mimic
-
-package admin
+package commands
 
 import (
 	"errors"
@@ -62,10 +60,10 @@ func TestResponse(t *testing.T) {
 	})
 
 	t.Run("noStatus", func(t *testing.T) {
-		assert.EqualValues(t, r1, r1.noStatus())
-		assert.EqualValues(t, r2, r2.noStatus())
+		assert.EqualValues(t, r1, r1.NoStatus())
+		assert.EqualValues(t, r2, r2.NoStatus())
 
-		x := r3.noStatus()
+		x := r3.NoStatus()
 		assert.EqualValues(t, ErrStatusNotEmpty, x.Unwrap())
 		assert.EqualValues(t, r3.Status(), x.Status())
 	})
@@ -82,16 +80,16 @@ func TestResponse(t *testing.T) {
 	})
 
 	t.Run("noData", func(t *testing.T) {
-		x := r1.noData()
+		x := r1.NoData()
 		assert.EqualValues(t, ErrBodyNotEmpty, x.Unwrap())
 		assert.EqualValues(t, r1.Status(), x.Status())
 
-		x = r3.noStatus()
+		x = r3.NoStatus()
 		assert.EqualValues(t, ErrStatusNotEmpty, x.Unwrap())
 		assert.EqualValues(t, r3.Status(), x.Status())
 
 		rtemp := response{}
-		assert.EqualValues(t, rtemp, rtemp.noData())
+		assert.EqualValues(t, rtemp, rtemp.NoData())
 	})
 
 	t.Run("filterDeprecated", func(t *testing.T) {
@@ -109,19 +107,19 @@ func TestResponse(t *testing.T) {
 
 	t.Run("unmarshal", func(t *testing.T) {
 		var v map[string]interface{}
-		assert.EqualValues(t, r1, r1.unmarshal(&v))
+		assert.EqualValues(t, r1, r1.Unmarshal(&v))
 		assert.EqualValues(t, "bar", v["foo"])
 
-		assert.EqualValues(t, r2, r2.unmarshal(&v))
+		assert.EqualValues(t, r2, r2.Unmarshal(&v))
 
 		rtemp := response{body: []byte("foo!")}
-		x := rtemp.unmarshal(&v)
+		x := rtemp.Unmarshal(&v)
 		assert.False(t, x.Ok())
 		assert.Contains(t, x.Error(), "invalid character")
 	})
 
 	t.Run("newResponse", func(t *testing.T) {
-		rtemp := newResponse(nil, "x", e2)
+		rtemp := commands.NewResponse(nil, "x", e2)
 		assert.False(t, rtemp.Ok())
 		assert.Equal(t, "x", rtemp.Status())
 	})