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
This commit is contained in:
		
							parent
							
								
									de1f48111a
								
							
						
					
					
						commit
						c5950d75fa
					
				|  | @ -10,6 +10,7 @@ import ( | ||||||
| 	"time" | 	"time" | ||||||
| 
 | 
 | ||||||
| 	"github.com/actions-runner-controller/actions-runner-controller/api/v1alpha1" | 	"github.com/actions-runner-controller/actions-runner-controller/api/v1alpha1" | ||||||
|  | 	"github.com/google/go-github/v39/github" | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
| const ( | const ( | ||||||
|  | @ -164,14 +165,24 @@ func (r *HorizontalRunnerAutoscalerReconciler) suggestReplicasByQueuedAndInProgr | ||||||
| 			fallback_cb() | 			fallback_cb() | ||||||
| 			return | 			return | ||||||
| 		} | 		} | ||||||
| 		jobs, _, err := r.GitHubClient.Actions.ListWorkflowJobs(context.TODO(), user, repoName, runID, nil) | 		opt := github.ListWorkflowJobsOptions{ListOptions: github.ListOptions{PerPage: 50}} | ||||||
| 		if err != nil { | 		var allJobs []*github.WorkflowJob | ||||||
| 			r.Log.Error(err, "Error listing workflow jobs") | 		for { | ||||||
| 			fallback_cb() | 			jobs, resp, err := r.GitHubClient.Actions.ListWorkflowJobs(context.TODO(), user, repoName, runID, &opt) | ||||||
| 		} else if len(jobs.Jobs) == 0 { | 			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() | 			fallback_cb() | ||||||
| 		} else { | 		} else { | ||||||
| 			for _, job := range jobs.Jobs { | 			for _, job := range allJobs { | ||||||
| 				switch job.GetStatus() { | 				switch job.GetStatus() { | ||||||
| 				case "completed": | 				case "completed": | ||||||
| 					// We add a case for `completed` so it is not counted in `unknown`.
 | 					// We add a case for `completed` so it is not counted in `unknown`.
 | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue