Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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 Dec 3, 2019
1 parent 2e9461f commit 5a300e5
Show file tree
Hide file tree
Showing 50 changed files with 138 additions and 129 deletions.
3 changes: 1 addition & 2 deletions prow/cmd/deck/BUILD.bazel
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
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
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
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
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
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
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
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
4 changes: 2 additions & 2 deletions prow/config/BUILD.bazel
Expand Up @@ -23,8 +23,8 @@ go_test(
deps = [
"//prow/apis/prowjobs/v1:go_default_library",
"//prow/config/secret:go_default_library",
"//prow/git:go_default_library",
"//prow/git/localgit:go_default_library",
"//prow/git/v2:go_default_library",
"//prow/github:go_default_library",
"//prow/github/fakegithub:go_default_library",
"//prow/kube:go_default_library",
Expand Down Expand Up @@ -52,7 +52,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
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 @@ -266,7 +266,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, but 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
10 changes: 5 additions & 5 deletions prow/config/inrepoconfig.go
Expand Up @@ -10,7 +10,7 @@ import (

"github.com/sirupsen/logrus"

"k8s.io/test-infra/prow/git"
"k8s.io/test-infra/prow/git/v2"
"sigs.k8s.io/yaml"
)

Expand All @@ -26,14 +26,14 @@ type ProwYAML struct {

// ProwYAMLGetter is used to retrieve a ProwYAML. Tests should provide
// their own implementation and set that on the Config.
type ProwYAMLGetter func(c *Config, gc *git.Client, identifier, baseSHA string, headSHAs ...string) (*ProwYAML, error)
type ProwYAMLGetter func(c *Config, gc git.ClientFactory, identifier, baseSHA string, headSHAs ...string) (*ProwYAML, error)

// Verify defaultProwYAMLGetter is a ProwYAMLGetter
var _ ProwYAMLGetter = defaultProwYAMLGetter

func defaultProwYAMLGetter(
c *Config,
gc *git.Client,
gc git.ClientFactory,
identifier string,
baseSHA string,
headSHAs ...string) (*ProwYAML, error) {
Expand All @@ -51,7 +51,7 @@ func defaultProwYAMLGetter(
return nil, fmt.Errorf("didn't get two but %d results when splitting repo identifier %q", len(identifierSlashSplit), identifier)
}
organization, repository := identifierSlashSplit[0], identifierSlashSplit[1]
repo, err := gc.Clone(organization, repository)
repo, err := gc.ClientFor(organization, repository)
if err != nil {
return nil, fmt.Errorf("failed to clone repo for %q: %v", identifier, err)
}
Expand All @@ -73,7 +73,7 @@ func defaultProwYAMLGetter(

mergeMethod := c.Tide.MergeMethod(organization, repository)
log.Debugf("Using merge strategy %q.", mergeMethod)
if err := repo.MergeAndCheckout(baseSHA, mergeMethod, headSHAs...); err != nil {
if err := repo.MergeAndCheckout(baseSHA, string(mergeMethod), headSHAs...); err != nil {
return nil, fmt.Errorf("failed to merge: %v", err)
}

Expand Down
4 changes: 2 additions & 2 deletions prow/config/tide.go
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
6 changes: 3 additions & 3 deletions prow/config/tide_test.go
Expand Up @@ -17,13 +17,13 @@ limitations under the License.
package config

import (
"k8s.io/test-infra/prow/git/v2"
"reflect"
"strings"
"testing"

"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"
utilpointer "k8s.io/utils/pointer"
Expand Down Expand Up @@ -580,7 +580,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 Expand Up @@ -1034,7 +1034,7 @@ func TestTideContextPolicy_MissingRequiredContexts(t *testing.T) {
}

func fakeProwYAMLGetterFactory(presubmits []Presubmit) ProwYAMLGetter {
return func(_ *Config, _ *git.Client, _, _ string, _ ...string) (*ProwYAML, error) {
return func(_ *Config, _ git.ClientFactory, _, _ string, _ ...string) (*ProwYAML, error) {
return &ProwYAML{
Presubmits: presubmits,
}, nil
Expand Down
2 changes: 1 addition & 1 deletion prow/external-plugins/cherrypicker/BUILD.bazel
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
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
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

0 comments on commit 5a300e5

Please sign in to comment.