msb-public
/
helm
mirror of https://github.com/helm/helm
1
0
Code Issues Projects Releases Wiki Activity

Revert "Improve helm dependency update performance"

Browse Source
revert-11726-fixDepUpPerformance
Matt Farina 2 months ago committed by GitHub
parent 7d1df7654c
commit 3b4180e660
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 26 additions and 60 deletions
Show Stats Download Patch File Download Diff File

44
internal/resolver/resolver.go
Unescape View File

@ -52,23 +52,21 @@ func New(chartpath, cachepath string, registryClient *registry.Client) *Resolver
}
// Resolve resolves dependencies and returns a lock file with the resolution.
func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string) (*chart.Lock, map[string]string, error) {
func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string) (*chart.Lock, error) {
// Now we clone the dependencies, locking as we go.
locked := make([]*chart.Dependency, len(reqs))
missing := []string{}
loadedIndexFiles := make(map[string]*repo.IndexFile)
urls := make(map[string]string)
for i, d := range reqs {
constraint, err := semver.NewConstraint(d.Version)
if err != nil {
return nil, nil, errors.Wrapf(err, "dependency %q has an invalid version/constraint format", d.Name)
return nil, errors.Wrapf(err, "dependency %q has an invalid version/constraint format", d.Name)
}
if d.Repository == "" {
// Local chart subfolder
if _, err := GetLocalPath(filepath.Join("charts", d.Name), r.chartpath); err != nil {
return nil, nil, err
return nil, err
}
locked[i] = &chart.Dependency{
@ -81,12 +79,12 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
if strings.HasPrefix(d.Repository, "file://") {
chartpath, err := GetLocalPath(d.Repository, r.chartpath)
if err != nil {
return nil, nil, err
return nil, err
}
ch, err := loader.LoadDir(chartpath)
if err != nil {
return nil, nil, err
return nil, err
}
v, err := semver.NewVersion(ch.Metadata.Version)
@ -124,26 +122,14 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
var ok bool
found := true
if !registry.IsOCI(d.Repository) {
filepath := filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName))
var repoIndex *repo.IndexFile
// 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, nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName)
}
} else {
repoIndex = indexFile
repoIndex, err := repo.LoadIndexFile(filepath.Join(r.cachepath, helmpath.CacheIndexFile(repoName)))
if err != nil {
return nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName)
}
vs, ok = repoIndex.Entries[d.Name]
if !ok {
return nil, nil, errors.Errorf("%s chart not found in repo %s", d.Name, d.Repository)
return nil, errors.Errorf("%s chart not found in repo %s", d.Name, d.Repository)
}
found = false
} else {
@ -165,7 +151,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
ref := fmt.Sprintf("%s/%s", strings.TrimPrefix(d.Repository, fmt.Sprintf("%s://", registry.OCIScheme)), d.Name)
tags, err := r.registryClient.Tags(ref)
if err != nil {
return nil, nil, errors.Wrapf(err, "could not retrieve list of tags for repository %s", d.Repository)
return nil, errors.Wrapf(err, "could not retrieve list of tags for repository %s", d.Repository)
}
vs = make(repo.ChartVersions, len(tags))
@ -186,7 +172,6 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
Repository: d.Repository,
Version: version,
}
// The version are already sorted and hence the first one to satisfy the constraint is used
for _, ver := range vs {
v, err := semver.NewVersion(ver.Version)
@ -197,9 +182,6 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
}
if constraint.Check(v) {
found = true
if len(ver.URLs) > 0 {
urls[d.Repository+ver.Name+ver.Version] = ver.URLs[0]
}
locked[i].Version = v.Original()
break
}
@ -210,19 +192,19 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
}
}
if len(missing) > 0 {
return nil, nil, errors.Errorf("can't get a valid version for %d subchart(s): %s. Make sure a matching chart version exists in the repo, or change the version constraint in Chart.yaml", len(missing), strings.Join(missing, ", "))
return nil, errors.Errorf("can't get a valid version for %d subchart(s): %s. Make sure a matching chart version exists in the repo, or change the version constraint in Chart.yaml", len(missing), strings.Join(missing, ", "))
}
digest, err := HashReq(reqs, locked)
if err != nil {
return nil, nil, err
return nil, err
}
return &chart.Lock{
Generated: time.Now(),
Digest: digest,
Dependencies: locked,
}, urls, nil
}, nil
}
// HashReq generates a hash of the dependencies.

2
internal/resolver/resolver_test.go
Unescape View File

