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.

Sign up for GitHub

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

Jump to bottom

git: refactor to use v1 clients adapted to v2 interfaces #15364

Merged
merged 2 commits into from
Dec 11, 2019
Merged

git: refactor to use v1 clients adapted to v2 interfaces #15364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b039de2
Revert "Merge pull request #15562 from stevekuznetsov/skuznets/cherry…
stevekuznetsov Dec 11, 2019
ee1f9ba
git: refactor to use v1 clients adapted to v2 interfaces
stevekuznetsov Nov 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions prow/cmd/deck/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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 @@ -538,7 +538,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 @@ -548,10 +548,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)
} else {
if len(cfg().InRepoConfig.Enabled) > 0 {
logrus.Fatal("--github-token-path must be configured with a valid token when using the inrepoconfig feature")
Expand Down Expand Up @@ -670,7 +671,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 @@ -857,7 +858,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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,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 @@ -203,7 +203,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 @@ -248,7 +248,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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"k8s.io/test-infra/prow/config"
"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/hook"
"k8s.io/test-infra/prow/logrusutil"
"k8s.io/test-infra/prow/metrics"
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"k8s.io/test-infra/prow/config"
"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/logrusutil"
"k8s.io/test-infra/prow/metrics"
"k8s.io/test-infra/prow/pjutil"
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,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 @@ -267,7 +267,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/sirupsen/logrus"
utilerrors "k8s.io/apimachinery/pkg/util/errors"

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

Expand All @@ -27,14 +27,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 @@ -52,7 +52,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 @@ -74,7 +74,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 {
stevekuznetsov marked this conversation as resolved.
Show resolved Hide resolved
return nil, fmt.Errorf("failed to merge: %v", err)
}

Expand Down
4 changes: 2 additions & 2 deletions prow/config/tide.go
View file
Open in desktop
Original file line number Diff line number Diff line change
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
9 changes: 5 additions & 4 deletions prow/config/tide_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ import (

"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/test-infra/prow/git"
utilpointer "k8s.io/utils/pointer"

"k8s.io/test-infra/prow/git/v2"
"k8s.io/test-infra/prow/github"
"k8s.io/test-infra/prow/labels"
utilpointer "k8s.io/utils/pointer"
)

var testQuery = TideQuery{
Expand Down Expand Up @@ -580,7 +581,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 +1035,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
1 change: 0 additions & 1 deletion prow/external-plugins/cherrypicker/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ go_test(
embed = [":go_default_library"],
deps = [
"//prow/git/localgit:go_default_library",
"//prow/git/v2:go_default_library",
"//prow/github:go_default_library",
"@com_github_sirupsen_logrus//:go_default_library",
],
Expand Down
7 changes: 3 additions & 4 deletions prow/external-plugins/cherrypicker/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package main

import (
"fmt"
"k8s.io/test-infra/prow/git/v2"
"sync"
"testing"

Expand Down Expand Up @@ -235,7 +234,7 @@ func TestCherryPickIC(t *testing.T) {

s := &Server{
botName: botName,
gc: git.ClientFactoryFrom(c),
gc: c,
push: func(newBranch string) error { return nil },
ghc: ghc,
tokenGenerator: getSecret,
Expand Down Expand Up @@ -373,7 +372,7 @@ func TestCherryPickPR(t *testing.T) {

s := &Server{
botName: botName,
gc: git.ClientFactoryFrom(c),
gc: c,
push: func(newBranch string) error { return nil },
ghc: ghc,
tokenGenerator: getSecret,
Expand Down Expand Up @@ -514,7 +513,7 @@ func TestCherryPickPRWithLabels(t *testing.T) {

s := &Server{
botName: botName,
gc: git.ClientFactoryFrom(c),
gc: c,
push: func(newBranch string) error { return nil },
ghc: ghc,
tokenGenerator: getSecret,
Expand Down
Loading