Skip to content

Commit

Permalink
helm: only use Digest to calculcate index revision
Browse files Browse the repository at this point in the history
In #1001 bits around the Helm repository reconciliation logic were
rewritten, mostly based on the documented behavior instead of the
actual code. This resulted in the reintroduction of a YAML marshal of
the (sorted) index YAML instead of reliance of just the checksum of the
file.

This to take situations into account in which a repository would e.g.
provide a new random order on every generation. However, this approach
is (extremely) expensive as the marshal goes through a JSON -> YAML
loop, eating lots of RAM in the process.

As the further (silently) introduced behavior has not resulted in any
reported issues, I deem this approach safe and better than e.g.
encoding to just JSON which would still require a substantial amount of
memory.

Signed-off-by: Hidde Beydals <hello@hidde.co>
  • Loading branch information
hiddeco committed Feb 22, 2023
1 parent 568b932 commit c0a1099
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
7 changes: 3 additions & 4 deletions controllers/helmrepository_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,8 +475,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc
}
if curDig.Validate() == nil {
// Short-circuit based on the fetched index being an exact match to the
// stored Artifact. This prevents having to unmarshal the YAML to calculate
// the (stable) revision, which is a memory expensive operation.
// stored Artifact.
if newDig := chartRepo.Digest(curDig.Algorithm()); newDig.Validate() == nil && (newDig == curDig) {
*artifact = *curArtifact
conditions.Delete(obj, sourcev1.FetchFailedCondition)
Expand All @@ -501,11 +500,11 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc
var changed bool
if artifact := obj.Status.Artifact; artifact != nil {
curRev := digest.Digest(sourcev1.TransformLegacyRevision(artifact.Revision))
changed = curRev.Validate() != nil || curRev != chartRepo.Revision(curRev.Algorithm())
changed = curRev.Validate() != nil || curRev != chartRepo.Digest(curRev.Algorithm())
}

// Calculate revision.
revision := chartRepo.Revision(intdigest.Canonical)
revision := chartRepo.Digest(intdigest.Canonical)
if revision.Validate() != nil {
e := &serror.Event{
Err: fmt.Errorf("failed to calculate revision: %w", err),
Expand Down
2 changes: 2 additions & 0 deletions internal/helm/repository/chart_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,8 @@ func (r *ChartRepository) DownloadIndex(w io.Writer) (err error) {

// Revision returns the revision of the ChartRepository's Index. It assumes
// the Index is stable sorted.
// Deprecated: because of expensive memory usage of (YAML) marshal operations.
// We only use Digest now.
func (r *ChartRepository) Revision(algorithm digest.Algorithm) digest.Digest {
if !r.HasIndex() {
return ""
Expand Down

0 comments on commit c0a1099

Please sign in to comment.