Skip to content

Commit 3013b5f

Browse files
author
Paulo Gomes
authored
Merge pull request #698 from cwyl02/cwyl02/cache-helmrepo-early
feat: cache helmrepo early after reconcil
2 parents a6072c3 + d5a75f6 commit 3013b5f

File tree

5 files changed

+143
-17
lines changed

5 files changed

+143
-17
lines changed

‎controllers/helmrepository_controller.go

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import (
4646
"github.com/fluxcd/pkg/runtime/predicates"
4747

4848
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
49+
"github.com/fluxcd/source-controller/internal/cache"
4950
serror "github.com/fluxcd/source-controller/internal/error"
5051
"github.com/fluxcd/source-controller/internal/helm/getter"
5152
"github.com/fluxcd/source-controller/internal/helm/repository"
@@ -105,6 +106,10 @@ type HelmRepositoryReconciler struct {
105106
Getters helmgetter.Providers
106107
Storage *Storage
107108
ControllerName string
109+
110+
Cache *cache.Cache
111+
TTL time.Duration
112+
*cache.CacheRecorder
108113
}
109114

110115
type HelmRepositoryReconcilerOptions struct {
@@ -451,7 +456,6 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
451456
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
452457
return sreconcile.ResultEmpty, e
453458
}
454-
chartRepo.Unload()
455459

456460
// Mark observations about the revision on the object.
457461
if !obj.GetArtifact().HasRevision(chartRepo.Checksum) {
@@ -492,6 +496,8 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *s
492496
"stored artifact for revision '%s'", artifact.Revision)
493497
}
494498

499+
chartRepo.Unload()
500+
495501
if err := chartRepo.RemoveCache(); err != nil {
496502
ctrl.LoggerFrom(ctx).Error(err, "failed to remove temporary cached index file")
497503
}
@@ -545,6 +551,26 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *s
545551
obj.Status.URL = indexURL
546552
}
547553
conditions.Delete(obj, sourcev1.StorageOperationFailedCondition)
554+
555+
// enable cache if applicable
556+
if r.Cache != nil && chartRepo.IndexCache == nil {
557+
chartRepo.SetMemCache(r.Storage.LocalPath(*artifact), r.Cache, r.TTL, func(event string) {
558+
r.IncCacheEvents(event, obj.GetName(), obj.GetNamespace())
559+
})
560+
}
561+
562+
// Cache the index if it was successfully retrieved
563+
// and the chart was successfully built
564+
if r.Cache != nil && chartRepo.Index != nil {
565+
// The cache key have to be safe in multi-tenancy environments,
566+
// as otherwise it could be used as a vector to bypass the helm repository's authentication.
567+
// Using r.Storage.LocalPath(*repo.GetArtifact() is safe as the path is in the format /<helm-repository-name>/<chart-name>/<filename>.
568+
err := chartRepo.CacheIndexInMemory()
569+
if err != nil {
570+
r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.CacheOperationFailedReason, "failed to cache index: %s", err)
571+
}
572+
}
573+
548574
return sreconcile.ResultSuccess, nil
549575
}
550576

