diff --git a/internal/controllers/core/clustercatalog_controller.go b/internal/controllers/core/clustercatalog_controller.go index 09b20ad..6b889cf 100644 --- a/internal/controllers/core/clustercatalog_controller.go +++ b/internal/controllers/core/clustercatalog_controller.go @@ -18,7 +18,6 @@ package core import ( "context" // #nosec - "errors" "fmt" "time" @@ -33,7 +32,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" "github.com/operator-framework/catalogd/api/core/v1alpha1" - catalogderrors "github.com/operator-framework/catalogd/internal/errors" "github.com/operator-framework/catalogd/internal/source" "github.com/operator-framework/catalogd/internal/storage" ) @@ -75,11 +73,6 @@ func (r *ClusterCatalogReconciler) Reconcile(ctx context.Context, req ctrl.Reque reconciledCatsrc := existingCatsrc.DeepCopy() res, reconcileErr := r.reconcile(ctx, reconciledCatsrc) - if errors.As(reconcileErr, &catalogderrors.Unrecoverable{}) { - l.Error(reconcileErr, "unrecoverable reconcile error") - reconcileErr = nil - } - // Update the status subresource before updating the main object. This is // necessary because, in many cases, the main object update will remove the // finalizer, which will cause the core Kubernetes deletion logic to diff --git a/internal/errors/unrecoverable.go b/internal/errors/unrecoverable.go deleted file mode 100644 index c28311d..0000000 --- a/internal/errors/unrecoverable.go +++ /dev/null @@ -1,12 +0,0 @@ -package errors - -// Unrecoverable represents an error that can not be recovered -// from without user intervention. When this error is returned -// the request should not be requeued. -type Unrecoverable struct { - error -} - -func NewUnrecoverable(err error) Unrecoverable { - return Unrecoverable{err} -} diff --git a/internal/source/containers_image.go b/internal/source/containers_image.go index e222be7..aa15169 100644 --- a/internal/source/containers_image.go +++ b/internal/source/containers_image.go @@ -26,9 +26,9 @@ import ( "github.com/opencontainers/go-digest" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" catalogdv1alpha1 "github.com/operator-framework/catalogd/api/core/v1alpha1" - catalogderrors "github.com/operator-framework/catalogd/internal/errors" ) const ConfigDirLabel = "operators.operatorframework.io.index.configs.v1" @@ -46,7 +46,7 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, catalog *catalogdv } if catalog.Spec.Source.Image == nil { - return nil, catalogderrors.NewUnrecoverable(fmt.Errorf("error parsing catalog, catalog %s has a nil image source", catalog.Name)) + return nil, reconcile.TerminalError(fmt.Errorf("error parsing catalog, catalog %s has a nil image source", catalog.Name)) } ////////////////////////////////////////////////////// @@ -191,7 +191,7 @@ func (i *ContainersImageRegistry) unpackPath(catalogName string, digest digest.D func resolveReferences(ctx context.Context, ref string, sourceContext *types.SystemContext) (reference.Named, reference.Canonical, bool, error) { imgRef, err := reference.ParseNamed(ref) if err != nil { - return nil, nil, false, catalogderrors.NewUnrecoverable(fmt.Errorf("error parsing image reference %q: %w", ref, err)) + return nil, nil, false, reconcile.TerminalError(fmt.Errorf("error parsing image reference %q: %w", ref, err)) } canonicalRef, isCanonical, err := resolveCanonicalRef(ctx, imgRef, sourceContext) @@ -208,7 +208,7 @@ func resolveCanonicalRef(ctx context.Context, imgRef reference.Named, imageCtx * srcRef, err := docker.NewReference(imgRef) if err != nil { - return nil, false, catalogderrors.NewUnrecoverable(fmt.Errorf("error creating reference: %w", err)) + return nil, false, reconcile.TerminalError(fmt.Errorf("error creating reference: %w", err)) } imgSrc, err := srcRef.NewImageSource(ctx, imageCtx) @@ -269,8 +269,8 @@ func (i *ContainersImageRegistry) unpackImage(ctx context.Context, unpackPath st if !ok { // If the spec is a tagged ref, retries could end up resolving a new digest, where the label // might show up. If the spec is canonical, no amount of retries will make the label appear. - // Therefore, we treat the error as unrecoverable if the reference from the spec is canonical. - return wrapUnrecoverable(fmt.Errorf("catalog image is missing the required label %q", ConfigDirLabel), specIsCanonical) + // Therefore, we treat the error as terminal if the reference from the spec is canonical. + return wrapTerminal(fmt.Errorf("catalog image is missing the required label %q", ConfigDirLabel), specIsCanonical) } if err := os.MkdirAll(unpackPath, 0700); err != nil { @@ -402,9 +402,9 @@ func deleteRecursive(root string) error { return os.RemoveAll(root) } -func wrapUnrecoverable(err error, isUnrecoverable bool) error { - if !isUnrecoverable { +func wrapTerminal(err error, isTerminal bool) error { + if !isTerminal { return err } - return catalogderrors.NewUnrecoverable(err) + return reconcile.TerminalError(err) } diff --git a/internal/source/containers_image_test.go b/internal/source/containers_image_test.go index bfd1e1c..6ed83e0 100644 --- a/internal/source/containers_image_test.go +++ b/internal/source/containers_image_test.go @@ -24,9 +24,9 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/operator-framework/catalogd/api/core/v1alpha1" - catalogderrors "github.com/operator-framework/catalogd/internal/errors" "github.com/operator-framework/catalogd/internal/source" ) @@ -39,7 +39,7 @@ func TestImageRegistry(t *testing.T) { // points to the registry created for the test catalog *v1alpha1.ClusterCatalog wantErr bool - unrecoverable bool + terminal bool image v1.Image digestAlreadyExists bool oldDigestExists bool @@ -60,9 +60,9 @@ func TestImageRegistry(t *testing.T) { }, }, }, - wantErr: true, - unrecoverable: true, - refType: "tag", + wantErr: true, + terminal: true, + refType: "tag", }, { name: ".spec.source.image.ref is unparsable", @@ -79,9 +79,9 @@ func TestImageRegistry(t *testing.T) { }, }, }, - wantErr: true, - unrecoverable: true, - refType: "tag", + wantErr: true, + terminal: true, + refType: "tag", }, { name: "tag based, image is missing required label", @@ -123,8 +123,8 @@ func TestImageRegistry(t *testing.T) { }, }, }, - wantErr: true, - unrecoverable: true, + wantErr: true, + terminal: true, image: func() v1.Image { img, err := random.Image(20, 3) if err != nil { @@ -408,8 +408,8 @@ func TestImageRegistry(t *testing.T) { } } else { assert.Error(t, err) - isUnrecov := errors.As(err, &catalogderrors.Unrecoverable{}) - assert.Equal(t, tt.unrecoverable, isUnrecov, "expected unrecoverable %v, got %v", tt.unrecoverable, isUnrecov) + isTerminal := errors.Is(err, reconcile.TerminalError(nil)) + assert.Equal(t, tt.terminal, isTerminal, "expected terminal %v, got %v", tt.terminal, isTerminal) } assert.NoError(t, imgReg.Cleanup(ctx, tt.catalog))