fix(install): failed installs no longer leave a phantom stopped app card
The optimistic package_data entry painted at install-spawn survived failure as a 'Stopped' tile for an app with zero footprint (netbird on the fresh ISO). On INSTALL FAIL with no container/user-stop marker for the app, the entry and the pre-saved dynamic app config are removed and an error notification is pushed; failures that left a real container keep today's behavior (visible exited state for retry/diagnosis). Install suite 39/39. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
archipelago
parent
9e2350a368
commit
62adc64703
@ -114,6 +114,16 @@ impl RpcHandler {
|
||||
Err(e) => {
|
||||
error!("package.install {} failed: {:#}", package_id_spawn, e);
|
||||
install_log(&format!("INSTALL FAIL: {} — {:#}", package_id_spawn, e)).await;
|
||||
// handle_package_install saves the catalog-provided
|
||||
// dynamic app config to /var/lib/archipelago/app-configs
|
||||
// BEFORE the install pipeline runs, so a failure can
|
||||
// strand that file (and the optimistic state entry) with
|
||||
// no container behind it. Probe once here; both cleanup
|
||||
// branches below only fire when the app has no footprint.
|
||||
// A retry re-saves the config (the frontend sends
|
||||
// containerConfig on every install), so removal is safe.
|
||||
let left_container =
|
||||
failed_install_left_container(&handler, &package_id_spawn).await;
|
||||
// Dependency-gate rejections happen BEFORE any resource
|
||||
// (container/image/data dir) exists for this package, so
|
||||
// keeping the optimistic entry would leave a phantom
|
||||
@ -123,30 +133,47 @@ impl RpcHandler {
|
||||
// surface the reason as a notification instead.
|
||||
if let Some(gate) = e.downcast_ref::<super::dependencies::DependencyGateError>()
|
||||
{
|
||||
let (mut data, _) = handler.state_manager.get_snapshot().await;
|
||||
data.package_data.remove(&package_id_spawn);
|
||||
data.notifications.push(crate::data_model::Notification {
|
||||
id: format!("install-deps-{package_id_spawn}"),
|
||||
level: crate::data_model::NotificationLevel::Error,
|
||||
title: format!("Could not install {package_id_spawn}"),
|
||||
message: gate.to_string(),
|
||||
timestamp: chrono::Utc::now().to_rfc3339(),
|
||||
app_id: Some(package_id_spawn.clone()),
|
||||
});
|
||||
while data.notifications.len() > 20 {
|
||||
data.notifications.remove(0);
|
||||
if !left_container {
|
||||
remove_dynamic_app_config(&package_id_spawn).await;
|
||||
}
|
||||
handler.state_manager.update_data(data).await;
|
||||
remove_entry_with_notification(
|
||||
&handler,
|
||||
&package_id_spawn,
|
||||
"install-deps",
|
||||
&gate.to_string(),
|
||||
)
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
// Don't remove the entry — that's what made the card
|
||||
// A failed install that left NO container behind has no
|
||||
// real footprint either — keeping the entry would leave
|
||||
// the same phantom "Stopped" tile in My Apps (and the
|
||||
// scanner-side absence eviction takes 3 scans to catch
|
||||
// it). Remove the saved config + entry and surface the
|
||||
// failure as a notification, exactly like the gate case.
|
||||
if !left_container {
|
||||
remove_dynamic_app_config(&package_id_spawn).await;
|
||||
remove_entry_with_notification(
|
||||
&handler,
|
||||
&package_id_spawn,
|
||||
"install-failed",
|
||||
&format!("Install failed: {:#}", e),
|
||||
)
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
// A container exists (crash-after-start kept for
|
||||
// visibility, retry over an existing install, upgrade) —
|
||||
// don't remove the entry, that's what made the card
|
||||
// vanish from My Apps mid-install / between retry-loop
|
||||
// attempts (e.g. tailscale's entrypoint failure). Leave
|
||||
// the entry visible with state=Stopped + the install
|
||||
// error in install_progress.message so the user can see
|
||||
// what went wrong and decide whether to retry or
|
||||
// uninstall. clear_install_progress would erase the
|
||||
// message, so we set it explicitly here instead.
|
||||
// message, so we set it explicitly here instead. The
|
||||
// phase is cleared (None) so no stale InstallPhase
|
||||
// lingers on the card.
|
||||
let err_msg = format!("Install failed: {:#}", e);
|
||||
let (mut data, _) = handler.state_manager.get_snapshot().await;
|
||||
if let Some(entry) = data.package_data.get_mut(&package_id_spawn) {
|
||||
@ -384,6 +411,77 @@ async fn flip_to_installing(state_manager: &StateManager, package_id: &str) {
|
||||
state_manager.update_data(data).await;
|
||||
}
|
||||
|
||||
/// True when the failed install still has a real footprint: any container
|
||||
/// belonging to `package_id` exists (any state — created/exited count too;
|
||||
/// the install-crash path deliberately keeps the exited container visible),
|
||||
/// or the app carries a user-stopped marker (Quadlet units run with `--rm`,
|
||||
/// so a cleanly user-stopped app legitimately has no podman record). Errors
|
||||
/// from the podman probe count as "exists" — never clean up on an uncertain
|
||||
/// reading.
|
||||
async fn failed_install_left_container(handler: &RpcHandler, package_id: &str) -> bool {
|
||||
if crate::crash_recovery::load_user_stopped(&handler.config.data_dir)
|
||||
.await
|
||||
.contains(package_id)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
match super::config::get_containers_for_app(package_id).await {
|
||||
Ok(containers) => !containers.is_empty(),
|
||||
Err(e) => {
|
||||
warn!(
|
||||
"install cleanup {}: container probe failed ({:#}); keeping saved config",
|
||||
package_id, e
|
||||
);
|
||||
true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Remove the catalog-provided dynamic app config that
|
||||
/// `handle_package_install` saved before the pipeline ran (mirror of the
|
||||
/// write in install.rs). Only called when the app has no container — for an
|
||||
/// existing install (retry/upgrade) the file is still the app's live runtime
|
||||
/// config and must be kept.
|
||||
async fn remove_dynamic_app_config(package_id: &str) {
|
||||
let config_path = format!("/var/lib/archipelago/app-configs/{}.json", package_id);
|
||||
match tokio::fs::remove_file(&config_path).await {
|
||||
Ok(()) => info!(
|
||||
"Removed dynamic app config for {} after failed install (no container)",
|
||||
package_id
|
||||
),
|
||||
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
|
||||
Err(e) => warn!(
|
||||
"Failed to remove dynamic app config for {}: {}",
|
||||
package_id, e
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
/// Remove the package's optimistic state entry (clearing any pending install
|
||||
/// phase with it) so the card reverts to installable, and surface the failure
|
||||
/// reason as an error notification instead.
|
||||
async fn remove_entry_with_notification(
|
||||
handler: &RpcHandler,
|
||||
package_id: &str,
|
||||
id_prefix: &str,
|
||||
message: &str,
|
||||
) {
|
||||
let (mut data, _) = handler.state_manager.get_snapshot().await;
|
||||
data.package_data.remove(package_id);
|
||||
data.notifications.push(crate::data_model::Notification {
|
||||
id: format!("{id_prefix}-{package_id}"),
|
||||
level: crate::data_model::NotificationLevel::Error,
|
||||
title: format!("Could not install {package_id}"),
|
||||
message: message.to_string(),
|
||||
timestamp: chrono::Utc::now().to_rfc3339(),
|
||||
app_id: Some(package_id.to_string()),
|
||||
});
|
||||
while data.notifications.len() > 20 {
|
||||
data.notifications.remove(0);
|
||||
}
|
||||
handler.state_manager.update_data(data).await;
|
||||
}
|
||||
|
||||
/// Flip an existing entry's state and return the pre-flip value (or None if
|
||||
/// no entry existed). Used for revert-on-failure.
|
||||
async fn flip_package_state(
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user