Skip to content

Commit 3d3c40b

Browse files
committed
Merge pull request 'Fix #2119: add per-request HTTP timeout + poll watchdog to gitea runner' (#2382) from task/2119-poll-watchdog-v2 into main
2 parents 2e4100f + b99968f commit 3d3c40b

7 files changed

Lines changed: 279 additions & 5 deletions

File tree

Cargo.lock

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/terraphim_gitea_runner/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ log = { workspace = true }
2525
chrono = { workspace = true, features = ["serde"] }
2626
uuid = { workspace = true, features = ["v4"] }
2727
reqwest = { version = "0.12", features = ["json", "rustls-tls"], default-features = false }
28+
sd-notify = "0.4"
2829
base64 = "0.22"
2930
flate2 = "1.0"
3031
env_logger = "0.11"

crates/terraphim_gitea_runner/src/bin/terraphim-gitea-runner.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
//! - `RUNNER_LEGACY_TOKEN` enable the legacy commit-status mirror with this API token
1515
//! - `RUNNER_LEGACY_CONTEXT` legacy mirror context, default `adf/build`
1616
//! - `RUNNER_CHECKOUT_DIR` checkout root; per-repo trees at `<root>/<owner>/<repo>` (default `.`)
17+
//! - `RUNNER_HTTP_TIMEOUT` per-request HTTP timeout in seconds (default 30)
1718
1819
use std::path::PathBuf;
1920
use std::sync::Arc;
@@ -60,6 +61,12 @@ async fn main() -> anyhow::Result<()> {
6061
context: env_or("RUNNER_LEGACY_CONTEXT", "adf/build"),
6162
});
6263

