Skip to content

Commit

Permalink
Merge pull request #917 from fluxcd/improv-ocirepo-optimized-reconcile
Browse files Browse the repository at this point in the history
OCIRepositoryReconciler no-op improvements
  • Loading branch information
stefanprodan authored Sep 29, 2022
2 parents 5ea4922 + dcd0db4 commit 95cbf40
Show file tree
Hide file tree
Showing 5 changed files with 363 additions and 15 deletions.
11 changes: 11 additions & 0 deletions api/v1beta2/ocirepository_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,17 @@ type OCIRepositoryStatus struct {
// +optional
Artifact *Artifact `json:"artifact,omitempty"`

// ContentConfigChecksum is a checksum of all the configurations related to
// the content of the source artifact:
// - .spec.ignore
// - .spec.layerSelector
// observed in .status.observedGeneration version of the object. This can
// be used to determine if the content configuration has changed and the
// artifact needs to be rebuilt.
// It has the format of `<algo>:<checksum>`, for example: `sha256:<checksum>`.
// +optional
ContentConfigChecksum string `json:"contentConfigChecksum,omitempty"`

meta.ReconcileRequestStatus `json:",inline"`
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,14 @@ spec:
- type
type: object
type: array
contentConfigChecksum:
description: 'ContentConfigChecksum is a checksum of all the configurations
related to the content of the source artifact: - .spec.ignore -
.spec.layerSelector observed in .status.observedGeneration version
of the object. This can be used to determine if the content configuration
has changed and the artifact needs to be rebuilt. It has the format
of `<algo>:<checksum>`, for example: `sha256:<checksum>`.'
type: string
lastHandledReconcileAt:
description: LastHandledReconcileAt holds the value of the most recent
reconcile request value, so a change of the annotation value can
Expand Down
51 changes: 46 additions & 5 deletions controllers/ocirepository_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controllers

import (
"context"
"crypto/sha256"
"crypto/tls"
"crypto/x509"
"errors"
Expand Down Expand Up @@ -60,6 +61,7 @@ import (
"github.com/fluxcd/pkg/runtime/events"
"github.com/fluxcd/pkg/runtime/patch"
"github.com/fluxcd/pkg/runtime/predicates"
"github.com/fluxcd/pkg/sourceignore"
"github.com/fluxcd/pkg/untar"
"github.com/fluxcd/pkg/version"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
Expand Down Expand Up @@ -418,8 +420,10 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sour
conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of revision %s", revision)
}

// Skip pulling if the artifact revision hasn't changes
if obj.GetArtifact().HasRevision(revision) {
// Skip pulling if the artifact revision and the content config checksum has
// not changed.
if obj.GetArtifact().HasRevision(revision) &&
r.calculateContentConfigChecksum(obj) == obj.Status.ContentConfigChecksum {
conditions.Delete(obj, sourcev1.FetchFailedCondition)
return sreconcile.ResultSuccess, nil
}
Expand Down Expand Up @@ -922,17 +926,22 @@ func (r *OCIRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
artifact := r.Storage.NewArtifactFor(obj.Kind, obj, revision,
fmt.Sprintf("%s.tar.gz", r.digestFromRevision(revision)))

// Calculate the content config checksum.
ccc := r.calculateContentConfigChecksum(obj)

// Set the ArtifactInStorageCondition if there's no drift.
defer func() {
if obj.GetArtifact().HasRevision(artifact.Revision) {
if obj.GetArtifact().HasRevision(artifact.Revision) &&
obj.Status.ContentConfigChecksum == ccc {
conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition)
conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason,
"stored artifact for digest '%s'", artifact.Revision)
}
}()

// The artifact is up-to-date
if obj.GetArtifact().HasRevision(artifact.Revision) {
if obj.GetArtifact().HasRevision(artifact.Revision) &&
obj.Status.ContentConfigChecksum == ccc {
r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.ArtifactUpToDateReason,
"artifact up-to-date with remote revision: '%s'", artifact.Revision)
return sreconcile.ResultSuccess, nil
Expand Down Expand Up @@ -984,7 +993,20 @@ func (r *OCIRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
return sreconcile.ResultEmpty, e
}
default:
if err := r.Storage.Archive(&artifact, dir, nil); err != nil {
// Load ignore rules for archiving.
ignoreDomain := strings.Split(dir, string(filepath.Separator))
ps, err := sourceignore.LoadIgnorePatterns(dir, ignoreDomain)
if err != nil {
return sreconcile.ResultEmpty, serror.NewGeneric(
fmt.Errorf("failed to load source ignore patterns from repository: %w", err),
"SourceIgnoreError",
)
}
if obj.Spec.Ignore != nil {
ps = append(ps, sourceignore.ReadPatterns(strings.NewReader(*obj.Spec.Ignore), ignoreDomain)...)
}

if err := r.Storage.Archive(&artifact, dir, SourceIgnoreFilter(ps, ignoreDomain)); err != nil {
e := serror.NewGeneric(
fmt.Errorf("unable to archive artifact to storage: %s", err),
sourcev1.ArchiveOperationFailedReason,
Expand All @@ -997,6 +1019,7 @@ func (r *OCIRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
// Record it on the object
obj.Status.Artifact = artifact.DeepCopy()
obj.Status.Artifact.Metadata = metadata.Metadata
obj.Status.ContentConfigChecksum = ccc

// Update symlink on a "best effort" basis
url, err := r.Storage.Symlink(artifact, "latest.tar.gz")
Expand Down Expand Up @@ -1125,3 +1148,21 @@ func (r *OCIRepositoryReconciler) notify(ctx context.Context, oldObj, newObj *so
}
}
}

// calculateContentConfigChecksum calculates a checksum of all the
// configurations that result in a change in the source artifact. It can be used
// to decide if further reconciliation is needed when an artifact already exists
// for a set of configurations.
func (r *OCIRepositoryReconciler) calculateContentConfigChecksum(obj *sourcev1.OCIRepository) string {
c := []byte{}
// Consider the ignore rules.
if obj.Spec.Ignore != nil {
c = append(c, []byte(*obj.Spec.Ignore)...)
}
// Consider the layer selector.
if obj.Spec.LayerSelector != nil {
c = append(c, []byte(obj.GetLayerMediaType()+obj.GetLayerOperation())...)
}

return fmt.Sprintf("sha256:%x", sha256.Sum256(c))
}
Loading

0 comments on commit 95cbf40

Please sign in to comment.