Advisor: Create checks following a schedule (#100282)

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
 }
 

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,

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
+}

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)
+}

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,

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 {