Skip to content

Commit 8840058

Browse files
authored
Merge pull request #14830 from alvaroaleman/tide-index-prowjobs
Tide: Use an index on the lister to filter prowjobs for subpool
2 parents 18fa563 + 67f02d0 commit 8840058

File tree

8 files changed

+295
-52
lines changed

8 files changed

+295
-52
lines changed

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ require (
4040
github.com/go-logr/zapr v0.1.1 // indirect
4141
github.com/go-openapi/spec v0.19.2
4242
github.com/go-sql-driver/mysql v0.0.0-20160411075031-7ebe0a500653 // indirect
43+
github.com/go-test/deep v1.0.4
4344
github.com/gobuffalo/envy v1.6.15 // indirect
4445
github.com/gogo/protobuf v1.2.1 // indirect
4546
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@ github.com/go-openapi/validate v0.18.0/go.mod h1:Uh4HdOzKt19xGIGm1qHf/ofbX1YQ4Y+
165165
github.com/go-sql-driver/mysql v0.0.0-20160411075031-7ebe0a500653 h1:cRYCw9putX9pmpNdGkE/rWrY6gNGqZvdgGT1RPl6K18=
166166
github.com/go-sql-driver/mysql v0.0.0-20160411075031-7ebe0a500653/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w=
167167
github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
168+
github.com/go-test/deep v1.0.4 h1:u2CU3YKy9I2pmu9pX0eq50wCgjfGIt539SqR7FbHiho=
169+
github.com/go-test/deep v1.0.4/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA=
168170
github.com/go-yaml/yaml v2.1.0+incompatible h1:RYi2hDdss1u4YE7GwixGzWwVo47T8UQwnTLB6vQiq+o=
169171
github.com/go-yaml/yaml v2.1.0+incompatible/go.mod h1:w2MrLa16VYP0jy6N7M5kHaCkaLENm+P+Tv+MfurjSw0=
170172
github.com/gobuffalo/envy v1.6.5 h1:X3is06x7v0nW2xiy2yFbbIjwHz57CD6z6MkvqULTCm8=

prow/cmd/tide/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ go_library(
1515
deps = [
1616
"//pkg/flagutil:go_default_library",
1717
"//pkg/io:go_default_library",
18-
"//prow/apis/prowjobs/v1:go_default_library",
1918
"//prow/config:go_default_library",
2019
"//prow/config/secret:go_default_library",
2120
"//prow/flagutil:go_default_library",

prow/cmd/tide/main.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030

3131
"k8s.io/test-infra/pkg/flagutil"
3232
"k8s.io/test-infra/pkg/io"
33-
prowjobsv1 "k8s.io/test-infra/prow/apis/prowjobs/v1"
3433
"k8s.io/test-infra/prow/config"
3534
"k8s.io/test-infra/prow/config/secret"
3635
prowflagutil "k8s.io/test-infra/prow/flagutil"
@@ -171,26 +170,21 @@ func main() {
171170
if err != nil {
172171
logrus.WithError(err).Fatal("Error constructing mgr.")
173172
}
174-
// Make sure the manager creates a cache for ProwJobs by requesting an informer
175-
if _, err := mgr.GetCache().GetInformer(&prowjobsv1.ProwJob{}); err != nil {
176-
logrus.WithError(err).Fatal("Error getting ProwJob informer.")
173+
c, err := tide.NewController(githubSync, githubStatus, mgr, cfg, gitClient, o.maxRecordsPerPool, opener, o.historyURI, o.statusURI, nil)
174+
if err != nil {
175+
logrus.WithError(err).Fatal("Error creating Tide controller.")
177176
}
178-
179177
interrupts.Run(func(ctx context.Context) {
180178
if err := mgr.Start(ctx.Done()); err != nil {
181179
logrus.WithError(err).Fatal("Mgr failed.")
182180
}
181+
logrus.Info("Mgr finished gracefully.")
183182
})
184183
mgrSyncCtx, mgrSyncCtxCancel := context.WithTimeout(context.Background(), 30*time.Second)
185184
defer mgrSyncCtxCancel()
186185
if synced := mgr.GetCache().WaitForCacheSync(mgrSyncCtx.Done()); !synced {
187186
logrus.Fatal("Timed out waiting for cachesync")
188187
}
189-
190-
c, err := tide.NewController(githubSync, githubStatus, mgr.GetClient(), cfg, gitClient, o.maxRecordsPerPool, opener, o.historyURI, o.statusURI, nil)
191-
if err != nil {
192-
logrus.WithError(err).Fatal("Error creating Tide controller.")
193-
}
194188
interrupts.OnInterrupt(func() {
195189
c.Shutdown()
196190
if err := gitClient.Clean(); err != nil {

prow/tide/BUILD.bazel

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ go_library(
2323
"@com_github_shurcool_githubv4//:go_default_library",
2424
"@com_github_sirupsen_logrus//:go_default_library",
2525
"@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library",
26+
"@io_k8s_apimachinery//pkg/runtime:go_default_library",
2627
"@io_k8s_apimachinery//pkg/util/sets:go_default_library",
2728
"@io_k8s_sigs_controller_runtime//pkg/client:go_default_library",
2829
"@io_k8s_sigs_yaml//:go_default_library",
@@ -63,12 +64,15 @@ go_test(
6364
"//prow/github:go_default_library",
6465
"//prow/tide/blockers:go_default_library",
6566
"//prow/tide/history:go_default_library",
67+
"@com_github_go_test_deep//:go_default_library",
6668
"@com_github_shurcool_githubv4//:go_default_library",
6769
"@com_github_sirupsen_logrus//:go_default_library",
6870
"@io_k8s_apimachinery//pkg/api/equality:go_default_library",
6971
"@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library",
72+
"@io_k8s_apimachinery//pkg/runtime:go_default_library",
7073
"@io_k8s_apimachinery//pkg/util/diff:go_default_library",
7174
"@io_k8s_apimachinery//pkg/util/sets:go_default_library",
75+
"@io_k8s_sigs_controller_runtime//pkg/client:go_default_library",
7276
"@io_k8s_sigs_controller_runtime//pkg/client/fake:go_default_library",
7377
],
7478
)

prow/tide/tide.go

Lines changed: 67 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/prometheus/client_golang/prometheus"
3434
githubql "github.com/shurcooL/githubv4"
3535
"github.com/sirupsen/logrus"
36+
"k8s.io/apimachinery/pkg/runtime"
3637
"k8s.io/apimachinery/pkg/util/sets"
3738
ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
3839

@@ -203,15 +204,21 @@ func init() {
203204
prometheus.MustRegister(tideMetrics.poolErrors)
204205
}
205206

207+
type manager interface {
208+
GetClient() ctrlruntimeclient.Client
209+
GetFieldIndexer() ctrlruntimeclient.FieldIndexer
210+
}
211+
206212
// NewController makes a Controller out of the given clients.
207-
func NewController(ghcSync, ghcStatus github.Client, prowJobClient ctrlruntimeclient.Client, cfg config.Getter, gc *git.Client, maxRecordsPerPool int, opener io.Opener, historyURI, statusURI string, logger *logrus.Entry) (*Controller, error) {
213+
func NewController(ghcSync, ghcStatus github.Client, mgr manager, cfg config.Getter, gc *git.Client, maxRecordsPerPool int, opener io.Opener, historyURI, statusURI string, logger *logrus.Entry) (*Controller, error) {
208214
if logger == nil {
209215
logger = logrus.NewEntry(logrus.StandardLogger())
210216
}
211217
hist, err := history.New(maxRecordsPerPool, opener, historyURI)
212218
if err != nil {
213219
return nil, fmt.Errorf("error initializing history client from %q: %v", historyURI, err)
214220
}
221+
215222
sc := &statusController{
216223
logger: logger.WithField("controller", "status-update"),
217224
ghc: ghcStatus,
@@ -223,11 +230,30 @@ func NewController(ghcSync, ghcStatus github.Client, prowJobClient ctrlruntimecl
223230
path: statusURI,
224231
}
225232
go sc.run()
233+
return newSyncController(logger, ghcSync, mgr, cfg, gc, sc, hist)
234+
}
235+
236+
func newSyncController(
237+
logger *logrus.Entry,
238+
ghcSync githubClient,
239+
mgr manager,
240+
cfg config.Getter,
241+
gc *git.Client,
242+
sc *statusController,
243+
hist *history.History,
244+
) (*Controller, error) {
245+
if err := mgr.GetFieldIndexer().IndexField(
246+
&prowapi.ProwJob{},
247+
cacheIndexName,
248+
cacheIndexFunc,
249+
); err != nil {
250+
return nil, fmt.Errorf("failed to add baseSHA index to cache: %v", err)
251+
}
226252
return &Controller{
227253
ctx: context.Background(),
228254
logger: logger.WithField("controller", "sync"),
229255
ghc: ghcSync,
230-
prowJobClient: prowJobClient,
256+
prowJobClient: mgr.GetClient(),
231257
config: cfg,
232258
gc: gc,
233259
sc: sc,
@@ -308,19 +334,9 @@ func (c *Controller) Sync() error {
308334
"duration", time.Since(start).String(),
309335
).Debugf("Found %d (unfiltered) pool PRs.", len(prs))
310336

311-
var pjs []prowapi.ProwJob
312337
var blocks blockers.Blockers
313338
var err error
314339
if len(prs) > 0 {
315-
start := time.Now()
316-
pjList := &prowapi.ProwJobList{}
317-
if err := c.prowJobClient.List(c.ctx, pjList, ctrlruntimeclient.InNamespace(c.config().ProwJobNamespace)); err != nil {
318-
c.logger.WithField("duration", time.Since(start).String()).Debug("Failed to list ProwJobs from the cluster.")
319-
return err
320-
}
321-
c.logger.WithField("duration", time.Since(start).String()).Debug("Listed ProwJobs from the cluster.")
322-
pjs = pjList.Items
323-
324340
if label := c.config().Tide.BlockerLabel; label != "" {
325341
c.logger.Debugf("Searching for blocking issues (label %q).", label)
326342
orgExcepts, repos := c.config().Tide.Queries.OrgExceptionsAndRepos()
@@ -336,7 +352,7 @@ func (c *Controller) Sync() error {
336352
}
337353
}
338354
// Partition PRs into subpools and filter out non-pool PRs.
339-
rawPools, err := c.dividePool(prs, pjs)
355+
rawPools, err := c.dividePool(prs)
340356
if err != nil {
341357
return err
342358
}
@@ -1382,7 +1398,7 @@ func poolKey(org, repo, branch string) string {
13821398

13831399
// dividePool splits up the list of pull requests and prow jobs into a group
13841400
// per repo and branch. It only keeps ProwJobs that match the latest branch.
1385-
func (c *Controller) dividePool(pool map[string]PullRequest, pjs []prowapi.ProwJob) (map[string]*subpool, error) {
1401+
func (c *Controller) dividePool(pool map[string]PullRequest) (map[string]*subpool, error) {
13861402
sps := make(map[string]*subpool)
13871403
for _, pr := range pool {
13881404
org := string(pr.Repository.Owner.Login)
@@ -1410,15 +1426,19 @@ func (c *Controller) dividePool(pool map[string]PullRequest, pjs []prowapi.ProwJ
14101426
}
14111427
sps[fn].prs = append(sps[fn].prs, pr)
14121428
}
1413-
for _, pj := range pjs {
1414-
if pj.Spec.Type != prowapi.PresubmitJob && pj.Spec.Type != prowapi.BatchJob {
1415-
continue
1416-
}
1417-
fn := poolKey(pj.Spec.Refs.Org, pj.Spec.Refs.Repo, pj.Spec.Refs.BaseRef)
1418-
if sps[fn] == nil || pj.Spec.Refs.BaseSHA != sps[fn].sha {
1419-
continue
1429+
1430+
for subpoolkey, sp := range sps {
1431+
pjs := &prowapi.ProwJobList{}
1432+
err := c.prowJobClient.List(
1433+
c.ctx,
1434+
pjs,
1435+
ctrlruntimeclient.MatchingField(cacheIndexName, cacheIndexKey(sp.org, sp.repo, sp.branch, sp.sha)),
1436+
ctrlruntimeclient.InNamespace(c.config().ProwJobNamespace))
1437+
if err != nil {
1438+
return nil, fmt.Errorf("failed to list jobs for subpool %s: %v", subpoolkey, err)
14201439
}
1421-
sps[fn].pjs = append(sps[fn].pjs, pj)
1440+
c.logger.WithField("subpool", subpoolkey).Debugf("Found %d prowjobs.", len(pjs.Items))
1441+
sps[subpoolkey].pjs = pjs.Items
14221442
}
14231443
return sps, nil
14241444
}
@@ -1571,3 +1591,28 @@ func orgRepoQueryString(orgs, repos []string, orgExceptions map[string]sets.Stri
15711591
}
15721592
return strings.Join(toks, " ")
15731593
}
1594+
1595+
// cacheIndexName is the name of the index that indexes presubmit+batch ProwJobs by
1596+
// org+repo+branch+baseSHA. Use the cacheIndexKey func to get the correct key.
1597+
const cacheIndexName = "tide-global-index"
1598+
1599+
// cacheIndexKey returns the index key for the tideCacheIndex
1600+
func cacheIndexKey(org, repo, branch, baseSHA string) string {
1601+
return fmt.Sprintf("%s/%s:%s@%s", org, repo, branch, baseSHA)
1602+
}
1603+
1604+
func cacheIndexFunc(obj runtime.Object) []string {
1605+
pj, ok := obj.(*prowapi.ProwJob)
1606+
// Should never happen
1607+
if !ok {
1608+
return nil
1609+
}
1610+
// We do not care about jobs other than presubmit and batch
1611+
if pj.Spec.Type != prowapi.PresubmitJob && pj.Spec.Type != prowapi.BatchJob {
1612+
return nil
1613+
}
1614+
if pj.Spec.Refs == nil {
1615+
return nil
1616+
}
1617+
return []string{cacheIndexKey(pj.Spec.Refs.Org, pj.Spec.Refs.Repo, pj.Spec.Refs.BaseRef, pj.Spec.Refs.BaseSHA)}
1618+
}

0 commit comments

Comments
 (0)