fix: add cross-process locking and refresh coordination for chart downloads#2301
Closed
aditmeno wants to merge 2 commits into
Closed
fix: add cross-process locking and refresh coordination for chart downloads#2301aditmeno wants to merge 2 commits into
aditmeno wants to merge 2 commits into
Conversation
…nloads This commit fixes race conditions and duplicate downloads when multiple helmfile processes run in parallel (e.g., helmfile.d with multiple release files). Changes: 1. OCI Registry Login URL Fix: - Extract registry host from URL before helm registry login - e.g., "account.dkr.ecr.region.amazonaws.com/charts" -> "account.dkr.ecr.region.amazonaws.com" - Fixes authentication issues with ECR and other registries 2. Cross-Process Chart Locking: - Add file-based locking (flock) to prevent race conditions - Serialize chart downloads across parallel helmfile processes - Applies to both OCI and non-OCI charts 3. Refresh Mode Coordination: - Use marker files with 60-second TTL to coordinate refresh - First process to acquire lock performs refresh, others use cache - Prevents duplicate downloads while still supporting --skip-refresh=false 4. Code Refactoring: - Extract common locking logic into acquireChartLock() helper - Reduce code duplication between forcedDownloadChart and getOCIChart - Add unit tests for OCI registry login and SyncRepos
aditmeno
requested review from
itscaro,
mumoshu,
xiaomudk,
yxxhero and
zhaque44
as code owners
8 tasks
aditmeno
added a commit
to aditmeno/helmfile
that referenced
this pull request
Nov 26, 2025
… chart locking This merge integrates changes from PR helmfile#2301 into the issue fixes branch: 1. OCI Registry Login Fix: - Added extractRegistryHost() function to extract just the registry host - Fixed helm registry login to use host only, not full URL with path - e.g., "account.dkr.ecr.region.amazonaws.com/charts" → "account.dkr.ecr.region.amazonaws.com" 2. Refactored Chart Locking: - Introduced acquireChartLock() helper function for cleaner code - Added chartLockResult struct with action types - Added refresh marker coordination for cross-process cache management - Removed legacy OCI cache variables (downloadedOCICharts) 3. Test Fixes: - Removed debug log statements that polluted test output - All TestLint tests now pass Files changed: - pkg/state/state.go - Main logic changes - pkg/state/state_test.go - New unit tests - pkg/exectest/helm.go - Mock updates for testing
aditmeno
added a commit
to aditmeno/helmfile
that referenced
this pull request
Nov 26, 2025
… chart locking This merge integrates changes from PR helmfile#2301 into the issue fixes branch: 1. OCI Registry Login Fix: - Added extractRegistryHost() function to extract just the registry host - Fixed helm registry login to use host only, not full URL with path - e.g., "account.dkr.ecr.region.amazonaws.com/charts" → "account.dkr.ecr.region.amazonaws.com" 2. Refactored Chart Locking: - Introduced acquireChartLock() helper function for cleaner code - Added chartLockResult struct with action types - Added refresh marker coordination for cross-process cache management - Removed legacy OCI cache variables (downloadedOCICharts) 3. Test Fixes: - Removed debug log statements that polluted test output - All TestLint tests now pass Files changed: - pkg/state/state.go - Main logic changes - pkg/state/state_test.go - New unit tests - pkg/exectest/helm.go - Mock updates for testing Signed-off-by: Aditya Menon <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes race conditions and duplicate downloads when multiple helmfile processes run in parallel (e.g.,
helmfile.dwith multiple release files).Problem
When using
helmfile.d/with multiple release files that reference the same OCI chart:Solution
OCI Registry Login URL Fix
helm registry loginaccount.dkr.ecr.region.amazonaws.com/charts→account.dkr.ecr.region.amazonaws.comCross-Process Chart Locking
flock) to prevent race conditionsRefresh Mode Coordination
--skip-refresh=falseCode Refactoring
acquireChartLock()helper functionforcedDownloadChart()andgetOCIChart()Files Changed
pkg/state/state.go- Main logic changespkg/state/state_test.go- New unit testspkg/exectest/helm.go- Mock updates for testingTest Plan
Test_extractRegistryHost,TestHelmState_SyncRepos_OCI)