public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Advisor: Avoid one GCOM request per plugin (#105180)

Browse Source
This commit is contained in:
Andres Martinez Gotor
2025-05-12 09:45:56 +02:00
committed by GitHub
parent cb28213f1d
commit 0d4884cb3b
13 changed files with 137 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/advisor/pkg/app/checks/authchecks/check.go
+4
View File
@@ -28,6 +28,10 @@ func (c *check) ID() string {
return CheckID
}
func (c *check) Init(ctx context.Context) error {
return nil
}
func (c *check) Steps() []checks.Step {
return []checks.Step{
&listFormatValidation{},
apps/advisor/pkg/app/checks/datasourcecheck/check.go
+18 -5
View File
@@ -32,6 +32,7 @@ type check struct {
PluginClient plugins.Client
PluginRepo repo.Service
GrafanaVersion string
pluginIndex map[string]repo.PluginInfo
}
func New(
@@ -79,6 +80,19 @@ func (c *check) ID() string {
return CheckID
}
func (c *check) Init(ctx context.Context) error {
compatOpts := repo.NewCompatOpts(c.GrafanaVersion, sysruntime.GOOS, sysruntime.GOARCH)
plugins, err := c.PluginRepo.GetPluginsInfo(ctx, compatOpts)
if err != nil {
return err
}
c.pluginIndex = make(map[string]repo.PluginInfo)
for _, p := range plugins {
c.pluginIndex[p.Slug] = p
}
return nil
}
func (c *check) Steps() []checks.Step {
return []checks.Step{
&uidValidationStep{},
@@ -88,8 +102,8 @@ func (c *check) Steps() []checks.Step {
},
&missingPluginStep{
PluginStore: c.PluginStore,
PluginRepo: c.PluginRepo,
GrafanaVersion: c.GrafanaVersion,
pluginIndex: c.pluginIndex,
},
}
}
@@ -207,8 +221,8 @@ func (s *healthCheckStep) Run(ctx context.Context, log logging.Logger, obj *advi
type missingPluginStep struct {
PluginStore pluginstore.Store
PluginRepo repo.Service
GrafanaVersion string
pluginIndex map[string]repo.PluginInfo
}
func (s *missingPluginStep) Title() string {
@@ -241,9 +255,8 @@ func (s *missingPluginStep) Run(ctx context.Context, log logging.Logger, obj *ad
Url: fmt.Sprintf("/connections/datasources/edit/%s", ds.UID),
},
}
compatOpts := repo.NewCompatOpts(s.GrafanaVersion, sysruntime.GOOS, sysruntime.GOARCH)
_, err := s.PluginRepo.PluginInfo(ctx, ds.Type, compatOpts)
if err == nil {
_, ok := s.pluginIndex[ds.Type]
if ok {
// Plugin is available in the repo
links = append(links, advisor.CheckErrorLink{
Message: "Install plugin",
apps/advisor/pkg/app/checks/datasourcecheck/check_test.go
+27 -14
View File
@@ -2,7 +2,6 @@ package datasourcecheck
import (
"context"
"errors"
"testing"
"github.com/grafana/grafana-app-sdk/logging"
@@ -26,6 +25,10 @@ func runChecks(check *check) ([]advisor.CheckReportFailure, error) {
}
failures := []advisor.CheckReportFailure{}
err = check.Init(ctx)
if err != nil {
return nil, err
}
for _, step := range check.Steps() {
for _, item := range items {
stepFailures, err := step.Run(ctx, logging.DefaultLogger, &advisor.CheckSpec{}, item)
@@ -51,7 +54,10 @@ func TestCheck_Run(t *testing.T) {
mockDatasourceSvc := &MockDatasourceSvc{dss: datasources}
mockPluginContextProvider := &MockPluginContextProvider{pCtx: backend.PluginContext{}}
mockPluginClient := &MockPluginClient{res: &backend.CheckHealthResult{Status: backend.HealthStatusOk}}
mockPluginRepo := &MockPluginRepo{exists: true}
mockPluginRepo := &MockPluginRepo{plugins: []repo.PluginInfo{
{ID: 1, Slug: "prometheus", Status: "active"},
{ID: 2, Slug: "mysql", Status: "active"},
}}
mockPluginStore := &MockPluginStore{exists: true}
check := &check{
@@ -75,7 +81,9 @@ func TestCheck_Run(t *testing.T) {
mockDatasourceSvc := &MockDatasourceSvc{dss: datasources}
mockPluginContextProvider := &MockPluginContextProvider{pCtx: backend.PluginContext{}}
mockPluginClient := &MockPluginClient{res: &backend.CheckHealthResult{Status: backend.HealthStatusOk}}
mockPluginRepo := &MockPluginRepo{exists: true}
mockPluginRepo := &MockPluginRepo{plugins: []repo.PluginInfo{
{ID: 1, Slug: "prometheus", Status: "active"},
}}
mockPluginStore := &MockPluginStore{exists: true}
check := &check{
@@ -100,7 +108,9 @@ func TestCheck_Run(t *testing.T) {
mockDatasourceSvc := &MockDatasourceSvc{dss: datasources}
mockPluginContextProvider := &MockPluginContextProvider{pCtx: backend.PluginContext{}}
mockPluginClient := &MockPluginClient{res: &backend.CheckHealthResult{Status: backend.HealthStatusError}}
mockPluginRepo := &MockPluginRepo{exists: true}
mockPluginRepo := &MockPluginRepo{plugins: []repo.PluginInfo{
{ID: 1, Slug: "prometheus", Status: "active"},
}}
mockPluginStore := &MockPluginStore{exists: true}
check := &check{
@@ -124,7 +134,9 @@ func TestCheck_Run(t *testing.T) {
mockDatasourceSvc := &MockDatasourceSvc{dss: datasources}
mockPluginContextProvider := &MockPluginContextProvider{pCtx: backend.PluginContext{}}
mockPluginClient := &MockPluginClient{err: plugins.ErrMethodNotImplemented}
mockPluginRepo := &MockPluginRepo{exists: true}
mockPluginRepo := &MockPluginRepo{plugins: []repo.PluginInfo{
{ID: 1, Slug: "prometheus", Status: "active"},
}}
mockPluginStore := &MockPluginStore{exists: true}
check := &check{
@@ -147,7 +159,9 @@ func TestCheck_Run(t *testing.T) {
mockDatasourceSvc := &MockDatasourceSvc{dss: datasources}
mockPluginContextProvider := &MockPluginContextProvider{pCtx: backend.PluginContext{}}
mockPluginClient := &MockPluginClient{err: plugins.ErrPluginNotRegistered}
mockPluginRepo := &MockPluginRepo{exists: true}
mockPluginRepo := &MockPluginRepo{plugins: []repo.PluginInfo{
{ID: 1, Slug: "prometheus", Status: "active"},
}}
mockPluginStore := &MockPluginStore{exists: true}
check := &check{
@@ -171,7 +185,9 @@ func TestCheck_Run(t *testing.T) {
mockDatasourceSvc := &MockDatasourceSvc{dss: datasources}
mockPluginContextProvider := &MockPluginContextProvider{pCtx: backend.PluginContext{}}
mockPluginClient := &MockPluginClient{res: &backend.CheckHealthResult{Status: backend.HealthStatusOk}}
mockPluginRepo := &MockPluginRepo{exists: true}
mockPluginRepo := &MockPluginRepo{plugins: []repo.PluginInfo{
{ID: 1, Slug: "prometheus", Status: "active"},
}}
mockPluginStore := &MockPluginStore{exists: false}
check := &check{
@@ -196,7 +212,7 @@ func TestCheck_Run(t *testing.T) {
mockDatasourceSvc := &MockDatasourceSvc{dss: datasources}
mockPluginContextProvider := &MockPluginContextProvider{pCtx: backend.PluginContext{}}
mockPluginClient := &MockPluginClient{res: &backend.CheckHealthResult{Status: backend.HealthStatusOk}}
mockPluginRepo := &MockPluginRepo{exists: false}
mockPluginRepo := &MockPluginRepo{plugins: []repo.PluginInfo{}}
mockPluginStore := &MockPluginStore{exists: false}
check := &check{
@@ -257,12 +273,9 @@ func (m *MockPluginStore) Plugin(context.Context, string) (pluginstore.Plugin, b
type MockPluginRepo struct {
repo.Service
exists bool
plugins []repo.PluginInfo
}
func (m *MockPluginRepo) PluginInfo(context.Context, string, repo.CompatOpts) (*repo.PluginInfo, error) {
if !m.exists {
return nil, errors.New("plugin not found")
}
return &repo.PluginInfo{}, nil
func (m *MockPluginRepo) GetPluginsInfo(context.Context, repo.CompatOpts) ([]repo.PluginInfo, error) {
return m.plugins, nil
}
apps/advisor/pkg/app/checks/ifaces.go
+3
View File
@@ -17,6 +17,9 @@ type Check interface {
Items(ctx context.Context) ([]any, error)
// Steps returns the list of steps that will be executed
Steps() []Step
// Init initializes the check. It's called before running the steps and should be idempotent.
// The result should not be cached, it should be initialized from scratch.
Init(ctx context.Context) error
}
// Step is a single step in a check, including its metadata
apps/advisor/pkg/app/checks/plugincheck/check.go
+23 -23
View File
@@ -5,7 +5,6 @@ import (
"fmt"
sysruntime "runtime"
"github.com/Masterminds/semver/v3"
"github.com/grafana/grafana-app-sdk/logging"
advisor "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1"
"github.com/grafana/grafana/apps/advisor/pkg/app/checks"
@@ -39,6 +38,7 @@ type check struct {
PluginRepo repo.Service
updateChecker pluginchecker.PluginUpdateChecker
GrafanaVersion string
pluginIndex map[string]repo.PluginInfo
}
func (c *check) ID() string {
@@ -62,25 +62,38 @@ func (c *check) Item(ctx context.Context, id string) (any, error) {
return p, nil
}
func (c *check) Init(ctx context.Context) error {
compatOpts := repo.NewCompatOpts(c.GrafanaVersion, sysruntime.GOOS, sysruntime.GOARCH)
plugins, err := c.PluginRepo.GetPluginsInfo(ctx, compatOpts)
if err != nil {
return err
}
c.pluginIndex = make(map[string]repo.PluginInfo)
for _, p := range plugins {
c.pluginIndex[p.Slug] = p
}
return nil
}
func (c *check) Steps() []checks.Step {
return []checks.Step{
&deprecationStep{
PluginRepo: c.PluginRepo,
GrafanaVersion: c.GrafanaVersion,
updateChecker: c.updateChecker,
pluginIndex: c.pluginIndex,
},
&updateStep{
PluginRepo: c.PluginRepo,
GrafanaVersion: c.GrafanaVersion,
updateChecker: c.updateChecker,
pluginIndex: c.pluginIndex,
},
}
}
type deprecationStep struct {
PluginRepo repo.Service
GrafanaVersion string
updateChecker pluginchecker.PluginUpdateChecker
pluginIndex map[string]repo.PluginInfo
}
func (s *deprecationStep) Title() string {
@@ -111,9 +124,8 @@ func (s *deprecationStep) Run(ctx context.Context, log logging.Logger, _ *adviso
}
// Check if plugin is deprecated
compatOpts := repo.NewCompatOpts(s.GrafanaVersion, sysruntime.GOOS, sysruntime.GOARCH)
i, err := s.PluginRepo.PluginInfo(ctx, p.ID, compatOpts)
if err != nil {
i, ok := s.pluginIndex[p.ID]
if !ok {
// Unable to check deprecation status
return nil, nil
}
@@ -135,9 +147,9 @@ func (s *deprecationStep) Run(ctx context.Context, log logging.Logger, _ *adviso
}
type updateStep struct {
PluginRepo repo.Service
GrafanaVersion string
updateChecker pluginchecker.PluginUpdateChecker
pluginIndex map[string]repo.PluginInfo
}
func (s *updateStep) Title() string {
@@ -167,13 +179,12 @@ func (s *updateStep) Run(ctx context.Context, log logging.Logger, _ *advisor.Che
}
// Check if plugin has a newer version available
compatOpts := repo.NewCompatOpts(s.GrafanaVersion, sysruntime.GOOS, sysruntime.GOARCH)
info, err := s.PluginRepo.GetPluginArchiveInfo(ctx, p.ID, "", compatOpts)
if err != nil {
info, ok := s.pluginIndex[p.ID]
if !ok {
// Unable to check updates
return nil, nil
}
if hasUpdate(p, info) {
if s.updateChecker.CanUpdate(p.ID, p.Info.Version, info.Version, false) {
return []advisor.CheckReportFailure{checks.NewCheckReportFailure(
advisor.CheckReportFailureSeverityLow,
s.ID(),
@@ -190,14 +201,3 @@ func (s *updateStep) Run(ctx context.Context, log logging.Logger, _ *advisor.Che
return nil, nil
}
func hasUpdate(current pluginstore.Plugin, latest *repo.PluginArchiveInfo) bool {
// If both versions are semver-valid, compare them
v1, err1 := semver.NewVersion(current.Info.Version)
v2, err2 := semver.NewVersion(latest.Version)
if err1 == nil && err2 == nil {
return v1.LessThan(v2)
}
// In other case, assume that a different latest version will always be newer
return current.Info.Version != latest.Version
}
apps/advisor/pkg/app/checks/plugincheck/check_test.go
+20 -56
View File
@@ -19,8 +19,7 @@ func TestRun(t *testing.T) {
tests := []struct {
name string
plugins []pluginstore.Plugin
pluginInfo map[string]*repo.PluginInfo
pluginArchives map[string]*repo.PluginArchiveInfo
pluginInfo []repo.PluginInfo
pluginPreinstalled []string
pluginManaged []string
pluginProvisioned []string
@@ -36,11 +35,8 @@ func TestRun(t *testing.T) {
plugins: []pluginstore.Plugin{
{JSONData: plugins.JSONData{ID: "plugin1", Name: "Plugin 1", Info: plugins.Info{Version: "1.0.0"}}},
},
pluginInfo: map[string]*repo.PluginInfo{
"plugin1": {Status: "deprecated"},
},
pluginArchives: map[string]*repo.PluginArchiveInfo{
"plugin1": {Version: "1.0.0"},
pluginInfo: []repo.PluginInfo{
{Status: "deprecated", Slug: "plugin1", Version: "1.0.0"},
},
expectedFailures: []advisor.CheckReportFailure{
{
@@ -62,11 +58,8 @@ func TestRun(t *testing.T) {
plugins: []pluginstore.Plugin{
{JSONData: plugins.JSONData{ID: "plugin2", Name: "Plugin 2", Info: plugins.Info{Version: "1.0.0"}}},
},
pluginInfo: map[string]*repo.PluginInfo{
"plugin2": {Status: "active"},
},
pluginArchives: map[string]*repo.PluginArchiveInfo{
"plugin2": {Version: "1.1.0"},
pluginInfo: []repo.PluginInfo{
{Status: "active", Slug: "plugin2", Version: "1.1.0"},
},
expectedFailures: []advisor.CheckReportFailure{
{
@@ -88,37 +81,18 @@ func TestRun(t *testing.T) {
plugins: []pluginstore.Plugin{
{JSONData: plugins.JSONData{ID: "plugin2", Name: "Plugin 2", Info: plugins.Info{Version: "alpha"}}},
},
pluginInfo: map[string]*repo.PluginInfo{
"plugin2": {Status: "active"},
},
pluginArchives: map[string]*repo.PluginArchiveInfo{
"plugin2": {Version: "beta"},
},
expectedFailures: []advisor.CheckReportFailure{
{
Severity: advisor.CheckReportFailureSeverityLow,
StepID: "update",
Item: "Plugin 2",
ItemID: "plugin2",
Links: []advisor.CheckErrorLink{
{
Url: "/plugins/plugin2?page=version-history",
Message: "Upgrade",
},
},
},
pluginInfo: []repo.PluginInfo{
{Status: "active", Slug: "plugin2", Version: "beta"},
},
expectedFailures: []advisor.CheckReportFailure{}, // Cannot be compared because the version is not semver
},
{
name: "Plugin pinned",
plugins: []pluginstore.Plugin{
{JSONData: plugins.JSONData{ID: "plugin3", Name: "Plugin 3", Info: plugins.Info{Version: "1.0.0"}}},
},
pluginInfo: map[string]*repo.PluginInfo{
"plugin3": {Status: "deprecated"}, // This should be ignored
},
pluginArchives: map[string]*repo.PluginArchiveInfo{
"plugin3": {Version: "1.1.0"},
pluginInfo: []repo.PluginInfo{
{Status: "deprecated", Slug: "plugin3"}, // This should be ignored
},
pluginPreinstalled: []string{"plugin3"},
expectedFailures: []advisor.CheckReportFailure{},
@@ -128,11 +102,8 @@ func TestRun(t *testing.T) {
plugins: []pluginstore.Plugin{
{JSONData: plugins.JSONData{ID: "plugin4", Name: "Plugin 4", Info: plugins.Info{Version: "1.0.0"}}},
},
pluginInfo: map[string]*repo.PluginInfo{
"plugin4": {Status: "deprecated"}, // This should be ignored
},
pluginArchives: map[string]*repo.PluginArchiveInfo{
"plugin4": {Version: "1.1.0"},
pluginInfo: []repo.PluginInfo{
{Status: "deprecated", Slug: "plugin4", Version: "1.1.0"}, // This should be ignored
},
pluginManaged: []string{"plugin4"},
expectedFailures: []advisor.CheckReportFailure{},
@@ -142,11 +113,8 @@ func TestRun(t *testing.T) {
plugins: []pluginstore.Plugin{
{JSONData: plugins.JSONData{ID: "plugin5", Name: "Plugin 5", Info: plugins.Info{Version: "1.0.0"}}},
},
pluginInfo: map[string]*repo.PluginInfo{
"plugin5": {Status: "deprecated"}, // This should be ignored
},
pluginArchives: map[string]*repo.PluginArchiveInfo{
"plugin5": {Version: "1.1.0"},
pluginInfo: []repo.PluginInfo{
{Status: "deprecated", Slug: "plugin5", Version: "1.1.0"}, // This should be ignored
},
pluginProvisioned: []string{"plugin5"},
expectedFailures: []advisor.CheckReportFailure{},
@@ -157,8 +125,7 @@ func TestRun(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
pluginStore := &mockPluginStore{plugins: tt.plugins}
pluginRepo := &mockPluginRepo{
pluginInfo: tt.pluginInfo,
pluginArchiveInfo: tt.pluginArchives,
pluginInfo: tt.pluginInfo,
}
pluginPreinstall := &mockPluginPreinstall{pinned: tt.pluginPreinstalled}
managedPlugins := &mockManagedPlugins{managed: tt.pluginManaged}
@@ -169,6 +136,8 @@ func TestRun(t *testing.T) {
items, err := check.Items(context.Background())
assert.NoError(t, err)
failures := []advisor.CheckReportFailure{}
err = check.Init(context.Background())
assert.NoError(t, err)
for _, step := range check.Steps() {
for _, item := range items {
stepFailures, err := step.Run(context.Background(), logging.DefaultLogger, &advisor.CheckSpec{}, item)
@@ -196,16 +165,11 @@ func (m *mockPluginStore) Plugins(ctx context.Context, t ...plugins.Type) []plug
type mockPluginRepo struct {
repo.Service
pluginInfo map[string]*repo.PluginInfo
pluginArchiveInfo map[string]*repo.PluginArchiveInfo
pluginInfo []repo.PluginInfo
}
func (m *mockPluginRepo) PluginInfo(ctx context.Context, id string, compatOpts repo.CompatOpts) (*repo.PluginInfo, error) {
return m.pluginInfo[id], nil
}
func (m *mockPluginRepo) GetPluginArchiveInfo(ctx context.Context, id, version string, opts repo.CompatOpts) (*repo.PluginArchiveInfo, error) {
return m.pluginArchiveInfo[id], nil
func (m *mockPluginRepo) GetPluginsInfo(ctx context.Context, compatOpts repo.CompatOpts) ([]repo.PluginInfo, error) {
return m.pluginInfo, nil
}
type mockPluginPreinstall struct {
apps/advisor/pkg/app/utils.go
+8
View File
@@ -42,6 +42,10 @@ func processCheck(ctx context.Context, log logging.Logger, client resource.Clien
return fmt.Errorf("invalid object type")
}
// Get the items to check
err := check.Init(ctx)
if err != nil {
return fmt.Errorf("error initializing check: %w", err)
}
items, err := check.Items(ctx)
if err != nil {
setErr := checks.SetStatusAnnotation(ctx, client, obj, checks.StatusAnnotationError)
@@ -95,6 +99,10 @@ func processCheckRetry(ctx context.Context, log logging.Logger, client resource.
return fmt.Errorf("invalid object type")
}
// Get the items to check
err := check.Init(ctx)
if err != nil {
return fmt.Errorf("error initializing check: %w", err)
}
item, err := check.Item(ctx, itemToRetry)
if err != nil {
setErr := checks.SetStatusAnnotation(ctx, client, obj, checks.StatusAnnotationError)
apps/advisor/pkg/app/utils_test.go
+4
View File
@@ -252,6 +252,10 @@ func (m *mockCheck) Item(ctx context.Context, id string) (any, error) {
return m.items[0], nil
}
func (m *mockCheck) Init(ctx context.Context) error {
return nil
}
func (m *mockCheck) Steps() []checks.Step {
return []checks.Step{
&mockStep{err: m.err, panics: m.runPanics},