Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add HELM_GIT_CHART_CACHE_STRATEGY variable to customize chart caching #319

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hwo411
Copy link

@hwo411 hwo411 commented Feb 2, 2025

Fixes #318

This MR allows cache customization via HELM_GIT_CHART_CACHE_STRATEGY variable.

The default behavior causes N^2 builds if package=1 and you deploy multiple chats from the same repo.

There are 2 ways to fix it:

If you set HELM_GIT_CHART_CACHE_STRATEGY to repo, then every chart will reuse the same cache from the repo. That's done by using helm_repo_uri as the source for request_hash.
If you set HELM_GIT_CHART_CACHE_STRATEGY to chart, then every chart will have only single chart packaged, not all repository.

Other values preserve the old behavior.

I'm not changing the default behavior since it can break existing setups and requires major version update according to SemVer.

One thing to consider is if we really need both strategies. If no, then we can only leave 1 and change the variable to boolean one.

In my opinion "repo" strategy is the right way to cache in all cases, but I don't have the bigger picture of how the plugin is used by others.

Tests cover both of the strategies usage.

For repo we ensure that cache is reused if we clone repo first and then try to fetch a chart.
For chart we ensure that without strategy all charts are built (> 1), with it turned on only a single one.

@aslafy-z aslafy-z changed the title Add HELM_GIT_CHART_CACHE_STRATEGY variable to customize chart caching feat: Add HELM_GIT_CHART_CACHE_STRATEGY variable to customize chart caching Feb 4, 2025
@hwo411
Copy link
Author

hwo411 commented Feb 6, 2025

@aslafy-z sorry, was short on time, just noticed failed pipelines. I see that helm v2 tests are failing, I'm going to try to find an older version of istio which is supported by it, presumably on this weekend. The main thing here is that we need repo with multiple charts, but not too big, otherwise tests take ages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: N^2 chart builds when using multiple charts from the same repo
1 participant