From f74092b26239b8ddf9269265de40b5faf9faa805 Mon Sep 17 00:00:00 2001 From: Todd Treece <360020+toddtreece@users.noreply.github.com> Date: Wed, 19 Feb 2025 14:35:00 -0500 Subject: [PATCH 01/10] K8s: Add discovery client (#100995) --- pkg/services/apiserver/client/discovery.go | 65 ++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 pkg/services/apiserver/client/discovery.go diff --git a/pkg/services/apiserver/client/discovery.go b/pkg/services/apiserver/client/discovery.go new file mode 100644 index 00000000000..bdf2a46a29e --- /dev/null +++ b/pkg/services/apiserver/client/discovery.go @@ -0,0 +1,65 @@ +package client + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/discovery" + "k8s.io/client-go/rest" +) + +type DiscoveryClient interface { + discovery.DiscoveryInterface + GetResourceForKind(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) + GetKindForResource(gvr schema.GroupVersionResource) (schema.GroupVersionKind, error) +} + +type DiscoveryClientImpl struct { + restConfig *rest.Config + discovery.DiscoveryInterface +} + +func NewDiscoveryClient(restConfig *rest.Config) (*DiscoveryClientImpl, error) { + discoveryClient, err := discovery.NewDiscoveryClientForConfig(restConfig) + if err != nil { + return nil, err + } + return &DiscoveryClientImpl{ + restConfig: restConfig, + DiscoveryInterface: discoveryClient, + }, nil +} + +func (d *DiscoveryClientImpl) GetResourceForKind(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) { + resourceList, err := d.ServerResourcesForGroupVersion(gvk.GroupVersion().String()) + if err != nil { + return schema.GroupVersionResource{}, err + } + for _, resource := range resourceList.APIResources { + if resource.Kind == gvk.Kind { + return schema.GroupVersionResource{ + Group: gvk.Group, + Version: gvk.Version, + Resource: resource.Name, + }, nil + } + } + return schema.GroupVersionResource{}, fmt.Errorf("resource not found for %s", gvk.String()) +} + +func (d *DiscoveryClientImpl) GetKindForResource(gvr schema.GroupVersionResource) (schema.GroupVersionKind, error) { + resourceList, err := d.ServerResourcesForGroupVersion(gvr.GroupVersion().String()) + if err != nil { + return schema.GroupVersionKind{}, err + } + for _, resource := range resourceList.APIResources { + if resource.Name == gvr.Resource { + return schema.GroupVersionKind{ + Group: gvr.Group, + Version: gvr.Version, + Kind: resource.Kind, + }, nil + } + } + return schema.GroupVersionKind{}, fmt.Errorf("kind not found for %s", gvr.String()) +} From 94c6f7d1e5b71742ff18cd8a31a33f27d41640f2 Mon Sep 17 00:00:00 2001 From: Todd Treece <360020+toddtreece@users.noreply.github.com> Date: Wed, 19 Feb 2025 15:30:45 -0500 Subject: [PATCH 02/10] K8s: Remove duplicate aggregation metric (#101002) --- pkg/services/apiserver/aggregator/aggregator.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/services/apiserver/aggregator/aggregator.go b/pkg/services/apiserver/aggregator/aggregator.go index 76a446c8990..0945ac83139 100644 --- a/pkg/services/apiserver/aggregator/aggregator.go +++ b/pkg/services/apiserver/aggregator/aggregator.go @@ -33,6 +33,7 @@ import ( "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/cache" + "k8s.io/component-base/metrics" "k8s.io/component-base/metrics/legacyregistry" "k8s.io/klog/v2" v1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" @@ -266,11 +267,11 @@ func CreateAggregatorServer(config *Config, delegateAPIServer genericapiserver.D } } - metrics := newAvailabilityMetrics() - + registry := legacyregistry.DefaultGatherer.(metrics.KubeRegistry) + availibilityMetrics := newAvailabilityMetrics() // create shared (remote and local) availability metrics // TODO: decouple from legacyregistry - registerIntoLegacyRegistryOnce.Do(func() { err = metrics.Register(legacyregistry.Register, legacyregistry.CustomRegister) }) + registerIntoLegacyRegistryOnce.Do(func() { err = availibilityMetrics.Register(registry.Register, registry.CustomRegister) }) if err != nil { return nil, err } @@ -282,7 +283,7 @@ func CreateAggregatorServer(config *Config, delegateAPIServer genericapiserver.D nil, proxyCurrentCertKeyContentFunc, completedConfig.ExtraConfig.ServiceResolver, - metrics, + availibilityMetrics, ) if err != nil { return nil, err From ffefe9013edd86153266210953f479c7c7fbde3b Mon Sep 17 00:00:00 2001 From: Robin Schneider <45321827+robinschneider@users.noreply.github.com> Date: Wed, 19 Feb 2025 21:37:11 +0100 Subject: [PATCH 03/10] Dockerfile: Add `org.opencontainers.image.source` label (#97462) Add org.opencontainers.image.source label to Dockerfile Co-authored-by: Matheus Macabu --- Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Dockerfile b/Dockerfile index 1d4bec46014..9914a48dce0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -124,6 +124,7 @@ FROM ${JS_SRC} AS js-src FROM ${BASE_IMAGE} LABEL maintainer="Grafana Labs " +LABEL org.opencontainers.image.source="https://github.com/grafana/grafana" ARG GF_UID="472" ARG GF_GID="0" From b00f2e02c7b682765bf8c6b71fc3607f02687df9 Mon Sep 17 00:00:00 2001 From: Charandas <542168+charandas@users.noreply.github.com> Date: Wed, 19 Feb 2025 12:39:40 -0800 Subject: [PATCH 04/10] K8s: don't publish metrics for local services that are always available (#101003) --- .../apiserver/aggregator/availableController.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/pkg/services/apiserver/aggregator/availableController.go b/pkg/services/apiserver/aggregator/availableController.go index e1935469c97..045e45a6d2e 100644 --- a/pkg/services/apiserver/aggregator/availableController.go +++ b/pkg/services/apiserver/aggregator/availableController.go @@ -127,7 +127,10 @@ func NewAvailableConditionController( func (c *AvailableConditionController) sync(key string) error { originalAPIService, err := c.apiServiceLister.Get(key) if apierrors.IsNotFound(err) { - c.metrics.ForgetAPIService(key) + if originalAPIService.Spec.Service != nil { + // Only reset state, if the service was a remote service + c.metrics.ForgetAPIService(key) + } return nil } if err != nil { @@ -292,7 +295,10 @@ func (c *AvailableConditionController) sync(key string) error { // apiservices. Doing that means we don't want to quickly issue no-op updates. func (c *AvailableConditionController) updateAPIServiceStatus(originalAPIService, newAPIService *apiregistrationv1.APIService) (*apiregistrationv1.APIService, error) { // update this metric on every sync operation to reflect the actual state - c.metrics.SetUnavailableGauge(newAPIService) + if newAPIService.Spec.Service != nil { + // Only expose the metric for remote services, trusts the type on the new object + c.metrics.SetUnavailableGauge(newAPIService) + } if equality.Semantic.DeepEqual(originalAPIService.Status, newAPIService.Status) { return newAPIService, nil @@ -319,7 +325,10 @@ func (c *AvailableConditionController) updateAPIServiceStatus(originalAPIService return nil, err } - c.metrics.SetUnavailableCounter(originalAPIService, newAPIService) + if newAPIService.Spec.Service != nil { + // Only expose the metric for remote services, trusts the type on the new object + c.metrics.SetUnavailableCounter(originalAPIService, newAPIService) + } return newAPIService, nil } From 55aaf4aac0db76bba135ea5e4b91e231b99bbeaa Mon Sep 17 00:00:00 2001 From: Todd Treece <360020+toddtreece@users.noreply.github.com> Date: Wed, 19 Feb 2025 16:11:26 -0500 Subject: [PATCH 05/10] Metrics: Add deny list in MultiRegistry (#101010) --- pkg/infra/metrics/service.go | 12 ++++++++++-- pkg/infra/metrics/service_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/pkg/infra/metrics/service.go b/pkg/infra/metrics/service.go index 99c1e048d4f..599c2c5ff8d 100644 --- a/pkg/infra/metrics/service.go +++ b/pkg/infra/metrics/service.go @@ -124,11 +124,16 @@ func (g *addPrefixWrapper) Gather() ([]*dto.MetricFamily, error) { var _ prometheus.Gatherer = (*multiRegistry)(nil) type multiRegistry struct { + denyList map[string]struct{} gatherers []prometheus.Gatherer } func NewMultiRegistry(gatherers ...prometheus.Gatherer) *multiRegistry { + denyList := map[string]struct{}{ + "grafana_apiserver_request_slo_duration_seconds_bucket": {}, + } return &multiRegistry{ + denyList: denyList, gatherers: gatherers, } } @@ -143,9 +148,12 @@ func (r *multiRegistry) Gather() (mfs []*dto.MetricFamily, err error) { for i := 0; i < len(mf); i++ { m := mf[i] - _, exists := names[*m.Name] + // skip metrics in the deny list + if _, denied := r.denyList[*m.Name]; denied { + continue + } // prevent duplicate metric names - if exists { + if _, exists := names[*m.Name]; exists { // we can skip go_ and process_ metrics without returning an error // because they are known to be duplicates in both // the k8s and prometheus gatherers. diff --git a/pkg/infra/metrics/service_test.go b/pkg/infra/metrics/service_test.go index 4ee13be81b2..5275ee0ba89 100644 --- a/pkg/infra/metrics/service_test.go +++ b/pkg/infra/metrics/service_test.go @@ -131,6 +131,30 @@ func TestMultiRegistry_Gather(t *testing.T) { require.NoError(t, err) require.Equal(t, expectedMF, mf) }) + + t.Run("denied metrics are not included", func(t *testing.T) { + one.GatherFunc = func() ([]*dto.MetricFamily, error) { + return []*dto.MetricFamily{ + {Name: strptr("grafana_apiserver_request_slo_duration_seconds_bucket")}, + }, nil + } + + two.GatherFunc = func() ([]*dto.MetricFamily, error) { + return []*dto.MetricFamily{ + {Name: strptr("b")}, + {Name: strptr("a")}, + }, nil + } + + expectedMF := []*dto.MetricFamily{ + {Name: strptr("a")}, + {Name: strptr("b")}, + } + + mf, err := g.Gather() + require.NoError(t, err) + require.Equal(t, expectedMF, mf) + }) } type mockGatherer struct { From 146af8f2be5992442dbc7ac8d7ed2ae1b5bbb255 Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Wed, 19 Feb 2025 15:40:05 -0700 Subject: [PATCH 06/10] K8s: Dashboards: Only get folder if exists on dashboard (#101017) --- pkg/services/dashboards/service/dashboard_service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index 6b6a768b992..aad82d9f7f8 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -366,7 +366,7 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d } // Validate folder - if dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesClientDashboardsFolders) { + if dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesClientDashboardsFolders) && (dash.FolderID != 0 || dash.FolderUID != "") { // nolint:staticcheck folder, err := dr.folderService.Get(ctx, &folder.GetFolderQuery{ OrgID: dash.OrgID, UID: &dash.FolderUID, From 29fe2df00a080ce5139cb083ff8e88f860f82904 Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Wed, 19 Feb 2025 16:43:58 -0700 Subject: [PATCH 07/10] Revert "feat(unified-storage): enhance gRPC client with dskit" (#101020) Revert "feat(unified-storage): enhance gRPC client with dskit (#100993)" This reverts commit 3f8a34303ce1b9a7cb0879a7f88f031d50e431e9. --- pkg/storage/unified/client.go | 52 ++++------------------------------- 1 file changed, 5 insertions(+), 47 deletions(-) diff --git a/pkg/storage/unified/client.go b/pkg/storage/unified/client.go index 75febc247e0..7b34b95ab92 100644 --- a/pkg/storage/unified/client.go +++ b/pkg/storage/unified/client.go @@ -6,7 +6,6 @@ import ( "path/filepath" "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "gocloud.dev/blob/fileblob" "google.golang.org/grpc" @@ -14,8 +13,6 @@ import ( authnlib "github.com/grafana/authlib/authn" "github.com/grafana/authlib/types" - "github.com/grafana/dskit/grpcclient" - "github.com/grafana/dskit/middleware" infraDB "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/tracing" @@ -42,10 +39,6 @@ type Options struct { Docs resource.DocumentBuilderSupplier } -type ClientMetrics struct { - requestDuration *prometheus.HistogramVec -} - // This adds a UnifiedStorage client into the wire dependency tree func ProvideUnifiedStorageClient(opts *Options) (resource.ResourceClient, error) { // See: apiserver.ApplyGrafanaConfig(cfg, features, o) @@ -111,8 +104,11 @@ func newClient(opts options.StorageOptions, return nil, fmt.Errorf("expecting address for storage_type: %s", opts.StorageType) } - // Create a connection to the gRPC server. - conn, err := grpcConn(opts.Address, reg) + // Create a connection to the gRPC server + conn, err := grpc.NewClient(opts.Address, + grpc.WithStatsHandler(otelgrpc.NewClientHandler()), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) if err != nil { return nil, err } @@ -157,41 +153,3 @@ func newResourceClient(conn *grpc.ClientConn, cfg *setting.Cfg, features feature } return resource.NewRemoteResourceClient(tracer, conn, clientCfgMapping(grpcutils.ReadGrpcClientConfig(cfg)), cfg.Env == setting.Dev) } - -// grpcConn creates a new gRPC connection to the provided address. -func grpcConn(address string, reg prometheus.Registerer) (*grpc.ClientConn, error) { - // This works for now as the Provide function is only called once during startup. - // We might eventually want to tie this factory to a struct for more runtime control. - metrics := ClientMetrics{ - requestDuration: promauto.With(reg).NewHistogramVec(prometheus.HistogramOpts{ - Name: "resource_server_client_request_duration_seconds", - Help: "Time spent executing requests to the resource server.", - Buckets: prometheus.ExponentialBuckets(0.008, 4, 7), - }, []string{"operation", "status_code"}), - } - - // Report gRPC status code errors as labels. - var instrumentationOptions []middleware.InstrumentationOption - instrumentationOptions = append(instrumentationOptions, middleware.ReportGRPCStatusOption) - unary, stream := grpcclient.Instrument(metrics.requestDuration, instrumentationOptions...) - - // We can later pass in the gRPC config here, i.e. to set MaxRecvMsgSize etc. - cfg := grpcclient.Config{} - opts, err := cfg.DialOption(unary, stream) - if err != nil { - return nil, fmt.Errorf("could not instrument grpc client: %w", err) - } - - opts = append(opts, grpc.WithStatsHandler(otelgrpc.NewClientHandler())) - opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials())) - - // Use round_robin to balance requests more evenly over the available Storage server. - opts = append(opts, grpc.WithDefaultServiceConfig(`{"loadBalancingPolicy":"round_robin"}`)) - - // Disable looking up service config from TXT DNS records. - // This reduces the number of requests made to the DNS servers. - opts = append(opts, grpc.WithDisableServiceConfig()) - - // Create a connection to the gRPC server - return grpc.NewClient(address, opts...) -} From 9637ebd2fa30551ae614c8751e6faa1dac28f02b Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Wed, 19 Feb 2025 17:16:05 -0700 Subject: [PATCH 08/10] K8s: Folder: Delete parent last (#101022) --- pkg/services/folder/folderimpl/folder_unifiedstorage.go | 4 +++- pkg/services/folder/folderimpl/folder_unifiedstorage_test.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage.go b/pkg/services/folder/folderimpl/folder_unifiedstorage.go index 92bb8e7180e..18202327590 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage.go @@ -637,10 +637,12 @@ func (s *Service) deleteFromApiServer(ctx context.Context, cmd *folder.DeleteFol return err } - folders := []string{cmd.UID} + folders := []string{} for _, f := range descFolders { folders = append(folders, f.UID) } + // must delete children first, then the parent folder + folders = append(folders, cmd.UID) if cmd.ForceDeleteRules { if err := s.deleteChildrenInFolder(ctx, cmd.OrgID, folders, cmd.SignedInUser); err != nil { diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go index 6ed300d80f5..244f11043f8 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go @@ -825,7 +825,7 @@ func TestDeleteFoldersFromApiServer(t *testing.T) { { Key: resource.SEARCH_FIELD_FOLDER, Operator: string(selection.In), - Values: []string{"uid", "uid2"}, + Values: []string{"uid2", "uid"}, }, }, }, From 66c420e067b169bb73e7f512011aed83f4f9b20f Mon Sep 17 00:00:00 2001 From: "grafana-pr-automation[bot]" <140550294+grafana-pr-automation[bot]@users.noreply.github.com> Date: Thu, 20 Feb 2025 02:33:19 +0200 Subject: [PATCH 09/10] I18n: Download translations from Crowdin (#101023) New Crowdin translations by GitHub Action Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- public/locales/de-DE/grafana.json | 2 ++ public/locales/es-ES/grafana.json | 2 ++ public/locales/fr-FR/grafana.json | 2 ++ public/locales/pt-BR/grafana.json | 2 ++ public/locales/zh-Hans/grafana.json | 2 ++ 5 files changed, 10 insertions(+) diff --git a/public/locales/de-DE/grafana.json b/public/locales/de-DE/grafana.json index 817b7b49c7b..ec82fb1aab8 100644 --- a/public/locales/de-DE/grafana.json +++ b/public/locales/de-DE/grafana.json @@ -2536,6 +2536,8 @@ "history-wrapper": { "collapse": "", "expand": "", + "icon-selected": "", + "icon-unselected": "", "show-more": "", "today": "", "yesterday": "" diff --git a/public/locales/es-ES/grafana.json b/public/locales/es-ES/grafana.json index aef7d9c6d99..47b9c17242a 100644 --- a/public/locales/es-ES/grafana.json +++ b/public/locales/es-ES/grafana.json @@ -2536,6 +2536,8 @@ "history-wrapper": { "collapse": "", "expand": "", + "icon-selected": "", + "icon-unselected": "", "show-more": "", "today": "", "yesterday": "" diff --git a/public/locales/fr-FR/grafana.json b/public/locales/fr-FR/grafana.json index 816cafefcc1..bdd9bbee422 100644 --- a/public/locales/fr-FR/grafana.json +++ b/public/locales/fr-FR/grafana.json @@ -2536,6 +2536,8 @@ "history-wrapper": { "collapse": "", "expand": "", + "icon-selected": "", + "icon-unselected": "", "show-more": "", "today": "", "yesterday": "" diff --git a/public/locales/pt-BR/grafana.json b/public/locales/pt-BR/grafana.json index 9edeeba05c5..2226b4a5d49 100644 --- a/public/locales/pt-BR/grafana.json +++ b/public/locales/pt-BR/grafana.json @@ -2536,6 +2536,8 @@ "history-wrapper": { "collapse": "", "expand": "", + "icon-selected": "", + "icon-unselected": "", "show-more": "", "today": "", "yesterday": "" diff --git a/public/locales/zh-Hans/grafana.json b/public/locales/zh-Hans/grafana.json index ca894dee800..d849cb5e829 100644 --- a/public/locales/zh-Hans/grafana.json +++ b/public/locales/zh-Hans/grafana.json @@ -2526,6 +2526,8 @@ "history-wrapper": { "collapse": "", "expand": "", + "icon-selected": "", + "icon-unselected": "", "show-more": "", "today": "", "yesterday": "" From f69e81facf5cc72afba4ebe7590bce573b487f7d Mon Sep 17 00:00:00 2001 From: Charandas <542168+charandas@users.noreply.github.com> Date: Wed, 19 Feb 2025 18:51:15 -0800 Subject: [PATCH 10/10] K8s: reduce discovery requests pressure on aggregated services (#101012) --- pkg/services/apiserver/aggregator/availableController.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/services/apiserver/aggregator/availableController.go b/pkg/services/apiserver/aggregator/availableController.go index 045e45a6d2e..fb285715f7d 100644 --- a/pkg/services/apiserver/aggregator/availableController.go +++ b/pkg/services/apiserver/aggregator/availableController.go @@ -204,6 +204,10 @@ func (c *AvailableConditionController) sync(key string) error { results := make(chan error, attempts) for i := 0; i < attempts; i++ { go func() { + // stagger these requests to reduce pressure on aggregated services + waitDuration := time.Second * time.Duration(int32(i)) + time.Sleep(waitDuration) + discoveryURL, err := c.serviceResolver.ResolveEndpoint(apiService.Spec.Service.Namespace, apiService.Spec.Service.Name, *apiService.Spec.Service.Port) if err != nil { results <- err