@ -144,7 +144,7 @@ func TestResolve(t *testing.T) {
r := New("testdata/chartpath", "testdata/repository", registryClient)
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
l, _, err := r.Resolve(tt.req, repoNames)
l, err := r.Resolve(tt.req, repoNames)
if err != nil {
if tt.err {
return

32
pkg/downloader/manager.go
Unescape View File

@ -141,7 +141,7 @@ func (m *Manager) Build() error {
}
// Now we need to fetch every package here into charts/
return m.downloadAll(lock.Dependencies, nil)
return m.downloadAll(lock.Dependencies)
}
// Update updates a local charts directory.
@ -191,13 +191,13 @@ func (m *Manager) Update() error {
// Now we need to find out which version of a chart best satisfies the
// dependencies in the Chart.yaml
lock, urls, err := m.resolve(req, repoNames)
lock, err := m.resolve(req, repoNames)
if err != nil {
return err
}
// Now we need to fetch every package here into charts/
if err := m.downloadAll(lock.Dependencies, urls); err != nil {
if err := m.downloadAll(lock.Dependencies); err != nil {
return err
}
@ -230,7 +230,7 @@ func (m *Manager) loadChartDir() (*chart.Chart, error) {
// resolve takes a list of dependencies and translates them into an exact version to download.
//
// This returns a lock file, which has all of the dependencies normalized to a specific version.
func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string) (*chart.Lock, map[string]string, error) {
func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string) (*chart.Lock, error) {
res := resolver.New(m.ChartPath, m.RepositoryCache, m.RegistryClient)
return res.Resolve(req, repoNames)
}
@ -239,7 +239,7 @@ func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string)
//
// It will delete versions of the chart that exist on disk and might cause
// a conflict.
func (m *Manager) downloadAll(deps []*chart.Dependency, urls map[string]string) error {
func (m *Manager) downloadAll(deps []*chart.Dependency) error {
repos, err := m.loadChartRepositories()
if err != nil {
return err
@ -312,7 +312,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency, urls map[string]string)
// 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, urls)
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
@ -501,7 +501,6 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart.
var ru []*repo.Entry
Outer:
for _, dd := range deps {
// If the chart is in the local charts directory no repository needs
@ -529,14 +528,6 @@ Outer:
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
// access controls the repository needs to be added through the user
// managed system. This path will work for public charts, like those
@ -712,7 +703,7 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error {
// repoURL is the repository to search
//
// 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, urls map[string]string) (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) (url, username, password string, insecureskiptlsverify, passcredentialsall bool, caFile, certFile, keyFile string, err error) {
if registry.IsOCI(repoURL) {
return fmt.Sprintf("%s/%s:%s", repoURL, name, version), "", "", false, false, "", "", "", nil
}
@ -751,14 +742,7 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*
return
}
}
urlsKey := repoURL + name + version
if _, ok := urls[urlsKey]; ok {
url = urls[urlsKey]
} else {
url, err = repo.FindChartInRepoURL(repoURL, name, version, certFile, keyFile, caFile, m.Getters)
}
url, err = repo.FindChartInRepoURL(repoURL, name, version, certFile, keyFile, caFile, m.Getters)
if err == nil {
return url, username, password, false, false, "", "", "", err
}

8
pkg/downloader/manager_test.go
Unescape View File

@ -84,7 +84,7 @@ func TestFindChartURL(t *testing.T) {
version := "0.1.0"
repoURL := "http://example.com/charts"
churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(name, version, repoURL, repos, make(map[string]string))
churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(name, version, repoURL, repos)
if err != nil {
t.Fatal(err)
}
@ -109,7 +109,7 @@ func TestFindChartURL(t *testing.T) {
version = "1.2.3"
repoURL = "https://example-https-insecureskiptlsverify.com"
churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos, make(map[string]string))
churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos)
if err != nil {
t.Fatal(err)
}
@ -255,7 +255,7 @@ func TestDownloadAll(t *testing.T) {
if err := os.MkdirAll(filepath.Join(chartPath, "tmpcharts"), 0755); err != nil {
t.Fatal(err)
}
if err := m.downloadAll([]*chart.Dependency{signDep, localDep}, make(map[string]string)); err != nil {
if err := m.downloadAll([]*chart.Dependency{signDep, localDep}); err != nil {
t.Error(err)
}
@ -284,7 +284,7 @@ version: 0.1.0`
Version: "0.1.0",
}
err = m.downloadAll([]*chart.Dependency{badLocalDep}, make(map[string]string))
err = m.downloadAll([]*chart.Dependency{badLocalDep})
if err == nil {
t.Fatal("Expected error for bad dependency name")
}

Write Preview
Loading…
Cancel
Save