Alerting: Break up store.RuleStore interface, delete dead code (#55776)

* Refactor state manager to not depend on rule store interface

* Refactor grafana and proxied ruler APIs to not depend on store.RuleStore

* Refactor folder subscription logic to not use store.RuleStore

* Delete dead code

* Delete store.RuleStore

pkg/services/ngalert/api/api.go

@@ -68,7 +68,7 @@ type API struct {
 	Schedule             schedule.ScheduleService
 	TransactionManager   provisioning.TransactionManager
 	ProvenanceStore      provisioning.ProvisioningStore
-	RuleStore            store.RuleStore
+	RuleStore            RuleStore
 	AlertingStore        AlertingStore
 	AdminConfigStore     store.AdminConfigurationStore
 	DataProxy            *datasourceproxy.DataSourceProxyService

pkg/services/ngalert/api/api_prometheus.go

@@ -18,7 +18,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/ngalert/eval"
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/state"
-	"github.com/grafana/grafana/pkg/services/ngalert/store"
 
 	apiv1 "github.com/prometheus/client_golang/api/prometheus/v1"
 )
@@ -26,7 +25,7 @@ import (
 type PrometheusSrv struct {
 	log     log.Logger
 	manager state.AlertInstanceManager
-	store   store.RuleStore
+	store   RuleStore
 	ac      accesscontrol.AccessControl
 }
 

pkg/services/ngalert/api/api_ruler.go

@@ -36,7 +36,7 @@ type ConditionValidator interface {
 type RulerSrv struct {
 	xactManager        provisioning.TransactionManager
 	provenanceStore    provisioning.ProvisioningStore
-	store              store.RuleStore
+	store              RuleStore
 	QuotaService       quota.Service
 	scheduleService    schedule.ScheduleService
 	log                log.Logger

pkg/services/ngalert/api/persist.go

@@ -0,0 +1,27 @@
+package api
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/models"
+	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
+	"github.com/grafana/grafana/pkg/services/ngalert/store"
+	"github.com/grafana/grafana/pkg/services/user"
+)
+
+// RuleStore is the interface for persisting alert rules and instances
+type RuleStore interface {
+	GetUserVisibleNamespaces(context.Context, int64, *user.SignedInUser) (map[string]*models.Folder, error)
+	GetNamespaceByTitle(context.Context, string, int64, *user.SignedInUser, bool) (*models.Folder, error)
+	GetAlertRulesGroupByRuleUID(ctx context.Context, query *ngmodels.GetAlertRulesGroupByRuleUIDQuery) error
+	ListAlertRules(ctx context.Context, query *ngmodels.ListAlertRulesQuery) error
+
+	// InsertAlertRules will insert all alert rules passed into the function
+	// and return the map of uuid to id.
+	InsertAlertRules(ctx context.Context, rule []ngmodels.AlertRule) (map[string]int64, error)
+	UpdateAlertRules(ctx context.Context, rule []store.UpdateRule) error
+	DeleteAlertRulesByUID(ctx context.Context, orgID int64, ruleUID ...string) error
+
+	// IncreaseVersionForAllRulesInNamespace Increases version for all rules that have specified namespace. Returns all rules that belong to the namespace
+	IncreaseVersionForAllRulesInNamespace(ctx context.Context, orgID int64, namespaceUID string) ([]ngmodels.AlertRuleKeyWithVersion, error)
+}

pkg/services/ngalert/models/alert_rule.go

@@ -365,12 +365,6 @@ type ListNamespaceAlertRulesQuery struct {
 	Result []*AlertRule
 }
 
-// ListRuleGroupsQuery is the query for listing unique rule groups
-// across all organizations
-type ListRuleGroupsQuery struct {
-	Result []string
-}
-
 // ListOrgRuleGroupsQuery is the query for listing unique rule groups
 // for an organization
 type ListOrgRuleGroupsQuery struct {

pkg/services/ngalert/ngalert.go

@@ -216,7 +216,7 @@ func (ng *AlertNG) init() error {
 	return DeclareFixedRoles(ng.accesscontrolService)
 }
 
-func subscribeToFolderChanges(logger log.Logger, bus bus.Bus, dbStore store.RuleStore, scheduler schedule.ScheduleService) {
+func subscribeToFolderChanges(logger log.Logger, bus bus.Bus, dbStore api.RuleStore, scheduler schedule.ScheduleService) {
 	// if folder title is changed, we update all alert rules in that folder to make sure that all peers (in HA mode) will update folder title and
 	// clean up the current state
 	bus.AddEventListener(func(ctx context.Context, e *events.FolderTitleUpdated) error {

pkg/services/ngalert/state/manager.go

@@ -20,7 +20,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/ngalert/image"
 	"github.com/grafana/grafana/pkg/services/ngalert/metrics"
 	ngModels "github.com/grafana/grafana/pkg/services/ngalert/models"
-	"github.com/grafana/grafana/pkg/services/ngalert/store"
 
 	"github.com/grafana/grafana/pkg/services/screenshot"
 )
@@ -42,7 +41,7 @@ type Manager struct {
 	quit        chan struct{}
 	ResendDelay time.Duration
 
-	ruleStore        store.RuleStore
+	ruleStore        RuleReader
 	instanceStore    InstanceStore
 	dashboardService dashboards.DashboardService
 	imageService     image.ImageService
@@ -50,7 +49,7 @@ type Manager struct {
 }
 
 func NewManager(logger log.Logger, metrics *metrics.State, externalURL *url.URL,
-	ruleStore store.RuleStore, instanceStore InstanceStore,
+	ruleStore RuleReader, instanceStore InstanceStore,
 	dashboardService dashboards.DashboardService, imageService image.ImageService, clock clock.Clock, annotationsRepo annotations.Repository) *Manager {
 	manager := &Manager{
 		cache:            newCache(logger, metrics, externalURL),

pkg/services/ngalert/state/persist.go

@@ -6,6 +6,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 )
 
+// InstanceStore represents the ability to fetch and write alert instances.
 type InstanceStore interface {
 	FetchOrgIds(ctx context.Context) ([]int64, error)
 	ListAlertInstances(ctx context.Context, cmd *models.ListAlertInstancesQuery) error
@@ -13,3 +14,8 @@ type InstanceStore interface {
 	DeleteAlertInstance(ctx context.Context, orgID int64, ruleUID, labelsHash string) error
 	DeleteAlertInstancesByRule(ctx context.Context, key models.AlertRuleKey) error
 }
+
+// RuleReader represents the ability to fetch alert rules.
+type RuleReader interface {
+	ListAlertRules(ctx context.Context, query *models.ListAlertRulesQuery) error
+}

pkg/services/ngalert/store/alert_rule.go

@@ -38,28 +38,6 @@ var (
 	ErrOptimisticLock         = errors.New("version conflict while updating a record in the database with optimistic locking")
 )
 
-// RuleStore is the interface for persisting alert rules and instances
-type RuleStore interface {
-	DeleteAlertRulesByUID(ctx context.Context, orgID int64, ruleUID ...string) error
-	DeleteAlertInstancesByRuleUID(ctx context.Context, orgID int64, ruleUID string) error
-	GetAlertRuleByUID(ctx context.Context, query *ngmodels.GetAlertRuleByUIDQuery) error
-	GetAlertRulesGroupByRuleUID(ctx context.Context, query *ngmodels.GetAlertRulesGroupByRuleUIDQuery) error
-	ListAlertRules(ctx context.Context, query *ngmodels.ListAlertRulesQuery) error
-	// GetRuleGroups returns the unique rule groups across all organizations.
-	GetRuleGroups(ctx context.Context, query *ngmodels.ListRuleGroupsQuery) error
-	GetRuleGroupInterval(ctx context.Context, orgID int64, namespaceUID string, ruleGroup string) (int64, error)
-	GetUserVisibleNamespaces(context.Context, int64, *user.SignedInUser) (map[string]*models.Folder, error)
-	GetNamespaceByTitle(context.Context, string, int64, *user.SignedInUser, bool) (*models.Folder, error)
-	GetNamespaceByUID(context.Context, string, int64, *user.SignedInUser) (*models.Folder, error)
-	// InsertAlertRules will insert all alert rules passed into the function
-	// and return the map of uuid to id.
-	InsertAlertRules(ctx context.Context, rule []ngmodels.AlertRule) (map[string]int64, error)
-	UpdateAlertRules(ctx context.Context, rule []UpdateRule) error
-
-	// IncreaseVersionForAllRulesInNamespace Increases version for all rules that have specified namespace. Returns all rules that belong to the namespace
-	IncreaseVersionForAllRulesInNamespace(ctx context.Context, orgID int64, namespaceUID string) ([]ngmodels.AlertRuleKeyWithVersion, error)
-}
-
 func getAlertRuleByUID(sess *sqlstore.DBSession, alertRuleUID string, orgID int64) (*ngmodels.AlertRule, error) {
 	// we consider optionally enabling some caching
 	alertRule := ngmodels.AlertRule{OrgID: orgID, UID: alertRuleUID}
@@ -112,17 +90,6 @@ func (st DBstore) IncreaseVersionForAllRulesInNamespace(ctx context.Context, org
 	return keys, err
 }
 
-// DeleteAlertInstancesByRuleUID is a handler for deleting alert instances by alert rule UID when a rule has been updated
-func (st DBstore) DeleteAlertInstancesByRuleUID(ctx context.Context, orgID int64, ruleUID string) error {
-	return st.SQLStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
-		_, err := sess.Exec("DELETE FROM alert_instance WHERE rule_org_id = ? AND rule_uid = ?", orgID, ruleUID)
-		if err != nil {
-			return err
-		}
-		return nil
-	})
-}
-
 // GetAlertRuleByUID is a handler for retrieving an alert rule from that database by its UID and organisation ID.
 // It returns ngmodels.ErrAlertRuleNotFound if no alert rule is found for the provided ID.
 func (st DBstore) GetAlertRuleByUID(ctx context.Context, query *ngmodels.GetAlertRuleByUIDQuery) error {
@@ -313,17 +280,6 @@ func (st DBstore) ListAlertRules(ctx context.Context, query *ngmodels.ListAlertR
 	})
 }
 
-func (st DBstore) GetRuleGroups(ctx context.Context, query *ngmodels.ListRuleGroupsQuery) error {
-	return st.SQLStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {
-		ruleGroups := make([]string, 0)
-		if err := sess.Table("alert_rule").Distinct("rule_group").Find(&ruleGroups); err != nil {
-			return err
-		}
-		query.Result = ruleGroups
-		return nil
-	})
-}
-
 func (st DBstore) GetRuleGroupInterval(ctx context.Context, orgID int64, namespaceUID string, ruleGroup string) (int64, error) {
 	var interval int64 = 0
 	return interval, st.SQLStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {

pkg/services/ngalert/store/testing.go

@@ -171,9 +171,6 @@ func (f *FakeRuleStore) DeleteAlertRulesByUID(_ context.Context, orgID int64, UI
 	return nil
 }
 
-func (f *FakeRuleStore) DeleteAlertInstancesByRuleUID(_ context.Context, _ int64, _ string) error {
-	return nil
-}
 func (f *FakeRuleStore) GetAlertRuleByUID(_ context.Context, q *models.GetAlertRuleByUIDQuery) error {
 	f.mtx.Lock()
 	defer f.mtx.Unlock()
@@ -328,25 +325,6 @@ func (f *FakeRuleStore) ListAlertRules(_ context.Context, q *models.ListAlertRul
 	return nil
 }
 
-func (f *FakeRuleStore) GetRuleGroups(_ context.Context, q *models.ListRuleGroupsQuery) error {
-	f.mtx.Lock()
-	defer f.mtx.Unlock()
-	f.RecordedOps = append(f.RecordedOps, *q)
-
-	m := make(map[string]struct{})
-	for _, rules := range f.Rules {
-		for _, rule := range rules {
-			m[rule.RuleGroup] = struct{}{}
-		}
-	}
-
-	for s := range m {
-		q.Result = append(q.Result, s)
-	}
-
-	return nil
-}
-
 func (f *FakeRuleStore) GetUserVisibleNamespaces(_ context.Context, orgID int64, _ *user.SignedInUser) (map[string]*models2.Folder, error) {
 	f.mtx.Lock()
 	defer f.mtx.Unlock()