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

Advisor: Cloud fixes (#101136)

Browse Source
This commit is contained in:
Andres Martinez Gotor
2025-02-25 11:41:44 +01:00
committed by GitHub
parent 03e94e7a3e
commit 120b277664
11 changed files with 162 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/advisor/pkg/app/authorizer.go
+1 -1
View File
@@ -22,7 +22,7 @@ func GetAuthorizer() authorizer.Authorizer {
}
// check if is admin
if u.GetIsGrafanaAdmin() {
if u.HasRole(identity.RoleAdmin) {
return authorizer.DecisionAllow, "", nil
}
apps/advisor/pkg/app/authorizer_test.go
+4
View File
@@ -75,4 +75,8 @@ func (m *mockUser) GetIsGrafanaAdmin() bool {
return m.isGrafanaAdmin
}
func (m *mockUser) HasRole(role identity.RoleType) bool {
return role == identity.RoleAdmin && m.isGrafanaAdmin
}
// Implement other methods of identity.Requester as needed
apps/advisor/pkg/app/checkregistry/checkregistry.go
+1
View File
@@ -62,4 +62,5 @@ func (s *Service) Checks() []checks.Check {
type AdvisorAppConfig struct {
CheckRegistry CheckService
PluginConfig map[string]string
StackID string
}
apps/advisor/pkg/app/checks/datasourcecheck/check.go
+16 -2
View File
@@ -2,17 +2,18 @@ package datasourcecheck
import (
"context"
"errors"
"fmt"
"github.com/grafana/grafana-plugin-sdk-go/backend"
advisor "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1"
"github.com/grafana/grafana/apps/advisor/pkg/app/checks"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore"
"github.com/grafana/grafana/pkg/util"
"k8s.io/klog/v2"
)
type check struct {
@@ -20,6 +21,7 @@ type check struct {
PluginStore pluginstore.Store
PluginContextProvider pluginContextProvider
PluginClient plugins.Client
log log.Logger
}
func New(
@@ -33,6 +35,7 @@ func New(
PluginStore: pluginStore,
PluginContextProvider: pluginContextProvider,
PluginClient: pluginClient,
log: log.New("advisor.datasourcecheck"),
}
}
@@ -58,6 +61,7 @@ func (c *check) Steps() []checks.Step {
&healthCheckStep{
PluginContextProvider: c.PluginContextProvider,
PluginClient: c.PluginClient,
log: c.log,
},
}
}
@@ -102,6 +106,7 @@ func (s *uidValidationStep) Run(ctx context.Context, obj *advisor.CheckSpec, i a
type healthCheckStep struct {
PluginContextProvider pluginContextProvider
PluginClient plugins.Client
log log.Logger
}
func (s *healthCheckStep) Title() string {
@@ -134,7 +139,7 @@ func (s *healthCheckStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any
pCtx, err := s.PluginContextProvider.GetWithDataSource(ctx, ds.Type, requester, ds)
if err != nil {
// Unable to check health check
klog.Error("Failed to get plugin context", "datasource_uid", ds.UID, "error", err)
s.log.Error("Failed to get plugin context", "datasource_uid", ds.UID, "error", err)
return nil, nil
}
req := &backend.CheckHealthRequest{
@@ -143,6 +148,15 @@ func (s *healthCheckStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any
}
resp, err := s.PluginClient.CheckHealth(ctx, req)
if err != nil || resp.Status != backend.HealthStatusOk {
if err != nil {
s.log.Debug("Failed to check health", "datasource_uid", ds.UID, "error", err)
if errors.Is(err, plugins.ErrMethodNotImplemented) || errors.Is(err, plugins.ErrPluginUnavailable) {
// The plugin does not support backend health checks
return nil, nil
}
} else {
s.log.Debug("Failed to check health", "datasource_uid", ds.UID, "status", resp.Status, "message", resp.Message)
}
return checks.NewCheckReportFailure(
advisor.CheckReportFailureSeverityHigh,
s.ID(),
apps/advisor/pkg/app/checks/datasourcecheck/check_test.go
+40 -1
View File
@@ -7,6 +7,7 @@ import (
"github.com/grafana/grafana-plugin-sdk-go/backend"
advisor "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/user"
@@ -28,6 +29,7 @@ func TestCheck_Run(t *testing.T) {
DatasourceSvc: mockDatasourceSvc,
PluginContextProvider: mockPluginContextProvider,
PluginClient: mockPluginClient,
log: log.New("advisor.datasourcecheck"),
}
ctx := identity.WithRequester(context.Background(), &user.SignedInUser{})
@@ -62,6 +64,7 @@ func TestCheck_Run(t *testing.T) {
DatasourceSvc: mockDatasourceSvc,
PluginContextProvider: mockPluginContextProvider,
PluginClient: mockPluginClient,
log: log.New("advisor.datasourcecheck"),
}
ctx := identity.WithRequester(context.Background(), &user.SignedInUser{})
@@ -97,6 +100,7 @@ func TestCheck_Run(t *testing.T) {
DatasourceSvc: mockDatasourceSvc,
PluginContextProvider: mockPluginContextProvider,
PluginClient: mockPluginClient,
log: log.New("advisor.datasourcecheck"),
}
ctx := identity.WithRequester(context.Background(), &user.SignedInUser{})
@@ -118,6 +122,40 @@ func TestCheck_Run(t *testing.T) {
assert.Len(t, failures, 1)
assert.Equal(t, "health-check", failures[0].StepID)
})
t.Run("should skip health check when plugin does not support backend health checks", func(t *testing.T) {
datasources := []*datasources.DataSource{
{UID: "valid-uid-1", Type: "prometheus", Name: "Prometheus"},
}
mockDatasourceSvc := &MockDatasourceSvc{dss: datasources}
mockPluginContextProvider := &MockPluginContextProvider{pCtx: backend.PluginContext{}}
mockPluginClient := &MockPluginClient{err: plugins.ErrMethodNotImplemented}
check := &check{
DatasourceSvc: mockDatasourceSvc,
PluginContextProvider: mockPluginContextProvider,
PluginClient: mockPluginClient,
log: log.New("advisor.datasourcecheck"),
}
ctx := identity.WithRequester(context.Background(), &user.SignedInUser{})
items, err := check.Items(ctx)
assert.NoError(t, err)
failures := []advisor.CheckReportFailure{}
for _, step := range check.Steps() {
for _, item := range items {
stepFailures, err := step.Run(ctx, &advisor.CheckSpec{}, item)
assert.NoError(t, err)
if stepFailures != nil {
failures = append(failures, *stepFailures)
}
}
}
assert.NoError(t, err)
assert.Equal(t, 1, len(items))
assert.Len(t, failures, 0)
})
}
type MockDatasourceSvc struct {
@@ -142,8 +180,9 @@ type MockPluginClient struct {
plugins.Client
res *backend.CheckHealthResult
err error
}
func (m *MockPluginClient) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) {
return m.res, nil
return m.res, m.err
}
apps/advisor/pkg/app/checks/utils.go
+16
View File
@@ -1,7 +1,12 @@
package checks
import (
"fmt"
"strconv"
"github.com/grafana/authlib/types"
advisor "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
const (
@@ -22,3 +27,14 @@ func NewCheckReportFailure(
Links: links,
}
}
func GetNamespace(stackID string) (string, error) {
if stackID == "" {
return metav1.NamespaceDefault, nil
}
stackId, err := strconv.ParseInt(stackID, 10, 64)
if err != nil {
return "", fmt.Errorf("invalid stack id: %s", stackID)
}
return types.CloudNamespaceFormatter(stackId), nil
}
apps/advisor/pkg/app/checks/utils_test.go
+46
View File
@@ -0,0 +1,46 @@
package checks
import (
"testing"
"github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
func TestGetNamespace(t *testing.T) {
tests := []struct {
name string
input string
expected string
expectedErr string
}{
{
name: "empty stack ID",
input: "",
expected: metav1.NamespaceDefault,
},
{
name: "valid stack ID",
input: "1234567890",
expected: "stacks-1234567890",
},
{
name: "invalid stack ID",
input: "invalid",
expected: "",
expectedErr: "invalid stack id: invalid",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result, err := GetNamespace(tt.input)
if tt.expectedErr != "" {
assert.EqualError(t, err, tt.expectedErr)
} else {
assert.NoError(t, err)
}
assert.Equal(t, tt.expected, result)
})
}
}
apps/advisor/pkg/app/checkscheduler/checkscheduler.go
+9 -3
View File
@@ -29,6 +29,7 @@ type Runner struct {
client resource.Client
evaluationInterval time.Duration
maxHistory int
namespace string
}
// NewRunner creates a new Runner.
@@ -47,6 +48,10 @@ func New(cfg app.Config) (app.Runnable, error) {
if err != nil {
return nil, err
}
namespace, err := checks.GetNamespace(specificConfig.StackID)
if err != nil {
return nil, err
}
// Prepare storage client
clientGenerator := k8s.NewClientRegistry(cfg.KubeConfig, k8s.ClientConfig{})
@@ -60,6 +65,7 @@ func New(cfg app.Config) (app.Runnable, error) {
client: client,
evaluationInterval: evalInterval,
maxHistory: maxHistory,
namespace: namespace,
}, nil
}
@@ -114,7 +120,7 @@ func (r *Runner) Run(ctx context.Context) error {
// regardless of its ID. This assumes that the checks are created in batches
// so a batch will have a similar creation time.
func (r *Runner) checkLastCreated(ctx context.Context) (time.Time, error) {
list, err := r.client.List(ctx, metav1.NamespaceDefault, resource.ListOptions{})
list, err := r.client.List(ctx, r.namespace, resource.ListOptions{})
if err != nil {
return time.Time{}, err
}
@@ -134,7 +140,7 @@ func (r *Runner) createChecks(ctx context.Context) error {
obj := &advisorv0alpha1.Check{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "check-",
Namespace: metav1.NamespaceDefault,
Namespace: r.namespace,
Labels: map[string]string{
checks.TypeLabel: check.ID(),
},
@@ -152,7 +158,7 @@ func (r *Runner) createChecks(ctx context.Context) error {
// cleanupChecks deletes the olders checks if the number of checks exceeds the limit.
func (r *Runner) cleanupChecks(ctx context.Context) error {
list, err := r.client.List(ctx, metav1.NamespaceDefault, resource.ListOptions{Limit: -1})
list, err := r.client.List(ctx, r.namespace, resource.ListOptions{Limit: -1})
if err != nil {
return err
}
apps/advisor/pkg/app/checktyperegisterer/checktyperegisterer.go
+8 -1
View File
@@ -9,6 +9,7 @@ import (
"github.com/grafana/grafana-app-sdk/resource"
advisorv0alpha1 "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1"
"github.com/grafana/grafana/apps/advisor/pkg/app/checkregistry"
"github.com/grafana/grafana/apps/advisor/pkg/app/checks"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
@@ -19,6 +20,7 @@ import (
type Runner struct {
checkRegistry checkregistry.CheckService
client resource.Client
namespace string
}
// NewRunner creates a new Runner.
@@ -29,6 +31,10 @@ func New(cfg app.Config) (app.Runnable, error) {
return nil, fmt.Errorf("invalid config type")
}
checkRegistry := specificConfig.CheckRegistry
namespace, err := checks.GetNamespace(specificConfig.StackID)
if err != nil {
return nil, err
}
// Prepare storage client
clientGenerator := k8s.NewClientRegistry(cfg.KubeConfig, k8s.ClientConfig{})
@@ -40,6 +46,7 @@ func New(cfg app.Config) (app.Runnable, error) {
return &Runner{
checkRegistry: checkRegistry,
client: client,
namespace: namespace,
}, nil
}
@@ -58,7 +65,7 @@ func (r *Runner) Run(ctx context.Context) error {
obj := &advisorv0alpha1.CheckType{
ObjectMeta: metav1.ObjectMeta{
Name: t.ID(),
Namespace: metav1.NamespaceDefault,
Namespace: r.namespace,
},
Spec: advisorv0alpha1.CheckTypeSpec{
Name: t.ID(),
apps/advisor/pkg/app/checktyperegisterer/checktyperegisterer_test.go
+20
View File
@@ -3,6 +3,7 @@ package checktyperegisterer
import (
"context"
"errors"
"fmt"
"testing"
"github.com/grafana/grafana-app-sdk/resource"
@@ -88,6 +89,24 @@ func TestCheckTypesRegisterer_Run(t *testing.T) {
},
expectedErr: errors.New("update error"),
},
{
name: "custom namespace",
checks: []checks.Check{
&mockCheck{
id: "check1",
steps: []checks.Step{
&mockStep{id: "step1", title: "Step 1", description: "Description 1"},
},
},
},
createFunc: func(ctx context.Context, id resource.Identifier, obj resource.Object, opts resource.CreateOptions) (resource.Object, error) {
if obj.GetNamespace() != "custom-namespace" {
return nil, fmt.Errorf("expected namespace %s, got %s", "custom-namespace", obj.GetNamespace())
}
return obj, nil
},
expectedErr: nil,
},
}
for _, tt := range tests {
@@ -98,6 +117,7 @@ func TestCheckTypesRegisterer_Run(t *testing.T) {
createFunc: tt.createFunc,
updateFunc: tt.updateFunc,
},
namespace: "custom-namespace",
}
err := r.Run(context.Background())
if err != nil {