From 4ad501c85a17611001a476b67b6d4771e227d0d3 Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Tue, 7 Feb 2023 00:41:32 +0000 Subject: [PATCH] perf: download charts in parallel Signed-off-by: Felipe Santos --- pkg/downloader/manager.go | 111 ++++++++++++++++++++++++-------------- 1 file changed, 70 insertions(+), 41 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 18b28dde1..05509b196 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -270,7 +270,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { fmt.Fprintf(m.Out, "Saving %d charts\n", len(deps)) var saveError error - churls := make(map[string]struct{}) + var depsToDownload []*chart.Dependency for _, dep := range deps { // No repository means the chart is in charts directory if dep.Repository == "" { @@ -310,57 +310,71 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { dep.Version = ver continue } + // Add dep to download list + depsToDownload = append(depsToDownload, dep) + } - // Any failure to resolve/download a chart should fail: - // https://github.com/helm/helm/issues/1439 - churl, username, password, insecureskiptlsverify, passcredentialsall, caFile, certFile, keyFile, err := m.findChartURL(dep.Name, dep.Version, dep.Repository, repos) - if err != nil { - saveError = errors.Wrapf(err, "could not find %s", churl) - break - } + // De-duplicate depsToDownload + deduplicatedDepsToDownload := deduplicateDeps(depsToDownload) - if _, ok := churls[churl]; ok { - fmt.Fprintf(m.Out, "Already downloaded %s from repo %s\n", dep.Name, dep.Repository) - continue - } + var wg sync.WaitGroup - fmt.Fprintf(m.Out, "Downloading %s from repo %s\n", dep.Name, dep.Repository) + sem := make(chan struct{}, 4) - dl := ChartDownloader{ - Out: m.Out, - Verify: m.Verify, - Keyring: m.Keyring, - RepositoryConfig: m.RepositoryConfig, - RepositoryCache: m.RepositoryCache, - RegistryClient: m.RegistryClient, - Getters: m.Getters, - Options: []getter.Option{ - getter.WithBasicAuth(username, password), - getter.WithPassCredentialsAll(passcredentialsall), - getter.WithInsecureSkipVerifyTLS(insecureskiptlsverify), - getter.WithTLSClientConfig(certFile, keyFile, caFile), - }, - } + for _, dep := range deduplicatedDepsToDownload { + wg.Add(1) + sem <- struct{}{} - version := "" - if registry.IsOCI(churl) { - churl, version, err = parseOCIRef(churl) + go func(dep *chart.Dependency) { + defer wg.Done() + + fmt.Fprintf(m.Out, "Downloading %s from repo %s\n", dep.Name, dep.Repository) + + churl, username, password, insecureskiptlsverify, passcredentialsall, caFile, certFile, keyFile, err := m.findChartURL(dep.Name, dep.Version, dep.Repository, repos) if err != nil { - return errors.Wrapf(err, "could not parse OCI reference") + saveError = errors.Wrapf(err, "could not find %s", churl) + return } - dl.Options = append(dl.Options, - getter.WithRegistryClient(m.RegistryClient), - getter.WithTagName(version)) - } - if _, _, err = dl.DownloadTo(churl, version, tmpPath); err != nil { - saveError = errors.Wrapf(err, "could not download %s", churl) - break - } + dl := ChartDownloader{ + Out: m.Out, + Verify: m.Verify, + Keyring: m.Keyring, + RepositoryConfig: m.RepositoryConfig, + RepositoryCache: m.RepositoryCache, + RegistryClient: m.RegistryClient, + Getters: m.Getters, + Options: []getter.Option{ + getter.WithBasicAuth(username, password), + getter.WithPassCredentialsAll(passcredentialsall), + getter.WithInsecureSkipVerifyTLS(insecureskiptlsverify), + getter.WithTLSClientConfig(certFile, keyFile, caFile), + }, + } - churls[churl] = struct{}{} + version := "" + if registry.IsOCI(churl) { + churl, version, err = parseOCIRef(churl) + if err != nil { + saveError = errors.Wrapf(err, "could not parse OCI reference") + return + } + dl.Options = append(dl.Options, + getter.WithRegistryClient(m.RegistryClient), + getter.WithTagName(version)) + } + + if _, _, err = dl.DownloadTo(churl, version, tmpPath); err != nil { + saveError = errors.Wrapf(err, "could not download %s", churl) + return + } + + <-sem + }(dep) } + wg.Wait() + // TODO: this should probably be refactored to be a []error, so we can capture and provide more information rather than "last error wins". if saveError == nil { // now we can move all downloaded charts to destPath and delete outdated dependencies @@ -374,6 +388,21 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { return nil } +func deduplicateDeps(deps []*chart.Dependency) []*chart.Dependency { + depsMap := make(map[string]bool) + var depsResult []*chart.Dependency + + for _, dep := range deps { + depKey := fmt.Sprintf("%s/%s/%s", dep.Repository, dep.Name, dep.Version) + if _, ok := depsMap[depKey]; !ok { + depsMap[depKey] = true + depsResult = append(deps, dep) + } + } + + return depsResult +} + func parseOCIRef(chartRef string) (string, string, error) { refTagRegexp := regexp.MustCompile(`^(oci://[^:]+(:[0-9]{1,5})?[^:]+):(.*)$`) caps := refTagRegexp.FindStringSubmatch(chartRef)