Skip to content
Sign up

flagon-io/g1t

public
0

Commit

Runner: timeout-minutes stops uses steps too

A step's timeout-minutes was only passed to run: steps' shells; a uses: step ignored it. Each step now sets a deadline every process it starts stops by (actions, Docker, git, artifact transfers), nested steps of a composite action taking the nearer of theirs and the uses: step's. The job's own limit is checked apart from it, so a step's timeout fails the step, not the job.

syntaqxcommitted Parent111c3baBrowse files
4 files+87−120/4 viewed
+1−0
3131 | --- | --- |
3232 | `on:` `push` (branches, tags, paths), `pull_request`, `pull_request_target`, `issues`, `issue_comment`, `pull_request_review`, `schedule`, `workflow_dispatch`, `workflow_run`, `merge_group`, `create`, `repository_dispatch` | The same, from g1t's own pushes, pull requests, issues, comments and [merge queue](/guides/merge-queue/). `create` starts on each new branch or tag; `repository_dispatch` on [a dispatch event](#repository-dispatch). |
3333 | `jobs`, `needs`, `if`, `outputs`, `env`, `defaults`, `timeout-minutes`, `continue-on-error` | The same. |
34+| `timeout-minutes` and `continue-on-error` on a step | The same, for `run:` and `uses:` steps alike. A `uses:` step's action is stopped at its limit, with every process it started; a step inside a composite action stops at its own limit or the `uses:` step's, whichever comes first. A step stopped this way fails, unless `continue-on-error` lets the job go on. |
3435 | `strategy.matrix` with `include` and `exclude`, `fail-fast`, `max-parallel`, a matrix from `fromJSON(needs.…)` | The same. |
3536 | `concurrency` with `cancel-in-progress`, for the workflow or for one job | The same: one run, or one job, of a group at a time. |
3637 | `permissions:` for the workflow or for one job, `read-all`, `write-all` | The same: they decide what [the job's token](#the-jobs-token) may do. |
+2−2
5050 let mut command = Command::new("bash");
5151 command.args(["-c", script]).current_dir(&self.workspace);
5252 let mut commands = Commands::default();
53− matches!(process::run(command, Duration::from_secs(1800), &mut self.log, &mut commands), Ok(Ended::Exited(0)))
53+ matches!(process::run(command, Duration::from_secs(1800).min(self.remaining_time()), &mut self.log, &mut commands), Ok(Ended::Exited(0)))
5454 }
5555
5656 /// Downloads into `file`, as it comes; `Ok(None)` when there is
632632 let mut command = Command::new("bash");
633633 command.args(["-c", script]).current_dir(&self.workspace);
634634 let mut commands = Commands::default();
635− match process::run(command, Duration::from_secs(1800), &mut self.log, &mut commands) {
635+ match process::run(command, Duration::from_secs(1800).min(self.remaining_time()), &mut self.log, &mut commands) {
636636 Ok(Ended::Exited(code)) => Some(code),
637637 _ => None,
638638 }
+78−4
8282 pub(crate) posts: Vec<Post>,
8383 step_names: Vec<String>,
8484 deadline: Instant,
85+ /// The running step's `timeout-minutes`, as an instant: the nearest of
86+ /// the step's own and those of the steps around it (a composite
87+ /// action's step inside a `uses:` step). Every process a step starts
88+ /// stops by it, as every one stops by the job's deadline.
89+ step_deadline: Option<Instant>,
8590 debug: bool,
8691 /// What the last Node process left, for the step that ran it.
8792 pub(crate) last_node_outputs: BTreeMap<String, String>,
229234 self.remaining()
230235 }
231236
237+ /// What is left for the running step: its `timeout-minutes`, within
238+ /// the job's.
232239 fn remaining(&self) -> Duration {
240+ let until = self.step_deadline.map_or(self.deadline, |step| step.min(self.deadline));
241+ until.saturating_duration_since(Instant::now())
242+ }
243+
244+ /// What is left of the job's own time, whatever the step's.
245+ fn job_remaining(&self) -> Duration {
233246 self.deadline.saturating_duration_since(Instant::now())
234247 }
235248
382395 env.insert(name.clone(), expr::to_text(&value));
383396 }
384397 }
385− let timeout = step
398+ let minutes = step
386399 .get("timeout-minutes")
387400 .and_then(|v| self.with_scope(&contexts, |scope| expr::interpolate_value(v, scope)).ok())
388− .and_then(|v| v.as_f64().or_else(|| expr::to_text(&v).parse().ok()))
389− .map_or(Duration::from_secs(6 * 3600), |minutes| Duration::from_secs_f64(minutes * 60.0));
401+ .and_then(|v| v.as_f64().or_else(|| expr::to_text(&v).parse().ok()));
402+ let timeout = step_timeout(minutes);
403+ // Every step stops at its own `timeout-minutes`, `run` or `uses`: a
404+ // `uses:` step's action, and every process it starts, included.
405+ let outer_deadline = self.step_deadline;
406+ self.step_deadline = step_deadline(outer_deadline, Instant::now(), minutes);
390407 let continue_on_error = step
391408 .get("continue-on-error")
392409 .and_then(|v| self.with_scope(&contexts, |scope| expr::interpolate_value(v, scope)).ok())
440457 self.log.line("##[error]A step needs `run` or `uses`.");
441458 (false, BTreeMap::new())
442459 };
460+ let own_deadline = self.step_deadline.filter(|_| self.step_deadline != outer_deadline);
461+ self.step_deadline = outer_deadline;
462+ let ok = if own_deadline.is_some_and(|until| Instant::now() >= until) {
463+ self.log.line(&format!(
464+ "##[error]The step ran past its timeout-minutes ({}) and was stopped.",
465+ minutes.unwrap_or_default()
466+ ));
467+ false
468+ } else {
469+ ok
470+ };
443471
444472 let outcome = if ok { "success" } else { "failure" };
445473 let conclusion = if ok || continue_on_error { "success" } else { "failure" };
464492 }
465493 }
466494
495+/// How long a step's processes may run: its `timeout-minutes`, or six
496+/// hours (the job's own limit still applies).
497+fn step_timeout(minutes: Option<f64>) -> Duration {
498+ match minutes {
499+ Some(minutes) if minutes.is_finite() && minutes > 0.0 => Duration::from_secs_f64(minutes * 60.0),
500+ Some(_) => Duration::ZERO,
501+ None => Duration::from_secs(6 * 3600),
502+ }
503+}
504+
505+/// When a step must end by: its `timeout-minutes` from `now`, or `outer`
506+/// (the deadline of the `uses:` step it runs inside, for a composite
507+/// action's step), whichever is nearer. None when neither sets one.
508+fn step_deadline(outer: Option<Instant>, now: Instant, minutes: Option<f64>) -> Option<Instant> {
509+ let own = minutes.map(|minutes| now + step_timeout(Some(minutes)));
510+ match (outer, own) {
511+ (Some(outer), Some(own)) => Some(outer.min(own)),
512+ (outer, own) => outer.or(own),
513+ }
514+}
515+
467516 /// PowerShell on Windows: `pwsh` (PowerShell 7) if it is installed, as on
468517 /// GitHub's Windows runners, else Windows PowerShell.
469518 fn windows_powershell() -> String {
560609 posts: Vec::new(),
561610 step_names: Vec::new(),
562611 deadline: Instant::now() + Duration::from_secs(timeout * 60),
612+ step_deadline: None,
563613 debug,
564614 last_node_outputs: BTreeMap::new(),
565615 last_node_state: BTreeMap::new(),
657707 for (index, step) in steps.iter().enumerate().filter(|_| containers_started) {
658708 job.step(&mut frame, step, index as u32 + 1, true, &defaults);
659709 job.log_docker_notes();
660− if job.remaining().is_zero() {
710+ if job.job_remaining().is_zero() {
661711 job.log.line("##[error]The job ran past its time limit.");
662712 job.failed = true;
663713 break;
745795 run_job(&mut job);
746796 if job.failed { 1 } else { 0 }
747797 }
798+
799+#[cfg(test)]
800+mod tests {
801+ use super::*;
802+
803+ #[test]
804+ fn a_steps_timeout_is_its_own_or_the_nearer_one_around_it() {
805+ let now = Instant::now();
806+ // No timeout-minutes anywhere: only the job's deadline applies.
807+ assert_eq!(step_deadline(None, now, None), None);
808+ // A step's own, in minutes, fractions included.
809+ assert_eq!(step_deadline(None, now, Some(1.5)), Some(now + Duration::from_secs(90)));
810+ // A composite action's step inside a `uses:` step with its own:
811+ // the nearer of the two.
812+ let outer = now + Duration::from_secs(60);
813+ assert_eq!(step_deadline(Some(outer), now, Some(10.0)), Some(outer));
814+ assert_eq!(step_deadline(Some(outer), now, Some(0.5)), Some(now + Duration::from_secs(30)));
815+ assert_eq!(step_deadline(Some(outer), now, None), Some(outer));
816+ // Zero or less is a time already up.
817+ assert_eq!(step_deadline(None, now, Some(0.0)), Some(now));
818+ assert_eq!(step_timeout(None), Duration::from_secs(6 * 3600));
819+ assert_eq!(step_timeout(Some(-1.0)), Duration::ZERO);
820+ }
821+}
+6−6
4343 }
4444 command.args(args).env("GIT_TERMINAL_PROMPT", "0");
4545 let mut commands = Commands::default();
46− matches!(process::run(command, Duration::from_secs(600), &mut self.log, &mut commands), Ok(Ended::Exited(0)))
46+ matches!(process::run(command, Duration::from_secs(600).min(self.remaining_time()), &mut self.log, &mut commands), Ok(Ended::Exited(0)))
4747 }
4848
4949 /// A fetch, tried again after a short wait when it fails: a transfer
186186 let mut command = Command::new("bash");
187187 command.args(["-c", &script]);
188188 let mut commands = Commands::default();
189− match process::run(command, Duration::from_secs(300), &mut self.log, &mut commands) {
189+ match process::run(command, Duration::from_secs(300).min(self.remaining_time()), &mut self.log, &mut commands) {
190190 Ok(Ended::Exited(0)) => {
191191 let _ = std::fs::write(dir.join(".g1t-fetched"), "");
192192 Some(dir)
225225 };
226226 let ended = process::run(command, Duration::from_secs(6 * 3600).min(self.deadline_left()), &mut self.log, &mut commands);
227227 let ok = matches!(ended, Ok(Ended::Exited(0)));
228− if let Ok(Ended::Exited(code)) = ended
229− && code != 0
230− {
231− self.log.line(&format!("##[error]The action exited with code {code}."));
228+ match ended {
229+ Ok(Ended::Exited(code)) if code != 0 => self.log.line(&format!("##[error]The action exited with code {code}.")),
230+ Ok(Ended::TimedOut) => self.log.line("##[error]The action ran past its time limit and was stopped."),
231+ _ => {}
232232 }
233233 let (outputs, state) = self.absorb(&files, &commands);
234234 self.last_node_outputs = outputs;