Repository navigation
Remove leaked Helm index cache on error - #2201
Merged
Merged
Conversation
CacheIndex writes chart-index-*.yaml under the controller temp directory and leaves removal to the caller. reconcileArtifact deletes that file, but reconcile returns on the first error, so a failed index load, revision calculation, or status patch never reaches it. Each failed reconcile keeps the file until the pod restarts. Remove the cached index on those three returns. Leave it in place when reconcileSource succeeds, including the unchanged-revision short circuit, because reconcileArtifact still needs the file. Assisted-by: Grok/grok-4.7 Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
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
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.
ChartRepository.CacheIndexwriteschart-index-*.yamlunder the controller temp directory and documents that the caller removes it.reconcileArtifactdoes that in a defer.reconcilestops on the first error, so threereconcileSourcereturns never reach that defer:LoadFromPathrejects itEach failed reconcile leaves the file until the pod restarts. The controller points
TMPDIRat its own temp root, and startup purge only removes leftovers from the previous process.This removes the cached index on those three returns. A successful fetch, including the unchanged-revision short circuit, still leaves the file for
reconcileArtifact. A close error is logged with the same messagereconcileArtifactalready uses, and it does not replace the original reconcile error.The leak was reported in #1451 and #1462. #1457 refuses to keep an index larger than the maximum size, which covers the oversized file those issues described. An HTTP 200 body that is not a valid Helm index, and a later status patch failure, still left the file. The returns without removal come from #1001.
#824 and #2063 change the error text for an invalid repository. They do not remove the temp file.
Tests
TestHelmRepositoryReconciler_reconcileSource_cachedIndexFilesetsTMPDIRto a private directory and globschart-index-*.yaml.On the previous code, the invalid-index and status-patch cases returned the expected errors and still left a
chart-index-*.yamlfile. The successful fetch already kept the file.After this change, the two error cases leave no
chart-index-*.yamlfile, and the successful fetch still keepschartRepo.Path.