Skip to content

Commit 7269130

Browse files
Zettat123Zettat123
authored
Fix missing outputs for jobs with matrix (#32823)
Fix #32795 If a job uses a matrix, multiple `ActionRunJobs` may have the same `JobID`. We need to merge the outputs of these jobs to make them available to the jobs that need them.
1 parent a66c16d commit 7269130

File tree

8 files changed

+230
-18
lines changed

8 files changed

+230
-18
lines changed

‎models/actions/run_job.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col
137137
if err != nil {
138138
return 0, err
139139
}
140-
run.Status = aggregateJobStatus(jobs)
140+
run.Status = AggregateJobStatus(jobs)
141141
if run.Started.IsZero() && run.Status.IsRunning() {
142142
run.Started = timeutil.TimeStampNow()
143143
}
@@ -152,7 +152,7 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col
152152
return affected, nil
153153
}
154154

155-
func aggregateJobStatus(jobs []*ActionRunJob) Status {
155+
func AggregateJobStatus(jobs []*ActionRunJob) Status {
156156
allDone := true
157157
allWaiting := true
158158
hasFailure := false

‎models/fixtures/action_run.yml‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,22 @@
3636
updated: 1683636626
3737
need_approval: 0
3838
approved_by: 0
39+
-
40+
id: 793
41+
title: "job output"
42+
repo_id: 4
43+
owner_id: 1
44+
workflow_id: "test.yaml"
45+
index: 189
46+
trigger_user_id: 1
47+
ref: "refs/heads/master"
48+
commit_sha: "c2d72f548424103f01ee1dc02889c1e2bff816b0"
49+
event: "push"
50+
is_fork_pull_request: 0
51+
status: 1
52+
started: 1683636528
53+
stopped: 1683636626
54+
created: 1683636108
55+
updated: 1683636626
56+
need_approval: 0
57+
approved_by: 0

‎models/fixtures/action_run_job.yml‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,46 @@
2626
status: 1
2727
started: 1683636528
2828
stopped: 1683636626
29+
-
30+
id: 194
31+
run_id: 793
32+
repo_id: 4
33+
owner_id: 1
34+
commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0
35+
is_fork_pull_request: 0
36+
name: job1 (1)
37+
attempt: 1
38+
job_id: job1
39+
task_id: 49
40+
status: 1
41+
started: 1683636528
42+
stopped: 1683636626
43+
-
44+
id: 195
45+
run_id: 793
46+
repo_id: 4
47+
owner_id: 1
48+
commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0
49+
is_fork_pull_request: 0
50+
name: job1 (2)
51+
attempt: 1
52+
job_id: job1
53+
task_id: 50
54+
status: 1
55+
started: 1683636528
56+
stopped: 1683636626
57+
-
58+
id: 196
59+
run_id: 793
60+
repo_id: 4
61+
owner_id: 1
62+
commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0
63+
is_fork_pull_request: 0
64+
name: job2
65+
attempt: 1
66+
job_id: job2
67+
needs: [job1]
68+
task_id: 51
69+
status: 5
70+
started: 1683636528
71+
stopped: 1683636626

‎models/fixtures/action_task.yml‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,63 @@
5757
log_length: 707
5858
log_size: 90179
5959
log_expired: 0
60+
-
61+
id: 49
62+
job_id: 194
63+
attempt: 1
64+
runner_id: 1
65+
status: 1 # success
66+
started: 1683636528
67+
stopped: 1683636626
68+
repo_id: 4
69+
owner_id: 1
70+
commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0
71+
is_fork_pull_request: 0
72+
token_hash: b8d3962425466b6709b9ac51446f93260c54afe8e7b6d3686e34f991fb8a8953822b0deed86fe41a103f34bc48dbc4784220
73+
token_salt: ffffffffff
74+
token_last_eight: ffffffff
75+
log_filename: artifact-test2/2f/47.log
76+
log_in_storage: 1
77+
log_length: 707
78+
log_size: 90179
79+
log_expired: 0
80+
-
81+
id: 50
82+
job_id: 195
83+
attempt: 1
84+
runner_id: 1
85+
status: 1 # success
86+
started: 1683636528
87+
stopped: 1683636626
88+
repo_id: 4
89+
owner_id: 1
90+
commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0
91+
is_fork_pull_request: 0
92+
token_hash: b8d3962425466b6709b9ac51446f93260c54afe8e7b6d3686e34f991fb8a8953822b0deed86fe41a103f34bc48dbc4784221
93+
token_salt: ffffffffff
94+
token_last_eight: ffffffff
95+
log_filename: artifact-test2/2f/47.log
96+
log_in_storage: 1
97+
log_length: 707
98+
log_size: 90179
99+
log_expired: 0
100+
-
101+
id: 51
102+
job_id: 196
103+
attempt: 1
104+
runner_id: 1
105+
status: 6 # running
106+
started: 1683636528
107+
stopped: 1683636626
108+
repo_id: 4
109+
owner_id: 1
110+
commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0
111+
is_fork_pull_request: 0
112+
token_hash: b8d3962425466b6709b9ac51446f93260c54afe8e7b6d3686e34f991fb8a8953822b0deed86fe41a103f34bc48dbc4784222
113+
token_salt: ffffffffff
114+
token_last_eight: ffffffff
115+
log_filename: artifact-test2/2f/47.log
116+
log_in_storage: 1
117+
log_length: 707
118+
log_size: 90179
119+
log_expired: 0

‎models/fixtures/action_task_output.yml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
-
2+
id: 1
3+
task_id: 49
4+
output_key: output_a
5+
output_value: abc
6+
-
7+
id: 2
8+
task_id: 49
9+
output_key: output_b
10+
output_value: ''
11+
-
12+
id: 3
13+
task_id: 50
14+
output_key: output_a
15+
output_value: ''
16+
-
17+
id: 4
18+
task_id: 50
19+
output_key: output_b
20+
output_value: bbb

‎routers/api/actions/runner/main_test.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package runner
5+
6+
import (
7+
"testing"
8+
9+
"code.gitea.io/gitea/models/unittest"
10+
)
11+
12+
func TestMain(m *testing.M) {
13+
unittest.MainTest(m)
14+
}

‎routers/api/actions/runner/utils.go‎

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -162,28 +162,56 @@ func findTaskNeeds(ctx context.Context, task *actions_model.ActionTask) (map[str
162162
return nil, fmt.Errorf("FindRunJobs: %w", err)
163163
}
164164

165-
ret := make(map[string]*runnerv1.TaskNeed, len(needs))
165+
jobIDJobs := make(map[string][]*actions_model.ActionRunJob)
166166
for _, job := range jobs {
167-
if !needs.Contains(job.JobID) {
168-
continue
169-
}
170-
if job.TaskID == 0 || !job.Status.IsDone() {
171-
// it shouldn't happen, or the job has been rerun
167+
jobIDJobs[job.JobID] = append(jobIDJobs[job.JobID], job)
168+
}
169+
170+
ret := make(map[string]*runnerv1.TaskNeed, len(needs))
171+
for jobID, jobsWithSameID := range jobIDJobs {
172+
if !needs.Contains(jobID) {
172173
continue
173174
}
174-
outputs := make(map[string]string)
175-
got, err := actions_model.FindTaskOutputByTaskID(ctx, job.TaskID)
176-
if err != nil {
177-
return nil, fmt.Errorf("FindTaskOutputByTaskID: %w", err)
175+
var jobOutputs map[string]string
176+
for _, job := range jobsWithSameID {
177+
if job.TaskID == 0 || !job.Status.IsDone() {
178+
// it shouldn't happen, or the job has been rerun
179+
continue
180+
}
181+
got, err := actions_model.FindTaskOutputByTaskID(ctx, job.TaskID)
182+
if err != nil {
183+
return nil, fmt.Errorf("FindTaskOutputByTaskID: %w", err)
184+
}
185+
outputs := make(map[string]string, len(got))
186+
for _, v := range got {
187+
outputs[v.OutputKey] = v.OutputValue
188+
}
189+
if len(jobOutputs) == 0 {
190+
jobOutputs = outputs
191+
} else {
192+
jobOutputs = mergeTwoOutputs(outputs, jobOutputs)
193+
}
178194
}
179-
for _, v := range got {
180-
outputs[v.OutputKey] = v.OutputValue
181-
}
182-
ret[job.JobID] = &runnerv1.TaskNeed{
183-
Outputs: outputs,
184-
Result: runnerv1.Result(job.Status),
195+
ret[jobID] = &runnerv1.TaskNeed{
196+
Outputs: jobOutputs,
197+
Result: runnerv1.Result(actions_model.AggregateJobStatus(jobsWithSameID)),
185198
}
186199
}
187200

188201
return ret, nil
189202
}
203+
204+
// mergeTwoOutputs merges two outputs from two different ActionRunJobs
205+
// Values with the same output name may be overridden. The user should ensure the output names are unique.
206+
// See https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#using-job-outputs-in-a-matrix-job
207+
func mergeTwoOutputs(o1, o2 map[string]string) map[string]string {
208+
ret := make(map[string]string, len(o1))
209+
for k1, v1 := range o1 {
210+
if len(v1) > 0 {
211+
ret[k1] = v1
212+
} else {
213+
ret[k1] = o2[k1]
214+
}
215+
}
216+
return ret
217+
}

‎routers/api/actions/runner/utils_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package runner
5+
6+
import (
7+
"context"
8+
"testing"
9+
10+
actions_model "code.gitea.io/gitea/models/actions"
11+
"code.gitea.io/gitea/models/unittest"
12+
13+
"github.com/stretchr/testify/assert"
14+
)
15+
16+
func Test_findTaskNeeds(t *testing.T) {
17+
assert.NoError(t, unittest.PrepareTestDatabase())
18+
19+
task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 51})
20+
21+
ret, err := findTaskNeeds(context.Background(), task)
22+
assert.NoError(t, err)
23+
assert.Len(t, ret, 1)
24+
assert.Contains(t, ret, "job1")
25+
assert.Len(t, ret["job1"].Outputs, 2)
26+
assert.Equal(t, "abc", ret["job1"].Outputs["output_a"])
27+
assert.Equal(t, "bbb", ret["job1"].Outputs["output_b"])
28+
}

0 commit comments

Comments
 (0)