Storage: Read desired mode from config instead of feature flags (grafana#88353)

* Read desired mode from config
* Update playlist integration tests
* Add mode 1 playlist integration tests
* Add mode 0 dual writing to playlist integration tests
* Add documentation for the different dual writing modes

Author: suntala

.golangci.toml

@@ -78,6 +78,8 @@ allow = [
   "github.com/grafana/grafana/pkg/services/apiserver/utils",
   "github.com/grafana/grafana/pkg/services/featuremgmt",
   "github.com/grafana/grafana/pkg/infra/kvstore",
+  "github.com/grafana/grafana/pkg/services/apiserver/options",
+  "github.com/grafana/grafana/pkg/apis/playlist/v0alpha1",
 ]
 deny = [
   { pkg = "github.com/grafana/grafana/pkg", desc = "apiserver is not allowed to import grafana core" }

pkg/apis/playlist/v0alpha1/register.go

@@ -8,13 +8,15 @@ import (
 )
 
 const (
-	GROUP      = "playlist.grafana.app"
-	VERSION    = "v0alpha1"
-	APIVERSION = GROUP + "/" + VERSION
+	GROUP         = "playlist.grafana.app"
+	VERSION       = "v0alpha1"
+	APIVERSION    = GROUP + "/" + VERSION
+	RESOURCE      = "playlists"
+	GROUPRESOURCE = GROUP + "/" + RESOURCE
 )
 
 var PlaylistResourceInfo = common.NewResourceInfo(GROUP, VERSION,
-	"playlists", "playlist", "Playlist",
+	RESOURCE, "playlist", "Playlist",
 	func() runtime.Object { return &Playlist{} },
 	func() runtime.Object { return &PlaylistList{} },
 )

pkg/apiserver/builder/common.go

@@ -3,6 +3,7 @@ package builder
 import (
 	"net/http"
 
+	grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
@@ -27,7 +28,7 @@ type APIGroupBuilder interface {
 		scheme *runtime.Scheme,
 		codecs serializer.CodecFactory,
 		optsGetter generic.RESTOptionsGetter,
-		dualWrite bool,
+		desiredMode grafanarest.DualWriterMode,
 	) (*genericapiserver.APIGroupInfo, error)
 
 	// Get OpenAPI definitions
@@ -40,6 +41,11 @@ type APIGroupBuilder interface {
 	// Standard namespace checking will happen before this is called, specifically
 	// the namespace must matches an org|stack that the user belongs to
 	GetAuthorizer() authorizer.Authorizer
+
+	// Get the desired dual writing mode. These are modes 1, 2, 3 and 4 if
+	// the feature flag `unifiedStorage` is enabled and mode 0 if it is not enabled.
+	// #TODO add type for map[string]grafanarest.DualWriterMode?
+	GetDesiredDualWriterMode(dualWrite bool, toMode map[string]grafanarest.DualWriterMode) grafanarest.DualWriterMode
 }
 
 // Builders that implement OpenAPIPostProcessor are given a chance to modify the schema directly

pkg/apiserver/builder/helper.go

@@ -24,6 +24,7 @@ import (
 	"k8s.io/kube-openapi/pkg/common"
 
 	"github.com/grafana/grafana/pkg/apiserver/endpoints/filters"
+	"github.com/grafana/grafana/pkg/services/apiserver/options"
 )
 
 // TODO: this is a temporary hack to make rest.Connecter work with resource level routes
@@ -126,10 +127,16 @@ func InstallAPIs(
 	server *genericapiserver.GenericAPIServer,
 	optsGetter generic.RESTOptionsGetter,
 	builders []APIGroupBuilder,
-	dualWrite bool,
+	storageOpts *options.StorageOptions,
 ) error {
+	// dual writing is only enabled when the storage type is not legacy.
+	// this is needed to support setting a default RESTOptionsGetter for new APIs that don't
+	// support the legacy storage type.
+	dualWriteEnabled := storageOpts.StorageType != options.StorageTypeLegacy
+
 	for _, b := range builders {
-		g, err := b.GetAPIGroupInfo(scheme, codecs, optsGetter, dualWrite)
+		mode := b.GetDesiredDualWriterMode(dualWriteEnabled, storageOpts.DualWriterDesiredModes)
+		g, err := b.GetAPIGroupInfo(scheme, codecs, optsGetter, mode)
 		if err != nil {
 			return err
 		}

pkg/apiserver/rest/dualwriter.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 
 	"github.com/grafana/grafana/pkg/infra/kvstore"
-	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/registry/rest"
@@ -82,15 +81,25 @@ type DualWriter interface {
 type DualWriterMode int
 
 const (
-	Mode1 DualWriterMode = iota + 1
+	// Mode0 represents writing to and reading from solely LegacyStorage. This mode is enabled when the
+	// `unifiedStorage` feature flag is not set. All reads and writes are made to LegacyStorage. None are made to Storage.
+	Mode0 DualWriterMode = iota
+	// Mode1 represents writing to and reading from LegacyStorage for all primary functionality while additionally
+	// reading and writing to Storage on a best effort basis for the sake of collecting metrics.
+	Mode1
+	// Mode2 is the dual writing mode that represents writing to LegacyStorage and Storage and reading from LegacyStorage.
 	Mode2
+	// Mode3 represents writing to LegacyStorage and Storage and reading from Storage.
 	Mode3
+	// Mode4 represents writing and reading from Storage.
 	Mode4
 )
 
 // NewDualWriter returns a new DualWriter.
 func NewDualWriter(mode DualWriterMode, legacy LegacyStorage, storage Storage) DualWriter {
 	switch mode {
+	// It is not possible to initialize a mode 0 dual writer. Mode 0 represents
+	// writing to legacy storage without `unifiedStorage` enabled.
 	case Mode1:
 		// read and write only from legacy storage
 		return newDualWriterMode1(legacy, storage)
@@ -129,12 +138,14 @@ func (u *updateWrapper) UpdatedObject(ctx context.Context, oldObj runtime.Object
 func SetDualWritingMode(
 	ctx context.Context,
 	kvs *kvstore.NamespacedKVStore,
-	features featuremgmt.FeatureToggles,
-	entity string,
 	legacy LegacyStorage,
 	storage Storage,
+	entity string,
+	desiredMode DualWriterMode,
 ) (DualWriter, error) {
 	toMode := map[string]DualWriterMode{
+		// It is not possible to initialize a mode 0 dual writer. Mode 0 represents
+		// writing to legacy storage without `unifiedStorage` enabled.
 		"1": Mode1,
 		"2": Mode2,
 		"3": Mode3,
@@ -166,7 +177,7 @@ func SetDualWritingMode(
 	}
 
 	// Desired mode is 2 and current mode is 1
-	if features.IsEnabledGlobally(featuremgmt.FlagDualWritePlaylistsMode2) && (currentMode == Mode1) {
+	if (desiredMode == Mode2) && (currentMode == Mode1) {
 		// This is where we go through the different gates to allow the instance to migrate from mode 1 to mode 2.
 		// There are none between mode 1 and mode 2
 		currentMode = Mode2
@@ -176,17 +187,16 @@ func SetDualWritingMode(
 			return nil, errDualWriterSetCurrentMode
 		}
 	}
-	// #TODO enable this check when we have a flag/config for setting mode 1 as the desired mode
-	// if features.IsEnabledGlobally(featuremgmt.FlagDualWritePlaylistsMode1) && (currentMode == Mode2) {
-	// 	// This is where we go through the different gates to allow the instance to migrate from mode 2 to mode 1.
-	// 	// There are none between mode 1 and mode 2
-	// 	currentMode = Mode1
-
-	// 	err := kvs.Set(ctx, entity, fmt.Sprint(currentMode))
-	// 	if err != nil {
-	// 		return nil, errDualWriterSetCurrentMode
-	// 	}
-	// }
+	if (desiredMode == Mode1) && (currentMode == Mode2) {
+		// This is where we go through the different gates to allow the instance to migrate from mode 2 to mode 1.
+		// There are none between mode 1 and mode 2
+		currentMode = Mode1
+
+		err := kvs.Set(ctx, entity, fmt.Sprint(currentMode))
+		if err != nil {
+			return nil, errDualWriterSetCurrentMode
+		}
+	}
 
 	// 	#TODO add support for other combinations of desired and current modes
 

pkg/apiserver/rest/dualwriter_test.go

@@ -5,33 +5,33 @@ import (
 	"fmt"
 	"testing"
 
+	playlist "github.com/grafana/grafana/pkg/apis/playlist/v0alpha1"
 	"github.com/grafana/grafana/pkg/infra/kvstore"
-	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 )
 
 func TestSetDualWritingMode(t *testing.T) {
 	type testCase struct {
 		name         string
-		features     []any
 		stackID      string
+		desiredMode  DualWriterMode
 		expectedMode DualWriterMode
 	}
 	tests :=
 		// #TODO add test cases for kv store failures. Requires adding support in kvstore test_utils.go
 		[]testCase{
 			{
-				name:         "should return a mode 1 dual writer when no desired mode is set",
-				features:     []any{},
+				name:         "should return a mode 2 dual writer when mode 2 is set as the desired mode",
 				stackID:      "stack-1",
-				expectedMode: Mode1,
+				desiredMode:  Mode2,
+				expectedMode: Mode2,
 			},
 			{
-				name:         "should return a mode 2 dual writer when mode 2 is set as the desired mode",
-				features:     []any{featuremgmt.FlagDualWritePlaylistsMode2},
+				name:         "should return a mode 1 dual writer when mode 1 is set as the desired mode",
 				stackID:      "stack-1",
-				expectedMode: Mode2,
+				desiredMode:  Mode1,
+				expectedMode: Mode1,
 			},
 		}
 
@@ -43,17 +43,14 @@ func TestSetDualWritingMode(t *testing.T) {
 		ls := legacyStoreMock{m, l}
 		us := storageMock{m, s}
 
-		f := featuremgmt.WithFeatures(tt.features...)
 		kvStore := kvstore.WithNamespace(kvstore.NewFakeKVStore(), 0, "storage.dualwriting."+tt.stackID)
 
-		key := "playlist"
-
-		dw, err := SetDualWritingMode(context.Background(), kvStore, f, key, ls, us)
+		dw, err := SetDualWritingMode(context.Background(), kvStore, ls, us, playlist.GROUPRESOURCE, tt.desiredMode)
 		assert.NoError(t, err)
 		assert.Equal(t, tt.expectedMode, dw.Mode())
 
 		// check kv store
-		val, ok, err := kvStore.Get(context.Background(), key)
+		val, ok, err := kvStore.Get(context.Background(), playlist.GROUPRESOURCE)
 		assert.True(t, ok)
 		assert.NoError(t, err)
 		assert.Equal(t, val, fmt.Sprint(tt.expectedMode))

pkg/cmd/grafana/apiserver/server.go

@@ -162,7 +162,8 @@ func (o *APIServerOptions) RunAPIServer(config *genericapiserver.RecommendedConf
 	}
 
 	// Install the API Group+version
-	err = builder.InstallAPIs(grafanaAPIServer.Scheme, grafanaAPIServer.Codecs, server, config.RESTOptionsGetter, o.builders, true)
+	// #TODO figure out how to configure storage type in o.Options.StorageOptions
+	err = builder.InstallAPIs(grafanaAPIServer.Scheme, grafanaAPIServer.Codecs, server, config.RESTOptionsGetter, o.builders, o.Options.StorageOptions)
 	if err != nil {
 		return err
 	}

pkg/registry/apis/dashboard/register.go

@@ -73,6 +73,11 @@ func (b *DashboardsAPIBuilder) GetGroupVersion() schema.GroupVersion {
 	return v0alpha1.DashboardResourceInfo.GroupVersion()
 }
 
+func (b *DashboardsAPIBuilder) GetDesiredDualWriterMode(dualWrite bool, modeMap map[string]grafanarest.DualWriterMode) grafanarest.DualWriterMode {
+	// Add required configuration support in order to enable other modes. For an example, see pkg/registry/apis/playlist/register.go
+	return grafanarest.Mode0
+}
+
 func addKnownTypes(scheme *runtime.Scheme, gv schema.GroupVersion) {
 	scheme.AddKnownTypes(gv,
 		&v0alpha1.Dashboard{},
@@ -109,7 +114,7 @@ func (b *DashboardsAPIBuilder) GetAPIGroupInfo(
 	scheme *runtime.Scheme,
 	codecs serializer.CodecFactory, // pointer?
 	optsGetter generic.RESTOptionsGetter,
-	dualWrite bool,
+	desiredMode grafanarest.DualWriterMode,
 ) (*genericapiserver.APIGroupInfo, error) {
 	apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(v0alpha1.GROUP, scheme, metav1.ParameterCodec, codecs)
 
@@ -135,7 +140,7 @@ func (b *DashboardsAPIBuilder) GetAPIGroupInfo(
 	}
 
 	// Dual writes if a RESTOptionsGetter is provided
-	if dualWrite && optsGetter != nil {
+	if desiredMode != grafanarest.Mode0 && optsGetter != nil {
 		options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: grafanaregistry.GetAttrs}
 		if err := store.CompleteWithOptions(options); err != nil {
 			return nil, err

pkg/registry/apis/dashboardsnapshot/register.go

@@ -22,6 +22,7 @@ import (
 
 	dashboardsnapshot "github.com/grafana/grafana/pkg/apis/dashboardsnapshot/v0alpha1"
 	"github.com/grafana/grafana/pkg/apiserver/builder"
+	grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
 	"github.com/grafana/grafana/pkg/infra/appcontext"
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/infra/log"
@@ -86,6 +87,11 @@ func (b *SnapshotsAPIBuilder) GetGroupVersion() schema.GroupVersion {
 	return resourceInfo.GroupVersion()
 }
 
+func (b *SnapshotsAPIBuilder) GetDesiredDualWriterMode(dualWrite bool, modeMap map[string]grafanarest.DualWriterMode) grafanarest.DualWriterMode {
+	// Add required configuration support in order to enable other modes. For an example, see pkg/registry/apis/playlist/register.go
+	return grafanarest.Mode0
+}
+
 func addKnownTypes(scheme *runtime.Scheme, gv schema.GroupVersion) {
 	scheme.AddKnownTypes(gv,
 		&dashboardsnapshot.DashboardSnapshot{},
@@ -122,7 +128,7 @@ func (b *SnapshotsAPIBuilder) GetAPIGroupInfo(
 	scheme *runtime.Scheme,
 	codecs serializer.CodecFactory, // pointer?
 	optsGetter generic.RESTOptionsGetter,
-	dualWrite bool,
+	_ grafanarest.DualWriterMode,
 ) (*genericapiserver.APIGroupInfo, error) {
 	apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(dashboardsnapshot.GROUP, scheme, metav1.ParameterCodec, codecs)
 	storage := map[string]rest.Storage{}

pkg/registry/apis/datasource/register.go

@@ -23,6 +23,7 @@ import (
 	datasource "github.com/grafana/grafana/pkg/apis/datasource/v0alpha1"
 	query "github.com/grafana/grafana/pkg/apis/query/v0alpha1"
 	"github.com/grafana/grafana/pkg/apiserver/builder"
+	grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/promlib/models"
 	"github.com/grafana/grafana/pkg/registry/apis/query/queryschema"
@@ -151,6 +152,11 @@ func (b *DataSourceAPIBuilder) GetGroupVersion() schema.GroupVersion {
 	return b.connectionResourceInfo.GroupVersion()
 }
 
+func (b *DataSourceAPIBuilder) GetDesiredDualWriterMode(dualWrite bool, modeMap map[string]grafanarest.DualWriterMode) grafanarest.DualWriterMode {
+	// Add required configuration support in order to enable other modes. For an example, see pkg/registry/apis/playlist/register.go
+	return grafanarest.Mode0
+}
+
 func addKnownTypes(scheme *runtime.Scheme, gv schema.GroupVersion) {
 	scheme.AddKnownTypes(gv,
 		&datasource.DataSourceConnection{},
@@ -198,7 +204,7 @@ func (b *DataSourceAPIBuilder) GetAPIGroupInfo(
 	scheme *runtime.Scheme,
 	codecs serializer.CodecFactory, // pointer?
 	_ generic.RESTOptionsGetter,
-	_ bool,
+	_ grafanarest.DualWriterMode,
 ) (*genericapiserver.APIGroupInfo, error) {
 	storage := map[string]rest.Storage{}
 

pkg/registry/apis/example/register.go

@@ -22,6 +22,7 @@ import (
 
 	example "github.com/grafana/grafana/pkg/apis/example/v0alpha1"
 	"github.com/grafana/grafana/pkg/apiserver/builder"
+	grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
 	"github.com/grafana/grafana/pkg/infra/appcontext"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 )
@@ -53,6 +54,11 @@ func (b *TestingAPIBuilder) GetGroupVersion() schema.GroupVersion {
 	return b.gv
 }
 
+func (b *TestingAPIBuilder) GetDesiredDualWriterMode(dualWrite bool, modeMap map[string]grafanarest.DualWriterMode) grafanarest.DualWriterMode {
+	// Add required configuration support in order to enable other modes. For an example, see pkg/registry/apis/playlist/register.go
+	return grafanarest.Mode0
+}
+
 func addKnownTypes(scheme *runtime.Scheme, gv schema.GroupVersion) {
 	scheme.AddKnownTypes(gv,
 		&example.RuntimeInfo{},
@@ -85,7 +91,7 @@ func (b *TestingAPIBuilder) GetAPIGroupInfo(
 	scheme *runtime.Scheme,
 	codecs serializer.CodecFactory, // pointer?
 	_ generic.RESTOptionsGetter,
-	_ bool,
+	_ grafanarest.DualWriterMode,
 ) (*genericapiserver.APIGroupInfo, error) {
 	b.codecs = codecs
 	apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(b.gv.Group, scheme, metav1.ParameterCodec, codecs)

pkg/registry/apis/featuretoggle/register.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1"
 	"github.com/grafana/grafana/pkg/apiserver/builder"
+	grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/setting"
@@ -49,6 +50,11 @@ func (b *FeatureFlagAPIBuilder) GetGroupVersion() schema.GroupVersion {
 	return gv
 }
 
+func (b *FeatureFlagAPIBuilder) GetDesiredDualWriterMode(dualWrite bool, modeMap map[string]grafanarest.DualWriterMode) grafanarest.DualWriterMode {
+	// Add required configuration support in order to enable other modes. For an example, see pkg/registry/apis/playlist/register.go
+	return grafanarest.Mode0
+}
+
 func addKnownTypes(scheme *runtime.Scheme, gv schema.GroupVersion) {
 	scheme.AddKnownTypes(gv,
 		&v0alpha1.Feature{},
@@ -82,7 +88,7 @@ func (b *FeatureFlagAPIBuilder) GetAPIGroupInfo(
 	scheme *runtime.Scheme,
 	codecs serializer.CodecFactory, // pointer?
 	_ generic.RESTOptionsGetter,
-	_ bool,
+	_ grafanarest.DualWriterMode,
 ) (*genericapiserver.APIGroupInfo, error) {
 	apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(v0alpha1.GROUP, scheme, metav1.ParameterCodec, codecs)