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 <jeff.van.dam@est.tech>
pull/11726/head
JvD_Ericsson 3 years ago committed by Jeff van Dam
parent b2d5235c1c
commit 8c80f58196

@ -57,6 +57,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
// Now we clone the dependencies, locking as we go. // Now we clone the dependencies, locking as we go.
locked := make([]*chart.Dependency, len(reqs)) locked := make([]*chart.Dependency, len(reqs))
missing := []string{} missing := []string{}
loadedIndexFiles := make(map[string]*repo.IndexFile)
for i, d := range reqs { for i, d := range reqs {
constraint, err := semver.NewConstraint(d.Version) constraint, err := semver.NewConstraint(d.Version)
if err != nil { if err != nil {
@ -123,9 +124,21 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
var ok bool var ok bool
found := true found := true
if !registry.IsOCI(d.Repository) { if !registry.IsOCI(d.Repository) {
repoIndex, err := repo.LoadIndexFile(filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName))) filepath := filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName))
if err != nil { var repoIndex *repo.IndexFile
return nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName)
// Store previously loaded index files in a map. If repositories share the
// same index file there is no need to reload the same file again. This
// improves performance.
if indexFile, loaded := loadedIndexFiles[filepath]; !loaded {
var err error
repoIndex, err = repo.LoadIndexFile(filepath)
loadedIndexFiles[filepath] = repoIndex
if err != nil {
return nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName)
}
} else {
repoIndex = indexFile
} }
vs, ok = repoIndex.Entries[d.Name] vs, ok = repoIndex.Entries[d.Name]

@ -27,7 +27,6 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"helm.sh/helm/v3/internal/fileutil" "helm.sh/helm/v3/internal/fileutil"
"helm.sh/helm/v3/internal/urlutil"
"helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/getter"
"helm.sh/helm/v3/pkg/helmpath" "helm.sh/helm/v3/pkg/helmpath"
"helm.sh/helm/v3/pkg/provenance" "helm.sh/helm/v3/pkg/provenance"
@ -184,11 +183,11 @@ func (c *ChartDownloader) getOciURI(ref, version string, u *url.URL) (*url.URL,
// //
// A version is a SemVer string (1.2.3-beta.1+f334a6789). // A version is a SemVer string (1.2.3-beta.1+f334a6789).
// //
// - For fully qualified URLs, the version will be ignored (since URLs aren't versioned) // - For fully qualified URLs, the version will be ignored (since URLs aren't versioned)
// - For a chart reference // - For a chart reference
// * If version is non-empty, this will return the URL for that version // - If version is non-empty, this will return the URL for that version
// * If version is empty, this will return the URL for the latest version // - If version is empty, this will return the URL for the latest version
// * If no version can be found, an error is returned // - If no version can be found, an error is returned
func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, error) { func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, error) {
u, err := url.Parse(ref) u, err := url.Parse(ref)
if err != nil { if err != nil {
@ -378,19 +377,13 @@ func (c *ChartDownloader) scanReposForURL(u string, rf *repo.File) (*repo.Entry,
} }
idxFile := filepath.Join(c.RepositoryCache, helmpath.CacheIndexFile(r.Config.Name)) idxFile := filepath.Join(c.RepositoryCache, helmpath.CacheIndexFile(r.Config.Name))
i, err := repo.LoadIndexFile(idxFile) yamlFile, err := os.ReadFile(idxFile)
if err != nil { if err != nil {
return nil, errors.Wrap(err, "no cached repo found. (try 'helm repo update')") return nil, errors.Wrap(err, "no cached repo found. (try 'helm repo update')")
} }
file := string(yamlFile[:])
for _, entry := range i.Entries { if strings.Contains(file, u) {
for _, ver := range entry { return rc, nil
for _, dl := range ver.URLs {
if urlutil.Equal(u, dl) {
return rc, nil
}
}
}
} }
} }
// This means that there is no repo file for the given URL. // This means that there is no repo file for the given URL.

@ -271,6 +271,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
fmt.Fprintf(m.Out, "Saving %d charts\n", len(deps)) fmt.Fprintf(m.Out, "Saving %d charts\n", len(deps))
var saveError error var saveError error
churls := make(map[string]struct{}) churls := make(map[string]struct{})
baseChartUrls := make(map[string]string)
for _, dep := range deps { for _, dep := range deps {
// No repository means the chart is in charts directory // No repository means the chart is in charts directory
if dep.Repository == "" { if dep.Repository == "" {
@ -313,7 +314,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
// Any failure to resolve/download a chart should fail: // Any failure to resolve/download a chart should fail:
// https://github.com/helm/helm/issues/1439 // 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) churl, username, password, insecureskiptlsverify, passcredentialsall, caFile, certFile, keyFile, err := m.findChartURL(dep.Name, dep.Version, dep.Repository, repos, baseChartUrls)
if err != nil { if err != nil {
saveError = errors.Wrapf(err, "could not find %s", churl) saveError = errors.Wrapf(err, "could not find %s", churl)
break break
@ -502,6 +503,7 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart.
var ru []*repo.Entry var ru []*repo.Entry
Outer:
for _, dd := range deps { for _, dd := range deps {
// If the chart is in the local charts directory no repository needs // 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.
repoNames[dd.Name] = rn repoNames[dd.Name] = rn
// If repository is already present don't add to array. This will skip
// unnecessary index file downloading improving performance.
for _, item := range ru {
if item.URL == dd.Repository {
continue Outer
}
}
// Assuming the repository is generally available. For Helm managed // Assuming the repository is generally available. For Helm managed
// access controls the repository needs to be added through the user // access controls the repository needs to be added through the user
// managed system. This path will work for public charts, like those // managed system. This path will work for public charts, like those
@ -703,7 +713,7 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error {
// repoURL is the repository to search // repoURL is the repository to search
// //
// If it finds a URL that is "relative", it will prepend the repoURL. // If it finds a URL that is "relative", it will prepend the repoURL.
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) { 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) {
if registry.IsOCI(repoURL) { if registry.IsOCI(repoURL) {
return fmt.Sprintf("%s/%s:%s", repoURL, name, version), "", "", false, false, "", "", "", nil return fmt.Sprintf("%s/%s:%s", repoURL, name, version), "", "", false, false, "", "", "", nil
} }
@ -735,7 +745,19 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*
return return
} }
} }
url, err = repo.FindChartInRepoURL(repoURL, name, version, certFile, keyFile, caFile, m.Getters)
// Store previously found chart URLs in a map. If repositories share the
// same repo URL there is no need to find the same repo URL again. This
// improves performance.
if baseURL, ok := baseChartUrls[repoURL]; !ok {
url, err = repo.FindChartInRepoURL(repoURL, name, version, certFile, keyFile, caFile, m.Getters)
if err == nil {
slice := strings.SplitAfter(url, "/")
baseChartUrls[repoURL] = strings.Join(slice[:len(slice)-1], "")
}
} else {
url = fmt.Sprintf("%s%s-%s.tgz", baseURL, name, version)
}
if err == nil { if err == nil {
return url, username, password, false, false, "", "", "", err return url, username, password, false, false, "", "", "", err
} }

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

Loading…
Cancel
Save