fix(uninstall): never ghost a removed app in My Apps on cleanup residue
handle_package_uninstall lumped every teardown failure into one `errors` vec and returned Err on any of them BEFORE removing the package state entry — so a non-fatal cleanup hiccup (a slow/failed `sudo rm -rf` of a large data dir, a volume/network removal) left the app's containers gone but its entry in package_data → a ghost in My Apps, and the spawned task reverted it to Installed. Split the failures: container removal that even force-rm can't complete (app genuinely still present) keeps the entry + returns Err; everything after the containers are gone is best-effort. Remove the state entry as soon as the containers are gone — BEFORE the slow volume/data teardown — so My Apps updates immediately and residue can never ghost the app. set_uninstall_stage is a no-op once the entry is gone (if-let guard), so the later stages don't re-create it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
4346007d37
commit
e57514b690
@ -312,7 +312,16 @@ impl RpcHandler {
|
|||||||
|
|
||||||
let mut stopped = 0u32;
|
let mut stopped = 0u32;
|
||||||
let mut removed = 0u32;
|
let mut removed = 0u32;
|
||||||
let mut errors = Vec::new();
|
// Two distinct failure classes, kept separate so they don't get
|
||||||
|
// conflated (the old single `errors` vec did, which caused the "ghost in
|
||||||
|
// My Apps" bug): `container_errors` means a container could NOT be
|
||||||
|
// removed (force-rm failed too) — the app is genuinely still present, so
|
||||||
|
// we keep its state entry and surface a hard error. `cleanup_errors`
|
||||||
|
// means volume/network/data-dir teardown left residue — the containers
|
||||||
|
// are already gone, so the app IS uninstalled and MUST disappear from My
|
||||||
|
// Apps; the residue is logged but never ghosts the app.
|
||||||
|
let mut container_errors: Vec<String> = Vec::new();
|
||||||
|
let mut cleanup_errors: Vec<String> = Vec::new();
|
||||||
|
|
||||||
self.set_uninstall_stage(
|
self.set_uninstall_stage(
|
||||||
package_id,
|
package_id,
|
||||||
@ -370,7 +379,7 @@ impl RpcHandler {
|
|||||||
let msg =
|
let msg =
|
||||||
format!("Failed to remove {}: {}; {}", name, stderr.trim(), e);
|
format!("Failed to remove {}: {}; {}", name, stderr.trim(), e);
|
||||||
tracing::error!("Uninstall {}: {}", package_id, msg);
|
tracing::error!("Uninstall {}: {}", package_id, msg);
|
||||||
errors.push(msg);
|
container_errors.push(msg);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -379,12 +388,35 @@ impl RpcHandler {
|
|||||||
Err(force_err) => {
|
Err(force_err) => {
|
||||||
let msg = format!("Failed to remove {}: {}; {}", name, e, force_err);
|
let msg = format!("Failed to remove {}: {}; {}", name, e, force_err);
|
||||||
tracing::error!("Uninstall {}: {}", package_id, msg);
|
tracing::error!("Uninstall {}: {}", package_id, msg);
|
||||||
errors.push(msg);
|
container_errors.push(msg);
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A container that survived even force-remove means the app is NOT
|
||||||
|
// actually uninstalled — keep its state entry and fail so the spawned
|
||||||
|
// task reverts it to its prior state (and the user can retry), rather
|
||||||
|
// than orphaning a live container that's missing from My Apps.
|
||||||
|
if !container_errors.is_empty() {
|
||||||
|
tracing::error!(
|
||||||
|
"Uninstall {}: containers could not be removed: {:?}",
|
||||||
|
package_id,
|
||||||
|
container_errors
|
||||||
|
);
|
||||||
|
return Err(anyhow::anyhow!(
|
||||||
|
"Uninstall {} failed: {}",
|
||||||
|
package_id,
|
||||||
|
container_errors.join("; ")
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Containers are gone → the app is uninstalled. Remove its state entry
|
||||||
|
// NOW, before the (possibly slow, possibly fallible) volume/data
|
||||||
|
// teardown below, so My Apps updates immediately and a residue failure
|
||||||
|
// can never leave a ghost. Reinstall/scan no longer see a stale entry.
|
||||||
|
self.remove_package_state_entry(package_id).await;
|
||||||
|
|
||||||
self.set_uninstall_stage(package_id, "Cleaning up volumes")
|
self.set_uninstall_stage(package_id, "Cleaning up volumes")
|
||||||
.await;
|
.await;
|
||||||
// Avoid global Podman volume prune on production nodes: store-wide
|
// Avoid global Podman volume prune on production nodes: store-wide
|
||||||
@ -432,45 +464,55 @@ impl RpcHandler {
|
|||||||
let stderr = String::from_utf8_lossy(&o.stderr);
|
let stderr = String::from_utf8_lossy(&o.stderr);
|
||||||
let msg = format!("Failed to remove data {}: {}", dir, stderr.trim());
|
let msg = format!("Failed to remove data {}: {}", dir, stderr.trim());
|
||||||
tracing::error!("Uninstall {}: {}", package_id, msg);
|
tracing::error!("Uninstall {}: {}", package_id, msg);
|
||||||
errors.push(msg);
|
cleanup_errors.push(msg);
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
let msg = format!("Failed to remove data {}: {}", dir, e);
|
let msg = format!("Failed to remove data {}: {}", dir, e);
|
||||||
tracing::error!("Uninstall {}: {}", package_id, msg);
|
tracing::error!("Uninstall {}: {}", package_id, msg);
|
||||||
errors.push(msg);
|
cleanup_errors.push(msg);
|
||||||
}
|
}
|
||||||
_ => {}
|
_ => {}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if !errors.is_empty() {
|
// The app is already gone from My Apps (entry removed above). Residual
|
||||||
|
// volume/data cleanup failures are logged but NEVER ghost the app — a
|
||||||
|
// reinstall and the next uninstall both tolerate leftover dirs.
|
||||||
|
if !cleanup_errors.is_empty() {
|
||||||
tracing::error!(
|
tracing::error!(
|
||||||
"Uninstall {} completed with errors: {:?}",
|
"Uninstall {} removed but left cleanup residue: {:?}",
|
||||||
package_id,
|
package_id,
|
||||||
errors
|
cleanup_errors
|
||||||
);
|
);
|
||||||
return Err(anyhow::anyhow!(
|
|
||||||
"Uninstall {} partially failed: {}",
|
|
||||||
package_id,
|
|
||||||
errors.join("; ")
|
|
||||||
));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
tracing::info!(
|
tracing::info!(
|
||||||
"Uninstall {} complete: stopped={}, removed={}",
|
"Uninstall {} complete: stopped={}, removed={}, cleanup_errors={}",
|
||||||
package_id,
|
package_id,
|
||||||
stopped,
|
stopped,
|
||||||
removed
|
removed,
|
||||||
|
cleanup_errors.len()
|
||||||
);
|
);
|
||||||
|
|
||||||
// Immediately remove from in-memory state so the UI updates without
|
Ok(serde_json::json!({
|
||||||
// waiting for the scanner's absence threshold (3 scans × 60s each).
|
"status": "uninstalled",
|
||||||
{
|
"stopped": stopped,
|
||||||
|
"removed": removed,
|
||||||
|
"cleanup_warnings": cleanup_errors,
|
||||||
|
}))
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Remove a package's entry (and any alias keys) from persisted state so it
|
||||||
|
/// disappears from My Apps immediately, without waiting for the scanner's
|
||||||
|
/// absence threshold (3 scans × 60s). Called as soon as an uninstall has
|
||||||
|
/// removed the app's containers — before the slower volume/data teardown —
|
||||||
|
/// so a residue failure can never leave a ghost entry behind.
|
||||||
|
async fn remove_package_state_entry(&self, package_id: &str) {
|
||||||
let (mut data, _rev) = self.state_manager.get_snapshot().await;
|
let (mut data, _rev) = self.state_manager.get_snapshot().await;
|
||||||
let before = data.package_data.len();
|
let before = data.package_data.len();
|
||||||
data.package_data.remove(package_id);
|
data.package_data.remove(package_id);
|
||||||
// Also remove any alias keys (e.g. "bitcoin-knots" vs "bitcoin")
|
// Also remove any alias keys (e.g. "bitcoin-knots" vs "bitcoin").
|
||||||
let aliases: Vec<String> = data
|
let aliases: Vec<String> = data
|
||||||
.package_data
|
.package_data
|
||||||
.keys()
|
.keys()
|
||||||
@ -489,13 +531,6 @@ impl RpcHandler {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
Ok(serde_json::json!({
|
|
||||||
"status": "uninstalled",
|
|
||||||
"stopped": stopped,
|
|
||||||
"removed": removed,
|
|
||||||
}))
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Start a bundled app (create container from pre-loaded image if needed).
|
/// Start a bundled app (create container from pre-loaded image if needed).
|
||||||
pub(in crate::api::rpc) async fn handle_bundled_app_start(
|
pub(in crate::api::rpc) async fn handle_bundled_app_start(
|
||||||
&self,
|
&self,
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user