From f187d09987aa7cbc18e5661798add2427c6ced68 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 25 Sep 2025 00:42:05 +0000 Subject: [PATCH] Implement chart fetch deduplication mechanism Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/state/state.go | 97 +++++++++++++++++++++++++++++++++-------- pkg/state/state_test.go | 55 +++++++++++++++++++++++ 2 files changed, 133 insertions(+), 19 deletions(-) diff --git a/pkg/state/state.go b/pkg/state/state.go index af26d017..f17baa19 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1415,29 +1415,39 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre // For helm 2, we `helm fetch` with the version flags and call `helm template` // WITHOUT the version flags. } else { - chartPath, err = generateChartPath(chartName, dir, release, opts.OutputDirTemplate) - if err != nil { - results <- &chartPrepareResult{err: err} - return - } - - // only fetch chart if it is not already fetched - if _, err := os.Stat(chartPath); os.IsNotExist(err) { - var fetchFlags []string - fetchFlags = st.appendChartVersionFlags(fetchFlags, release) - fetchFlags = append(fetchFlags, "--untar", "--untardir", chartPath) - if err := helm.Fetch(chartName, fetchFlags...); err != nil { + // Check global chart cache first for non-OCI charts + cacheKey := st.getChartCacheKey(release) + if cachedPath, exists := st.checkChartCache(cacheKey); exists && st.fs.DirectoryExistsAt(cachedPath) { + st.logger.Debugf("Chart %s:%s already downloaded, using cached version at %s", chartName, release.Version, cachedPath) + chartPath = cachedPath + } else { + chartPath, err = generateChartPath(chartName, dir, release, opts.OutputDirTemplate) + if err != nil { results <- &chartPrepareResult{err: err} return } - } else { - st.logger.Infof("\"%s\" has not been downloaded because the output directory \"%s\" already exists", chartName, chartPath) - } - // Set chartPath to be the path containing Chart.yaml, if found - fullChartPath, err := findChartDirectory(chartPath) - if err == nil { - chartPath = filepath.Dir(fullChartPath) + // only fetch chart if it is not already fetched + if _, err := os.Stat(chartPath); os.IsNotExist(err) { + var fetchFlags []string + fetchFlags = st.appendChartVersionFlags(fetchFlags, release) + fetchFlags = append(fetchFlags, "--untar", "--untardir", chartPath) + if err := helm.Fetch(chartName, fetchFlags...); err != nil { + results <- &chartPrepareResult{err: err} + return + } + } else { + st.logger.Infof("\"%s\" has not been downloaded because the output directory \"%s\" already exists", chartName, chartPath) + } + + // Set chartPath to be the path containing Chart.yaml, if found + fullChartPath, err := findChartDirectory(chartPath) + if err == nil { + chartPath = filepath.Dir(fullChartPath) + } + + // Add to global chart cache + st.addToChartCache(cacheKey, chartPath) } } @@ -4040,9 +4050,47 @@ func (st *HelmState) Reverse() { } } +// Chart cache for both OCI and non-OCI charts to avoid duplicate downloads +type ChartCacheKey struct { + Chart string + Version string +} + +var downloadedCharts = make(map[ChartCacheKey]string) // key -> chart path +var downloadedChartsMutex sync.RWMutex + +// Legacy OCI-specific cache (kept for backward compatibility) var downloadedOCICharts = make(map[string]bool) var downloadedOCIMutex sync.RWMutex +// getChartCacheKey creates a cache key for a chart and version +func (st *HelmState) getChartCacheKey(release *ReleaseSpec) ChartCacheKey { + version := release.Version + if version == "" { + // Use empty string for latest version + version = "" + } + return ChartCacheKey{ + Chart: release.Chart, + Version: version, + } +} + +// checkChartCache checks if a chart is already downloaded and returns its path +func (st *HelmState) checkChartCache(key ChartCacheKey) (string, bool) { + downloadedChartsMutex.RLock() + defer downloadedChartsMutex.RUnlock() + path, exists := downloadedCharts[key] + return path, exists +} + +// addToChartCache adds a chart to the cache +func (st *HelmState) addToChartCache(key ChartCacheKey, path string) { + downloadedChartsMutex.Lock() + defer downloadedChartsMutex.Unlock() + downloadedCharts[key] = path +} + func (st *HelmState) getOCIChart(release *ReleaseSpec, tempDir string, helm helmexec.Interface, opts ChartPrepareOptions) (*string, error) { qualifiedChartName, chartName, chartVersion, err := st.getOCIQualifiedChartName(release, helm) if err != nil { @@ -4053,6 +4101,13 @@ func (st *HelmState) getOCIChart(release *ReleaseSpec, tempDir string, helm helm return nil, nil } + // Check global chart cache first + cacheKey := st.getChartCacheKey(release) + if cachedPath, exists := st.checkChartCache(cacheKey); exists && st.fs.DirectoryExistsAt(cachedPath) { + st.logger.Debugf("OCI chart %s:%s already downloaded, using cached version at %s", chartName, chartVersion, cachedPath) + return &cachedPath, nil + } + if opts.OutputDirTemplate == "" { tempDir = remote.CacheDir() } @@ -4085,6 +4140,7 @@ func (st *HelmState) getOCIChart(release *ReleaseSpec, tempDir string, helm helm if st.fs.DirectoryExistsAt(chartPath) { st.logger.Debugf("chart already exists at %s", chartPath) } else { + st.logger.Infof("Fetching %s", chartName) flags := st.chartOCIFlags(release) flags = st.appendVerifyFlags(flags, release) flags = st.appendKeyringFlags(flags, release) @@ -4113,6 +4169,9 @@ func (st *HelmState) getOCIChart(release *ReleaseSpec, tempDir string, helm helm chartPath = filepath.Dir(fullChartPath) + // Add to global chart cache + st.addToChartCache(cacheKey, chartPath) + return &chartPath, nil } diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 57774b23..bb47605b 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -4638,3 +4638,58 @@ func TestPrepareSyncReleases_ValueControlReleaseOverride(t *testing.T) { require.Equal(t, tt.flags, r.flags, "Wrong value control flag for release %s", r.release.Name) } } + +func TestChartCacheKey(t *testing.T) { +st := &HelmState{} + +// Test case 1: release with version +release1 := &ReleaseSpec{ +Chart: "stable/nginx", +Version: "1.2.3", +} + +key1 := st.getChartCacheKey(release1) +expected1 := ChartCacheKey{Chart: "stable/nginx", Version: "1.2.3"} + +if key1 != expected1 { +t.Errorf("Expected %+v, got %+v", expected1, key1) +} + +// Test case 2: release without version +release2 := &ReleaseSpec{ +Chart: "stable/nginx", +} + +key2 := st.getChartCacheKey(release2) +expected2 := ChartCacheKey{Chart: "stable/nginx", Version: ""} + +if key2 != expected2 { +t.Errorf("Expected %+v, got %+v", expected2, key2) +} +} + +func TestChartCache(t *testing.T) { +st := &HelmState{} + +// Create a test key +key := ChartCacheKey{Chart: "stable/test", Version: "1.0.0"} +path := "/tmp/test-chart" + +// Initially, chart should not be in cache +_, exists := st.checkChartCache(key) +if exists { +t.Error("Chart should not be in cache initially") +} + +// Add to cache +st.addToChartCache(key, path) + +// Now chart should be in cache +cachedPath, exists := st.checkChartCache(key) +if !exists { +t.Error("Chart should be in cache after adding") +} +if cachedPath != path { +t.Errorf("Expected path %s, got %s", path, cachedPath) +} +}