Skip to content

Commit 472902e

Browse files
committed
Add prometheus metrics for cache events
If implemented this will record cache events. Signed-off-by: Soule BA <soule@weave.works>
1 parent fe17137 commit 472902e

File tree

8 files changed

+212
-24
lines changed

8 files changed

+212
-24
lines changed

controllers/helmchart_controller.go

+15-13
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
"time"
3030

3131
helmgetter "helm.sh/helm/v3/pkg/getter"
32-
helmrepo "helm.sh/helm/v3/pkg/repo"
3332
corev1 "k8s.io/api/core/v1"
3433
apierrs "k8s.io/apimachinery/pkg/api/errors"
3534
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -123,6 +122,7 @@ type HelmChartReconciler struct {
123122

124123
Cache *cache.Cache
125124
TTL time.Duration
125+
*cache.CacheRecorder
126126
}
127127

128128
func (r *HelmChartReconciler) SetupWithManager(mgr ctrl.Manager) error {
@@ -484,7 +484,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
484484
}
485485

486486
// Initialize the chart repository
487-
chartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts)
487+
chartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts,
488+
repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
489+
r.IncCacheEvents(event, obj.Name, obj.Namespace)
490+
}))
488491
if err != nil {
489492
// Any error requires a change in generation,
490493
// which we should be informed about by the watcher
@@ -506,13 +509,6 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
506509
}
507510
}
508511

509-
// Try to retrieve the repository index from the cache
510-
if r.Cache != nil {
511-
if index, found := r.Cache.Get(r.Storage.LocalPath(*repo.GetArtifact())); found {
512-
chartRepo.Index = index.(*helmrepo.IndexFile)
513-
}
514-
}
515-
516512
// Construct the chart builder with scoped configuration
517513
cb := chart.NewRemoteBuilder(chartRepo)
518514
opts := chart.BuildOptions{
@@ -543,11 +539,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
543539
// The cache key have to be safe in multi-tenancy environments,
544540
// as otherwise it could be used as a vector to bypass the helm repository's authentication.
545541
// Using r.Storage.LocalPath(*repo.GetArtifact() is safe as the path is in the format /<helm-repository-name>/<chart-name>/<filename>.
546-
err := r.Cache.Set(r.Storage.LocalPath(*repo.GetArtifact()), chartRepo.Index, r.TTL)
542+
err := chartRepo.CacheIndexInMemory()
547543
if err != nil {
548544
r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.CacheOperationFailedReason, "failed to cache index: %s", err)
549545
}
550-
551546
}
552547

553548
// Delete the index reference
@@ -615,7 +610,7 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
615610

616611
// Setup dependency manager
617612
dm := chart.NewDependencyManager(
618-
chart.WithRepositoryCallback(r.namespacedChartRepositoryCallback(ctx, obj.GetNamespace())),
613+
chart.WithRepositoryCallback(r.namespacedChartRepositoryCallback(ctx, obj.GetName(), obj.GetNamespace())),
619614
)
620615
defer dm.Clear()
621616

@@ -847,7 +842,7 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *sourcev1.
847842
// namespacedChartRepositoryCallback returns a chart.GetChartRepositoryCallback scoped to the given namespace.
848843
// The returned callback returns a repository.ChartRepository configured with the retrieved v1beta1.HelmRepository,
849844
// or a shim with defaults if no object could be found.
850-
func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, namespace string) chart.GetChartRepositoryCallback {
845+
func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartRepositoryCallback {
851846
return func(url string) (*repository.ChartRepository, error) {
852847
var tlsConfig *tls.Config
853848
repo, err := r.resolveDependencyRepository(ctx, url, namespace)
@@ -888,8 +883,15 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
888883
if err != nil {
889884
return nil, err
890885
}
886+
887+
// Ensure that the cache key is the same as the artifact path
888+
// otherwise don't enable caching. We don't want to cache indexes
889+
// for repositories that are not reconciled by the source controller.
891890
if repo.Status.Artifact != nil {
892891
chartRepo.CachePath = r.Storage.LocalPath(*repo.GetArtifact())
892+
chartRepo.SetMemCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
893+
r.IncCacheEvents(event, name, namespace)
894+
})
893895
}
894896
return chartRepo, nil
895897
}

