Skip to content

Commit 8c80f58

Browse files
JvD-EricssonJvD-Ericsson
committed
Improve helm dependency update performance
What this PR does / why we need it: This PR was created to improve performance of the dependency update command by skipping unnecessary downloading and loading of index files that have already been downloaded and loaded I believe this would close refs #9865 Signed-off-by: Jeff van Dam <[email protected]>
1 parent b2d5235 commit 8c80f58

File tree

4 files changed

+52
-24
lines changed

4 files changed

+52
-24
lines changed

‎internal/resolver/resolver.go

+16-3
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
5757
// Now we clone the dependencies, locking as we go.
5858
locked := make([]*chart.Dependency, len(reqs))
5959
missing := []string{}
60+
loadedIndexFiles := make(map[string]*repo.IndexFile)
6061
for i, d := range reqs {
6162
constraint, err := semver.NewConstraint(d.Version)
6263
if err != nil {
@@ -123,9 +124,21 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
123124
var ok bool
124125
found := true
125126
if !registry.IsOCI(d.Repository) {
126-
repoIndex, err := repo.LoadIndexFile(filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName)))
127-
if err != nil {
128-
return nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName)
127+
filepath := filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName))
128+
var repoIndex *repo.IndexFile
129+
130+
// Store previously loaded index files in a map. If repositories share the
131+
// same index file there is no need to reload the same file again. This
132+
// improves performance.
133+
if indexFile, loaded := loadedIndexFiles[filepath]; !loaded {
134+
var err error
135+
repoIndex, err = repo.LoadIndexFile(filepath)
136+
loadedIndexFiles[filepath] = repoIndex
137+
if err != nil {
138+
return nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName)
139+
}
140+
} else {
141+
repoIndex = indexFile
129142
}
130143

131144
vs, ok = repoIndex.Entries[d.Name]

‎pkg/downloader/chart_downloader.go

+9-16
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727
"github.com/pkg/errors"
2828

