diff --git a/app/src-tauri/src/commands/update_commands.rs b/app/src-tauri/src/commands/update_commands.rs index c3d571a..bb5d129 100644 --- a/app/src-tauri/src/commands/update_commands.rs +++ b/app/src-tauri/src/commands/update_commands.rs @@ -169,7 +169,7 @@ fn pick_update<'a>( } /// Parse a semver string like "0.2.5" -> (0, 2, 5) -fn parse_semver(version: &str) -> Option<(u32, u32, u32)> { +pub(crate) fn parse_semver(version: &str) -> Option<(u32, u32, u32)> { let clean = version.trim_start_matches('v'); let parts: Vec<&str> = clean.split('.').collect(); if parts.len() >= 3 { diff --git a/app/src-tauri/src/docker/gateway.rs b/app/src-tauri/src/docker/gateway.rs index 1cfb3d8..97f94eb 100644 --- a/app/src-tauri/src/docker/gateway.rs +++ b/app/src-tauri/src/docker/gateway.rs @@ -51,11 +51,26 @@ const GATEWAY_CONFIG_VOLUME: &str = "triple-c-gateway-config"; /// lockstep. const GATEWAY_REGISTRY_IMAGE: &str = "ghcr.io/berriai/litellm:v1.106.6"; -/// Local builds are tagged with the pin (`triple-c-gateway:v1.106.6`), never a -/// floating tag. A build made for an older pin then simply doesn't match after -/// an app update, instead of quietly winning over the new pinned image the way -/// the old `triple-c-gateway:latest` tag did. Builds tagged `latest` by earlier -/// app versions are no longer used. +/// The digest [`GATEWAY_REGISTRY_IMAGE`] is pinned to — the same one the +/// Dockerfile's `FROM` uses. Pulls and lookups go by digest: a tag can be +/// re-pushed upstream, a digest cannot. +const GATEWAY_REGISTRY_DIGEST: &str = + "sha256:37e2dafb088a7723beb998590682957386dfc83ffea1848486f8e0e5cfac0b06"; + +fn registry_image_ref() -> String { + let repo = GATEWAY_REGISTRY_IMAGE + .rsplit_once(':') + .map(|(repo, _)| repo) + .unwrap_or(GATEWAY_REGISTRY_IMAGE); + format!("{}@{}", repo, GATEWAY_REGISTRY_DIGEST) +} + +/// Local builds are tagged with the pin plus a hash of the embedded build +/// context (`triple-c-gateway:v1.106.6-1a2b3c4d`), never a floating tag. A +/// build made by an app version with a different pin *or* a different +/// Dockerfile/config then simply doesn't match, instead of quietly winning +/// over the current image the way the old `triple-c-gateway:latest` tag did. +/// Builds tagged `latest` by earlier app versions are no longer used. const GATEWAY_LOCAL_REPO: &str = "triple-c-gateway"; /// The LiteLLM release this app is pinned to — the tag of @@ -68,7 +83,13 @@ fn pinned_litellm_version() -> &'static str { } fn local_image_ref() -> String { - format!("{}:{}", GATEWAY_LOCAL_REPO, pinned_litellm_version()) + let context = sha256_hex(&format!("{}\0{}", GATEWAY_DOCKERFILE, GATEWAY_DEFAULT_CONFIG)); + format!( + "{}:{}-{}", + GATEWAY_LOCAL_REPO, + pinned_litellm_version(), + &context[..8] + ) } /// The image the gateway container should run, and its content id. The id goes @@ -90,7 +111,7 @@ fn choose_gateway_image(local_id: Option, registry_id: Option) - }) .or_else(|| { registry_id.map(|id| ChosenImage { - reference: GATEWAY_REGISTRY_IMAGE.to_string(), + reference: registry_image_ref(), id, }) }) @@ -109,18 +130,34 @@ impl PinnedImages { } } -/// Other Docker errors are returned so "Docker is unreachable" is not mistaken -/// for "image missing". +/// A failed lookup of one image doesn't hide the other. An error is returned +/// only when it leaves the answer unknown — so "Docker is unreachable" is not +/// mistaken for "image missing". async fn pinned_images() -> Result { - let local_ref = local_image_ref(); + let (local_ref, registry_ref) = (local_image_ref(), registry_image_ref()); let (local, registry) = tokio::join!( super::migration::image_id(&local_ref), - super::migration::image_id(GATEWAY_REGISTRY_IMAGE) + super::migration::image_id(®istry_ref) ); - Ok(PinnedImages { - local: local?, - registry: registry?, - }) + match (local, registry) { + (Ok(local), Ok(registry)) => Ok(PinnedImages { local, registry }), + // The local build is preferred, so it alone decides when present. + (Ok(Some(local)), Err(e)) => { + log::warn!("{}", e); + Ok(PinnedImages { + local: Some(local), + registry: None, + }) + } + (Err(e), Ok(Some(registry))) => { + log::warn!("{}", e); + Ok(PinnedImages { + local: None, + registry: Some(registry), + }) + } + (Err(e), _) | (_, Err(e)) => Err(e), + } } /// Whether an existing gateway container runs something other than the image @@ -139,30 +176,10 @@ fn container_image_outdated( } } -/// Pull the pinned upstream image if neither pinned image is present. Called -/// at app launch *after* the gateway is up on whatever image it has (never on -/// Start or a settings save, which must not stall for a multi-hundred-MB -/// download); the caller then re-runs `ensure` to move it onto the new pin. -/// Returns whether a pull happened. -pub async fn pull_pinned_image_if_missing(settings: &GatewaySettings) -> Result { - // A gateway that can't start on configuration grounds doesn't need the - // download yet. - let configured = !settings.valid_models().is_empty() && secure::has_gateway_api_key(); - if !configured || pinned_images().await?.chosen().is_some() { - return Ok(false); - } - log::info!( - "No gateway image for LiteLLM {} — pulling {}", - pinned_litellm_version(), - GATEWAY_REGISTRY_IMAGE - ); - super::image::pull_image(GATEWAY_REGISTRY_IMAGE, |_| {}).await?; - Ok(true) -} - -/// Drop `triple-c-gateway:*` tags other than the current pin — the old -/// floating `latest`, and builds for earlier pins — so a superseded LiteLLM -/// build doesn't sit on disk forever as a tagged orphan. Best effort. +/// Drop superseded `triple-c-gateway:*` tags — the old floating `latest`, +/// builds for earlier pins, and earlier builds of this pin — so they don't sit +/// on disk forever as tagged orphans. Best effort, and never forced: an image +/// still used by a container keeps its tag. async fn remove_superseded_local_builds() { let Ok(docker) = get_docker() else { return }; let filters: HashMap> = HashMap::from([( @@ -185,43 +202,50 @@ async fn remove_superseded_local_builds() { let current = local_image_ref(); for tag in images.iter().flat_map(|i| i.repo_tags.iter()) { if is_superseded_local_build(tag, ¤t) { - if let Err(e) = super::container::remove_image_by_name(tag).await { - log::warn!("Could not remove old gateway build {}: {}", tag, e); + match docker + .remove_image( + tag, + Some(bollard::image::RemoveImageOptions { + force: false, + noprune: false, + }), + None, + ) + .await + { + Ok(_) => log::info!("Removed superseded gateway build {}", tag), + Err(e) => log::debug!("Kept gateway build {}: {}", tag, e), } } } } -/// Only the old floating `latest` and builds for an *older* pin. A newer pin -/// (an app downgrade, another branch) and any custom tag are left alone. +/// `latest`, a build for an older pin, or an earlier build of the current pin. +/// A newer pin (an app downgrade, another branch) and any custom tag are left +/// alone. fn is_superseded_local_build(tag: &str, current: &str) -> bool { - let Some(version) = tag + let Some(name) = tag .strip_prefix(GATEWAY_LOCAL_REPO) .and_then(|rest| rest.strip_prefix(':')) else { return false; }; - if version == "latest" { + if name == "latest" { return true; } - let current = current.rsplit_once(':').map(|(_, v)| v).unwrap_or_default(); - match (parse_release(version), parse_release(current)) { - (Some(old), Some(cur)) => old < cur, + if tag == current { + return false; + } + let version = name.split_once('-').map(|(v, _)| v).unwrap_or(name); + match ( + crate::commands::update_commands::parse_semver(version), + crate::commands::update_commands::parse_semver(pinned_litellm_version()), + ) { + (Some(old), Some(pin)) => old <= pin, _ => false, } } -/// `v1.106.6` → `(1, 106, 6)`; anything else (custom tags) → `None`. -fn parse_release(tag: &str) -> Option<(u32, u32, u32)> { - let mut parts = tag.strip_prefix('v')?.split('.'); - let parsed = ( - parts.next()?.parse().ok()?, - parts.next()?.parse().ok()?, - parts.next()?.parse().ok()?, - ); - parts.next().is_none().then_some(parsed) -} - const GATEWAY_DOCKERFILE: &str = include_str!("../../../../gateway-container/Dockerfile"); const GATEWAY_DEFAULT_CONFIG: &str = include_str!("../../../../gateway-container/config.yaml"); @@ -697,7 +721,12 @@ async fn create_gateway_container( async fn remove_gateway_container(docker: &Docker, id: &str, state: &str) -> Result<(), String> { if state == "running" { docker - .stop_container(id, None::) + .stop_container( + id, + Some(StopContainerOptions { + t: GATEWAY_STOP_GRACE_SECS, + }), + ) .await .map_err(|e| format!("Failed to stop gateway container: {}", e))?; } @@ -752,10 +781,10 @@ async fn ensure_gateway_running_locked( let on_pin = chosen.is_some(); let image = match chosen { Some(image) => image, - // No pinned image yet (the launch-time pull failed or is still - // running). Never tear down a working gateway over that: keep it on - // the image it already runs — recreated with the new config if that - // changed — and let status report it as outdated. + // No pinned image yet (after an app update moved the pin, until the + // user pulls or builds). Never tear down a working gateway over that: + // keep it on the image it already runs — recreated with the new + // config if that changed — and let status report it as outdated. None => match existing.as_ref().and_then(|c| c.3.clone()) { Some(id) => { log::warn!( @@ -828,7 +857,8 @@ async fn ensure_gateway_running_locked( image.reference ); if on_pin { - remove_superseded_local_builds().await; + // Off the lock and off the return path. + tokio::spawn(remove_superseded_local_builds()); } get_gateway_status(settings).await @@ -888,7 +918,7 @@ pub async fn pull_gateway_image(on_progress: F) -> Result<(), String> where F: Fn(String) + Send + 'static, { - super::image::pull_image(GATEWAY_REGISTRY_IMAGE, on_progress).await + super::image::pull_image(®istry_image_ref(), on_progress).await } pub async fn build_gateway_image(on_progress: F) -> Result<(), String> @@ -992,10 +1022,23 @@ mod tests { } #[test] - fn local_builds_are_tagged_with_the_pin() { + fn local_builds_are_tagged_with_the_pin_and_context() { let v = pinned_litellm_version(); assert!(v.starts_with('v') && !v.contains(['/', '@', ':']), "bad pin tag {v:?}"); - assert_eq!(local_image_ref(), format!("triple-c-gateway:{}", v)); + let tag = local_image_ref(); + let suffix = tag + .strip_prefix(&format!("triple-c-gateway:{}-", v)) + .expect("tag is repo:pin-hash"); + assert_eq!(suffix.len(), 8); + assert!(suffix.chars().all(|c| c.is_ascii_hexdigit())); + } + + #[test] + fn registry_lookups_go_by_digest() { + assert_eq!( + registry_image_ref(), + format!("ghcr.io/berriai/litellm@{}", GATEWAY_REGISTRY_DIGEST) + ); } #[test] @@ -1007,7 +1050,7 @@ mod tests { .find(|l| l.starts_with("FROM ")) .expect("Dockerfile has a FROM line"); assert!( - from.starts_with(&format!("FROM {}@sha256:", GATEWAY_REGISTRY_IMAGE)), + from.trim() == format!("FROM {}@{}", GATEWAY_REGISTRY_IMAGE, GATEWAY_REGISTRY_DIGEST), "gateway Dockerfile FROM ({}) is out of lockstep with {}", from, GATEWAY_REGISTRY_IMAGE @@ -1022,14 +1065,14 @@ mod tests { ); assert_eq!( choose_gateway_image(None, Some("sha256:reg".into())), - Some(chosen(GATEWAY_REGISTRY_IMAGE, "sha256:reg")) + Some(chosen(®istry_image_ref(), "sha256:reg")) ); assert_eq!(choose_gateway_image(None, None), None); } #[test] fn outdated_matches_what_ensure_would_recreate() { - let pinned = chosen(GATEWAY_REGISTRY_IMAGE, "sha256:new"); + let pinned = chosen(®istry_image_ref(), "sha256:new"); assert!(!container_image_outdated(Some("sha256:new"), Some(Some(&pinned)))); assert!(container_image_outdated(Some("sha256:old"), Some(Some(&pinned)))); // No pinned image at all: whatever runs is not it. @@ -1040,16 +1083,19 @@ mod tests { } #[test] - fn only_latest_and_older_pins_are_superseded() { - let current = "triple-c-gateway:v1.106.6"; - assert!(is_superseded_local_build("triple-c-gateway:latest", current)); - assert!(is_superseded_local_build("triple-c-gateway:v1.82.6", current)); - assert!(is_superseded_local_build("triple-c-gateway:v1.106.5", current)); - assert!(!is_superseded_local_build(current, current)); - assert!(!is_superseded_local_build("triple-c-gateway:v1.110.0", current)); - assert!(!is_superseded_local_build("triple-c-gateway:dev", current)); - assert!(!is_superseded_local_build("triple-c-gateway-other:latest", current)); - assert!(!is_superseded_local_build("ghcr.io/berriai/litellm:v1.82.6", current)); + fn only_latest_older_pins_and_older_builds_are_superseded() { + let v = pinned_litellm_version(); + let current = local_image_ref(); + assert!(is_superseded_local_build("triple-c-gateway:latest", ¤t)); + assert!(is_superseded_local_build("triple-c-gateway:v1.82.6", ¤t)); + assert!(is_superseded_local_build("triple-c-gateway:v1.82.6-0badc0de", ¤t)); + // An earlier build of this pin, from a different Dockerfile/config. + assert!(is_superseded_local_build(&format!("triple-c-gateway:{v}-0badc0de"), ¤t)); + assert!(!is_superseded_local_build(¤t, ¤t)); + assert!(!is_superseded_local_build("triple-c-gateway:v9.0.0-0badc0de", ¤t)); + assert!(!is_superseded_local_build("triple-c-gateway:dev", ¤t)); + assert!(!is_superseded_local_build("triple-c-gateway-other:latest", ¤t)); + assert!(!is_superseded_local_build("ghcr.io/berriai/litellm:v1.82.6", ¤t)); } #[test] diff --git a/app/src-tauri/src/docker/image.rs b/app/src-tauri/src/docker/image.rs index 7c0970a..d1556cb 100644 --- a/app/src-tauri/src/docker/image.rs +++ b/app/src-tauri/src/docker/image.rs @@ -69,25 +69,32 @@ pub async fn get_local_image_digest(image_name: &str) -> Result, Ok(None) } +/// Split an image reference into the `fromImage` and `tag` the pull API takes. +/// A digest reference (`repo@sha256:…`, optionally `repo:tag@sha256:…`) is +/// pulled by digest: the API takes the digest in the `tag` field. +fn split_image_ref(image_name: &str) -> (&str, &str) { + if let Some((name, digest)) = image_name.split_once('@') { + let repo = match name.rfind(':') { + Some(pos) if !name[pos + 1..].contains('/') => &name[..pos], + _ => name, + }; + return (repo, digest); + } + match image_name.rfind(':') { + // The colon is part of a port (e.g., host:port/repo), not a tag. + Some(pos) if image_name[pos + 1..].contains('/') => (image_name, "latest"), + Some(pos) => (&image_name[..pos], &image_name[pos + 1..]), + None => (image_name, "latest"), + } +} + pub async fn pull_image(image_name: &str, on_progress: F) -> Result<(), String> where F: Fn(String) + Send + 'static, { let docker = get_docker()?; - // Parse image name into from_image and tag - let (from_image, tag) = if let Some(pos) = image_name.rfind(':') { - // Check that the colon is part of a tag, not a port - let after_colon = &image_name[pos + 1..]; - if after_colon.contains('/') { - // The colon is part of a port (e.g., host:port/repo) - (image_name, "latest") - } else { - (&image_name[..pos], after_colon) - } - } else { - (image_name, "latest") - }; + let (from_image, tag) = split_image_ref(image_name); let options = CreateImageOptions { from_image, @@ -390,3 +397,19 @@ mod apt_refresh_tests { assert!(BUILD_WORKFLOW.contains("push: ${{ gitea.event_name != 'pull_request' }}")); } } + +#[cfg(test)] +mod pull_ref_tests { + use super::split_image_ref; + + #[test] + fn split_image_ref_handles_tags_ports_and_digests() { + assert_eq!(split_image_ref("repo:v1"), ("repo", "v1")); + assert_eq!(split_image_ref("repo"), ("repo", "latest")); + assert_eq!(split_image_ref("host:5000/repo"), ("host:5000/repo", "latest")); + assert_eq!(split_image_ref("host:5000/repo:v1"), ("host:5000/repo", "v1")); + assert_eq!(split_image_ref("ghcr.io/a/b@sha256:abc"), ("ghcr.io/a/b", "sha256:abc")); + assert_eq!(split_image_ref("ghcr.io/a/b:v1@sha256:abc"), ("ghcr.io/a/b", "sha256:abc")); + assert_eq!(split_image_ref("host:5000/b@sha256:abc"), ("host:5000/b", "sha256:abc")); + } +} diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index 5294f47..70f905f 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -398,29 +398,8 @@ pub fn run() { let cancel = lifecycle_setup.cancellation(); let handle = tauri::async_runtime::spawn(async move { autostart_with_retry("Model gateway", cancel, || async { - // Start on whatever image is present first, so the - // gateway isn't down for a download. Then, if an app - // update moved the LiteLLM pin, fetch the new image and - // move the gateway onto it. A failed pull only logs: - // the gateway keeps serving and Settings says it's - // outdated. - let first = docker::gateway::ensure_gateway_running(&gateway_settings).await; - let pulled = match docker::gateway::pull_pinned_image_if_missing( - &gateway_settings, - ) - .await - { - Ok(pulled) => pulled, - Err(e) => { - log::warn!("Model gateway image pull failed: {}", e); - false - } - }; - let status = if pulled { - docker::gateway::ensure_gateway_running(&gateway_settings).await? - } else { - first? - }; + let status = + docker::gateway::ensure_gateway_running(&gateway_settings).await?; if status.running { log::info!( "Model gateway auto-started on port {}", diff --git a/app/src/components/settings/GatewaySettings.test.tsx b/app/src/components/settings/GatewaySettings.test.tsx index c4a0651..6bcf535 100644 --- a/app/src/components/settings/GatewaySettings.test.tsx +++ b/app/src/components/settings/GatewaySettings.test.tsx @@ -131,6 +131,7 @@ describe("GatewaySettings", () => { render(); expect(await screen.findByRole("button", { name: "Start" })).toBeInTheDocument(); - expect(screen.getByText("Stopped")).toBeInTheDocument(); + // Starting it keeps the old LiteLLM, so say so. + expect(screen.getByText(/Stopped — LiteLLM update available/)).toBeInTheDocument(); }); }); diff --git a/app/src/components/settings/GatewaySettings.tsx b/app/src/components/settings/GatewaySettings.tsx index 624f1bd..d1ff86f 100644 --- a/app/src/components/settings/GatewaySettings.tsx +++ b/app/src/components/settings/GatewaySettings.tsx @@ -196,7 +196,9 @@ export default function GatewaySettings() { : `Running on port ${status.port} — LiteLLM update available, Pull or Build to get it` : `Running on port ${status.port}` : status?.container_exists - ? "Stopped" + ? status.image_outdated && !status.image_exists + ? "Stopped — LiteLLM update available, Pull or Build to get it" + : "Stopped" : !status?.image_exists ? "No image" : "Image ready";