‎controllers/helmrepository_controller_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1299,3 +1299,61 @@ func TestHelmRepositoryReconciler_ReconcileSpecUpdatePredicateFilter(t *testing.
12991299
return false
13001300
}, timeout).Should(BeTrue())
13011301
}
1302+
1303+
func TestHelmRepositoryReconciler_InMemoryCaching(t *testing.T) {
1304+
g := NewWithT(t)
1305+
testCache.Clear()
1306+
1307+
testServer, err := helmtestserver.NewTempHelmServer()
1308+
g.Expect(err).NotTo(HaveOccurred())
1309+
defer os.RemoveAll(testServer.Root())
1310+
1311+
g.Expect(testServer.PackageChartWithVersion("testdata/charts/helmchart", "0.1.0")).To(Succeed())
1312+
g.Expect(testServer.GenerateIndex()).To(Succeed())
1313+
1314+
testServer.Start()
1315+
defer testServer.Stop()
1316+
1317+
ns, err := testEnv.CreateNamespace(ctx, "helmrepository")
1318+
g.Expect(err).ToNot(HaveOccurred())
1319+
defer func() { g.Expect(testEnv.Delete(ctx, ns)).To(Succeed()) }()
1320+
1321+
helmRepo := &sourcev1.HelmRepository{
1322+
ObjectMeta: metav1.ObjectMeta{
1323+
GenerateName: "helmrepository-",
1324+
Namespace: ns.Name,
1325+
},
1326+
Spec: sourcev1.HelmRepositorySpec{
1327+
URL: testServer.URL(),
1328+
},
1329+
}
1330+
g.Expect(testEnv.CreateAndWait(ctx, helmRepo)).To(Succeed())
1331+
1332+
key := client.ObjectKey{Name: helmRepo.Name, Namespace: helmRepo.Namespace}
1333+
// Wait for finalizer to be set
1334+
g.Eventually(func() bool {
1335+
if err := testEnv.Get(ctx, key, helmRepo); err != nil {
1336+
return false
1337+
}
1338+
return len(helmRepo.Finalizers) > 0
1339+
}, timeout).Should(BeTrue())
1340+
1341+
// Wait for HelmRepository to be Ready
1342+
g.Eventually(func() bool {
1343+
if err := testEnv.Get(ctx, key, helmRepo); err != nil {
1344+
return false
1345+
}
1346+
if !conditions.IsReady(helmRepo) || helmRepo.Status.Artifact == nil {
1347+
return false
1348+
}
1349+
readyCondition := conditions.Get(helmRepo, meta.ReadyCondition)
1350+
return helmRepo.Generation == readyCondition.ObservedGeneration &&
1351+
helmRepo.Generation == helmRepo.Status.ObservedGeneration
1352+
}, timeout).Should(BeTrue())
1353+
1354+
err = testEnv.Get(ctx, key, helmRepo)
1355+
g.Expect(err).ToNot(HaveOccurred())
1356+
localPath := testStorage.LocalPath(*helmRepo.GetArtifact())
1357+
_, cacheHit := testCache.Get(localPath)
1358+
g.Expect(cacheHit).To(BeTrue())
1359+
}

‎controllers/suite_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,12 +229,18 @@ func TestMain(m *testing.M) {
229229
panic(fmt.Sprintf("Failed to start BucketReconciler: %v", err))
230230
}
231231

232+
testCache = cache.New(5, 1*time.Second)
233+
cacheRecorder := cache.MustMakeMetrics()
234+
232235
if err := (&HelmRepositoryReconciler{
233236
Client: testEnv,
234237
EventRecorder: record.NewFakeRecorder(32),
235238
Metrics: testMetricsH,
236239
Getters: testGetters,
237240
Storage: testStorage,
241+
Cache: testCache,
242+
TTL: 1 * time.Second,
243+
CacheRecorder: cacheRecorder,
238244
}).SetupWithManager(testEnv); err != nil {
239245
panic(fmt.Sprintf("Failed to start HelmRepositoryReconciler: %v", err))
240246
}
@@ -249,8 +255,6 @@ func TestMain(m *testing.M) {
249255
panic(fmt.Sprintf("Failed to start HelmRepositoryOCIReconciler: %v", err))
250256
}
251257

