mirror of
https://github.com/grafana/grafana.git
synced 2025-07-31 21:52:34 +08:00
Unified Storage: Return an already exists error (#102857)
* Unified Storage: Return an already exists error When inserting a resource that already exists (i.e. race condition), we can safely catch UNIQUE constraint violations and transform them into a `k8s.io/apimachinery/pkg/api/errors` error that stands the test of `errors.IsAlreadyExists`. * feat: clarify existing conflict error * chore: make update-workspace * feat: make new package for backend error * fix: assign dependency owner * feat: use dialect for checking error type * chore: go generate * revert: to 5af369166d6
This commit is contained in:

committed by
GitHub

parent
9f7df8b788
commit
f7b9f1ce69
2
go.mod
2
go.mod
@ -396,7 +396,7 @@ require (
|
||||
github.com/invopop/jsonschema v0.13.0 // indirect
|
||||
github.com/jackc/pgpassfile v1.0.0 // indirect
|
||||
github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect
|
||||
github.com/jackc/pgx/v5 v5.7.2 // indirect
|
||||
github.com/jackc/pgx/v5 v5.7.2 // @grafana/grafana-search-and-storage
|
||||
github.com/jackc/puddle/v2 v2.2.2 // indirect
|
||||
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect
|
||||
github.com/jcmturner/aescts/v2 v2.0.0 // indirect
|
||||
|
21
pkg/storage/unified/backend/errors.go
Normal file
21
pkg/storage/unified/backend/errors.go
Normal file
@ -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,
|
||||
},
|
||||
}
|
||||
)
|
14
pkg/storage/unified/backend/errors_test.go
Normal file
14
pkg/storage/unified/backend/errors_test.go
Normal file
@ -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")
|
||||
}
|
@ -480,6 +480,7 @@ func (s *server) Create(ctx context.Context, req *CreateRequest) (*CreateRespons
|
||||
if found != nil && len(found.Value) > 0 {
|
||||
rsp.Error = &ErrorResult{
|
||||
Code: http.StatusConflict,
|
||||
Reason: string(metav1.StatusReasonAlreadyExists),
|
||||
Message: "key already exists", // TODO?? soft delete replace?
|
||||
}
|
||||
return rsp, nil
|
||||
|
@ -9,7 +9,11 @@ import (
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"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"
|
||||
@ -337,6 +341,9 @@ func (b *backend) create(ctx context.Context, event resource.WriteEvent) (int64,
|
||||
Folder: folder,
|
||||
GUID: guid,
|
||||
}); err != nil {
|
||||
if isRowAlreadyExistsError(err) {
|
||||
return guid, unifiedbackend.ErrResourceAlreadyExists
|
||||
}
|
||||
return guid, fmt.Errorf("insert into resource: %w", err)
|
||||
}
|
||||
|
||||
@ -377,6 +384,28 @@ func (b *backend) create(ctx context.Context, event resource.WriteEvent) (int64,
|
||||
return rv, nil
|
||||
}
|
||||
|
||||
// isRowAlreadyExistsError checks if the error is the result of the row inserted already existing.
|
||||
func isRowAlreadyExistsError(err error) bool {
|
||||
var sqlite sqlite3.Error
|
||||
if errors.As(err, &sqlite) {
|
||||
return sqlite.ExtendedCode == sqlite3.ErrConstraintUnique
|
||||
}
|
||||
|
||||
var pg *pgconn.PgError
|
||||
if errors.As(err, &pg) {
|
||||
// https://www.postgresql.org/docs/current/errcodes-appendix.html
|
||||
return pg.Code == "23505" // unique_violation
|
||||
}
|
||||
|
||||
var mysqlerr *mysql.MySQLError
|
||||
if errors.As(err, &mysqlerr) {
|
||||
// https://dev.mysql.com/doc/mysql-errors/8.0/en/server-error-reference.html
|
||||
return mysqlerr.Number == 1062 // ER_DUP_ENTRY
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
func (b *backend) update(ctx context.Context, event resource.WriteEvent) (int64, error) {
|
||||
ctx, span := b.tracer.Start(ctx, tracePrefix+"Update")
|
||||
defer span.End()
|
||||
|
@ -8,10 +8,12 @@ import (
|
||||
"testing"
|
||||
|
||||
sqlmock "github.com/DATA-DOG/go-sqlmock"
|
||||
"github.com/mattn/go-sqlite3"
|
||||
"github.com/stretchr/testify/require"
|
||||
"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"
|
||||
@ -229,6 +231,29 @@ func TestBackend_create(t *testing.T) {
|
||||
require.Equal(t, int64(200), v)
|
||||
})
|
||||
|
||||
t.Run("resource already exists", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
b, ctx := setupBackendTest(t)
|
||||
b.SQLMock.ExpectBegin()
|
||||
expectSuccessfulResourceVersionExec(t, b.TestDBProvider,
|
||||
func() { b.ExecWithResult("insert resource", 0, 1) },
|
||||
func() { b.ExecWithResult("insert resource_history", 0, 1) },
|
||||
)
|
||||
b.SQLMock.ExpectCommit()
|
||||
b.SQLMock.ExpectBegin()
|
||||
b.SQLMock.ExpectExec("insert resource").WillReturnError(sqlite3.Error{Code: sqlite3.ErrConstraint, ExtendedCode: sqlite3.ErrConstraintUnique})
|
||||
b.SQLMock.ExpectRollback()
|
||||
|
||||
// First we insert the resource successfully. This is what the happy path test does as well.
|
||||
v, err := b.create(ctx, event)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, int64(200), v)
|
||||
|
||||
// Then we try to insert the same resource again. This should fail.
|
||||
_, err = b.create(ctx, event)
|
||||
require.ErrorIs(t, err, unifiedbackend.ErrResourceAlreadyExists)
|
||||
})
|
||||
|
||||
t.Run("error inserting into resource", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
b, ctx := setupBackendTest(t)
|
||||
|
Reference in New Issue
Block a user