From 946358900c43b470d640230d73b0624aa93239de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Jim=C3=A9nez=20S=C3=A1nchez?= Date: Wed, 23 Jul 2025 11:33:25 +0200 Subject: [PATCH] Provisioning: Add warning job status and different strictness (#108446) * Add warning job status * Different strictness for export than for pulling * Fix some unit tests * Fix unit tests * Update frontend to display the warning status * Fix status badge * Add missing return statement * Improve text in alert --- pkg/apis/provisioning/v0alpha1/jobs.go | 5 ++- .../v0alpha1/zz_generated.openapi.go | 8 ++--- .../jobs/job_progress_recorder_mock.go | 27 ++++++++-------- .../apis/provisioning/jobs/migrate/legacy.go | 4 +++ .../provisioning/jobs/migrate/legacy_test.go | 31 ++++++++++++++++--- .../jobs/migrate/unifiedstorage.go | 2 ++ .../jobs/migrate/unifiedstorage_test.go | 4 +++ .../apis/provisioning/jobs/migrate/worker.go | 1 - .../provisioning/jobs/migrate/worker_test.go | 6 ---- .../apis/provisioning/jobs/progress.go | 18 ++++++----- .../provisioning/jobs/progress_fn_mock.go | 2 +- pkg/registry/apis/provisioning/jobs/queue.go | 2 +- .../provisioning/jobs/repo_getter_mock.go | 2 +- .../apis/provisioning/jobs/sync/worker.go | 2 ++ .../provisioning/jobs/sync/worker_test.go | 7 +++++ .../apis/provisioning/jobs/worker_mock.go | 2 +- .../apis/provisioning/resources/resources.go | 3 +- .../provisioning.grafana.app-v0alpha1.json | 6 ++-- .../provisioning/v0alpha1/endpoints.gen.ts | 6 ++-- .../provisioning/Job/FinishedJobStatus.tsx | 3 ++ .../features/provisioning/Job/JobContent.tsx | 11 ++++++- .../features/provisioning/Job/RecentJobs.tsx | 5 ++- .../provisioning/Shared/StatusBadge.tsx | 9 ++++-- public/locales/en-US/grafana.json | 3 +- 24 files changed, 119 insertions(+), 50 deletions(-) diff --git a/pkg/apis/provisioning/v0alpha1/jobs.go b/pkg/apis/provisioning/v0alpha1/jobs.go index f9801049cf4..f4fa20e8934 100644 --- a/pkg/apis/provisioning/v0alpha1/jobs.go +++ b/pkg/apis/provisioning/v0alpha1/jobs.go @@ -54,10 +54,13 @@ const ( // Finished with errors JobStateError JobState = "error" + + // Finished with some non-critical errors + JobStateWarning JobState = "warning" ) func (j JobState) Finished() bool { - return j == JobStateSuccess || j == JobStateError + return j == JobStateSuccess || j == JobStateError || j == JobStateWarning } type JobSpec struct { diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go index cdac15c0c6f..58a67095b53 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go @@ -890,10 +890,10 @@ func schema_pkg_apis_provisioning_v0alpha1_JobStatus(ref common.ReferenceCallbac Properties: map[string]spec.Schema{ "state": { SchemaProps: spec.SchemaProps{ - Description: "Possible enum values:\n - `\"error\"` Finished with errors\n - `\"pending\"` Job has been submitted, but not processed yet\n - `\"success\"` Finished with success\n - `\"working\"` The job is running", + Description: "Possible enum values:\n - `\"error\"` Finished with errors\n - `\"pending\"` Job has been submitted, but not processed yet\n - `\"success\"` Finished with success\n - `\"warning\"` Finished with some non-critical errors\n - `\"working\"` The job is running", Type: []string{"string"}, Format: "", - Enum: []interface{}{"error", "pending", "success", "working"}, + Enum: []interface{}{"error", "pending", "success", "warning", "working"}, }, }, "started": { @@ -2135,11 +2135,11 @@ func schema_pkg_apis_provisioning_v0alpha1_SyncStatus(ref common.ReferenceCallba Properties: map[string]spec.Schema{ "state": { SchemaProps: spec.SchemaProps{ - Description: "pending, running, success, error\n\nPossible enum values:\n - `\"error\"` Finished with errors\n - `\"pending\"` Job has been submitted, but not processed yet\n - `\"success\"` Finished with success\n - `\"working\"` The job is running", + Description: "pending, running, success, error\n\nPossible enum values:\n - `\"error\"` Finished with errors\n - `\"pending\"` Job has been submitted, but not processed yet\n - `\"success\"` Finished with success\n - `\"warning\"` Finished with some non-critical errors\n - `\"working\"` The job is running", Default: "", Type: []string{"string"}, Format: "", - Enum: []interface{}{"error", "pending", "success", "working"}, + Enum: []interface{}{"error", "pending", "success", "warning", "working"}, }, }, "job": { diff --git a/pkg/registry/apis/provisioning/jobs/job_progress_recorder_mock.go b/pkg/registry/apis/provisioning/jobs/job_progress_recorder_mock.go index 7ead8a206e7..54b07e6a38a 100644 --- a/pkg/registry/apis/provisioning/jobs/job_progress_recorder_mock.go +++ b/pkg/registry/apis/provisioning/jobs/job_progress_recorder_mock.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.53.4. DO NOT EDIT. +// Code generated by mockery v2.52.4. DO NOT EDIT. package jobs @@ -237,34 +237,35 @@ func (_c *MockJobProgressRecorder_SetTotal_Call) RunAndReturn(run func(context.C return _c } -// Strict provides a mock function with no fields -func (_m *MockJobProgressRecorder) Strict() { - _m.Called() +// StrictMaxErrors provides a mock function with given fields: maxErrors +func (_m *MockJobProgressRecorder) StrictMaxErrors(maxErrors int) { + _m.Called(maxErrors) } -// MockJobProgressRecorder_Strict_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Strict' -type MockJobProgressRecorder_Strict_Call struct { +// MockJobProgressRecorder_StrictMaxErrors_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'StrictMaxErrors' +type MockJobProgressRecorder_StrictMaxErrors_Call struct { *mock.Call } -// Strict is a helper method to define mock.On call -func (_e *MockJobProgressRecorder_Expecter) Strict() *MockJobProgressRecorder_Strict_Call { - return &MockJobProgressRecorder_Strict_Call{Call: _e.mock.On("Strict")} +// StrictMaxErrors is a helper method to define mock.On call +// - maxErrors int +func (_e *MockJobProgressRecorder_Expecter) StrictMaxErrors(maxErrors interface{}) *MockJobProgressRecorder_StrictMaxErrors_Call { + return &MockJobProgressRecorder_StrictMaxErrors_Call{Call: _e.mock.On("StrictMaxErrors", maxErrors)} } -func (_c *MockJobProgressRecorder_Strict_Call) Run(run func()) *MockJobProgressRecorder_Strict_Call { +func (_c *MockJobProgressRecorder_StrictMaxErrors_Call) Run(run func(maxErrors int)) *MockJobProgressRecorder_StrictMaxErrors_Call { _c.Call.Run(func(args mock.Arguments) { - run() + run(args[0].(int)) }) return _c } -func (_c *MockJobProgressRecorder_Strict_Call) Return() *MockJobProgressRecorder_Strict_Call { +func (_c *MockJobProgressRecorder_StrictMaxErrors_Call) Return() *MockJobProgressRecorder_StrictMaxErrors_Call { _c.Call.Return() return _c } -func (_c *MockJobProgressRecorder_Strict_Call) RunAndReturn(run func()) *MockJobProgressRecorder_Strict_Call { +func (_c *MockJobProgressRecorder_StrictMaxErrors_Call) RunAndReturn(run func(int)) *MockJobProgressRecorder_StrictMaxErrors_Call { _c.Run(run) return _c } diff --git a/pkg/registry/apis/provisioning/jobs/migrate/legacy.go b/pkg/registry/apis/provisioning/jobs/migrate/legacy.go index 191e0c4922f..f2a78b5181e 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/legacy.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/legacy.go @@ -41,6 +41,10 @@ func (m *LegacyMigrator) Migrate(ctx context.Context, rw repository.ReaderWriter Timeout: 10 * time.Minute, } + // Fail if migrating at least one + progress.StrictMaxErrors(1) + progress.SetMessage(ctx, "migrating legacy resources") + if err := m.wrapWithStageFn(ctx, rw, stageOptions, func(repo repository.Repository, staged bool) error { rw, ok := repo.(repository.ReaderWriter) if !ok { diff --git a/pkg/registry/apis/provisioning/jobs/migrate/legacy_test.go b/pkg/registry/apis/provisioning/jobs/migrate/legacy_test.go index 877bd01fd33..3b25de7aeaf 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/legacy_test.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/legacy_test.go @@ -34,6 +34,8 @@ func TestWrapWithStageFn(t *testing.T) { ) progress := jobs.NewMockJobProgressRecorder(t) + progress.On("StrictMaxErrors", 1).Return() + progress.On("SetMessage", mock.Anything, "migrating legacy resources").Return() // Execute repo := repository.NewMockRepository(t) @@ -67,6 +69,8 @@ func TestWrapWithCloneFn_Error(t *testing.T) { ) progress := jobs.NewMockJobProgressRecorder(t) + progress.On("StrictMaxErrors", 1).Return() + progress.On("SetMessage", mock.Anything, "migrating legacy resources").Return() // Execute repo := repository.NewMockRepository(t) repo.On("Config").Return(&provisioning.Repository{ @@ -109,6 +113,8 @@ func TestLegacyMigrator_MigrateFails(t *testing.T) { ) progress := jobs.NewMockJobProgressRecorder(t) + progress.On("StrictMaxErrors", 1).Return() + progress.On("SetMessage", mock.Anything, "migrating legacy resources").Return() // Execute repo := repository.NewMockRepository(t) @@ -158,7 +164,9 @@ func TestLegacyMigrator_ResetUnifiedStorageFails(t *testing.T) { ) progress := jobs.NewMockJobProgressRecorder(t) - progress.On("SetMessage", mock.Anything, mock.Anything).Return() + progress.On("StrictMaxErrors", 1).Return() + progress.On("SetMessage", mock.Anything, "migrating legacy resources").Return() + progress.On("SetMessage", mock.Anything, "resetting unified storage").Return() // Execute repo := repository.NewMockRepository(t) @@ -213,8 +221,12 @@ func TestLegacyMigrator_SyncFails(t *testing.T) { ) progress := jobs.NewMockJobProgressRecorder(t) - progress.On("SetMessage", mock.Anything, mock.Anything).Return() + progress.On("StrictMaxErrors", 1).Return() + progress.On("SetMessage", mock.Anything, "migrating legacy resources").Return() + progress.On("SetMessage", mock.Anything, "resetting unified storage").Return() progress.On("ResetResults").Return() + progress.On("SetMessage", mock.Anything, "pulling resources").Return() + progress.On("SetMessage", mock.Anything, "error importing resources, reverting").Return() // Execute repo := repository.NewMockRepository(t) @@ -268,8 +280,12 @@ func TestLegacyMigrator_SyncFails(t *testing.T) { ) progress := jobs.NewMockJobProgressRecorder(t) - progress.On("SetMessage", mock.Anything, mock.Anything).Return() + progress.On("StrictMaxErrors", 1).Return() + progress.On("SetMessage", mock.Anything, "migrating legacy resources").Return() + progress.On("SetMessage", mock.Anything, "resetting unified storage").Return() progress.On("ResetResults").Return() + progress.On("SetMessage", mock.Anything, "pulling resources").Return() + progress.On("SetMessage", mock.Anything, "error importing resources, reverting").Return() // Execute repo := repository.NewMockRepository(t) @@ -321,8 +337,11 @@ func TestLegacyMigrator_Success(t *testing.T) { ) progress := jobs.NewMockJobProgressRecorder(t) - progress.On("SetMessage", mock.Anything, mock.Anything).Return() + progress.On("StrictMaxErrors", 1).Return() + progress.On("SetMessage", mock.Anything, "migrating legacy resources").Return() + progress.On("SetMessage", mock.Anything, "resetting unified storage").Return() progress.On("ResetResults").Return() + progress.On("SetMessage", mock.Anything, "pulling resources").Return() // Execute repo := repository.NewMockRepository(t) @@ -364,6 +383,8 @@ func TestLegacyMigrator_BeforeFnExecution(t *testing.T) { progress := jobs.NewMockJobProgressRecorder(t) // No progress messages expected in current staging implementation + progress.On("StrictMaxErrors", 1).Return() + progress.On("SetMessage", mock.Anything, "migrating legacy resources").Return() // Execute repo := repository.NewMockRepository(t) @@ -398,6 +419,8 @@ func TestLegacyMigrator_ProgressScanner(t *testing.T) { progress := jobs.NewMockJobProgressRecorder(t) // No progress messages expected in current staging implementation + progress.On("StrictMaxErrors", 1).Return() + progress.On("SetMessage", mock.Anything, "migrating legacy resources").Return() repo := repository.NewMockRepository(t) repo.On("Config").Return(&provisioning.Repository{ diff --git a/pkg/registry/apis/provisioning/jobs/migrate/unifiedstorage.go b/pkg/registry/apis/provisioning/jobs/migrate/unifiedstorage.go index e2caeb7b3f4..d0b9739d72e 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/unifiedstorage.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/unifiedstorage.go @@ -33,6 +33,8 @@ func NewUnifiedStorageMigrator( func (m *UnifiedStorageMigrator) Migrate(ctx context.Context, repo repository.ReaderWriter, options provisioning.MigrateJobOptions, progress jobs.JobProgressRecorder) error { namespace := repo.Config().GetNamespace() progress.SetMessage(ctx, "export resources") + progress.StrictMaxErrors(1) // strict as we want the entire instance to be managed + exportJob := provisioning.Job{ Spec: provisioning.JobSpec{ Push: &provisioning.ExportJobOptions{}, diff --git a/pkg/registry/apis/provisioning/jobs/migrate/unifiedstorage_test.go b/pkg/registry/apis/provisioning/jobs/migrate/unifiedstorage_test.go index 992f99cb299..3cb48119842 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/unifiedstorage_test.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/unifiedstorage_test.go @@ -30,6 +30,7 @@ func TestUnifiedStorageMigrator_Migrate(t *testing.T) { }, }) pr.On("SetMessage", mock.Anything, "export resources").Return() + pr.On("StrictMaxErrors", 1).Return() ew.On("Process", mock.Anything, rw, mock.MatchedBy(func(job provisioning.Job) bool { return job.Spec.Push != nil }), pr).Return(errors.New("export failed")) @@ -46,6 +47,7 @@ func TestUnifiedStorageMigrator_Migrate(t *testing.T) { }, }) pr.On("SetMessage", mock.Anything, "export resources").Return() + pr.On("StrictMaxErrors", 1).Return() ew.On("Process", mock.Anything, rw, mock.MatchedBy(func(job provisioning.Job) bool { return job.Spec.Push != nil }), pr).Return(nil) @@ -67,6 +69,7 @@ func TestUnifiedStorageMigrator_Migrate(t *testing.T) { }, }) pr.On("SetMessage", mock.Anything, "export resources").Return() + pr.On("StrictMaxErrors", 1).Return() nc.On("Clean", mock.Anything, "test-namespace", pr).Return(errors.New("clean failed")) // Export and sync jobs succeed @@ -92,6 +95,7 @@ func TestUnifiedStorageMigrator_Migrate(t *testing.T) { }, }) pr.On("SetMessage", mock.Anything, "export resources").Return() + pr.On("StrictMaxErrors", 1).Return() // Export job succeeds ew.On("Process", mock.Anything, rw, mock.MatchedBy(func(job provisioning.Job) bool { return job.Spec.Push != nil diff --git a/pkg/registry/apis/provisioning/jobs/migrate/worker.go b/pkg/registry/apis/provisioning/jobs/migrate/worker.go index 736ecfc81b4..625b49ea32c 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/worker.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/worker.go @@ -43,7 +43,6 @@ func (w *MigrationWorker) Process(ctx context.Context, repo repository.Repositor return errors.New("missing migrate settings") } - progress.Strict() progress.SetTotal(ctx, 10) // will show a progress bar rw, ok := repo.(repository.ReaderWriter) if !ok { diff --git a/pkg/registry/apis/provisioning/jobs/migrate/worker_test.go b/pkg/registry/apis/provisioning/jobs/migrate/worker_test.go index 31242a48189..e7d8c7c6175 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/worker_test.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/worker_test.go @@ -62,7 +62,6 @@ func TestMigrationWorker_ProcessNotReaderWriter(t *testing.T) { } progressRecorder := jobs.NewMockJobProgressRecorder(t) progressRecorder.On("SetTotal", mock.Anything, 10).Return() - progressRecorder.On("Strict").Return() repo := repository.NewMockReader(t) err := worker.Process(context.Background(), repo, job, progressRecorder) @@ -87,7 +86,6 @@ func TestMigrationWorker_WithHistory(t *testing.T) { t.Run("fail local", func(t *testing.T) { progressRecorder := jobs.NewMockJobProgressRecorder(t) progressRecorder.On("SetTotal", mock.Anything, 10).Return() - progressRecorder.On("Strict").Return() repo := local.NewLocal(&provisioning.Repository{}, nil) err := worker.Process(context.Background(), repo, job, progressRecorder) @@ -97,7 +95,6 @@ func TestMigrationWorker_WithHistory(t *testing.T) { t.Run("fail unified", func(t *testing.T) { progressRecorder := jobs.NewMockJobProgressRecorder(t) progressRecorder.On("SetTotal", mock.Anything, 10).Return() - progressRecorder.On("Strict").Return() repo := repository.NewMockRepository(t) repo.On("Config").Return(&provisioning.Repository{ @@ -144,7 +141,6 @@ func TestMigrationWorker_Process(t *testing.T) { isLegacyActive: true, setupMocks: func(lm *MockMigrator, um *MockMigrator, ds *dualwrite.MockService, pr *jobs.MockJobProgressRecorder) { pr.On("SetTotal", mock.Anything, 10).Return() - pr.On("Strict").Return() ds.On("ReadFromUnified", mock.Anything, mock.Anything).Return(false, nil) lm.On("Migrate", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) }, @@ -160,7 +156,6 @@ func TestMigrationWorker_Process(t *testing.T) { isLegacyActive: false, setupMocks: func(lm *MockMigrator, um *MockMigrator, ds *dualwrite.MockService, pr *jobs.MockJobProgressRecorder) { pr.On("SetTotal", mock.Anything, 10).Return() - pr.On("Strict").Return() ds.On("ReadFromUnified", mock.Anything, mock.Anything).Return(true, nil) um.On("Migrate", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) }, @@ -176,7 +171,6 @@ func TestMigrationWorker_Process(t *testing.T) { isLegacyActive: true, setupMocks: func(lm *MockMigrator, um *MockMigrator, ds *dualwrite.MockService, pr *jobs.MockJobProgressRecorder) { pr.On("SetTotal", mock.Anything, 10).Return() - pr.On("Strict").Return() ds.On("ReadFromUnified", mock.Anything, mock.Anything).Return(false, nil) lm.On("Migrate", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(errors.New("migration failed")) }, diff --git a/pkg/registry/apis/provisioning/jobs/progress.go b/pkg/registry/apis/provisioning/jobs/progress.go index d23abcd9cd2..081ac9a3b86 100644 --- a/pkg/registry/apis/provisioning/jobs/progress.go +++ b/pkg/registry/apis/provisioning/jobs/progress.go @@ -60,8 +60,7 @@ type jobProgressRecorder struct { func newJobProgressRecorder(ProgressFn ProgressFn) JobProgressRecorder { return &jobProgressRecorder{ - maxErrors: 20, - started: time.Now(), + started: time.Now(), // Have a faster notifier for messages and total notifyImmediatelyFn: maybeNotifyProgress(500*time.Millisecond, ProgressFn), maybeNotifyFn: maybeNotifyProgress(5*time.Second, ProgressFn), @@ -126,9 +125,9 @@ func (r *jobProgressRecorder) SetTotal(ctx context.Context, total int) { r.notifyImmediately(ctx) } -func (r *jobProgressRecorder) Strict() { +func (r *jobProgressRecorder) StrictMaxErrors(maxErrors int) { r.mu.Lock() - r.maxErrors = 1 + r.maxErrors = maxErrors r.mu.Unlock() } @@ -136,7 +135,7 @@ func (r *jobProgressRecorder) TooManyErrors() error { r.mu.RLock() defer r.mu.RUnlock() - if r.errorCount >= r.maxErrors { + if r.maxErrors > 0 && r.errorCount >= r.maxErrors { return fmt.Errorf("too many errors: %d", r.errorCount) } @@ -256,8 +255,13 @@ func (r *jobProgressRecorder) Complete(ctx context.Context, err error) provision // Check for errors during execution if len(jobStatus.Errors) > 0 && jobStatus.State != provisioning.JobStateError { - jobStatus.State = provisioning.JobStateError - jobStatus.Message = "completed with errors" + if r.TooManyErrors() != nil { + jobStatus.Message = "completed with too many errors" + jobStatus.State = provisioning.JobStateError + } else { + jobStatus.Message = "completed with errors" + jobStatus.State = provisioning.JobStateWarning + } } // Override message if progress have a more explicit message diff --git a/pkg/registry/apis/provisioning/jobs/progress_fn_mock.go b/pkg/registry/apis/provisioning/jobs/progress_fn_mock.go index cfe7e46f88b..fd634d60844 100644 --- a/pkg/registry/apis/provisioning/jobs/progress_fn_mock.go +++ b/pkg/registry/apis/provisioning/jobs/progress_fn_mock.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.53.4. DO NOT EDIT. +// Code generated by mockery v2.52.4. DO NOT EDIT. package jobs diff --git a/pkg/registry/apis/provisioning/jobs/queue.go b/pkg/registry/apis/provisioning/jobs/queue.go index 0502fb7886a..32b935e4be8 100644 --- a/pkg/registry/apis/provisioning/jobs/queue.go +++ b/pkg/registry/apis/provisioning/jobs/queue.go @@ -24,7 +24,7 @@ type JobProgressRecorder interface { SetMessage(ctx context.Context, msg string) SetTotal(ctx context.Context, total int) TooManyErrors() error - Strict() + StrictMaxErrors(maxErrors int) Complete(ctx context.Context, err error) provisioning.JobStatus } diff --git a/pkg/registry/apis/provisioning/jobs/repo_getter_mock.go b/pkg/registry/apis/provisioning/jobs/repo_getter_mock.go index b7dc38de650..d0152e725bc 100644 --- a/pkg/registry/apis/provisioning/jobs/repo_getter_mock.go +++ b/pkg/registry/apis/provisioning/jobs/repo_getter_mock.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.53.4. DO NOT EDIT. +// Code generated by mockery v2.52.4. DO NOT EDIT. package jobs diff --git a/pkg/registry/apis/provisioning/jobs/sync/worker.go b/pkg/registry/apis/provisioning/jobs/sync/worker.go index 0fc3d839834..5b8eb701b34 100644 --- a/pkg/registry/apis/provisioning/jobs/sync/worker.go +++ b/pkg/registry/apis/provisioning/jobs/sync/worker.go @@ -97,6 +97,8 @@ func (r *SyncWorker) Process(ctx context.Context, repo repository.Repository, jo } progress.SetMessage(ctx, "execute sync job") + progress.StrictMaxErrors(20) // make it stop after 20 errors + currentRef, syncError := r.syncer.Sync(ctx, rw, *job.Spec.Pull, repositoryResources, clients, progress) jobStatus := progress.Complete(ctx, syncError) syncStatus = jobStatus.ToSyncStatus(job.Name) diff --git a/pkg/registry/apis/provisioning/jobs/sync/worker_test.go b/pkg/registry/apis/provisioning/jobs/sync/worker_test.go index b0171fc7fc3..02e152afb6a 100644 --- a/pkg/registry/apis/provisioning/jobs/sync/worker_test.go +++ b/pkg/registry/apis/provisioning/jobs/sync/worker_test.go @@ -225,6 +225,7 @@ func TestSyncWorker_Process(t *testing.T) { // Sync execution succeeds pr.On("SetMessage", mock.Anything, "execute sync job").Return() + pr.On("StrictMaxErrors", 20).Return() s.On("Sync", mock.Anything, rw, mock.MatchedBy(func(opts provisioning.SyncJobOptions) bool { return true // Add specific sync options validation if needed }), mockRepoResources, mock.Anything, pr).Return("new-ref", nil) @@ -277,6 +278,7 @@ func TestSyncWorker_Process(t *testing.T) { // Sync execution fails pr.On("SetMessage", mock.Anything, "execute sync job").Return() + pr.On("StrictMaxErrors", 20).Return() syncError := errors.New("sync operation failed") s.On("Sync", mock.Anything, rw, mock.MatchedBy(func(opts provisioning.SyncJobOptions) bool { return true // Add specific sync options validation if needed @@ -317,6 +319,7 @@ func TestSyncWorker_Process(t *testing.T) { mockClients := resources.NewMockResourceClients(t) cf.On("Clients", mock.Anything, mock.Anything).Return(mockClients, nil) pr.On("SetMessage", mock.Anything, mock.Anything).Return() + pr.On("StrictMaxErrors", 20).Return() pr.On("Complete", mock.Anything, mock.Anything).Return(provisioning.JobStatus{State: provisioning.JobStateSuccess}) rpf.On("Execute", mock.Anything, mock.Anything, mock.Anything).Return(nil) s.On("Sync", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("new-ref", nil) @@ -348,6 +351,7 @@ func TestSyncWorker_Process(t *testing.T) { mockClients := resources.NewMockResourceClients(t) cf.On("Clients", mock.Anything, mock.Anything).Return(mockClients, nil) pr.On("SetMessage", mock.Anything, mock.Anything).Return() + pr.On("StrictMaxErrors", 20).Return() pr.On("Complete", mock.Anything, mock.Anything).Return(provisioning.JobStatus{State: provisioning.JobStateSuccess}) s.On("Sync", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("new-ref", nil) }, @@ -407,6 +411,7 @@ func TestSyncWorker_Process(t *testing.T) { mockClients := resources.NewMockResourceClients(t) cf.On("Clients", mock.Anything, mock.Anything).Return(mockClients, nil) pr.On("SetMessage", mock.Anything, mock.Anything).Return() + pr.On("StrictMaxErrors", 20).Return() pr.On("Complete", mock.Anything, mock.Anything).Return(provisioning.JobStatus{State: provisioning.JobStateSuccess}) s.On("Sync", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("new-ref", nil) }, @@ -459,6 +464,7 @@ func TestSyncWorker_Process(t *testing.T) { mockClients := resources.NewMockResourceClients(t) cf.On("Clients", mock.Anything, mock.Anything).Return(mockClients, nil) pr.On("SetMessage", mock.Anything, mock.Anything).Return() + pr.On("StrictMaxErrors", 20).Return() pr.On("Complete", mock.Anything, mock.Anything).Return(provisioning.JobStatus{State: provisioning.JobStateSuccess}) s.On("Sync", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("new-ref", nil) }, @@ -489,6 +495,7 @@ func TestSyncWorker_Process(t *testing.T) { // Sync succeeds pr.On("SetMessage", mock.Anything, mock.Anything).Return() + pr.On("StrictMaxErrors", 20).Return() s.On("Sync", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("new-ref", nil) pr.On("Complete", mock.Anything, nil).Return(provisioning.JobStatus{State: provisioning.JobStateSuccess}) diff --git a/pkg/registry/apis/provisioning/jobs/worker_mock.go b/pkg/registry/apis/provisioning/jobs/worker_mock.go index 59e3c0df1b4..b264b258d1c 100644 --- a/pkg/registry/apis/provisioning/jobs/worker_mock.go +++ b/pkg/registry/apis/provisioning/jobs/worker_mock.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.53.4. DO NOT EDIT. +// Code generated by mockery v2.52.4. DO NOT EDIT. package jobs diff --git a/pkg/registry/apis/provisioning/resources/resources.go b/pkg/registry/apis/provisioning/resources/resources.go index b1f5cef53c2..28dd06ea43a 100644 --- a/pkg/registry/apis/provisioning/resources/resources.go +++ b/pkg/registry/apis/provisioning/resources/resources.go @@ -21,6 +21,7 @@ import ( var ( ErrAlreadyInRepository = errors.New("already in repository") + ErrDuplicateName = errors.New("duplicate name in repository") ErrMissingName = field.Required(field.NewPath("name", "metadata", "name"), "missing name in resource") ) @@ -152,7 +153,7 @@ func (r *ResourcesManager) WriteResourceFromFile(ctx context.Context, path strin } existing, found := r.resourcesLookup[id] if found { - return "", parsed.GVK, fmt.Errorf("duplicate resource name: %s, %s and %s", parsed.Obj.GetName(), path, existing) + return "", parsed.GVK, fmt.Errorf("duplicate resource name: %s, %s and %s: %w", parsed.Obj.GetName(), path, existing, ErrDuplicateName) } r.resourcesLookup[id] = path diff --git a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json index 3fc3d7c5bb4..76fe0149441 100644 --- a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json @@ -3096,12 +3096,13 @@ "format": "int64" }, "state": { - "description": "Possible enum values:\n - `\"error\"` Finished with errors\n - `\"pending\"` Job has been submitted, but not processed yet\n - `\"success\"` Finished with success\n - `\"working\"` The job is running", + "description": "Possible enum values:\n - `\"error\"` Finished with errors\n - `\"pending\"` Job has been submitted, but not processed yet\n - `\"success\"` Finished with success\n - `\"warning\"` Finished with some non-critical errors\n - `\"working\"` The job is running", "type": "string", "enum": [ "error", "pending", "success", + "warning", "working" ] }, @@ -4048,13 +4049,14 @@ "format": "int64" }, "state": { - "description": "pending, running, success, error\n\nPossible enum values:\n - `\"error\"` Finished with errors\n - `\"pending\"` Job has been submitted, but not processed yet\n - `\"success\"` Finished with success\n - `\"working\"` The job is running", + "description": "pending, running, success, error\n\nPossible enum values:\n - `\"error\"` Finished with errors\n - `\"pending\"` Job has been submitted, but not processed yet\n - `\"success\"` Finished with success\n - `\"warning\"` Finished with some non-critical errors\n - `\"working\"` The job is running", "type": "string", "default": "", "enum": [ "error", "pending", "success", + "warning", "working" ] } diff --git a/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts b/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts index 77a65889cca..7122a2e268b 100644 --- a/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts +++ b/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts @@ -819,8 +819,9 @@ export type JobStatus = { - `"error"` Finished with errors - `"pending"` Job has been submitted, but not processed yet - `"success"` Finished with success + - `"warning"` Finished with some non-critical errors - `"working"` The job is running */ - state?: 'error' | 'pending' | 'success' | 'working'; + state?: 'error' | 'pending' | 'success' | 'warning' | 'working'; /** Summary of processed actions */ summary?: JobResourceSummary[]; }; @@ -991,8 +992,9 @@ export type SyncStatus = { - `"error"` Finished with errors - `"pending"` Job has been submitted, but not processed yet - `"success"` Finished with success + - `"warning"` Finished with some non-critical errors - `"working"` The job is running */ - state: 'error' | 'pending' | 'success' | 'working'; + state: 'error' | 'pending' | 'success' | 'warning' | 'working'; }; export type WebhookStatus = { encryptedSecret?: string; diff --git a/public/app/features/provisioning/Job/FinishedJobStatus.tsx b/public/app/features/provisioning/Job/FinishedJobStatus.tsx index e51b79ebb5c..b5fd32c4353 100644 --- a/public/app/features/provisioning/Job/FinishedJobStatus.tsx +++ b/public/app/features/provisioning/Job/FinishedJobStatus.tsx @@ -42,6 +42,9 @@ export function FinishedJobStatus({ jobUid, repositoryName }: FinishedJobProps) }); } else if (job.status.state === 'success') { setStepStatusInfo({ status: 'success' }); + } else if (job.status.state === 'warning') { + // We treat warnings as success for now, but this could be changed later + setStepStatusInfo({ status: 'success' }); } } diff --git a/public/app/features/provisioning/Job/JobContent.tsx b/public/app/features/provisioning/Job/JobContent.tsx index 864da5adbc1..841800f2f7f 100644 --- a/public/app/features/provisioning/Job/JobContent.tsx +++ b/public/app/features/provisioning/Job/JobContent.tsx @@ -29,13 +29,22 @@ export function JobContent({ job, isFinishedJob = false }: JobContentProps) { title={t('provisioning.job-status.status.title-job-completed-successfully', 'Job completed successfully')} /> ); + case 'warning': + return ( + + {errors?.length ? errors?.join('\n') : message} + + ); case 'error': return ( - {message ?? errors?.join('\n')} + {errors?.length ? errors?.join('\n') : message} ); } diff --git a/public/app/features/provisioning/Job/RecentJobs.tsx b/public/app/features/provisioning/Job/RecentJobs.tsx index 8cf520979d3..cf1291bcb2f 100644 --- a/public/app/features/provisioning/Job/RecentJobs.tsx +++ b/public/app/features/provisioning/Job/RecentJobs.tsx @@ -26,8 +26,11 @@ const getStatusColor = (state?: SyncStatus['state']) => { case 'success': return 'green'; case 'working': - case 'pending': + return 'blue'; + case 'warning': return 'orange'; + case 'pending': + return 'darkgrey'; case 'error': return 'red'; default: diff --git a/public/app/features/provisioning/Shared/StatusBadge.tsx b/public/app/features/provisioning/Shared/StatusBadge.tsx index afd010c3b67..7923cf6549c 100644 --- a/public/app/features/provisioning/Shared/StatusBadge.tsx +++ b/public/app/features/provisioning/Shared/StatusBadge.tsx @@ -27,7 +27,7 @@ export function StatusBadge({ repo }: StatusBadgeProps) { text = 'Automatic pulling disabled'; icon = 'info-circle'; } else if (!repo.status?.sync?.state?.length) { - color = 'orange'; + color = 'darkgrey'; text = 'Pending'; icon = 'spinner'; tooltip = 'Waiting for health check to run'; @@ -40,8 +40,13 @@ export function StatusBadge({ repo }: StatusBadgeProps) { color = 'green'; break; case 'working': - case 'pending': + case 'warning': color = 'orange'; + text = 'warning'; + icon = 'exclamation-triangle'; + break; + case 'pending': + color = 'darkgrey'; text = 'Pulling'; icon = 'spinner'; break; diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 42438db155f..7a1a46a7138 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -11208,7 +11208,8 @@ "starting": "Starting...", "status": { "title-error-running-job": "Error running job", - "title-job-completed-successfully": "Job completed successfully" + "title-job-completed-successfully": "Job completed successfully", + "title-warning-running-job": "Job completed with warnings" }, "summary": "Summary" },