Storage: Read desired mode from config instead of feature flags (#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
This commit is contained in:
Arati R
2024-05-31 19:29:59 +02:00
committed by GitHub
parent d3fa52a730
commit 36f42853dd
25 changed files with 300 additions and 72 deletions
+7 -1
View File
@@ -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
+9 -2
View File
@@ -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
}
+25 -15
View File
@@ -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
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
// }
// }
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
+10 -13
View File
@@ -5,8 +5,8 @@ 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"
)
@@ -14,24 +14,24 @@ import (
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))