From f607bcd43f2a8ce0c08dbb084a303e32526ec1d3 Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Fri, 7 Feb 2025 15:48:18 +0100 Subject: [PATCH] Advisor: Replace Error naming with Failure (#100262) --- apps/advisor/kinds/check.cue | 16 ++++---- .../apis/advisor/v0alpha1/check_status_gen.go | 30 +++++++------- .../apis/advisor/v0alpha1/zz_openapi_gen.go | 24 +++++------ apps/advisor/pkg/apis/advisor_manifest.go | 2 +- .../pkg/app/checks/datasourcecheck/check.go | 12 +++--- .../app/checks/datasourcecheck/check_test.go | 40 +++++++++---------- apps/advisor/pkg/app/checks/ifaces.go | 2 +- .../pkg/app/checks/plugincheck/check.go | 12 +++--- .../pkg/app/checks/plugincheck/check_test.go | 36 ++++++++--------- apps/advisor/pkg/app/checks/utils.go | 8 ++-- apps/advisor/pkg/app/utils.go | 14 +++---- apps/advisor/pkg/app/utils_test.go | 6 +-- 12 files changed, 101 insertions(+), 101 deletions(-) diff --git a/apps/advisor/kinds/check.cue b/apps/advisor/kinds/check.cue index a564f888e2f..7ae338e888e 100644 --- a/apps/advisor/kinds/check.cue +++ b/apps/advisor/kinds/check.cue @@ -21,23 +21,23 @@ check: { // Generic data input that a check can receive data?: [string]: string } - #ReportError: { - // Severity of the error + #ReportFailure: { + // Severity of the failure severity: "high" | "low" - // Human readable reason for the error + // Human readable reason for the failure reason: string - // Action to take to resolve the error + // Action to take to resolve the failure action: string - // Step ID that the error is associated with + // Step ID that the failure is associated with stepID: string - // Item ID that the error is associated with + // Item ID that the failure is associated with itemID: string } #Report: { // Number of elements analyzed count: int - // List of errors - errors: [...#ReportError] + // List of failures + failures: [...#ReportFailure] } spec: #Data status: { diff --git a/apps/advisor/pkg/apis/advisor/v0alpha1/check_status_gen.go b/apps/advisor/pkg/apis/advisor/v0alpha1/check_status_gen.go index edc67575fe8..342751c6ac3 100644 --- a/apps/advisor/pkg/apis/advisor/v0alpha1/check_status_gen.go +++ b/apps/advisor/pkg/apis/advisor/v0alpha1/check_status_gen.go @@ -3,22 +3,22 @@ package v0alpha1 // +k8s:openapi-gen=true -type CheckReportError struct { - // Severity of the error - Severity CheckReportErrorSeverity `json:"severity"` - // Human readable reason for the error +type CheckReportFailure struct { + // Severity of the failure + Severity CheckReportFailureSeverity `json:"severity"` + // Human readable reason for the failure Reason string `json:"reason"` - // Action to take to resolve the error + // Action to take to resolve the failure Action string `json:"action"` - // Step ID that the error is associated with + // Step ID that the failure is associated with StepID string `json:"stepID"` - // Item ID that the error is associated with + // Item ID that the failure is associated with ItemID string `json:"itemID"` } -// NewCheckReportError creates a new CheckReportError object. -func NewCheckReportError() *CheckReportError { - return &CheckReportError{} +// NewCheckReportFailure creates a new CheckReportFailure object. +func NewCheckReportFailure() *CheckReportFailure { + return &CheckReportFailure{} } // +k8s:openapi-gen=true @@ -57,11 +57,11 @@ func NewCheckStatus() *CheckStatus { } // +k8s:openapi-gen=true -type CheckReportErrorSeverity string +type CheckReportFailureSeverity string const ( - CheckReportErrorSeverityHigh CheckReportErrorSeverity = "high" - CheckReportErrorSeverityLow CheckReportErrorSeverity = "low" + CheckReportFailureSeverityHigh CheckReportFailureSeverity = "high" + CheckReportFailureSeverityLow CheckReportFailureSeverity = "low" ) // +k8s:openapi-gen=true @@ -77,8 +77,8 @@ const ( type CheckV0alpha1StatusReport struct { // Number of elements analyzed Count int64 `json:"count"` - // List of errors - Errors []CheckReportError `json:"errors"` + // List of failures + Failures []CheckReportFailure `json:"failures"` } // NewCheckV0alpha1StatusReport creates a new CheckV0alpha1StatusReport object. diff --git a/apps/advisor/pkg/apis/advisor/v0alpha1/zz_openapi_gen.go b/apps/advisor/pkg/apis/advisor/v0alpha1/zz_openapi_gen.go index 10a2de58e84..6f84fa9ef4c 100644 --- a/apps/advisor/pkg/apis/advisor/v0alpha1/zz_openapi_gen.go +++ b/apps/advisor/pkg/apis/advisor/v0alpha1/zz_openapi_gen.go @@ -14,7 +14,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA return map[string]common.OpenAPIDefinition{ "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1.Check": schema_pkg_apis_advisor_v0alpha1_Check(ref), "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1.CheckList": schema_pkg_apis_advisor_v0alpha1_CheckList(ref), - "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1.CheckReportError": schema_pkg_apis_advisor_v0alpha1_CheckReportError(ref), + "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1.CheckReportFailure": schema_pkg_apis_advisor_v0alpha1_CheckReportFailure(ref), "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1.CheckSpec": schema_pkg_apis_advisor_v0alpha1_CheckSpec(ref), "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1.CheckStatus": schema_pkg_apis_advisor_v0alpha1_CheckStatus(ref), "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1.CheckType": schema_pkg_apis_advisor_v0alpha1_CheckType(ref), @@ -123,7 +123,7 @@ func schema_pkg_apis_advisor_v0alpha1_CheckList(ref common.ReferenceCallback) co } } -func schema_pkg_apis_advisor_v0alpha1_CheckReportError(ref common.ReferenceCallback) common.OpenAPIDefinition { +func schema_pkg_apis_advisor_v0alpha1_CheckReportFailure(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ SchemaProps: spec.SchemaProps{ @@ -131,7 +131,7 @@ func schema_pkg_apis_advisor_v0alpha1_CheckReportError(ref common.ReferenceCallb Properties: map[string]spec.Schema{ "severity": { SchemaProps: spec.SchemaProps{ - Description: "Severity of the error", + Description: "Severity of the failure", Default: "", Type: []string{"string"}, Format: "", @@ -139,7 +139,7 @@ func schema_pkg_apis_advisor_v0alpha1_CheckReportError(ref common.ReferenceCallb }, "reason": { SchemaProps: spec.SchemaProps{ - Description: "Human readable reason for the error", + Description: "Human readable reason for the failure", Default: "", Type: []string{"string"}, Format: "", @@ -147,7 +147,7 @@ func schema_pkg_apis_advisor_v0alpha1_CheckReportError(ref common.ReferenceCallb }, "action": { SchemaProps: spec.SchemaProps{ - Description: "Action to take to resolve the error", + Description: "Action to take to resolve the failure", Default: "", Type: []string{"string"}, Format: "", @@ -155,7 +155,7 @@ func schema_pkg_apis_advisor_v0alpha1_CheckReportError(ref common.ReferenceCallb }, "stepID": { SchemaProps: spec.SchemaProps{ - Description: "Step ID that the error is associated with", + Description: "Step ID that the failure is associated with", Default: "", Type: []string{"string"}, Format: "", @@ -163,7 +163,7 @@ func schema_pkg_apis_advisor_v0alpha1_CheckReportError(ref common.ReferenceCallb }, "itemID": { SchemaProps: spec.SchemaProps{ - Description: "Item ID that the error is associated with", + Description: "Item ID that the failure is associated with", Default: "", Type: []string{"string"}, Format: "", @@ -528,26 +528,26 @@ func schema_pkg_apis_advisor_v0alpha1_CheckV0alpha1StatusReport(ref common.Refer Format: "int64", }, }, - "errors": { + "failures": { SchemaProps: spec.SchemaProps{ - Description: "List of errors", + Description: "List of failures", Type: []string{"array"}, Items: &spec.SchemaOrArray{ Schema: &spec.Schema{ SchemaProps: spec.SchemaProps{ Default: map[string]interface{}{}, - Ref: ref("github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1.CheckReportError"), + Ref: ref("github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1.CheckReportFailure"), }, }, }, }, }, }, - Required: []string{"count", "errors"}, + Required: []string{"count", "failures"}, }, }, Dependencies: []string{ - "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1.CheckReportError"}, + "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1.CheckReportFailure"}, } } diff --git a/apps/advisor/pkg/apis/advisor_manifest.go b/apps/advisor/pkg/apis/advisor_manifest.go index 8b39d626436..4048f4f687a 100644 --- a/apps/advisor/pkg/apis/advisor_manifest.go +++ b/apps/advisor/pkg/apis/advisor_manifest.go @@ -12,7 +12,7 @@ import ( ) var ( - rawSchemaCheckv0alpha1 = []byte(`{"spec":{"properties":{"data":{"additionalProperties":{"type":"string"},"description":"Generic data input that a check can receive","type":"object"}},"type":"object"},"status":{"properties":{"additionalFields":{"description":"additionalFields is reserved for future use","type":"object","x-kubernetes-preserve-unknown-fields":true},"operatorStates":{"additionalProperties":{"properties":{"descriptiveState":{"description":"descriptiveState is an optional more descriptive state field which has no requirements on format","type":"string"},"details":{"description":"details contains any extra information that is operator-specific","type":"object","x-kubernetes-preserve-unknown-fields":true},"lastEvaluation":{"description":"lastEvaluation is the ResourceVersion last evaluated","type":"string"},"state":{"description":"state describes the state of the lastEvaluation.\nIt is limited to three possible states for machine evaluation.","enum":["success","in_progress","failed"],"type":"string"}},"required":["lastEvaluation","state"],"type":"object"},"description":"operatorStates is a map of operator ID to operator state evaluations.\nAny operator which consumes this kind SHOULD add its state evaluation information to this field.","type":"object"},"report":{"properties":{"count":{"description":"Number of elements analyzed","type":"integer"},"errors":{"description":"List of errors","items":{"properties":{"action":{"description":"Action to take to resolve the error","type":"string"},"itemID":{"description":"Item ID that the error is associated with","type":"string"},"reason":{"description":"Human readable reason for the error","type":"string"},"severity":{"description":"Severity of the error","enum":["high","low"],"type":"string"},"stepID":{"description":"Step ID that the error is associated with","type":"string"}},"required":["severity","reason","action","stepID","itemID"],"type":"object"},"type":"array"}},"required":["count","errors"],"type":"object"}},"required":["report"],"type":"object","x-kubernetes-preserve-unknown-fields":true}}`) + rawSchemaCheckv0alpha1 = []byte(`{"spec":{"properties":{"data":{"additionalProperties":{"type":"string"},"description":"Generic data input that a check can receive","type":"object"}},"type":"object"},"status":{"properties":{"additionalFields":{"description":"additionalFields is reserved for future use","type":"object","x-kubernetes-preserve-unknown-fields":true},"operatorStates":{"additionalProperties":{"properties":{"descriptiveState":{"description":"descriptiveState is an optional more descriptive state field which has no requirements on format","type":"string"},"details":{"description":"details contains any extra information that is operator-specific","type":"object","x-kubernetes-preserve-unknown-fields":true},"lastEvaluation":{"description":"lastEvaluation is the ResourceVersion last evaluated","type":"string"},"state":{"description":"state describes the state of the lastEvaluation.\nIt is limited to three possible states for machine evaluation.","enum":["success","in_progress","failed"],"type":"string"}},"required":["lastEvaluation","state"],"type":"object"},"description":"operatorStates is a map of operator ID to operator state evaluations.\nAny operator which consumes this kind SHOULD add its state evaluation information to this field.","type":"object"},"report":{"properties":{"count":{"description":"Number of elements analyzed","type":"integer"},"failures":{"description":"List of failures","items":{"properties":{"action":{"description":"Action to take to resolve the failure","type":"string"},"itemID":{"description":"Item ID that the failure is associated with","type":"string"},"reason":{"description":"Human readable reason for the failure","type":"string"},"severity":{"description":"Severity of the failure","enum":["high","low"],"type":"string"},"stepID":{"description":"Step ID that the failure is associated with","type":"string"}},"required":["severity","reason","action","stepID","itemID"],"type":"object"},"type":"array"}},"required":["count","failures"],"type":"object"}},"required":["report"],"type":"object","x-kubernetes-preserve-unknown-fields":true}}`) versionSchemaCheckv0alpha1 app.VersionSchema _ = json.Unmarshal(rawSchemaCheckv0alpha1, &versionSchemaCheckv0alpha1) rawSchemaCheckTypev0alpha1 = []byte(`{"spec":{"properties":{"name":{"type":"string"},"steps":{"items":{"properties":{"description":{"type":"string"},"stepID":{"type":"string"},"title":{"type":"string"}},"required":["title","description","stepID"],"type":"object"},"type":"array"}},"required":["name","steps"],"type":"object"},"status":{"properties":{"additionalFields":{"description":"additionalFields is reserved for future use","type":"object","x-kubernetes-preserve-unknown-fields":true},"operatorStates":{"additionalProperties":{"properties":{"descriptiveState":{"description":"descriptiveState is an optional more descriptive state field which has no requirements on format","type":"string"},"details":{"description":"details contains any extra information that is operator-specific","type":"object","x-kubernetes-preserve-unknown-fields":true},"lastEvaluation":{"description":"lastEvaluation is the ResourceVersion last evaluated","type":"string"},"state":{"description":"state describes the state of the lastEvaluation.\nIt is limited to three possible states for machine evaluation.","enum":["success","in_progress","failed"],"type":"string"}},"required":["lastEvaluation","state"],"type":"object"},"description":"operatorStates is a map of operator ID to operator state evaluations.\nAny operator which consumes this kind SHOULD add its state evaluation information to this field.","type":"object"}},"type":"object","x-kubernetes-preserve-unknown-fields":true}}`) diff --git a/apps/advisor/pkg/app/checks/datasourcecheck/check.go b/apps/advisor/pkg/app/checks/datasourcecheck/check.go index e946f2e2819..ff11740c62e 100644 --- a/apps/advisor/pkg/app/checks/datasourcecheck/check.go +++ b/apps/advisor/pkg/app/checks/datasourcecheck/check.go @@ -75,7 +75,7 @@ func (s *uidValidationStep) Description() string { return "Check if the UID of each data source is valid." } -func (s *uidValidationStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any) (*advisor.CheckReportError, error) { +func (s *uidValidationStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any) (*advisor.CheckReportFailure, error) { ds, ok := i.(*datasources.DataSource) if !ok { return nil, fmt.Errorf("invalid item type %T", i) @@ -83,8 +83,8 @@ func (s *uidValidationStep) Run(ctx context.Context, obj *advisor.CheckSpec, i a // Data source UID validation err := util.ValidateUID(ds.UID) if err != nil { - return checks.NewCheckReportError( - advisor.CheckReportErrorSeverityLow, + return checks.NewCheckReportFailure( + advisor.CheckReportFailureSeverityLow, fmt.Sprintf("Invalid UID '%s' for data source %s", ds.UID, ds.Name), "Check the documentation for more information.", s.ID(), @@ -111,7 +111,7 @@ func (s *healthCheckStep) ID() string { return "health-check" } -func (s *healthCheckStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any) (*advisor.CheckReportError, error) { +func (s *healthCheckStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any) (*advisor.CheckReportFailure, error) { ds, ok := i.(*datasources.DataSource) if !ok { return nil, fmt.Errorf("invalid item type %T", i) @@ -132,8 +132,8 @@ func (s *healthCheckStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any } resp, err := s.PluginClient.CheckHealth(ctx, req) if err != nil || resp.Status != backend.HealthStatusOk { - return checks.NewCheckReportError( - advisor.CheckReportErrorSeverityHigh, + return checks.NewCheckReportFailure( + advisor.CheckReportFailureSeverityHigh, fmt.Sprintf("Health check failed for %s", ds.Name), fmt.Sprintf( "Go to the data source configuration"+ diff --git a/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go b/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go index 5cfe595de67..c71122efc1b 100644 --- a/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go +++ b/apps/advisor/pkg/app/checks/datasourcecheck/check_test.go @@ -14,7 +14,7 @@ import ( ) func TestCheck_Run(t *testing.T) { - t.Run("should return no errors when all datasources are healthy", func(t *testing.T) { + t.Run("should return no failures when all datasources are healthy", func(t *testing.T) { datasources := []*datasources.DataSource{ {UID: "valid-uid-1", Type: "prometheus", Name: "Prometheus"}, {UID: "valid-uid-2", Type: "mysql", Name: "MySQL"}, @@ -33,23 +33,23 @@ func TestCheck_Run(t *testing.T) { ctx := identity.WithRequester(context.Background(), &user.SignedInUser{}) items, err := check.Items(ctx) assert.NoError(t, err) - errs := []advisor.CheckReportError{} + failures := []advisor.CheckReportFailure{} for _, step := range check.Steps() { for _, item := range items { - stepErr, err := step.Run(ctx, &advisor.CheckSpec{}, item) + stepFailures, err := step.Run(ctx, &advisor.CheckSpec{}, item) assert.NoError(t, err) - if stepErr != nil { - errs = append(errs, *stepErr) + if stepFailures != nil { + failures = append(failures, *stepFailures) } } } assert.NoError(t, err) assert.Equal(t, 2, len(items)) - assert.Empty(t, errs) + assert.Empty(t, failures) }) - t.Run("should return errors when datasource UID is invalid", func(t *testing.T) { + t.Run("should return failures when datasource UID is invalid", func(t *testing.T) { datasources := []*datasources.DataSource{ {UID: "invalid uid", Type: "prometheus", Name: "Prometheus"}, } @@ -67,24 +67,24 @@ func TestCheck_Run(t *testing.T) { ctx := identity.WithRequester(context.Background(), &user.SignedInUser{}) items, err := check.Items(ctx) assert.NoError(t, err) - errs := []advisor.CheckReportError{} + failures := []advisor.CheckReportFailure{} for _, step := range check.Steps() { for _, item := range items { - stepErr, err := step.Run(ctx, &advisor.CheckSpec{}, item) + stepFailures, err := step.Run(ctx, &advisor.CheckSpec{}, item) assert.NoError(t, err) - if stepErr != nil { - errs = append(errs, *stepErr) + if stepFailures != nil { + failures = append(failures, *stepFailures) } } } assert.NoError(t, err) assert.Equal(t, 1, len(items)) - assert.Len(t, errs, 1) - assert.Equal(t, "Invalid UID 'invalid uid' for data source Prometheus", errs[0].Reason) + assert.Len(t, failures, 1) + assert.Equal(t, "Invalid UID 'invalid uid' for data source Prometheus", failures[0].Reason) }) - t.Run("should return errors when datasource health check fails", func(t *testing.T) { + t.Run("should return failures when datasource health check fails", func(t *testing.T) { datasources := []*datasources.DataSource{ {UID: "valid-uid-1", Type: "prometheus", Name: "Prometheus"}, } @@ -102,21 +102,21 @@ func TestCheck_Run(t *testing.T) { ctx := identity.WithRequester(context.Background(), &user.SignedInUser{}) items, err := check.Items(ctx) assert.NoError(t, err) - errs := []advisor.CheckReportError{} + failures := []advisor.CheckReportFailure{} for _, step := range check.Steps() { for _, item := range items { - stepErr, err := step.Run(ctx, &advisor.CheckSpec{}, item) + stepFailures, err := step.Run(ctx, &advisor.CheckSpec{}, item) assert.NoError(t, err) - if stepErr != nil { - errs = append(errs, *stepErr) + if stepFailures != nil { + failures = append(failures, *stepFailures) } } } assert.NoError(t, err) assert.Equal(t, 1, len(items)) - assert.Len(t, errs, 1) - assert.Equal(t, "Health check failed for Prometheus", errs[0].Reason) + assert.Len(t, failures, 1) + assert.Equal(t, "Health check failed for Prometheus", failures[0].Reason) }) } diff --git a/apps/advisor/pkg/app/checks/ifaces.go b/apps/advisor/pkg/app/checks/ifaces.go index 915d986a4a2..7972baf3c57 100644 --- a/apps/advisor/pkg/app/checks/ifaces.go +++ b/apps/advisor/pkg/app/checks/ifaces.go @@ -25,5 +25,5 @@ type Step interface { // Description returns the description of the step Description() string // Run executes the step for an item and returns a report - Run(ctx context.Context, obj *advisorv0alpha1.CheckSpec, item any) (*advisorv0alpha1.CheckReportError, error) + Run(ctx context.Context, obj *advisorv0alpha1.CheckSpec, item any) (*advisorv0alpha1.CheckReportFailure, error) } diff --git a/apps/advisor/pkg/app/checks/plugincheck/check.go b/apps/advisor/pkg/app/checks/plugincheck/check.go index f9dc853d0b8..40b4328d9e0 100644 --- a/apps/advisor/pkg/app/checks/plugincheck/check.go +++ b/apps/advisor/pkg/app/checks/plugincheck/check.go @@ -78,7 +78,7 @@ func (s *deprecationStep) ID() string { return "deprecation" } -func (s *deprecationStep) Run(ctx context.Context, _ *advisor.CheckSpec, it any) (*advisor.CheckReportError, error) { +func (s *deprecationStep) Run(ctx context.Context, _ *advisor.CheckSpec, it any) (*advisor.CheckReportFailure, error) { p, ok := it.(pluginstore.Plugin) if !ok { return nil, fmt.Errorf("invalid item type %T", it) @@ -96,8 +96,8 @@ func (s *deprecationStep) Run(ctx context.Context, _ *advisor.CheckSpec, it any) return nil, nil } if i.Status == "deprecated" { - return checks.NewCheckReportError( - advisor.CheckReportErrorSeverityHigh, + return checks.NewCheckReportFailure( + advisor.CheckReportFailureSeverityHigh, fmt.Sprintf("Plugin deprecated: %s", p.ID), "Check the documentation for recommended steps.", s.ID(), @@ -125,7 +125,7 @@ func (s *updateStep) ID() string { return "update" } -func (s *updateStep) Run(ctx context.Context, _ *advisor.CheckSpec, i any) (*advisor.CheckReportError, error) { +func (s *updateStep) Run(ctx context.Context, _ *advisor.CheckSpec, i any) (*advisor.CheckReportFailure, error) { p, ok := i.(pluginstore.Plugin) if !ok { return nil, fmt.Errorf("invalid item type %T", i) @@ -149,8 +149,8 @@ func (s *updateStep) Run(ctx context.Context, _ *advisor.CheckSpec, i any) (*adv return nil, nil } if hasUpdate(p, info) { - return checks.NewCheckReportError( - advisor.CheckReportErrorSeverityLow, + return checks.NewCheckReportFailure( + advisor.CheckReportFailureSeverityLow, fmt.Sprintf("New version available for %s", p.ID), fmt.Sprintf( "Go to the plugin admin page"+ diff --git a/apps/advisor/pkg/app/checks/plugincheck/check_test.go b/apps/advisor/pkg/app/checks/plugincheck/check_test.go index ff5190278e2..1013d9818d1 100644 --- a/apps/advisor/pkg/app/checks/plugincheck/check_test.go +++ b/apps/advisor/pkg/app/checks/plugincheck/check_test.go @@ -21,12 +21,12 @@ func TestRun(t *testing.T) { pluginArchives map[string]*repo.PluginArchiveInfo pluginPreinstalled []string pluginManaged []string - expectedErrors []advisor.CheckReportError + expectedFailures []advisor.CheckReportFailure }{ { - name: "No plugins", - plugins: []pluginstore.Plugin{}, - expectedErrors: []advisor.CheckReportError{}, + name: "No plugins", + plugins: []pluginstore.Plugin{}, + expectedFailures: []advisor.CheckReportFailure{}, }, { name: "Deprecated plugin", @@ -39,9 +39,9 @@ func TestRun(t *testing.T) { pluginArchives: map[string]*repo.PluginArchiveInfo{ "plugin1": {Version: "1.0.0"}, }, - expectedErrors: []advisor.CheckReportError{ + expectedFailures: []advisor.CheckReportFailure{ { - Severity: advisor.CheckReportErrorSeverityHigh, + Severity: advisor.CheckReportFailureSeverityHigh, Reason: "Plugin deprecated: plugin1", Action: "Check the documentation for recommended steps.", StepID: "deprecation", @@ -60,9 +60,9 @@ func TestRun(t *testing.T) { pluginArchives: map[string]*repo.PluginArchiveInfo{ "plugin2": {Version: "1.1.0"}, }, - expectedErrors: []advisor.CheckReportError{ + expectedFailures: []advisor.CheckReportFailure{ { - Severity: advisor.CheckReportErrorSeverityLow, + Severity: advisor.CheckReportFailureSeverityLow, Reason: "New version available for plugin2", Action: "Go to the plugin admin page and upgrade to the latest version.", StepID: "update", @@ -81,9 +81,9 @@ func TestRun(t *testing.T) { pluginArchives: map[string]*repo.PluginArchiveInfo{ "plugin2": {Version: "beta"}, }, - expectedErrors: []advisor.CheckReportError{ + expectedFailures: []advisor.CheckReportFailure{ { - Severity: advisor.CheckReportErrorSeverityLow, + Severity: advisor.CheckReportFailureSeverityLow, Reason: "New version available for plugin2", Action: "Go to the plugin admin page and upgrade to the latest version.", StepID: "update", @@ -103,7 +103,7 @@ func TestRun(t *testing.T) { "plugin3": {Version: "1.1.0"}, }, pluginPreinstalled: []string{"plugin3"}, - expectedErrors: []advisor.CheckReportError{}, + expectedFailures: []advisor.CheckReportFailure{}, }, { name: "Managed plugin", @@ -116,8 +116,8 @@ func TestRun(t *testing.T) { pluginArchives: map[string]*repo.PluginArchiveInfo{ "plugin4": {Version: "1.1.0"}, }, - pluginManaged: []string{"plugin4"}, - expectedErrors: []advisor.CheckReportError{}, + pluginManaged: []string{"plugin4"}, + expectedFailures: []advisor.CheckReportFailure{}, }, } @@ -134,19 +134,19 @@ func TestRun(t *testing.T) { items, err := check.Items(context.Background()) assert.NoError(t, err) - errs := []advisor.CheckReportError{} + failures := []advisor.CheckReportFailure{} for _, step := range check.Steps() { for _, item := range items { - stepErr, err := step.Run(context.Background(), &advisor.CheckSpec{}, item) + stepFailures, err := step.Run(context.Background(), &advisor.CheckSpec{}, item) assert.NoError(t, err) - if stepErr != nil { - errs = append(errs, *stepErr) + if stepFailures != nil { + failures = append(failures, *stepFailures) } } } assert.NoError(t, err) assert.Equal(t, len(tt.plugins), len(items)) - assert.Equal(t, tt.expectedErrors, errs) + assert.Equal(t, tt.expectedFailures, failures) }) } } diff --git a/apps/advisor/pkg/app/checks/utils.go b/apps/advisor/pkg/app/checks/utils.go index 053dc591389..ee33d57076b 100644 --- a/apps/advisor/pkg/app/checks/utils.go +++ b/apps/advisor/pkg/app/checks/utils.go @@ -4,14 +4,14 @@ import ( advisor "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1" ) -func NewCheckReportError( - severity advisor.CheckReportErrorSeverity, +func NewCheckReportFailure( + severity advisor.CheckReportFailureSeverity, reason string, action string, stepID string, itemID string, -) *advisor.CheckReportError { - return &advisor.CheckReportError{ +) *advisor.CheckReportFailure { + return &advisor.CheckReportFailure{ Severity: severity, Reason: reason, Action: action, diff --git a/apps/advisor/pkg/app/utils.go b/apps/advisor/pkg/app/utils.go index b1809dd2ee3..731d16e8737 100644 --- a/apps/advisor/pkg/app/utils.go +++ b/apps/advisor/pkg/app/utils.go @@ -84,7 +84,7 @@ func processCheck(ctx context.Context, client resource.Client, obj resource.Obje } // Run the steps steps := check.Steps() - reportErrors, err := runStepsInParallel(ctx, &c.Spec, steps, items) + failures, err := runStepsInParallel(ctx, &c.Spec, steps, items) if err != nil { setErr := setStatusAnnotation(ctx, client, obj, "error") if setErr != nil { @@ -94,8 +94,8 @@ func processCheck(ctx context.Context, client resource.Client, obj resource.Obje } report := &advisorv0alpha1.CheckV0alpha1StatusReport{ - Errors: reportErrors, - Count: int64(len(items)), + Failures: failures, + Count: int64(len(items)), } err = setStatusAnnotation(ctx, client, obj, "processed") if err != nil { @@ -110,8 +110,8 @@ func processCheck(ctx context.Context, client resource.Client, obj resource.Obje }, resource.PatchOptions{}, obj) } -func runStepsInParallel(ctx context.Context, spec *advisorv0alpha1.CheckSpec, steps []checks.Step, items []any) ([]advisorv0alpha1.CheckReportError, error) { - reportErrs := []advisorv0alpha1.CheckReportError{} +func runStepsInParallel(ctx context.Context, spec *advisorv0alpha1.CheckSpec, steps []checks.Step, items []any) ([]advisorv0alpha1.CheckReportFailure, error) { + reportFailures := []advisorv0alpha1.CheckReportFailure{} var internalErr error var wg sync.WaitGroup var mu sync.Mutex @@ -133,11 +133,11 @@ func runStepsInParallel(ctx context.Context, spec *advisorv0alpha1.CheckSpec, st return } if stepErr != nil { - reportErrs = append(reportErrs, *stepErr) + reportFailures = append(reportFailures, *stepErr) } }(step, item) } } wg.Wait() - return reportErrs, internalErr + return reportFailures, internalErr } diff --git a/apps/advisor/pkg/app/utils_test.go b/apps/advisor/pkg/app/utils_test.go index 5905e2a9c44..ada844fdba4 100644 --- a/apps/advisor/pkg/app/utils_test.go +++ b/apps/advisor/pkg/app/utils_test.go @@ -105,7 +105,7 @@ func TestProcessMultipleCheckItems(t *testing.T) { assert.Equal(t, "processed", obj.GetAnnotations()[statusAnnotation]) r := client.lastValue.(advisorv0alpha1.CheckV0alpha1StatusReport) assert.Equal(t, r.Count, int64(100)) - assert.Len(t, r.Errors, 50) + assert.Len(t, r.Failures, 50) } func TestProcessCheck_AlreadyProcessed(t *testing.T) { @@ -173,12 +173,12 @@ type mockStep struct { err error } -func (m *mockStep) Run(ctx context.Context, obj *advisorv0alpha1.CheckSpec, items any) (*advisorv0alpha1.CheckReportError, error) { +func (m *mockStep) Run(ctx context.Context, obj *advisorv0alpha1.CheckSpec, items any) (*advisorv0alpha1.CheckReportFailure, error) { if m.err != nil { return nil, m.err } if _, ok := items.(error); ok { - return &advisorv0alpha1.CheckReportError{}, nil + return &advisorv0alpha1.CheckReportFailure{}, nil } return nil, nil }