From c8af2b638a9e10ba5bc8c74d190290342af77d4a Mon Sep 17 00:00:00 2001 From: Mike Perez Date: Tue, 7 May 2024 15:51:05 -0700 Subject: [PATCH] util: Removing JoinError in favor of fmt.Errorf Signed-off-by: Mike Perez --- internal/cephfs/core/volume.go | 6 +-- internal/cephfs/store/volumeoptions.go | 2 +- internal/journal/omap.go | 2 +- internal/rbd/rbd_util.go | 8 +-- internal/util/cephcmds.go | 8 +-- internal/util/connection.go | 2 +- internal/util/errors.go | 26 ---------- internal/util/errors_test.go | 71 -------------------------- 8 files changed, 14 insertions(+), 111 deletions(-) delete mode 100644 internal/util/errors_test.go diff --git a/internal/cephfs/core/volume.go b/internal/cephfs/core/volume.go index 3e4a9c4892c..42aa6b01fbd 100644 --- a/internal/cephfs/core/volume.go +++ b/internal/cephfs/core/volume.go @@ -140,7 +140,7 @@ func (s *subVolumeClient) GetVolumeRootPathCeph(ctx context.Context) (string, er if err != nil { log.ErrorLog(ctx, "failed to get the rootpath for the vol %s: %s", s.VolID, err) if errors.Is(err, rados.ErrNotFound) { - return "", util.JoinErrors(cerrors.ErrVolumeNotFound, err) + return "", fmt.Errorf("Failed as %w (internal %w)", cerrors.ErrVolumeNotFound, err) } return "", err @@ -303,10 +303,10 @@ func (s *subVolumeClient) PurgeVolume(ctx context.Context, force bool) error { if err != nil { log.ErrorLog(ctx, "failed to purge subvolume %s in fs %s: %s", s.VolID, s.FsName, err) if strings.Contains(err.Error(), cerrors.VolumeNotEmpty) { - return util.JoinErrors(cerrors.ErrVolumeHasSnapshots, err) + return fmt.Errorf("Failed as %w (internal %w)", cerrors.ErrVolumeHasSnapshots, err) } if errors.Is(err, rados.ErrNotFound) { - return util.JoinErrors(cerrors.ErrVolumeNotFound, err) + return fmt.Errorf("Failed as %w (internal %w)", cerrors.ErrVolumeNotFound, err) } return err diff --git a/internal/cephfs/store/volumeoptions.go b/internal/cephfs/store/volumeoptions.go index 6ec246513c1..396de7f5f7e 100644 --- a/internal/cephfs/store/volumeoptions.go +++ b/internal/cephfs/store/volumeoptions.go @@ -390,7 +390,7 @@ func NewVolumeOptionsFromVolID( if err != nil { err = fmt.Errorf("error decoding volume ID (%s): %w", volID, err) - return nil, nil, util.JoinErrors(cerrors.ErrInvalidVolID, err) + return nil, nil, fmt.Errorf("Failed as %w (internal %w)", cerrors.ErrInvalidVolID, err) } volOptions.ClusterID = vi.ClusterID vid.VolumeID = volID diff --git a/internal/journal/omap.go b/internal/journal/omap.go index c62ca51dee5..c25d62b3daf 100644 --- a/internal/journal/omap.go +++ b/internal/journal/omap.go @@ -164,7 +164,7 @@ func setOMapKeys( func omapPoolError(err error) error { if errors.Is(err, rados.ErrNotFound) { - return util.JoinErrors(util.ErrPoolNotFound, err) + return fmt.Errorf("Failed as %w (internal %w)", util.ErrPoolNotFound, err) } return err diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index 7b37a70eb61..ee955d7458b 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -510,7 +510,7 @@ func (ri *rbdImage) open() (*librbd.Image, error) { image, err := librbd.OpenImage(ri.ioctx, ri.RbdImageName, librbd.NoSnapshot) if err != nil { if errors.Is(err, librbd.ErrNotFound) { - err = util.JoinErrors(ErrImageNotFound, err) + err = fmt.Errorf("Failed as %w (internal %w)", ErrImageNotFound, err) } return nil, err @@ -896,7 +896,7 @@ func (ri *rbdImage) flatten() error { // rbd image flatten will fail if the rbd image does not have a parent parent, pErr := ri.getParentName() if pErr != nil { - return util.JoinErrors(err, pErr) + return fmt.Errorf("Failed as %w (internal %w)", err, pErr) } if parent == "" { return nil @@ -1448,7 +1448,7 @@ func (ri *rbdImage) deleteSnapshot(ctx context.Context, pOpts *rbdSnapshot) erro } err = snap.Remove() if errors.Is(err, librbd.ErrNotFound) { - return util.JoinErrors(ErrSnapNotFound, err) + return fmt.Errorf("Failed as %w (internal %w)", ErrSnapNotFound, err) } return err @@ -1779,7 +1779,7 @@ func lookupRBDImageMetadataStash(metaDataPath string) (rbdImageMetadataStash, er return imgMeta, fmt.Errorf("failed to read stashed JSON image metadata from path (%s): %w", fPath, err) } - return imgMeta, util.JoinErrors(ErrMissingStash, err) + return imgMeta, fmt.Errorf("Failed as %w (internal %w)", ErrMissingStash, err) } err = json.Unmarshal(encodedBytes, &imgMeta) diff --git a/internal/util/cephcmds.go b/internal/util/cephcmds.go index d8647c13fc1..8f3b58d40c2 100644 --- a/internal/util/cephcmds.go +++ b/internal/util/cephcmds.go @@ -235,7 +235,7 @@ func CreateObject(ctx context.Context, monitors string, cr *Credentials, poolNam ioctx, err := conn.GetIoctx(poolName) if err != nil { if errors.Is(err, ErrPoolNotFound) { - err = JoinErrors(ErrObjectNotFound, err) + err = fmt.Errorf("Failed as %w (internal %w)", ErrObjectNotFound, err) } return err @@ -248,7 +248,7 @@ func CreateObject(ctx context.Context, monitors string, cr *Credentials, poolNam err = ioctx.Create(objectName, rados.CreateExclusive) if errors.Is(err, rados.ErrObjectExists) { - return JoinErrors(ErrObjectExists, err) + return fmt.Errorf("Failed as %w (internal %w)", ErrObjectExists, err) } else if err != nil { log.ErrorLog(ctx, "failed creating omap (%s) in pool (%s): (%v)", objectName, poolName, err) @@ -271,7 +271,7 @@ func RemoveObject(ctx context.Context, monitors string, cr *Credentials, poolNam ioctx, err := conn.GetIoctx(poolName) if err != nil { if errors.Is(err, ErrPoolNotFound) { - err = JoinErrors(ErrObjectNotFound, err) + err = fmt.Errorf("Failed as %w (internal %w)", ErrObjectNotFound, err) } return err @@ -284,7 +284,7 @@ func RemoveObject(ctx context.Context, monitors string, cr *Credentials, poolNam err = ioctx.Delete(oMapName) if errors.Is(err, rados.ErrNotFound) { - return JoinErrors(ErrObjectNotFound, err) + return fmt.Errorf("Failed as %w (internal %w)", ErrObjectNotFound, err) } else if err != nil { log.ErrorLog(ctx, "failed removing omap (%s) in pool (%s): (%v)", oMapName, poolName, err) diff --git a/internal/util/connection.go b/internal/util/connection.go index 48fba7ef231..a052ddca472 100644 --- a/internal/util/connection.go +++ b/internal/util/connection.go @@ -96,7 +96,7 @@ func (cc *ClusterConnection) GetIoctx(pool string) (*rados.IOContext, error) { if err != nil { // ErrNotFound indicates the Pool was not found if errors.Is(err, rados.ErrNotFound) { - err = JoinErrors(ErrPoolNotFound, err) + err = fmt.Errorf("Failed as %w (internal %w)", ErrPoolNotFound, err) } else { err = fmt.Errorf("failed to open IOContext for pool %s: %w", pool, err) } diff --git a/internal/util/errors.go b/internal/util/errors.go index 795e6630bb6..118ed15e3c7 100644 --- a/internal/util/errors.go +++ b/internal/util/errors.go @@ -18,7 +18,6 @@ package util import ( "errors" - "fmt" ) var ( @@ -38,28 +37,3 @@ var ( // ErrMissingConfigForMonitor is returned when clusterID is not found for the mon. ErrMissingConfigForMonitor = errors.New("missing configuration of cluster ID for monitor") ) - -type pairError struct { - first error - second error -} - -func (e pairError) Error() string { - return fmt.Sprintf("%v: %v", e.first, e.second) -} - -// Is checks if target error is wrapped in the first error. -func (e pairError) Is(target error) bool { - return errors.Is(e.first, target) -} - -// Unwrap returns the second error. -func (e pairError) Unwrap() error { - return e.second -} - -// JoinErrors combines two errors. Of the returned error, Is() follows the first -// branch, Unwrap() follows the second branch. -func JoinErrors(e1, e2 error) error { - return pairError{e1, e2} -} diff --git a/internal/util/errors_test.go b/internal/util/errors_test.go deleted file mode 100644 index 941d34f9d92..00000000000 --- a/internal/util/errors_test.go +++ /dev/null @@ -1,71 +0,0 @@ -/* -Copyright 2020 ceph-csi authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package util - -import ( - "errors" - "fmt" - "testing" -) - -var ( - errFoo = errors.New("foo") - errBar = errors.New("bar") -) - -func wrapError(e error) error { - return fmt.Errorf("w{%w}", e) -} - -func TestJoinErrors(t *testing.T) { - t.Parallel() - assertErrorIs := func(e1, e2 error, ok bool) { - if errors.Is(e1, e2) != ok { - t.Errorf("errors.Is(e1, e2) != %v - e1: %#v - e2: %#v", ok, e1, e2) - } - } - - assertErrorIs(errFoo, errBar, false) - assertErrorIs(errFoo, errFoo, true) - - fooBar := JoinErrors(errFoo, errBar) - assertErrorIs(fooBar, errFoo, true) - assertErrorIs(fooBar, errBar, true) - - w2Foo := wrapError(wrapError(errFoo)) - w1Bar := wrapError(errBar) - w1w2Foow1Bar := wrapError(JoinErrors(w2Foo, w1Bar)) - assertErrorIs(w1w2Foow1Bar, errFoo, true) - assertErrorIs(w1w2Foow1Bar, errBar, true) - - w2X := wrapError(wrapError(errors.New("x"))) - w2FooBar := wrapError(wrapError(fooBar)) - w1w2Xw2FooBar := wrapError(JoinErrors(w2X, w2FooBar)) - assertErrorIs(w1w2Xw2FooBar, errFoo, true) - assertErrorIs(w1w2Xw2FooBar, errBar, true) - - x := errors.Unwrap(errors.Unwrap(errors.Unwrap(errors.Unwrap(w1w2Xw2FooBar)))) - assertErrorIs(x, fooBar, true) - x = errors.Unwrap(x) - assertErrorIs(x, fooBar, false) - assertErrorIs(x, errFoo, false) - assertErrorIs(x, errBar, true) - s1 := "w{w{w{x}}: w{w{foo: bar}}}" - if s2 := w1w2Xw2FooBar.Error(); s1 != s2 { - t.Errorf("%s != %s", s1, s2) - } -}