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

git v2 client: keep the primary clone up to date #30520

Merged
merged 2 commits into from
Aug 26, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions prow/config/inrepoconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions prow/git/v2/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
44 changes: 11 additions & 33 deletions prow/git/v2/client_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}
}
}

Expand All @@ -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
}
38 changes: 38 additions & 0 deletions prow/git/v2/interactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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")
Expand Down