diff --git a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go index 0b7ffa04ea4..0fa8b7d7115 100644 --- a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go +++ b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go @@ -224,7 +224,7 @@ func (a *dashboardSqlAccess) CountResources(ctx context.Context, opts MigrateOpt case "folder.grafana.app/folders": summary := &resourcepb.BulkResponse_Summary{} summary.Group = folders.GROUP - summary.Group = folders.RESOURCE + summary.Resource = folders.RESOURCE _, err = sess.SQL("SELECT COUNT(*) FROM "+sql.Table("dashboard")+ " WHERE is_folder=TRUE AND org_id=?", orgId).Get(&summary.Count) rsp.Summary = append(rsp.Summary, summary) diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index aafc9167ce4..1155a4ead7f 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -637,6 +637,8 @@ type UnifiedStorageConfig struct { // EnableMigration indicates whether migration is enabled for the resource. // If not set, will use the default from MigratedUnifiedResources. EnableMigration bool + // AutoMigrationThreshold is the threshold below which a resource is automatically migrated. + AutoMigrationThreshold int } type InstallPlugin struct { diff --git a/pkg/setting/setting_unified_storage.go b/pkg/setting/setting_unified_storage.go index 8086151c40a..21a3f455993 100644 --- a/pkg/setting/setting_unified_storage.go +++ b/pkg/setting/setting_unified_storage.go @@ -8,6 +8,10 @@ import ( "github.com/grafana/grafana/pkg/util/osutil" ) +// DefaultAutoMigrationThreshold is the default threshold for auto migration switching. +// If a resource has entries at or below this count, it will be migrated. +const DefaultAutoMigrationThreshold = 10 + const ( PlaylistResource = "playlists.playlist.grafana.app" FolderResource = "folders.folder.grafana.app" @@ -21,6 +25,13 @@ var MigratedUnifiedResources = map[string]bool{ DashboardResource: false, } +// AutoMigratedUnifiedResources maps resources that support auto-migration +// TODO: remove this before Grafana 13 GA: https://github.com/grafana/search-and-storage-team/issues/613 +var AutoMigratedUnifiedResources = map[string]bool{ + FolderResource: true, + DashboardResource: true, +} + // read storage configs from ini file. They look like: // [unified_storage..] // = @@ -59,6 +70,13 @@ func (cfg *Cfg) setUnifiedStorageConfig() { enableMigration = section.Key("enableMigration").MustBool(MigratedUnifiedResources[resourceName]) } + // parse autoMigrationThreshold from resource section + autoMigrationThreshold := 0 + autoMigrate := AutoMigratedUnifiedResources[resourceName] + if autoMigrate { + autoMigrationThreshold = section.Key("autoMigrationThreshold").MustInt(DefaultAutoMigrationThreshold) + } + storageConfig[resourceName] = UnifiedStorageConfig{ DualWriterMode: rest.DualWriterMode(dualWriterMode), DualWriterPeriodicDataSyncJobEnabled: dualWriterPeriodicDataSyncJobEnabled, @@ -66,6 +84,7 @@ func (cfg *Cfg) setUnifiedStorageConfig() { DataSyncerRecordsLimit: dataSyncerRecordsLimit, DataSyncerInterval: dataSyncerInterval, EnableMigration: enableMigration, + AutoMigrationThreshold: autoMigrationThreshold, } } cfg.UnifiedStorage = storageConfig @@ -73,13 +92,13 @@ func (cfg *Cfg) setUnifiedStorageConfig() { // Set indexer config for unified storage section := cfg.Raw.Section("unified_storage") cfg.DisableDataMigrations = section.Key("disable_data_migrations").MustBool(false) - if !cfg.DisableDataMigrations && cfg.getUnifiedStorageType() == "unified" { + if !cfg.DisableDataMigrations && cfg.UnifiedStorageType() == "unified" { // Helper log to find instances running migrations in the future cfg.Logger.Info("Unified migration configs enforced") cfg.enforceMigrationToUnifiedConfigs() } else { // Helper log to find instances disabling migration - cfg.Logger.Info("Unified migration configs enforcement disabled", "storage_type", cfg.getUnifiedStorageType(), "disable_data_migrations", cfg.DisableDataMigrations) + cfg.Logger.Info("Unified migration configs enforcement disabled", "storage_type", cfg.UnifiedStorageType(), "disable_data_migrations", cfg.DisableDataMigrations) } cfg.EnableSearch = section.Key("enable_search").MustBool(false) cfg.MaxPageSizeBytes = section.Key("max_page_size_bytes").MustInt(0) @@ -147,14 +166,15 @@ func (cfg *Cfg) enforceMigrationToUnifiedConfigs() { DualWriterMode: 5, DualWriterMigrationDataSyncDisabled: true, EnableMigration: true, + AutoMigrationThreshold: resourceCfg.AutoMigrationThreshold, } } } -// getUnifiedStorageType returns the configured storage type without creating or mutating keys. +// UnifiedStorageType returns the configured storage type without creating or mutating keys. // Precedence: env > ini > default ("unified"). // Used to decide unified storage behavior early without side effects. -func (cfg *Cfg) getUnifiedStorageType() string { +func (cfg *Cfg) UnifiedStorageType() string { const ( grafanaAPIServerSectionName = "grafana-apiserver" storageTypeKeyName = "storage_type" @@ -168,3 +188,23 @@ func (cfg *Cfg) getUnifiedStorageType() string { } return defaultStorageType } + +// UnifiedStorageConfig returns the UnifiedStorageConfig for a resource. +func (cfg *Cfg) UnifiedStorageConfig(resource string) UnifiedStorageConfig { + if cfg.UnifiedStorage == nil { + return UnifiedStorageConfig{} + } + return cfg.UnifiedStorage[resource] +} + +// EnableMode5 enables migration and sets mode 5 for a resource. +func (cfg *Cfg) EnableMode5(resource string) { + if cfg.UnifiedStorage == nil { + cfg.UnifiedStorage = make(map[string]UnifiedStorageConfig) + } + config := cfg.UnifiedStorage[resource] + config.DualWriterMode = rest.Mode5 + config.DualWriterMigrationDataSyncDisabled = true + config.EnableMigration = true + cfg.UnifiedStorage[resource] = config +} diff --git a/pkg/setting/setting_unified_storage_test.go b/pkg/setting/setting_unified_storage_test.go index c112dc85962..58f0fe56a60 100644 --- a/pkg/setting/setting_unified_storage_test.go +++ b/pkg/setting/setting_unified_storage_test.go @@ -43,10 +43,16 @@ func TestCfg_setUnifiedStorageConfig(t *testing.T) { } assert.Equal(t, exists, true, migratedResource) + expectedThreshold := 0 + if AutoMigratedUnifiedResources[migratedResource] { + expectedThreshold = DefaultAutoMigrationThreshold + } + assert.Equal(t, UnifiedStorageConfig{ DualWriterMode: 5, DualWriterMigrationDataSyncDisabled: true, EnableMigration: isEnabled, + AutoMigrationThreshold: expectedThreshold, }, resourceCfg, migratedResource) } } @@ -71,6 +77,7 @@ func TestCfg_setUnifiedStorageConfig(t *testing.T) { DualWriterPeriodicDataSyncJobEnabled: true, DataSyncerRecordsLimit: 1001, DataSyncerInterval: time.Minute * 10, + AutoMigrationThreshold: 0, }) validateMigratedResources(false) diff --git a/pkg/storage/unified/migrations/migrator_test.go b/pkg/storage/unified/migrations/migrator_test.go index 8552092d066..cfd909a9255 100644 --- a/pkg/storage/unified/migrations/migrator_test.go +++ b/pkg/storage/unified/migrations/migrator_test.go @@ -214,8 +214,18 @@ func runMigrationTestSuite(t *testing.T, testCases []resourceMigratorTestCase) { for _, state := range testStates { t.Run(state.tc.name(), func(t *testing.T) { - // Verify resources now exist in unified storage after migration - state.tc.verify(t, helper, true) + shouldExist := true + for _, gvr := range state.tc.resources() { + resourceKey := fmt.Sprintf("%s.%s", gvr.Resource, gvr.Group) + // Resources exist if they're either: + // 1. In MigratedUnifiedResources (enabled by default), OR + // 2. In AutoMigratedUnifiedResources (auto-migrated because count is below threshold) + if !setting.MigratedUnifiedResources[resourceKey] && !setting.AutoMigratedUnifiedResources[resourceKey] { + shouldExist = false + break + } + } + state.tc.verify(t, helper, shouldExist) }) } @@ -270,7 +280,7 @@ const ( var migrationIDsToDefault = map[string]bool{ playlistsID: true, - foldersAndDashboardsID: false, + foldersAndDashboardsID: true, // Auto-migrated when resource count is below threshold } func verifyRegisteredMigrations(t *testing.T, helper *apis.K8sTestHelper, onlyDefault bool, optOut bool) { diff --git a/pkg/storage/unified/migrations/resource_migration.go b/pkg/storage/unified/migrations/resource_migration.go index 5bbed9c5722..d551ae32bde 100644 --- a/pkg/storage/unified/migrations/resource_migration.go +++ b/pkg/storage/unified/migrations/resource_migration.go @@ -10,9 +10,11 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacy" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/grafana/grafana/pkg/storage/unified/resourcepb" "github.com/grafana/grafana/pkg/util/xorm" + "github.com/grafana/grafana/pkg/util/xorm/core" "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -31,6 +33,20 @@ type ResourceMigration struct { migrationID string validators []Validator // Optional: custom validation logic for this migration log log.Logger + cfg *setting.Cfg + autoMigrate bool // If true, auto-migrate resource if count is below threshold + hadErrors bool // Tracks if errors occurred during migration (used with ignoreErrors) +} + +// ResourceMigrationOption is a functional option for configuring ResourceMigration. +type ResourceMigrationOption func(*ResourceMigration) + +// WithAutoMigrate configures the migration to auto-migrate resource if count is below threshold. +func WithAutoMigrate(cfg *setting.Cfg) ResourceMigrationOption { + return func(m *ResourceMigration) { + m.cfg = cfg + m.autoMigrate = true + } } // NewResourceMigration creates a new migration for the specified resources. @@ -39,14 +55,24 @@ func NewResourceMigration( resources []schema.GroupResource, migrationID string, validators []Validator, + opts ...ResourceMigrationOption, ) *ResourceMigration { - return &ResourceMigration{ + m := &ResourceMigration{ migrator: migrator, resources: resources, migrationID: migrationID, validators: validators, log: log.New("storage.unified.resource_migration." + migrationID), } + for _, opt := range opts { + opt(m) + } + return m +} + +func (m *ResourceMigration) SkipMigrationLog() bool { + // Skip populating the log table if auto-migrate is enabled and errors occurred + return m.autoMigrate && m.hadErrors } var _ migrator.CodeMigration = (*ResourceMigration)(nil) @@ -57,7 +83,23 @@ func (m *ResourceMigration) SQL(_ migrator.Dialect) string { } // Exec implements migrator.CodeMigration interface. Executes the migration across all organizations. -func (m *ResourceMigration) Exec(sess *xorm.Session, mg *migrator.Migrator) error { +func (m *ResourceMigration) Exec(sess *xorm.Session, mg *migrator.Migrator) (err error) { + // Track any errors that occur during migration + defer func() { + if err != nil { + if m.autoMigrate { + m.log.Warn( + `[WARN] Resource migration failed and is currently skipped. +This migration will be enforced in the next major Grafana release, where failures will block startup or resource loading. + +This warning is intended to help you detect and report issues early. +Please investigate the failure and report it to the Grafana team so it can be addressed before the next major release.`, + "error", err) + } + m.hadErrors = true + } + }() + ctx := context.Background() orgs, err := m.getAllOrgs(sess) @@ -75,7 +117,8 @@ func (m *ResourceMigration) Exec(sess *xorm.Session, mg *migrator.Migrator) erro if mg.Dialect.DriverName() == migrator.SQLite { // reuse transaction in SQLite to avoid "database is locked" errors - tx, err := sess.Tx() + var tx *core.Tx + tx, err = sess.Tx() if err != nil { m.log.Error("Failed to get transaction from session", "error", err) return fmt.Errorf("failed to get transaction: %w", err) @@ -85,12 +128,22 @@ func (m *ResourceMigration) Exec(sess *xorm.Session, mg *migrator.Migrator) erro } for _, org := range orgs { - if err := m.migrateOrg(ctx, sess, org); err != nil { + if err = m.migrateOrg(ctx, sess, org); err != nil { return err } } + // Auto-enable mode 5 for resources after successful migration + // TODO: remove this before Grafana 13 GA: https://github.com/grafana/search-and-storage-team/issues/613 + if m.autoMigrate { + for _, gr := range m.resources { + m.log.Info("Auto-enabling mode 5 for resource", "resource", gr.Resource+"."+gr.Group) + m.cfg.EnableMode5(gr.Resource + "." + gr.Group) + } + } + m.log.Info("Migration completed successfully for all organizations", "org_count", len(orgs)) + return nil } diff --git a/pkg/storage/unified/migrations/resources.go b/pkg/storage/unified/migrations/resources.go index ea32bb54322..debbb270642 100644 --- a/pkg/storage/unified/migrations/resources.go +++ b/pkg/storage/unified/migrations/resources.go @@ -1,11 +1,13 @@ package migrations import ( + "context" "fmt" v1beta1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" playlists "github.com/grafana/grafana/apps/playlist/pkg/apis/playlist/v0alpha1" + "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacy" sqlstoremigrator "github.com/grafana/grafana/pkg/services/sqlstore/migrator" "github.com/grafana/grafana/pkg/setting" @@ -14,69 +16,70 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ) -type ResourceDefinition struct { - GroupResource schema.GroupResource - MigratorFunc string // Name of the method: "MigrateFolders", "MigrateDashboards", etc. +type resourceDefinition struct { + groupResource schema.GroupResource + migratorFunc string // Name of the method: "MigrateFolders", "MigrateDashboards", etc. } type migrationDefinition struct { name string + migrationID string // The ID stored in the migration log table (e.g., "playlists migration") resources []string - registerFunc func(mg *sqlstoremigrator.Migrator, migrator UnifiedMigrator, client resource.ResourceClient) + registerFunc func(mg *sqlstoremigrator.Migrator, migrator UnifiedMigrator, client resource.ResourceClient, opts ...ResourceMigrationOption) } -var resourceRegistry = []ResourceDefinition{ +var resourceRegistry = []resourceDefinition{ { - GroupResource: schema.GroupResource{Group: folders.GROUP, Resource: folders.RESOURCE}, - MigratorFunc: "MigrateFolders", + groupResource: schema.GroupResource{Group: folders.GROUP, Resource: folders.RESOURCE}, + migratorFunc: "MigrateFolders", }, { - GroupResource: schema.GroupResource{Group: v1beta1.GROUP, Resource: v1beta1.LIBRARY_PANEL_RESOURCE}, - MigratorFunc: "MigrateLibraryPanels", + groupResource: schema.GroupResource{Group: v1beta1.GROUP, Resource: v1beta1.LIBRARY_PANEL_RESOURCE}, + migratorFunc: "MigrateLibraryPanels", }, { - GroupResource: schema.GroupResource{Group: v1beta1.GROUP, Resource: v1beta1.DASHBOARD_RESOURCE}, - MigratorFunc: "MigrateDashboards", + groupResource: schema.GroupResource{Group: v1beta1.GROUP, Resource: v1beta1.DASHBOARD_RESOURCE}, + migratorFunc: "MigrateDashboards", }, { - GroupResource: schema.GroupResource{Group: playlists.APIGroup, Resource: "playlists"}, - MigratorFunc: "MigratePlaylists", + groupResource: schema.GroupResource{Group: playlists.APIGroup, Resource: "playlists"}, + migratorFunc: "MigratePlaylists", }, } var migrationRegistry = []migrationDefinition{ { name: "playlists", + migrationID: "playlists migration", resources: []string{setting.PlaylistResource}, registerFunc: registerPlaylistMigration, }, { name: "folders and dashboards", + migrationID: "folders and dashboards migration", resources: []string{setting.FolderResource, setting.DashboardResource}, registerFunc: registerDashboardAndFolderMigration, }, } -func registerMigrations(cfg *setting.Cfg, mg *sqlstoremigrator.Migrator, migrator UnifiedMigrator, client resource.ResourceClient) error { +func registerMigrations(ctx context.Context, + cfg *setting.Cfg, + mg *sqlstoremigrator.Migrator, + migrator UnifiedMigrator, + client resource.ResourceClient, + sqlStore db.DB, +) error { for _, migration := range migrationRegistry { - var ( - hasValue bool - allEnabled bool - ) - - for _, res := range migration.resources { - enabled := cfg.UnifiedStorage[res].EnableMigration - if !hasValue { - allEnabled = enabled - hasValue = true - continue - } - if enabled != allEnabled { - return fmt.Errorf("cannot migrate resources separately: %v migration must be either all enabled or all disabled", migration.resources) - } + if shouldAutoMigrate(ctx, migration, cfg, sqlStore) { + migration.registerFunc(mg, migrator, client, WithAutoMigrate(cfg)) + continue } - if !allEnabled { + enabled, err := isMigrationEnabled(migration, cfg) + if err != nil { + return err + } + if !enabled { logger.Info("Migration is disabled in config, skipping", "migration", migration.name) continue } @@ -85,10 +88,193 @@ func registerMigrations(cfg *setting.Cfg, mg *sqlstoremigrator.Migrator, migrato return nil } -func getResourceDefinition(group, resource string) *ResourceDefinition { +func registerDashboardAndFolderMigration(mg *sqlstoremigrator.Migrator, + migrator UnifiedMigrator, + client resource.ResourceClient, + opts ...ResourceMigrationOption, +) { + foldersDef := getResourceDefinition("folder.grafana.app", "folders") + dashboardsDef := getResourceDefinition("dashboard.grafana.app", "dashboards") + driverName := mg.Dialect.DriverName() + + folderCountValidator := NewCountValidator( + client, + foldersDef.groupResource, + "dashboard", + "org_id = ? and is_folder = true", + driverName, + ) + + dashboardCountValidator := NewCountValidator( + client, + dashboardsDef.groupResource, + "dashboard", + "org_id = ? and is_folder = false", + driverName, + ) + + folderTreeValidator := NewFolderTreeValidator(client, foldersDef.groupResource, driverName) + + dashboardsAndFolders := NewResourceMigration( + migrator, + []schema.GroupResource{foldersDef.groupResource, dashboardsDef.groupResource}, + "folders-dashboards", + []Validator{folderCountValidator, dashboardCountValidator, folderTreeValidator}, + opts..., + ) + mg.AddMigration("folders and dashboards migration", dashboardsAndFolders) +} + +func registerPlaylistMigration(mg *sqlstoremigrator.Migrator, + migrator UnifiedMigrator, + client resource.ResourceClient, + opts ...ResourceMigrationOption, +) { + playlistsDef := getResourceDefinition("playlist.grafana.app", "playlists") + driverName := mg.Dialect.DriverName() + + playlistCountValidator := NewCountValidator( + client, + playlistsDef.groupResource, + "playlist", + "org_id = ?", + driverName, + ) + + playlistsMigration := NewResourceMigration( + migrator, + []schema.GroupResource{playlistsDef.groupResource}, + "playlists", + []Validator{playlistCountValidator}, + opts..., + ) + mg.AddMigration("playlists migration", playlistsMigration) +} + +// TODO: remove this before Grafana 13 GA: https://github.com/grafana/search-and-storage-team/issues/613 +func shouldAutoMigrate(ctx context.Context, migration migrationDefinition, cfg *setting.Cfg, sqlStore db.DB) bool { + autoMigrate := false + + for _, res := range migration.resources { + config := cfg.UnifiedStorageConfig(res) + + if config.DualWriterMode == 5 { + return false + } + + if !setting.AutoMigratedUnifiedResources[res] { + continue + } + + if checkIfAlreadyMigrated(ctx, migration, sqlStore) { + for _, res := range migration.resources { + cfg.EnableMode5(res) + } + logger.Info("Auto-migration already completed, enabling mode 5 for resources", "migration", migration.name) + return true + } + + autoMigrate = true + threshold := int64(setting.DefaultAutoMigrationThreshold) + if config.AutoMigrationThreshold > 0 { + threshold = int64(config.AutoMigrationThreshold) + } + + count, err := countResource(ctx, sqlStore, res) + if err != nil { + logger.Warn("Failed to count resource for auto migration check", "resource", res, "error", err) + return false + } + + logger.Info("Resource count for auto migration check", "resource", res, "count", count, "threshold", threshold) + + if count > threshold { + return false + } + } + + if !autoMigrate { + return false + } + + logger.Info("Auto-migration enabled for migration", "migration", migration.name) + return true +} + +func checkIfAlreadyMigrated(ctx context.Context, migration migrationDefinition, sqlStore db.DB) bool { + if migration.migrationID == "" { + return false + } + + exists, err := migrationExists(ctx, sqlStore, migration.migrationID) + if err != nil { + logger.Warn("Failed to check if migration exists", "migration", migration.name, "error", err) + return false + } + + return exists +} + +func isMigrationEnabled(migration migrationDefinition, cfg *setting.Cfg) (bool, error) { + var ( + hasValue bool + allEnabled bool + ) + + for _, res := range migration.resources { + enabled := cfg.UnifiedStorage[res].EnableMigration + if !hasValue { + allEnabled = enabled + hasValue = true + continue + } + if enabled != allEnabled { + return false, fmt.Errorf("cannot migrate resources separately: %v migration must be either all enabled or all disabled", migration.resources) + } + } + + return allEnabled, nil +} + +// TODO: remove this before Grafana 13 GA: https://github.com/grafana/search-and-storage-team/issues/613 +func countResource(ctx context.Context, sqlStore db.DB, resourceName string) (int64, error) { + var count int64 + err := sqlStore.WithDbSession(ctx, func(sess *db.Session) error { + switch resourceName { + case setting.DashboardResource: + var err error + count, err = sess.Table("dashboard").Where("is_folder = ?", false).Count() + return err + case setting.FolderResource: + var err error + count, err = sess.Table("dashboard").Where("is_folder = ?", true).Count() + return err + default: + return fmt.Errorf("unknown resource: %s", resourceName) + } + }) + return count, err +} + +const migrationLogTableName = "unifiedstorage_migration_log" + +func migrationExists(ctx context.Context, sqlStore db.DB, migrationID string) (bool, error) { + var count int64 + err := sqlStore.WithDbSession(ctx, func(sess *db.Session) error { + var err error + count, err = sess.Table(migrationLogTableName).Where("migration_id = ?", migrationID).Count() + return err + }) + if err != nil { + return false, fmt.Errorf("failed to check migration existence: %w", err) + } + return count > 0, nil +} + +func getResourceDefinition(group, resource string) *resourceDefinition { for i := range resourceRegistry { r := &resourceRegistry[i] - if r.GroupResource.Group == group && r.GroupResource.Resource == resource { + if r.groupResource.Group == group && r.groupResource.Resource == resource { return r } } @@ -102,8 +288,8 @@ func buildResourceKey(group, resource, namespace string) *resourcepb.ResourceKey } return &resourcepb.ResourceKey{ Namespace: namespace, - Group: def.GroupResource.Group, - Resource: def.GroupResource.Resource, + Group: def.groupResource.Group, + Resource: def.groupResource.Resource, } } @@ -113,7 +299,7 @@ func getMigratorFunc(accessor legacy.MigrationDashboardAccessor, group, resource return nil } - switch def.MigratorFunc { + switch def.migratorFunc { case "MigrateFolders": return accessor.MigrateFolders case "MigrateLibraryPanels": @@ -130,7 +316,7 @@ func getMigratorFunc(accessor legacy.MigrationDashboardAccessor, group, resource func validateRegisteredResources() error { registeredMap := make(map[string]bool) for _, gr := range resourceRegistry { - key := fmt.Sprintf("%s.%s", gr.GroupResource.Resource, gr.GroupResource.Group) + key := fmt.Sprintf("%s.%s", gr.groupResource.Resource, gr.groupResource.Group) registeredMap[key] = true } diff --git a/pkg/storage/unified/migrations/resources_test.go b/pkg/storage/unified/migrations/resources_test.go index 5a519e9ad62..261cd49e244 100644 --- a/pkg/storage/unified/migrations/resources_test.go +++ b/pkg/storage/unified/migrations/resources_test.go @@ -1,12 +1,15 @@ package migrations import ( + "context" + "strings" "testing" sqlstoremigrator "github.com/grafana/grafana/pkg/services/sqlstore/migrator" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime/schema" ) // TestRegisterMigrations exercises registerMigrations with various EnableMigration configs using a table-driven test. @@ -14,20 +17,28 @@ func TestRegisterMigrations(t *testing.T) { origRegistry := migrationRegistry t.Cleanup(func() { migrationRegistry = origRegistry }) + // Use fake resource names that are NOT in setting.AutoMigratedUnifiedResources + // to avoid triggering the auto-migrate code path which requires a non-nil sqlStore. + const ( + fakePlaylistResource = "fake.playlists.resource" + fakeFolderResource = "fake.folders.resource" + fakeDashboardResource = "fake.dashboards.resource" + ) + // helper to build a fake registry with custom register funcs that bump counters makeFakeRegistry := func(migrationCalls map[string]int) []migrationDefinition { return []migrationDefinition{ { name: "playlists", - resources: []string{setting.PlaylistResource}, - registerFunc: func(mg *sqlstoremigrator.Migrator, migrator UnifiedMigrator, client resource.ResourceClient) { + resources: []string{fakePlaylistResource}, + registerFunc: func(mg *sqlstoremigrator.Migrator, migrator UnifiedMigrator, client resource.ResourceClient, opts ...ResourceMigrationOption) { migrationCalls["playlists"]++ }, }, { name: "folders and dashboards", - resources: []string{setting.FolderResource, setting.DashboardResource}, - registerFunc: func(mg *sqlstoremigrator.Migrator, migrator UnifiedMigrator, client resource.ResourceClient) { + resources: []string{fakeFolderResource, fakeDashboardResource}, + registerFunc: func(mg *sqlstoremigrator.Migrator, migrator UnifiedMigrator, client resource.ResourceClient, opts ...ResourceMigrationOption) { migrationCalls["folders and dashboards"]++ }, }, @@ -38,7 +49,9 @@ func TestRegisterMigrations(t *testing.T) { makeCfg := func(vals map[string]bool) *setting.Cfg { cfg := &setting.Cfg{UnifiedStorage: make(map[string]setting.UnifiedStorageConfig)} for k, v := range vals { - cfg.UnifiedStorage[k] = setting.UnifiedStorageConfig{EnableMigration: v} + cfg.UnifiedStorage[k] = setting.UnifiedStorageConfig{ + EnableMigration: v, + } } return cfg } @@ -71,13 +84,13 @@ func TestRegisterMigrations(t *testing.T) { migrationRegistry = makeFakeRegistry(migrationCalls) cfg := makeCfg(map[string]bool{ - setting.PlaylistResource: tt.enablePlaylist, - setting.FolderResource: tt.enableFolder, - setting.DashboardResource: tt.enableDashboard, + fakePlaylistResource: tt.enablePlaylist, + fakeFolderResource: tt.enableFolder, + fakeDashboardResource: tt.enableDashboard, }) // We pass nils for migrator dependencies because our fake registerFuncs don't use them - err := registerMigrations(cfg, nil, nil, nil) + err := registerMigrations(context.Background(), cfg, nil, nil, nil, nil) if tt.wantErr { require.Error(t, err, "expected error for mismatched enablement") @@ -90,3 +103,176 @@ func TestRegisterMigrations(t *testing.T) { }) } } + +// TestResourceMigration_AutoMigrateEnablesMode5 verifies the autoMigrate behavior: +// - When autoMigrate=true AND cfg is set AND storage type is "unified", mode 5 should be enabled +// - In all other cases, mode 5 should NOT be enabled +func TestResourceMigration_AutoMigrateEnablesMode5(t *testing.T) { + // Helper to create a cfg with unified storage type + makeUnifiedCfg := func() *setting.Cfg { + cfg := setting.NewCfg() + cfg.Raw.Section("grafana-apiserver").Key("storage_type").SetValue("unified") + cfg.UnifiedStorage = make(map[string]setting.UnifiedStorageConfig) + return cfg + } + + // Helper to create a cfg with legacy storage type + makeLegacyCfg := func() *setting.Cfg { + cfg := setting.NewCfg() + cfg.Raw.Section("grafana-apiserver").Key("storage_type").SetValue("legacy") + cfg.UnifiedStorage = make(map[string]setting.UnifiedStorageConfig) + return cfg + } + + tests := []struct { + name string + autoMigrate bool + cfg *setting.Cfg + resources []string + wantMode5Enabled bool + description string + }{ + { + name: "autoMigrate enabled with unified storage", + autoMigrate: true, + cfg: makeUnifiedCfg(), + resources: []string{setting.DashboardResource}, + wantMode5Enabled: true, + description: "Should enable mode 5 when autoMigrate=true and storage type is unified", + }, + { + name: "autoMigrate disabled with unified storage", + autoMigrate: false, + cfg: makeUnifiedCfg(), + resources: []string{setting.DashboardResource}, + wantMode5Enabled: false, + description: "Should NOT enable mode 5 when autoMigrate=false", + }, + { + name: "autoMigrate enabled with legacy storage", + autoMigrate: true, + cfg: makeLegacyCfg(), + resources: []string{setting.DashboardResource}, + wantMode5Enabled: false, + description: "Should NOT enable mode 5 when storage type is legacy", + }, + { + name: "autoMigrate enabled with nil cfg", + autoMigrate: true, + cfg: nil, + resources: []string{setting.DashboardResource}, + wantMode5Enabled: false, + description: "Should NOT enable mode 5 when cfg is nil", + }, + { + name: "autoMigrate enabled with multiple resources", + autoMigrate: true, + cfg: makeUnifiedCfg(), + resources: []string{setting.FolderResource, setting.DashboardResource}, + wantMode5Enabled: true, + description: "Should enable mode 5 for all resources when autoMigrate=true", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Build schema.GroupResource from resource strings + resources := make([]schema.GroupResource, 0, len(tt.resources)) + for _, r := range tt.resources { + parts := strings.SplitN(r, ".", 2) + resources = append(resources, schema.GroupResource{ + Resource: parts[0], + Group: parts[1], + }) + } + + // Create the migration with options + var opts []ResourceMigrationOption + if tt.autoMigrate { + opts = append(opts, WithAutoMigrate(tt.cfg)) + } + + m := NewResourceMigration(nil, resources, "test-auto-migrate", nil, opts...) + + // Simulate what happens at the end of a successful migration + // This is the logic from Exec() that we're testing + if m.autoMigrate && m.cfg != nil && m.cfg.UnifiedStorageType() == "unified" { + for _, gr := range m.resources { + m.cfg.EnableMode5(gr.Resource + "." + gr.Group) + } + } + + // Verify mode 5 was enabled (or not) for each resource + for _, resourceName := range tt.resources { + if tt.cfg == nil { + // If cfg is nil, we can't check - just verify we didn't panic + continue + } + config := tt.cfg.UnifiedStorageConfig(resourceName) + if tt.wantMode5Enabled { + require.Equal(t, 5, int(config.DualWriterMode), "%s: %s", tt.description, resourceName) + require.True(t, config.EnableMigration, "%s: EnableMigration should be true for %s", tt.description, resourceName) + require.True(t, config.DualWriterMigrationDataSyncDisabled, "%s: DualWriterMigrationDataSyncDisabled should be true for %s", tt.description, resourceName) + } else { + require.Equal(t, 0, int(config.DualWriterMode), "%s: mode should be 0 for %s", tt.description, resourceName) + } + } + }) + } +} + +// TestResourceMigration_SkipMigrationLog verifies the SkipMigrationLog behavior: +// - When ignoreErrors=true AND errors occurred (hadErrors=true), skip writing to migration log +// This allows the migration to be re-run on the next startup +// - In all other cases, write to migration log normally +// +// This is important for the folders/dashboards migration which uses WithIgnoreErrors() to handle +// partial failures gracefully while still allowing retry on next startup. +func TestResourceMigration_SkipMigrationLog(t *testing.T) { + tests := []struct { + name string + autoMigrate bool + hadErrors bool + want bool + description string + }{ + { + name: "normal migration success", + autoMigrate: false, + hadErrors: false, + want: false, + description: "Normal successful migration should write to log", + }, + { + name: "ignoreErrors migration success", + autoMigrate: true, + hadErrors: false, + want: false, + description: "Migration with ignoreErrors that succeeds should still write to log", + }, + { + name: "normal migration with errors", + autoMigrate: false, + hadErrors: true, + want: false, + description: "Migration that fails without ignoreErrors should write error to log", + }, + { + name: "ignoreErrors migration with errors - skip log", + autoMigrate: true, + hadErrors: true, + want: true, + description: "Migration with ignoreErrors that has errors should SKIP log to allow retry", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := &ResourceMigration{ + autoMigrate: tt.autoMigrate, + hadErrors: tt.hadErrors, + } + require.Equal(t, tt.want, m.SkipMigrationLog(), tt.description) + }) + } +} diff --git a/pkg/storage/unified/migrations/service.go b/pkg/storage/unified/migrations/service.go index 4ac9cdad218..8dec7ed019e 100644 --- a/pkg/storage/unified/migrations/service.go +++ b/pkg/storage/unified/migrations/service.go @@ -14,7 +14,6 @@ import ( "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel" - "k8s.io/apimachinery/pkg/runtime/schema" ) var tracer = otel.Tracer("github.com/grafana/grafana/pkg/storage/unified/migrations") @@ -54,6 +53,7 @@ func (p *UnifiedStorageMigrationServiceImpl) Run(ctx context.Context) error { logger.Info("Data migrations are disabled, skipping") return nil } + logger.Info("Running migrations for unified storage") metrics.MUnifiedStorageMigrationStatus.Set(3) return RegisterMigrations(ctx, p.migrator, p.cfg, p.sqlStore, p.client) @@ -79,7 +79,7 @@ func RegisterMigrations( return err } - if err := registerMigrations(cfg, mg, migrator, client); err != nil { + if err := registerMigrations(ctx, cfg, mg, migrator, client, sqlStore); err != nil { return err } @@ -92,65 +92,13 @@ func RegisterMigrations( db.SetMaxOpenConns(3) defer db.SetMaxOpenConns(maxOpenConns) } - if err := mg.RunMigrations(ctx, + err := mg.RunMigrations(ctx, sec.Key("migration_locking").MustBool(true), - sec.Key("locking_attempt_timeout_sec").MustInt()); err != nil { + sec.Key("locking_attempt_timeout_sec").MustInt()) + if err != nil { return fmt.Errorf("unified storage data migration failed: %w", err) } logger.Info("Unified storage migrations completed successfully") return nil } - -func registerDashboardAndFolderMigration(mg *sqlstoremigrator.Migrator, migrator UnifiedMigrator, client resource.ResourceClient) { - foldersDef := getResourceDefinition("folder.grafana.app", "folders") - dashboardsDef := getResourceDefinition("dashboard.grafana.app", "dashboards") - driverName := mg.Dialect.DriverName() - - folderCountValidator := NewCountValidator( - client, - foldersDef.GroupResource, - "dashboard", - "org_id = ? and is_folder = true", - driverName, - ) - - dashboardCountValidator := NewCountValidator( - client, - dashboardsDef.GroupResource, - "dashboard", - "org_id = ? and is_folder = false", - driverName, - ) - - folderTreeValidator := NewFolderTreeValidator(client, foldersDef.GroupResource, driverName) - - dashboardsAndFolders := NewResourceMigration( - migrator, - []schema.GroupResource{foldersDef.GroupResource, dashboardsDef.GroupResource}, - "folders-dashboards", - []Validator{folderCountValidator, dashboardCountValidator, folderTreeValidator}, - ) - mg.AddMigration("folders and dashboards migration", dashboardsAndFolders) -} - -func registerPlaylistMigration(mg *sqlstoremigrator.Migrator, migrator UnifiedMigrator, client resource.ResourceClient) { - playlistsDef := getResourceDefinition("playlist.grafana.app", "playlists") - driverName := mg.Dialect.DriverName() - - playlistCountValidator := NewCountValidator( - client, - playlistsDef.GroupResource, - "playlist", - "org_id = ?", - driverName, - ) - - playlistsMigration := NewResourceMigration( - migrator, - []schema.GroupResource{playlistsDef.GroupResource}, - "playlists", - []Validator{playlistCountValidator}, - ) - mg.AddMigration("playlists migration", playlistsMigration) -} diff --git a/pkg/storage/unified/migrations/threshold/migrator_threshold_test.go b/pkg/storage/unified/migrations/threshold/migrator_threshold_test.go new file mode 100644 index 00000000000..4a542b3ae0b --- /dev/null +++ b/pkg/storage/unified/migrations/threshold/migrator_threshold_test.go @@ -0,0 +1,211 @@ +package threshold + +import ( + "context" + "fmt" + "net/http" + "os" + "testing" + + authlib "github.com/grafana/authlib/types" + "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/services/folder" + "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" + "github.com/grafana/grafana/pkg/util/testutil" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// TODO: remove this test before Grafana 13 GA +func TestMain(m *testing.M) { + testsuite.Run(m) +} + +// TestIntegrationAutoMigrateThresholdExceeded verifies that auto-migration is skipped when +// resource count exceeds the configured threshold. +// TODO: remove this test before Grafana 13 GA +func TestIntegrationAutoMigrateThresholdExceeded(t *testing.T) { + testutil.SkipIntegrationTestInShortMode(t) + + if db.IsTestDbSQLite() { + // Share the same SQLite DB file between steps + tmpDir := t.TempDir() + dbPath := tmpDir + "/shared-threshold-test.db" + + oldVal := os.Getenv("SQLITE_TEST_DB") + require.NoError(t, os.Setenv("SQLITE_TEST_DB", dbPath)) + t.Cleanup(func() { + if oldVal == "" { + _ = os.Unsetenv("SQLITE_TEST_DB") + } else { + _ = os.Setenv("SQLITE_TEST_DB", oldVal) + } + }) + t.Logf("Using shared database path: %s", dbPath) + } + + var org1 *apis.OrgUsers + var orgB *apis.OrgUsers + + dashboardGVR := schema.GroupVersionResource{ + Group: "dashboard.grafana.app", + Version: "v1beta1", + Resource: "dashboards", + } + folderGVR := schema.GroupVersionResource{ + Group: "folder.grafana.app", + Version: "v1beta1", + Resource: "folders", + } + + dashboardKey := fmt.Sprintf("%s.%s", dashboardGVR.Resource, dashboardGVR.Group) + folderKey := fmt.Sprintf("%s.%s", folderGVR.Resource, folderGVR.Group) + playlistKey := "playlists.playlist.grafana.app" + + // Step 1: Create resources exceeding the threshold (3 resources, threshold=1) + t.Run("Step 1: Create resources exceeding threshold", func(t *testing.T) { + unifiedConfig := map[string]setting.UnifiedStorageConfig{} + helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ + AppModeProduction: true, + DisableAnonymous: true, + DisableDataMigrations: true, + DisableDBCleanup: true, + APIServerStorageType: "unified", + UnifiedStorageConfig: unifiedConfig, + }) + org1 = &helper.Org1 + orgB = &helper.OrgB + + // Create 3 dashboards + for i := 1; i <= 3; i++ { + createTestDashboard(t, helper, fmt.Sprintf("Threshold Dashboard %d", i)) + } + + // Create 3 folders + for i := 1; i <= 3; i++ { + createTestFolder(t, helper, fmt.Sprintf("folder-%d", i), fmt.Sprintf("Threshold Folder %d", i), "") + } + + // Explicitly shutdown helper before Step 1 ends to ensure database is properly closed + helper.Shutdown() + }) + + // Set SKIP_DB_TRUNCATE to prevent truncation in subsequent steps + oldSkipTruncate := os.Getenv("SKIP_DB_TRUNCATE") + require.NoError(t, os.Setenv("SKIP_DB_TRUNCATE", "true")) + t.Cleanup(func() { + if oldSkipTruncate == "" { + _ = os.Unsetenv("SKIP_DB_TRUNCATE") + } else { + _ = os.Setenv("SKIP_DB_TRUNCATE", oldSkipTruncate) + } + }) + + // Step 2: Verify auto-migration is skipped due to threshold + t.Run("Step 2: Verify auto-migration skipped (threshold exceeded)", func(t *testing.T) { + // Set threshold=1, but we have 3 resources of each type, so migration should be skipped + // Disable playlists migration since we're only testing dashboard/folder threshold behavior + unifiedConfig := map[string]setting.UnifiedStorageConfig{ + dashboardKey: {AutoMigrationThreshold: 1, EnableMigration: false}, + folderKey: {AutoMigrationThreshold: 1, EnableMigration: false}, + playlistKey: {EnableMigration: false}, + } + helper := apis.NewK8sTestHelperWithOpts(t, apis.K8sTestHelperOpts{ + GrafanaOpts: testinfra.GrafanaOpts{ + AppModeProduction: true, + DisableAnonymous: true, + DisableDataMigrations: false, // Allow migration system to run + APIServerStorageType: "unified", + UnifiedStorageConfig: unifiedConfig, + }, + Org1Users: org1, + OrgBUsers: orgB, + }) + t.Cleanup(helper.Shutdown) + + namespace := authlib.OrgNamespaceFormatter(helper.Org1.OrgID) + + dashCli := helper.GetResourceClient(apis.ResourceClientArgs{ + User: helper.Org1.Admin, + Namespace: namespace, + GVR: dashboardGVR, + }) + verifyResourceCount(t, dashCli, 3) + + folderCli := helper.GetResourceClient(apis.ResourceClientArgs{ + User: helper.Org1.Admin, + Namespace: namespace, + GVR: folderGVR, + }) + verifyResourceCount(t, folderCli, 3) + + // Verify migration did NOT run by checking the migration log + count, err := helper.GetEnv().SQLStore.GetEngine().Table("unifiedstorage_migration_log"). + Where("migration_id = ?", "folders and dashboards migration"). + Count() + require.NoError(t, err) + require.Equal(t, int64(0), count, "Migration should not have run") + }) +} + +func createTestDashboard(t *testing.T, helper *apis.K8sTestHelper, title string) string { + t.Helper() + + payload := fmt.Sprintf(`{"dashboard": {"title": "%s", "panels": []}, "overwrite": false}`, title) + + result := apis.DoRequest(helper, apis.RequestParams{ + User: helper.Org1.Admin, + Method: "POST", + Path: "/api/dashboards/db", + Body: []byte(payload), + }, &map[string]interface{}{}) + + require.NotNil(t, result.Response) + require.Equal(t, 200, result.Response.StatusCode) + + uid := (*result.Result)["uid"].(string) + require.NotEmpty(t, uid) + return uid +} + +func createTestFolder(t *testing.T, helper *apis.K8sTestHelper, uid, title, parentUID string) *folder.Folder { + t.Helper() + + payload := fmt.Sprintf(`{ + "title": "%s", + "uid": "%s"`, title, uid) + + if parentUID != "" { + payload += fmt.Sprintf(`, + "parentUid": "%s"`, parentUID) + } + + payload += "}" + + folderCreate := apis.DoRequest(helper, apis.RequestParams{ + User: helper.Org1.Admin, + Method: http.MethodPost, + Path: "/api/folders", + Body: []byte(payload), + }, &folder.Folder{}) + + require.NotNil(t, folderCreate.Result) + return folderCreate.Result +} + +// verifyResourceCount verifies that the expected number of resources exist in K8s storage +func verifyResourceCount(t *testing.T, client *apis.K8sResourceClient, expectedCount int) { + t.Helper() + + l, err := client.Resource.List(context.Background(), metav1.ListOptions{}) + require.NoError(t, err) + + resources, err := meta.ExtractList(l) + require.NoError(t, err) + require.Equal(t, expectedCount, len(resources)) +} diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index b5e04d65a40..17f1e9d84b2 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/pkg/tests/testinfra/testinfra.go @@ -557,6 +557,8 @@ func CreateGrafDir(t *testing.T, opts GrafanaOpts) (string, string) { require.NoError(t, err) _, err = section.NewKey("enableMigration", fmt.Sprintf("%t", v.EnableMigration)) require.NoError(t, err) + _, err = section.NewKey("autoMigrationThreshold", fmt.Sprintf("%d", v.AutoMigrationThreshold)) + require.NoError(t, err) } } if opts.UnifiedStorageEnableSearch {