From b3a529de48f039ecf7ee8f21e81a67bc876887c8 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Thu, 20 Mar 2025 22:50:04 +0300 Subject: [PATCH] K8s/Dashboard: Use upsert to create or update dashboards (#102536) --- .../dashboards/service/dashboard_service.go | 37 ++++--------------- .../service/dashboard_service_test.go | 10 ++--- 2 files changed, 11 insertions(+), 36 deletions(-) diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index a8516b0d594..9e26875ff4c 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -11,8 +11,6 @@ import ( "time" "github.com/google/uuid" - "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacysearcher" - "github.com/grafana/grafana/pkg/util/retryer" "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel" "golang.org/x/exp/maps" @@ -34,6 +32,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/infra/slugify" + "github.com/grafana/grafana/pkg/registry/apis/dashboard/legacysearcher" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/apiserver" "github.com/grafana/grafana/pkg/services/apiserver/client" @@ -56,6 +55,7 @@ import ( "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/grafana/grafana/pkg/storage/unified/search" "github.com/grafana/grafana/pkg/util" + "github.com/grafana/grafana/pkg/util/retryer" ) var ( @@ -1576,12 +1576,13 @@ func (dr *DashboardServiceImpl) saveProvisionedDashboardThroughK8s(ctx context.C meta.SetManagerProperties(m) meta.SetSourceProperties(s) - out, err := dr.createOrUpdateDash(ctx, obj, cmd.OrgID) + // Update will create if not exists (upsert!) + out, err := dr.k8sclient.Update(ctx, obj, cmd.OrgID) if err != nil { return nil, err } - return out, nil + return dr.UnstructuredToLegacyDashboard(ctx, out, cmd.OrgID) } func (dr *DashboardServiceImpl) saveDashboardThroughK8s(ctx context.Context, cmd *dashboards.SaveDashboardCommand, orgID int64) (*dashboards.Dashboard, error) { @@ -1592,35 +1593,13 @@ func (dr *DashboardServiceImpl) saveDashboardThroughK8s(ctx context.Context, cmd dashboard.SetPluginIDMeta(obj, cmd.PluginID) - out, err := dr.createOrUpdateDash(ctx, obj, orgID) + // Update will create if not exists (upsert!) + out, err := dr.k8sclient.Update(ctx, obj, orgID) if err != nil { return nil, err } - return out, nil -} - -func (dr *DashboardServiceImpl) createOrUpdateDash(ctx context.Context, obj *unstructured.Unstructured, orgID int64) (*dashboards.Dashboard, error) { - var out *unstructured.Unstructured - current, err := dr.k8sclient.Get(ctx, obj.GetName(), orgID, v1.GetOptions{}) - if current == nil || err != nil { - out, err = dr.k8sclient.Create(ctx, obj, orgID) - if err != nil { - return nil, err - } - } else { - out, err = dr.k8sclient.Update(ctx, obj, orgID) - if err != nil { - return nil, err - } - } - - finalDash, err := dr.UnstructuredToLegacyDashboard(ctx, out, orgID) - if err != nil { - return nil, err - } - - return finalDash, nil + return dr.UnstructuredToLegacyDashboard(ctx, out, orgID) } func (dr *DashboardServiceImpl) deleteAllDashboardThroughK8s(ctx context.Context, orgID int64) error { diff --git a/pkg/services/dashboards/service/dashboard_service_test.go b/pkg/services/dashboards/service/dashboard_service_test.go index 0f68c97d2ec..78c38656329 100644 --- a/pkg/services/dashboards/service/dashboard_service_test.go +++ b/pkg/services/dashboards/service/dashboard_service_test.go @@ -1360,9 +1360,8 @@ func TestSaveProvisionedDashboard(t *testing.T) { t.Run("Should use Kubernetes create if feature flags are enabled", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) fakeStore.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&dashboards.Dashboard{}, nil) - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil) k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) - k8sCliMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) + k8sCliMock.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) k8sCliMock.On("GetNamespace", mock.Anything).Return("default") dashboard, err := service.SaveProvisionedDashboard(ctx, query, &dashboards.DashboardProvisioning{}) @@ -1422,10 +1421,9 @@ func TestSaveDashboard(t *testing.T) { t.Run("Should use Kubernetes create if feature flags are enabled and dashboard doesn't exist", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil) k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) k8sCliMock.On("GetNamespace", mock.Anything).Return("default") - k8sCliMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) + k8sCliMock.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) dashboard, err := service.SaveDashboard(ctx, query, false) require.NoError(t, err) @@ -1434,7 +1432,6 @@ func TestSaveDashboard(t *testing.T) { t.Run("Should use Kubernetes update if feature flags are enabled and dashboard exists", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) k8sCliMock.On("GetNamespace", mock.Anything).Return("default") k8sCliMock.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) @@ -1446,9 +1443,8 @@ func TestSaveDashboard(t *testing.T) { t.Run("Should return an error if uid is invalid", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, nil) k8sCliMock.On("GetNamespace", mock.Anything).Return("default") - k8sCliMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) + k8sCliMock.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&dashboardUnstructured, nil) query.Dashboard.UID = "invalid/uid" _, err := service.SaveDashboard(ctx, query, false)