diff --git a/api/v1beta2/artifact_types.go b/api/v1beta2/artifact_types.go index 4bd1bfede..9ae05ed94 100644 --- a/api/v1beta2/artifact_types.go +++ b/api/v1beta2/artifact_types.go @@ -56,8 +56,8 @@ type Artifact struct { Size *int64 `json:"size,omitempty"` } -// HasRevision returns true if the given revision matches the current Revision -// of the Artifact. +// HasRevision returns if the given revision matches the current Revision of +// the Artifact. func (in *Artifact) HasRevision(revision string) bool { if in == nil { return false @@ -65,6 +65,15 @@ func (in *Artifact) HasRevision(revision string) bool { return in.Revision == revision } +// HasChecksum returns if the given checksum matches the current Checksum of +// the Artifact. +func (in *Artifact) HasChecksum(checksum string) bool { + if in == nil { + return false + } + return in.Checksum == checksum +} + // ArtifactDir returns the artifact dir path in the form of // '//'. func ArtifactDir(kind, namespace, name string) string { diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index f8c7fd210..fb0ced3a8 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -398,6 +398,8 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou return sreconcile.ResultEmpty, e } } + + // Fetch the repository index from remote. checksum, err := newChartRepo.CacheIndex() if err != nil { e := &serror.Event{ @@ -410,6 +412,15 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou } *chartRepo = *newChartRepo + // 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. + if obj.GetArtifact().HasChecksum(checksum) { + *artifact = *obj.GetArtifact() + conditions.Delete(obj, sourcev1.FetchFailedCondition) + return sreconcile.ResultSuccess, nil + } + // Load the cached repository index to ensure it passes validation. if err := chartRepo.LoadFromCache(); err != nil { e := &serror.Event{ @@ -419,23 +430,24 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } - defer chartRepo.Unload() + chartRepo.Unload() // Mark observations about the revision on the object. - if !obj.GetArtifact().HasRevision(checksum) { + if !obj.GetArtifact().HasRevision(newChartRepo.Checksum) { message := fmt.Sprintf("new index revision '%s'", checksum) conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message) conditions.MarkReconciling(obj, "NewRevision", message) } - conditions.Delete(obj, sourcev1.FetchFailedCondition) - // Create potential new artifact. *artifact = r.Storage.NewArtifactFor(obj.Kind, obj.ObjectMeta.GetObjectMeta(), chartRepo.Checksum, fmt.Sprintf("index-%s.yaml", checksum)) + // Delete any stale failure observation + conditions.Delete(obj, sourcev1.FetchFailedCondition) + return sreconcile.ResultSuccess, nil } @@ -462,7 +474,7 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *s } }() - if obj.GetArtifact().HasRevision(artifact.Revision) { + if obj.GetArtifact().HasRevision(artifact.Revision) && obj.GetArtifact().HasChecksum(artifact.Checksum) { r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.ArtifactUpToDateReason, "artifact up-to-date with remote revision: '%s'", artifact.Revision) return sreconcile.ResultSuccess, nil } diff --git a/internal/helm/repository/chart_repository.go b/internal/helm/repository/chart_repository.go index 5ccc1b7de..c54888daa 100644 --- a/internal/helm/repository/chart_repository.go +++ b/internal/helm/repository/chart_repository.go @@ -66,7 +66,8 @@ type ChartRepository struct { // Index contains a loaded chart repository index if not nil. Index *repo.IndexFile // Checksum contains the SHA256 checksum of the loaded chart repository - // index bytes. + // index bytes. This is different from the checksum of the CachePath, which + // may contain unordered entries. Checksum string tlsConfig *tls.Config @@ -87,7 +88,7 @@ type cacheInfo struct { RecordIndexCacheMetric RecordMetricsFunc } -// ChartRepositoryOptions is a function that can be passed to NewChartRepository +// ChartRepositoryOption is a function that can be passed to NewChartRepository // to configure a ChartRepository. type ChartRepositoryOption func(*ChartRepository) error