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

Advisor: Fix issues (#106612)

Browse Source
This commit is contained in:
Andres Martinez Gotor
2025-06-12 15:37:07 +00:00
committed by GitHub
parent 8e815159b1
commit a3e85d8319
3 changed files with 76 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/advisor/pkg/app/app.go
+2 -2
View File
@@ -71,7 +71,7 @@ func New(cfg app.Config) (app.App, error) {
logger.Error("Error getting requester", "error", err)
return
}
ctx = identity.WithRequester(context.Background(), requester)
ctx = identity.WithServiceIdentityContext(context.WithoutCancel(ctx), requester.GetOrgID())
err = processCheck(ctx, logger, client, typesClient, req.Object, check)
if err != nil {
logger.Error("Error processing check", "error", err)
@@ -87,7 +87,7 @@ func New(cfg app.Config) (app.App, error) {
logger.Error("Error getting requester", "error", err)
return
}
ctx = identity.WithRequester(context.Background(), requester)
ctx = identity.WithServiceIdentityContext(context.WithoutCancel(ctx), requester.GetOrgID())
err = processCheckRetry(ctx, logger, client, typesClient, req.Object, check)
if err != nil {
logger.Error("Error processing check retry", "error", err)
apps/advisor/pkg/app/checkscheduler/checkscheduler.go
+24 -3
View File
@@ -22,10 +22,16 @@ import (
const defaultEvaluationInterval = 7 * 24 * time.Hour // 7 days
const defaultMaxHistory = 10
var (
waitInterval = 5 * time.Second
waitMaxRetries = 3
)
// Runner is a "runnable" app used to be able to expose and API endpoint
// with the existing checks types. This does not need to be a CRUD resource, but it is
// the only way existing at the moment to expose the check types.
type Runner struct {
checkRegistry checkregistry.CheckService
client resource.Client
typesClient resource.Client
evaluationInterval time.Duration
@@ -41,6 +47,7 @@ func New(cfg app.Config, log logging.Logger) (app.Runnable, error) {
if !ok {
return nil, fmt.Errorf("invalid config type")
}
checkRegistry := specificConfig.CheckRegistry
evalInterval, err := getEvaluationInterval(specificConfig.PluginConfig)
if err != nil {
return nil, err
@@ -66,6 +73,7 @@ func New(cfg app.Config, log logging.Logger) (app.Runnable, error) {
}
return &Runner{
checkRegistry: checkRegistry,
client: client,
typesClient: typesClient,
evaluationInterval: evalInterval,
@@ -85,7 +93,7 @@ func (r *Runner) Run(ctx context.Context) error {
} else {
// do an initial creation if necessary
if lastCreated.IsZero() {
err = r.createChecks(ctx)
err = r.createChecks(ctx, logger)
if err != nil {
logger.Error("Error creating new check reports", "error", err)
} else {
@@ -101,7 +109,7 @@ func (r *Runner) Run(ctx context.Context) error {
for {
select {
case <-ticker.C:
err = r.createChecks(ctx)
err = r.createChecks(ctx, logger)
if err != nil {
logger.Error("Error creating new check reports", "error", err)
}
@@ -150,12 +158,25 @@ func (r *Runner) checkLastCreated(ctx context.Context, log logging.Logger) (time
}
// createChecks creates a new check for each check type in the registry.
func (r *Runner) createChecks(ctx context.Context) error {
func (r *Runner) createChecks(ctx context.Context, logger logging.Logger) error {
// List existing CheckType objects
list, err := r.typesClient.List(ctx, r.namespace, resource.ListOptions{})
if err != nil {
return fmt.Errorf("error listing check types: %w", err)
}
// This may be run before the check types are registered, so we need to wait for them to be registered.
allChecksRegistered := len(list.GetItems()) == len(r.checkRegistry.Checks())
retryCount := 0
for !allChecksRegistered && retryCount < waitMaxRetries {
logger.Error("Waiting for all check types to be registered", "retryCount", retryCount, "waitInterval", waitInterval)
time.Sleep(waitInterval)
list, err = r.typesClient.List(ctx, r.namespace, resource.ListOptions{})
if err != nil {
return fmt.Errorf("error listing check types: %w", err)
}
allChecksRegistered = len(list.GetItems()) == len(r.checkRegistry.Checks())
retryCount++
}
// Create checks for each CheckType
for _, item := range list.GetItems() {
apps/advisor/pkg/app/checkscheduler/checkscheduler_test.go
+50 -21
View File
@@ -36,7 +36,7 @@ func TestRunner_Run(t *testing.T) {
runner := &Runner{
client: mockClient,
typesClient: mockTypesClient,
log: logging.DefaultLogger,
log: &logging.NoOpLogger{},
evaluationInterval: 1 * time.Hour,
}
@@ -56,10 +56,10 @@ func TestRunner_checkLastCreated_ErrorOnList(t *testing.T) {
runner := &Runner{
client: mockClient,
log: logging.DefaultLogger,
log: &logging.NoOpLogger{},
}
lastCreated, err := runner.checkLastCreated(context.Background(), logging.DefaultLogger)
lastCreated, err := runner.checkLastCreated(context.Background(), &logging.NoOpLogger{})
assert.Error(t, err)
assert.True(t, lastCreated.IsZero())
}
@@ -89,10 +89,10 @@ func TestRunner_checkLastCreated_UnprocessedCheck(t *testing.T) {
runner := &Runner{
client: mockClient,
log: logging.DefaultLogger,
log: &logging.NoOpLogger{},
}
lastCreated, err := runner.checkLastCreated(context.Background(), logging.DefaultLogger)
lastCreated, err := runner.checkLastCreated(context.Background(), &logging.NoOpLogger{})
assert.NoError(t, err)
assert.True(t, lastCreated.IsZero())
assert.Equal(t, "check-1", identifier.Name)
@@ -104,6 +104,8 @@ func TestRunner_checkLastCreated_UnprocessedCheck(t *testing.T) {
}
func TestRunner_createChecks_ErrorOnCreate(t *testing.T) {
mockCheckService := &MockCheckService{checks: []checks.Check{&mockCheck{id: "check-1"}}}
mockClient := &MockClient{
createFunc: func(ctx context.Context, id resource.Identifier, obj resource.Object, opts resource.CreateOptions) (resource.Object, error) {
return nil, errors.New("create error")
@@ -121,16 +123,19 @@ func TestRunner_createChecks_ErrorOnCreate(t *testing.T) {
}
runner := &Runner{
client: mockClient,
typesClient: mockTypesClient,
log: logging.DefaultLogger,
checkRegistry: mockCheckService,
client: mockClient,
typesClient: mockTypesClient,
log: &logging.NoOpLogger{},
}
err := runner.createChecks(context.Background())
err := runner.createChecks(context.Background(), &logging.NoOpLogger{})
assert.Error(t, err)
}
func TestRunner_createChecks_Success(t *testing.T) {
mockCheckService := &MockCheckService{checks: []checks.Check{&mockCheck{id: "check-1"}}}
mockClient := &MockClient{
createFunc: func(ctx context.Context, id resource.Identifier, obj resource.Object, opts resource.CreateOptions) (resource.Object, error) {
return &advisorv0alpha1.Check{}, nil
@@ -148,12 +153,13 @@ func TestRunner_createChecks_Success(t *testing.T) {
}
runner := &Runner{
client: mockClient,
typesClient: mockTypesClient,
log: logging.DefaultLogger,
checkRegistry: mockCheckService,
client: mockClient,
typesClient: mockTypesClient,
log: &logging.NoOpLogger{},
}
err := runner.createChecks(context.Background())
err := runner.createChecks(context.Background(), &logging.NoOpLogger{})
assert.NoError(t, err)
}
@@ -166,10 +172,10 @@ func TestRunner_cleanupChecks_ErrorOnList(t *testing.T) {
runner := &Runner{
client: mockClient,
log: logging.DefaultLogger,
log: &logging.NoOpLogger{},
}
err := runner.cleanupChecks(context.Background(), logging.DefaultLogger)
err := runner.cleanupChecks(context.Background(), &logging.NoOpLogger{})
assert.Error(t, err)
}
@@ -187,10 +193,10 @@ func TestRunner_cleanupChecks_WithinMax(t *testing.T) {
runner := &Runner{
client: mockClient,
log: logging.DefaultLogger,
log: &logging.NoOpLogger{},
}
err := runner.cleanupChecks(context.Background(), logging.DefaultLogger)
err := runner.cleanupChecks(context.Background(), &logging.NoOpLogger{})
assert.NoError(t, err)
}
@@ -217,9 +223,9 @@ func TestRunner_cleanupChecks_ErrorOnDelete(t *testing.T) {
runner := &Runner{
client: mockClient,
maxHistory: defaultMaxHistory,
log: logging.DefaultLogger,
log: &logging.NoOpLogger{},
}
err := runner.cleanupChecks(context.Background(), logging.DefaultLogger)
err := runner.cleanupChecks(context.Background(), &logging.NoOpLogger{})
assert.ErrorContains(t, err, "delete error")
}
@@ -253,9 +259,9 @@ func TestRunner_cleanupChecks_Success(t *testing.T) {
runner := &Runner{
client: mockClient,
maxHistory: defaultMaxHistory,
log: logging.DefaultLogger,
log: &logging.NoOpLogger{},
}
err := runner.cleanupChecks(context.Background(), logging.DefaultLogger)
err := runner.cleanupChecks(context.Background(), &logging.NoOpLogger{})
assert.NoError(t, err)
assert.Equal(t, []string{"check-0"}, itemsDeleted)
}
@@ -334,3 +340,26 @@ func (m *MockClient) Delete(ctx context.Context, identifier resource.Identifier,
func (m *MockClient) PatchInto(ctx context.Context, identifier resource.Identifier, patch resource.PatchRequest, options resource.PatchOptions, into resource.Object) error {
return m.patchFunc(ctx, identifier, patch, options, into)
}
type MockCheckService struct {
checks []checks.Check
}
func (m *MockCheckService) Checks() []checks.Check {
return m.checks
}
type mockCheck struct {
checks.Check
id string
steps []checks.Step
}
func (m *mockCheck) ID() string {
return m.id
}
func (m *mockCheck) Steps() []checks.Step {
return m.steps
}