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..7912a2d 100644 --- a/cephfs/admin/fsadmin.go +++ b/cephfs/admin/fsadmin.go @@ -101,7 +101,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 +114,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 response = commands.Response -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 -} - -// 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/response_test.go b/cephfs/admin/response_test.go deleted file mode 100644 index ee00541..0000000 --- a/cephfs/admin/response_test.go +++ /dev/null @@ -1,152 +0,0 @@ -// +build !luminous,!mimic - -package admin - -import ( - "errors" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestResponse(t *testing.T) { - e1 := errors.New("error one") - e2 := errors.New("error two") - r1 := response{ - body: []byte(`{"foo": "bar", "baz": 1}`), - } - r2 := response{ - status: "System notice: disabled for maintenance", - err: e1, - } - r3 := response{ - body: []byte(`{"oof": "RAB", "baz": 8}`), - status: "reversed polarity detected", - } - r4 := response{ - body: []byte(`{"whoops": true, "state": "total protonic reversal"}`), - status: "", - err: e2, - } - - t.Run("ok", func(t *testing.T) { - assert.True(t, r1.Ok()) - assert.False(t, r2.Ok()) - assert.True(t, r3.Ok()) - }) - - t.Run("error", func(t *testing.T) { - assert.Equal(t, - "error one: \"System notice: disabled for maintenance\"", - r2.Error()) - assert.Equal(t, - e2.Error(), - r4.Error()) - }) - - t.Run("unwrap", func(t *testing.T) { - assert.Equal(t, e1, r2.Unwrap()) - assert.Equal(t, e2, r4.Unwrap()) - }) - - t.Run("status", func(t *testing.T) { - assert.Equal(t, "", r1.Status()) - assert.Equal(t, "System notice: disabled for maintenance", r2.Status()) - assert.Equal(t, "reversed polarity detected", r3.Status()) - }) - - t.Run("end", func(t *testing.T) { - assert.Nil(t, r1.End()) - assert.NotNil(t, r2.End()) - assert.EqualValues(t, r2, r2.End()) - }) - - t.Run("noStatus", func(t *testing.T) { - assert.EqualValues(t, r1, r1.noStatus()) - assert.EqualValues(t, r2, r2.noStatus()) - - x := r3.noStatus() - assert.EqualValues(t, ErrStatusNotEmpty, x.Unwrap()) - assert.EqualValues(t, r3.Status(), x.Status()) - }) - - t.Run("noBody", func(t *testing.T) { - x := r1.noBody() - assert.EqualValues(t, ErrBodyNotEmpty, x.Unwrap()) - assert.EqualValues(t, r1.Status(), x.Status()) - - assert.EqualValues(t, r2, r2.noBody()) - - rtemp := response{} - assert.EqualValues(t, rtemp, rtemp.noBody()) - }) - - t.Run("noData", func(t *testing.T) { - x := r1.noData() - assert.EqualValues(t, ErrBodyNotEmpty, x.Unwrap()) - assert.EqualValues(t, r1.Status(), x.Status()) - - x = r3.noStatus() - assert.EqualValues(t, ErrStatusNotEmpty, x.Unwrap()) - assert.EqualValues(t, r3.Status(), x.Status()) - - rtemp := response{} - assert.EqualValues(t, rtemp, rtemp.noData()) - }) - - t.Run("filterDeprecated", func(t *testing.T) { - assert.EqualValues(t, r1, r1.filterDeprecated()) - assert.EqualValues(t, r2, r2.filterDeprecated()) - - rtemp := response{ - status: "blorple call is deprecated and will be removed in a future release", - } - x := rtemp.filterDeprecated() - assert.True(t, x.Ok()) - assert.Nil(t, x.End()) - assert.Equal(t, "", x.Status()) - }) - - t.Run("unmarshal", func(t *testing.T) { - var v map[string]interface{} - assert.EqualValues(t, r1, r1.unmarshal(&v)) - assert.EqualValues(t, "bar", v["foo"]) - - assert.EqualValues(t, r2, r2.unmarshal(&v)) - - rtemp := response{body: []byte("foo!")} - 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) - assert.False(t, rtemp.Ok()) - assert.Equal(t, "x", rtemp.Status()) - }) - - t.Run("notImplemented", func(t *testing.T) { - rtemp := response{ - status: "No handler found for this function", - err: myCephError(-22), - } - if assert.False(t, rtemp.Ok()) { - err := rtemp.End() - assert.Error(t, err) - var n NotImplementedError - assert.True(t, errors.As(err, &n)) - assert.Contains(t, err.Error(), "not implemented") - } - }) -} - -type myCephError int - -func (myCephError) Error() string { - return "oops" -} - -func (e myCephError) ErrorCode() int { - return int(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/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]