mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-28 11:14:54 -04:00
Drop run-level ?attempt from run-logs ZIP endpoint (run has no unified attempt; jobs re-run independently and diverge), keep ?attempt on the per-job endpoint where it's well-defined, simplify WriteRunLogsZip to always use job.Attempt for entry names, remove attempt=1/attempt=999 run-logs subtests, regen swagger
This commit is contained in:
Zach Handley
parent
f8efba782e
commit
ca34faee51
4 changed files with 26 additions and 126 deletions
|
|
@ -1592,18 +1592,16 @@ func GetActionRunLogs(ctx *context.APIContext) {
|
|||
// - name: run_id
|
||||
// in: path
|
||||
// description: >
|
||||
// ID of the workflow run. The ZIP contains logs for the latest attempt
|
||||
// of each job in the run, with each entry named `{job-name}-{job-id}.log`
|
||||
// (the job ID prevents collisions when two jobs share a name).
|
||||
// ID of the workflow run. The ZIP contains the latest attempt of each
|
||||
// job in the run, with each entry named
|
||||
// `{job-name}-{job-id}-attempt-{N}.log` (the job ID prevents collisions
|
||||
// when two jobs share a name; the attempt number records which run the
|
||||
// log came from). The run itself has no attempt number — jobs are
|
||||
// re-run independently, so use the per-job logs endpoint with `?attempt`
|
||||
// to fetch a specific historical attempt of one job.
|
||||
// type: integer
|
||||
// format: int64
|
||||
// required: true
|
||||
// - name: attempt
|
||||
// in: query
|
||||
// description: 1-based attempt number; omit to fetch the latest attempt of each job. Jobs that don't have that attempt recorded contribute a `.MISSING` placeholder entry instead of a `.log`.
|
||||
// type: integer
|
||||
// format: int64
|
||||
// required: false
|
||||
// responses:
|
||||
// "200":
|
||||
// description: ZIP archive of per-job log files
|
||||
|
|
@ -1634,20 +1632,12 @@ func GetActionRunLogs(ctx *context.APIContext) {
|
|||
return
|
||||
}
|
||||
|
||||
var attempt optional.Option[int64]
|
||||
if ctx.FormString("attempt") != "" {
|
||||
attempt = optional.Some(ctx.FormInt64("attempt"))
|
||||
}
|
||||
|
||||
zipFilename := fmt.Sprintf("run-%d-logs.zip", run.ID)
|
||||
if has, v := attempt.Get(); has {
|
||||
zipFilename = fmt.Sprintf("run-%d-attempt-%d-logs.zip", run.ID, v)
|
||||
}
|
||||
|
||||
ctx.Resp.Header().Set("Content-Type", "application/zip")
|
||||
ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=\"%s\"", zipFilename))
|
||||
|
||||
if err := actions_service.WriteRunLogsZip(ctx, ctx.Resp, run, attempt); err != nil {
|
||||
if err := actions_service.WriteRunLogsZip(ctx, ctx.Resp, run); err != nil {
|
||||
ctx.Error(http.StatusInternalServerError, "WriteRunLogsZip", err)
|
||||
return
|
||||
}
|
||||
|
|
|
|||
|
|
@ -153,20 +153,19 @@ func OpenJobLogReader(
|
|||
return nopReadSeekCloser{bytes.NewReader(buf)}, stepFilename, modtime, nil
|
||||
}
|
||||
|
||||
// WriteRunLogsZip writes a ZIP of per-job logs for the run to w. When attempt
|
||||
// is set, each job's log is taken from that historical attempt (jobs without
|
||||
// that attempt get a .MISSING placeholder); when unset, the latest attempt is
|
||||
// used. Jobs that haven't run, can't be looked up, or have expired logs get
|
||||
// .MISSING; a mid-stream read failure gets a sibling .PARTIAL marker. Caller
|
||||
// sets Content-Type / Content-Disposition before calling.
|
||||
func WriteRunLogsZip(ctx context.Context, w io.Writer, run *actions_model.ActionRun, attempt optional.Option[int64]) error {
|
||||
// WriteRunLogsZip writes a ZIP of the latest per-job logs for the run to w.
|
||||
// Each entry is named {job-name}-{job-id}-attempt-{N}.log, where N is that
|
||||
// job's current attempt — the run itself has no attempt number, so jobs that
|
||||
// were re-run independently show different attempts here. Jobs that haven't
|
||||
// run, can't be looked up, or have expired logs get a .MISSING marker; a
|
||||
// mid-stream read failure gets a sibling .PARTIAL marker. Caller sets
|
||||
// Content-Type / Content-Disposition before calling.
|
||||
func WriteRunLogsZip(ctx context.Context, w io.Writer, run *actions_model.ActionRun) error {
|
||||
jobs, err := actions_model.GetRunJobsByRunID(ctx, run.ID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("get jobs for run %d: %w", run.ID, err)
|
||||
}
|
||||
|
||||
hasAttempt, attemptVal := attempt.Get()
|
||||
|
||||
zw := zip.NewWriter(w)
|
||||
defer zw.Close()
|
||||
|
||||
|
|
@ -185,15 +184,8 @@ func WriteRunLogsZip(ctx context.Context, w io.Writer, run *actions_model.Action
|
|||
return cleaned
|
||||
}
|
||||
|
||||
attemptFor := func(job *actions_model.ActionRunJob) int64 {
|
||||
if hasAttempt {
|
||||
return attemptVal
|
||||
}
|
||||
return job.Attempt
|
||||
}
|
||||
|
||||
entryName := func(job *actions_model.ActionRunJob, suffix string) string {
|
||||
return fmt.Sprintf("%s-%d-attempt-%d.%s", sanitize(job.Name), job.ID, attemptFor(job), suffix)
|
||||
return fmt.Sprintf("%s-%d-attempt-%d.%s", sanitize(job.Name), job.ID, job.Attempt, suffix)
|
||||
}
|
||||
|
||||
writeMarker := func(job *actions_model.ActionRunJob, suffix, msg string) {
|
||||
|
|
@ -204,27 +196,14 @@ func WriteRunLogsZip(ctx context.Context, w io.Writer, run *actions_model.Action
|
|||
}
|
||||
|
||||
for _, job := range jobs {
|
||||
var task *actions_model.ActionTask
|
||||
if hasAttempt {
|
||||
task, err = actions_model.GetTaskByJobAttempt(ctx, job.ID, attemptVal)
|
||||
if err != nil {
|
||||
if errors.Is(err, util.ErrNotExist) {
|
||||
writeMarker(job, "MISSING", fmt.Sprintf("no attempt %d recorded for this job\n", attemptVal))
|
||||
} else {
|
||||
writeMarker(job, "MISSING", fmt.Sprintf("task lookup failed: %v\n", err))
|
||||
}
|
||||
continue
|
||||
}
|
||||
} else {
|
||||
if job.TaskID == 0 {
|
||||
writeMarker(job, "MISSING", "job has not been executed yet\n")
|
||||
continue
|
||||
}
|
||||
task, err = actions_model.GetTaskByID(ctx, job.TaskID)
|
||||
if err != nil {
|
||||
writeMarker(job, "MISSING", fmt.Sprintf("task lookup failed: %v\n", err))
|
||||
continue
|
||||
}
|
||||
if job.TaskID == 0 {
|
||||
writeMarker(job, "MISSING", "job has not been executed yet\n")
|
||||
continue
|
||||
}
|
||||
task, err := actions_model.GetTaskByID(ctx, job.TaskID)
|
||||
if err != nil {
|
||||
writeMarker(job, "MISSING", fmt.Sprintf("task lookup failed: %v\n", err))
|
||||
continue
|
||||
}
|
||||
|
||||
if task.LogExpired {
|
||||
|
|
|
|||
9
templates/swagger/v1_json.tmpl
generated
9
templates/swagger/v1_json.tmpl
generated
|
|
@ -6506,17 +6506,10 @@
|
|||
{
|
||||
"type": "integer",
|
||||
"format": "int64",
|
||||
"description": "ID of the workflow run. The ZIP contains logs for the latest attempt of each job in the run, with each entry named `{job-name}-{job-id}.log` (the job ID prevents collisions when two jobs share a name).\n",
|
||||
"description": "ID of the workflow run. The ZIP contains the latest attempt of each job in the run, with each entry named `{job-name}-{job-id}-attempt-{N}.log` (the job ID prevents collisions when two jobs share a name; the attempt number records which run the log came from). The run itself has no attempt number — jobs are re-run independently, so use the per-job logs endpoint with `?attempt` to fetch a specific historical attempt of one job.\n",
|
||||
"name": "run_id",
|
||||
"in": "path",
|
||||
"required": true
|
||||
},
|
||||
{
|
||||
"type": "integer",
|
||||
"format": "int64",
|
||||
"description": "1-based attempt number; omit to fetch the latest attempt of each job. Jobs that don't have that attempt recorded contribute a `.MISSING` placeholder entry instead of a `.log`.",
|
||||
"name": "attempt",
|
||||
"in": "query"
|
||||
}
|
||||
],
|
||||
"responses": {
|
||||
|
|
|
|||
|
|
@ -10,7 +10,6 @@ import (
|
|||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
|
|
@ -192,67 +191,6 @@ jobs:
|
|||
MakeRequest(t, req, http.StatusNotFound)
|
||||
})
|
||||
|
||||
t.Run("attempt=1: 200 same content, attempt-suffixed entry names", func(t *testing.T) {
|
||||
req := NewRequestf(t, "GET",
|
||||
"/api/v1/repos/%s/actions/runs/%d/logs?attempt=1",
|
||||
repoA.FullName(), runID,
|
||||
)
|
||||
req.AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusOK)
|
||||
|
||||
assert.Contains(t, resp.Header().Get("Content-Disposition"),
|
||||
fmt.Sprintf("run-%d-attempt-1-logs.zip", runID))
|
||||
|
||||
r, err := zip.NewReader(bytes.NewReader(resp.Body.Bytes()), int64(resp.Body.Len()))
|
||||
require.NoError(t, err)
|
||||
|
||||
entries := map[string]string{}
|
||||
for _, f := range r.File {
|
||||
fr, err := f.Open()
|
||||
require.NoError(t, err)
|
||||
data, err := io.ReadAll(fr)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, fr.Close())
|
||||
entries[f.Name] = string(data)
|
||||
}
|
||||
|
||||
job1Name := fmt.Sprintf("%s-%d-attempt-1.log", actionRunJob1.Name, actionRunJob1.ID)
|
||||
job2Name := fmt.Sprintf("%s-%d-attempt-1.log", actionRunJob2.Name, actionRunJob2.ID)
|
||||
utf8Name := fmt.Sprintf("%s-%d-attempt-1.log", actionRunJob3.Name, actionRunJob3.ID)
|
||||
|
||||
require.Len(t, entries, 3)
|
||||
require.Contains(t, entries, job1Name)
|
||||
require.Contains(t, entries, job2Name)
|
||||
require.Contains(t, entries, utf8Name)
|
||||
assert.Contains(t, entries[job1Name], "job1-output line one")
|
||||
assert.Contains(t, entries[job2Name], "job2-output line one")
|
||||
assert.Contains(t, entries[utf8Name], "utf8-output line one")
|
||||
})
|
||||
|
||||
t.Run("attempt=999: 200 with all .MISSING entries", func(t *testing.T) {
|
||||
req := NewRequestf(t, "GET",
|
||||
"/api/v1/repos/%s/actions/runs/%d/logs?attempt=999",
|
||||
repoA.FullName(), runID,
|
||||
)
|
||||
req.AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusOK)
|
||||
|
||||
r, err := zip.NewReader(bytes.NewReader(resp.Body.Bytes()), int64(resp.Body.Len()))
|
||||
require.NoError(t, err)
|
||||
|
||||
require.Len(t, r.File, 3, "every job should contribute one .MISSING marker")
|
||||
for _, f := range r.File {
|
||||
assert.True(t, strings.HasSuffix(f.Name, "-attempt-999.MISSING"),
|
||||
"every entry should be an attempt-999 marker; got %q", f.Name)
|
||||
fr, err := f.Open()
|
||||
require.NoError(t, err)
|
||||
data, err := io.ReadAll(fr)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, fr.Close())
|
||||
assert.Contains(t, string(data), "no attempt 999 recorded")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("wrong scope: 403 without read:repository", func(t *testing.T) {
|
||||
// Token with only user scope, no repository access.
|
||||
weakToken := getTokenForLoggedInUser(t, session,
|
||||
|
|
|
|||
Loading…
Reference in a new issue