From 02a51c0a218061df978e88231d7507625afacd8e Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Thu, 11 Sep 2025 14:43:26 +0000 Subject: [PATCH] fix: enforce job..timeout-minutes (#982) - enforce timeout-minutes timeout for jobs in a way similar to how it is done for steps - minimal refactor of evaluateStepTimeout evaluateTimeout so it can be used by jobs as well, with additional debug information and error logging if parsing fails - add integration tests for both step and job timeout-minutes, verifying expressions are allowed and evaluated Resolves forgejo/runner#979 --- Manually verified to work as expected https://v13.next.forgejo.org/earl-warren/testtimeout-minutes/actions/runs/3/jobs/0/attempt/1 ```yaml on: [push] jobs: test: runs-on: docker timeout-minutes: 1 steps: - run: | set -x while : ; do sleep 30 done ``` ![image](/attachments/047ddd15-7109-4931-a6ed-43073e4d31f9) - bug fixes - [PR](https://code.forgejo.org/forgejo/runner/pulls/982): fix: enforce job..timeout-minutes Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/982 Reviewed-by: Michael Kriese Reviewed-by: Mathieu Fenniak Co-authored-by: Earl Warren Co-committed-by: Earl Warren --- act/runner/run_context.go | 5 ++++- act/runner/runner_test.go | 2 ++ act/runner/step.go | 11 +++++++---- act/runner/testdata/basic/push.yml | 3 ++- act/runner/testdata/timeout-minutes-job/push.yml | 12 ++++++++++++ act/runner/testdata/timeout-minutes-step/push.yml | 12 ++++++++++++ 6 files changed, 39 insertions(+), 6 deletions(-) create mode 100644 act/runner/testdata/timeout-minutes-job/push.yml create mode 100644 act/runner/testdata/timeout-minutes-step/push.yml diff --git a/act/runner/run_context.go b/act/runner/run_context.go index f6978b64..698fa0b4 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -943,7 +943,10 @@ func (rc *RunContext) Executor() (common.Executor, error) { return err } if res { - return executor(ctx) + timeoutctx, cancelTimeOut := evaluateTimeout(ctx, rc.ExprEval, rc.Run.Job().TimeoutMinutes) + defer cancelTimeOut() + + return executor(timeoutctx) } return nil }, nil diff --git a/act/runner/runner_test.go b/act/runner/runner_test.go index 86c0ad6e..0d29eca8 100644 --- a/act/runner/runner_test.go +++ b/act/runner/runner_test.go @@ -272,6 +272,8 @@ func TestRunner_RunEvent(t *testing.T) { {workdir, "evalmatrix-merge-array", "push", "", platforms, secrets}, {workdir, "basic", "push", "", platforms, secrets}, + {workdir, "timeout-minutes-stop", "push", "Job 'check' failed", platforms, secrets}, + {workdir, "timeout-minutes-job", "push", "context deadline exceeded", platforms, secrets}, {workdir, "fail", "push", "Job 'build' failed", platforms, secrets}, {workdir, "runs-on", "push", "", platforms, secrets}, {workdir, "checkout", "push", "", platforms, secrets}, diff --git a/act/runner/step.go b/act/runner/step.go index 25c67cfd..322b6f7e 100644 --- a/act/runner/step.go +++ b/act/runner/step.go @@ -177,7 +177,7 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo Mode: 0o666, })(ctx) - timeoutctx, cancelTimeOut := evaluateStepTimeout(ctx, rc.ExprEval, stepModel) + timeoutctx, cancelTimeOut := evaluateTimeout(ctx, rc.ExprEval, stepModel.TimeoutMinutes) defer cancelTimeOut() err = executor(timeoutctx) @@ -213,12 +213,15 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo } } -func evaluateStepTimeout(ctx context.Context, exprEval ExpressionEvaluator, stepModel *model.Step) (context.Context, context.CancelFunc) { - timeout := exprEval.Interpolate(ctx, stepModel.TimeoutMinutes) +func evaluateTimeout(ctx context.Context, exprEval ExpressionEvaluator, timeoutMinutes string) (context.Context, context.CancelFunc) { + timeout := exprEval.Interpolate(ctx, timeoutMinutes) if timeout != "" { - if timeOutMinutes, err := strconv.ParseInt(timeout, 10, 64); err == nil { + timeOutMinutes, err := strconv.ParseInt(timeout, 10, 64) + if err == nil { + common.Logger(ctx).Debugf("the step will stop in timeout-minutes %s", timeout) return context.WithTimeout(ctx, time.Duration(timeOutMinutes)*time.Minute) } + common.Logger(ctx).Errorf("timeout-minutes %s cannot be parsed and will be ignored: %w", timeout, err) } return ctx, func() {} } diff --git a/act/runner/testdata/basic/push.yml b/act/runner/testdata/basic/push.yml index 8cb1836f..8b40283e 100644 --- a/act/runner/testdata/basic/push.yml +++ b/act/runner/testdata/basic/push.yml @@ -18,7 +18,8 @@ jobs: - run: ls - run: echo 'hello world' - run: echo ${GITHUB_SHA} >> $(dirname "${GITHUB_WORKSPACE}")/sha.txt - - run: cat $(dirname "${GITHUB_WORKSPACE}")/sha.txt | grep ${GITHUB_SHA} + - timeout-minutes: 30 + run: cat $(dirname "${GITHUB_WORKSPACE}")/sha.txt | grep ${GITHUB_SHA} build: runs-on: ubuntu-latest needs: [check] diff --git a/act/runner/testdata/timeout-minutes-job/push.yml b/act/runner/testdata/timeout-minutes-job/push.yml new file mode 100644 index 00000000..be5f6c2d --- /dev/null +++ b/act/runner/testdata/timeout-minutes-job/push.yml @@ -0,0 +1,12 @@ +name: timeout-minutes +on: push + +env: + TIMEOUT_MINUTES: 0 + +jobs: + check: + runs-on: ubuntu-latest + timeout-minutes: ${{ env.TIMEOUT_MINUTES }} + steps: + - run: sleep 10 diff --git a/act/runner/testdata/timeout-minutes-step/push.yml b/act/runner/testdata/timeout-minutes-step/push.yml new file mode 100644 index 00000000..a79bb63a --- /dev/null +++ b/act/runner/testdata/timeout-minutes-step/push.yml @@ -0,0 +1,12 @@ +name: timeout-minutes +on: push + +env: + TIMEOUT_MINUTES: 0 + +jobs: + check: + runs-on: ubuntu-latest + steps: + - timeout-minutes: ${{ env.TIMEOUT_MINUTES }} + run: sleep 10