From c5950d75fa5ded14e1a3cdcd476accdb141b6816 Mon Sep 17 00:00:00 2001 From: Lars Haugan Date: Fri, 24 Dec 2021 01:12:36 +0100 Subject: [PATCH] fix: pagination for ListWorkflowJobs in autoscaler (#990) (#992) Adding handling of paginated results when calling `ListWorkflowJobs`. By default the `per_page` is 30, which potentially would return 30 queued and 30 in_progress jobs. This change should enable the autoscaler to scale workflows with more than 60 jobs to the exact number of runners needed. Problem: I did not find any support for pagination in the Github fake client, and have not been able to test this (as I have not been able to push an image to an environment where I can verify this). If anyone is able to help out verifying this PR, i would really appreciate it. Resolves #990 --- controllers/autoscaling.go | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/controllers/autoscaling.go b/controllers/autoscaling.go index ea9e9c68..1b71013c 100644 --- a/controllers/autoscaling.go +++ b/controllers/autoscaling.go @@ -10,6 +10,7 @@ import ( "time" "github.com/actions-runner-controller/actions-runner-controller/api/v1alpha1" + "github.com/google/go-github/v39/github" ) const ( @@ -164,14 +165,24 @@ func (r *HorizontalRunnerAutoscalerReconciler) suggestReplicasByQueuedAndInProgr fallback_cb() return } - jobs, _, err := r.GitHubClient.Actions.ListWorkflowJobs(context.TODO(), user, repoName, runID, nil) - if err != nil { - r.Log.Error(err, "Error listing workflow jobs") - fallback_cb() - } else if len(jobs.Jobs) == 0 { + opt := github.ListWorkflowJobsOptions{ListOptions: github.ListOptions{PerPage: 50}} + var allJobs []*github.WorkflowJob + for { + jobs, resp, err := r.GitHubClient.Actions.ListWorkflowJobs(context.TODO(), user, repoName, runID, &opt) + if err != nil { + r.Log.Error(err, "Error listing workflow jobs") + return //err + } + allJobs = append(allJobs, jobs.Jobs...) + if resp.NextPage == 0 { + break + } + opt.Page = resp.NextPage + } + if len(allJobs) == 0 { fallback_cb() } else { - for _, job := range jobs.Jobs { + for _, job := range allJobs { switch job.GetStatus() { case "completed": // We add a case for `completed` so it is not counted in `unknown`.