252-
testCache = cache.New(5, 1*time.Second)
253-
cacheRecorder := cache.MustMakeMetrics()
254258
if err := (&HelmChartReconciler{
255259
Client: testEnv,
256260
EventRecorder: record.NewFakeRecorder(32),

‎internal/helm/repository/chart_repository_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"testing"
2727
"time"
2828

29+
"github.com/fluxcd/source-controller/internal/cache"
2930
"github.com/fluxcd/source-controller/internal/helm"
3031
. "github.com/onsi/gomega"
3132
"helm.sh/helm/v3/pkg/chart"
@@ -450,6 +451,39 @@ func TestChartRepository_StrategicallyLoadIndex(t *testing.T) {
450451
g.Expect(r.RemoveCache()).To(Succeed())
451452
}
452453

454+
func TestChartRepository_CacheIndexInMemory(t *testing.T) {
455+
g := NewWithT(t)
456+
457+
interval, _ := time.ParseDuration("5s")
458+
memCache := cache.New(1, interval)
459+
indexPath := "/multi-tenent-safe/mock/index.yaml"
460+
r := newChartRepository()
461+
r.Index = repo.NewIndexFile()
462+
indexFile := *r.Index
463+
g.Expect(
464+
indexFile.MustAdd(
465+
&chart.Metadata{
466+
Name: "grafana",
467+
Version: "6.17.4",
468+
},
469+
"grafana-6.17.4.tgz",
470+
"http://example.com/charts",
471+
"sha256:1234567890abc",
472+
)).To(Succeed())
473+
indexFile.WriteFile(indexPath, 0o640)
474+
ttl, _ := time.ParseDuration("1m")
475+
r.SetMemCache(indexPath, memCache, ttl, func(event string) {
476+
fmt.Println(event)
477+
})
478+
r.CacheIndexInMemory()
479+
_, cacheHit := r.IndexCache.Get(indexPath)
480+
g.Expect(cacheHit).To(Equal(true))
481+
r.Unload()
482+
g.Expect(r.Index).To(BeNil())
483+
g.Expect(r.StrategicallyLoadIndex()).To(Succeed())
484+
g.Expect(r.Index.Entries["grafana"][0].Digest).To(Equal("sha256:1234567890abc"))
485+
}
486+
453487
func TestChartRepository_LoadFromCache(t *testing.T) {
454488
tests := []struct {
455489
name string

‎main.go

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -224,20 +224,6 @@ func main() {
224224
setupLog.Error(err, "unable to create controller", "controller", sourcev1.GitRepositoryKind)
225225
os.Exit(1)
226226
}
227-
if err = (&controllers.HelmRepositoryReconciler{
228-
Client: mgr.GetClient(),
229-
EventRecorder: eventRecorder,
230-
Metrics: metricsH,
231-
Storage: storage,
232-
Getters: getters,
233-
ControllerName: controllerName,
234-
}).SetupWithManagerAndOptions(mgr, controllers.HelmRepositoryReconcilerOptions{
235-
MaxConcurrentReconciles: concurrent,
236-
RateLimiter: helper.GetRateLimiter(rateLimiterOptions),
237-
}); err != nil {
238-
setupLog.Error(err, "unable to create controller", "controller", sourcev1.HelmRepositoryKind, "type", "default")
239-
os.Exit(1)
240-
}
241227

242228
if err = (&controllers.HelmRepositoryOCIReconciler{
243229
Client: mgr.GetClient(),
@@ -274,6 +260,24 @@ func main() {
274260

275261
cacheRecorder := cache.MustMakeMetrics()
276262

263+
if err = (&controllers.HelmRepositoryReconciler{
264+
Client: mgr.GetClient(),
265+
EventRecorder: eventRecorder,
266+
Metrics: metricsH,
267+
Storage: storage,
268+
Getters: getters,
269+
ControllerName: controllerName,
270+
Cache: c,
271+
TTL: ttl,
272+
CacheRecorder: cacheRecorder,
273+
}).SetupWithManagerAndOptions(mgr, controllers.HelmRepositoryReconcilerOptions{
274+
MaxConcurrentReconciles: concurrent,
275+
RateLimiter: helper.GetRateLimiter(rateLimiterOptions),
276+
}); err != nil {
277+
setupLog.Error(err, "unable to create controller", "controller", sourcev1.HelmRepositoryKind)
278+
os.Exit(1)
279+
}
280+
277281
if err = (&controllers.HelmChartReconciler{
278282
Client: mgr.GetClient(),
279283
RegistryClientGenerator: registry.ClientGenerator,

0 commit comments

Comments
 (0)