Skip to content

Commit

Permalink
git: refactor to use v1 clients adapted to v2 interfaces
Browse files Browse the repository at this point in the history 
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
stevekuznetsov committed Nov 26, 2019
1 parent 59e7e9b commit fd7ce56
Show file tree
Hide file tree
Showing 49 changed files with 122 additions and 119 deletions.
3 changes: 1 addition & 2 deletions prow/cmd/deck/BUILD.bazel
View file
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
11 changes: 6 additions & 5 deletions prow/cmd/deck/main.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 == "" {
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions prow/cmd/deck/pr_history.go
View file
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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{}

Expand Down
3 changes: 1 addition & 2 deletions prow/cmd/deck/pr_history_test.go
View file
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}
Expand Down
1 change: 1 addition & 0 deletions prow/cmd/hook/BUILD.bazel
View file
Expand Up @@ -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",
Expand Down
5 changes: 3 additions & 2 deletions prow/cmd/hook/main.go
View file
Expand Up @@ -18,6 +18,7 @@ package main

import (
"flag"
"k8s.io/test-infra/prow/git/v2"
"net/http"
"os"
"strconv"
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions prow/cmd/tide/BUILD.bazel
View file
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion prow/cmd/tide/main.go
View file
Expand Up @@ -19,6 +19,7 @@ package main
import (
"context"
"flag"
"k8s.io/test-infra/prow/git/v2"
"net/http"
"os"
"strconv"
Expand Down Expand Up @@ -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.")
}
Expand Down
3 changes: 1 addition & 2 deletions prow/config/BUILD.bazel
View file
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions prow/config/config.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
}
Expand Down
4 changes: 2 additions & 2 deletions prow/config/tide.go
View file
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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...)
Expand Down
3 changes: 1 addition & 2 deletions prow/config/tide_test.go
View file
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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))
}
Expand Down
2 changes: 1 addition & 1 deletion prow/external-plugins/cherrypicker/BUILD.bazel
View file
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion prow/external-plugins/cherrypicker/main.go
View file
Expand Up @@ -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"
)

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

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

Expand Down
8 changes: 4 additions & 4 deletions prow/external-plugins/cherrypicker/server.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -422,7 +422,7 @@ 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
}
Expand Down
14 changes: 7 additions & 7 deletions prow/git/git_test.go
View file
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
}
Expand Down
5 changes: 4 additions & 1 deletion prow/git/localgit/BUILD.bazel
View file
Expand Up @@ -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(
Expand Down

0 comments on commit fd7ce56

Please sign in to comment.