From 6836bfe1ea1bf62f4eb66dc1328a456183874f81 Mon Sep 17 00:00:00 2001 From: Arati R <33031346+suntala@users.noreply.github.com> Date: Tue, 14 May 2024 14:11:56 +0200 Subject: [PATCH] Storage: Customise setting dual writing modes (#87668) * Add feature toggles for mode 2 and 3 playlist dual writing * Make current mode customised based on kind * Check feature flags when initialising dual writer * Fix linting * Refactor NewDualWriter --- .golangci.toml | 1 + .../feature-toggles/index.md | 2 + .../src/types/featureToggles.gen.ts | 2 + pkg/apiserver/rest/dualwriter.go | 43 ++++++++----------- pkg/apiserver/rest/dualwriter_mode1_test.go | 12 +++--- pkg/apiserver/rest/dualwriter_mode2_test.go | 12 +++--- pkg/registry/apis/dashboard/register.go | 2 +- pkg/registry/apis/folders/register.go | 2 +- pkg/registry/apis/playlist/register.go | 11 ++++- pkg/services/featuremgmt/registry.go | 12 ++++++ pkg/services/featuremgmt/toggles_gen.csv | 2 + pkg/services/featuremgmt/toggles_gen.go | 8 ++++ pkg/services/featuremgmt/toggles_gen.json | 24 +++++++++++ 13 files changed, 92 insertions(+), 41 deletions(-) diff --git a/.golangci.toml b/.golangci.toml index a01d05a5855..c32f1e7efc5 100644 --- a/.golangci.toml +++ b/.golangci.toml @@ -76,6 +76,7 @@ allow = [ "github.com/grafana/grafana/pkg/apimachinery", "github.com/grafana/grafana/pkg/apiserver", "github.com/grafana/grafana/pkg/services/apiserver/utils", + "github.com/grafana/grafana/pkg/services/featuremgmt", ] deny = [ { pkg = "github.com/grafana/grafana/pkg", desc = "apiserver is not allowed to import grafana core" } diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index b6ea344bd2d..1607452506d 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -120,6 +120,8 @@ Experimental features might be changed or removed without prior notice. | `disableSecretsCompatibility` | Disable duplicated secret storage in legacy tables | | `logRequestsInstrumentedAsUnknown` | Logs the path for requests that are instrumented as unknown | | `unifiedStorage` | SQL-based k8s storage | +| `dualWritePlaylistsMode2` | Enables dual writing of playlists to both legacy and k8s storage in mode 2 | +| `dualWritePlaylistsMode3` | Enables dual writing of playlists to both legacy and k8s storage in mode 3 | | `showDashboardValidationWarnings` | Show warnings when dashboards do not validate against the schema | | `mysqlAnsiQuotes` | Use double quotes to escape keyword in a MySQL query | | `alertingBacktesting` | Rule backtesting API for alerting | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 2e67c8418e9..6bc6776b125 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -47,6 +47,8 @@ export interface FeatureToggles { returnToPrevious?: boolean; grpcServer?: boolean; unifiedStorage?: boolean; + dualWritePlaylistsMode2?: boolean; + dualWritePlaylistsMode3?: boolean; cloudWatchCrossAccountQuerying?: boolean; showDashboardValidationWarnings?: boolean; mysqlAnsiQuotes?: boolean; diff --git a/pkg/apiserver/rest/dualwriter.go b/pkg/apiserver/rest/dualwriter.go index 3f20d34f973..b75398a0d6c 100644 --- a/pkg/apiserver/rest/dualwriter.go +++ b/pkg/apiserver/rest/dualwriter.go @@ -80,14 +80,24 @@ const ( Mode4 ) -var CurrentMode = Mode2 - -//TODO: make CurrentMode customisable and specific to each entity -// change DualWriter signature to get the current mode as an argument - // NewDualWriter returns a new DualWriter. -func NewDualWriter(legacy LegacyStorage, storage Storage) DualWriter { - return selectDualWriter(CurrentMode, legacy, storage) +func NewDualWriter(mode DualWriterMode, legacy LegacyStorage, storage Storage) DualWriter { + switch mode { + case Mode1: + // read and write only from legacy storage + return NewDualWriterMode1(legacy, storage) + case Mode2: + // write to both, read from storage but use legacy as backup + return NewDualWriterMode2(legacy, storage) + case Mode3: + // write to both, read from storage only + return NewDualWriterMode3(legacy, storage) + case Mode4: + // read and write only from storage + return NewDualWriterMode4(legacy, storage) + default: + return NewDualWriterMode1(legacy, storage) + } } type updateWrapper struct { @@ -107,22 +117,3 @@ func (u *updateWrapper) Preconditions() *metav1.Preconditions { func (u *updateWrapper) UpdatedObject(ctx context.Context, oldObj runtime.Object) (newObj runtime.Object, err error) { return u.updated, nil } - -func selectDualWriter(mode DualWriterMode, legacy LegacyStorage, storage Storage) DualWriter { - switch mode { - case Mode1: - // read and write only from legacy storage - return NewDualWriterMode1(legacy, storage) - case Mode2: - // write to both, read from storage but use legacy as backup - return NewDualWriterMode2(legacy, storage) - case Mode3: - // write to both, read from storage only - return NewDualWriterMode3(legacy, storage) - case Mode4: - // read and write only from storage - return NewDualWriterMode4(legacy, storage) - default: - return NewDualWriterMode1(legacy, storage) - } -} diff --git a/pkg/apiserver/rest/dualwriter_mode1_test.go b/pkg/apiserver/rest/dualwriter_mode1_test.go index 688faa8a68a..0f7b5378add 100644 --- a/pkg/apiserver/rest/dualwriter_mode1_test.go +++ b/pkg/apiserver/rest/dualwriter_mode1_test.go @@ -65,7 +65,7 @@ func TestMode1_Create(t *testing.T) { tt.setupStorageFn(m, tt.input) } - dw := selectDualWriter(Mode1, ls, us) + dw := NewDualWriter(Mode1, ls, us) obj, err := dw.Create(context.Background(), tt.input, func(context.Context, runtime.Object) error { return nil }, &metav1.CreateOptions{}) @@ -126,7 +126,7 @@ func TestMode1_Get(t *testing.T) { tt.setupStorageFn(m, tt.input) } - dw := selectDualWriter(Mode1, ls, us) + dw := NewDualWriter(Mode1, ls, us) obj, err := dw.Get(context.Background(), tt.input, &metav1.GetOptions{}) @@ -175,7 +175,7 @@ func TestMode1_List(t *testing.T) { tt.setupStorageFn(m) } - dw := selectDualWriter(Mode1, ls, us) + dw := NewDualWriter(Mode1, ls, us) _, err := dw.List(context.Background(), &metainternalversion.ListOptions{}) @@ -228,7 +228,7 @@ func TestMode1_Delete(t *testing.T) { tt.setupStorageFn(m, tt.input) } - dw := selectDualWriter(Mode1, ls, us) + dw := NewDualWriter(Mode1, ls, us) obj, _, err := dw.Delete(context.Background(), tt.input, func(ctx context.Context, obj runtime.Object) error { return nil }, &metav1.DeleteOptions{}) @@ -285,7 +285,7 @@ func TestMode1_DeleteCollection(t *testing.T) { tt.setupStorageFn(m, tt.input) } - dw := selectDualWriter(Mode1, ls, us) + dw := NewDualWriter(Mode1, ls, us) obj, err := dw.DeleteCollection(context.Background(), func(ctx context.Context, obj runtime.Object) error { return nil }, tt.input, &metainternalversion.ListOptions{}) @@ -359,7 +359,7 @@ func TestMode1_Update(t *testing.T) { tt.setupGetFn(m, tt.input) } - dw := selectDualWriter(Mode1, ls, us) + dw := NewDualWriter(Mode1, ls, us) obj, _, err := dw.Update(context.Background(), tt.input, UpdatedObjInfoObj{}, func(ctx context.Context, obj runtime.Object) error { return nil }, func(ctx context.Context, obj, old runtime.Object) error { return nil }, false, &metav1.UpdateOptions{}) diff --git a/pkg/apiserver/rest/dualwriter_mode2_test.go b/pkg/apiserver/rest/dualwriter_mode2_test.go index 15cb5b8db86..845b9586e9b 100644 --- a/pkg/apiserver/rest/dualwriter_mode2_test.go +++ b/pkg/apiserver/rest/dualwriter_mode2_test.go @@ -64,7 +64,7 @@ func TestMode2_Create(t *testing.T) { tt.setupStorageFn(m, tt.input) } - dw := selectDualWriter(Mode2, ls, us) + dw := NewDualWriter(Mode2, ls, us) obj, err := dw.Create(context.Background(), tt.input, createFn, &metav1.CreateOptions{}) @@ -138,7 +138,7 @@ func TestMode2_Get(t *testing.T) { tt.setupStorageFn(m, tt.input) } - dw := selectDualWriter(Mode2, ls, us) + dw := NewDualWriter(Mode2, ls, us) obj, err := dw.Get(context.Background(), tt.input, &metav1.GetOptions{}) @@ -189,7 +189,7 @@ func TestMode2_List(t *testing.T) { tt.setupStorageFn(m) } - dw := selectDualWriter(Mode2, ls, us) + dw := NewDualWriter(Mode2, ls, us) obj, err := dw.List(context.Background(), &metainternalversion.ListOptions{}) @@ -281,7 +281,7 @@ func TestMode2_Delete(t *testing.T) { tt.setupStorageFn(m, tt.input) } - dw := selectDualWriter(Mode2, ls, us) + dw := NewDualWriter(Mode2, ls, us) obj, _, err := dw.Delete(context.Background(), tt.input, func(context.Context, runtime.Object) error { return nil }, &metav1.DeleteOptions{}) @@ -351,7 +351,7 @@ func TestMode2_DeleteCollection(t *testing.T) { tt.setupStorageFn(m) } - dw := selectDualWriter(Mode2, ls, us) + dw := NewDualWriter(Mode2, ls, us) obj, err := dw.DeleteCollection(context.Background(), func(ctx context.Context, obj runtime.Object) error { return nil }, &metav1.DeleteOptions{TypeMeta: metav1.TypeMeta{Kind: tt.input}}, &metainternalversion.ListOptions{}) @@ -458,7 +458,7 @@ func TestMode2_Update(t *testing.T) { tt.setupStorageFn(m, tt.input) } - dw := selectDualWriter(Mode2, ls, us) + dw := NewDualWriter(Mode2, ls, us) obj, _, err := dw.Update(context.Background(), tt.input, UpdatedObjInfoObj{}, func(ctx context.Context, obj runtime.Object) error { return nil }, func(ctx context.Context, obj, old runtime.Object) error { return nil }, false, &metav1.UpdateOptions{}) diff --git a/pkg/registry/apis/dashboard/register.go b/pkg/registry/apis/dashboard/register.go index 98b928a560f..3cba9ecbaad 100644 --- a/pkg/registry/apis/dashboard/register.go +++ b/pkg/registry/apis/dashboard/register.go @@ -178,7 +178,7 @@ func (b *DashboardsAPIBuilder) GetAPIGroupInfo( if err := store.CompleteWithOptions(options); err != nil { return nil, err } - storage[resourceInfo.StoragePath()] = grafanarest.NewDualWriter(legacyStore, store) + storage[resourceInfo.StoragePath()] = grafanarest.NewDualWriter(grafanarest.Mode1, legacyStore, store) } // Summary diff --git a/pkg/registry/apis/folders/register.go b/pkg/registry/apis/folders/register.go index 4caaa232b30..b10d5bd4edc 100644 --- a/pkg/registry/apis/folders/register.go +++ b/pkg/registry/apis/folders/register.go @@ -139,7 +139,7 @@ func (b *FolderAPIBuilder) GetAPIGroupInfo( if err != nil { return nil, err } - storage[resourceInfo.StoragePath()] = grafanarest.NewDualWriter(legacyStore, store) + storage[resourceInfo.StoragePath()] = grafanarest.NewDualWriter(grafanarest.Mode1, legacyStore, store) } apiGroupInfo.VersionedResourcesStorageMap[v0alpha1.VERSION] = storage diff --git a/pkg/registry/apis/playlist/register.go b/pkg/registry/apis/playlist/register.go index 5bda0f177ec..68623623e8a 100644 --- a/pkg/registry/apis/playlist/register.go +++ b/pkg/registry/apis/playlist/register.go @@ -19,6 +19,7 @@ import ( grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" "github.com/grafana/grafana/pkg/services/apiserver/utils" + "github.com/grafana/grafana/pkg/services/featuremgmt" playlistsvc "github.com/grafana/grafana/pkg/services/playlist" "github.com/grafana/grafana/pkg/setting" ) @@ -30,16 +31,19 @@ type PlaylistAPIBuilder struct { service playlistsvc.Service namespacer request.NamespaceMapper gv schema.GroupVersion + features featuremgmt.FeatureToggles } func RegisterAPIService(p playlistsvc.Service, apiregistration builder.APIRegistrar, cfg *setting.Cfg, + features featuremgmt.FeatureToggles, ) *PlaylistAPIBuilder { builder := &PlaylistAPIBuilder{ service: p, namespacer: request.GetNamespaceMapper(cfg), gv: playlist.PlaylistResourceInfo.GroupVersion(), + features: features, } apiregistration.RegisterAPI(builder) return builder @@ -118,7 +122,12 @@ func (b *PlaylistAPIBuilder) GetAPIGroupInfo( if err != nil { return nil, err } - storage[resource.StoragePath()] = grafanarest.NewDualWriterMode2(legacyStore, store) + + mode := grafanarest.Mode1 + if b.features.IsEnabledGlobally(featuremgmt.FlagDualWritePlaylistsMode2) { + mode = grafanarest.Mode2 + } + storage[resource.StoragePath()] = grafanarest.NewDualWriter(mode, legacyStore, store) } apiGroupInfo.VersionedResourcesStorageMap[playlist.VERSION] = storage diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 331c4cc97f7..2da8f76522f 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -228,6 +228,18 @@ var ( RequiresRestart: true, // new SQL tables created Owner: grafanaAppPlatformSquad, }, + { + Name: "dualWritePlaylistsMode2", + Description: "Enables dual writing of playlists to both legacy and k8s storage in mode 2", + Stage: FeatureStageExperimental, + Owner: grafanaSearchAndStorageSquad, + }, + { + Name: "dualWritePlaylistsMode3", + Description: "Enables dual writing of playlists to both legacy and k8s storage in mode 3", + Stage: FeatureStageExperimental, + Owner: grafanaSearchAndStorageSquad, + }, { Name: "cloudWatchCrossAccountQuerying", Description: "Enables cross-account querying in CloudWatch datasources", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 27696e5b9ca..83f9ab5a28c 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -28,6 +28,8 @@ topnav,deprecated,@grafana/grafana-frontend-platform,false,false,false returnToPrevious,GA,@grafana/grafana-frontend-platform,false,false,true grpcServer,preview,@grafana/grafana-app-platform-squad,false,false,false unifiedStorage,experimental,@grafana/grafana-app-platform-squad,false,true,false +dualWritePlaylistsMode2,experimental,@grafana/search-and-storage,false,false,false +dualWritePlaylistsMode3,experimental,@grafana/search-and-storage,false,false,false cloudWatchCrossAccountQuerying,GA,@grafana/aws-datasources,false,false,false showDashboardValidationWarnings,experimental,@grafana/dashboards-squad,false,false,false mysqlAnsiQuotes,experimental,@grafana/search-and-storage,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index d28d2e341ad..f5ee8152d88 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -123,6 +123,14 @@ const ( // SQL-based k8s storage FlagUnifiedStorage = "unifiedStorage" + // FlagDualWritePlaylistsMode2 + // Enables dual writing of playlists to both legacy and k8s storage in mode 2 + FlagDualWritePlaylistsMode2 = "dualWritePlaylistsMode2" + + // FlagDualWritePlaylistsMode3 + // Enables dual writing of playlists to both legacy and k8s storage in mode 3 + FlagDualWritePlaylistsMode3 = "dualWritePlaylistsMode3" + // FlagCloudWatchCrossAccountQuerying // Enables cross-account querying in CloudWatch datasources FlagCloudWatchCrossAccountQuerying = "cloudWatchCrossAccountQuerying" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 8cc7bfa349c..e6ec5319015 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -2179,6 +2179,30 @@ "stage": "experimental", "codeowner": "@grafana/grafana-frontend-platform" } + }, + { + "metadata": { + "name": "dualWritePlaylistsMode2", + "resourceVersion": "1715369873132", + "creationTimestamp": "2024-05-10T19:37:53Z" + }, + "spec": { + "description": "Enables dual writing of playlists to both legacy and k8s storage in mode 2", + "stage": "experimental", + "codeowner": "@grafana/search-and-storage" + } + }, + { + "metadata": { + "name": "dualWritePlaylistsMode3", + "resourceVersion": "1715369873132", + "creationTimestamp": "2024-05-10T19:37:53Z" + }, + "spec": { + "description": "Enables dual writing of playlists to both legacy and k8s storage in mode 3", + "stage": "experimental", + "codeowner": "@grafana/search-and-storage" + } } ] } \ No newline at end of file