Skip to content
This repository was archived by the owner on Jun 14, 2019. It is now read-only.

Limit rehearsals to 15 jobs #78

Merged
openshift-merge-robot merged 1 commit into from
Feb 14, 2019
Merged

Limit rehearsals to 15 jobs #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 13 additions & 4 deletions cmd/pj-rehearse/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,8 @@ type options struct {
noFail bool
debugLogPath string

configPath string
jobConfigPath string

candidatePath string
candidatePath string
rehearsalLimit int
}

func gatherOptions() options {
Expand All @@ -60,6 +58,7 @@ func gatherOptions() options {
fs.StringVar(&o.debugLogPath, "debug-log", "", "Alternate file for debug output, defaults to stderr")

fs.StringVar(&o.candidatePath, "candidate-path", "", "Path to a openshift/release working copy with a revision to be tested")
fs.IntVar(&o.rehearsalLimit, "rehearsal-limit", 15, "Upper limit of jobs attempted to rehearse (if more jobs would be rehearsed, none will)")

fs.Parse(os.Args[1:])
return o
Expand Down Expand Up @@ -165,6 +164,16 @@ func main() {
loggers := rehearse.Loggers{Job: logger, Debug: debugLogger.WithField(prowgithub.PrLogField, prNumber)}

executor := rehearse.NewExecutor(changedPresubmits, prNumber, o.candidatePath, jobSpec.Refs, o.dryRun, loggers, pjclient)

if executor.RehearsalJobCount > o.rehearsalLimit {
jobCountFields := logrus.Fields{
"rehearsal-threshold": o.rehearsalLimit,
"rehearsal-jobs": executor.RehearsalJobCount,
}
logger.WithFields(jobCountFields).Info("Would rehearse too many jobs, will not proceed")
os.Exit(0)
}

success, err := executor.ExecuteJobs()
if err != nil {
logger.WithError(err).Error("Failed to rehearse jobs")
Expand Down
4 changes: 4 additions & 0 deletions pkg/rehearse/jobs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@ func configureRehearsalJobs(toBeRehearsed map[string][]prowconfig.Presubmit, prR

// Executor holds all the information needed for the jobs to be executed.
type Executor struct {
RehearsalJobCount int

dryRun bool
rehearsals []*prowconfig.Presubmit
prNumber int
Expand All @@ -220,6 +222,8 @@ func NewExecutor(toBeRehearsed map[string][]prowconfig.Presubmit, prNumber int,
dryRun bool, loggers Loggers, pjclient pj.ProwJobInterface) *Executor {
rehearsals := configureRehearsalJobs(toBeRehearsed, prRepo, prNumber, loggers)
Copy link
Member

@droslean droslean Feb 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we add more stuff to the constructor, I am starting thinking that it's better to export this function and call it from main and pass the rehearsals to the constructor directly. Then we don't have to include the RehearsalJobCount in the executor and we can make the limit check in main without creating the executor.

Copy link
Member Author

@petr-muller petr-muller Feb 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think when we have an extra layer already, let's use it to hide some details.

return &Executor{
RehearsalJobCount: len(rehearsals),

Copy link
Member

@droslean droslean Feb 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: no need for the extra line here

Copy link
Member Author

@petr-muller petr-muller Feb 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to separate private and exported fields

dryRun: dryRun,
rehearsals: rehearsals,
prNumber: prNumber,
Expand Down