diff --git a/apps/advisor/pkg/app/checkscheduler/checkscheduler.go b/apps/advisor/pkg/app/checkscheduler/checkscheduler.go index 392ec2d792d..f344326a59f 100644 --- a/apps/advisor/pkg/app/checkscheduler/checkscheduler.go +++ b/apps/advisor/pkg/app/checkscheduler/checkscheduler.go @@ -3,6 +3,7 @@ package checkscheduler import ( "context" "fmt" + "sort" "time" "github.com/grafana/grafana-app-sdk/app" @@ -16,6 +17,7 @@ import ( ) const evaluateChecksInterval = 24 * time.Hour +const maxChecks = 10 // 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 @@ -78,6 +80,11 @@ func (r *Runner) Run(ctx context.Context) error { klog.Error("Error creating new check reports", "error", err) } + err = r.cleanupChecks(ctx) + if err != nil { + klog.Error("Error cleaning up old check reports", "error", err) + } + if nextSendInterval != evaluateChecksInterval { nextSendInterval = evaluateChecksInterval } @@ -127,3 +134,45 @@ func (r *Runner) createChecks(ctx context.Context) error { } return nil } + +// 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}) + if err != nil { + return err + } + + // organize checks by type + checksByType := map[string][]resource.Object{} + for _, check := range list.GetItems() { + labels := check.GetLabels() + checkType, ok := labels[checks.TypeLabel] + if !ok { + klog.Error("Check type not found in labels", "check", check) + continue + } + checksByType[checkType] = append(checksByType[checkType], check) + } + + for _, checks := range checksByType { + if len(checks) > maxChecks { + // Sort checks by creation time + sort.Slice(checks, func(i, j int) bool { + ti := checks[i].GetCreationTimestamp().Time + tj := checks[j].GetCreationTimestamp().Time + return ti.Before(tj) + }) + // Delete the oldest checks + for i := 0; i < len(checks)-maxChecks; i++ { + check := checks[i] + id := check.GetStaticMetadata().Identifier() + err := r.client.Delete(ctx, id, resource.DeleteOptions{}) + if err != nil { + return fmt.Errorf("error deleting check: %w", err) + } + } + } + } + + return nil +} diff --git a/apps/advisor/pkg/app/checkscheduler/checkscheduler_test.go b/apps/advisor/pkg/app/checkscheduler/checkscheduler_test.go index 8b1c7fb47e5..b969e96745c 100644 --- a/apps/advisor/pkg/app/checkscheduler/checkscheduler_test.go +++ b/apps/advisor/pkg/app/checkscheduler/checkscheduler_test.go @@ -3,51 +3,18 @@ package checkscheduler import ( "context" "errors" + "fmt" + "math/rand/v2" "testing" + "time" "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/checks" "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -type MockCheckService struct { - checks []checks.Check -} - -func (m *MockCheckService) Checks() []checks.Check { - return m.checks -} - -type MockClient struct { - resource.Client - listFunc func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) - createFunc func(ctx context.Context, identifier resource.Identifier, obj resource.Object, options resource.CreateOptions) (resource.Object, error) -} - -func (m *MockClient) List(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { - return m.listFunc(ctx, namespace, options) -} - -func (m *MockClient) Create(ctx context.Context, identifier resource.Identifier, obj resource.Object, options resource.CreateOptions) (resource.Object, error) { - return m.createFunc(ctx, identifier, obj, options) -} - -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 -} - func TestRunner_Run_ErrorOnList(t *testing.T) { mockCheckService := &MockCheckService{} mockClient := &MockClient{ @@ -129,3 +96,142 @@ func TestRunner_createChecks_Success(t *testing.T) { err := runner.createChecks(context.Background()) assert.NoError(t, err) } + +func TestRunner_cleanupChecks_ErrorOnList(t *testing.T) { + mockClient := &MockClient{ + listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { + return nil, errors.New("list error") + }, + } + + runner := &Runner{ + client: mockClient, + } + + err := runner.cleanupChecks(context.Background()) + assert.Error(t, err) +} + +func TestRunner_cleanupChecks_WithinMax(t *testing.T) { + 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{ + client: mockClient, + } + + err := runner.cleanupChecks(context.Background()) + assert.NoError(t, err) +} + +func TestRunner_cleanupChecks_ErrorOnDelete(t *testing.T) { + mockClient := &MockClient{ + listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { + items := make([]advisorv0alpha1.Check, 0, maxChecks+1) + for i := 0; i < maxChecks+1; i++ { + item := advisorv0alpha1.Check{} + item.ObjectMeta.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, + } + err := runner.cleanupChecks(context.Background()) + assert.ErrorContains(t, err, "delete error") +} + +func TestRunner_cleanupChecks_Success(t *testing.T) { + itemsDeleted := []string{} + items := make([]advisorv0alpha1.Check, 0, maxChecks+1) + for i := 0; i < maxChecks+1; i++ { + item := advisorv0alpha1.Check{} + item.ObjectMeta.SetName(fmt.Sprintf("check-%d", i)) + item.ObjectMeta.SetLabels(map[string]string{ + checks.TypeLabel: "mock", + }) + item.ObjectMeta.SetCreationTimestamp(metav1.NewTime(time.Time{}.Add(time.Duration(i) * time.Hour))) + items = append(items, item) + } + // shuffle the items to ensure the oldest are deleted + rand.Shuffle(len(items), func(i, j int) { items[i], items[j] = items[j], items[i] }) + + mockClient := &MockClient{ + listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { + return &advisorv0alpha1.CheckList{ + Items: items, + }, nil + }, + deleteFunc: func(ctx context.Context, identifier resource.Identifier, options resource.DeleteOptions) error { + itemsDeleted = append(itemsDeleted, identifier.Name) + return nil + }, + } + + runner := &Runner{ + client: mockClient, + } + err := runner.cleanupChecks(context.Background()) + assert.NoError(t, err) + assert.Equal(t, []string{"check-0"}, itemsDeleted) +} + +type MockCheckService struct { + checks []checks.Check +} + +func (m *MockCheckService) Checks() []checks.Check { + return m.checks +} + +type MockClient struct { + resource.Client + listFunc func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) + createFunc func(ctx context.Context, identifier resource.Identifier, obj resource.Object, options resource.CreateOptions) (resource.Object, error) + deleteFunc func(ctx context.Context, identifier resource.Identifier, options resource.DeleteOptions) error +} + +func (m *MockClient) List(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { + return m.listFunc(ctx, namespace, options) +} + +func (m *MockClient) Create(ctx context.Context, identifier resource.Identifier, obj resource.Object, options resource.CreateOptions) (resource.Object, error) { + return m.createFunc(ctx, identifier, obj, options) +} + +func (m *MockClient) Delete(ctx context.Context, identifier resource.Identifier, options resource.DeleteOptions) error { + return m.deleteFunc(ctx, identifier, options) +} + +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 +}