From 455651f4222eba7164e4175906381810465e9ed4 Mon Sep 17 00:00:00 2001 From: Mariell Hoversholm Date: Wed, 26 Mar 2025 09:52:44 +0100 Subject: [PATCH] feat: make new package for backend error --- pkg/storage/unified/backend/errors.go | 21 +++++++++++++++++++ pkg/storage/unified/backend/errors_test.go | 14 +++++++++++++ pkg/storage/unified/sql/backend.go | 24 ++-------------------- pkg/storage/unified/sql/backend_test.go | 10 ++------- 4 files changed, 39 insertions(+), 30 deletions(-) create mode 100644 pkg/storage/unified/backend/errors.go create mode 100644 pkg/storage/unified/backend/errors_test.go diff --git a/pkg/storage/unified/backend/errors.go b/pkg/storage/unified/backend/errors.go new file mode 100644 index 00000000000..49a5a232b6a --- /dev/null +++ b/pkg/storage/unified/backend/errors.go @@ -0,0 +1,21 @@ +package backend + +import ( + "net/http" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// Errors that may be returned by the storage backend, to be converted by the resource layer. +// These are defined here to let all storage backends use the same error types without depending on one another. +var ( + ErrResourceAlreadyExists error = &apierrors.StatusError{ + ErrStatus: metav1.Status{ + Status: metav1.StatusFailure, + Reason: metav1.StatusReasonAlreadyExists, + Message: "the resource already exists", + Code: http.StatusConflict, + }, + } +) diff --git a/pkg/storage/unified/backend/errors_test.go b/pkg/storage/unified/backend/errors_test.go new file mode 100644 index 00000000000..644b7342c96 --- /dev/null +++ b/pkg/storage/unified/backend/errors_test.go @@ -0,0 +1,14 @@ +package backend + +import ( + "testing" + + "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" +) + +func TestErrResourceAlreadyExistsIsRecognisable(t *testing.T) { + t.Parallel() + + require.True(t, apierrors.IsAlreadyExists(ErrResourceAlreadyExists), "ErrResourceAlreadyExists should be recognised as an AlreadyExists error") +} diff --git a/pkg/storage/unified/sql/backend.go b/pkg/storage/unified/sql/backend.go index d0c0f27daf6..2e411730aae 100644 --- a/pkg/storage/unified/sql/backend.go +++ b/pkg/storage/unified/sql/backend.go @@ -6,22 +6,19 @@ import ( "errors" "fmt" "math" - "net/http" "sync" "time" - "cloud.google.com/go/spanner" "github.com/go-sql-driver/mysql" "github.com/google/uuid" + unifiedbackend "github.com/grafana/grafana/pkg/storage/unified/backend" "github.com/jackc/pgx/v5/pgconn" "github.com/mattn/go-sqlite3" "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/trace" "go.opentelemetry.io/otel/trace/noop" - "google.golang.org/grpc/codes" "google.golang.org/protobuf/proto" apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/storage/unified/resource" @@ -31,15 +28,6 @@ import ( "github.com/grafana/grafana/pkg/util/debouncer" ) -var ErrResourceAlreadyExists error = &apierrors.StatusError{ - ErrStatus: metav1.Status{ - Status: metav1.StatusFailure, - Reason: metav1.StatusReasonAlreadyExists, - Message: "the resource already exists", - Code: http.StatusConflict, - }, -} - const tracePrefix = "sql.resource." const defaultPollingInterval = 100 * time.Millisecond const defaultWatchBufferSize = 100 // number of events to buffer in the watch stream @@ -354,7 +342,7 @@ func (b *backend) create(ctx context.Context, event resource.WriteEvent) (int64, GUID: guid, }); err != nil { if isRowAlreadyExistsError(err) { - return guid, ErrResourceAlreadyExists + return guid, unifiedbackend.ErrResourceAlreadyExists } return guid, fmt.Errorf("insert into resource: %w", err) } @@ -397,9 +385,6 @@ func (b *backend) create(ctx context.Context, event resource.WriteEvent) (int64, } // isRowAlreadyExistsError checks if the error is the result of the row inserted already existing. -// -// On SQLite and Postgres, this is known as a UNIQUE constraint violation. On MySQL, it's known as a duplicate entry. -// On Spanner, it's a gRPC ALREADY_EXISTS error. func isRowAlreadyExistsError(err error) bool { var sqlite sqlite3.Error if errors.As(err, &sqlite) { @@ -418,11 +403,6 @@ func isRowAlreadyExistsError(err error) bool { return mysqlerr.Number == 1062 // ER_DUP_ENTRY } - // ErrCode returns Unknown for non-gRPC errors. - if spanner.ErrCode(err) == codes.AlreadyExists { - return true - } - return false } diff --git a/pkg/storage/unified/sql/backend_test.go b/pkg/storage/unified/sql/backend_test.go index de6a4028782..d4fbc781479 100644 --- a/pkg/storage/unified/sql/backend_test.go +++ b/pkg/storage/unified/sql/backend_test.go @@ -10,10 +10,10 @@ import ( sqlmock "github.com/DATA-DOG/go-sqlmock" "github.com/mattn/go-sqlite3" "github.com/stretchr/testify/require" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/grafana/grafana/pkg/apimachinery/utils" + unifiedbackend "github.com/grafana/grafana/pkg/storage/unified/backend" "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/grafana/grafana/pkg/storage/unified/sql/db/dbimpl" "github.com/grafana/grafana/pkg/storage/unified/sql/test" @@ -251,7 +251,7 @@ func TestBackend_create(t *testing.T) { // Then we try to insert the same resource again. This should fail. _, err = b.create(ctx, event) - require.ErrorIs(t, err, ErrResourceAlreadyExists) + require.ErrorIs(t, err, unifiedbackend.ErrResourceAlreadyExists) }) t.Run("error inserting into resource", func(t *testing.T) { @@ -667,12 +667,6 @@ func TestBackend_getHistoryPagination(t *testing.T) { }) } -func TestErrResourceAlreadyExistsIsRecognisable(t *testing.T) { - t.Parallel() - - require.True(t, apierrors.IsAlreadyExists(ErrResourceAlreadyExists), "ErrResourceAlreadyExists should be recognised as an AlreadyExists error") -} - // setupHistoryTest creates the necessary mock expectations for a history test func setupHistoryTest(b testBackend, resourceVersions []int64, latestRV int64) *sqlmock.Rows { // Expect fetch latest RV call - set to the highest resource version