Use dw dynamic config (#91882)

* Remove kubernetesPlaylists feature_toggle

* Remove unified_storage_mode

* Remove double import

* Read from config instead from feature_toggle

* cover scenario for when unified storage is not defined

* Be temporarily retro compatible with previous feature toggle

* Properly read unified_storage section

* [WIP] Read new format of config

* Fix test

* Fix other tests

* Generate feature flags file

* Use <group>.<resource> schema

* Use <group>.resource format on the FE as well

* Hide UniStore config from Frontend

Signed-off-by: Maicon Costa <maiconscosta@gmail.com>

* unwanted changes

* Use feature toggles in the FE. Enforce FTs are present before enabling dual writing
Co-authored-by: Ryan McKinley <ryantxu@users.noreply.github.com>

* use kubernetes playlists feature toggle on the FE

* Remove unwanted code

* Remove configs from the FE

* Remove commented code

* Add more explicit example

---------

Signed-off-by: Maicon Costa <maiconscosta@gmail.com>
Co-authored-by: Maicon Costa <maiconscosta@gmail.com>
This commit is contained in:
Leonor Oliveira 2024-08-30 10:59:42 +01:00 committed by GitHub
parent f8765087b5
commit 2e451b2ed7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 239 additions and 58 deletions

View File

@ -12,7 +12,7 @@ import (
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/apis/playlist/v0alpha1"
playlistalpha1 "github.com/grafana/grafana/pkg/apis/playlist/v0alpha1"
"github.com/grafana/grafana/pkg/middleware"
internalplaylist "github.com/grafana/grafana/pkg/registry/apis/playlist"
grafanaapiserver "github.com/grafana/grafana/pkg/services/apiserver"
@ -27,6 +27,7 @@ import (
func (hs *HTTPServer) registerPlaylistAPI(apiRoute routing.RouteRegister) {
// Register the actual handlers
apiRoute.Group("/playlists", func(playlistRoute routing.RouteRegister) {
// TODO: remove kubernetesPlaylists feature flag
if hs.Features.IsEnabledGlobally(featuremgmt.FlagKubernetesPlaylists) {
// Use k8s client to implement legacy API
handler := newPlaylistK8sHandler(hs)
@ -330,7 +331,7 @@ type playlistK8sHandler struct {
func newPlaylistK8sHandler(hs *HTTPServer) *playlistK8sHandler {
return &playlistK8sHandler{
gvr: v0alpha1.PlaylistResourceInfo.GroupVersionResource(),
gvr: playlistalpha1.PlaylistResourceInfo.GroupVersionResource(),
namespacer: request.GetNamespaceMapper(hs.Cfg),
clientConfigProvider: hs.clientConfigProvider,
}

View File

@ -62,8 +62,11 @@ For kubectl to work, grafana needs to run over https. To simplify development,
app_mode = development
[feature_toggles]
grafanaAPIServerEnsureKubectlAccess = true
grafanaAPIServerEnsureKubectlAccess = true
kubernetesPlaylists = true
[unified_storage.playlists.playlist.grafana.app]
DualWriterMode = 2
```
This will create a development kubeconfig and start a parallel ssl listener. It can be registered by
@ -90,4 +93,4 @@ The folder structure aims to follow the patterns established in standard (https:
* [pkg/apis](/pkg/apis) - where API resource types are defined. this is based on the structure of the [sample-apiserver](https://github.com/kubernetes/sample-apiserver/tree/master/pkg/apis)
* [hack/update-codegen.sh](/hack#kubernetes-hack-alert) - this script is used to run [k8s codegen](https://github.com/kubernetes/code-generator/), which generates the code that is used by the API server to handle the types defined in `pkg/apis`. it is based on the [update-codegen.sh from sample-apiserver](https://github.com/kubernetes/sample-apiserver/blob/master/hack/update-codegen.sh)
* [pkg/registry/apis](/pkg/registry/apis) - where all of the types in `pkg/apis` are registered with the API server by implementing the [builder](/pkg/services/apiserver/builder/common.go#L18) interface. this pattern is unique to grafana, and is needed to support using wire dependencies in legacy storage implementations. this is separated from `pkg/apis` to avoid issues with k8s codegen.
* [pkg/cmd/grafana/apiserver](/pkg/cmd/grafana/apiserver) - this is where the apiserver is configured for the `grafana apiserver` CLI command, which can be used to launch standalone API servers. this will eventually be merged with the config in `pkg/services/apiserver` to reduce duplication.
* [pkg/cmd/grafana/apiserver](/pkg/cmd/grafana/apiserver) - this is where the apiserver is configured for the `grafana apiserver` CLI command, which can be used to launch standalone API servers. this will eventually be merged with the config in `pkg/services/apiserver` to reduce duplication.

View File

@ -165,7 +165,11 @@ func InstallAPIs(
// Get the option from custom.ini/command line
// when missing this will default to mode zero (legacy only)
mode := storageOpts.DualWriterDesiredModes[key]
var mode = grafanarest.DualWriterMode(0)
resourceConfig, resourceExists := storageOpts.UnifiedStorageConfig[key]
if resourceExists {
mode = resourceConfig.DualWriterMode
}
// TODO: inherited context from main Grafana process
ctx := context.Background()

View File

@ -6,8 +6,6 @@ import (
"path/filepath"
"strconv"
playlist "github.com/grafana/grafana/pkg/apis/playlist/v0alpha1"
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
"github.com/grafana/grafana/pkg/services/apiserver/options"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/setting"
@ -55,20 +53,18 @@ func applyGrafanaConfig(cfg *setting.Cfg, features featuremgmt.FeatureToggles, o
o.StorageOptions.StorageType = options.StorageType(apiserverCfg.Key("storage_type").MustString(string(options.StorageTypeLegacy)))
o.StorageOptions.DataPath = apiserverCfg.Key("storage_path").MustString(filepath.Join(cfg.DataPath, "grafana-apiserver"))
o.StorageOptions.Address = apiserverCfg.Key("address").MustString(o.StorageOptions.Address)
o.StorageOptions.DualWriterDesiredModes = map[string]grafanarest.DualWriterMode{
// TODO: use the new config from HGAPI after https://github.com/grafana/hosted-grafana/pull/5707
playlist.GROUPRESOURCE: 2,
}
// unified storage configs look like
// [unified_storage.<group>.<resource>]
// config = <value>
unifiedStorageCfg := cfg.UnifiedStorage
o.StorageOptions.UnifiedStorageConfig = unifiedStorageCfg
o.StorageOptions.DualWriterDataSyncJobEnabled = map[string]bool{
// TODO: This will be enabled later, when we get a dedicated config section for unified_storage
// playlist.RESOURCE + "." + playlist.GROUP: true,
}
// TODO: ensure backwards compatibility with production
// remove this after changing the unified_storage_mode key format in HGAPI
o.StorageOptions.DualWriterDesiredModes[playlist.RESOURCE+"."+playlist.GROUP] = o.StorageOptions.DualWriterDesiredModes[playlist.GROUPRESOURCE]
o.ExtraOptions.DevMode = features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerEnsureKubectlAccess)
o.ExtraOptions.ExternalAddress = host
o.ExtraOptions.APIURL = apiURL

View File

@ -8,7 +8,8 @@ import (
genericapiserver "k8s.io/apiserver/pkg/server"
"k8s.io/apiserver/pkg/server/options"
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/setting"
)
type StorageType string
@ -25,7 +26,7 @@ type StorageOptions struct {
StorageType StorageType
DataPath string
Address string
DualWriterDesiredModes map[string]grafanarest.DualWriterMode
UnifiedStorageConfig map[string]setting.UnifiedStorageConfig
DualWriterDataSyncJobEnabled map[string]bool
}
@ -61,3 +62,21 @@ func (o *StorageOptions) ApplyTo(serverConfig *genericapiserver.RecommendedConfi
// TODO: move storage setup here
return nil
}
// EnforceFeatureToggleAfterMode1 makes sure there is a feature toggle set for resources with DualWriterMode > 1.
// This is needed to ensure that we use the K8s client before enabling dual writing.
func (o *StorageOptions) EnforceFeatureToggleAfterMode1(features featuremgmt.FeatureToggles) error {
if o.StorageType != StorageTypeLegacy {
for rg, s := range o.UnifiedStorageConfig {
if s.DualWriterMode > 1 {
switch rg {
case "playlists.playlist.grafana.app":
if !features.IsEnabledGlobally(featuremgmt.FlagKubernetesPlaylists) {
return fmt.Errorf("feature toggle FlagKubernetesPlaylists to be set")
}
}
}
}
}
return nil
}

View File

@ -0,0 +1,64 @@
package options
import (
"testing"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/assert"
)
func TestStorageOptions_CheckFeatureToggle(t *testing.T) {
tests := []struct {
name string
StorageType StorageType
UnifiedStorageConfig map[string]setting.UnifiedStorageConfig
features any
wantErr bool
}{
{
name: "with legacy storage",
StorageType: StorageTypeLegacy,
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{"playlists.playlist.grafana.app": {DualWriterMode: 2}},
features: featuremgmt.WithFeatures(),
},
{
name: "with unified storage and without config for resource",
StorageType: StorageTypeUnified,
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{},
features: featuremgmt.WithFeatures(),
},
{
name: "with unified storage, mode > 1 and with toggle for resource",
StorageType: StorageTypeUnified,
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{"playlists.playlist.grafana.app": {DualWriterMode: 2}},
features: featuremgmt.WithFeatures(featuremgmt.FlagKubernetesPlaylists),
},
{
name: "with unified storage, mode > 1 and without toggle for resource",
StorageType: StorageTypeUnified,
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{"playlists.playlist.grafana.app": {DualWriterMode: 2}},
features: featuremgmt.WithFeatures(),
wantErr: true,
},
{
name: "with unified storage and mode = 1",
StorageType: StorageTypeUnified,
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{"playlists.playlist.grafana.app": {DualWriterMode: 1}},
features: featuremgmt.WithFeatures(),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
o := &StorageOptions{
StorageType: tt.StorageType,
UnifiedStorageConfig: tt.UnifiedStorageConfig,
}
err := o.EnforceFeatureToggleAfterMode1(tt.features.(featuremgmt.FeatureToggles))
if tt.wantErr {
return
}
assert.NoError(t, err)
})
}
}

View File

@ -263,6 +263,14 @@ func (s *service) start(ctx context.Context) error {
return errs[0]
}
// This will check that required feature toggles are enabled for more advanced storage modes
// Any required preconditions should be hardcoded here
if o.StorageOptions != nil {
if err := o.StorageOptions.EnforceFeatureToggleAfterMode1(s.features); err != nil {
return err
}
}
serverConfig := genericapiserver.NewRecommendedConfig(Codecs)
if err := o.ApplyTo(serverConfig); err != nil {
return err

View File

@ -29,6 +29,7 @@ import (
"github.com/grafana/grafana-plugin-sdk-go/backend/gtime"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/apiserver/rest"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/util/osutil"
@ -523,6 +524,13 @@ type Cfg struct {
//Short Links
ShortLinkExpiration int
// Unified Storage
UnifiedStorage map[string]UnifiedStorageConfig
}
type UnifiedStorageConfig struct {
DualWriterMode rest.DualWriterMode
}
type InstallPlugin struct {
@ -1324,6 +1332,9 @@ func (cfg *Cfg) parseINIFile(iniFile *ini.File) error {
cfg.ScopesListScopesURL = scopesSection.Key("list_scopes_endpoint").MustString("")
cfg.ScopesListDashboardsURL = scopesSection.Key("list_dashboards_endpoint").MustString("")
// read unifed storage config
cfg.setUnifiedStorageConfig()
return nil
}

View File

@ -0,0 +1,31 @@
package setting
import (
"strings"
"github.com/grafana/grafana/pkg/apiserver/rest"
)
// read storage configs from ini file. They look like:
// [unified_storage.<group>.<resource>]
// <field> = <value>
// e.g.
// [unified_storage.playlists.playlist.grafana.app]
// dualWriterMode = 2
func (cfg *Cfg) setUnifiedStorageConfig() {
storageConfig := make(map[string]UnifiedStorageConfig)
sections := cfg.Raw.Sections()
for _, section := range sections {
sectionName := section.Name()
if !strings.HasPrefix(sectionName, "unified_storage.") {
continue
}
// the resource name is the part after the first dot
resourceName := strings.SplitAfterN(sectionName, ".", 2)[1]
// parse dualWriter modes from the section
dualWriterMode := section.Key("dualWriterMode").MustInt(0)
storageConfig[resourceName] = UnifiedStorageConfig{DualWriterMode: rest.DualWriterMode(dualWriterMode)}
}
cfg.UnifiedStorage = storageConfig
}

View File

@ -0,0 +1,28 @@
package setting
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestCfg_setUnifiedStorageConfig(t *testing.T) {
t.Run("read unified_storage configs", func(t *testing.T) {
cfg := NewCfg()
err := cfg.Load(CommandLineArgs{HomePath: "../../", Config: "../../conf/defaults.ini"})
assert.NoError(t, err)
s, err := cfg.Raw.NewSection("unified_storage.playlists.playlist.grafana.app")
assert.NoError(t, err)
_, err = s.NewKey("dualWriterMode", "2")
assert.NoError(t, err)
cfg.setUnifiedStorageConfig()
value, exists := cfg.UnifiedStorage["playlists.playlist.grafana.app"]
assert.Equal(t, exists, true)
assert.Equal(t, value, UnifiedStorageConfig{DualWriterMode: 2})
})
}

View File

@ -18,6 +18,7 @@ import (
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/playlist"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/tests/apis"
"github.com/grafana/grafana/pkg/tests/testinfra"
"github.com/grafana/grafana/pkg/tests/testsuite"
@ -92,12 +93,14 @@ func TestIntegrationPlaylist(t *testing.T) {
AppModeProduction: true,
DisableAnonymous: true,
APIServerStorageType: "file", // write the files to disk
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
playlistv0alpha1.GROUPRESOURCE: {
DualWriterMode: grafanarest.Mode0,
},
},
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesPlaylists, // Required so that legacy calls are also written
},
DualWriterDesiredModes: map[string]grafanarest.DualWriterMode{
playlistv0alpha1.GROUPRESOURCE: grafanarest.Mode0,
},
}))
})
@ -106,12 +109,14 @@ func TestIntegrationPlaylist(t *testing.T) {
AppModeProduction: true,
DisableAnonymous: true,
APIServerStorageType: "file", // write the files to disk
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
playlistv0alpha1.GROUPRESOURCE: {
DualWriterMode: grafanarest.Mode1,
},
},
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesPlaylists, // Required so that legacy calls are also written
},
DualWriterDesiredModes: map[string]grafanarest.DualWriterMode{
playlistv0alpha1.GROUPRESOURCE: grafanarest.Mode1,
},
}))
})
@ -120,12 +125,14 @@ func TestIntegrationPlaylist(t *testing.T) {
AppModeProduction: true,
DisableAnonymous: true,
APIServerStorageType: "file", // write the files to disk
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
playlistv0alpha1.GROUPRESOURCE: {
DualWriterMode: grafanarest.Mode2,
},
},
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesPlaylists, // Required so that legacy calls are also written
},
DualWriterDesiredModes: map[string]grafanarest.DualWriterMode{
playlistv0alpha1.GROUPRESOURCE: grafanarest.Mode2,
},
}))
})
@ -134,12 +141,14 @@ func TestIntegrationPlaylist(t *testing.T) {
AppModeProduction: true,
DisableAnonymous: true,
APIServerStorageType: "file", // write the files to disk
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
playlistv0alpha1.GROUPRESOURCE: {
DualWriterMode: grafanarest.Mode3,
},
},
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesPlaylists, // Required so that legacy calls are also written
},
DualWriterDesiredModes: map[string]grafanarest.DualWriterMode{
playlistv0alpha1.GROUPRESOURCE: grafanarest.Mode3,
},
}))
})
@ -151,8 +160,10 @@ func TestIntegrationPlaylist(t *testing.T) {
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesPlaylists, // Required so that legacy calls are also written
},
DualWriterDesiredModes: map[string]grafanarest.DualWriterMode{
playlistv0alpha1.GROUPRESOURCE: grafanarest.Mode0,
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
playlistv0alpha1.GROUPRESOURCE: {
DualWriterMode: grafanarest.Mode0,
},
},
}))
})
@ -162,12 +173,7 @@ func TestIntegrationPlaylist(t *testing.T) {
AppModeProduction: false, // required for unified storage
DisableAnonymous: true,
APIServerStorageType: "unified", // use the entity api tables
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesPlaylists, // Required so that legacy calls are also written
},
DualWriterDesiredModes: map[string]grafanarest.DualWriterMode{
playlistv0alpha1.GROUPRESOURCE: grafanarest.Mode1,
},
EnableFeatureToggles: []string{},
}))
})
@ -179,8 +185,10 @@ func TestIntegrationPlaylist(t *testing.T) {
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesPlaylists, // Required so that legacy calls are also written
},
DualWriterDesiredModes: map[string]grafanarest.DualWriterMode{
playlistv0alpha1.GROUPRESOURCE: grafanarest.Mode2,
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
playlistv0alpha1.GROUPRESOURCE: {
DualWriterMode: grafanarest.Mode2,
},
},
}))
})
@ -193,8 +201,10 @@ func TestIntegrationPlaylist(t *testing.T) {
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesPlaylists, // Required so that legacy calls are also written
},
DualWriterDesiredModes: map[string]grafanarest.DualWriterMode{
playlistv0alpha1.GROUPRESOURCE: grafanarest.Mode3,
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
playlistv0alpha1.GROUPRESOURCE: {
DualWriterMode: grafanarest.Mode3,
},
},
}))
})
@ -207,12 +217,14 @@ func TestIntegrationPlaylist(t *testing.T) {
AppModeProduction: true,
DisableAnonymous: true,
APIServerStorageType: "etcd", // requires etcd running on localhost:2379
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
playlistv0alpha1.GROUPRESOURCE: {
DualWriterMode: grafanarest.Mode0,
},
},
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesPlaylists, // Required so that legacy calls are also written
},
DualWriterDesiredModes: map[string]grafanarest.DualWriterMode{
playlistv0alpha1.GROUPRESOURCE: grafanarest.Mode0,
},
})
// Clear the collection before starting (etcd)
@ -237,8 +249,10 @@ func TestIntegrationPlaylist(t *testing.T) {
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesPlaylists, // Required so that legacy calls are also written
},
DualWriterDesiredModes: map[string]grafanarest.DualWriterMode{
playlistv0alpha1.GROUPRESOURCE: grafanarest.Mode1,
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
playlistv0alpha1.GROUPRESOURCE: {
DualWriterMode: grafanarest.Mode1,
},
},
})
@ -264,8 +278,10 @@ func TestIntegrationPlaylist(t *testing.T) {
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesPlaylists, // Required so that legacy calls are also written
},
DualWriterDesiredModes: map[string]grafanarest.DualWriterMode{
playlistv0alpha1.GROUPRESOURCE: grafanarest.Mode2,
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
playlistv0alpha1.GROUPRESOURCE: {
DualWriterMode: grafanarest.Mode2,
},
},
})
@ -291,8 +307,10 @@ func TestIntegrationPlaylist(t *testing.T) {
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesPlaylists, // Required so that legacy calls are also written
},
DualWriterDesiredModes: map[string]grafanarest.DualWriterMode{
playlistv0alpha1.GROUPRESOURCE: grafanarest.Mode3,
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
playlistv0alpha1.GROUPRESOURCE: {
DualWriterMode: grafanarest.Mode3,
},
},
})

View File

@ -17,7 +17,6 @@ import (
"gopkg.in/ini.v1"
"github.com/grafana/grafana/pkg/api"
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
"github.com/grafana/grafana/pkg/extensions"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/fs"
@ -390,12 +389,11 @@ func CreateGrafDir(t *testing.T, opts ...GrafanaOpts) (string, string) {
_, err = grafanaComSection.NewKey("api_url", o.GrafanaComAPIURL)
require.NoError(t, err)
}
if o.DualWriterDesiredModes != nil {
unifiedStorageMode, err := getOrCreateSection("unified_storage_mode")
require.NoError(t, err)
for k, v := range o.DualWriterDesiredModes {
_, err = unifiedStorageMode.NewKey(k, fmt.Sprint(v))
if o.UnifiedStorageConfig != nil {
for k, v := range o.UnifiedStorageConfig {
section, err := getOrCreateSection(fmt.Sprintf("unified_storage.%s", k))
require.NoError(t, err)
_, err = section.NewKey("dualWriterMode", fmt.Sprintf("%d", v.DualWriterMode))
require.NoError(t, err)
}
}
@ -446,7 +444,7 @@ type GrafanaOpts struct {
QueryRetries int64
APIServerStorageType string
GrafanaComAPIURL string
DualWriterDesiredModes map[string]grafanarest.DualWriterMode
UnifiedStorageConfig map[string]setting.UnifiedStorageConfig
}
func CreateUser(t *testing.T, store db.DB, cfg *setting.Cfg, cmd user.CreateUserCommand) *user.User {