feat: Implement caching for pulling OCI charts#2171
Conversation
mustdiechik
requested review from
itscaro,
mumoshu,
xiaomudk,
yxxhero and
zhaque44
as code owners
mustdiechik
force-pushed
the
oci_cache
branch
2 times, most recently
from
a52aa4e to
a6b31ba
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements caching for OCI charts by modifying the chart pulling mechanism to use a persistent cache directory instead of temporary directories. The goal is to improve performance for large releases and high network latency scenarios by avoiding repeated chart downloads.
Key changes:
- Modified
getOCIChartfunction to accept fullChartPrepareOptionsinstead of just the output directory template - Introduced persistent cache directory using
os.TempDir() + "/helmfile-oci-cache" - Added logic to conditionally clear cached charts based on
SkipDepsandSkipRefreshoptions
mustdiechik
force-pushed
the
oci_cache
branch
from
a6b31ba to
5e88563
Compare
|
@mustdiechik did you test this locally? and maybe we should add some docs for this feature? |
mustdiechik
force-pushed
the
oci_cache
branch
3 times, most recently
from
445cca1 to
f7e776c
Compare
mustdiechik
force-pushed
the
oci_cache
branch
2 times, most recently
from
2f9e5e1 to
aeea415
Compare
Yes, I tested build on working repos and via 'make test".
|
| } | ||
|
|
||
| pathElems = append(pathElems, release.Name, chartName, safeVersionPath(chartVersion)) | ||
| pathElems = append(pathElems, release.Chart, safeVersionPath(release.Version)) |
There was a problem hiding this comment.
Using release.Chart directly in the path may cause issues when the chart name contains special characters or path separators. The original code used chartName which was likely sanitized. Consider extracting just the chart name portion from release.Chart or applying path sanitization.
| pathElems = append(pathElems, release.Chart, safeVersionPath(release.Version)) | |
| _, chartName := st.GetRepositoryAndNameFromChartName(release.Chart) | |
| pathElems = append(pathElems, chartName, safeVersionPath(release.Version)) |
mustdiechik
force-pushed
the
oci_cache
branch
from
b93ab3c to
64df5ec
Compare
|
@yxxhero fixed concurrency issue. tested with |
mustdiechik
force-pushed
the
oci_cache
branch
from
64df5ec to
33af99b
Compare
Signed-off-by: Ruslan Khizhnyak <[email protected]>
Signed-off-by: Ruslan Khizhnyak <[email protected]> Co-authored-by: Ruslan Khizhnyak <[email protected]>
To avoid pulling chart from OCI registries for each time.
It is pain for large releases and high network latency.