Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not refresh if no repo changes #4085

Merged
merged 3 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions porch/pkg/cache/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ type cachedRepository struct {
repo repository.Repository
cancel context.CancelFunc

lastVersion string

mutex sync.Mutex
cachedPackageRevisions map[repository.PackageRevisionKey]*cachedPackageRevision
cachedPackages map[repository.PackageKey]*cachedPackage
Expand Down Expand Up @@ -86,6 +88,10 @@ func newRepository(id string, repoSpec *configapi.Repository, repo repository.Re
return r
}

func (r *cachedRepository) Version(ctx context.Context) (string, error) {
return r.repo.Version(ctx)
}

func (r *cachedRepository) ListPackageRevisions(ctx context.Context, filter repository.ListPackageRevisionFilter) ([]repository.PackageRevision, error) {
packages, err := r.getPackageRevisions(ctx, filter, false)
if err != nil {
Expand Down Expand Up @@ -390,6 +396,15 @@ func (r *cachedRepository) refreshAllCachedPackages(ctx context.Context) (map[re
start := time.Now()
defer func() { klog.Infof("repo %s: refresh finished in %f secs", r.id, time.Since(start).Seconds()) }()

curVer, err := r.Version(ctx)
if err != nil {
return nil, nil, err
}

if curVer == r.lastVersion {
return r.cachedPackages, r.cachedPackageRevisions, nil
}

// Look up all existing PackageRevCRs so we an compare those to the
// actual Packagerevisions found in git/oci, and add/prune PackageRevCRs
// as necessary.
Expand Down Expand Up @@ -538,6 +553,7 @@ func (r *cachedRepository) refreshAllCachedPackages(ctx context.Context) (map[re

r.cachedPackageRevisions = newPackageRevisionMap
r.cachedPackages = newPackageMap
r.lastVersion = curVer

return newPackageMap, newPackageRevisionMap, nil
}
4 changes: 4 additions & 0 deletions porch/pkg/engine/fake/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ func (r *Repository) Close() error {
return nil
}

func (r *Repository) Version(ctx context.Context) (string, error) {
return "foo", nil
}

func (r *Repository) ListPackageRevisions(_ context.Context, filter repository.ListPackageRevisionFilter) ([]repository.PackageRevision, error) {
var revs []repository.PackageRevision
for _, rev := range r.PackageRevisions {
Expand Down
38 changes: 38 additions & 0 deletions porch/pkg/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
package git

import (
"bytes"
"context"
"crypto/sha256"
"encoding/hex"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -186,6 +189,35 @@ func (r *gitRepository) Close() error {
return nil
}

func (r *gitRepository) Version(ctx context.Context) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should we think about the version for the git repository calculated here? If I understand the code correctly it essentially hashes all the refs and uses that as the version. Would this handle commits that happens to existing refs (which I think primarily branches and tags) in git? In particular, would this version change if someone adds an additional commit on the main branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This checks the existence of refs, plus the commit hash to which they point. A push to main updates the main head ref to point to that new commit.

So this will change for any new or deleted tag, for a commit to any branch, for a new or deleted branch, or for a forced push, or for a tag move. AFAIK, there is no change you could make to a remote that this would not pick up. Maybe some sort of magic where you add a commit but somehow have no ref to it. But we would not care about such a change anyway (if it is even possible).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, by using both the refs and the commit hash it should cover all scenarios I can think of.

ctx, span := tracer.Start(ctx, "gitRepository::Version", trace.WithAttributes())
defer span.End()
r.mutex.Lock()
defer r.mutex.Unlock()

if err := r.fetchRemoteRepository(ctx); err != nil {
return "", err
}

refs, err := r.repo.References()
if err != nil {
return "", err
}

b := bytes.Buffer{}
for {
ref, err := refs.Next()
if err == io.EOF {
break
}

b.WriteString(ref.String())
}

hash := sha256.Sum256(b.Bytes())
return hex.EncodeToString(hash[:]), nil
}

func (r *gitRepository) ListPackages(ctx context.Context, filter repository.ListPackageFilter) ([]repository.Package, error) {
ctx, span := tracer.Start(ctx, "gitRepository::ListPackages", trace.WithAttributes())
defer span.End()
Expand Down Expand Up @@ -629,6 +661,9 @@ func (r *gitRepository) discoverFinalizedPackages(ctx context.Context, ref *plum

// loadDraft will load the draft package. If the package isn't found (we now require a Kptfile), it will return (nil, nil)
func (r *gitRepository) loadDraft(ctx context.Context, ref *plumbing.Reference) (*gitPackageRevision, error) {
ctx, span := tracer.Start(ctx, "gitRepository::loadDraft", trace.WithAttributes())
defer span.End()

name, workspaceName, err := parseDraftName(ref)
if err != nil {
return nil, err
Expand Down Expand Up @@ -719,6 +754,9 @@ func parseDraftName(draft *plumbing.Reference) (name string, workspaceName v1alp
}

func (r *gitRepository) loadTaggedPackages(ctx context.Context, tag *plumbing.Reference) ([]*gitPackageRevision, error) {
ctx, span := tracer.Start(ctx, "gitRepository::loadTaggedPackages", trace.WithAttributes())
defer span.End()

name, ok := getTagNameInLocalRepo(tag.Name())
if !ok {
return nil, fmt.Errorf("invalid tag ref: %q", tag)
Expand Down
55 changes: 55 additions & 0 deletions porch/pkg/oci/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
package oci

import (
"bytes"
"context"
"crypto/sha1"
"crypto/sha256"
"encoding/hex"
"fmt"
"strings"
Expand Down Expand Up @@ -67,6 +69,59 @@ func (r *ociRepository) Close() error {
return nil
}

// there is probably a more efficient way to do this
func (r *ociRepository) Version(ctx context.Context) (string, error) {
ctx, span := tracer.Start(ctx, "ociRepository::Version")
defer span.End()

if r.content != configapi.RepositoryContentPackage {
return "", nil
}

ociRepo, err := name.NewRepository(r.spec.Registry)
if err != nil {
return "", err
}

options := r.storage.CreateOptions(ctx)

tags, err := google.List(ociRepo, options...)
if err != nil {
return "", err
}

klog.Infof("tags: %#v", tags)

b := bytes.Buffer{}
for _, childName := range tags.Children {
path := fmt.Sprintf("%s/%s", r.spec.Registry, childName)
child, err := name.NewRepository(path, name.StrictValidation)
if err != nil {
klog.Warningf("Cannot create nested repository %q: %v", path, err)
continue
}

childTags, err := google.List(child, options...)
if err != nil {
klog.Warningf("Cannot list nested repository %q: %v", path, err)
continue
}

// klog.Infof("childTags: %#v", childTags)

for digest, m := range childTags.Manifests {
b.WriteString(digest)
mb, err := m.MarshalJSON()
if err != nil {
return "", err
}
b.Write(mb)
}
}
hash := sha256.Sum256(b.Bytes())
return hex.EncodeToString(hash[:]), nil
}

func (r *ociRepository) ListPackageRevisions(ctx context.Context, filter repository.ListPackageRevisionFilter) ([]repository.PackageRevision, error) {
if r.content != configapi.RepositoryContentPackage {
return []repository.PackageRevision{}, nil
Expand Down
3 changes: 3 additions & 0 deletions porch/pkg/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ type Repository interface {
// DeletePackage deletes a package
DeletePackage(ctx context.Context, old Package) error

// Version returns a string that is guaranteed to be different if any change has been made to the repo contents
Version(ctx context.Context) (string, error)

// Close cleans up any resources associated with the repository
Close() error
}
Expand Down
Loading