Advisor: Avoid automatic check creation (#111678)

This commit is contained in:
Andres Martinez Gotor
2025-10-21 15:40:00 +02:00
committed by GitHub
parent 5f9a51418c
commit 12e294d8ab
2 changed files with 322 additions and 354 deletions
@@ -23,21 +23,22 @@ const defaultEvaluationInterval = 7 * 24 * time.Hour // 7 days
const defaultMaxHistory = 10 const defaultMaxHistory = 10
var ( var (
waitInterval = 5 * time.Second waitInterval = 5 * time.Second
waitMaxRetries = 3 waitMaxRetries = 3
evalIntervalRandomVariation = 1 * time.Hour
) )
// Runner is a "runnable" app used to be able to expose and API endpoint // 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 // 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. // the only way existing at the moment to expose the check types.
type Runner struct { type Runner struct {
checkRegistry checkregistry.CheckService checkRegistry checkregistry.CheckService
client resource.Client checksClient resource.Client
typesClient resource.Client typesClient resource.Client
evaluationInterval time.Duration defaultEvalInterval time.Duration
maxHistory int maxHistory int
namespace string namespace string
log logging.Logger log logging.Logger
} }
// NewRunner creates a new Runner. // NewRunner creates a new Runner.
@@ -73,13 +74,13 @@ func New(cfg app.Config, log logging.Logger) (app.Runnable, error) {
} }
return &Runner{ return &Runner{
checkRegistry: checkRegistry, checkRegistry: checkRegistry,
client: client, checksClient: client,
typesClient: typesClient, typesClient: typesClient,
evaluationInterval: evalInterval, defaultEvalInterval: evalInterval,
maxHistory: maxHistory, maxHistory: maxHistory,
namespace: namespace, namespace: namespace,
log: log.With("runner", "advisor.checkscheduler"), log: log.With("runner", "advisor.checkscheduler"),
}, nil }, nil
} }
@@ -91,47 +92,47 @@ func (r *Runner) Run(ctx context.Context) error {
lastCreated, err := r.checkLastCreated(ctxWithoutCancel, logger) lastCreated, err := r.checkLastCreated(ctxWithoutCancel, logger)
if err != nil { if err != nil {
logger.Error("Error getting last check creation time", "error", err) logger.Error("Error getting last check creation time", "error", err)
// Wait for interval to create the next scheduled check return err
lastCreated = time.Now() }
} else { // If there are checks already created, run an initial cleanup to remove old checks
// do an initial creation if necessary if !lastCreated.IsZero() {
if lastCreated.IsZero() { err = r.cleanupChecks(ctxWithoutCancel, logger)
err = r.createChecks(ctxWithoutCancel, logger) if err != nil {
if err != nil { logger.Error("Error cleaning up old check reports", "error", err)
logger.Error("Error creating new check reports", "error", err) return err
} else {
lastCreated = time.Now()
}
} else {
// Run an initial cleanup to remove old checks
err = r.cleanupChecks(ctxWithoutCancel, logger)
if err != nil {
logger.Error("Error cleaning up old check reports", "error", err)
}
} }
} }
nextSendInterval := getNextSendInterval(lastCreated, r.evaluationInterval) nextEvalTime := r.getNextEvalTime(r.defaultEvalInterval, lastCreated)
ticker := time.NewTicker(nextSendInterval) ticker := time.NewTicker(nextEvalTime)
defer ticker.Stop() defer ticker.Stop()
for { for {
select { select {
case <-ticker.C: case <-ticker.C:
err = r.createChecks(ctxWithoutCancel, logger) lastCreated, err := r.checkLastCreated(ctxWithoutCancel, logger)
if err != nil { if err != nil {
logger.Error("Error creating new check reports", "error", err) logger.Error("Error getting last check creation time", "error", err)
return err
} }
err = r.cleanupChecks(ctxWithoutCancel, logger) // If there are checks already created, then we can automatically create more
if err != nil { if !lastCreated.IsZero() {
logger.Error("Error cleaning up old check reports", "error", err) err = r.createChecks(ctxWithoutCancel, logger)
if err != nil {
logger.Error("Error creating new check reports", "error", err)
}
// Clean up old checks to avoid going over the limit
err = r.cleanupChecks(ctxWithoutCancel, logger)
if err != nil {
logger.Error("Error cleaning up old check reports", "error", err)
}
} }
if nextSendInterval != r.evaluationInterval { // Reset the ticker to the next send interval
nextSendInterval = r.evaluationInterval nextEvalTime = r.getNextEvalTime(r.defaultEvalInterval, lastCreated)
} ticker.Reset(nextEvalTime)
ticker.Reset(nextSendInterval)
case <-ctx.Done(): case <-ctx.Done():
return ctx.Err() return ctx.Err()
} }
@@ -139,7 +140,7 @@ func (r *Runner) Run(ctx context.Context) error {
} }
func (r *Runner) listChecks(ctx context.Context, logger logging.Logger) ([]resource.Object, error) { func (r *Runner) listChecks(ctx context.Context, logger logging.Logger) ([]resource.Object, error) {
list, err := r.client.List(ctx, r.namespace, resource.ListOptions{ list, err := r.checksClient.List(ctx, r.namespace, resource.ListOptions{
Limit: 1000, // Avoid pagination for normal uses cases, which is a costly operation Limit: 1000, // Avoid pagination for normal uses cases, which is a costly operation
}) })
if err != nil { if err != nil {
@@ -149,7 +150,7 @@ func (r *Runner) listChecks(ctx context.Context, logger logging.Logger) ([]resou
checks := list.GetItems() checks := list.GetItems()
for list.GetContinue() != "" { for list.GetContinue() != "" {
logger.Debug("List has continue token, listing next page", "continue", list.GetContinue()) logger.Debug("List has continue token, listing next page", "continue", list.GetContinue())
list, err = r.client.List(ctx, r.namespace, resource.ListOptions{Continue: list.GetContinue(), Limit: 1000}) list, err = r.checksClient.List(ctx, r.namespace, resource.ListOptions{Continue: list.GetContinue(), Limit: 1000})
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -177,7 +178,7 @@ func (r *Runner) checkLastCreated(ctx context.Context, log logging.Logger) (time
// If the check is unprocessed, set it to error // If the check is unprocessed, set it to error
if checks.GetStatusAnnotation(item) == "" { if checks.GetStatusAnnotation(item) == "" {
log.Info("Check is unprocessed, marking as error", "check", item.GetStaticMetadata().Identifier()) log.Info("Check is unprocessed, marking as error", "check", item.GetStaticMetadata().Identifier())
err := checks.SetStatusAnnotation(ctx, r.client, item, checks.StatusAnnotationError) err := checks.SetStatusAnnotation(ctx, r.checksClient, item, checks.StatusAnnotationError)
if err != nil { if err != nil {
log.Error("Error setting check status to error", "error", err) log.Error("Error setting check status to error", "error", err)
} }
@@ -225,7 +226,7 @@ func (r *Runner) createChecks(ctx context.Context, logger logging.Logger) error
Spec: advisorv0alpha1.CheckSpec{}, Spec: advisorv0alpha1.CheckSpec{},
} }
id := obj.GetStaticMetadata().Identifier() id := obj.GetStaticMetadata().Identifier()
_, err := r.client.Create(ctx, id, obj, resource.CreateOptions{}) _, err := r.checksClient.Create(ctx, id, obj, resource.CreateOptions{})
if err != nil { if err != nil {
return fmt.Errorf("error creating check: %w", err) return fmt.Errorf("error creating check: %w", err)
} }
@@ -268,7 +269,7 @@ func (r *Runner) cleanupChecks(ctx context.Context, logger logging.Logger) error
for i := 0; i < len(checks)-r.maxHistory; i++ { for i := 0; i < len(checks)-r.maxHistory; i++ {
check := checks[i] check := checks[i]
id := check.GetStaticMetadata().Identifier() id := check.GetStaticMetadata().Identifier()
err := r.client.Delete(ctx, id, resource.DeleteOptions{}) err := r.checksClient.Delete(ctx, id, resource.DeleteOptions{})
if err != nil { if err != nil {
return fmt.Errorf("error deleting check: %w", err) return fmt.Errorf("error deleting check: %w", err)
} }
@@ -293,15 +294,25 @@ func getEvaluationInterval(pluginConfig map[string]string) (time.Duration, error
return evaluationInterval, nil return evaluationInterval, nil
} }
func getNextSendInterval(lastCreated time.Time, evaluationInterval time.Duration) time.Duration { func (r *Runner) getNextEvalTime(defaultEvaluationInterval time.Duration, lastCreated time.Time) time.Duration {
nextSendInterval := time.Until(lastCreated.Add(evaluationInterval)) nextEvalTime := defaultEvaluationInterval
// Add random variation of one hour
randomVariation := time.Duration(rand.Int63n(time.Hour.Nanoseconds())) baseTime := lastCreated
nextSendInterval += randomVariation if lastCreated.IsZero() {
if nextSendInterval < time.Minute { baseTime = time.Now()
nextSendInterval = 1 * time.Minute
} }
return nextSendInterval
// Calculate the next evaluation time and add random variation
nextEvalTime = time.Until(baseTime.Add(nextEvalTime))
randomVariation := time.Duration(rand.Int63n(evalIntervalRandomVariation.Nanoseconds()))
nextEvalTime += randomVariation
// Ensure we always return a positive duration to avoid ticker panics
if nextEvalTime <= 0 {
nextEvalTime = 1 * time.Millisecond
}
return nextEvalTime
} }
func getMaxHistory(pluginConfig map[string]string) (int, error) { func getMaxHistory(pluginConfig map[string]string) (int, error) {
@@ -4,26 +4,49 @@ import (
"context" "context"
"errors" "errors"
"fmt" "fmt"
"math/rand/v2"
"testing" "testing"
"time" "time"
"github.com/grafana/grafana-app-sdk/logging" "github.com/grafana/grafana-app-sdk/logging"
"github.com/grafana/grafana-app-sdk/resource" "github.com/grafana/grafana-app-sdk/resource"
advisorv0alpha1 "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1" 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" "github.com/grafana/grafana/apps/advisor/pkg/app/checks"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
) )
func init() {
waitInterval = 1 * time.Millisecond
evalIntervalRandomVariation = 1 * time.Millisecond
}
// TestRunner_Run tests the main Run function with various scenarios
func TestRunner_Run(t *testing.T) { func TestRunner_Run(t *testing.T) {
t.Run("does not crash when error on list", func(t *testing.T) { t.Run("handles context cancellation gracefully", func(t *testing.T) {
runner := createTestRunner(&MockClient{}, &MockClient{})
ctx, cancel := context.WithCancel(context.Background())
cancel() // Cancel immediately
err := runner.Run(ctx)
assert.ErrorAs(t, err, &context.Canceled)
})
t.Run("handles timeout gracefully", func(t *testing.T) {
runner := createTestRunner(&MockClient{}, &MockClient{})
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond)
defer cancel()
err := runner.Run(ctx)
assert.ErrorAs(t, err, &context.DeadlineExceeded)
})
t.Run("handles check list error gracefully", func(t *testing.T) {
mockClient := &MockClient{ mockClient := &MockClient{
listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
return nil, errors.New("list error") return nil, errors.New("list checks error")
},
createFunc: func(ctx context.Context, id resource.Identifier, obj resource.Object, opts resource.CreateOptions) (resource.Object, error) {
return &advisorv0alpha1.Check{}, nil
}, },
} }
@@ -33,352 +56,287 @@ func TestRunner_Run(t *testing.T) {
}, },
} }
runner := &Runner{ runner := createTestRunner(mockClient, mockTypesClient)
client: mockClient, err := runner.Run(context.Background())
typesClient: mockTypesClient, assert.ErrorContains(t, err, "list checks error")
log: &logging.NoOpLogger{},
evaluationInterval: 1 * time.Hour,
}
ctx, cancel := context.WithCancel(context.Background())
cancel()
err := runner.Run(ctx)
assert.ErrorAs(t, err, &context.Canceled)
}) })
} }
func TestRunner_checkLastCreated_ErrorOnList(t *testing.T) { // TestRunner_Run_CheckCreation tests check creation scenarios
mockClient := &MockClient{ func TestRunner_Run_CheckCreation(t *testing.T) {
listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { t.Run("does not create checks on first run when no previous checks exist", func(t *testing.T) {
return nil, errors.New("list error") checksCreated := []string{}
},
}
runner := &Runner{ mockClient := &MockClient{
client: mockClient, listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
log: &logging.NoOpLogger{}, // Return empty list - no previous checks
} return &advisorv0alpha1.CheckList{Items: []advisorv0alpha1.Check{}}, nil
},
createFunc: func(ctx context.Context, id resource.Identifier, obj resource.Object, opts resource.CreateOptions) (resource.Object, error) {
checksCreated = append(checksCreated, id.Name)
return obj, nil
},
}
lastCreated, err := runner.checkLastCreated(context.Background(), &logging.NoOpLogger{}) mockTypesClient := &MockClient{
assert.Error(t, err) listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
assert.True(t, lastCreated.IsZero()) return &advisorv0alpha1.CheckTypeList{
} Items: []advisorv0alpha1.CheckType{
{
func TestRunner_checkLastCreated_UnprocessedCheck(t *testing.T) { ObjectMeta: metav1.ObjectMeta{
patchOperation := resource.PatchOperation{} Name: "test-check",
identifier := resource.Identifier{} },
Spec: advisorv0alpha1.CheckTypeSpec{
mockClient := &MockClient{ Name: "test-check",
listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { },
return &advisorv0alpha1.CheckList{
Items: []advisorv0alpha1.Check{
{
ObjectMeta: metav1.ObjectMeta{
Name: "check-1",
}, },
}, },
}, }, nil
}, nil },
}, }
patchFunc: func(ctx context.Context, id resource.Identifier, patch resource.PatchRequest, options resource.PatchOptions, into resource.Object) error {
patchOperation = patch.Operations[0]
identifier = id
return nil
},
}
runner := &Runner{ // Create a mock check service with one check to match the check type
client: mockClient, mockCheckService := &MockCheckService{checks: []checks.Check{&mockCheck{id: "test-check"}}}
log: &logging.NoOpLogger{}, runner := createTestRunnerWithRegistry(mockClient, mockTypesClient, mockCheckService)
}
lastCreated, err := runner.checkLastCreated(context.Background(), &logging.NoOpLogger{}) err := runAndTimeout(runner)
assert.NoError(t, err) assert.ErrorAs(t, err, &context.DeadlineExceeded)
assert.True(t, lastCreated.IsZero()) // Should not create checks on first run when no previous checks exist
assert.Equal(t, "check-1", identifier.Name) assert.Empty(t, checksCreated, "Should not create checks on first run when no previous checks exist")
assert.Equal(t, "/metadata/annotations", patchOperation.Path) })
expectedAnnotations := map[string]string{
checks.StatusAnnotation: "error",
}
assert.Equal(t, expectedAnnotations, patchOperation.Value)
}
func TestRunner_checkLastCreated_PaginatedResponse(t *testing.T) { t.Run("creates checks when evaluation interval has passed", func(t *testing.T) {
// Create checks with different creation times checksCreated := []string{}
past := time.Now().Add(-1 * time.Hour)
now := time.Now()
mockClient := &MockClient{ mockClient := &MockClient{
listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
if options.Continue == "" { // Return a check that was created long ago (past the evaluation interval)
// First page - return oldest and middle checks with continue token
return &advisorv0alpha1.CheckList{ return &advisorv0alpha1.CheckList{
ListMeta: metav1.ListMeta{
Continue: "continue-token-123",
},
Items: []advisorv0alpha1.Check{ Items: []advisorv0alpha1.Check{
{ {
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: "check-1", Name: "old-check",
CreationTimestamp: metav1.NewTime(past), CreationTimestamp: metav1.NewTime(time.Now().Add(-15 * 24 * time.Hour)), // 15 days ago
Annotations: map[string]string{ Annotations: map[string]string{
checks.StatusAnnotation: "completed", checks.StatusAnnotation: checks.StatusAnnotationProcessed,
},
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "check-2",
CreationTimestamp: metav1.NewTime(past),
Annotations: map[string]string{
checks.StatusAnnotation: "completed",
}, },
}, },
}, },
}, },
}, nil }, nil
} },
// Second page - verify continue token is passed and return newest check createFunc: func(ctx context.Context, id resource.Identifier, obj resource.Object, opts resource.CreateOptions) (resource.Object, error) {
assert.Equal(t, "continue-token-123", options.Continue) checksCreated = append(checksCreated, id.Name)
return &advisorv0alpha1.CheckList{ return obj, nil
Items: []advisorv0alpha1.Check{ },
{ }
ObjectMeta: metav1.ObjectMeta{
Name: "check-3", mockTypesClient := &MockClient{
CreationTimestamp: metav1.NewTime(now), listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
Annotations: map[string]string{ return &advisorv0alpha1.CheckTypeList{
checks.StatusAnnotation: "completed", Items: []advisorv0alpha1.CheckType{
{
ObjectMeta: metav1.ObjectMeta{
Name: "test-check",
},
Spec: advisorv0alpha1.CheckTypeSpec{
Name: "test-check",
}, },
}, },
}, },
}, }, nil
}, nil },
}, }
}
runner := &Runner{ // Create a mock check service with one check to match the check type
client: mockClient, mockCheckService := &MockCheckService{checks: []checks.Check{&mockCheck{id: "test-check"}}}
log: &logging.NoOpLogger{}, runner := createTestRunnerWithRegistry(mockClient, mockTypesClient, mockCheckService)
}
lastCreated, err := runner.checkLastCreated(context.Background(), &logging.NoOpLogger{}) err := runAndTimeout(runner)
assert.NoError(t, err) assert.ErrorAs(t, err, &context.DeadlineExceeded)
assert.Equal(t, now.Truncate(time.Second), lastCreated.Truncate(time.Second)) // Should create checks when the evaluation interval has passed
assert.Greater(t, len(checksCreated), 0, "Should create checks when evaluation interval has passed")
})
} }
func TestRunner_createChecks_ErrorOnCreate(t *testing.T) { // TestRunner_Run_CheckCleanup tests check cleanup scenarios
mockCheckService := &MockCheckService{checks: []checks.Check{&mockCheck{id: "check-1"}}} func TestRunner_Run_CheckCleanup(t *testing.T) {
t.Run("cleans up old checks when limit exceeded", func(t *testing.T) {
checksDeleted := []string{}
mockClient := &MockClient{ // Create checks that exceed the max history limit
createFunc: func(ctx context.Context, id resource.Identifier, obj resource.Object, opts resource.CreateOptions) (resource.Object, error) { items := make([]advisorv0alpha1.Check, 0, defaultMaxHistory+2)
return nil, errors.New("create error") for i := 0; i < defaultMaxHistory+2; i++ {
}, item := advisorv0alpha1.Check{}
} item.SetName(fmt.Sprintf("check-%d", i))
item.SetLabels(map[string]string{
checks.TypeLabel: "test-type",
})
item.SetCreationTimestamp(metav1.NewTime(time.Now().Add(-time.Duration(i) * time.Hour)))
items = append(items, item)
}
mockTypesClient := &MockClient{ mockClient := &MockClient{
listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
checkType := &advisorv0alpha1.CheckType{} return &advisorv0alpha1.CheckList{Items: items}, nil
checkType.Spec.Name = "check-1" },
return &advisorv0alpha1.CheckTypeList{ deleteFunc: func(ctx context.Context, id resource.Identifier, opts resource.DeleteOptions) error {
Items: []advisorv0alpha1.CheckType{*checkType}, checksDeleted = append(checksDeleted, id.Name)
}, nil return nil
}, },
} }
runner := &Runner{ mockTypesClient := &MockClient{
checkRegistry: mockCheckService, listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
client: mockClient, return &advisorv0alpha1.CheckTypeList{Items: []advisorv0alpha1.CheckType{}}, nil
typesClient: mockTypesClient, },
log: &logging.NoOpLogger{}, }
}
err := runner.createChecks(context.Background(), &logging.NoOpLogger{}) runner := createTestRunner(mockClient, mockTypesClient)
assert.Error(t, err)
err := runAndTimeout(runner)
assert.ErrorAs(t, err, &context.DeadlineExceeded)
// Should delete some checks due to cleanup
assert.Greater(t, len(checksDeleted), 0)
})
} }
func TestRunner_createChecks_Success(t *testing.T) { // TestRunner_Run_UnprocessedChecks tests handling of unprocessed checks
mockCheckService := &MockCheckService{checks: []checks.Check{&mockCheck{id: "check-1"}}} func TestRunner_Run_UnprocessedChecks(t *testing.T) {
t.Run("marks unprocessed checks as error", func(t *testing.T) {
patchOperations := []resource.PatchOperation{}
mockClient := &MockClient{ mockClient := &MockClient{
createFunc: func(ctx context.Context, id resource.Identifier, obj resource.Object, opts resource.CreateOptions) (resource.Object, error) { listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
return &advisorv0alpha1.Check{}, nil return &advisorv0alpha1.CheckList{
}, Items: []advisorv0alpha1.Check{
} {
ObjectMeta: metav1.ObjectMeta{
Name: "unprocessed-check",
// No status annotation - unprocessed
},
},
},
}, nil
},
patchFunc: func(ctx context.Context, id resource.Identifier, patch resource.PatchRequest, options resource.PatchOptions, into resource.Object) error {
patchOperations = append(patchOperations, patch.Operations...)
return nil
},
}
mockTypesClient := &MockClient{ mockTypesClient := &MockClient{
listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
checkType := &advisorv0alpha1.CheckType{} return &advisorv0alpha1.CheckTypeList{Items: []advisorv0alpha1.CheckType{}}, nil
checkType.Spec.Name = "check-1" },
return &advisorv0alpha1.CheckTypeList{ }
Items: []advisorv0alpha1.CheckType{*checkType},
}, nil
},
}
runner := &Runner{ runner := createTestRunner(mockClient, mockTypesClient)
checkRegistry: mockCheckService,
client: mockClient,
typesClient: mockTypesClient,
log: &logging.NoOpLogger{},
}
err := runner.createChecks(context.Background(), &logging.NoOpLogger{}) err := runAndTimeout(runner)
assert.NoError(t, err) assert.ErrorAs(t, err, &context.DeadlineExceeded)
// Should patch unprocessed check with error status
assert.Greater(t, len(patchOperations), 0)
})
} }
func TestRunner_cleanupChecks_ErrorOnList(t *testing.T) { // TestRunner_Run_Pagination tests pagination handling
mockClient := &MockClient{ func TestRunner_Run_Pagination(t *testing.T) {
listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { t.Run("handles paginated check lists", func(t *testing.T) {
return nil, errors.New("list error") callCount := 0
},
}
runner := &Runner{ mockClient := &MockClient{
client: mockClient, listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
log: &logging.NoOpLogger{}, callCount++
} if callCount == 1 {
return &advisorv0alpha1.CheckList{
ListMeta: metav1.ListMeta{Continue: "continue-token"},
Items: []advisorv0alpha1.Check{
{ObjectMeta: metav1.ObjectMeta{Name: "check-1"}},
},
}, nil
}
return &advisorv0alpha1.CheckList{
Items: []advisorv0alpha1.Check{
{ObjectMeta: metav1.ObjectMeta{Name: "check-2"}},
},
}, nil
},
}
err := runner.cleanupChecks(context.Background(), &logging.NoOpLogger{}) mockTypesClient := &MockClient{
assert.Error(t, err) listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
return &advisorv0alpha1.CheckTypeList{Items: []advisorv0alpha1.CheckType{}}, nil
},
}
runner := createTestRunner(mockClient, mockTypesClient)
err := runAndTimeout(runner)
assert.ErrorAs(t, err, &context.DeadlineExceeded)
// Should handle pagination correctly
assert.GreaterOrEqual(t, callCount, 2)
})
} }
func TestRunner_cleanupChecks_WithinMax(t *testing.T) { // Helper functions
mockClient := &MockClient{
listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
return &advisorv0alpha1.CheckList{
Items: []advisorv0alpha1.Check{{}, {}},
}, nil
},
deleteFunc: func(ctx context.Context, identifier resource.Identifier, options resource.DeleteOptions) error {
return fmt.Errorf("shouldn't be called")
},
}
runner := &Runner{ // runAndTimeout runs a runner with a short timeout for testing purposes.
client: mockClient, // This is used to terminate the runner's infinite loop in tests that don't specifically test timeout behavior.
log: &logging.NoOpLogger{}, func runAndTimeout(runner *Runner) error {
} ctx, cancel := context.WithTimeout(context.Background(), 5*time.Millisecond)
defer cancel()
err := runner.cleanupChecks(context.Background(), &logging.NoOpLogger{}) return runner.Run(ctx)
assert.NoError(t, err)
} }
func TestRunner_cleanupChecks_ErrorOnDelete(t *testing.T) { // createTestRunner creates a test runner with mock clients
mockClient := &MockClient{ func createTestRunner(checkClient, typesClient *MockClient) *Runner {
listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { return createTestRunnerWithRegistry(checkClient, typesClient, &MockCheckService{checks: []checks.Check{}})
items := make([]advisorv0alpha1.Check, 0, defaultMaxHistory+1)
for i := 0; i < defaultMaxHistory+1; i++ {
item := advisorv0alpha1.Check{}
item.SetLabels(map[string]string{
checks.TypeLabel: "mock",
})
items = append(items, item)
}
return &advisorv0alpha1.CheckList{
Items: items,
}, nil
},
deleteFunc: func(ctx context.Context, identifier resource.Identifier, options resource.DeleteOptions) error {
return errors.New("delete error")
},
}
runner := &Runner{
client: mockClient,
maxHistory: defaultMaxHistory,
log: &logging.NoOpLogger{},
}
err := runner.cleanupChecks(context.Background(), &logging.NoOpLogger{})
assert.ErrorContains(t, err, "delete error")
} }
func TestRunner_cleanupChecks_Success(t *testing.T) { // createTestRunnerWithRegistry creates a test runner with mock clients and custom registry
itemsDeleted := []string{} func createTestRunnerWithRegistry(checkClient, typesClient *MockClient, checkRegistry checkregistry.CheckService) *Runner {
items := make([]advisorv0alpha1.Check, 0, defaultMaxHistory+1) // Ensure mock clients have default implementations
for i := 0; i < defaultMaxHistory+1; i++ { if checkClient.listFunc == nil {
item := advisorv0alpha1.Check{} checkClient.listFunc = func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
item.SetName(fmt.Sprintf("check-%d", i)) return &advisorv0alpha1.CheckList{Items: []advisorv0alpha1.Check{}}, nil
item.SetLabels(map[string]string{ }
checks.TypeLabel: "mock",
})
item.SetCreationTimestamp(metav1.NewTime(time.Time{}.Add(time.Duration(i) * time.Hour)))
items = append(items, item)
} }
// shuffle the items to ensure the oldest are deleted if checkClient.createFunc == nil {
rand.Shuffle(len(items), func(i, j int) { items[i], items[j] = items[j], items[i] }) checkClient.createFunc = func(ctx context.Context, id resource.Identifier, obj resource.Object, opts resource.CreateOptions) (resource.Object, error) {
return obj, nil
mockClient := &MockClient{ }
listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { }
return &advisorv0alpha1.CheckList{ if checkClient.deleteFunc == nil {
Items: items, checkClient.deleteFunc = func(ctx context.Context, id resource.Identifier, opts resource.DeleteOptions) error {
}, nil
},
deleteFunc: func(ctx context.Context, identifier resource.Identifier, options resource.DeleteOptions) error {
itemsDeleted = append(itemsDeleted, identifier.Name)
return nil return nil
}, }
}
if checkClient.patchFunc == nil {
checkClient.patchFunc = func(ctx context.Context, id resource.Identifier, patch resource.PatchRequest, opts resource.PatchOptions, into resource.Object) error {
return nil
}
} }
runner := &Runner{ if typesClient.listFunc == nil {
client: mockClient, typesClient.listFunc = func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) {
maxHistory: defaultMaxHistory, // Return empty list to match the empty MockCheckService
log: &logging.NoOpLogger{}, return &advisorv0alpha1.CheckTypeList{Items: []advisorv0alpha1.CheckType{}}, nil
}
}
return &Runner{
checkRegistry: checkRegistry,
checksClient: checkClient,
typesClient: typesClient,
defaultEvalInterval: 5 * time.Millisecond,
maxHistory: defaultMaxHistory,
namespace: "test-namespace",
log: &logging.NoOpLogger{},
} }
err := runner.cleanupChecks(context.Background(), &logging.NoOpLogger{})
assert.NoError(t, err)
assert.Equal(t, []string{"check-0"}, itemsDeleted)
} }
func Test_getEvaluationInterval(t *testing.T) { // Mock implementations
t.Run("default", func(t *testing.T) {
interval, err := getEvaluationInterval(map[string]string{})
assert.NoError(t, err)
assert.Equal(t, 7*24*time.Hour, interval)
})
t.Run("invalid", func(t *testing.T) {
interval, err := getEvaluationInterval(map[string]string{"evaluation_interval": "invalid"})
assert.Error(t, err)
assert.Zero(t, interval)
})
t.Run("custom", func(t *testing.T) {
interval, err := getEvaluationInterval(map[string]string{"evaluation_interval": "1h"})
assert.NoError(t, err)
assert.Equal(t, time.Hour, interval)
})
}
func Test_getMaxHistory(t *testing.T) {
t.Run("default", func(t *testing.T) {
history, err := getMaxHistory(map[string]string{})
assert.NoError(t, err)
assert.Equal(t, 10, history)
})
t.Run("invalid", func(t *testing.T) {
history, err := getMaxHistory(map[string]string{"max_history": "invalid"})
assert.Error(t, err)
assert.Zero(t, history)
})
t.Run("custom", func(t *testing.T) {
history, err := getMaxHistory(map[string]string{"max_history": "5"})
assert.NoError(t, err)
assert.Equal(t, 5, history)
})
}
func Test_getNextSendInterval(t *testing.T) {
lastCreated := time.Now().Add(-7 * 24 * time.Hour)
evaluationInterval := 7 * 24 * time.Hour
nextSendInterval := getNextSendInterval(lastCreated, evaluationInterval)
// The next send interval should be in < 1 hour
assert.True(t, nextSendInterval < time.Hour)
// Calculate the next send interval again and it should be different
nextSendInterval2 := getNextSendInterval(lastCreated, evaluationInterval)
assert.NotEqual(t, nextSendInterval, nextSendInterval2)
}
type MockClient struct { type MockClient struct {
resource.Client resource.Client
@@ -414,7 +372,6 @@ func (m *MockCheckService) Checks() []checks.Check {
type mockCheck struct { type mockCheck struct {
checks.Check checks.Check
id string id string
steps []checks.Step steps []checks.Step
} }