feat: cache upstream during update (#3154)

Author: bharathkkb

internal/gitutil/gitutil.go

@@ -137,13 +137,26 @@ func (g *GitLocalRunner) run(ctx context.Context, verbose bool, command string,
 	}, nil
 }
 
+type NewGitUpstreamRepoOption func(*GitUpstreamRepo)
+
+func WithFetchedRefs(a map[string]bool) NewGitUpstreamRepoOption {
+	return func(g *GitUpstreamRepo) {
+		g.fetchedRefs = a
+	}
+}
+
 // NewGitUpstreamRepo returns a new GitUpstreamRepo for an upstream package.
-func NewGitUpstreamRepo(ctx context.Context, uri string) (*GitUpstreamRepo, error) {
+func NewGitUpstreamRepo(ctx context.Context, uri string, opts ...NewGitUpstreamRepoOption) (*GitUpstreamRepo, error) {
 	const op errors.Op = "gitutil.NewGitUpstreamRepo"
-
 	g := &GitUpstreamRepo{
 		URI: uri,
 	}
+	for _, opt := range opts {
+		opt(g)
+	}
+	if g.fetchedRefs == nil {
+		g.fetchedRefs = map[string]bool{}
+	}
 	if err := g.updateRefs(ctx); err != nil {
 		return nil, errors.E(op, errors.Repo(uri), err)
 	}
@@ -161,6 +174,17 @@ type GitUpstreamRepo struct {
 	// Tags contains all tag refs in the upstream repo as well as the
 	// each of the are referencing.
 	Tags map[string]string
+
+	// fetchedRefs keeps track of refs already fetched from remote
+	fetchedRefs map[string]bool
+}
+
+func (gur *GitUpstreamRepo) GetFetchedRefs() []string {
+	fetchedRefs := make([]string, 0, len(gur.fetchedRefs))
+	for ref := range gur.fetchedRefs {
+		fetchedRefs = append(fetchedRefs, ref)
+	}
+	return fetchedRefs
 }
 
 // updateRefs fetches all refs from the upstream git repo, parses the results
@@ -374,20 +398,26 @@ loop:
 		// commit sha.
 		validFullSha := s == strings.TrimSpace(rr.Stdout)
 		_, resolved := gur.ResolveRef(s)
-
+		// check if ref was previously fetched
+		// we use the ref s as the cache key
+		_, fetched := gur.fetchedRefs[s]
 		switch {
+		case fetched:
+			// skip refetching if previously fetched
+			break
 		case resolved || validFullSha:
 			// If the ref references a branch or a tag, or is a valid commit
-			// sha, we can fetch just a single commit.
+			// sha and has not already been fetched, we can fetch just a single commit.
 			if _, err := gitRunner.RunVerbose(ctx, "fetch", "origin", "--depth=1", s); err != nil {
 				AmendGitExecError(err, func(e *GitExecError) {
 					e.Repo = uri
-					e.Command = "origin"
+					e.Command = "fetch"
 					e.Ref = s
 				})
 				return "", errors.E(op, errors.Git, fmt.Errorf(
 					"error running `git fetch` for ref %q: %w", s, err))
 			}
+			gur.fetchedRefs[s] = true
 		default:
 			// In other situations (like a short commit sha), we have to do
 			// a full fetch from the remote.
@@ -407,6 +437,7 @@ loop:
 				return "", errors.E(op, errors.Git, fmt.Errorf(
 					"error verifying results from fetch: %w", err))
 			}
+			gur.fetchedRefs[s] = true
 			// If we did a full fetch, we already have all refs, so we can just
 			// exit the loop.
 			break loop

internal/util/fetch/fetch.go

@@ -62,7 +62,7 @@ func (c Command) Run(ctx context.Context) error {
 		Path:    g.Directory,
 		Ref:     g.Ref,
 	}
-	err = cloneAndCopy(ctx, repoSpec, c.Pkg.UniquePath.String())
+	err = NewCloner(repoSpec).cloneAndCopy(ctx, c.Pkg.UniquePath.String())
 	if err != nil {
 		return errors.E(op, c.Pkg.UniquePath, err)
 	}
@@ -94,21 +94,55 @@ func (c Command) validate(kf *kptfilev1.KptFile) error {
 	return nil
 }
 
+// Cloner clones an upstream repo defined by a repoSpec.
+// Optionally, previously cloned repos can be cached
+// rather than recloning them each time.
+type Cloner struct {
+	// repoSpec spec to clone
+	repoSpec *git.RepoSpec
+
+	// cachedRepos
+	cachedRepo map[string]*gitutil.GitUpstreamRepo
+}
+
+type NewClonerOption func(*Cloner)
+
+func WithCachedRepo(r map[string]*gitutil.GitUpstreamRepo) NewClonerOption {
+	return func(c *Cloner) {
+		c.cachedRepo = r
+	}
+}
+
+func NewCloner(r *git.RepoSpec, opts ...NewClonerOption) *Cloner {
+	c := &Cloner{
+		repoSpec: r,
+	}
+	for _, opt := range opts {
+		opt(c)
+	}
+	if c.cachedRepo == nil {
+		c.cachedRepo = make(map[string]*gitutil.GitUpstreamRepo)
+	}
+	return c
+}
+
 // cloneAndCopy fetches the provided repo and copies the content into the
 // directory specified by dest. The provided name is set as `metadata.name`
 // of the Kptfile of the package.
-func cloneAndCopy(ctx context.Context, r *git.RepoSpec, dest string) error {
+func (c *Cloner) cloneAndCopy(ctx context.Context, dest string) error {
 	const op errors.Op = "fetch.cloneAndCopy"
 	pr := printer.FromContextOrDie(ctx)
 
-	err := ClonerUsingGitExec(ctx, r)
+	err := c.ClonerUsingGitExec(ctx)
 	if err != nil {
 		return errors.E(op, errors.Git, types.UniquePath(dest), err)
 	}
-	defer os.RemoveAll(r.Dir)
+	defer os.RemoveAll(c.repoSpec.Dir)
+	// update cache before removing clone dir
+	defer delete(c.cachedRepo, c.repoSpec.CloneSpec())
 
-	sourcePath := filepath.Join(r.Dir, r.Path)
-	pr.Printf("Adding package %q.\n", strings.TrimPrefix(r.Path, "/"))
+	sourcePath := filepath.Join(c.repoSpec.Dir, c.repoSpec.Path)
+	pr.Printf("Adding package %q.\n", strings.TrimPrefix(c.repoSpec.Path, "/"))
 	if err := pkgutil.CopyPackage(sourcePath, dest, true, pkg.All); err != nil {
 		return errors.E(op, types.UniquePath(dest), err)
 	}
@@ -117,7 +151,7 @@ func cloneAndCopy(ctx context.Context, r *git.RepoSpec, dest string) error {
 		return errors.E(op, types.UniquePath(dest), err)
 	}
 
-	if err := kptfileutil.UpdateUpstreamLockFromGit(dest, r); err != nil {
+	if err := kptfileutil.UpdateUpstreamLockFromGit(dest, c.repoSpec); err != nil {
 		return errors.E(op, errors.Git, types.UniquePath(dest), err)
 	}
 	return nil
@@ -129,46 +163,51 @@ func cloneAndCopy(ctx context.Context, r *git.RepoSpec, dest string) error {
 // for versioning multiple kpt packages in a single repo independently. It
 // relies on the private clonerUsingGitExec function to try fetching different
 // refs.
-func ClonerUsingGitExec(ctx context.Context, repoSpec *git.RepoSpec) error {
+func (c *Cloner) ClonerUsingGitExec(ctx context.Context) error {
 	const op errors.Op = "fetch.ClonerUsingGitExec"
 
 	// Create a local representation of the upstream repo. This will initialize
 	// the cache for the specified repo uri if it isn't already there. It also
 	// fetches and caches all tag and branch refs from the upstream repo.
-	upstreamRepo, err := gitutil.NewGitUpstreamRepo(ctx, repoSpec.CloneSpec())
-	if err != nil {
-		return errors.E(op, errors.Git, errors.Repo(repoSpec.CloneSpec()), err)
+	upstreamRepo, exists := c.cachedRepo[c.repoSpec.CloneSpec()]
+	if !exists {
+		newUpstreamRemp, err := gitutil.NewGitUpstreamRepo(ctx, c.repoSpec.CloneSpec())
+		if err != nil {
+			return errors.E(op, errors.Git, errors.Repo(c.repoSpec.CloneSpec()), err)
+		}
+		upstreamRepo = newUpstreamRemp
+		c.cachedRepo[c.repoSpec.CloneSpec()] = upstreamRepo
 	}
 
 	// Check if we have a ref in the upstream that matches the package-specific
 	// reference. If we do, we use that reference.
-	ps := strings.Split(repoSpec.Path, "/")
+	ps := strings.Split(c.repoSpec.Path, "/")
 	for len(ps) != 0 {
 		p := path.Join(ps...)
-		packageRef := path.Join(strings.TrimLeft(p, "/"), repoSpec.Ref)
+		packageRef := path.Join(strings.TrimLeft(p, "/"), c.repoSpec.Ref)
 		if _, found := upstreamRepo.ResolveTag(packageRef); found {
-			repoSpec.Ref = packageRef
+			c.repoSpec.Ref = packageRef
 			break
 		}
 		ps = ps[:len(ps)-1]
 	}
 
 	// Pull the required ref into the repo git cache.
-	dir, err := upstreamRepo.GetRepo(ctx, []string{repoSpec.Ref})
+	dir, err := upstreamRepo.GetRepo(ctx, []string{c.repoSpec.Ref})
 	if err != nil {
-		return errors.E(op, errors.Git, errors.Repo(repoSpec.CloneSpec()), err)
+		return errors.E(op, errors.Git, errors.Repo(c.repoSpec.CloneSpec()), err)
 	}
 
 	gitRunner, err := gitutil.NewLocalGitRunner(dir)
 	if err != nil {
-		return errors.E(op, errors.Git, errors.Repo(repoSpec.CloneSpec()), err)
+		return errors.E(op, errors.Git, errors.Repo(c.repoSpec.CloneSpec()), err)
 	}
 
 	// Find the commit SHA for the ref that was just fetched. We need the SHA
 	// rather than the ref to be able to do a hard reset of the cache repo.
-	commit, found := upstreamRepo.ResolveRef(repoSpec.Ref)
+	commit, found := upstreamRepo.ResolveRef(c.repoSpec.Ref)
 	if !found {
-		commit = repoSpec.Ref
+		commit = c.repoSpec.Ref
 	}
 
 	// Reset the local repo to the commit we need. Doing a hard reset instead of
@@ -178,34 +217,34 @@ func ClonerUsingGitExec(ctx context.Context, repoSpec *git.RepoSpec) error {
 	_, err = gitRunner.Run(ctx, "reset", "--hard", commit)
 	if err != nil {
 		gitutil.AmendGitExecError(err, func(e *gitutil.GitExecError) {
-			e.Repo = repoSpec.CloneSpec()
+			e.Repo = c.repoSpec.CloneSpec()
 			e.Ref = commit
 		})
-		return errors.E(op, errors.Git, errors.Repo(repoSpec.CloneSpec()), err)
+		return errors.E(op, errors.Git, errors.Repo(c.repoSpec.CloneSpec()), err)
 	}
 
 	// We need to create a temp directory where we can copy the content of the repo.
 	// During update, we need to checkout multiple versions of the same repo, so
 	// we can't do merges directly from the cache.
-	repoSpec.Dir, err = ioutil.TempDir("", "kpt-get-")
+	c.repoSpec.Dir, err = ioutil.TempDir("", "kpt-get-")
 	if err != nil {
 		return errors.E(op, errors.Internal, fmt.Errorf("error creating temp directory: %w", err))
 	}
-	repoSpec.Commit = commit
+	c.repoSpec.Commit = commit
 
-	pkgPath := filepath.Join(dir, repoSpec.Path)
+	pkgPath := filepath.Join(dir, c.repoSpec.Path)
 	// Verify that the requested path exists in the repo.
 	_, err = os.Stat(pkgPath)
 	if os.IsNotExist(err) {
 		return errors.E(op,
 			errors.Internal,
 			err,
-			fmt.Errorf("path %q does not exist in repo %q", repoSpec.Path, repoSpec.OrgRepo))
+			fmt.Errorf("path %q does not exist in repo %q", c.repoSpec.Path, c.repoSpec.OrgRepo))
 	}
 
 	// Copy the content of the pkg into the temp directory.
 	// Note that we skip the content outside the package directory.
-	err = copyDir(ctx, pkgPath, repoSpec.AbsPath())
+	err = copyDir(ctx, pkgPath, c.repoSpec.AbsPath())
 	if err != nil {
 		return errors.E(op, errors.Internal, fmt.Errorf("error copying package: %w", err))
 	}
@@ -226,7 +265,7 @@ func ClonerUsingGitExec(ctx context.Context, repoSpec *git.RepoSpec) error {
 		var kfError *pkg.KptfileError
 		if errors.As(err, &kfError) {
 			return &pkg.RemoteKptfileError{
-				RepoSpec: repoSpec,
+				RepoSpec: c.repoSpec,
 				Err:      kfError.Err,
 			}
 		}

internal/util/update/update.go

@@ -25,6 +25,7 @@ import (
 	"strings"
 
 	"github.com/GoogleContainerTools/kpt/internal/errors"
+	"github.com/GoogleContainerTools/kpt/internal/gitutil"
 	"github.com/GoogleContainerTools/kpt/internal/pkg"
 	"github.com/GoogleContainerTools/kpt/internal/printer"
 	"github.com/GoogleContainerTools/kpt/internal/types"
@@ -102,10 +103,13 @@ type Command struct {
 
 	// Strategy is the update strategy to use
 	Strategy kptfilev1.UpdateStrategyType
+
+	// cachedUpstreamRepos is an upstream repo already fetched for a given repoSpec CloneRef
+	cachedUpstreamRepos map[string]*gitutil.GitUpstreamRepo
 }
 
 // Run runs the Command.
-func (u Command) Run(ctx context.Context) error {
+func (u *Command) Run(ctx context.Context) error {
 	const op errors.Op = "update.Run"
 	pr := printer.FromContextOrDie(ctx)
 
@@ -133,7 +137,9 @@ func (u Command) Run(ctx context.Context) error {
 	if err != nil {
 		return errors.E(op, u.Pkg.UniquePath, err)
 	}
-
+	if u.cachedUpstreamRepos == nil {
+		u.cachedUpstreamRepos = make(map[string]*gitutil.GitUpstreamRepo)
+	}
 	packageCount := 0
 
 	// Use stack to keep track of paths with a Kptfile that might contain
@@ -182,6 +188,11 @@ func (u Command) Run(ctx context.Context) error {
 	return nil
 }
 
+// GetCachedUpstreamRepos returns repos cached during update
+func (u Command) GetCachedUpstreamRepos() map[string]*gitutil.GitUpstreamRepo {
+	return u.cachedUpstreamRepos
+}
+
 // updateSubKf updates subpackage with given ref and update strategy
 func updateSubKf(subKf *kptfilev1.KptFile, ref string, strategy kptfilev1.UpdateStrategyType) {
 	// check if explicit ref provided
@@ -250,7 +261,8 @@ func (u Command) updateRootPackage(ctx context.Context, p *pkg.Pkg) error {
 	g := kf.Upstream.Git
 	updated := &git.RepoSpec{OrgRepo: g.Repo, Path: g.Directory, Ref: g.Ref}
 	pr.Printf("Fetching upstream from %s@%s\n", kf.Upstream.Git.Repo, kf.Upstream.Git.Ref)
-	if err := fetch.ClonerUsingGitExec(ctx, updated); err != nil {
+	cloner := fetch.NewCloner(updated, fetch.WithCachedRepo(u.cachedUpstreamRepos))
+	if err := cloner.ClonerUsingGitExec(ctx); err != nil {
 		return errors.E(op, p.UniquePath, err)
 	}
 	defer os.RemoveAll(updated.AbsPath())
@@ -260,7 +272,7 @@ func (u Command) updateRootPackage(ctx context.Context, p *pkg.Pkg) error {
 		gLock := kf.UpstreamLock.Git
 		originRepoSpec := &git.RepoSpec{OrgRepo: gLock.Repo, Path: gLock.Directory, Ref: gLock.Commit}
 		pr.Printf("Fetching origin from %s@%s\n", kf.Upstream.Git.Repo, kf.Upstream.Git.Ref)
-		if err := fetch.ClonerUsingGitExec(ctx, originRepoSpec); err != nil {
+		if err := fetch.NewCloner(originRepoSpec, fetch.WithCachedRepo(u.cachedUpstreamRepos)).ClonerUsingGitExec(ctx); err != nil {
 			return errors.E(op, p.UniquePath, err)
 		}
 		origin = originRepoSpec