Skip to content

Commit 724353f

Browse files
authored
fix: reconcile draining generations over SSH (#10691)
1 parent c24dae6 commit 724353f

2 files changed

Lines changed: 250 additions & 6 deletions

File tree

crates/homeboy-lab-runner/src/connection.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1321,7 +1321,11 @@ pub fn status(runner_id: &str) -> Result<RunnerStatusReport> {
13211321
// A dead controller tunnel must be reattached or reported as disconnected
13221322
// before polling every draining local projection.
13231323
if connected {
1324-
super::generation_store::reconcile(runner_id, session.as_ref())?;
1324+
if let Ok(Some((_, _, client))) = remote_daemon::resolve_ssh_runner(&runner) {
1325+
super::generation_store::reconcile_with_ssh(runner_id, session.as_ref(), &client)?;
1326+
} else {
1327+
super::generation_store::reconcile(runner_id, session.as_ref())?;
1328+
}
13251329
}
13261330
let stale_daemon = stale_daemon_warning(&runner, session.as_ref(), connected)?;
13271331
let local_daemon_freshness = runner_daemon_freshness(&runner, session.as_ref(), connected)?;

crates/homeboy-lab-runner/src/generation_store.rs

Lines changed: 245 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ use std::io::Write;
33
use std::path::PathBuf;
44
use std::time::Duration;
55

6+
use homeboy_core::engine::shell;
67
use homeboy_core::error::{Error, Result};
78
use homeboy_core::paths;
9+
use homeboy_core::server::SshClient;
810

911
use crate::rolling_generation::RollingResultOwnerRetirement;
1012
use crate::{RollingGenerations, RunnerDaemonGenerationStatus, RunnerSession};
@@ -759,6 +761,15 @@ struct HttpGenerationEndpointOperations {
759761
client: reqwest::blocking::Client,
760762
}
761763

764+
struct SshGenerationEndpointOperations<'a> {
765+
client: &'a SshClient,
766+
}
767+
768+
struct FallbackGenerationEndpointOperations<'a, Primary, Fallback> {
769+
primary: &'a Primary,
770+
fallback: &'a Fallback,
771+
}
772+
762773
impl GenerationEndpointOperations for HttpGenerationEndpointOperations {
763774
fn reconcile_terminal_jobs(&self, session: &RunnerSession) -> bool {
764775
let Some(local_url) = session.local_url.as_deref() else {
@@ -810,6 +821,123 @@ impl GenerationEndpointOperations for HttpGenerationEndpointOperations {
810821
}
811822
}
812823

824+
impl SshGenerationEndpointOperations<'_> {
825+
fn endpoint_url(session: &RunnerSession, path: &str) -> Option<String> {
826+
if session.mode != crate::RunnerTunnelMode::DirectSsh {
827+
return None;
828+
}
829+
let address = session.remote_daemon_address.as_deref()?;
830+
let address = address.parse::<std::net::SocketAddr>().ok()?;
831+
if !address.ip().is_loopback() {
832+
return None;
833+
}
834+
Some(format!("http://{address}{path}"))
835+
}
836+
837+
fn request(
838+
&self,
839+
method: &str,
840+
session: &RunnerSession,
841+
path: &str,
842+
body: Option<&str>,
843+
) -> Option<String> {
844+
let url = Self::endpoint_url(session, path)?;
845+
let mut command = format!(
846+
"curl --fail --silent --show-error --max-time 5 --request {} {}",
847+
shell::quote_arg(method),
848+
shell::quote_arg(&url),
849+
);
850+
if let Some(body) = body {
851+
command.push_str(&format!(
852+
" --header 'Content-Type: application/json' --data {}",
853+
shell::quote_arg(body),
854+
));
855+
}
856+
let output = self
857+
.client
858+
.execute_with_timeout(&command, Duration::from_secs(10));
859+
output.success.then_some(output.stdout)
860+
}
861+
862+
fn authenticated_health(&self, session: &RunnerSession) -> Option<serde_json::Value> {
863+
let output = self.request("GET", session, "/health", None)?;
864+
let health = serde_json::from_str::<serde_json::Value>(&output).ok()?;
865+
Self::health_matches_session(session, &health).then_some(health)
866+
}
867+
868+
fn health_matches_session(session: &RunnerSession, health: &serde_json::Value) -> bool {
869+
let Some(expected_lease) = session.remote_daemon_lease_id.as_deref() else {
870+
return false;
871+
};
872+
let Some(expected_pid) = session.remote_daemon_pid.map(u64::from) else {
873+
return false;
874+
};
875+
health
876+
.pointer("/lease/lease_id")
877+
.and_then(serde_json::Value::as_str)
878+
== Some(expected_lease)
879+
&& health.get("pid").and_then(serde_json::Value::as_u64) == Some(expected_pid)
880+
}
881+
}
882+
883+
impl GenerationEndpointOperations for SshGenerationEndpointOperations<'_> {
884+
fn reconcile_terminal_jobs(&self, session: &RunnerSession) -> bool {
885+
self.authenticated_health(session).is_some()
886+
&& self
887+
.request("POST", session, "/jobs/reconcile-terminal", None)
888+
.is_some()
889+
}
890+
891+
fn active_jobs(&self, session: &RunnerSession) -> Option<usize> {
892+
self.authenticated_health(session)?
893+
.pointer("/freshness/active_jobs")
894+
.and_then(serde_json::Value::as_u64)
895+
.and_then(|count| usize::try_from(count).ok())
896+
}
897+
898+
fn stop(&self, session: &RunnerSession) -> bool {
899+
let Some(lease_id) = session.remote_daemon_lease_id.as_deref() else {
900+
return false;
901+
};
902+
if self.authenticated_health(session).is_none() {
903+
return false;
904+
}
905+
let body = serde_json::json!({ "lease_id": lease_id, "force": false }).to_string();
906+
self.request("POST", session, "/lifecycle/stop", Some(&body))
907+
.is_some()
908+
}
909+
910+
fn terminate_tunnel(&self, pid: u32) {
911+
crate::connection::terminate_generation_tunnel(pid);
912+
}
913+
}
914+
915+
impl<Primary, Fallback> GenerationEndpointOperations
916+
for FallbackGenerationEndpointOperations<'_, Primary, Fallback>
917+
where
918+
Primary: GenerationEndpointOperations,
919+
Fallback: GenerationEndpointOperations,
920+
{
921+
fn reconcile_terminal_jobs(&self, session: &RunnerSession) -> bool {
922+
self.primary.reconcile_terminal_jobs(session)
923+
|| self.fallback.reconcile_terminal_jobs(session)
924+
}
925+
926+
fn active_jobs(&self, session: &RunnerSession) -> Option<usize> {
927+
self.primary
928+
.active_jobs(session)
929+
.or_else(|| self.fallback.active_jobs(session))
930+
}
931+
932+
fn stop(&self, session: &RunnerSession) -> bool {
933+
self.primary.stop(session) || self.fallback.stop(session)
934+
}
935+
936+
fn terminate_tunnel(&self, pid: u32) {
937+
self.primary.terminate_tunnel(pid);
938+
}
939+
}
940+
813941
/// Reconciliation is intentionally fail-closed: an unreachable draining
814942
/// endpoint remains recorded and routable. Each reachable draining daemon first
815943
/// settles terminal durable handoffs, then its own health response becomes the
@@ -829,6 +957,34 @@ pub(crate) fn reconcile(runner_id: &str, legacy: Option<&RunnerSession>) -> Resu
829957
)
830958
}
831959

