public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[release-12.0.1] Dashboards: Fix cleanup job (#104824)

Browse Source
This commit is contained in:
Stephanie Hingtgen
2025-05-01 16:51:44 +03:00
committed by GitHub
parent 5f3107c401
commit 3b2165d787
4 changed files with 44 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/dashboards/dashboard.go
+1 -1
View File
@@ -35,7 +35,7 @@ type DashboardService interface {
CountInFolders(ctx context.Context, orgID int64, folderUIDs []string, user identity.Requester) (int64, error)
GetAllDashboards(ctx context.Context) ([]*Dashboard, error)
GetAllDashboardsByOrgId(ctx context.Context, orgID int64) ([]*Dashboard, error)
CleanUpDashboard(ctx context.Context, dashboardUID string, orgId int64) error
CleanUpDashboard(ctx context.Context, dashboardUID string, dashboardId int64, orgId int64) error
CountDashboardsInOrg(ctx context.Context, orgID int64) (int64, error)
SetDefaultPermissions(ctx context.Context, dto *SaveDashboardDTO, dash *Dashboard, provisioned bool)
UnstructuredToLegacyDashboard(ctx context.Context, item *unstructured.Unstructured, orgID int64) (*Dashboard, error)
pkg/services/dashboards/dashboard_service_mock.go
+5 -5
View File
@@ -48,17 +48,17 @@ func (_m *FakeDashboardService) BuildSaveDashboardCommand(ctx context.Context, d
return r0, r1
}
// CleanUpDashboard provides a mock function with given fields: ctx, dashboardUID, orgId
func (_m *FakeDashboardService) CleanUpDashboard(ctx context.Context, dashboardUID string, orgId int64) error {
ret := _m.Called(ctx, dashboardUID, orgId)
// CleanUpDashboard provides a mock function with given fields: ctx, dashboardUID, dashboardId, orgId
func (_m *FakeDashboardService) CleanUpDashboard(ctx context.Context, dashboardUID string, dashboardId int64, orgId int64) error {
ret := _m.Called(ctx, dashboardUID, dashboardId, orgId)
if len(ret) == 0 {
panic("no return value specified for CleanUpDashboard")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, string, int64) error); ok {
r0 = rf(ctx, dashboardUID, orgId)
if rf, ok := ret.Get(0).(func(context.Context, string, int64, int64) error); ok {
r0 = rf(ctx, dashboardUID, dashboardId, orgId)
} else {
r0 = ret.Error(0)
}
pkg/services/dashboards/service/dashboard_service.go
+11 -3
View File
@@ -95,6 +95,7 @@ type DashboardServiceImpl struct {
publicDashboardService publicdashboards.ServiceWrapper
serverLockService *serverlock.ServerLockService
kvstore kvstore.KVStore
dual dualwrite.Service
dashboardPermissionsReady chan struct{}
}
@@ -149,6 +150,12 @@ func (dr *DashboardServiceImpl) cleanupK8sDashboardResources(ctx context.Context
return nil
}
readingFromLegacy := dualwrite.IsReadingLegacyDashboardsAndFolders(ctx, dr.dual)
if readingFromLegacy {
// Legacy does its own cleanup
return nil
}
// Create a timeout context to ensure we complete before the lock expires
ctx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()
@@ -344,7 +351,7 @@ func (dr *DashboardServiceImpl) processDashboardBatch(ctx context.Context, orgID
"deletionTimestamp", deletionTimestamp,
"resourceVersion", resourceVersion)
if err = dr.CleanUpDashboard(ctx, dash.UID, orgID); err != nil {
if err = dr.CleanUpDashboard(ctx, dash.UID, dash.ID, orgID); err != nil {
errs = append(errs, fmt.Errorf("failed to clean up dashboard %s: %w", dash.UID, err))
}
itemsProcessed++
@@ -394,6 +401,7 @@ func ProvideDashboardServiceImpl(
publicDashboardService: publicDashboardService,
serverLockService: serverLockService,
kvstore: kvstore,
dual: dual,
}
defaultLimits, err := readQuotaConfig(cfg)
@@ -1779,7 +1787,7 @@ func (dr *DashboardServiceImpl) DeleteInFolders(ctx context.Context, orgID int64
func (dr *DashboardServiceImpl) Kind() string { return entity.StandardKindDashboard }
func (dr *DashboardServiceImpl) CleanUpDashboard(ctx context.Context, dashboardUID string, orgId int64) error {
func (dr *DashboardServiceImpl) CleanUpDashboard(ctx context.Context, dashboardUID string, dashboardID int64, orgId int64) error {
ctx, span := tracer.Start(ctx, "dashboards.service.CleanUpDashboard")
defer span.End()
@@ -1789,7 +1797,7 @@ func (dr *DashboardServiceImpl) CleanUpDashboard(ctx context.Context, dashboardU
return err
}
return dr.dashboardStore.CleanupAfterDelete(ctx, &dashboards.DeleteDashboardCommand{OrgID: orgId, UID: dashboardUID})
return dr.dashboardStore.CleanupAfterDelete(ctx, &dashboards.DeleteDashboardCommand{OrgID: orgId, UID: dashboardUID, ID: dashboardID})
}
// -----------------------------------------------------------------------------------------
pkg/services/dashboards/service/dashboard_service_test.go
+27 -12
View File
@@ -43,6 +43,7 @@ import (
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/storage/legacysql/dualwrite"
"github.com/grafana/grafana/pkg/storage/unified/resource"
)
@@ -2577,6 +2578,7 @@ func TestCleanUpDashboard(t *testing.T) {
ctx := context.Background()
dashboardUID := "dash-uid"
dashboardID := int64(1)
orgID := int64(1)
// Setup mocks
@@ -2586,11 +2588,12 @@ func TestCleanUpDashboard(t *testing.T) {
fakeStore.On("CleanupAfterDelete", mock.Anything, &dashboards.DeleteDashboardCommand{
OrgID: orgID,
UID: dashboardUID,
ID: dashboardID,
}).Return(tc.cleanupError).Maybe()
}
// Execute
err := service.CleanUpDashboard(ctx, dashboardUID, orgID)
err := service.CleanUpDashboard(ctx, dashboardUID, dashboardID, orgID)
// Assert
if tc.expectedError != nil {
@@ -2608,11 +2611,12 @@ func TestCleanUpDashboard(t *testing.T) {
func TestK8sDashboardCleanupJob(t *testing.T) {
tests := []struct {
name string
featureEnabled bool
batchSize int
setupFunc func(*DashboardServiceImpl, context.Context, *client.MockK8sHandler)
verifyFunc func(*testing.T, *DashboardServiceImpl, context.Context, *client.MockK8sHandler, *kvstore.FakeKVStore)
name string
featureEnabled bool
readFromUnified bool
batchSize int
setupFunc func(*DashboardServiceImpl, context.Context, *client.MockK8sHandler)
verifyFunc func(*testing.T, *DashboardServiceImpl, context.Context, *client.MockK8sHandler, *kvstore.FakeKVStore)
}{
{
name: "Should not run cleanup when feature flag is disabled",
@@ -2620,9 +2624,16 @@ func TestK8sDashboardCleanupJob(t *testing.T) {
batchSize: 10,
},
{
name: "Should process dashboard cleanup for all orgs",
featureEnabled: true,
batchSize: 10,
name: "Should not run cleanup when feature flag is enabled but we're reading from legacy",
featureEnabled: true,
readFromUnified: false,
batchSize: 10,
},
{
name: "Should process dashboard cleanup for all orgs",
featureEnabled: true,
readFromUnified: true,
batchSize: 10,
setupFunc: func(service *DashboardServiceImpl, ctx context.Context, k8sCliMock *client.MockK8sHandler) {
// Test organizations
fakeOrgService := service.orgService.(*orgtest.FakeOrgService)
@@ -2690,9 +2701,10 @@ func TestK8sDashboardCleanupJob(t *testing.T) {
},
},
{
name: "Should handle pagination and batching when processing large sets of dashboards",
featureEnabled: true,
batchSize: 3,
name: "Should handle pagination and batching when processing large sets of dashboards",
featureEnabled: true,
readFromUnified: true,
batchSize: 3,
setupFunc: func(service *DashboardServiceImpl, ctx context.Context, k8sCliMock *client.MockK8sHandler) {
// Test organization
fakeOrgService := service.orgService.(*orgtest.FakeOrgService)
@@ -2776,6 +2788,8 @@ func TestK8sDashboardCleanupJob(t *testing.T) {
sqlStore, _ := sqlstore.InitTestDB(t)
lockService := serverlock.ProvideService(sqlStore, tracing.InitializeTracerForTest())
kv := kvstore.NewFakeKVStore()
dual := dualwrite.NewMockService(t)
dual.On("ReadFromUnified", mock.Anything, mock.Anything).Return(tc.readFromUnified, nil)
fakeStore := dashboards.FakeDashboardStore{}
fakePublicDashboardService := publicdashboards.NewFakePublicDashboardServiceWrapper(t)
@@ -2795,6 +2809,7 @@ func TestK8sDashboardCleanupJob(t *testing.T) {
serverLockService: lockService,
kvstore: kv,
features: features,
dual: dual,
}
ctx, k8sCliMock := setupK8sDashboardTests(service)