diff --git a/pkg/storage/legacysql/dualwrite/dualwriter.go b/pkg/storage/legacysql/dualwrite/dualwriter.go index 0cb1091b13b..afb97bb5432 100644 --- a/pkg/storage/legacysql/dualwrite/dualwriter.go +++ b/pkg/storage/legacysql/dualwrite/dualwriter.go @@ -39,16 +39,7 @@ type dualWriter struct { func (d *dualWriter) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { // If we read from unified, we can just do that and return. if d.readUnified { - unifiedGet, unifiedErr := d.unified.Get(ctx, name, options) - if apierrors.IsNotFound(unifiedErr) { - // If resource is not found in unified storage, fallback to legacy. - // This fixes cases in where records (stored in multiple tables, including permissions) - // are inserted first in legacy and then on Unified. - log := logging.FromContext(ctx).With("method", "Get") - log.Error("resource not found in Unified Storage, trying to GET from legacy", "err", unifiedErr) - return d.legacy.Get(ctx, name, options) - } - return unifiedGet, unifiedErr + return d.unified.Get(ctx, name, options) } // If legacy is still our main store, lets first read from it. legacyGet, err := d.legacy.Get(ctx, name, options) @@ -196,6 +187,22 @@ func (d *dualWriter) Create(ctx context.Context, in runtime.Object, createValida return nil, fmt.Errorf("name or generatename have to be set") } + readFromUnifiedWriteToBothStorages := d.readUnified && d.legacy != nil && d.unified != nil + + permissions := "" + if readFromUnifiedWriteToBothStorages { + objIn, err := utils.MetaAccessor(in) + if err != nil { + return nil, err + } + + // keep permissions, we will set it back after the object is created + permissions = objIn.GetAnnotation(utils.AnnoKeyGrantPermissions) + if permissions != "" { + objIn.SetAnnotation(utils.AnnoKeyGrantPermissions, "") // remove the annotation for now + } + } + // create in legacy first, and then unistore. if unistore fails, but legacy succeeds, // will try to cleanup the object in legacy. createdFromLegacy, err := d.legacy.Create(ctx, in, createValidation, options) @@ -212,6 +219,17 @@ func (d *dualWriter) Create(ctx context.Context, in runtime.Object, createValida accCreated.SetResourceVersion("") accCreated.SetUID("") + if readFromUnifiedWriteToBothStorages { + objCopy, err := utils.MetaAccessor(createdCopy) + if err != nil { + return nil, err + } + // restore the permissions annotation, as we removed it before creating in legacy + if permissions != "" { + objCopy.SetAnnotation(utils.AnnoKeyGrantPermissions, permissions) + } + } + // If unified storage is the primary storage, let's just create it in the foreground and return it. if d.readUnified { storageObj, errObjectSt := d.unified.Create(ctx, createdCopy, createValidation, options) diff --git a/pkg/storage/legacysql/dualwrite/dualwriter_mode3_test.go b/pkg/storage/legacysql/dualwrite/dualwriter_mode3_test.go index 00753d1d656..3076d8a9409 100644 --- a/pkg/storage/legacysql/dualwrite/dualwriter_mode3_test.go +++ b/pkg/storage/legacysql/dualwrite/dualwriter_mode3_test.go @@ -109,40 +109,12 @@ func TestMode3_Get(t *testing.T) { }, }, { - name: "should return an error when getting an object in the unified store fails", + name: "should return an error when getting an object in the unified store fails, and should not go to legacy", setupStorageFn: func(m *mock.Mock, name string) { m.On("Get", mock.Anything, name, mock.Anything).Return(nil, errors.New("error")) }, wantErr: true, }, - { - name: "should return an error when getting an object in the unified store fails with not found error, and legacy fails as well", - setupLegacyFn: func(m *mock.Mock, name string) { - m.On("Get", mock.Anything, name, mock.Anything).Return(nil, errors.New("error")) - }, - setupStorageFn: func(m *mock.Mock, name string) { - m.On("Get", mock.Anything, name, mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "dashboards.dashboard.grafana.app", Resource: "dashboard"}, "uid")) - }, - wantErr: true, - }, - { - name: "should succeed when getting an object in the UnifiedStorage fails with NotFound, but Legacy succeeds", - setupLegacyFn: func(m *mock.Mock, name string) { - m.On("Get", mock.Anything, name, mock.Anything).Return(exampleObj, nil) - }, - setupStorageFn: func(m *mock.Mock, name string) { - m.On("Get", mock.Anything, name, mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "dashboards.dashboard.grafana.app", Resource: "dashboard"}, "uid")) - }, - }, - { - name: "should succeed when getting an object in the LegacyStorage fails", - setupLegacyFn: func(m *mock.Mock, name string) { - m.On("Get", mock.Anything, name, mock.Anything).Return(nil, errors.New("error")) - }, - setupStorageFn: func(m *mock.Mock, name string) { - m.On("Get", mock.Anything, name, mock.Anything).Return(exampleObj, nil) - }, - }, } name := "foo"