From 6eb101af06983191a33fa093e944e217ec0c8231 Mon Sep 17 00:00:00 2001 From: Stephen Kitt Date: Mon, 16 Oct 2023 12:52:33 +0200 Subject: [PATCH] Drop github.com/pkg/errors The package is unmaintained. Apart from the wrapper functions, its features are provided by built-in errors and the "errors" package. Most of the changes are straightforward; it might be worth discussing whether to leave the conditional wrappers as-is (or even add a utility function for them). Signed-off-by: Stephen Kitt --- go.mod | 2 +- pkg/fake/conflict_reactor.go | 2 +- pkg/fake/delete_colection_reactor.go | 2 +- pkg/fake/dynamic_client.go | 2 +- pkg/fake/fail_on_action_reactor.go | 2 +- pkg/fake/filtering_watch_reactor.go | 3 ++- pkg/finalizer/finalizer.go | 12 +++++++--- pkg/reporter/adapter.go | 5 ++-- pkg/resource/rest.go | 8 +++---- pkg/resource/rest_test.go | 2 +- pkg/resource/util.go | 4 ++-- pkg/syncer/broker/config.go | 3 +-- pkg/syncer/broker/syncer.go | 21 ++++++++++------ pkg/syncer/resource_syncer.go | 20 +++++++++++----- pkg/util/create_or_update.go | 36 +++++++++++++++++++--------- pkg/util/create_or_update_test.go | 2 +- pkg/util/helpers.go | 7 +++--- pkg/watcher/resource_watcher.go | 7 +++--- 18 files changed, 86 insertions(+), 54 deletions(-) diff --git a/go.mod b/go.mod index 0b861dc9..4ba3e063 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,6 @@ require ( github.com/kelseyhightower/envconfig v1.4.0 github.com/onsi/ginkgo/v2 v2.12.0 github.com/onsi/gomega v1.27.10 - github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.17.0 github.com/rs/zerolog v1.31.0 github.com/submariner-io/shipyard v0.16.0-m4 @@ -53,6 +52,7 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/pkg/errors v0.9.1 // indirect github.com/prometheus/client_model v0.4.1-0.20230718164431-9a2bf3000d16 // indirect github.com/prometheus/common v0.44.0 // indirect github.com/prometheus/procfs v0.11.1 // indirect diff --git a/pkg/fake/conflict_reactor.go b/pkg/fake/conflict_reactor.go index c4c278dd..3c9de151 100644 --- a/pkg/fake/conflict_reactor.go +++ b/pkg/fake/conflict_reactor.go @@ -19,9 +19,9 @@ limitations under the License. package fake import ( + "errors" "sync" - "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" diff --git a/pkg/fake/delete_colection_reactor.go b/pkg/fake/delete_colection_reactor.go index 8acdad1f..64c1308e 100644 --- a/pkg/fake/delete_colection_reactor.go +++ b/pkg/fake/delete_colection_reactor.go @@ -19,12 +19,12 @@ limitations under the License. package fake import ( + "errors" "fmt" "strings" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "github.com/pkg/errors" "github.com/submariner-io/admiral/pkg/syncer/test" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/fake/dynamic_client.go b/pkg/fake/dynamic_client.go index 565235a2..0adba847 100644 --- a/pkg/fake/dynamic_client.go +++ b/pkg/fake/dynamic_client.go @@ -20,6 +20,7 @@ package fake import ( "context" + "errors" "fmt" "strconv" "sync" @@ -27,7 +28,6 @@ import ( "time" . "github.com/onsi/gomega" - "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" diff --git a/pkg/fake/fail_on_action_reactor.go b/pkg/fake/fail_on_action_reactor.go index fd404839..1b6f928f 100644 --- a/pkg/fake/fail_on_action_reactor.go +++ b/pkg/fake/fail_on_action_reactor.go @@ -19,9 +19,9 @@ limitations under the License. package fake import ( + "errors" "sync/atomic" - "github.com/pkg/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/testing" ) diff --git a/pkg/fake/filtering_watch_reactor.go b/pkg/fake/filtering_watch_reactor.go index d3fc3346..30096670 100644 --- a/pkg/fake/filtering_watch_reactor.go +++ b/pkg/fake/filtering_watch_reactor.go @@ -19,7 +19,8 @@ limitations under the License. package fake import ( - "github.com/pkg/errors" + "errors" + "github.com/submariner-io/admiral/pkg/resource" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/watch" diff --git a/pkg/finalizer/finalizer.go b/pkg/finalizer/finalizer.go index 650c1aa0..1d970bde 100644 --- a/pkg/finalizer/finalizer.go +++ b/pkg/finalizer/finalizer.go @@ -20,8 +20,8 @@ package finalizer import ( "context" + "fmt" - "github.com/pkg/errors" "github.com/submariner-io/admiral/pkg/resource" "github.com/submariner-io/admiral/pkg/util" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -44,8 +44,11 @@ func Add[T runtime.Object](ctx context.Context, client resource.Interface[T], ob return existing, nil }) + if err != nil { + return false, fmt.Errorf("error adding finalizer %q to %q: %w", finalizerName, objMeta.GetName(), err) + } - return err == nil, errors.Wrapf(err, "error adding finalizer %q to %q", finalizerName, objMeta.GetName()) + return true, nil } func Remove[T runtime.Object](ctx context.Context, client resource.Interface[T], obj T, finalizerName string) error { @@ -70,8 +73,11 @@ func Remove[T runtime.Object](ctx context.Context, client resource.Interface[T], return existing, nil }) + if err != nil { + return fmt.Errorf("error removing finalizer %q from %q: %w", finalizerName, objMeta.GetName(), err) + } - return errors.Wrapf(err, "error removing finalizer %q from %q", finalizerName, objMeta.GetName()) + return nil } func IsPresent(objMeta metav1.Object, finalizerName string) bool { diff --git a/pkg/reporter/adapter.go b/pkg/reporter/adapter.go index dcc241a9..87824cbd 100644 --- a/pkg/reporter/adapter.go +++ b/pkg/reporter/adapter.go @@ -19,9 +19,8 @@ limitations under the License. package reporter import ( + "fmt" "unicode" - - "github.com/pkg/errors" ) type Adapter struct { @@ -34,7 +33,7 @@ func (a *Adapter) Error(err error, message string, args ...interface{}) error { } if message != "" { - err = errors.Wrapf(err, message, args...) + err = fmt.Errorf("%s: %w", fmt.Sprintf(message, args...), err) } capitalizeFirst := func(str string) string { diff --git a/pkg/resource/rest.go b/pkg/resource/rest.go index 87739a0f..09745fac 100644 --- a/pkg/resource/rest.go +++ b/pkg/resource/rest.go @@ -22,9 +22,9 @@ import ( "context" "crypto/x509" "encoding/base64" + "errors" "fmt" - "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -84,7 +84,7 @@ func BuildRestConfigFromData(apiServer, apiServerToken, caData string, tls *rest if !tls.Insecure && caData != "" { caDecoded, err := base64.StdEncoding.DecodeString(caData) if err != nil { - return nil, errors.Wrap(err, "error decoding CA data") + return nil, fmt.Errorf("error decoding CA data: %w", err) } tls.CAData = caDecoded @@ -116,12 +116,12 @@ func BuildRestConfigFromFiles(apiServer, apiServerTokenFile, caFile string, tls func IsAuthorizedFor(restConfig *rest.Config, gvr schema.GroupVersionResource, namespace string) (bool, error) { client, err := NewDynamicClient(restConfig) if err != nil { - return false, errors.Wrap(err, "error creating dynamic client") + return false, fmt.Errorf("error creating dynamic client: %w", err) } _, err = client.Resource(gvr).Namespace(namespace).Get(context.TODO(), "any", metav1.GetOptions{}) if IsUnknownAuthorityError(err) { - return false, errors.Wrapf(err, "cannot access the API server %q", restConfig.Host) + return false, fmt.Errorf("cannot access the API server %q: %w", restConfig.Host, err) } if apierrors.IsNotFound(err) { diff --git a/pkg/resource/rest_test.go b/pkg/resource/rest_test.go index 6249b7db..019c41f8 100644 --- a/pkg/resource/rest_test.go +++ b/pkg/resource/rest_test.go @@ -21,10 +21,10 @@ package resource_test import ( "crypto/x509" "encoding/base64" + "errors" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "github.com/pkg/errors" "github.com/submariner-io/admiral/pkg/fake" "github.com/submariner-io/admiral/pkg/resource" corev1 "k8s.io/api/core/v1" diff --git a/pkg/resource/util.go b/pkg/resource/util.go index 6eecc82b..ca486686 100644 --- a/pkg/resource/util.go +++ b/pkg/resource/util.go @@ -20,10 +20,10 @@ package resource import ( "encoding/json" + "fmt" "strings" "unicode" - "github.com/pkg/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -43,7 +43,7 @@ func ToUnstructuredUsingScheme(from runtime.Object, usingScheme *runtime.Scheme) to := &unstructured.Unstructured{} err := usingScheme.Convert(from, to, nil) if err != nil { - return nil, errors.Wrapf(err, "error converting %#v to unstructured.Unstructured", from) + return nil, fmt.Errorf("error converting %#v to unstructured.Unstructured: %w", from, err) } return to, nil diff --git a/pkg/syncer/broker/config.go b/pkg/syncer/broker/config.go index 966db614..e6815d45 100644 --- a/pkg/syncer/broker/config.go +++ b/pkg/syncer/broker/config.go @@ -24,7 +24,6 @@ import ( "strings" "github.com/kelseyhightower/envconfig" - "github.com/pkg/errors" ) type brokerSpecification struct { @@ -43,7 +42,7 @@ func getBrokerSpecification() (*brokerSpecification, error) { err := envconfig.Process(brokerConfigPrefix, &brokerSpec) if err != nil { - return nil, errors.Wrap(err, "error processing env configuration") + return nil, fmt.Errorf("error processing env configuration: %w", err) } return &brokerSpec, nil diff --git a/pkg/syncer/broker/syncer.go b/pkg/syncer/broker/syncer.go index 5ba930bc..2bd968f3 100644 --- a/pkg/syncer/broker/syncer.go +++ b/pkg/syncer/broker/syncer.go @@ -24,7 +24,6 @@ import ( "reflect" "time" - "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "github.com/submariner-io/admiral/pkg/federate" "github.com/submariner-io/admiral/pkg/log" @@ -159,14 +158,14 @@ func NewSyncer(config SyncerConfig) (*Syncer, error) { //nolint:gocritic // Mini if config.RestMapper == nil { config.RestMapper, err = util.BuildRestMapper(config.LocalRestConfig) if err != nil { - return nil, errors.Wrap(err, "error building the REST mapper") + return nil, fmt.Errorf("error building the REST mapper: %w", err) } } if config.LocalClient == nil { config.LocalClient, err = resource.NewDynamicClient(config.LocalRestConfig) if err != nil { - return nil, errors.Wrap(err, "error creating dynamic client") + return nil, fmt.Errorf("error creating dynamic client: %w", err) } } @@ -228,7 +227,7 @@ func NewSyncer(config SyncerConfig) (*Syncer, error) { //nolint:gocritic // Mini SyncCounter: syncCounter, }) if err != nil { - return nil, errors.Wrap(err, "error creating local resource syncer") + return nil, fmt.Errorf("error creating local resource syncer: %w", err) } brokerSyncer.syncers = append(brokerSyncer.syncers, localSyncer) @@ -256,7 +255,7 @@ func NewSyncer(config SyncerConfig) (*Syncer, error) { //nolint:gocritic // Mini SyncCounter: syncCounter, }) if err != nil { - return nil, errors.Wrap(err, "error creating remote resource syncer") + return nil, fmt.Errorf("error creating remote resource syncer: %w", err) } brokerSyncer.syncers = append(brokerSyncer.syncers, remoteSyncer) @@ -303,7 +302,11 @@ func createBrokerClient(config *SyncerConfig) error { } if !authorized { - return errors.Wrap(err, "error authorizing access to the broker API server") + if err != nil { + return fmt.Errorf("error authorizing access to the broker API server: %w", err) + } + + return nil } if err != nil { @@ -312,7 +315,11 @@ func createBrokerClient(config *SyncerConfig) error { config.BrokerClient, err = resource.NewDynamicClient(config.BrokerRestConfig) - return errors.Wrap(err, "error creating dynamic client") + if err != nil { + return fmt.Errorf("error creating dynamic client: %w", err) + } + + return nil } func (s *Syncer) Start(stopCh <-chan struct{}) error { diff --git a/pkg/syncer/resource_syncer.go b/pkg/syncer/resource_syncer.go index a5c79569..42a44298 100644 --- a/pkg/syncer/resource_syncer.go +++ b/pkg/syncer/resource_syncer.go @@ -25,7 +25,6 @@ import ( "sync" "time" - "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "github.com/submariner-io/admiral/pkg/federate" "github.com/submariner-io/admiral/pkg/log" @@ -303,7 +302,7 @@ func (r *resourceSyncer) AwaitStopped() { func (r *resourceSyncer) GetResource(name, namespace string) (runtime.Object, bool, error) { obj, exists, err := r.store.GetByKey(namespace + "/" + name) if err != nil { - return nil, false, errors.Wrap(err, "error retrieving resource") + return nil, false, fmt.Errorf("error retrieving resource: %w", err) } if !exists { @@ -428,7 +427,7 @@ func (r *resourceSyncer) runIfCacheSynced(defaultReturn any, run func() any) any func (r *resourceSyncer) processNextWorkItem(key, _, _ string) (bool, error) { obj, exists, err := r.store.GetByKey(key) if err != nil { - return true, errors.Wrapf(err, "error retrieving resource %q", key) + return true, fmt.Errorf("error retrieving resource %q: %w", key, err) } if !exists { @@ -461,8 +460,12 @@ func (r *resourceSyncer) processNextWorkItem(key, _, _ string) (bool, error) { r.log.V(log.LIBDEBUG).Infof("Syncer %q syncing resource %q", r.config.Name, resource.GetName()) err = r.config.Federator.Distribute(resource) - if err != nil || r.onSuccessfulSync(resource, transformed, op) { - return true, errors.Wrapf(err, "error distributing resource %q", key) + if err != nil { + return true, fmt.Errorf("error distributing resource %q: %w", key, err) + } + + if r.onSuccessfulSync(resource, transformed, op) { + return true, nil } if r.syncCounter != nil { @@ -515,7 +518,12 @@ func (r *resourceSyncer) handleDeleted(key string) (bool, error) { if err != nil || r.onSuccessfulSync(resource, transformed, Delete) { r.deleted.Store(key, deletedResource) - return true, errors.Wrapf(err, "error deleting resource %q", key) + + if err != nil { + return true, fmt.Errorf("error deleting resource %q: %w", key, err) + } + + return true, nil } if deleted { diff --git a/pkg/util/create_or_update.go b/pkg/util/create_or_update.go index 4beac538..da540da2 100644 --- a/pkg/util/create_or_update.go +++ b/pkg/util/create_or_update.go @@ -23,7 +23,6 @@ import ( "fmt" "time" - "github.com/pkg/errors" "github.com/submariner-io/admiral/pkg/log" "github.com/submariner-io/admiral/pkg/resource" "k8s.io/apimachinery/pkg/api/equality" @@ -113,7 +112,7 @@ func maybeCreateOrUpdate[T runtime.Object](ctx context.Context, client resource. } if err != nil { - return errors.Wrapf(err, "error retrieving %q", objMeta.GetName()) + return fmt.Errorf("error retrieving %q: %w", objMeta.GetName(), err) } origObj := resource.MustToUnstructuredUsingDefaultConverter(existing) @@ -149,7 +148,7 @@ func maybeCreateOrUpdate[T runtime.Object](ctx context.Context, client resource. unstructured.RemoveNestedField(newObj.Object, StatusField) resource.MustToMeta(toUpdate).SetResourceVersion(resource.MustToMeta(updated).GetResourceVersion()) } else if !apierrors.IsNotFound(err) { - return errors.Wrapf(err, "error updating status %s", resource.ToJSON(toUpdate)) + return fmt.Errorf("error updating status %s: %w", resource.ToJSON(toUpdate), err) } } @@ -162,10 +161,14 @@ func maybeCreateOrUpdate[T runtime.Object](ctx context.Context, client resource. result = OperationResultUpdated _, err = client.Update(ctx, toUpdate, metav1.UpdateOptions{}) - return errors.Wrapf(err, "error updating %s", resource.ToJSON(toUpdate)) + if err != nil { + return fmt.Errorf("error updating %s: %w", resource.ToJSON(toUpdate), err) + } + + return nil }) if err != nil { - return OperationResultNone, errors.Wrap(err, "error creating or updating resource") + return OperationResultNone, fmt.Errorf("error creating or updating resource: %w", err) } return result, nil @@ -209,7 +212,7 @@ func createResource[T runtime.Object](ctx context.Context, client resource.Inter } if err != nil { - return errors.Wrapf(err, "error creating %#v", obj) + return fmt.Errorf("error creating %#v: %w", obj, err) } status, ok := GetNestedField(resource.MustToUnstructuredUsingDefaultConverter(obj), StatusField).(map[string]interface{}) @@ -221,7 +224,7 @@ func createResource[T runtime.Object](ctx context.Context, client resource.Inter _, err := client.UpdateStatus(ctx, obj, metav1.UpdateOptions{}) if err != nil && !apierrors.IsNotFound(err) { - return errors.Wrapf(err, "error updating status for %#v", obj) + return fmt.Errorf("error updating status for %#v: %w", obj, err) } } @@ -246,13 +249,17 @@ func CreateAnew[T runtime.Object](ctx context.Context, client resource.Interface retObj, err = client.Create(ctx, obj, createOptions) if !apierrors.IsAlreadyExists(err) { - return true, errors.Wrapf(err, "error creating %#v", obj) + if err != nil { + return true, fmt.Errorf("error creating %#v: %w", obj, err) + } + + return true, nil } retObj, err = client.Get(ctx, resource.MustToMeta(obj).GetName(), metav1.GetOptions{}) if !apierrors.IsNotFound(err) { if err != nil { - return false, errors.Wrapf(err, "failed to retrieve pre-existing instance %q", name) + return false, fmt.Errorf("failed to retrieve pre-existing instance %q: %w", name, err) } if mutableFieldsEqual(retObj, obj) { @@ -265,10 +272,17 @@ func CreateAnew[T runtime.Object](ctx context.Context, client resource.Interface err = nil } - return false, errors.Wrapf(err, "failed to delete pre-existing instance %q", name) + if err != nil { + return false, fmt.Errorf("failed to delete pre-existing instance %q: %w", name, err) + } + + return false, nil }) + if err != nil { + return retObj, fmt.Errorf("error creating resource anew: %w", err) + } - return retObj, errors.Wrap(err, "error creating resource anew") + return retObj, nil } func mutableFieldsEqual(existingObj, newObj runtime.Object) bool { diff --git a/pkg/util/create_or_update_test.go b/pkg/util/create_or_update_test.go index 2b63ce43..c021ac00 100644 --- a/pkg/util/create_or_update_test.go +++ b/pkg/util/create_or_update_test.go @@ -19,11 +19,11 @@ package util_test import ( "context" + "errors" "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "github.com/pkg/errors" "github.com/submariner-io/admiral/pkg/fake" . "github.com/submariner-io/admiral/pkg/gomega" "github.com/submariner-io/admiral/pkg/resource" diff --git a/pkg/util/helpers.go b/pkg/util/helpers.go index 7f8bfecc..0d25c8fd 100644 --- a/pkg/util/helpers.go +++ b/pkg/util/helpers.go @@ -21,7 +21,6 @@ package util import ( "fmt" - "github.com/pkg/errors" resourceUtil "github.com/submariner-io/admiral/pkg/resource" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -42,12 +41,12 @@ const ( func BuildRestMapper(restConfig *rest.Config) (meta.RESTMapper, error) { discoveryClient, err := discovery.NewDiscoveryClientForConfig(restConfig) if err != nil { - return nil, errors.Wrap(err, "error creating discovery client") + return nil, fmt.Errorf("error creating discovery client: %w", err) } groupResources, err := restmapper.GetAPIGroupResources(discoveryClient) if err != nil { - return nil, errors.Wrap(err, "error retrieving API group resources") + return nil, fmt.Errorf("error retrieving API group resources: %w", err) } return restmapper.NewDiscoveryRESTMapper(groupResources), nil @@ -72,7 +71,7 @@ func FindGroupVersionResource(from *unstructured.Unstructured, restMapper meta.R gvk := from.GroupVersionKind() mapping, err := restMapper.RESTMapping(gvk.GroupKind(), gvk.Version) if err != nil { - return nil, errors.Wrapf(err, "error getting REST mapper for %#v", gvk) + return nil, fmt.Errorf("error getting REST mapper for %#v: %w", gvk, err) } return &mapping.Resource, nil diff --git a/pkg/watcher/resource_watcher.go b/pkg/watcher/resource_watcher.go index c3a4549f..d67bb2b6 100644 --- a/pkg/watcher/resource_watcher.go +++ b/pkg/watcher/resource_watcher.go @@ -25,7 +25,6 @@ import ( "reflect" "time" - "github.com/pkg/errors" "github.com/submariner-io/admiral/pkg/federate" "github.com/submariner-io/admiral/pkg/syncer" "github.com/submariner-io/admiral/pkg/util" @@ -125,14 +124,14 @@ func New(config *Config) (Interface, error) { restMapper := config.RestMapper if restMapper == nil { if restMapper, err = util.BuildRestMapper(config.RestConfig); err != nil { - return nil, errors.Wrap(err, "error building the REST mapper") + return nil, fmt.Errorf("error building the REST mapper: %w", err) } } client := config.Client if client == nil { if client, err = dynamic.NewForConfig(config.RestConfig); err != nil { - return nil, errors.Wrap(err, "error creating dynamic client") + return nil, fmt.Errorf("error creating dynamic client: %w", err) } } @@ -169,7 +168,7 @@ func New(config *Config) (Interface, error) { ResyncPeriod: config.ResyncPeriod, }) if err != nil { - return nil, errors.Wrapf(err, "error creating resource syncer %q", rc.Name) + return nil, fmt.Errorf("error creating resource syncer %q: %w", rc.Name, err) } watcher.syncers[reflect.TypeOf(rc.ResourceType)] = s