Skip to content
Permalink
Browse files

git: refactor to use v1 clients adapted to v2 interfaces

This is a large commit but it's mechanical:

 - update the import from git to git/v2
 - change Clone(name) to ClientFor(org, repo)

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
  • Loading branch information
stevekuznetsov committed Nov 19, 2019
1 parent 9a0d033 commit ed4f8bba242f617be80351c9ab9d4feda3e9b9ce
Showing with 123 additions and 120 deletions.
  1. +1 −2 prow/cmd/deck/BUILD.bazel
  2. +6 −5 prow/cmd/deck/main.go
  3. +3 −3 prow/cmd/deck/pr_history.go
  4. +1 −2 prow/cmd/deck/pr_history_test.go
  5. +1 −0 prow/cmd/hook/BUILD.bazel
  6. +3 −2 prow/cmd/hook/main.go
  7. +1 −0 prow/cmd/tide/BUILD.bazel
  8. +2 −1 prow/cmd/tide/main.go
  9. +1 −2 prow/config/BUILD.bazel
  10. +2 −2 prow/config/config.go
  11. +2 −2 prow/config/tide.go
  12. +1 −2 prow/config/tide_test.go
  13. +1 −1 prow/external-plugins/cherrypicker/BUILD.bazel
  14. +2 −1 prow/external-plugins/cherrypicker/main.go
  15. +5 −5 prow/external-plugins/cherrypicker/server.go
  16. +7 −7 prow/git/git_test.go
  17. +4 −1 prow/git/localgit/BUILD.bazel
  18. +4 −3 prow/git/localgit/localgit.go
  19. +11 −1 prow/git/v2/adapter.go
  20. +1 −1 prow/plugins/BUILD.bazel
  21. +1 −1 prow/plugins/buildifier/BUILD.bazel
  22. +4 −4 prow/plugins/buildifier/buildifier.go
  23. +1 −1 prow/plugins/golint/BUILD.bazel
  24. +5 −5 prow/plugins/golint/golint.go
  25. +1 −1 prow/plugins/mergecommitblocker/BUILD.bazel
  26. +3 −3 prow/plugins/mergecommitblocker/mergecommitblocker.go
  27. +1 −1 prow/plugins/override/BUILD.bazel
  28. +2 −2 prow/plugins/override/override.go
  29. +3 −3 prow/plugins/plugins.go
  30. +1 −2 prow/plugins/skip/BUILD.bazel
  31. +2 −2 prow/plugins/skip/skip.go
  32. +1 −2 prow/plugins/skip/skip_test.go
  33. +1 −2 prow/plugins/trigger/BUILD.bazel
  34. +1 −2 prow/plugins/trigger/generic-comment_test.go
  35. +1 −2 prow/plugins/trigger/pull-request_test.go
  36. +2 −2 prow/plugins/trigger/trigger.go
  37. +1 −2 prow/plugins/trigger/trigger_test.go
  38. +2 −2 prow/plugins/updateconfig/BUILD.bazel
  39. +3 −8 prow/plugins/updateconfig/updateconfig.go
  40. +2 −2 prow/plugins/updateconfig/updateconfig_test.go
  41. +1 −1 prow/plugins/verify-owners/BUILD.bazel
  42. +4 −4 prow/plugins/verify-owners/verify-owners.go
  43. +1 −1 prow/plugins/verify-owners/verify-owners_test.go
  44. +1 −1 prow/repoowners/BUILD.bazel
  45. +5 −5 prow/repoowners/repoowners.go
  46. +1 −2 prow/tide/BUILD.bazel
  47. +3 −3 prow/tide/status.go
  48. +6 −6 prow/tide/tide.go
  49. +4 −5 prow/tide/tide_test.go
