mirror of
https://github.com/go-gitea/gitea.git
synced 2026-05-13 17:35:18 +02:00
fix(actions): run TransferLogs on UpdateLog{Rows:[], NoMore:true} (#37631)
`UpdateLog` short-circuits on `len(Rows)==0` before honoring `NoMore`,
so a final empty `UpdateLog{NoMore:true}` never runs `TransferLogs`. The
task's `dbfs_data` rows are then never moved to log storage and never
deleted.
Fix: let `NoMore=true` with no new rows fall through to `TransferLogs`.
Bail when the runner has outrun the server (`Index > ack`) even with
`NoMore`, since archiving a log with a gap is worse than retrying.
Always call `WriteLogs` so `offset==0` bootstraps an empty DBFS file in
the no-output case (otherwise `TransferLogs` would fail at `dbfs.Open`).
Fixes: https://github.com/go-gitea/gitea/issues/37623
Ref: https://gitea.com/gitea/runner/pulls/952
Ref: https://gitea.com/gitea/runner/pulls/950
Co-authored-by: Claude (Opus 4.7) <noreply@anthropic.com>
This commit is contained in:
parent
f01953e764
commit
79f7062d9e
@ -261,7 +261,16 @@ func (s *Service) UpdateLog(
|
||||
}
|
||||
ack := task.LogLength
|
||||
|
||||
if len(req.Msg.Rows) == 0 || req.Msg.Index > ack || int64(len(req.Msg.Rows))+req.Msg.Index <= ack {
|
||||
// Trim rows the runner already had acked.
|
||||
var rows []*runnerv1.LogRow
|
||||
if req.Msg.Index <= ack && int64(len(req.Msg.Rows))+req.Msg.Index > ack {
|
||||
rows = req.Msg.Rows[ack-req.Msg.Index:]
|
||||
}
|
||||
|
||||
// Bail unless we have new rows or a NoMore to finalize. Even with
|
||||
// NoMore, bail when the runner has outrun the server — archiving a
|
||||
// log with a gap is worse than asking it to retry.
|
||||
if len(rows) == 0 && (!req.Msg.NoMore || req.Msg.Index > ack) {
|
||||
res.Msg.AckIndex = ack
|
||||
return res, nil
|
||||
}
|
||||
@ -270,7 +279,9 @@ func (s *Service) UpdateLog(
|
||||
return nil, status.Errorf(codes.AlreadyExists, "log file has been archived")
|
||||
}
|
||||
|
||||
rows := req.Msg.Rows[ack-req.Msg.Index:]
|
||||
// WriteLogs is called even with no rows: with offset==0 it bootstraps
|
||||
// an empty DBFS file so TransferLogs below has something to read when
|
||||
// the runner finalizes a task that produced no log output.
|
||||
ns, err := actions.WriteLogs(ctx, task.LogFilename, task.LogSize, rows)
|
||||
if err != nil {
|
||||
return nil, status.Errorf(codes.Internal, "unable to append logs to dbfs file: %v", err)
|
||||
|
||||
77
tests/integration/actions_log_finalize_test.go
Normal file
77
tests/integration/actions_log_finalize_test.go
Normal file
@ -0,0 +1,77 @@
|
||||
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package integration
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/url"
|
||||
"os"
|
||||
"testing"
|
||||
|
||||
actions_model "code.gitea.io/gitea/models/actions"
|
||||
auth_model "code.gitea.io/gitea/models/auth"
|
||||
"code.gitea.io/gitea/models/dbfs"
|
||||
repo_model "code.gitea.io/gitea/models/repo"
|
||||
"code.gitea.io/gitea/models/unittest"
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
actions_module "code.gitea.io/gitea/modules/actions"
|
||||
"code.gitea.io/gitea/modules/storage"
|
||||
|
||||
runnerv1 "code.gitea.io/actions-proto-go/runner/v1"
|
||||
"connectrpc.com/connect"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// Regression for https://gitea.com/gitea/runner/issues/950: a runner that
|
||||
// finalizes a task with no log output sends UpdateLog{Rows:[], NoMore:true}.
|
||||
// The previous short-circuit on len(Rows)==0 skipped TransferLogs, leaving
|
||||
// an orphan dbfs_data row. Verify the row is now archived and removed.
|
||||
func TestActionsLogFinalizeWithoutRows(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, _ *url.URL) {
|
||||
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
|
||||
session := loginUser(t, user2.Name)
|
||||
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
|
||||
|
||||
apiRepo := createActionsTestRepo(t, token, "actions-finalize-no-rows", false)
|
||||
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiRepo.ID})
|
||||
|
||||
runner := newMockRunner()
|
||||
runner.registerAsRepoRunner(t, user2.Name, repo.Name, "mock-runner", []string{"ubuntu-latest"}, false)
|
||||
|
||||
const wfTreePath = ".gitea/workflows/finalize-no-rows.yml"
|
||||
wfFileContent := fmt.Sprintf(`name: finalize-no-rows
|
||||
on:
|
||||
push:
|
||||
paths:
|
||||
- '%s'
|
||||
jobs:
|
||||
job1:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: noop
|
||||
`, wfTreePath)
|
||||
createWorkflowFile(t, token, user2.Name, repo.Name, wfTreePath, getWorkflowCreateFileOptions(user2, repo.DefaultBranch, "trigger", wfFileContent))
|
||||
|
||||
task := runner.fetchTask(t)
|
||||
|
||||
resp, err := runner.client.runnerServiceClient.UpdateLog(t.Context(), connect.NewRequest(&runnerv1.UpdateLogRequest{
|
||||
TaskId: task.Id,
|
||||
Index: 0,
|
||||
Rows: nil,
|
||||
NoMore: true,
|
||||
}))
|
||||
require.NoError(t, err)
|
||||
assert.EqualValues(t, 0, resp.Msg.AckIndex)
|
||||
|
||||
freshTask := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: task.Id})
|
||||
require.True(t, freshTask.LogInStorage, "log_in_storage must flip after empty NoMore=true")
|
||||
|
||||
_, err = storage.Actions.Stat(freshTask.LogFilename)
|
||||
assert.NoError(t, err, "archived log must exist in storage")
|
||||
|
||||
_, err = dbfs.Open(t.Context(), actions_module.DBFSPrefix+freshTask.LogFilename)
|
||||
assert.ErrorIs(t, err, os.ErrNotExist, "DBFS row must be cleaned up after TransferLogs")
|
||||
})
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user