From c45a342303a1682d70072ed2ade5afd59be0417e Mon Sep 17 00:00:00 2001 From: owensmallwood Date: Mon, 17 Mar 2025 16:46:26 -0600 Subject: [PATCH] Unified Storage Dashboard Provisioning: Wait for deleted dashboards to be updated in the indexer (#102243) * wait for deleted dashboards to be updated in the indexer * updates comment * adds test * make function private * fix failing test - had to add a couple more mock Search calls --- .../dashboards/service/dashboard_service.go | 23 ++++ .../service/dashboard_service_test.go | 104 +++++++++++++++++- 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index ad6c51afde2..16c68cff3e3 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -11,6 +11,7 @@ import ( "time" "github.com/google/uuid" + "github.com/grafana/grafana/pkg/util/retryer" "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel" "golang.org/x/exp/maps" @@ -559,6 +560,21 @@ func (dr *DashboardServiceImpl) ValidateDashboardBeforeSave(ctx context.Context, return isParentFolderChanged, nil } +// waitForSearchQuery waits for the search query to return the expected number of hits. +// Since US doesn't offer search-after-write guarantees, we can use this to wait after writes until the indexer is up to date. +func (dr *DashboardServiceImpl) waitForSearchQuery(ctx context.Context, query *dashboards.FindPersistedDashboardsQuery, maxRetries int, expectedHits int64) error { + return retryer.Retry(func() (retryer.RetrySignal, error) { + results, err := dr.searchDashboardsThroughK8sRaw(ctx, query) + if err != nil { + return retryer.FuncError, err + } + if results.TotalHits == expectedHits { + return retryer.FuncComplete, nil + } + return retryer.FuncFailure, nil + }, maxRetries, 1*time.Second, 5*time.Second) +} + func (dr *DashboardServiceImpl) DeleteOrphanedProvisionedDashboards(ctx context.Context, cmd *dashboards.DeleteOrphanedProvisionedDashboardsCommand) error { if dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesClientDashboardsFolders) { // check each org for orphaned provisioned dashboards @@ -580,10 +596,17 @@ func (dr *DashboardServiceImpl) DeleteOrphanedProvisionedDashboards(ctx context. } // delete them + var deletedUids []string for _, foundDash := range foundDashs { if err = dr.deleteDashboard(ctx, foundDash.DashboardID, foundDash.DashboardUID, org.ID, false); err != nil { return err } + deletedUids = append(deletedUids, foundDash.DashboardUID) + } + // wait for deleted dashboards to be removed from the index + err = dr.waitForSearchQuery(ctx, &dashboards.FindPersistedDashboardsQuery{OrgId: org.ID, DashboardUIDs: deletedUids}, 5, 0) + if err != nil { + return err } } return nil diff --git a/pkg/services/dashboards/service/dashboard_service_test.go b/pkg/services/dashboards/service/dashboard_service_test.go index 1f5f5d5f9ba..4c9562e11de 100644 --- a/pkg/services/dashboards/service/dashboard_service_test.go +++ b/pkg/services/dashboards/service/dashboard_service_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "gopkg.in/ini.v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -947,12 +946,115 @@ func TestDeleteOrphanedProvisionedDashboards(t *testing.T) { }, TotalHits: 2, }, nil).Once() + + // mock call to waitForSearchQuery() + k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.Anything).Return(&resource.ResourceSearchResponse{ + Results: &resource.ResourceTable{}, + TotalHits: 0, + }, nil).Twice() + err := service.DeleteOrphanedProvisionedDashboards(context.Background(), &dashboards.DeleteOrphanedProvisionedDashboardsCommand{ ReaderNames: []string{"test"}, }) require.NoError(t, err) k8sCliMock.AssertExpectations(t) }) + + t.Run("Should retry until deleted dashboard not found in search", func(t *testing.T) { + repo := "test" + singleOrgService := &DashboardServiceImpl{ + cfg: setting.NewCfg(), + dashboardStore: &fakeStore, + orgService: &orgtest.FakeOrgService{ + ExpectedOrgs: []*org.OrgDTO{{ID: 1}}, + }, + publicDashboardService: fakePublicDashboardService, + } + ctx, k8sCliMock := setupK8sDashboardTests(singleOrgService) + provisioningTimestamp := int64(1234567) + + // Call to searchProvisionedDashboardsThroughK8s() + k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default") + k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": dashboardv0alpha1.DashboardResourceInfo.GroupVersion().String(), + "kind": dashboardv0alpha1.DashboardResourceInfo.GroupVersionKind().Kind, + "metadata": map[string]interface{}{ + "name": "uid", + "labels": map[string]interface{}{ + utils.LabelKeyDeprecatedInternalID: "1", // nolint:staticcheck + }, + "annotations": map[string]interface{}{ + utils.AnnoKeyManagerKind: string(utils.ManagerKindClassicFP), // nolint:staticcheck + utils.AnnoKeyManagerIdentity: "test", + utils.AnnoKeySourceChecksum: "hash", + utils.AnnoKeySourcePath: "path/to/file", + utils.AnnoKeySourceTimestamp: fmt.Sprintf("%d", time.Unix(provisioningTimestamp, 0).UnixMilli()), + }, + }, + "spec": map[string]interface{}{ + "test": "test", + "version": int64(1), + "title": "testing slugify", + }, + }, + }, nil).Once() + k8sCliMock.On("Search", mock.Anything, int64(1), mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool { + // make sure the kind is added to the query + return req.Options.Fields[0].Values[0] == string(utils.ManagerKindClassicFP) && // nolint:staticcheck + req.Options.Fields[1].Values[0] == repo + })).Return(&resource.ResourceSearchResponse{ + Results: &resource.ResourceTable{ + Columns: []*resource.ResourceTableColumnDefinition{ + { + Name: "title", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + { + Name: "folder", + Type: resource.ResourceTableColumnDefinition_STRING, + }, + }, + Rows: []*resource.ResourceTableRow{ + { + Key: &resource.ResourceKey{ + Name: "uid", + Resource: "dashboard", + }, + Cells: [][]byte{ + []byte("Dashboard 1"), + []byte("folder 1"), + }, + }, + }, + }, + TotalHits: 1, + }, nil) + + // Mock deleteDashboard() + k8sCliMock.On("Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() + fakePublicDashboardService.On("DeleteByDashboardUIDs", mock.Anything, mock.Anything, mock.Anything).Return(nil) + fakeStore.On("CleanupAfterDelete", mock.Anything, mock.Anything).Return(nil).Once() + + // Mock WaitForSearchQuery() + // First call returns 1 hit + k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.Anything).Return(&resource.ResourceSearchResponse{ + Results: &resource.ResourceTable{}, + TotalHits: 1, + }, nil).Once() + + // Second call returns 0 hits + k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.Anything).Return(&resource.ResourceSearchResponse{ + Results: &resource.ResourceTable{}, + TotalHits: 0, + }, nil).Once() + + err := singleOrgService.DeleteOrphanedProvisionedDashboards(ctx, &dashboards.DeleteOrphanedProvisionedDashboardsCommand{ + ReaderNames: []string{"test"}, + }) + require.NoError(t, err) + k8sCliMock.AssertExpectations(t) + }) } func TestUnprovisionDashboard(t *testing.T) {