豆豆友情提示：这是一个非官方 GitHub 代理镜像，主要用于网络测试或访问加速。请勿在此进行登录、注册或处理任何敏感信息。进行这些操作请务必访问官方网站 github.com。 Raw 内容也通过此代理提供。
Skip to content
This repository was archived by the owner on Mar 3, 2026. It is now read-only.

Commit 4612148

Browse files
Langleumumoshu
authored and
unpollito
committed
fix: remove callbacks resulting in scales due to incomplete response (actions#2671)
Co-authored-by: Yusuke Kuoka <ykuoka@gmail.com>
1 parent 3e7caac commit 4612148

File tree

2 files changed

+71
-19
lines changed

2 files changed

+71
-19
lines changed

‎controllers/actions.summerwind.net/autoscaling.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,10 @@ func (r *HorizontalRunnerAutoscalerReconciler) suggestReplicasByQueuedAndInProgr
118118
}
119119

120120
var total, inProgress, queued, completed, unknown int
121-
type callback func()
122-
listWorkflowJobs := func(user string, repoName string, runID int64, fallback_cb callback) {
121+
listWorkflowJobs := func(user string, repoName string, runID int64) {
123122
if runID == 0 {
124-
fallback_cb()
123+
// should not happen in reality
124+
r.Log.Info("Detected run with no runID of 0, ignoring the case and not scaling.", "repo_name", repoName, "run_id", runID)
125125
return
126126
}
127127
opt := github.ListWorkflowJobsOptions{ListOptions: github.ListOptions{PerPage: 50}}
@@ -139,7 +139,8 @@ func (r *HorizontalRunnerAutoscalerReconciler) suggestReplicasByQueuedAndInProgr
139139
opt.Page = resp.NextPage
140140
}
141141
if len(allJobs) == 0 {
142-
fallback_cb()
142+
// GitHub API can return run with empty job array - should be ignored
143+
r.Log.Info("Detected run with no jobs, ignoring the case and not scaling.", "repo_name", repoName, "run_id", runID)
143144
} else {
144145
JOB:
145146
for _, job := range allJobs {
@@ -201,9 +202,9 @@ func (r *HorizontalRunnerAutoscalerReconciler) suggestReplicasByQueuedAndInProgr
201202
case "completed":
202203
completed++
203204
case "in_progress":
204-
listWorkflowJobs(user, repoName, run.GetID(), func() { inProgress++ })
205+
listWorkflowJobs(user, repoName, run.GetID())
205206
case "queued":
206-
listWorkflowJobs(user, repoName, run.GetID(), func() { queued++ })
207+
listWorkflowJobs(user, repoName, run.GetID())
207208
default:
208209
unknown++
209210
}

‎controllers/actions.summerwind.net/autoscaling_test.go‎

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -61,30 +61,38 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
6161
want int
6262
err string
6363
}{
64+
// case_0
6465
// Legacy functionality
65-
// 3 demanded, max at 3
66+
// 0 demanded due to zero runID, min at 2
6667
{
6768
repo: "test/valid",
6869
min: intPtr(2),
6970
max: intPtr(3),
7071
workflowRuns: `{"total_count": 4, "workflow_runs":[{"status":"queued"}, {"status":"in_progress"}, {"status":"in_progress"}, {"status":"completed"}]}"`,
7172
workflowRuns_queued: `{"total_count": 1, "workflow_runs":[{"status":"queued"}]}"`,
7273
workflowRuns_in_progress: `{"total_count": 2, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}]}"`,
73-
want: 3,
74+
want: 2,
7475
},
75-
// Explicitly speified the default `self-hosted` label which is ignored by the simulator,
76+
// case_1
77+
// Explicitly specified the default `self-hosted` label which is ignored by the simulator,
7678
// as we assume that GitHub Actions automatically associates the `self-hosted` label to every self-hosted runner.
7779
// 3 demanded, max at 3
7880
{
7981
repo: "test/valid",
8082
labels: []string{"self-hosted"},
8183
min: intPtr(2),
8284
max: intPtr(3),
83-
workflowRuns: `{"total_count": 4, "workflow_runs":[{"status":"queued"}, {"status":"in_progress"}, {"status":"in_progress"}, {"status":"completed"}]}"`,
84-
workflowRuns_queued: `{"total_count": 1, "workflow_runs":[{"status":"queued"}]}"`,
85-
workflowRuns_in_progress: `{"total_count": 2, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}]}"`,
86-
want: 3,
85+
workflowRuns: `{"total_count": 4, "workflow_runs":[{"id": 1, "status":"queued"}, {"id": 2, "status":"in_progress"}, {"id": 3, "status":"in_progress"}, {"status":"completed"}]}"`,
86+
workflowRuns_queued: `{"total_count": 1, "workflow_runs":[{"id": 1, "status":"queued"}]}"`,
87+
workflowRuns_in_progress: `{"total_count": 2, "workflow_runs":[{"id": 2, "status":"in_progress"}, {"id": 3, "status":"in_progress"}]}"`,
88+
workflowJobs: map[int]string{
89+
1: `{"jobs": [{"status": "queued", "labels":["self-hosted"]}]}`,
90+
2: `{"jobs": [{"status": "in_progress", "labels":["self-hosted"]}]}`,
91+
3: `{"jobs": [{"status": "in_progress", "labels":["self-hosted"]}]}`,
92+
},
93+
want: 3,
8794
},
95+
// case_2
8896
// 2 demanded, max at 3, currently 3, delay scaling down due to grace period
8997
{
9098
repo: "test/valid",
@@ -97,6 +105,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
97105
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
98106
want: 3,
99107
},
108+
// case_3
100109
// 3 demanded, max at 2
101110
{
102111
repo: "test/valid",
@@ -107,6 +116,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
107116
workflowRuns_in_progress: `{"total_count": 2, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}]}"`,
108117
want: 2,
109118
},
119+
// case_4
110120
// 2 demanded, min at 2
111121
{
112122
repo: "test/valid",
@@ -117,6 +127,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
117127
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
118128
want: 2,
119129
},
130+
// case_5
120131
// 1 demanded, min at 2
121132
{
122133
repo: "test/valid",
@@ -127,6 +138,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
127138
workflowRuns_in_progress: `{"total_count": 0, "workflow_runs":[]}"`,
128139
want: 2,
129140
},
141+
// case_6
130142
// 1 demanded, min at 2
131143
{
132144
repo: "test/valid",
@@ -137,6 +149,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
137149
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
138150
want: 2,
139151
},
152+
// case_7
140153
// 1 demanded, min at 1
141154
{
142155
repo: "test/valid",
@@ -147,6 +160,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
147160
workflowRuns_in_progress: `{"total_count": 0, "workflow_runs":[]}"`,
148161
want: 1,
149162
},
163+
// case_8
150164
// 1 demanded, min at 1
151165
{
152166
repo: "test/valid",
@@ -157,6 +171,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
157171
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
158172
want: 1,
159173
},
174+
// case_9
160175
// fixed at 3
161176
{
162177
repo: "test/valid",
@@ -166,9 +181,36 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
166181
workflowRuns: `{"total_count": 4, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}, {"status":"in_progress"}, {"status":"completed"}]}"`,
167182
workflowRuns_queued: `{"total_count": 0, "workflow_runs":[]}"`,
168183
workflowRuns_in_progress: `{"total_count": 3, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}, {"status":"in_progress"}]}"`,
169-
want: 3,
184+
want: 1,
185+
},
186+
// Case for empty GitHub Actions reponse - should not trigger scale up
187+
{
188+
description: "GitHub Actions Jobs Array is empty - no scale up",
189+
repo: "test/valid",
190+
min: intPtr(0),
191+
max: intPtr(3),
192+
workflowRuns: `{"total_count": 2, "workflow_runs":[{"status":"queued"}, {"status":"completed"}]}"`,
193+
workflowRuns_queued: `{"total_count": 1, "workflow_runs":[{"status":"queued"}]}"`,
194+
workflowRuns_in_progress: `{"total_count": 0, "workflow_runs":[]}"`,
195+
workflowJobs: map[int]string{
196+
1: `{"jobs": []}`,
197+
},
198+
want: 0,
199+
},
200+
// Case for hosted GitHub Actions run
201+
{
202+
description: "Hosted GitHub Actions run - no scale up",
203+
repo: "test/valid",
204+
min: intPtr(0),
205+
max: intPtr(3),
206+
workflowRuns: `{"total_count": 2, "workflow_runs":[{"id": 1, "status":"queued"}, {"status":"completed"}]}"`,
207+
workflowRuns_queued: `{"total_count": 1, "workflow_runs":[{"id": 1, "status":"queued"}]}"`,
208+
workflowRuns_in_progress: `{"total_count": 0, "workflow_runs":[]}"`,
209+
workflowJobs: map[int]string{
210+
1: `{"jobs": [{"status":"queued"}]}`,
211+
},
212+
want: 0,
170213
},
171-
172214
{
173215
description: "Job-level autoscaling with no explicit runner label (runners have implicit self-hosted, requested self-hosted, 5 jobs from 3 workflows)",
174216
repo: "test/valid",
@@ -422,7 +464,8 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
422464
want int
423465
err string
424466
}{
425-
// 3 demanded, max at 3
467+
// case_0
468+
// 0 demanded due to zero runID, min at 2
426469
{
427470
org: "test",
428471
repos: []string{"valid"},
@@ -431,8 +474,9 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
431474
workflowRuns: `{"total_count": 4, "workflow_runs":[{"status":"queued"}, {"status":"in_progress"}, {"status":"in_progress"}, {"status":"completed"}]}"`,
432475
workflowRuns_queued: `{"total_count": 1, "workflow_runs":[{"status":"queued"}]}"`,
433476
workflowRuns_in_progress: `{"total_count": 2, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}]}"`,
434-
want: 3,
477+
want: 2,
435478
},
479+
// case_1
436480
// 2 demanded, max at 3, currently 3, delay scaling down due to grace period
437481
{
438482
org: "test",
@@ -446,6 +490,7 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
446490
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
447491
want: 3,
448492
},
493+
// case_2
449494
// 3 demanded, max at 2
450495
{
451496
org: "test",
@@ -457,6 +502,7 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
457502
workflowRuns_in_progress: `{"total_count": 2, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}]}"`,
458503
want: 2,
459504
},
505+
// case_3
460506
// 2 demanded, min at 2
461507
{
462508
org: "test",
@@ -468,6 +514,7 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
468514
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
469515
want: 2,
470516
},
517+
// case_4
471518
// 1 demanded, min at 2
472519
{
473520
org: "test",
@@ -479,6 +526,7 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
479526
workflowRuns_in_progress: `{"total_count": 0, "workflow_runs":[]}"`,
480527
want: 2,
481528
},
529+
// case_5
482530
// 1 demanded, min at 2
483531
{
484532
org: "test",
@@ -512,6 +560,7 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
512560
workflowRuns_in_progress: `{"total_count": 1, "workflow_runs":[{"status":"in_progress"}]}"`,
513561
want: 1,
514562
},
563+
// case_6
515564
// fixed at 3
516565
{
517566
org: "test",
@@ -522,8 +571,9 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
522571
workflowRuns: `{"total_count": 4, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}, {"status":"in_progress"}, {"status":"completed"}]}"`,
523572
workflowRuns_queued: `{"total_count": 0, "workflow_runs":[]}"`,
524573
workflowRuns_in_progress: `{"total_count": 3, "workflow_runs":[{"status":"in_progress"},{"status":"in_progress"},{"status":"in_progress"}]}"`,
525-
want: 3,
574+
want: 1,
526575
},
576+
// case_7
527577
// org runner, fixed at 3
528578
{
529579
org: "test",
@@ -534,8 +584,9 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
534584
workflowRuns: `{"total_count": 4, "workflow_runs":[{"status":"in_progress"}, {"status":"in_progress"}, {"status":"in_progress"}, {"status":"completed"}]}"`,
535585
workflowRuns_queued: `{"total_count": 0, "workflow_runs":[]}"`,
536586
workflowRuns_in_progress: `{"total_count": 3, "workflow_runs":[{"status":"in_progress"},{"status":"in_progress"},{"status":"in_progress"}]}"`,
537-
want: 3,
587+
want: 1,
538588
},
589+
// case_8
539590
// org runner, 1 demanded, min at 1, no repos
540591
{
541592
org: "test",

0 commit comments

Comments
 (0)