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.

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

make watch job more efficient #70

Merged
merged 2 commits into from
Oct 5, 2020
Merged
Changes from all commits
Commits
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
54 changes: 36 additions & 18 deletions cmd_watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,29 @@ func waitCircle(parent context.Context, cfg watchConfig) (passed bool, started,
default:
}

finished, failed, err := evalWorkflow(client, cfg.workflowID, cfg.jobName)
if finished {
anyRunning, anyFailed, anyBlocked, err := evalWorkflow(client, cfg.workflowID, cfg.jobName)
if !anyRunning {
// if this is the first time we think we're finished store the timestamp
if checksLeft >= numChecks {
ended = time.Now()
}

if !anyBlocked && err == nil {
// we are legit done.
passed = !anyFailed
if passed {
fmt.Println("Build passed!")
} else {
fmt.Println("Build failed!")
}
return
}

// ok, carry on
checksLeft--
if checksLeft <= 0 {
// we're done checking.
passed = !failed
passed = !anyFailed
if passed {
fmt.Println("Build passed!")
} else {
Expand All @@ -183,8 +195,8 @@ func waitCircle(parent context.Context, cfg watchConfig) (passed bool, started,
fmt.Printf("Querying the CirlceCI API failed with %s; trying %d more times before giving up.\n", err.Error(), checksLeft)
continue
}
if failed {
// don't bother rechecking if the job has failed but didn't error
if anyFailed {
// don't bother rechecking if a job has failed
fmt.Printf("Build failed!\n")
ended = time.Now()
return
Expand All @@ -198,7 +210,6 @@ func waitCircle(parent context.Context, cfg watchConfig) (passed bool, started,
// finished.
passed = false
checksLeft = numChecks

}
}()

Expand All @@ -210,39 +221,46 @@ func waitCircle(parent context.Context, cfg watchConfig) (passed bool, started,
// and decides whether the build has finished and if finished, whether it
// failed. If an error is returned, it represents an error talking to the
// CircleCI API, not an error with the workflow.
func evalWorkflow(client *circleci.Client, wfID string, jobName string) (finished bool, failed bool, err error) {
func evalWorkflow(client *circleci.Client, wfID string, jobName string) (anyRunning bool, anyFailed bool, anyBlocked bool, err error) {
fmt.Printf("%s: polling for jobs: ", time.Now().Format(time.StampMilli))
wfJobs, err := getJobs(client, wfID)
if err != nil {
fmt.Printf("error polling: %s\n", err.Error())
return true, true, err
return true, true, false, err
}
fmt.Println(summarizeJobList(wfJobs))

anyRunning = false
anyBlocked = false
for _, job := range wfJobs {
// always count ourself as finished so we don't wait for ourself
// skip ourself so we don't wait if we're the only job running
if job.Name == jobName {
continue
}

switch job.Status {
Copy link
Member

Choose a reason for hiding this comment

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

should we introduce a default stanza here to deal with unknown CircleCI states in case they introduce a new one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah good call. hm, what should that stanza actually do though? ideally we'd include it in instrumentation, so we'd need to bubble it up someplace where we can send it.

Copy link
Member

Choose a reason for hiding this comment

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

treat it as a failure state, I'd say?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that worries me, anyone using circleci + buildevents would see failed builds if the watch job turns red, as opposed to broken traces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe treat it such that we don't early exit and run through the 2 minute "we think we might be done, sleeping and trying again" loop in the hopes that it'll change to a state we recognize, and also send it up in instrumentation? we can run a trigger against our own builds that would surface it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

going to address this in another PR so we have more space/time to think about it 👍

case "success", "blocked":
// success means it passed
case "success":
// success means it finished and passed, don't keep track of it
continue
case "blocked":
// blocked means it can't yet run, but that could be because either
// it's waiting on a running job or
// it's waiting on a running job, depends on a failed job, or
// it's not configured to run this build (because of a tag or something)
anyBlocked = true
continue
case "queued":
return false, failed, nil
// queued means a job is due to start running soon, so we consider it running
// already.
anyRunning = true
case "failed":
failed = true
anyFailed = true
continue
case "running":
// We can stop short as soon as we find an unfinished job
return false, failed, nil
anyRunning = true
}
}
return true, failed, nil

return anyRunning, anyFailed, anyBlocked, nil
}

// getJobs queries the CircleCI API for a list of all jobs in the current workflow
Expand Down Expand Up @@ -279,7 +297,7 @@ func summarizeJobList(wfJobs []*circleci.WorkflowJob) string {

// sort the statuses present to print them in a consistent order
sortedStatusList := make([]string, 0, len(countByStatus))
for key, _ := range countByStatus {
for key := range countByStatus {
sortedStatusList = append(sortedStatusList, key)
}
sort.Strings(sortedStatusList)
Expand Down