fix(aws): forget a stale stopped mark on a project's unattached target id
The death watch's stopped marks are keyed by target id, and an unattached AWS project's id (http://unattached.invalid/<id>/) is reused by every later unattached spell, but only an endpoint URL's mark was ever removed. A stale mark could make the watch report a live, billed task as stopped. Registering a tunnel now clears the project's placeholder mark, and reconcile forgets it before asking ECS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
884a665e7d
commit
33eb63d907
4 files changed
+77
-2
No files matched your search
@@ -1946,8 +1946,11 @@ pub async fn reconcile_aws_projects(state: State<'_, AppState>) -> Result<Vec<Pr
|
||||
// A target id left from the last run is a loopback URL whose port a new
|
||||
// tunnel (any project's) may now own; until a re-attach registers, the
|
||||
// project gets an id no tunnel can have (m9).
|
||||
// A stopped mark a past unattached spell left on that id is forgotten
|
||||
// first: reconcile asks ECS now, and a stale mark would make the death
|
||||
// watch report a live, billed task as stopped.
|
||||
for p in &projects {
|
||||
if let Some(id) = crate::host::aws_host::unattached_container_id(p) {
|
||||
if let Some(id) = crate::host::aws_host::prepare_unattached(p) {
|
||||
let _ = state.projects_store.set_container_id(&p.id, Some(id));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -928,10 +928,23 @@ pub fn unattached_container_id(p: &Project) -> Option<String> {
|
||||
if tunnels::get_endpoint(&p.id).is_some_and(|ep| ep.base_url.as_str() == stored) {
|
||||
return None;
|
||||
}
|
||||
let placeholder = format!("http://unattached.invalid/{}/", p.id);
|
||||
let placeholder = tunnels::unattached_id(&p.id);
|
||||
(stored != placeholder).then_some(placeholder)
|
||||
}
|
||||
|
||||
/// Before reconcile asks ECS about `p`: a project with no registered tunnel
|
||||
/// is (or is about to be) known by its [`tunnels::unattached_id`], which a
|
||||
/// past unattached spell may have left marked stopped. Reconcile is the
|
||||
/// authority about the task now, so that mark goes; the watch re-marks it if
|
||||
/// the task really is stopped. Returns the placeholder to store, if any.
|
||||
pub fn prepare_unattached(p: &Project) -> Option<String> {
|
||||
if !p.is_aws() || tunnels::get_endpoint(&p.id).is_some() {
|
||||
return None;
|
||||
}
|
||||
tunnels::forget_stopped(&tunnels::unattached_id(&p.id));
|
||||
unattached_container_id(p)
|
||||
}
|
||||
|
||||
/// Reconcile one AWS project the store has as Running (or Starting/
|
||||
/// Stopping/Error left by a crash): a gone task makes it Stopped (its task
|
||||
/// ARN cleared); a live one keeps it Running and its ARN stored. An ECS
|
||||
|
||||
@@ -754,6 +754,26 @@ async fn reconcile_falls_back_to_a_newer_live_task_when_the_stored_one_stopped()
|
||||
assert!(fake.called("list"), "it looked for another task first");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reconcile_forgets_a_stale_stop_on_the_unattached_id_before_reusing_it() {
|
||||
use crate::host::tunnels;
|
||||
let mut p = aws_project(StorageMode::Ephemeral);
|
||||
p.status = ProjectStatus::Running;
|
||||
p.aws.as_mut().unwrap().task_arn = Some(task_arn("u2"));
|
||||
let placeholder = tunnels::unattached_id(&p.id);
|
||||
// A past unattached spell's task was seen stopped under this id.
|
||||
tunnels::mark_stopped(&placeholder, StopKind::Spot);
|
||||
p.container_id = Some("http://127.0.0.1:61400/".into());
|
||||
assert_eq!(prepare_unattached(&p), Some(placeholder.clone()));
|
||||
assert!(!tunnels::is_stopped(&placeholder), "the live task must not read as stopped");
|
||||
// Already on the placeholder: nothing to store, the mark still goes.
|
||||
p.container_id = Some(placeholder.clone());
|
||||
tunnels::mark_stopped(&placeholder, StopKind::Spot);
|
||||
assert_eq!(prepare_unattached(&p), None);
|
||||
assert!(!tunnels::is_stopped(&placeholder));
|
||||
assert_eq!(prepare_unattached(&Project::new("l".into(), Vec::new())), None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn an_unattached_aws_project_gets_a_target_id_no_tunnel_can_have() {
|
||||
// m9: the last run's loopback port may belong to another project's
|
||||
|
||||
@@ -89,6 +89,21 @@ fn notify() {
|
||||
changed().send_modify(|n| *n = n.wrapping_add(1));
|
||||
}
|
||||
|
||||
/// The target id an AWS project has while no tunnel is registered for it
|
||||
/// (see `aws_host::unattached_container_id`): never a loopback URL, so a
|
||||
/// port a new tunnel reuses can't be confused with it.
|
||||
pub fn unattached_id(project_id: &str) -> String {
|
||||
format!("http://unattached.invalid/{project_id}/")
|
||||
}
|
||||
|
||||
/// Forget that the watch saw the task behind `target_id` stopped. For a
|
||||
/// project's [`unattached_id`], which (unlike an endpoint) is reused by
|
||||
/// every later unattached spell of the same project: a stale entry would
|
||||
/// make a later live task read as stopped.
|
||||
pub fn forget_stopped(target_id: &str) {
|
||||
lock().stopped.remove(target_id);
|
||||
}
|
||||
|
||||
/// Register `tunnel` for `project_id`, returning the one it replaces (the
|
||||
/// caller closes it).
|
||||
pub fn register(project_id: &str, tunnel: LiveTunnel) -> Option<LiveTunnel> {
|
||||
@@ -96,6 +111,7 @@ pub fn register(project_id: &str, tunnel: LiveTunnel) -> Option<LiveTunnel> {
|
||||
let mut r = lock();
|
||||
*r.generation.entry(project_id.to_string()).or_default() += 1;
|
||||
r.stopped.remove(tunnel.endpoint.base_url.as_str());
|
||||
r.stopped.remove(&unattached_id(project_id));
|
||||
r.pending.remove(project_id);
|
||||
r.live.insert(project_id.to_string(), tunnel)
|
||||
};
|
||||
@@ -165,6 +181,7 @@ pub fn register_if_current(
|
||||
return Err(Box::new(tunnel));
|
||||
}
|
||||
r.stopped.remove(tunnel.endpoint.base_url.as_str());
|
||||
r.stopped.remove(&unattached_id(project_id));
|
||||
r.pending.remove(project_id);
|
||||
r.live.insert(project_id.to_string(), tunnel)
|
||||
};
|
||||
@@ -358,6 +375,28 @@ pub(crate) mod tests {
|
||||
assert_eq!(closes.load(Ordering::SeqCst), 2);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_registering_tunnel_clears_a_stop_seen_under_the_projects_unattached_id() {
|
||||
let p = pid("unattached-stale");
|
||||
let placeholder = unattached_id(&p);
|
||||
mark_stopped(&placeholder, StopKind::Spot);
|
||||
let closes = Arc::new(AtomicUsize::new(0));
|
||||
register(&p, fake_tunnel(61301, closes.clone()));
|
||||
assert!(!is_stopped(&placeholder), "a later unattached spell must not read as stopped");
|
||||
close(&p).await;
|
||||
|
||||
// A re-attach registering clears it too; and it can be forgotten directly.
|
||||
let q = pid("unattached-stale-reattach");
|
||||
mark_stopped(&unattached_id(&q), StopKind::Spot);
|
||||
let g = begin_reattach(&q);
|
||||
assert!(register_if_current(&q, g, fake_tunnel(61302, closes.clone())).is_ok());
|
||||
assert!(!is_stopped(&unattached_id(&q)));
|
||||
close(&q).await;
|
||||
mark_stopped(&unattached_id(&q), StopKind::Idle);
|
||||
forget_stopped(&unattached_id(&q));
|
||||
assert!(!is_stopped(&unattached_id(&q)));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_reattach_that_began_before_a_stop_does_not_register() {
|
||||
let closes = Arc::new(AtomicUsize::new(0));
|
||||
|
||||
Reference in new issue
Block a user