960+
/// Reconcile generations through their controller-local tunnels, falling back
961+
/// to the same recorded loopback daemon endpoints over the trusted SSH runner.
962+
/// This restores observability after an old generation's local tunnel exits
963+
/// without weakening the daemon's terminal-job, zero-active-job, or stop gates.
964+
pub(crate) fn reconcile_with_ssh(
965+
runner_id: &str,
966+
legacy: Option<&RunnerSession>,
967+
ssh_client: &SshClient,
968+
) -> Result<()> {
969+
let client = reqwest::blocking::Client::builder()
970+
.no_proxy()
971+
.timeout(Duration::from_secs(5))
972+
.build()
973+
.map_err(|error| {
974+
Error::internal_unexpected(format!("build generation reconcile client: {error}"))
975+
})?;
976+
let local = HttpGenerationEndpointOperations { client };
977+
let remote = SshGenerationEndpointOperations { client: ssh_client };
978+
reconcile_with(
979+
runner_id,
980+
legacy,
981+
&FallbackGenerationEndpointOperations {
982+
primary: &local,
983+
fallback: &remote,
984+
},
985+
)
986+
}
987+
832988
fn reconcile_with(
833989
runner_id: &str,
834990
legacy: Option<&RunnerSession>,
@@ -1290,6 +1446,7 @@ mod tests {
12901446
struct FakeEndpointOperations {
12911447
active_jobs: RefCell<std::collections::BTreeMap<String, usize>>,
12921448
terminal_reconcile_failures: RefCell<std::collections::BTreeSet<String>>,
1449+
stop_failures: RefCell<std::collections::BTreeSet<String>>,
12931450
terminal_reconciled_leases: RefCell<Vec<String>>,
12941451
stopped_leases: RefCell<Vec<String>>,
12951452
terminated_pids: RefCell<Vec<u32>>,
@@ -1315,17 +1472,96 @@ mod tests {
13151472
}
13161473

13171474
fn stop(&self, session: &RunnerSession) -> bool {
1318-
self.stopped_leases
1319-
.borrow_mut()
1320-
.push(session.remote_daemon_lease_id.clone().expect("lease"));
1321-
true
1475+
let lease_id = session.remote_daemon_lease_id.clone().expect("lease");
1476+
self.stopped_leases.borrow_mut().push(lease_id.clone());
1477+
!self.stop_failures.borrow().contains(&lease_id)
13221478
}
13231479

13241480
fn terminate_tunnel(&self, pid: u32) {
13251481
self.terminated_pids.borrow_mut().push(pid);
13261482
}
13271483
}
13281484

1485+
#[test]
1486+
fn endpoint_fallback_retires_only_the_authoritatively_idle_draining_generation() {
1487+
test_support::with_isolated_home(|_| {
1488+
let a = session("lease-a", "daemon-a", Some(101));
1489+
let b = session("lease-b", "daemon-b", Some(202));
1490+
record_job("runner-a", &a, "job-a").expect("record A job");
1491+
activate(
1492+
"runner-a",
1493+
&a,
1494+
"lease-b".to_string(),
1495+
b.clone(),
1496+
&["job-a".to_string()],
1497+
)
1498+
.expect("activate B");
1499+
1500+
let local = FakeEndpointOperations::default();
1501+
local
1502+
.terminal_reconcile_failures
1503+
.borrow_mut()
1504+
.insert("lease-a".to_string());
1505+
local
1506+
.stop_failures
1507+
.borrow_mut()
1508+
.insert("lease-a".to_string());
1509+
local
1510+
.active_jobs
1511+
.borrow_mut()
1512+
.insert("lease-b".to_string(), 0);
1513+
let remote = FakeEndpointOperations::default();
1514+
remote
1515+
.active_jobs
1516+
.borrow_mut()
1517+
.insert("lease-a".to_string(), 0);
1518+
1519+
reconcile_with(
1520+
"runner-a",
1521+
Some(&b),
1522+
&FallbackGenerationEndpointOperations {
1523+
primary: &local,
1524+
fallback: &remote,
1525+
},
1526+
)
1527+
.expect("reconcile through fallback");
1528+
1529+
let registry = persisted_registry("runner-a");
1530+
assert_eq!(registry["admission_owner"], "lease-b");
1531+
assert!(registry["generations"].get("lease-a").is_none());
1532+
assert!(registry["generations"].get("lease-b").is_some());
1533+
assert!(registry["job_owners"].get("job-a").is_none());
1534+
assert_eq!(
1535+
remote.stopped_leases.borrow().as_slice(),
1536+
["lease-a"],
1537+
"the fallback stops only the drained endpoint"
1538+
);
1539+
assert_eq!(local.terminated_pids.borrow().as_slice(), [101]);
1540+
assert!(remote.terminated_pids.borrow().is_empty());
1541+
});
1542+
}
1543+
1544+
#[test]
1545+
fn ssh_fallback_authenticates_the_exact_generation_lease_and_pid() {
1546+
let expected = session("lease-a", "127.0.0.1", Some(101));
1547+
assert!(SshGenerationEndpointOperations::health_matches_session(
1548+
&expected,
1549+
&json!({
1550+
"pid": 42,
1551+
"lease": { "lease_id": "lease-a" },
1552+
"freshness": { "active_jobs": 0 },
1553+
}),
1554+
));
1555+
assert!(!SshGenerationEndpointOperations::health_matches_session(
1556+
&expected,
1557+
&json!({ "pid": 42, "lease": { "lease_id": "lease-reused" } }),
1558+
));
1559+
assert!(!SshGenerationEndpointOperations::health_matches_session(
1560+
&expected,
1561+
&json!({ "pid": 43, "lease": { "lease_id": "lease-a" } }),
1562+
));
1563+
}
1564+
13291565
fn persisted_registry(runner_id: &str) -> serde_json::Value {
13301566
let raw = std::fs::read_to_string(path(runner_id).expect("registry path"))
13311567
.expect("read registry");
@@ -2079,7 +2315,11 @@ mod tests {
20792315
};
20802316
assert_eq!(
20812317
serde_json::to_value(report).expect("serialize status")["generations"],
2082-
json!(projected)
2318+
json!({
2319+
"admission_owner": "build-b",
2320+
"draining": 1,
2321+
"total": 2,
2322+
})
20832323
);
20842324

20852325
let operations = FakeEndpointOperations::default();

0 commit comments

Comments
 (0)