@@ -66,7 +66,6 @@ go_test(
"//prow/client/clientset/versioned/fake:go_default_library",
"//prow/config:go_default_library",
"//prow/flagutil:go_default_library",
"//prow/git:go_default_library",
"//prow/github:go_default_library",
"//prow/github/fakegithub:go_default_library",
"//prow/githuboauth:go_default_library",
@@ -115,7 +114,7 @@ go_library(
"//prow/errorutil:go_default_library",
"//prow/flagutil:go_default_library",
"//prow/gcsupload:go_default_library",
"//prow/git:go_default_library",
"//prow/git/v2:go_default_library",
"//prow/github:go_default_library",
"//prow/githuboauth:go_default_library",
"//prow/interrupts:go_default_library",
@@ -61,7 +61,7 @@ import (
"k8s.io/test-infra/prow/config/secret"
"k8s.io/test-infra/prow/deck/jobs"
prowflagutil "k8s.io/test-infra/prow/flagutil"
"k8s.io/test-infra/prow/git"
"k8s.io/test-infra/prow/git/v2"
prowgithub "k8s.io/test-infra/prow/github"
"k8s.io/test-infra/prow/githuboauth"
"k8s.io/test-infra/prow/kube"
@@ -581,7 +581,7 @@ func prodOnlyMain(cfg config.Getter, pluginAgent *plugins.ConfigAgent, o options
// When inrepoconfig is enabled, both the GitHubClient and the gitClient are used to resolve
// presubmits dynamically which we need for the PR history page.
var githubClient deckGitHubClient
var gitClient *git.Client
var gitClient git.ClientFactory
secretAgent := &secret.Agent{}
if o.github.TokenPath != "" {
if err := secretAgent.Start([]string{o.github.TokenPath}); err != nil {
@@ -591,10 +591,11 @@ func prodOnlyMain(cfg config.Getter, pluginAgent *plugins.ConfigAgent, o options
if err != nil {
logrus.WithError(err).Fatal("Error getting GitHub client.")
}
gitClient, err = o.github.GitClient(secretAgent, o.dryRun)
g, err := o.github.GitClient(secretAgent, o.dryRun)
if err != nil {
logrus.WithError(err).Fatal("Error getting Git client.")
}
gitClient = git.ClientFactoryFrom(g)
}

if o.spyglass {
@@ -709,7 +710,7 @@ func prodOnlyMain(cfg config.Getter, pluginAgent *plugins.ConfigAgent, o options
return mux
}

func initSpyglass(cfg config.Getter, o options, mux *http.ServeMux, ja *jobs.JobAgent, gitHubClient deckGitHubClient, gitClient *git.Client) {
func initSpyglass(cfg config.Getter, o options, mux *http.ServeMux, ja *jobs.JobAgent, gitHubClient deckGitHubClient, gitClient git.ClientFactory) {
var c *storage.Client
var err error
if o.gcsCredentialsFile == "" {
@@ -896,7 +897,7 @@ func handleJobHistory(o options, cfg config.Getter, gcsClient *storage.Client, l
// The url must look like this:
//
// /pr-history?org=<org>&repo=<repo>&pr=<pr number>
func handlePRHistory(o options, cfg config.Getter, gcsClient *storage.Client, gitHubClient deckGitHubClient, gitClient *git.Client, log *logrus.Entry) http.HandlerFunc {
func handlePRHistory(o options, cfg config.Getter, gcsClient *storage.Client, gitHubClient deckGitHubClient, gitClient git.ClientFactory, log *logrus.Entry) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
setHeadersNoCaching(w)
tmpl, err := getPRHistory(r.URL, cfg(), gcsClient, gitHubClient, gitClient)
@@ -32,7 +32,7 @@ import (
v1 "k8s.io/test-infra/prow/apis/prowjobs/v1"
"k8s.io/test-infra/prow/config"
"k8s.io/test-infra/prow/gcsupload"
"k8s.io/test-infra/prow/git"
"k8s.io/test-infra/prow/git/v2"
"k8s.io/test-infra/prow/pod-utils/downwardapi"
)

@@ -202,7 +202,7 @@ func parsePullURL(u *url.URL) (org, repo string, pr int, err error) {
}

// getGCSDirsForPR returns a map from bucket names -> set of "directories" containing presubmit data
func getGCSDirsForPR(c *config.Config, gitHubClient deckGitHubClient, gitClient *git.Client, org, repo string, prNumber int) (map[string]sets.String, error) {
func getGCSDirsForPR(c *config.Config, gitHubClient deckGitHubClient, gitClient git.ClientFactory, org, repo string, prNumber int) (map[string]sets.String, error) {
toSearch := make(map[string]sets.String)
fullRepo := org + "/" + repo

@@ -244,7 +244,7 @@ func getGCSDirsForPR(c *config.Config, gitHubClient deckGitHubClient, gitClient
return toSearch, nil
}

func getPRHistory(url *url.URL, config *config.Config, gcsClient *storage.Client, gitHubClient deckGitHubClient, gitClient *git.Client) (prHistoryTemplate, error) {
func getPRHistory(url *url.URL, config *config.Config, gcsClient *storage.Client, gitHubClient deckGitHubClient, gitClient git.ClientFactory) (prHistoryTemplate, error) {
start := time.Now()
template := prHistoryTemplate{}

@@ -28,7 +28,6 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
prowapi "k8s.io/test-infra/prow/apis/prowjobs/v1"
"k8s.io/test-infra/prow/config"
"k8s.io/test-infra/prow/git"
"k8s.io/test-infra/prow/github"
"k8s.io/test-infra/prow/github/fakegithub"
)
@@ -425,7 +424,7 @@ func TestGetGCSDirsForPR(t *testing.T) {
123: {Number: 123},
},
}
toSearch, err := getGCSDirsForPR(tc.config, gitHubClient, &git.Client{}, tc.org, tc.repo, tc.pr)
toSearch, err := getGCSDirsForPR(tc.config, gitHubClient, nil, tc.org, tc.repo, tc.pr)
if (err != nil) != tc.expErr {
t.Errorf("%s: unexpected error %v", tc.name, err)
}
@@ -43,6 +43,7 @@ go_library(
"//prow/config:go_default_library",
"//prow/config/secret:go_default_library",
"//prow/flagutil:go_default_library",
"//prow/git/v2:go_default_library",
"//prow/hook:go_default_library",
"//prow/interrupts:go_default_library",
"//prow/logrusutil:go_default_library",
@@ -18,6 +18,7 @@ package main

import (
"flag"
"k8s.io/test-infra/prow/git/v2"
"net/http"
"os"
"strconv"
@@ -180,14 +181,14 @@ func main() {
ownersDirBlacklist := func() config.OwnersDirBlacklist {
return configAgent.Config().OwnersDirBlacklist
}
ownersClient := repoowners.NewClient(gitClient, githubClient, mdYAMLEnabled, skipCollaborators, ownersDirBlacklist)
ownersClient := repoowners.NewClient(git.ClientFactoryFrom(gitClient), githubClient, mdYAMLEnabled, skipCollaborators, ownersDirBlacklist)

clientAgent := &plugins.ClientAgent{
GitHubClient: githubClient,
ProwJobClient: prowJobClient,
KubernetesClient: infrastructureClient,
BuildClusterCoreV1Clients: buildClusterCoreV1Clients,
GitClient: gitClient,
GitClient: git.ClientFactoryFrom(gitClient),
SlackClient: slackClient,
OwnersClient: ownersClient,
BugzillaClient: bugzillaClient,
@@ -18,6 +18,7 @@ go_library(
"//prow/config:go_default_library",
"//prow/config/secret:go_default_library",
"//prow/flagutil:go_default_library",
"//prow/git/v2:go_default_library",
"//prow/interrupts:go_default_library",
"//prow/logrusutil:go_default_library",
"//prow/metrics:go_default_library",
@@ -19,6 +19,7 @@ package main
import (
"context"
"flag"
"k8s.io/test-infra/prow/git/v2"
"net/http"
"os"
"strconv"
@@ -170,7 +171,7 @@ func main() {
if err != nil {
logrus.WithError(err).Fatal("Error constructing mgr.")
}
c, err := tide.NewController(githubSync, githubStatus, mgr, cfg, gitClient, o.maxRecordsPerPool, opener, o.historyURI, o.statusURI, nil)
c, err := tide.NewController(githubSync, githubStatus, mgr, cfg, git.ClientFactoryFrom(gitClient), o.maxRecordsPerPool, opener, o.historyURI, o.statusURI, nil)
if err != nil {
logrus.WithError(err).Fatal("Error creating Tide controller.")
}
@@ -22,7 +22,6 @@ go_test(
deps = [
"//prow/apis/prowjobs/v1:go_default_library",
"//prow/config/secret:go_default_library",
"//prow/git:go_default_library",
"//prow/github:go_default_library",
"//prow/github/fakegithub:go_default_library",
"//prow/kube:go_default_library",
@@ -49,7 +48,7 @@ go_library(
importpath = "k8s.io/test-infra/prow/config",
deps = [
"//prow/apis/prowjobs/v1:go_default_library",
"//prow/git:go_default_library",
"//prow/git/v2:go_default_library",
"//prow/github:go_default_library",
"//prow/kube:go_default_library",
"//prow/pod-utils/decorate:go_default_library",
@@ -46,7 +46,7 @@ import (

pipelinev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
prowapi "k8s.io/test-infra/prow/apis/prowjobs/v1"
"k8s.io/test-infra/prow/git"
"k8s.io/test-infra/prow/git/v2"
"k8s.io/test-infra/prow/github"
"k8s.io/test-infra/prow/kube"
"k8s.io/test-infra/prow/pod-utils/decorate"
@@ -272,7 +272,7 @@ func (rg *RefGetterForGitHubPullRequest) BaseSHA() (string, error) {
// Consumers that pass in a RefGetter implementation that does a call to GitHub and who
// also need the result of that GitHub call just keep a pointer to its result, bust must
// nilcheck that pointer before accessing it.
func (c *Config) GetPresubmits(gc *git.Client, identifier string, baseSHAGetter RefGetter, headSHAGetters ...RefGetter) ([]Presubmit, error) {
func (c *Config) GetPresubmits(gc git.ClientFactory, identifier string, baseSHAGetter RefGetter, headSHAGetters ...RefGetter) ([]Presubmit, error) {
if identifier == "" {
return nil, errors.New("no identifier for repo given")
}
@@ -27,7 +27,7 @@ import (

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/test-infra/prow/git"
"k8s.io/test-infra/prow/git/v2"
"k8s.io/test-infra/prow/github"
)

@@ -489,7 +489,7 @@ func parseTideContextPolicyOptions(org, repo, branch string, options TideContext
// GetTideContextPolicy parses the prow config to find context merge options.
// If none are set, it will use the prow jobs configured and use the default github combined status.
// Otherwise if set it will use the branch protection setting, or the listed jobs.
func (c Config) GetTideContextPolicy(gitClient *git.Client, org, repo, branch string, baseSHAGetter RefGetter, headSHA string) (*TideContextPolicy, error) {
func (c Config) GetTideContextPolicy(gitClient git.ClientFactory, org, repo, branch string, baseSHAGetter RefGetter, headSHA string) (*TideContextPolicy, error) {
options := parseTideContextPolicyOptions(org, repo, branch, c.Tide.ContextOptions)
// Adding required and optional contexts from options
required := sets.NewString(options.RequiredContexts...)
@@ -23,7 +23,6 @@ import (

"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/test-infra/prow/git"
"k8s.io/test-infra/prow/github"
"k8s.io/test-infra/prow/labels"
)
@@ -573,7 +572,7 @@ func TestConfigGetTideContextPolicy(t *testing.T) {
baseSHAGetter := func() (string, error) {
return "baseSHA", nil
}
p, err := tc.config.GetTideContextPolicy(&git.Client{}, org, repo, branch, baseSHAGetter, "some-sha")
p, err := tc.config.GetTideContextPolicy(nil, org, repo, branch, baseSHAGetter, "some-sha")
if !reflect.DeepEqual(p, &tc.expected) {
t.Errorf("%s - did not get expected policy: %s", tc.name, diff.ObjectReflectDiff(&tc.expected, p))
}
@@ -13,7 +13,7 @@ go_library(
"//pkg/flagutil:go_default_library",
"//prow/config/secret:go_default_library",
"//prow/flagutil:go_default_library",
"//prow/git:go_default_library",
"//prow/git/v2:go_default_library",
"//prow/github:go_default_library",
"//prow/interrupts:go_default_library",
"//prow/pluginhelp:go_default_library",
@@ -29,6 +29,7 @@ import (
"k8s.io/test-infra/pkg/flagutil"
"k8s.io/test-infra/prow/config/secret"
prowflagutil "k8s.io/test-infra/prow/flagutil"
"k8s.io/test-infra/prow/git/v2"
"k8s.io/test-infra/prow/pluginhelp/externalplugins"
)

@@ -117,7 +118,7 @@ func main() {
botName: botName,
email: email,

gc: gitClient,
gc: git.ClientFactoryFrom(gitClient),
ghc: githubClient,
log: log,

@@ -30,7 +30,7 @@ import (

"github.com/sirupsen/logrus"

"k8s.io/test-infra/prow/git"
"k8s.io/test-infra/prow/git/v2"
"k8s.io/test-infra/prow/github"
"k8s.io/test-infra/prow/pluginhelp"
"k8s.io/test-infra/prow/plugins"
@@ -79,7 +79,7 @@ type Server struct {
botName string
email string

gc *git.Client
gc git.ClientFactory
// Used for unit testing
push func(newBranch string) error
ghc githubClient
@@ -358,7 +358,7 @@ func (s *Server) handle(l *logrus.Entry, requestor string, comment *github.Issue

// Clone the repo, checkout the target branch.
startClone := time.Now()
r, err := s.gc.Clone(org, repo)
r, err := s.gc.ClientFor(org, repo)
if err != nil {
return err
}
@@ -422,12 +422,12 @@ func (s *Server) handle(l *logrus.Entry, requestor string, comment *github.Issue
return s.createComment(org, repo, num, comment, resp)
}

push := r.Push
push := r.ForcePush
if s.push != nil {
push = s.push
}
// Push the new branch in the bot's fork.
if err := push(repo, newBranch); err != nil {
if err := push(newBranch); err != nil {
resp := fmt.Sprintf("failed to push cherry-picked changes in GitHub: %v", err)
s.log.WithFields(l.Data).Info(resp)
return s.createComment(org, repo, num, comment, resp)
@@ -51,7 +51,7 @@ func TestClone(t *testing.T) {
}

// Fresh clone, will be a cache miss.
r1, err := c.Clone("foo", "bar")
r1, err := c.ClientFor("foo", "bar")
if err != nil {
t.Fatalf("Cloning the first time: %v", err)
}
@@ -62,7 +62,7 @@ func TestClone(t *testing.T) {
}()

// Clone from the same org.
r2, err := c.Clone("foo", "baz")
r2, err := c.ClientFor("foo", "baz")
if err != nil {
t.Fatalf("Cloning another repo in the same org: %v", err)
}
@@ -76,7 +76,7 @@ func TestClone(t *testing.T) {
if err := lg.AddCommit("foo", "bar", map[string][]byte{"second": {}}); err != nil {
t.Fatalf("Adding second commit: %v", err)
}
r3, err := c.Clone("foo", "bar")
r3, err := c.ClientFor("foo", "bar")
if err != nil {
t.Fatalf("Cloning a second time: %v", err)
}
@@ -113,7 +113,7 @@ func TestCheckoutPR(t *testing.T) {
if err := lg.MakeFakeRepo("foo", "bar"); err != nil {
t.Fatalf("Making fake repo: %v", err)
}
r, err := c.Clone("foo", "bar")
r, err := c.ClientFor("foo", "bar")
if err != nil {
t.Fatalf("Cloning: %v", err)
}
@@ -154,7 +154,7 @@ func TestMergeCommitsExistBetween(t *testing.T) {
if err := lg.MakeFakeRepo("foo", "bar"); err != nil {
t.Fatalf("Making fake repo: %v", err)
}
r, err := c.Clone("foo", "bar")
r, err := c.ClientFor("foo", "bar")
if err != nil {
t.Fatalf("Cloning: %v", err)
}
@@ -368,7 +368,7 @@ func TestMergeAndCheckout(t *testing.T) {
}
}

clonedRepo, err := c.Clone(org, repo)
clonedRepo, err := c.ClientFor(org, repo)
if err != nil {
t.Fatalf("Cloning failed: %v", err)
}
@@ -382,7 +382,7 @@ func TestMergeAndCheckout(t *testing.T) {
t.Fatalf("failed to disable gpg signing for test repo: %v", err)
}

err = clonedRepo.MergeAndCheckout(baseSHA, commitsToMerge, tc.mergeStrategy)
err = clonedRepo.MergeAndCheckout(baseSHA, commitsToMerge, string(tc.mergeStrategy))
if err == nil && tc.err == "" {
return
}
@@ -9,7 +9,10 @@ go_library(
name = "go_default_library",
srcs = ["localgit.go"],
importpath = "k8s.io/test-infra/prow/git/localgit",
deps = ["//prow/git:go_default_library"],
deps = [
"//prow/git:go_default_library",
"//prow/git/v2:go_default_library",
],
)

filegroup(

0 comments on commit ed4f8bb

Please sign in to comment.
You can’t perform that action at this time.