diff --git a/apps/advisor/pkg/app/app.go b/apps/advisor/pkg/app/app.go
index 42848758d5a..554357caca2 100644
--- a/apps/advisor/pkg/app/app.go
+++ b/apps/advisor/pkg/app/app.go
@@ -37,7 +37,7 @@ func New(cfg app.Config) (app.App, error) {
// Initialize checks
checkMap := map[string]checks.Check{}
for _, c := range checkRegistry.Checks() {
- checkMap[c.Type()] = c
+ checkMap[c.ID()] = c
}
simpleConfig := simple.AppConfig{
diff --git a/apps/advisor/pkg/app/checks/datasourcecheck/check.go b/apps/advisor/pkg/app/checks/datasourcecheck/check.go
index df05bf40688..f7dbcc53024 100644
--- a/apps/advisor/pkg/app/checks/datasourcecheck/check.go
+++ b/apps/advisor/pkg/app/checks/datasourcecheck/check.go
@@ -15,6 +15,13 @@ import (
"k8s.io/klog/v2"
)
+type check struct {
+ DatasourceSvc datasources.DataSourceService
+ PluginStore pluginstore.Store
+ PluginContextProvider pluginContextProvider
+ PluginClient plugins.Client
+}
+
func New(
datasourceSvc datasources.DataSourceService,
pluginStore pluginstore.Store,
@@ -29,28 +36,53 @@ func New(
}
}
-type check struct {
- DatasourceSvc datasources.DataSourceService
- PluginStore pluginstore.Store
- PluginContextProvider pluginContextProvider
- PluginClient plugins.Client
-}
-
-func (c *check) Type() string {
- return "datasource"
-}
-
-func (c *check) Run(ctx context.Context, obj *advisor.CheckSpec) (*advisor.CheckV0alpha1StatusReport, error) {
- // Optionally read the check input encoded in the object
- // fmt.Println(obj.Data)
-
+func (c *check) Items(ctx context.Context) ([]any, error) {
dss, err := c.DatasourceSvc.GetAllDataSources(ctx, &datasources.GetAllDataSourcesQuery{})
if err != nil {
return nil, err
}
+ res := make([]any, len(dss))
+ for i, ds := range dss {
+ res[i] = ds
+ }
+ return res, nil
+}
+func (c *check) ID() string {
+ return "datasource"
+}
+
+func (c *check) Steps() []checks.Step {
+ return []checks.Step{
+ &uidValidationStep{},
+ &healthCheckStep{
+ PluginContextProvider: c.PluginContextProvider,
+ PluginClient: c.PluginClient,
+ },
+ }
+}
+
+type uidValidationStep struct{}
+
+func (s *uidValidationStep) ID() string {
+ return "uid-validation"
+}
+
+func (s *uidValidationStep) Title() string {
+ return "UID validation"
+}
+
+func (s *uidValidationStep) Description() string {
+ return "Check if the UID of each data source is valid."
+}
+
+func (s *uidValidationStep) Run(ctx context.Context, obj *advisor.CheckSpec, items []any) ([]advisor.CheckReportError, error) {
dsErrs := []advisor.CheckReportError{}
- for _, ds := range dss {
+ for _, i := range items {
+ ds, ok := i.(*datasources.DataSource)
+ if !ok {
+ return nil, fmt.Errorf("invalid item type %T", i)
+ }
// Data source UID validation
err := util.ValidateUID(ds.UID)
if err != nil {
@@ -60,13 +92,41 @@ func (c *check) Run(ctx context.Context, obj *advisor.CheckSpec) (*advisor.Check
Action: "Check the documentation for more information.",
})
}
+ }
+ return dsErrs, nil
+}
+
+type healthCheckStep struct {
+ PluginContextProvider pluginContextProvider
+ PluginClient plugins.Client
+}
+
+func (s *healthCheckStep) Title() string {
+ return "Health check"
+}
+
+func (s *healthCheckStep) Description() string {
+ return "Check if all data sources are healthy."
+}
+
+func (s *healthCheckStep) ID() string {
+ return "health-check"
+}
+
+func (s *healthCheckStep) Run(ctx context.Context, obj *advisor.CheckSpec, items []any) ([]advisor.CheckReportError, error) {
+ dsErrs := []advisor.CheckReportError{}
+ for _, i := range items {
+ ds, ok := i.(*datasources.DataSource)
+ if !ok {
+ return nil, fmt.Errorf("invalid item type %T", i)
+ }
// Health check execution
requester, err := identity.GetRequester(ctx)
if err != nil {
return nil, err
}
- pCtx, err := c.PluginContextProvider.GetWithDataSource(ctx, ds.Type, requester, ds)
+ pCtx, err := s.PluginContextProvider.GetWithDataSource(ctx, ds.Type, requester, ds)
if err != nil {
klog.ErrorS(err, "Error creating plugin context", "datasource", ds.Name)
continue
@@ -75,7 +135,7 @@ func (c *check) Run(ctx context.Context, obj *advisor.CheckSpec) (*advisor.Check
PluginContext: pCtx,
Headers: map[string]string{},
}
- resp, err := c.PluginClient.CheckHealth(ctx, req)
+ resp, err := s.PluginClient.CheckHealth(ctx, req)
if err != nil {
fmt.Println("Error checking health", err)
continue
@@ -90,11 +150,7 @@ func (c *check) Run(ctx context.Context, obj *advisor.CheckSpec) (*advisor.Check
})
}
}
-
- return &advisor.CheckV0alpha1StatusReport{
- Count: int64(len(dss)),
- Errors: dsErrs,
- }, nil
+ return dsErrs, nil
}
type pluginContextProvider interface {
diff --git a/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go b/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go
index 5c3dfb913e3..c360f96ad14 100644
--- a/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go
+++ b/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go
@@ -31,11 +31,18 @@ func TestCheck_Run(t *testing.T) {
}
ctx := identity.WithRequester(context.Background(), &user.SignedInUser{})
- report, err := check.Run(ctx, &advisor.CheckSpec{})
+ items, err := check.Items(ctx)
+ assert.NoError(t, err)
+ errs := []advisor.CheckReportError{}
+ for _, step := range check.Steps() {
+ stepErrs, err := step.Run(ctx, &advisor.CheckSpec{}, items)
+ assert.NoError(t, err)
+ errs = append(errs, stepErrs...)
+ }
assert.NoError(t, err)
- assert.Equal(t, int64(2), report.Count)
- assert.Empty(t, report.Errors)
+ assert.Equal(t, 2, len(items))
+ assert.Empty(t, errs)
})
t.Run("should return errors when datasource UID is invalid", func(t *testing.T) {
@@ -54,12 +61,19 @@ func TestCheck_Run(t *testing.T) {
}
ctx := identity.WithRequester(context.Background(), &user.SignedInUser{})
- report, err := check.Run(ctx, &advisor.CheckSpec{})
+ items, err := check.Items(ctx)
+ assert.NoError(t, err)
+ errs := []advisor.CheckReportError{}
+ for _, step := range check.Steps() {
+ stepErrs, err := step.Run(ctx, &advisor.CheckSpec{}, items)
+ assert.NoError(t, err)
+ errs = append(errs, stepErrs...)
+ }
assert.NoError(t, err)
- assert.Equal(t, int64(1), report.Count)
- assert.Len(t, report.Errors, 1)
- assert.Equal(t, "Invalid UID 'invalid uid' for data source Prometheus", report.Errors[0].Reason)
+ assert.Equal(t, 1, len(items))
+ assert.Len(t, errs, 1)
+ assert.Equal(t, "Invalid UID 'invalid uid' for data source Prometheus", errs[0].Reason)
})
t.Run("should return errors when datasource health check fails", func(t *testing.T) {
@@ -78,12 +92,19 @@ func TestCheck_Run(t *testing.T) {
}
ctx := identity.WithRequester(context.Background(), &user.SignedInUser{})
- report, err := check.Run(ctx, &advisor.CheckSpec{})
+ items, err := check.Items(ctx)
+ assert.NoError(t, err)
+ errs := []advisor.CheckReportError{}
+ for _, step := range check.Steps() {
+ stepErrs, err := step.Run(ctx, &advisor.CheckSpec{}, items)
+ assert.NoError(t, err)
+ errs = append(errs, stepErrs...)
+ }
assert.NoError(t, err)
- assert.Equal(t, int64(1), report.Count)
- assert.Len(t, report.Errors, 1)
- assert.Equal(t, "Health check failed for Prometheus", report.Errors[0].Reason)
+ assert.Equal(t, 1, len(items))
+ assert.Len(t, errs, 1)
+ assert.Equal(t, "Health check failed for Prometheus", errs[0].Reason)
})
}
diff --git a/apps/advisor/pkg/app/checks/ifaces.go b/apps/advisor/pkg/app/checks/ifaces.go
index 008f5656298..b630a84871c 100644
--- a/apps/advisor/pkg/app/checks/ifaces.go
+++ b/apps/advisor/pkg/app/checks/ifaces.go
@@ -6,8 +6,24 @@ import (
advisorv0alpha1 "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1"
)
-// Check defines the methods that a check must implement to be executed.
+// Check returns metadata about the check being executed and the list of Steps
type Check interface {
- Run(ctx context.Context, obj *advisorv0alpha1.CheckSpec) (*advisorv0alpha1.CheckV0alpha1StatusReport, error)
- Type() string
+ // ID returns the unique identifier of the check
+ ID() string
+ // Items returns the list of items that will be checked
+ Items(ctx context.Context) ([]any, error)
+ // Steps returns the list of steps that will be executed
+ Steps() []Step
+}
+
+// Step is a single step in a check, including its metadata
+type Step interface {
+ // ID returns the unique identifier of the step
+ ID() string
+ // Title returns the title of the step
+ Title() string
+ // Description returns the description of the step
+ Description() string
+ // Run executes the step and returns a list of errors
+ Run(ctx context.Context, obj *advisorv0alpha1.CheckSpec, items []any) ([]advisorv0alpha1.CheckReportError, error)
}
diff --git a/apps/advisor/pkg/app/checks/plugincheck/check.go b/apps/advisor/pkg/app/checks/plugincheck/check.go
index 8a20820f450..c64bc1e7579 100644
--- a/apps/advisor/pkg/app/checks/plugincheck/check.go
+++ b/apps/advisor/pkg/app/checks/plugincheck/check.go
@@ -36,22 +36,63 @@ type check struct {
ManagedPlugins managedplugins.Manager
}
-func (c *check) Type() string {
+func (c *check) ID() string {
return "plugin"
}
-func (c *check) Run(ctx context.Context, _ *advisor.CheckSpec) (*advisor.CheckV0alpha1StatusReport, error) {
+func (c *check) Items(ctx context.Context) ([]any, error) {
ps := c.PluginStore.Plugins(ctx)
+ res := make([]any, len(ps))
+ for i, p := range ps {
+ res[i] = p
+ }
+ return res, nil
+}
+func (c *check) Steps() []checks.Step {
+ return []checks.Step{
+ &deprecationStep{
+ PluginRepo: c.PluginRepo,
+ },
+ &updateStep{
+ PluginRepo: c.PluginRepo,
+ PluginPreinstall: c.PluginPreinstall,
+ ManagedPlugins: c.ManagedPlugins,
+ },
+ }
+}
+
+type deprecationStep struct {
+ PluginRepo repo.Service
+}
+
+func (s *deprecationStep) Title() string {
+ return "Deprecation check"
+}
+
+func (s *deprecationStep) Description() string {
+ return "Check if any installed plugins are deprecated."
+}
+
+func (s *deprecationStep) ID() string {
+ return "deprecation"
+}
+
+func (s *deprecationStep) Run(ctx context.Context, _ *advisor.CheckSpec, items []any) ([]advisor.CheckReportError, error) {
errs := []advisor.CheckReportError{}
- for _, p := range ps {
+ for _, i := range items {
+ p, ok := i.(pluginstore.Plugin)
+ if !ok {
+ return nil, fmt.Errorf("invalid item type %T", i)
+ }
+
// Skip if it's a core plugin
if p.IsCorePlugin() {
continue
}
// Check if plugin is deprecated
- i, err := c.PluginRepo.PluginInfo(ctx, p.ID)
+ i, err := s.PluginRepo.PluginInfo(ctx, p.ID)
if err != nil {
continue
}
@@ -62,13 +103,49 @@ func (c *check) Run(ctx context.Context, _ *advisor.CheckSpec) (*advisor.CheckV0
Action: "Check the documentation for recommended steps.",
})
}
+ }
+ return errs, nil
+}
- // Check if plugin has a newer version, only if it's not managed or pinned
- if c.isManaged(ctx, p.ID) || c.PluginPreinstall.IsPinned(p.ID) {
+type updateStep struct {
+ PluginRepo repo.Service
+ PluginPreinstall plugininstaller.Preinstall
+ ManagedPlugins managedplugins.Manager
+}
+
+func (s *updateStep) Title() string {
+ return "Update check"
+}
+
+func (s *updateStep) Description() string {
+ return "Check if any installed plugins have a newer version available."
+}
+
+func (s *updateStep) ID() string {
+ return "update"
+}
+
+func (s *updateStep) Run(ctx context.Context, _ *advisor.CheckSpec, items []any) ([]advisor.CheckReportError, error) {
+ errs := []advisor.CheckReportError{}
+ for _, i := range items {
+ p, ok := i.(pluginstore.Plugin)
+ if !ok {
+ return nil, fmt.Errorf("invalid item type %T", i)
+ }
+
+ // Skip if it's a core plugin
+ if p.IsCorePlugin() {
continue
}
+
+ // Skip if it's managed or pinned
+ if s.isManaged(ctx, p.ID) || s.PluginPreinstall.IsPinned(p.ID) {
+ continue
+ }
+
+ // Check if plugin has a newer version available
compatOpts := repo.NewCompatOpts(services.GrafanaVersion, sysruntime.GOOS, sysruntime.GOARCH)
- info, err := c.PluginRepo.GetPluginArchiveInfo(ctx, p.ID, "", compatOpts)
+ info, err := s.PluginRepo.GetPluginArchiveInfo(ctx, p.ID, "", compatOpts)
if err != nil {
continue
}
@@ -83,10 +160,7 @@ func (c *check) Run(ctx context.Context, _ *advisor.CheckSpec) (*advisor.CheckV0
}
}
- return &advisor.CheckV0alpha1StatusReport{
- Count: int64(len(ps)),
- Errors: errs,
- }, nil
+ return errs, nil
}
func hasUpdate(current pluginstore.Plugin, latest *repo.PluginArchiveInfo) bool {
@@ -100,8 +174,8 @@ func hasUpdate(current pluginstore.Plugin, latest *repo.PluginArchiveInfo) bool
return current.Info.Version != latest.Version
}
-func (c *check) isManaged(ctx context.Context, pluginID string) bool {
- for _, managedPlugin := range c.ManagedPlugins.ManagedPlugins(ctx) {
+func (s *updateStep) isManaged(ctx context.Context, pluginID string) bool {
+ for _, managedPlugin := range s.ManagedPlugins.ManagedPlugins(ctx) {
if managedPlugin == pluginID {
return true
}
diff --git a/apps/advisor/pkg/app/checks/plugincheck/check_test.go b/apps/advisor/pkg/app/checks/plugincheck/check_test.go
index 41c0705e904..53450ce7bc5 100644
--- a/apps/advisor/pkg/app/checks/plugincheck/check_test.go
+++ b/apps/advisor/pkg/app/checks/plugincheck/check_test.go
@@ -126,10 +126,17 @@ func TestRun(t *testing.T) {
managedPlugins := &mockManagedPlugins{managed: tt.pluginManaged}
check := New(pluginStore, pluginRepo, pluginPreinstall, managedPlugins)
- report, err := check.Run(context.Background(), nil)
+ items, err := check.Items(context.Background())
assert.NoError(t, err)
- assert.Equal(t, int64(len(tt.plugins)), report.Count)
- assert.Equal(t, tt.expectedErrors, report.Errors)
+ errs := []advisor.CheckReportError{}
+ for _, step := range check.Steps() {
+ stepErrs, err := step.Run(context.Background(), &advisor.CheckSpec{}, items)
+ assert.NoError(t, err)
+ errs = append(errs, stepErrs...)
+ }
+ assert.NoError(t, err)
+ assert.Equal(t, len(tt.plugins), len(items))
+ assert.Equal(t, tt.expectedErrors, errs)
})
}
}
diff --git a/apps/advisor/pkg/app/utils.go b/apps/advisor/pkg/app/utils.go
index 4eae090198b..c59dbaca21e 100644
--- a/apps/advisor/pkg/app/utils.go
+++ b/apps/advisor/pkg/app/utils.go
@@ -72,14 +72,32 @@ func processCheck(ctx context.Context, client resource.Client, obj resource.Obje
UserUID: uid,
FallbackType: typ,
})
- // Run the checks
- report, err := check.Run(ctx, &c.Spec)
+ // Get the items to check
+ items, err := check.Items(ctx)
if err != nil {
setErr := setStatusAnnotation(ctx, client, obj, "error")
if setErr != nil {
return setErr
}
- return err
+ return fmt.Errorf("error initializing check: %w", err)
+ }
+ // Run the steps
+ steps := check.Steps()
+ errs := []advisorv0alpha1.CheckReportError{}
+ for _, step := range steps {
+ stepErrs, err := step.Run(ctx, &c.Spec, items)
+ if err != nil {
+ setErr := setStatusAnnotation(ctx, client, obj, "error")
+ if setErr != nil {
+ return setErr
+ }
+ return fmt.Errorf("error running step %s: %w", step.Title(), err)
+ }
+ errs = append(errs, stepErrs...)
+ }
+ report := &advisorv0alpha1.CheckV0alpha1StatusReport{
+ Errors: errs,
+ Count: int64(len(items)),
}
err = setStatusAnnotation(ctx, client, obj, "processed")
if err != nil {
diff --git a/apps/advisor/pkg/app/utils_test.go b/apps/advisor/pkg/app/utils_test.go
index 6626f645eb7..a13880f8209 100644
--- a/apps/advisor/pkg/app/utils_test.go
+++ b/apps/advisor/pkg/app/utils_test.go
@@ -115,10 +115,42 @@ func (m *mockClient) PatchInto(ctx context.Context, id resource.Identifier, req
}
type mockCheck struct {
- checks.Check
err error
}
-func (m *mockCheck) Run(ctx context.Context, spec *advisorv0alpha1.CheckSpec) (*advisorv0alpha1.CheckV0alpha1StatusReport, error) {
- return &advisorv0alpha1.CheckV0alpha1StatusReport{}, m.err
+func (m *mockCheck) ID() string {
+ return "mock"
+}
+
+func (m *mockCheck) Items(ctx context.Context) ([]any, error) {
+ return []any{}, nil
+}
+
+func (m *mockCheck) Steps() []checks.Step {
+ return []checks.Step{
+ &mockStep{err: m.err},
+ }
+}
+
+type mockStep struct {
+ err error
+}
+
+func (m *mockStep) Run(ctx context.Context, obj *advisorv0alpha1.CheckSpec, items []any) ([]advisorv0alpha1.CheckReportError, error) {
+ if m.err != nil {
+ return nil, m.err
+ }
+ return nil, nil
+}
+
+func (m *mockStep) Title() string {
+ return "mock"
+}
+
+func (m *mockStep) Description() string {
+ return "mock"
+}
+
+func (m *mockStep) ID() string {
+ return "mock"
}