From d60131d16b279e398257df27112b9a6b351fcc01 Mon Sep 17 00:00:00 2001
From: Hidde Beydals <hello@hidde.co>
Date: Mon, 1 Nov 2021 09:20:48 +0100
Subject: [PATCH] internal/helm: optimize dependency manager

This commit starts with the optimization of the `DepenendencyManager`,
ensuring the chart indexes are lazy loaded, and replacing the
(limitless) concurrency with a configurable number of workers with a
default of 1.

Signed-off-by: Hidde Beydals <hello@hidde.co>
---
 internal/helm/dependency_manager.go      | 70 ++++++++++++++++--------
 internal/helm/dependency_manager_test.go | 11 ++--
 2 files changed, 52 insertions(+), 29 deletions(-)

diff --git a/internal/helm/dependency_manager.go b/internal/helm/dependency_manager.go
index 83b42d4d..19d56c88 100644
--- a/internal/helm/dependency_manager.go
+++ b/internal/helm/dependency_manager.go
@@ -28,6 +28,7 @@ import (
 	"github.com/Masterminds/semver/v3"
 	securejoin "github.com/cyphar/filepath-securejoin"
 	"golang.org/x/sync/errgroup"
+	"golang.org/x/sync/semaphore"
 	helmchart "helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chart/loader"
 )
@@ -58,38 +59,51 @@ type DependencyManager struct {
 	// Dependencies contains a list of dependencies, and the respective
 	// repository the dependency can be found at.
 	Dependencies []*DependencyWithRepository
+	// Workers is the number of concurrent chart-add operations during
+	// Build. Defaults to 1 (non-concurrent).
+	Workers int64
 
 	mu sync.Mutex
 }
 
-// Build compiles and builds the dependencies of the Chart.
+// Build compiles and builds the dependencies of the Chart with the
+// configured number of Workers.
 func (dm *DependencyManager) Build(ctx context.Context) error {
 	if len(dm.Dependencies) == 0 {
 		return nil
 	}
 
-	errs, ctx := errgroup.WithContext(ctx)
-	for _, i := range dm.Dependencies {
-		item := i
-		errs.Go(func() error {
-			select {
-			case <-ctx.Done():
-				return ctx.Err()
-			default:
-			}
-
-			var err error
-			switch item.Repository {
-			case nil:
-				err = dm.addLocalDependency(item)
-			default:
-				err = dm.addRemoteDependency(item)
-			}
-			return err
-		})
+	workers := dm.Workers
+	if workers <= 0 {
+		workers = 1
 	}
 
-	return errs.Wait()
+	defer func() {
+		for _, dep := range dm.Dependencies {
+			dep.Repository.UnloadIndex()
+		}
+	}()
+
+	group, groupCtx := errgroup.WithContext(ctx)
+	group.Go(func() error {
+		sem := semaphore.NewWeighted(workers)
+		for _, dep := range dm.Dependencies {
+			dep := dep
+			if err := sem.Acquire(groupCtx, 1); err != nil {
+				return err
+			}
+			group.Go(func() error {
+				defer sem.Release(1)
+				if dep.Repository == nil {
+					return dm.addLocalDependency(dep)
+				}
+				return dm.addRemoteDependency(dep)
+			})
+		}
+		return nil
+	})
+
+	return group.Wait()
 }
 
 func (dm *DependencyManager) addLocalDependency(dpr *DependencyWithRepository) error {
@@ -136,7 +150,18 @@ func (dm *DependencyManager) addLocalDependency(dpr *DependencyWithRepository) e
 
 func (dm *DependencyManager) addRemoteDependency(dpr *DependencyWithRepository) error {
 	if dpr.Repository == nil {
-		return fmt.Errorf("no ChartRepository given for '%s' dependency", dpr.Dependency.Name)
+		return fmt.Errorf("no HelmRepository for '%s' dependency", dpr.Dependency.Name)
+	}
+
+	if !dpr.Repository.HasIndex() {
+		if !dpr.Repository.HasCacheFile() {
+			if _, err := dpr.Repository.CacheIndex(); err != nil {
+				return err
+			}
+		}
+		if err := dpr.Repository.LoadFromCache(); err != nil {
+			return err
+		}
 	}
 
 	chartVer, err := dpr.Repository.Get(dpr.Dependency.Name, dpr.Dependency.Version)
@@ -157,7 +182,6 @@ func (dm *DependencyManager) addRemoteDependency(dpr *DependencyWithRepository)
 	dm.mu.Lock()
 	dm.Chart.AddDependency(ch)
 	dm.mu.Unlock()
-
 	return nil
 }
 
diff --git a/internal/helm/dependency_manager_test.go b/internal/helm/dependency_manager_test.go
index 6a38997b..a8e6a048 100644
--- a/internal/helm/dependency_manager_test.go
+++ b/internal/helm/dependency_manager_test.go
@@ -182,13 +182,12 @@ func TestBuild_WithRemoteChart(t *testing.T) {
 		t.Fatal(err)
 	}
 	i := repo.NewIndexFile()
-	i.Add(&helmchart.Metadata{Name: chartName, Version: chartVersion}, fmt.Sprintf("%s-%s.tgz", chartName, chartVersion), "http://example.com/charts", "sha256:1234567890")
+	i.MustAdd(&helmchart.Metadata{Name: chartName, Version: chartVersion}, fmt.Sprintf("%s-%s.tgz", chartName, chartVersion), "http://example.com/charts", "sha256:1234567890")
 	mg := mockGetter{response: b}
-	cr := &ChartRepository{
-		URL:    remoteDepFixture.Repository,
-		Index:  i,
-		Client: &mg,
-	}
+	cr := newChartRepository()
+	cr.URL = remoteDepFixture.Repository
+	cr.Index = i
+	cr.Client = &mg
 	dm := DependencyManager{
 		Chart: &chart,
 		Dependencies: []*DependencyWithRepository{