diff --git a/apps/advisor/pkg/app/app.go b/apps/advisor/pkg/app/app.go index 7982661c3b9..f645466c4e1 100644 --- a/apps/advisor/pkg/app/app.go +++ b/apps/advisor/pkg/app/app.go @@ -11,16 +11,12 @@ import ( 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/checkscheduler" "github.com/grafana/grafana/apps/advisor/pkg/app/checktyperegisterer" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/klog/v2" ) -const ( - typeLabel = "advisor.grafana.app/type" - statusAnnotation = "advisor.grafana.app/status" -) - func New(cfg app.Config) (app.App, error) { // Read config checkRegistry, ok := cfg.SpecificConfig.(checkregistry.CheckService) @@ -94,6 +90,13 @@ func New(cfg app.Config) (app.App, error) { } a.AddRunnable(ctr) + // Start scheduler + csch, err := checkscheduler.New(cfg) + if err != nil { + return nil, err + } + a.AddRunnable(csch) + return a, nil } diff --git a/apps/advisor/pkg/app/checks/utils.go b/apps/advisor/pkg/app/checks/utils.go index ee33d57076b..a02357cf013 100644 --- a/apps/advisor/pkg/app/checks/utils.go +++ b/apps/advisor/pkg/app/checks/utils.go @@ -4,6 +4,11 @@ import ( advisor "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1" ) +const ( + TypeLabel = "advisor.grafana.app/type" + StatusAnnotation = "advisor.grafana.app/status" +) + func NewCheckReportFailure( severity advisor.CheckReportFailureSeverity, reason string, diff --git a/apps/advisor/pkg/app/checkscheduler/checkscheduler.go b/apps/advisor/pkg/app/checkscheduler/checkscheduler.go new file mode 100644 index 00000000000..392ec2d792d --- /dev/null +++ b/apps/advisor/pkg/app/checkscheduler/checkscheduler.go @@ -0,0 +1,129 @@ +package checkscheduler + +import ( + "context" + "fmt" + "time" + + "github.com/grafana/grafana-app-sdk/app" + "github.com/grafana/grafana-app-sdk/k8s" + "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" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/klog/v2" +) + +const evaluateChecksInterval = 24 * time.Hour + +// 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 +} + +// NewRunner creates a new Runner. +func New(cfg app.Config) (app.Runnable, error) { + // Read config + checkRegistry, ok := cfg.SpecificConfig.(checkregistry.CheckService) + if !ok { + return nil, fmt.Errorf("invalid config type") + } + + // Prepare storage client + clientGenerator := k8s.NewClientRegistry(cfg.KubeConfig, k8s.ClientConfig{}) + client, err := clientGenerator.ClientFor(advisorv0alpha1.CheckKind()) + if err != nil { + return nil, err + } + + return &Runner{ + checkRegistry: checkRegistry, + client: client, + }, nil +} + +func (r *Runner) Run(ctx context.Context) error { + lastCreated, err := r.checkLastCreated(ctx) + if err != nil { + return err + } + + // do an initial creation if necessary + if lastCreated.IsZero() { + err = r.createChecks(ctx) + if err != nil { + klog.Error("Error creating new check reports", "error", err) + } else { + lastCreated = time.Now() + } + } + + nextSendInterval := time.Until(lastCreated.Add(evaluateChecksInterval)) + if nextSendInterval < time.Minute { + nextSendInterval = 1 * time.Minute + } + + ticker := time.NewTicker(nextSendInterval) + defer ticker.Stop() + + for { + select { + case <-ticker.C: + err = r.createChecks(ctx) + if err != nil { + klog.Error("Error creating new check reports", "error", err) + } + + if nextSendInterval != evaluateChecksInterval { + nextSendInterval = evaluateChecksInterval + } + ticker.Reset(nextSendInterval) + case <-ctx.Done(): + return ctx.Err() + } + } +} + +// checkLastCreated returns the creation time of the last check created +// 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{}) + if err != nil { + return time.Time{}, err + } + lastCreated := time.Time{} + for _, item := range list.GetItems() { + itemCreated := item.GetCreationTimestamp().Time + if itemCreated.After(lastCreated) { + lastCreated = itemCreated + } + } + return lastCreated, nil +} + +// createChecks creates a new check for each check type in the registry. +func (r *Runner) createChecks(ctx context.Context) error { + for _, check := range r.checkRegistry.Checks() { + obj := &advisorv0alpha1.Check{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "check-", + Namespace: metav1.NamespaceDefault, + Labels: map[string]string{ + checks.TypeLabel: check.ID(), + }, + }, + Spec: advisorv0alpha1.CheckSpec{}, + } + id := obj.GetStaticMetadata().Identifier() + _, err := r.client.Create(ctx, id, obj, resource.CreateOptions{}) + if err != nil { + return fmt.Errorf("error creating 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 new file mode 100644 index 00000000000..8b1c7fb47e5 --- /dev/null +++ b/apps/advisor/pkg/app/checkscheduler/checkscheduler_test.go @@ -0,0 +1,131 @@ +package checkscheduler + +import ( + "context" + "errors" + "testing" + + "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" +) + +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{ + listFunc: func(ctx context.Context, namespace string, options resource.ListOptions) (resource.ListObject, error) { + return nil, errors.New("list error") + }, + createFunc: func(ctx context.Context, id resource.Identifier, obj resource.Object, opts resource.CreateOptions) (resource.Object, error) { + return &advisorv0alpha1.Check{}, nil + }, + } + + runner := &Runner{ + checkRegistry: mockCheckService, + client: mockClient, + } + + err := runner.Run(context.Background()) + assert.Error(t, err) +} + +func TestRunner_checkLastCreated_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, + } + + lastCreated, err := runner.checkLastCreated(context.Background()) + assert.Error(t, err) + assert.True(t, lastCreated.IsZero()) +} + +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") + }, + } + + runner := &Runner{ + checkRegistry: mockCheckService, + client: mockClient, + } + + err := runner.createChecks(context.Background()) + 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 + }, + } + + runner := &Runner{ + checkRegistry: mockCheckService, + client: mockClient, + } + + err := runner.createChecks(context.Background()) + assert.NoError(t, err) +} diff --git a/apps/advisor/pkg/app/utils.go b/apps/advisor/pkg/app/utils.go index 731d16e8737..8c1b6c00eb8 100644 --- a/apps/advisor/pkg/app/utils.go +++ b/apps/advisor/pkg/app/utils.go @@ -15,16 +15,16 @@ import ( "github.com/grafana/grafana/pkg/services/user" ) -func getCheck(obj resource.Object, checks map[string]checks.Check) (checks.Check, error) { +func getCheck(obj resource.Object, checkMap map[string]checks.Check) (checks.Check, error) { labels := obj.GetLabels() - objTypeLabel, ok := labels[typeLabel] + objTypeLabel, ok := labels[checks.TypeLabel] if !ok { return nil, errors.New("missing check type as label") } - c, ok := checks[objTypeLabel] + c, ok := checkMap[objTypeLabel] if !ok { supportedTypes := "" - for k := range checks { + for k := range checkMap { supportedTypes += k + ", " } return nil, fmt.Errorf("unknown check type %s. Supported types are: %s", objTypeLabel, supportedTypes) @@ -34,12 +34,12 @@ func getCheck(obj resource.Object, checks map[string]checks.Check) (checks.Check } func getStatusAnnotation(obj resource.Object) string { - return obj.GetAnnotations()[statusAnnotation] + return obj.GetAnnotations()[checks.StatusAnnotation] } func setStatusAnnotation(ctx context.Context, client resource.Client, obj resource.Object, status string) error { annotations := obj.GetAnnotations() - annotations[statusAnnotation] = status + annotations[checks.StatusAnnotation] = status return client.PatchInto(ctx, obj.GetStaticMetadata().Identifier(), resource.PatchRequest{ Operations: []resource.PatchOperation{{ Operation: resource.PatchOpAdd, diff --git a/apps/advisor/pkg/app/utils_test.go b/apps/advisor/pkg/app/utils_test.go index ada844fdba4..090397333d0 100644 --- a/apps/advisor/pkg/app/utils_test.go +++ b/apps/advisor/pkg/app/utils_test.go @@ -15,7 +15,7 @@ import ( func TestGetCheck(t *testing.T) { obj := &advisorv0alpha1.Check{} - obj.SetLabels(map[string]string{typeLabel: "testType"}) + obj.SetLabels(map[string]string{checks.TypeLabel: "testType"}) checkMap := map[string]checks.Check{ "testType": &mockCheck{}, @@ -37,7 +37,7 @@ func TestGetCheck_MissingLabel(t *testing.T) { func TestGetCheck_UnknownType(t *testing.T) { obj := &advisorv0alpha1.Check{} - obj.SetLabels(map[string]string{typeLabel: "unknownType"}) + obj.SetLabels(map[string]string{checks.TypeLabel: "unknownType"}) checkMap := map[string]checks.Check{ "testType": &mockCheck{}, @@ -56,7 +56,7 @@ func TestSetStatusAnnotation(t *testing.T) { err := setStatusAnnotation(ctx, client, obj, "processed") assert.NoError(t, err) - assert.Equal(t, "processed", obj.GetAnnotations()[statusAnnotation]) + assert.Equal(t, "processed", obj.GetAnnotations()[checks.StatusAnnotation]) } func TestProcessCheck(t *testing.T) { @@ -75,7 +75,7 @@ func TestProcessCheck(t *testing.T) { err = processCheck(ctx, client, obj, check) assert.NoError(t, err) - assert.Equal(t, "processed", obj.GetAnnotations()[statusAnnotation]) + assert.Equal(t, "processed", obj.GetAnnotations()[checks.StatusAnnotation]) } func TestProcessMultipleCheckItems(t *testing.T) { @@ -102,7 +102,7 @@ func TestProcessMultipleCheckItems(t *testing.T) { err = processCheck(ctx, client, obj, check) assert.NoError(t, err) - assert.Equal(t, "processed", obj.GetAnnotations()[statusAnnotation]) + assert.Equal(t, "processed", obj.GetAnnotations()[checks.StatusAnnotation]) r := client.lastValue.(advisorv0alpha1.CheckV0alpha1StatusReport) assert.Equal(t, r.Count, int64(100)) assert.Len(t, r.Failures, 50) @@ -110,7 +110,7 @@ func TestProcessMultipleCheckItems(t *testing.T) { func TestProcessCheck_AlreadyProcessed(t *testing.T) { obj := &advisorv0alpha1.Check{} - obj.SetAnnotations(map[string]string{statusAnnotation: "processed"}) + obj.SetAnnotations(map[string]string{checks.StatusAnnotation: "processed"}) client := &mockClient{} ctx := context.TODO() check := &mockCheck{} @@ -137,7 +137,7 @@ func TestProcessCheck_RunError(t *testing.T) { err = processCheck(ctx, client, obj, check) assert.Error(t, err) - assert.Equal(t, "error", obj.GetAnnotations()[statusAnnotation]) + assert.Equal(t, "error", obj.GetAnnotations()[checks.StatusAnnotation]) } type mockClient struct {