controllers/suite_test.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -129,15 +129,17 @@ func TestMain(m *testing.M) {
129129
panic(fmt.Sprintf("Failed to start HelmRepositoryReconciler: %v", err))
130130
}
131131

132-
cache := cache.New(5, 1*time.Second)
132+
c := cache.New(5, 1*time.Second)
133+
cacheRecorder := cache.MustMakeMetrics()
133134
if err := (&HelmChartReconciler{
134135
Client: testEnv,
135136
EventRecorder: record.NewFakeRecorder(32),
136137
Metrics: testMetricsH,
137138
Getters: testGetters,
138139
Storage: testStorage,
139-
Cache: cache,
140+
Cache: c,
140141
TTL: 1 * time.Second,
142+
CacheRecorder: cacheRecorder,
141143
}).SetupWithManager(testEnv); err != nil {
142144
panic(fmt.Sprintf("Failed to start HelmRepositoryReconciler: %v", err))
143145
}

go.mod

+2-1
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ replace github.com/opencontainers/image-spec => github.com/opencontainers/image-
7676
// Fix CVE-2021-43816
7777
replace github.com/containerd/containerd => github.com/containerd/containerd v1.6.1
7878

79+
require github.com/prometheus/client_golang v1.12.1
80+
7981
require (
8082
cloud.google.com/go v0.100.2 // indirect
8183
cloud.google.com/go/compute v1.5.0 // indirect
@@ -178,7 +180,6 @@ require (
178180
github.com/pkg/browser v0.0.0-20210115035449-ce105d075bb4 // indirect
179181
github.com/pkg/errors v0.9.1 // indirect
180182
github.com/pmezard/go-difflib v1.0.0 // indirect
181-
github.com/prometheus/client_golang v1.12.1 // indirect
182183
github.com/prometheus/client_model v0.2.0 // indirect
183184
github.com/prometheus/common v0.32.1 // indirect
184185
github.com/prometheus/procfs v0.7.3 // indirect

internal/cache/metrics.go

+75
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/*
2+
Copyright 2022 The Flux authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package cache
18+
19+
import (
20+
"github.com/prometheus/client_golang/prometheus"
21+
"sigs.k8s.io/controller-runtime/pkg/metrics"
22+
)
23+
24+
const (
25+
// CacheEventTypeMiss is the event type for cache misses.
26+
CacheEventTypeMiss = "cache_miss"
27+
// CacheEventTypeHit is the event type for cache hits.
28+
CacheEventTypeHit = "cache_hit"
29+
)
30+
31+
// CacheRecorder is a recorder for cache events.
32+
type CacheRecorder struct {
33+
// cacheEventsCounter is a counter for cache events.
34+
cacheEventsCounter *prometheus.CounterVec
35+
}
36+
37+
// NewCacheRecorder returns a new CacheRecorder.
38+
// The configured labels are: event_type, name, namespace.
39+
// The event_type is one of:
40+
// - "miss"
41+
// - "hit"
42+
// - "update"
43+
// The name is the name of the reconciled resource.
44+
// The namespace is the namespace of the reconciled resource.
45+
func NewCacheRecorder() *CacheRecorder {
46+
return &CacheRecorder{
47+
cacheEventsCounter: prometheus.NewCounterVec(
48+
prometheus.CounterOpts{
49+
Name: "gotk_cache_events_total",
50+
Help: "Total number of cache retrieval events for a Gitops Toolkit resource reconciliation.",
51+
},
52+
[]string{"event_type", "name", "namespace"},
53+
),
54+
}
55+
}
56+
57+
// Collectors returns the metrics.Collector objects for the CacheRecorder.
58+
func (r *CacheRecorder) Collectors() []prometheus.Collector {
59+
return []prometheus.Collector{
60+
r.cacheEventsCounter,
61+
}
62+
}
63+
64+
// IncCacheEventCount increment by 1 the cache event count for the given event type, name and namespace.
65+
func (r *CacheRecorder) IncCacheEvents(event, name, namespace string) {
66+
r.cacheEventsCounter.WithLabelValues(event, name, namespace).Inc()
67+
}
68+
69+
// MustMakeMetrics creates a new CacheRecorder, and registers the metrics collectors in the controller-runtime metrics registry.
70+
func MustMakeMetrics() *CacheRecorder {
71+
r := NewCacheRecorder()
72+
metrics.Registry.MustRegister(r.Collectors()...)
73+
74+
return r
75+
}

internal/helm/chart/builder_remote.go

+3-5
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,9 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
7373
}
7474

7575
// Load the repository index if not already present.
76-
if b.remote.Index == nil {
77-
if err := b.remote.LoadFromCache(); err != nil {
78-
err = fmt.Errorf("could not load repository index for remote chart reference: %w", err)
79-
return nil, &BuildError{Reason: ErrChartPull, Err: err}
80-
}
76+
if err := b.remote.StrategicallyLoadIndex(); err != nil {
77+
err = fmt.Errorf("could not load repository index for remote chart reference: %w", err)
78+
return nil, &BuildError{Reason: ErrChartPull, Err: err}
8179
}
8280

8381
// Get the current version for the RemoteReference

internal/helm/chart/dependency_manager.go

+3
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ func NewDependencyManager(opts ...DependencyManagerOption) *DependencyManager {
9797
func (dm *DependencyManager) Clear() []error {
9898
var errs []error
9999
for _, v := range dm.repositories {
100+
if err := v.CacheIndexInMemory(); err != nil {
101+
errs = append(errs, err)
102+
}
100103
v.Unload()
101104
if err := v.RemoveCache(); err != nil {
102105
errs = append(errs, err)

internal/helm/repository/chart_repository.go

+106-3
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"sort"
3131
"strings"
3232
"sync"
33+
"time"
3334

3435
"github.com/Masterminds/semver/v3"
3536
"helm.sh/helm/v3/pkg/getter"
@@ -38,6 +39,7 @@ import (
3839

3940
"github.com/fluxcd/pkg/version"
4041

42+
"github.com/fluxcd/source-controller/internal/cache"
4143
"github.com/fluxcd/source-controller/internal/helm"
4244
"github.com/fluxcd/source-controller/internal/transport"
4345
)
@@ -70,13 +72,52 @@ type ChartRepository struct {
7072
tlsConfig *tls.Config
7173

7274
*sync.RWMutex
75+
76+
cacheInfo
77+
}
78+
79+
type cacheInfo struct {
80+
// In memory cache of the index.yaml file.
81+
IndexCache *cache.Cache
82+
// IndexKey is the cache key for the index.yaml file.
83+
IndexKey string
84+
// IndexTTL is the cache TTL for the index.yaml file.
85+
IndexTTL time.Duration
86+
// RecordIndexCacheMetric records the cache hit/miss metrics for the index.yaml file.
87+
RecordIndexCacheMetric RecordMetricsFunc
88+
}
89+
90+
// ChartRepositoryOptions is a function that can be passed to NewChartRepository
91+
// to configure a ChartRepository.
92+
type ChartRepositoryOptions func(*ChartRepository) error
93+
94+
// RecordMetricsFunc is a function that records metrics.
95+
type RecordMetricsFunc func(event string)
96+
97+
// WithMemoryCache returns a ChartRepositoryOptions that will enable the
98+
// ChartRepository to cache the index.yaml file in memory.
99+
// The cache key have to be safe in multi-tenancy environments,
100+
// as otherwise it could be used as a vector to bypass the helm repository's authentication.
101+
func WithMemoryCache(key string, c *cache.Cache, ttl time.Duration, rec RecordMetricsFunc) ChartRepositoryOptions {
102+
return func(r *ChartRepository) error {
103+
if c != nil {
104+
if key == "" {
105+
return errors.New("cache key cannot be empty")
106+
}
107+
}
108+
r.IndexCache = c
109+
r.IndexKey = key
110+
r.IndexTTL = ttl
111+
r.RecordIndexCacheMetric = rec
112+
return nil
113+
}
73114
}
74115

75116
// NewChartRepository constructs and returns a new ChartRepository with
76117
// the ChartRepository.Client configured to the getter.Getter for the
77118
// repository URL scheme. It returns an error on URL parsing failures,
78119
// or if there is no getter available for the scheme.
79-
func NewChartRepository(repositoryURL, cachePath string, providers getter.Providers, tlsConfig *tls.Config, opts []getter.Option) (*ChartRepository, error) {
120+
func NewChartRepository(repositoryURL, cachePath string, providers getter.Providers, tlsConfig *tls.Config, opts []getter.Option, chartRepoOpts ...ChartRepositoryOptions) (*ChartRepository, error) {
80121
u, err := url.Parse(repositoryURL)
81122
if err != nil {
82123
return nil, err
@@ -92,6 +133,13 @@ func NewChartRepository(repositoryURL, cachePath string, providers getter.Provid
92133
r.Client = c
93134
r.Options = opts
94135
r.tlsConfig = tlsConfig
136+
137+
for _, opt := range chartRepoOpts {
138+
if err := opt(r); err != nil {
139+
return nil, err
140+
}
141+
}
142+
95143
return r, nil
96144
}
97145

@@ -292,14 +340,39 @@ func (r *ChartRepository) CacheIndex() (string, error) {
292340
return hex.EncodeToString(h.Sum(nil)), nil
293341
}
294342

295-
// StrategicallyLoadIndex lazy-loads the Index from CachePath using
296-
// LoadFromCache if it does not HasIndex.
343+
// CacheIndexInMemory attempts to cache the index in memory.
344+
// It returns an error if it fails.
345+
// The cache key have to be safe in multi-tenancy environments,
346+
// as otherwise it could be used as a vector to bypass the helm repository's authentication.
347+
func (r *ChartRepository) CacheIndexInMemory() error {
348+
// Cache the index if it was successfully retrieved
349+
// and the chart was successfully built
350+
if r.IndexCache != nil && r.Index != nil {
351+
err := r.IndexCache.Set(r.IndexKey, r.Index, r.IndexTTL)
352+
if err != nil {
353+
return err
354+
}
355+
}
356+
357+
return nil
358+
}
359+
360+
// StrategicallyLoadIndex lazy-loads the Index
361+
// first from Indexcache,
362+
// then from CachePath using oadFromCache if it does not HasIndex.
297363
// If not HasCacheFile, a cache attempt is made using CacheIndex
298364
// before continuing to load.
299365
func (r *ChartRepository) StrategicallyLoadIndex() (err error) {
300366
if r.HasIndex() {
301367
return
302368
}
369+
370+
if r.IndexCache != nil {
371+
if found := r.LoadFromMemCache(); found {
372+
return
373+
}
374+
}
375+
303376
if !r.HasCacheFile() {
304377
if _, err = r.CacheIndex(); err != nil {
305378
err = fmt.Errorf("failed to strategically load index: %w", err)
@@ -313,6 +386,28 @@ func (r *ChartRepository) StrategicallyLoadIndex() (err error) {
313386
return
314387
}
315388

389+
// LoadFromMemCache attempts to load the Index from the provided cache.
390+
// It returns true if the Index was found in the cache, and false otherwise.
391+
func (r *ChartRepository) LoadFromMemCache() bool {
392+
if index, found := r.IndexCache.Get(r.IndexKey); found {
393+
r.Lock()
394+
r.Index = index.(*repo.IndexFile)
395+
r.Unlock()
396+
397+
// record the cache hit
398+
if r.RecordIndexCacheMetric != nil {
399+
r.RecordIndexCacheMetric(cache.CacheEventTypeHit)
400+
}
401+
return true
402+
}
403+
404+
// record the cache miss
405+
if r.RecordIndexCacheMetric != nil {
406+
r.RecordIndexCacheMetric(cache.CacheEventTypeMiss)
407+
}
408+
return false
409+
}
410+
316411
// LoadFromCache attempts to load the Index from the configured CachePath.
317412
// It returns an error if no CachePath is set, or if the load failed.
318413
func (r *ChartRepository) LoadFromCache() error {
@@ -375,6 +470,14 @@ func (r *ChartRepository) Unload() {
375470
r.Index = nil
376471
}
377472

473+
// SetMemCache sets the cache to use for this repository.
474+
func (r *ChartRepository) SetMemCache(key string, c *cache.Cache, ttl time.Duration, rec RecordMetricsFunc) {
475+
r.IndexKey = key
476+
r.IndexCache = c
477+
r.IndexTTL = ttl
478+
r.RecordIndexCacheMetric = rec
479+
}
480+
378481
// RemoveCache removes the CachePath if Cached.
379482
func (r *ChartRepository) RemoveCache() error {
380483
if r == nil {

0 commit comments

Comments
 (0)