64+
let http_timeout_secs: u64 = std::env::var("RUNNER_HTTP_TIMEOUT")
65+
.ok()
66+
.and_then(|v| v.parse().ok())
67+
.unwrap_or(30);
68+
let http_request_timeout = Duration::from_secs(http_timeout_secs);
69+
6370
let config = RunnerConfig {
6471
instance_url: env_or("GITEA_URL", "https://git.terraphim.cloud"),
6572
org: env_or("GITEA_ORG", "terraphim"),
@@ -69,11 +76,16 @@ async fn main() -> anyhow::Result<()> {
6976
poll_interval: Duration::from_secs(3),
7077
active_repos,
7178
legacy_status_mirror,
79+
http_request_timeout,
80+
poll_timeout: Duration::from_secs(http_timeout_secs * 2),
7281
};
7382
let checkout_dir = env_or("RUNNER_CHECKOUT_DIR", ".");
7483
let version = env!("CARGO_PKG_VERSION").to_string();
7584

76-
let client = Arc::new(ReqwestRunnerClient::new(config.instance_url.clone()));
85+
let client = Arc::new(ReqwestRunnerClient::new_with_timeout(
86+
config.instance_url.clone(),
87+
config.http_request_timeout,
88+
));
7789

7890
// Register if we have no persisted state.
7991
let state = match RunnerState::load(&config.state_file)? {

crates/terraphim_gitea_runner/src/client.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use crate::state::RunnerState;
99
use crate::types::*;
1010
use crate::{Result, RunnerError};
1111
use async_trait::async_trait;
12+
use std::time::Duration;
1213

1314
const SERVICE: &str = "api/actions/runner.v1.RunnerService";
1415

@@ -46,11 +47,24 @@ pub struct ReqwestRunnerClient {
4647
}
4748

4849
impl ReqwestRunnerClient {
49-
/// Create a client for the given Gitea instance base URL.
50+
/// Create a client with a 30-second per-request timeout.
5051
pub fn new(instance_url: impl Into<String>) -> Self {
52+
Self::new_with_timeout(instance_url, Duration::from_secs(30))
53+
}
54+
55+
/// Create a client with an explicit per-request timeout.
56+
///
57+
/// Pass the value from [`crate::config::RunnerConfig::http_request_timeout`].
58+
/// A hung `FetchTask` call returns an error within the timeout window rather
59+
/// than blocking the poll loop indefinitely.
60+
pub fn new_with_timeout(instance_url: impl Into<String>, timeout: Duration) -> Self {
61+
let http = reqwest::Client::builder()
62+
.timeout(timeout)
63+
.build()
64+
.expect("failed to build reqwest client");
5165
Self {
5266
base_url: instance_url.into().trim_end_matches('/').to_string(),
53-
http: reqwest::Client::new(),
67+
http,
5468
}
5569
}
5670

crates/terraphim_gitea_runner/src/config.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ pub struct RunnerConfig {
2525
/// Optional legacy commit-status mirror (e.g. `adf/build`) posted alongside
2626
/// the native result during migration. `None` disables the mirror.
2727
pub legacy_status_mirror: Option<LegacyStatusMirrorConfig>,
28+
/// Timeout applied to each HTTP request to the Gitea RunnerService.
29+
/// A hung `FetchTask` call is aborted after this duration rather than
30+
/// blocking the poll loop indefinitely.
31+
pub http_request_timeout: Duration,
32+
/// Belt-and-suspenders timeout wrapping each `poll_once` call in
33+
/// `run_forever`. Should exceed `http_request_timeout` so reqwest's own
34+
/// timeout fires first; defaults to `2 × http_request_timeout`.
35+
pub poll_timeout: Duration,
2836
}
2937

3038
/// Configuration for the optional legacy commit-status mirror.
@@ -47,6 +55,8 @@ impl Default for RunnerConfig {
4755
poll_interval: Duration::from_secs(3),
4856
active_repos: Vec::new(),
4957
legacy_status_mirror: None,
58+
http_request_timeout: Duration::from_secs(30),
59+
poll_timeout: Duration::from_secs(60),
5060
}
5161
}
5262
}

crates/terraphim_gitea_runner/src/poller.rs

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use crate::state::RunnerState;
99
use crate::status::SingleStatusWriter;
1010
use crate::task_worker::TaskWorker;
1111
use crate::workflow_payload;
12+
use sd_notify;
1213
use std::path::PathBuf;
1314
use std::sync::Arc;
1415

@@ -108,10 +109,37 @@ impl<C: GiteaRunnerClient + 'static, P: PolicyPlanner + 'static> Poller<C, P> {
108109
/// after our last poll would never be offered (no further version change)
109110
/// until an unrelated bump or a runner restart -- the stuck-run race. Sending
110111
/// 0 forces a pick each poll; the extra `PickTask` query is negligible.
112+
///
113+
/// Each `poll_once` call is wrapped in `config.poll_timeout`. If it does not
114+
/// return within that window the stall is logged and the loop continues.
115+
/// `config.http_request_timeout` (set on the reqwest client) is the primary
116+
/// guard; `poll_timeout` is belt-and-suspenders for kernel-level hangs.
117+
///
118+
/// After every successful poll a `WATCHDOG=1` notification is sent to systemd
119+
/// if `$NOTIFY_SOCKET` is set. Set `WatchdogSec=` in the `.service` unit to
120+
/// auto-restart when no heartbeat arrives within the window.
111121
pub async fn run_forever(&self, state: &RunnerState) -> Result<()> {
122+
let mut consecutive_errors = 0u32;
112123
loop {
113-
if let Err(e) = self.poll_once(state, 0).await {
114-
log::error!("poll error: {e}");
124+
match tokio::time::timeout(self.config.poll_timeout, self.poll_once(state, 0)).await {
125+
Ok(Ok(_tasks_version)) => {
126+
consecutive_errors = 0;
127+
// Heartbeat: no-op when $NOTIFY_SOCKET is unset.
128+
let _ = sd_notify::notify(false, &[sd_notify::NotifyState::Watchdog]);
129+
}
130+
Ok(Err(e)) => {
131+
consecutive_errors += 1;
132+
log::error!("poll error (streak={consecutive_errors}): {e}");
133+
}
134+
Err(_elapsed) => {
135+
consecutive_errors += 1;
136+
log::error!(
137+
"poll timed out after {:?} (streak={consecutive_errors}); \
138+
verify Gitea is reachable at {}",
139+
self.config.poll_timeout,
140+
self.config.instance_url,
141+
);
142+
}
115143
}
116144
tokio::time::sleep(self.config.poll_interval).await;
117145
}
Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
//! #2119 watchdog tests: poll timeout and error-streak tracking.
2+
//!
3+
//! Tests use a real axum fake-server (no internal mocks) to verify that
4+
//! `run_forever` does not hang when `FetchTask` is slow or erroneous.
5+
6+
use std::sync::{Arc, Mutex};
7+
use std::time::Duration;
8+
9+
use axum::{Json, Router, extract::State, routing::post};
10+
use serde_json::{Value, json};
11+
use terraphim_gitea_runner::client::ReqwestRunnerClient;
12+
use terraphim_gitea_runner::config::RunnerConfig;
13+
use terraphim_gitea_runner::policy::DeterministicPlanner;
14+
use terraphim_gitea_runner::poller::Poller;
15+
use terraphim_gitea_runner::state::RunnerState;
16+
use tokio::time::Instant;
17+
18+
fn runner_state() -> RunnerState {
19+
RunnerState {
20+
uuid: "uuid-1".into(),
21+
token: "tok-1".into(),
22+
name: "fake".into(),
23+
version: "0.1.0".into(),
24+
labels: vec!["terraphim-native".into()],
25+
ephemeral: false,
26+
}
27+
}
28+
29+
async fn register(Json(_b): Json<Value>) -> Json<Value> {
30+
Json(
31+
json!({"runner": {"id": 1, "uuid": "uuid-1", "token": "tok-1",
32+
"name": "fake", "version": "0.1.0", "labels": ["terraphim-native"], "ephemeral": false}}),
33+
)
34+
}
35+
async fn declare(Json(b): Json<Value>) -> Json<Value> {
36+
Json(json!({"version": b["version"], "labels": b["labels"]}))
37+
}
38+
async fn update_task(Json(_b): Json<Value>) -> Json<Value> {
39+
Json(json!({"tasksVersion": 1, "sentOutputs": {}}))
40+
}
41+
async fn update_log(Json(b): Json<Value>) -> Json<Value> {
42+
let ack = b["index"].as_i64().unwrap_or(0)
43+
+ b["rows"].as_array().map(|a| a.len() as i64).unwrap_or(0);
44+
Json(json!({"ackIndex": ack}))
45+
}
46+
47+
#[derive(Default, Clone)]
48+
struct Counters {
49+
fetch_calls: usize,
50+
}
51+
type Shared = Arc<Mutex<Counters>>;
52+
53+
/// Serve a slow FetchTask that sleeps longer than the client timeout.
54+
async fn fetch_slow(State(s): State<Shared>, Json(_b): Json<Value>) -> Json<Value> {
55+
s.lock().unwrap().fetch_calls += 1;
56+
tokio::time::sleep(Duration::from_millis(500)).await;
57+
Json(json!({"tasksVersion": 0}))
58+
}
59+
60+
/// Serve a fast FetchTask that immediately returns no task.
61+
async fn fetch_fast(State(s): State<Shared>, Json(_b): Json<Value>) -> Json<Value> {
62+
s.lock().unwrap().fetch_calls += 1;
63+
Json(json!({"tasksVersion": 0}))
64+
}
65+
66+
async fn spawn_server(shared: Shared, fetch: axum::routing::MethodRouter<Shared>) -> String {
67+
let base = "/api/actions/runner.v1.RunnerService";
68+
let app = Router::new()
69+
.route(&format!("{base}/Register"), post(register))
70+
.route(&format!("{base}/Declare"), post(declare))
71+
.route(&format!("{base}/FetchTask"), fetch)
72+
.route(&format!("{base}/UpdateTask"), post(update_task))
73+
.route(&format!("{base}/UpdateLog"), post(update_log))
74+
.with_state(shared);
75+
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
76+
let addr = listener.local_addr().unwrap();
77+
tokio::spawn(async move { axum::serve(listener, app).await.unwrap() });
78+
format!("http://{addr}")
79+
}
80+
81+
/// When FetchTask takes longer than `http_request_timeout`, `poll_once` returns
82+
/// an error (reqwest timeout) and `run_forever` continues without hanging.
83+
#[tokio::test(flavor = "multi_thread")]
84+
async fn poll_timeout_does_not_hang_forever() {
85+
let shared: Shared = Arc::new(Mutex::new(Counters::default()));
86+
// Use a short HTTP timeout: slow server sleeps 500ms, we timeout at 100ms.
87+
let http_timeout = Duration::from_millis(100);
88+
let http = reqwest::Client::builder()
89+
.timeout(http_timeout)
90+
.build()
91+
.unwrap();
92+
let url = spawn_server(shared.clone(), post(fetch_slow)).await;
93+
let config = RunnerConfig {
94+
instance_url: url.clone(),
95+
poll_interval: Duration::from_millis(10),
96+
poll_timeout: Duration::from_millis(200),
97+
http_request_timeout: http_timeout,
98+
..RunnerConfig::default()
99+
};
100+
let client =
101+
Arc::new(terraphim_gitea_runner::client::ReqwestRunnerClient::with_http(url, http));
102+
let _tmpdir = tempfile::tempdir().unwrap();
103+
let poller = Poller::new(
104+
client,
105+
Arc::new(DeterministicPlanner::default()),
106+
config,
107+
_tmpdir.path(),
108+
);
109+
let st = runner_state();
110+
111+
// run_forever loops without end; we let it run for 600ms and verify it
112+
// made multiple attempts (not frozen waiting on the slow server).
113+
let start = Instant::now();
114+
let _ = tokio::time::timeout(Duration::from_millis(600), poller.run_forever(&st)).await;
115+
let elapsed = start.elapsed();
116+
117+
// Must have returned within the outer timeout, not the server's 500ms sleep.
118+
assert!(
119+
elapsed < Duration::from_millis(700),
120+
"run_forever must not hang on slow FetchTask (elapsed={elapsed:?})"
121+
);
122+
// Must have made at least 2 poll attempts, proving it recovered and retried.
123+
let calls = shared.lock().unwrap().fetch_calls;
124+
assert!(
125+
calls >= 2,
126+
"at least 2 FetchTask attempts expected (got {calls})"
127+
);
128+
}
129+
130+
/// `new_with_timeout` builds a client whose underlying reqwest client has the
131+
/// configured timeout — verified by checking that a slow server causes an error
132+
/// response from `poll_once` faster than the server sleeps.
133+
#[tokio::test]
134+
async fn new_with_timeout_sets_request_timeout() {
135+
let shared: Shared = Arc::new(Mutex::new(Counters::default()));
136+
let url = spawn_server(shared.clone(), post(fetch_slow)).await;
137+
138+
// Client with 100ms timeout; server sleeps 500ms.
139+
let client = Arc::new(ReqwestRunnerClient::new_with_timeout(
140+
&url,
141+
Duration::from_millis(100),
142+
));
143+
let config = RunnerConfig {
144+
instance_url: url,
145+
poll_timeout: Duration::from_secs(10),
146+
http_request_timeout: Duration::from_millis(100),
147+
..RunnerConfig::default()
148+
};
149+
let _tmpdir2 = tempfile::tempdir().unwrap();
150+
let poller = Poller::new(
151+
client,
152+
Arc::new(DeterministicPlanner::default()),
153+
config,
154+
_tmpdir2.path(),
155+
);
156+
let st = runner_state();
157+
158+
let start = Instant::now();
159+
let result = poller.poll_once(&st, 0).await;
160+
let elapsed = start.elapsed();
161+
162+
assert!(result.is_err(), "poll_once must error when server is slow");
163+
assert!(
164+
elapsed < Duration::from_millis(300),
165+
"poll_once must time out before the server's 500ms sleep (elapsed={elapsed:?})"
166+
);
167+
}
168+
169+
/// A fast server + quick poll_interval: `run_forever` should call FetchTask
170+
/// multiple times within a short window, demonstrating the normal loop works.
171+
#[tokio::test(flavor = "multi_thread")]
172+
async fn run_forever_polls_repeatedly_on_no_task() {
173+
let shared: Shared = Arc::new(Mutex::new(Counters::default()));
174+
let url = spawn_server(shared.clone(), post(fetch_fast)).await;
175+
let config = RunnerConfig {
176+
instance_url: url.clone(),
177+
poll_interval: Duration::from_millis(10),
178+
http_request_timeout: Duration::from_secs(5),
179+
poll_timeout: Duration::from_secs(10),
180+
..RunnerConfig::default()
181+
};
182+
let client = Arc::new(ReqwestRunnerClient::new(url));
183+
let _tmpdir3 = tempfile::tempdir().unwrap();
184+
let poller = Poller::new(
185+
client,
186+
Arc::new(DeterministicPlanner::default()),
187+
config,
188+
_tmpdir3.path(),
189+
);
190+
let st = runner_state();
191+
192+
let _ = tokio::time::timeout(Duration::from_millis(120), poller.run_forever(&st)).await;
193+
194+
let calls = shared.lock().unwrap().fetch_calls;
195+
assert!(
196+
calls >= 5,
197+
"at least 5 FetchTask polls in 120ms with 10ms interval (got {calls})"
198+
);
199+
}

0 commit comments

Comments
 (0)