mirror of
https://github.com/go-gitea/gitea.git
synced 2026-10-04 07:33:44 +09:00
fix(actions): evaluate job-level if: before concurrency check (#39437)
Gitea doesn't evaluate a job's `if:` before checking the job's concurrency group, which causes a job that should have been skipped to incorrectly cancel other jobs in the same concurrency group. This PR makes Gitea decide `if:` for every job before it becomes waiting, including jobs without `needs` at insertion, on approval and on rerun. A skipped job therefore no longer takes part in job concurrency or holds a max-parallel slot, and a reusable caller whose `if:` is false is no longer expanded on approval or rerun. An invalid `if:` skips the job with an error summary. After this PR, Gitea decides all jobs' `if:` expressions and sends `if: always()` to the runner, so the runner no longer needs to evaluate a job's `if:` again ([gitea/runner `run_context.go`](https://gitea.com/gitea/runner/src/commit/81add274599355ec1838b6ebe45804890d40bab9/act/runner/run_context.go#L1195)). --------- Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
+22
-21
@@ -190,7 +190,7 @@ func buildRerunPlan(ctx context.Context, run *actions_model.ActionRun, triggerUs
|
||||
// Inside a single database transaction it then inserts the new attempt, clones all template jobs, evaluates job-level concurrency for rerun jobs,
|
||||
// and updates the run's latest_attempt_id.
|
||||
// Jobs not in the rerun set are cloned as pass-through: their status is preserved and SourceTaskID points to the original task so the UI can still display their results.
|
||||
// The attempt's final status is derived only from the rerun jobs, not the pass-through jobs.
|
||||
// The attempt's status aggregates all its jobs, pass-through ones included, as rerun jobs skipped by `if:` get no later update.
|
||||
// Notifications and commit statuses are sent after the transaction commits.
|
||||
func execRerunPlan(ctx context.Context, plan *rerunPlan) (*actions_model.ActionRunAttempt, error) {
|
||||
vars, err := actions_model.GetVariablesOfRun(ctx, plan.run)
|
||||
@@ -222,7 +222,6 @@ func execRerunPlan(ctx context.Context, plan *rerunPlan) (*actions_model.ActionR
|
||||
|
||||
var newJobs, newJobsToRerun actions_model.ActionJobList
|
||||
var cancelledConcurrencyJobs []*actions_model.ActionRunJob
|
||||
var hasWaitingCallerJobs bool
|
||||
|
||||
err = db.WithTx(ctx, func(ctx context.Context) error {
|
||||
newAttemptStatus, jobsToCancel, err := PrepareToStartRunWithConcurrency(ctx, newAttempt)
|
||||
@@ -281,10 +280,10 @@ func execRerunPlan(ctx context.Context, plan *rerunPlan) (*actions_model.ActionR
|
||||
newJob.ParentJobID = newParentID
|
||||
}
|
||||
|
||||
var invalidIf error
|
||||
if plan.rerunAttemptJobIDs.Contains(templateJob.AttemptJobID) {
|
||||
// A deferred-matrix placeholder must go through the emitter, which is the only place
|
||||
// that expands it: dispatching it directly would hand the runner the raw payload.
|
||||
shouldBlockJob := shouldBlock || plan.hasRerunDependency(templateJob) || newJob.IsMatrixDeferred
|
||||
// the emitter decides `if:` once all needs have results, and is the only place expanding a deferred matrix
|
||||
shouldBlockJob := shouldBlock || len(newJob.Needs) > 0 || newJob.IsMatrixDeferred
|
||||
|
||||
newJob.Status = util.Iif(shouldBlockJob, actions_model.StatusBlocked, actions_model.StatusWaiting)
|
||||
newJob.TaskID = 0
|
||||
@@ -300,8 +299,13 @@ func execRerunPlan(ctx context.Context, plan *rerunPlan) (*actions_model.ActionR
|
||||
newJob.CallPayload = ""
|
||||
}
|
||||
|
||||
invalidIf, err = decideJobIf(ctx, plan.run, newAttempt, newJob, vars)
|
||||
if err != nil {
|
||||
return fmt.Errorf("evaluate job if: %w", err)
|
||||
}
|
||||
|
||||
// A slot-starved job must not cancel its group peers.
|
||||
if newJob.RawConcurrency != "" && !shouldBlockJob && slots.available(newJob) {
|
||||
if newJob.RawConcurrency != "" && newJob.Status == actions_model.StatusWaiting && slots.available(newJob) {
|
||||
if err := EvaluateJobConcurrencyFillModel(ctx, plan.run, newAttempt, newJob, vars, nil); err != nil {
|
||||
return fmt.Errorf("evaluate job concurrency: %w", err)
|
||||
}
|
||||
@@ -327,17 +331,17 @@ func execRerunPlan(ctx context.Context, plan *rerunPlan) (*actions_model.ActionR
|
||||
return err
|
||||
}
|
||||
templateIDToNewID[templateJob.ID] = newJob.ID
|
||||
if invalidIf != nil {
|
||||
if err := upsertJobErrorSummary(ctx, newJob, "if", invalidIf); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
// expand reusable caller
|
||||
if newJob.IsReusableCaller && newJob.Status == actions_model.StatusWaiting && !newJob.IsExpanded {
|
||||
if err := expandReusableWorkflowCaller(ctx, plan.run, newAttempt, newJob, vars); err != nil {
|
||||
return fmt.Errorf("inline trigger caller %d ready: %w", newJob.ID, err)
|
||||
if err := expandInlineReusableCaller(ctx, plan.run, newAttempt, newJob, vars); err != nil {
|
||||
return err
|
||||
}
|
||||
// refresh the caller status
|
||||
if err := actions_model.RefreshReusableCallerStatus(ctx, newJob); err != nil {
|
||||
return fmt.Errorf("refresh caller %d status: %w", newJob.ID, err)
|
||||
}
|
||||
hasWaitingCallerJobs = true
|
||||
}
|
||||
|
||||
// A reusable caller is never dispatched to a runner, so it must not drive the task-version bump.
|
||||
@@ -356,7 +360,7 @@ func execRerunPlan(ctx context.Context, plan *rerunPlan) (*actions_model.ActionR
|
||||
}
|
||||
}
|
||||
|
||||
newAttempt.Status = actions_model.AggregateJobStatus(newJobsToRerun)
|
||||
newAttempt.Status = actions_model.AggregateJobStatus(newJobs)
|
||||
if err := actions_model.UpdateRunAttempt(ctx, newAttempt, "status"); err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -383,12 +387,9 @@ func execRerunPlan(ctx context.Context, plan *rerunPlan) (*actions_model.ActionR
|
||||
CreateCommitStatusForRunJobs(ctx, plan.run, newJobs...)
|
||||
NotifyWorkflowJobsAndRunsStatusUpdate(ctx, newJobsToRerun)
|
||||
|
||||
// Post-commit kick for expanded callers and restored matrix placeholders: let job_emitter
|
||||
// resolve child jobs, and re-expand a placeholder whose needs may all be pass-through and done.
|
||||
if hasWaitingCallerJobs || len(plan.matrixPlaceholderTemplateIDs) > 0 {
|
||||
if err := EmitJobsIfReadyByRun(plan.run.ID); err != nil {
|
||||
log.Error("emit run %d after rerun: %v", plan.run.ID, err)
|
||||
}
|
||||
// Post-commit kick: resolve rerun jobs with needs, children of expanded callers and dependents of skipped jobs.
|
||||
if err := EmitJobsIfReadyByRun(plan.run.ID); err != nil {
|
||||
log.Error("emit run %d after rerun: %v", plan.run.ID, err)
|
||||
}
|
||||
|
||||
return newAttempt, nil
|
||||
@@ -487,7 +488,7 @@ func (p *rerunPlan) expandRerunJobIDs(jobsToRerun []*actions_model.ActionRunJob)
|
||||
|
||||
// hasRerunDependency reports whether `job` has a needs-reference that points to a job which is itself being rerun (in rerunAttemptJobIDs)
|
||||
// or is an ancestor caller whose subtree is being rerun (in ancestorAttemptJobIDs).
|
||||
// Either case means `job` should start in Blocked status.
|
||||
// Either case means the needs of `job` may produce different results or outputs in the new attempt.
|
||||
func (p *rerunPlan) hasRerunDependency(job *actions_model.ActionRunJob) bool {
|
||||
if len(job.Needs) == 0 {
|
||||
return false
|
||||
|
||||
Reference in New Issue
Block a user