Merge pull request #30520 from listx/refresh-primary

git v2 client: keep the primary clone up to date

prow/config/inrepoconfig.go

@@ -146,6 +146,10 @@ func prowYAMLGetter(
 	// change was a fast-forward merge. So we need to dedupe it with sets.
 	repoOpts.FetchCommits = sets.New(baseSHA)
 	repoOpts.FetchCommits.Insert(headSHAs...)
+	// Only update the primary with the baseSHA, because it may be that the
+	// headSHAs never get merged into the base (perhaps the PR gets rejected or
+	// abandoned).
+	repoOpts.PrimaryCloneUpdateCommits = sets.New(baseSHA)
 	repo, err := gc.ClientForWithRepoOpts(orgRepo.Org, orgRepo.Repo, repoOpts)
 	inrepoconfigMetrics.gitCloneDuration.WithLabelValues(orgRepo.Org, orgRepo.Repo).Observe((float64(time.Since(timeBeforeClone).Seconds())))
 	if err != nil {

prow/git/v2/adapter.go

@@ -115,6 +115,10 @@ func (a *repoClientAdapter) FetchFromRemote(resolver RemoteResolver, branch stri
 	return errors.New("no FetchFromRemote implementation exists in the v1 repo client")
 }
 
+func (a *repoClientAdapter) FetchCommits(noFetchTags bool, commitSHAs []string) error {
+	return errors.New("no FetchCommits implementation exists in the v1 repo client")
+}
+
 func (a *repoClientAdapter) RemoteUpdate() error {
 	return errors.New("no RemoteUpdate implementation exists in the v1 repo client")
 }

prow/git/v2/client_factory.go

@@ -101,6 +101,12 @@ type RepoOpts struct {
 	// NoFetchTags determines whether we disable fetching tag objects). Defaults
 	// to false (tag objects are fetched).
 	NoFetchTags bool
+	// PrimaryCloneUpdateCommits are any additional SHAs we need to fetch into
+	// the primary clone to bring it up to speed. This should at least be the
+	// current base branch SHA. Needed when we're using shared objects because
+	// otherwise the primary will slowly get stale with no updates to it after
+	// its initial creation.
+	PrimaryCloneUpdateCommits sets.Set[string]
 }
 
 // Apply allows to use a ClientFactoryOpts as Opt
@@ -351,7 +357,7 @@ func (c *clientFactory) ClientForWithRepoOpts(org, repo string, repoOpts RepoOpt
 	if repoOpts.FetchCommits.Len() > 0 {
 		// Targeted fetch. Only fetch those commits which we want, and only
 		// if they are missing.
-		if err := ensureCommits(repoClient, repoOpts); err != nil {
+		if err := repoClientCloner.FetchCommits(repoOpts.NoFetchTags, repoOpts.FetchCommits.UnsortedList()); err != nil {
 			return nil, err
 		}
 	}
@@ -394,6 +400,10 @@ func (c *clientFactory) ensureFreshPrimary(cacheDir string, cacheClientCacher ca
 			if err := cacheClientCacher.RemoteUpdate(); err != nil {
 				return err
 			}
+		} else if repoOpts.PrimaryCloneUpdateCommits.Len() > 0 {
+			if err := cacheClientCacher.FetchCommits(repoOpts.NoFetchTags, repoOpts.PrimaryCloneUpdateCommits.UnsortedList()); err != nil {
+				return err
+			}
 		}
 	}
 
@@ -404,35 +414,3 @@ func (c *clientFactory) ensureFreshPrimary(cacheDir string, cacheClientCacher ca
 func (c *clientFactory) Clean() error {
 	return os.RemoveAll(c.cacheDir)
 }
-
-func ensureCommits(repoClient RepoClient, repoOpts RepoOpts) error {
-	fetchArgs := []string{"--no-write-fetch-head"}
-
-	if repoOpts.NoFetchTags {
-		fetchArgs = append(fetchArgs, "--no-tags")
-	}
-
-	// For each commit SHA, check if it already exists. If so, don't bother
-	// fetching it.
-	var missingCommits bool
-	for _, commitSHA := range repoOpts.FetchCommits.UnsortedList() {
-		if exists, _ := repoClient.ObjectExists(commitSHA); exists {
-			continue
-		}
-
-		fetchArgs = append(fetchArgs, commitSHA)
-		missingCommits = true
-	}
-
-	// Skip the fetch operation altogether if nothing is missing (we already
-	// fetched everything previously at some point).
-	if !missingCommits {
-		return nil
-	}
-
-	if err := repoClient.Fetch(fetchArgs...); err != nil {
-		return fmt.Errorf("failed to fetch %s: %v", fetchArgs, err)
-	}
-
-	return nil
-}

prow/git/v2/interactor.go

@@ -80,13 +80,17 @@ type cacher interface {
 	MirrorClone() error
 	// RemoteUpdate fetches all updates from the remote.
 	RemoteUpdate() error
+	// FetchCommits fetches only the given commits.
+	FetchCommits(bool, []string) error
 }
 
 // cloner knows how to clone repositories from a central cache
 type cloner interface {
 	// Clone clones the repository from a local path.
 	Clone(from string) error
 	CloneWithRepoOpts(from string, repoOpts RepoOpts) error
+	// FetchCommits fetches only the given commits.
+	FetchCommits(bool, []string) error
 }
 
 // MergeOpt holds options for git merge operations.
@@ -389,6 +393,40 @@ func (i *interactor) Am(path string) error {
 	return errors.New(string(bytes.TrimPrefix(out, []byte("The copy of the patch that failed is found in: .git/rebase-apply/patch"))))
 }
 
+// FetchCommits only fetches those commits which we want, and only if they are
+// missing.
+func (i *interactor) FetchCommits(noFetchTags bool, commitSHAs []string) error {
+	fetchArgs := []string{"--no-write-fetch-head"}
+
+	if noFetchTags {
+		fetchArgs = append(fetchArgs, "--no-tags")
+	}
+
+	// For each commit SHA, check if it already exists. If so, don't bother
+	// fetching it.
+	var missingCommits bool
+	for _, commitSHA := range commitSHAs {
+		if exists, _ := i.ObjectExists(commitSHA); exists {
+			continue
+		}
+
+		fetchArgs = append(fetchArgs, commitSHA)
+		missingCommits = true
+	}
+
+	// Skip the fetch operation altogether if nothing is missing (we already
+	// fetched everything previously at some point).
+	if !missingCommits {
+		return nil
+	}
+
+	if err := i.Fetch(fetchArgs...); err != nil {
+		return fmt.Errorf("failed to fetch %s: %v", fetchArgs, err)
+	}
+
+	return nil
+}
+
 // RemoteUpdate fetches all updates from the remote.
 func (i *interactor) RemoteUpdate() error {
 	i.logger.Info("Updating from remote")