From b56db69b32153d5c4682a69f683d4348a2a44109 Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Sun, 9 Mar 2025 03:19:40 -0600 Subject: [PATCH] K8s: Dashboard History: Improve Error Handling (#101816) --- .../dashboardversion/dashverimpl/dashver.go | 15 ++++++-- .../dashverimpl/dashver_test.go | 34 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/pkg/services/dashboardversion/dashverimpl/dashver.go b/pkg/services/dashboardversion/dashverimpl/dashver.go index 27bcbb845d1..dfa2d5444e7 100644 --- a/pkg/services/dashboardversion/dashverimpl/dashver.go +++ b/pkg/services/dashboardversion/dashverimpl/dashver.go @@ -7,6 +7,7 @@ import ( "strconv" "strings" + apierrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -223,8 +224,13 @@ func (s *Service) getDashIDMaybeEmpty(ctx context.Context, uid string, orgID int func (s *Service) getHistoryThroughK8s(ctx context.Context, orgID int64, dashboardUID string, rv int64) (*dashver.DashboardVersionDTO, error) { out, err := s.k8sclient.Get(ctx, dashboardUID, orgID, v1.GetOptions{ResourceVersion: strconv.FormatInt(rv, 10)}) if err != nil { + if apierrors.IsNotFound(err) { + return nil, dashboards.ErrDashboardNotFound + } + return nil, err - } else if out == nil { + } + if out == nil { return nil, dashboards.ErrDashboardNotFound } @@ -243,8 +249,13 @@ func (s *Service) listHistoryThroughK8s(ctx context.Context, orgID int64, dashbo Continue: continueToken, }) if err != nil { + if apierrors.IsNotFound(err) { + return nil, dashboards.ErrDashboardNotFound + } + return nil, err - } else if out == nil { + } + if out == nil { return nil, dashboards.ErrDashboardNotFound } diff --git a/pkg/services/dashboardversion/dashverimpl/dashver_test.go b/pkg/services/dashboardversion/dashverimpl/dashver_test.go index df074594e9e..03b2c2302c8 100644 --- a/pkg/services/dashboardversion/dashverimpl/dashver_test.go +++ b/pkg/services/dashboardversion/dashverimpl/dashver_test.go @@ -7,8 +7,10 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/components/simplejson" @@ -114,6 +116,23 @@ func TestDashboardVersionService(t *testing.T) { Data: simplejson.NewFromAny(map[string]any{"uid": "uid", "version": int64(11)}), }) }) + + t.Run("should dashboard not found error when k8s returns not found", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := Service{dashSvc: dashboardService, features: featuremgmt.WithFeatures()} + mockCli := new(client.MockK8sHandler) + dashboardVersionService.k8sclient = mockCli + dashboardVersionService.features = featuremgmt.WithFeatures(featuremgmt.FlagKubernetesClientDashboardsFolders) + dashboardService.On("GetDashboardUIDByID", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardRefByIDQuery")).Return(&dashboards.DashboardRef{UID: "uid"}, nil) + mockCli.On("Get", mock.Anything, "uid", int64(1), v1.GetOptions{ResourceVersion: "10"}, mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "dashboards.dashboard.grafana.app", Resource: "dashboard"}, "uid")) + + _, err := dashboardVersionService.Get(context.Background(), &dashver.GetDashboardVersionQuery{ + DashboardID: 42, + OrgID: 1, + Version: 10, + }) + require.ErrorIs(t, err, dashboards.ErrDashboardNotFound) + }) } func TestDeleteExpiredVersions(t *testing.T) { @@ -266,6 +285,21 @@ func TestListDashboardVersions(t *testing.T) { Data: simplejson.NewFromAny(map[string]any{"uid": "uid", "version": int64(5)}), }}}, res) }) + + t.Run("should return dashboard not found error when k8s client says not found", func(t *testing.T) { + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := Service{dashSvc: dashboardService, features: featuremgmt.WithFeatures()} + mockCli := new(client.MockK8sHandler) + dashboardVersionService.k8sclient = mockCli + dashboardVersionService.features = featuremgmt.WithFeatures(featuremgmt.FlagKubernetesClientDashboardsFolders) + dashboardService.On("GetDashboardUIDByID", mock.Anything, + mock.AnythingOfType("*dashboards.GetDashboardRefByIDQuery")). + Return(&dashboards.DashboardRef{UID: "uid"}, nil) + mockCli.On("List", mock.Anything, mock.Anything, mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "dashboards.dashboard.grafana.app", Resource: "dashboard"}, "uid")) + query := dashver.ListDashboardVersionsQuery{DashboardID: 42} + _, err := dashboardVersionService.List(context.Background(), &query) + require.ErrorIs(t, dashboards.ErrDashboardNotFound, err) + }) } type FakeDashboardVersionStore struct {