2929
"helm.sh/helm/v3/internal/fileutil"
30-
"helm.sh/helm/v3/internal/urlutil"
3130
"helm.sh/helm/v3/pkg/getter"
3231
"helm.sh/helm/v3/pkg/helmpath"
3332
"helm.sh/helm/v3/pkg/provenance"
@@ -184,11 +183,11 @@ func (c *ChartDownloader) getOciURI(ref, version string, u *url.URL) (*url.URL,
184183
//
185184
// A version is a SemVer string (1.2.3-beta.1+f334a6789).
186185
//
187-
// - For fully qualified URLs, the version will be ignored (since URLs aren't versioned)
188-
// - For a chart reference
189-
// * If version is non-empty, this will return the URL for that version
190-
// * If version is empty, this will return the URL for the latest version
191-
// * If no version can be found, an error is returned
186+
// - For fully qualified URLs, the version will be ignored (since URLs aren't versioned)
187+
// - For a chart reference
188+
// - If version is non-empty, this will return the URL for that version
189+
// - If version is empty, this will return the URL for the latest version
190+
// - If no version can be found, an error is returned
192191
func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, error) {
193192
u, err := url.Parse(ref)
194193
if err != nil {
@@ -378,19 +377,13 @@ func (c *ChartDownloader) scanReposForURL(u string, rf *repo.File) (*repo.Entry,
378377
}
379378

380379
idxFile := filepath.Join(c.RepositoryCache, helmpath.CacheIndexFile(r.Config.Name))
381-
i, err := repo.LoadIndexFile(idxFile)
380+
yamlFile, err := os.ReadFile(idxFile)
382381
if err != nil {
383382
return nil, errors.Wrap(err, "no cached repo found. (try 'helm repo update')")
384383
}
385-
386-
for _, entry := range i.Entries {
387-
for _, ver := range entry {
388-
for _, dl := range ver.URLs {
389-
if urlutil.Equal(u, dl) {
390-
return rc, nil
391-
}
392-
}
393-
}
384+
file := string(yamlFile[:])
385+
if strings.Contains(file, u) {
386+
return rc, nil
394387
}
395388
}
396389
// This means that there is no repo file for the given URL.

‎pkg/downloader/manager.go

+25-3
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
271271
fmt.Fprintf(m.Out, "Saving %d charts\n", len(deps))
272272
var saveError error
273273
churls := make(map[string]struct{})
274+
baseChartUrls := make(map[string]string)
274275
for _, dep := range deps {
275276
// No repository means the chart is in charts directory
276277
if dep.Repository == "" {
@@ -313,7 +314,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
313314

314315
// Any failure to resolve/download a chart should fail:
315316
// https://github.com/helm/helm/issues/1439
316-
churl, username, password, insecureskiptlsverify, passcredentialsall, caFile, certFile, keyFile, err := m.findChartURL(dep.Name, dep.Version, dep.Repository, repos)
317+
churl, username, password, insecureskiptlsverify, passcredentialsall, caFile, certFile, keyFile, err := m.findChartURL(dep.Name, dep.Version, dep.Repository, repos, baseChartUrls)
317318
if err != nil {
318319
saveError = errors.Wrapf(err, "could not find %s", churl)
319320
break
@@ -502,6 +503,7 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart.
502503

503504
var ru []*repo.Entry
504505

506+
Outer:
505507
for _, dd := range deps {
506508

507509
// If the chart is in the local charts directory no repository needs
@@ -529,6 +531,14 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart.
529531

530532
repoNames[dd.Name] = rn
531533

534+
// If repository is already present don't add to array. This will skip
535+
// unnecessary index file downloading improving performance.
536+
for _, item := range ru {
537+
if item.URL == dd.Repository {
538+
continue Outer
539+
}
540+
}
541+
532542
// Assuming the repository is generally available. For Helm managed
533543
// access controls the repository needs to be added through the user
534544
// managed system. This path will work for public charts, like those
@@ -703,7 +713,7 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error {
703713
// repoURL is the repository to search
704714
//
705715
// If it finds a URL that is "relative", it will prepend the repoURL.
706-
func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*repo.ChartRepository) (url, username, password string, insecureskiptlsverify, passcredentialsall bool, caFile, certFile, keyFile string, err error) {
716+
func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*repo.ChartRepository, baseChartUrls map[string]string) (url, username, password string, insecureskiptlsverify, passcredentialsall bool, caFile, certFile, keyFile string, err error) {
707717
if registry.IsOCI(repoURL) {
708718
return fmt.Sprintf("%s/%s:%s", repoURL, name, version), "", "", false, false, "", "", "", nil
709719
}
@@ -735,7 +745,19 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*
735745
return
736746
}
737747
}
738-
url, err = repo.FindChartInRepoURL(repoURL, name, version, certFile, keyFile, caFile, m.Getters)
748+
749+
// Store previously found chart URLs in a map. If repositories share the
750+
// same repo URL there is no need to find the same repo URL again. This
751+
// improves performance.
752+
if baseURL, ok := baseChartUrls[repoURL]; !ok {
753+
url, err = repo.FindChartInRepoURL(repoURL, name, version, certFile, keyFile, caFile, m.Getters)
754+
if err == nil {
755+
slice := strings.SplitAfter(url, "/")
756+
baseChartUrls[repoURL] = strings.Join(slice[:len(slice)-1], "")
757+
}
758+
} else {
759+
url = fmt.Sprintf("%s%s-%s.tgz", baseURL, name, version)
760+
}
739761
if err == nil {
740762
return url, username, password, false, false, "", "", "", err
741763
}

‎pkg/downloader/manager_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func TestFindChartURL(t *testing.T) {
8484
version := "0.1.0"
8585
repoURL := "http://example.com/charts"
8686

87-
churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(name, version, repoURL, repos)
87+
churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(name, version, repoURL, repos, make(map[string]string))
8888
if err != nil {
8989
t.Fatal(err)
9090
}
@@ -109,7 +109,7 @@ func TestFindChartURL(t *testing.T) {
109109
version = "1.2.3"
110110
repoURL = "https://example-https-insecureskiptlsverify.com"
111111

112-
churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos)
112+
churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos, make(map[string]string))
113113
if err != nil {
114114
t.Fatal(err)
115115
}

0 commit comments

Comments
 (0)