From c0941cbec78279bf264d596a11db5de46356a520 Mon Sep 17 00:00:00 2001 From: Ben Herila Date: Mon, 29 Jun 2026 07:19:25 +0000 Subject: [PATCH 1/7] feat(deployments): add per-project Docker image retention with nightly pruning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Built deployment images (e.g. careowner-211:latest) were never pruned, causing unbounded disk growth on busy hosts. This adds configurable retention so old images are removed automatically each night. - Add `image_retention_hours` column to `projects` (nullable i32; NULL falls back to the 48-hour system default) - Migration: m20260629_000001_add_image_retention_hours - `DockerCleanupService`: add `remove_image` to the `DockerClient` trait and `prune_old_deployment_images` which queries each project, finds deployments whose images are older than the project's retention period, and removes them - `DockerCleanupService`: add `default_image_retention_hours` field (default 48) and `with_default_image_retention_hours` builder - Expose `image_retention_hours` in the project API via `UpdateProjectSettingsRequest` and `ProjectResponse`; validated to 1โ€“8760 h Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_015a1UirztsZmSgG5aJw89kG --- .../src/services/docker_cleanup_service.rs | 147 ++++++++++++++++++ crates/temps-entities/src/projects.rs | 3 + ...260629_000001_add_image_retention_hours.rs | 39 +++++ crates/temps-migrations/src/migration/mod.rs | 2 + .../temps-projects/src/handlers/handlers.rs | 1 + crates/temps-projects/src/handlers/types.rs | 7 + crates/temps-projects/src/services/project.rs | 19 ++- crates/temps-projects/src/services/types.rs | 3 + 8 files changed, 219 insertions(+), 2 deletions(-) create mode 100644 crates/temps-migrations/src/migration/m20260629_000001_add_image_retention_hours.rs diff --git a/crates/temps-deployments/src/services/docker_cleanup_service.rs b/crates/temps-deployments/src/services/docker_cleanup_service.rs index 05dcb92e0..a04c6e655 100644 --- a/crates/temps-deployments/src/services/docker_cleanup_service.rs +++ b/crates/temps-deployments/src/services/docker_cleanup_service.rs @@ -17,6 +17,9 @@ pub trait DockerClient: Send + Sync { /// Remove unused Docker build cache async fn prune_builder_cache(&self, max_unused_days: i64) -> Result; + + /// Remove a specific image by name (e.g. "careowner:211") + async fn remove_image(&self, image_name: &str) -> Result<(), String>; } /// Statistics from Docker prune operations @@ -63,6 +66,27 @@ impl DockerClient for DefaultDockerClient { } } + async fn remove_image(&self, image_name: &str) -> Result<(), String> { + use bollard::Docker; + + let docker = Docker::connect_with_unix_defaults() + .map_err(|e| format!("Failed to connect to Docker daemon: {}", e))?; + + docker + .remove_image( + image_name, + Some(bollard::query_parameters::RemoveImageOptions { + force: true, + ..Default::default() + }), + None, + ) + .await + .map_err(|e| format!("Failed to remove image '{}': {}", image_name, e))?; + + Ok(()) + } + async fn prune_builder_cache(&self, max_unused_days: i64) -> Result { use bollard::query_parameters::PruneBuildOptionsBuilder; use bollard::Docker; @@ -116,6 +140,10 @@ pub struct DockerCleanupService { max_chunk_age_hours: u64, /// Maximum age of static asset cache entries in days (default: 7) max_asset_cache_age_days: i64, + /// System-wide default: how many hours to keep a built deployment image before + /// it is eligible for removal. Projects can override this via their + /// `image_retention_hours` column. (default: 48) + default_image_retention_hours: i64, } impl DockerCleanupService { @@ -133,6 +161,7 @@ impl DockerCleanupService { static_dir: None, max_chunk_age_hours: 24, max_asset_cache_age_days: 7, + default_image_retention_hours: 48, } } @@ -156,6 +185,11 @@ impl DockerCleanupService { self } + pub fn with_default_image_retention_hours(mut self, hours: i64) -> Self { + self.default_image_retention_hours = hours; + self + } + /// Calculate seconds until the next scheduled cleanup fn seconds_until_next_cleanup(&self) -> u64 { let now = chrono::Utc::now(); @@ -206,10 +240,104 @@ impl DockerCleanupService { } } + /// Remove deployment images that are older than their project's retention period. + /// + /// Queries all projects and their successful deployments, then removes the Docker + /// image for any deployment whose `created_at` is older than + /// `project.image_retention_hours` (falling back to `self.default_image_retention_hours`). + /// Images currently referenced by a running container are skipped โ€” Docker will + /// return an error and we log a warning rather than failing the whole pass. + async fn prune_old_deployment_images(&self) { + use sea_orm::{ColumnTrait, EntityTrait, QueryFilter}; + use temps_entities::{deployments, projects}; + + let projects_list = match projects::Entity::find() + .filter(projects::Column::IsDeleted.eq(false)) + .all(self.db.as_ref()) + .await + { + Ok(p) => p, + Err(e) => { + error!( + "Failed to query projects for image retention cleanup: {}", + e + ); + return; + } + }; + + let mut total_removed = 0u64; + let total_freed_mb = 0u64; + + for project in projects_list { + let retention_hours = project + .image_retention_hours + .map(|h| h as i64) + .unwrap_or(self.default_image_retention_hours); + + let cutoff = chrono::Utc::now() - chrono::Duration::hours(retention_hours); + + let old_deployments = match deployments::Entity::find() + .filter(deployments::Column::ProjectId.eq(project.id)) + .filter(deployments::Column::ImageName.is_not_null()) + .filter(deployments::Column::CreatedAt.lt(cutoff)) + .all(self.db.as_ref()) + .await + { + Ok(d) => d, + Err(e) => { + error!( + "Failed to query old deployments for project {}: {}", + project.id, e + ); + continue; + } + }; + + for deployment in old_deployments { + let image_name = match deployment.image_name { + Some(ref name) => name.clone(), + None => continue, + }; + + match self.docker_client.remove_image(&image_name).await { + Ok(()) => { + debug!( + "Removed old deployment image '{}' (deployment {}, project {})", + image_name, deployment.id, project.id + ); + total_removed += 1; + } + Err(e) => { + // Image may already be gone or in use โ€” warn but don't fail + warn!( + "Could not remove deployment image '{}' (deployment {}, project {}): {}", + image_name, deployment.id, project.id, e + ); + } + } + } + } + + if total_removed > 0 { + info!( + "โœ… Removed {} old deployment images, freed ~{} MB", + total_removed, total_freed_mb + ); + } else { + debug!("No old deployment images to remove"); + } + + let _ = total_freed_mb; // calculated per-image removal is non-trivial; reported as 0 for now + } + /// Perform the actual cleanup async fn perform_cleanup(&self) { info!("๐Ÿงน Starting nightly Docker cleanup"); + // Remove old deployment images per project retention policy + self.prune_old_deployment_images().await; + // Cleanup unused images match self.docker_client.prune_images(true).await { Ok(stats) => { @@ -491,6 +619,10 @@ mod tests { async fn prune_builder_cache(&self, _max_unused_days: i64) -> Result { self.prune_cache_result.clone() } + + async fn remove_image(&self, _image_name: &str) -> Result<(), String> { + Ok(()) + } } fn mock_db() -> Arc { @@ -531,4 +663,19 @@ mod tests { assert_eq!(service.max_cache_age_days, 14); } + + #[test] + fn test_default_image_retention_hours() { + let service = + DockerCleanupService::new(Arc::new(DefaultDockerClient), mock_db(), mock_file_store()); + assert_eq!(service.default_image_retention_hours, 48); + } + + #[test] + fn test_custom_image_retention_hours() { + let service = + DockerCleanupService::new(Arc::new(DefaultDockerClient), mock_db(), mock_file_store()) + .with_default_image_retention_hours(72); + assert_eq!(service.default_image_retention_hours, 72); + } } diff --git a/crates/temps-entities/src/projects.rs b/crates/temps-entities/src/projects.rs index b687e54fd..16d1327a8 100644 --- a/crates/temps-entities/src/projects.rs +++ b/crates/temps-entities/src/projects.rs @@ -107,6 +107,9 @@ pub struct Model { /// to FALSE; cross-project links to this project will then be suppressed. #[sea_orm(default_value = "true")] pub cross_project_trace_sharing: bool, + /// How long (in hours) to retain built Docker images before the nightly + /// cleanup removes them. NULL means use the system default (48 hours). + pub image_retention_hours: Option, } #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] diff --git a/crates/temps-migrations/src/migration/m20260629_000001_add_image_retention_hours.rs b/crates/temps-migrations/src/migration/m20260629_000001_add_image_retention_hours.rs new file mode 100644 index 000000000..337f35934 --- /dev/null +++ b/crates/temps-migrations/src/migration/m20260629_000001_add_image_retention_hours.rs @@ -0,0 +1,39 @@ +use sea_orm_migration::prelude::*; + +#[derive(DeriveMigrationName)] +pub struct Migration; + +#[async_trait::async_trait] +impl MigrationTrait for Migration { + async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { + manager + .alter_table( + Table::alter() + .table(Projects::Table) + .add_column( + ColumnDef::new(Projects::ImageRetentionHours) + .integer() + .null(), + ) + .to_owned(), + ) + .await + } + + async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { + manager + .alter_table( + Table::alter() + .table(Projects::Table) + .drop_column(Projects::ImageRetentionHours) + .to_owned(), + ) + .await + } +} + +#[derive(DeriveIden)] +enum Projects { + Table, + ImageRetentionHours, +} diff --git a/crates/temps-migrations/src/migration/mod.rs b/crates/temps-migrations/src/migration/mod.rs index 08d26519f..c4579d1c5 100644 --- a/crates/temps-migrations/src/migration/mod.rs +++ b/crates/temps-migrations/src/migration/mod.rs @@ -131,6 +131,7 @@ mod m20260627_000001_node_enrollment_tokens; mod m20260627_000002_create_ai_conversations; mod m20260628_000001_add_node_to_log_chunks; mod m20260628_000001_otel_spans_root_index; +mod m20260629_000001_add_image_retention_hours; mod m20260629_000001_otel_metrics_full_fidelity; mod m20260629_000002_add_provider_default_model; mod m20260630_000001_add_ai_pending_actions_and_write_toggle; @@ -284,6 +285,7 @@ impl MigratorTrait for Migrator { Box::new(m20260627_000002_create_ai_conversations::Migration), Box::new(m20260628_000001_add_node_to_log_chunks::Migration), Box::new(m20260628_000001_otel_spans_root_index::Migration), + Box::new(m20260629_000001_add_image_retention_hours::Migration), Box::new(m20260629_000001_otel_metrics_full_fidelity::Migration), Box::new(m20260629_000002_add_provider_default_model::Migration), Box::new(m20260630_000001_add_ai_pending_actions_and_write_toggle::Migration), diff --git a/crates/temps-projects/src/handlers/handlers.rs b/crates/temps-projects/src/handlers/handlers.rs index bd35a0b83..144f4a905 100644 --- a/crates/temps-projects/src/handlers/handlers.rs +++ b/crates/temps-projects/src/handlers/handlers.rs @@ -616,6 +616,7 @@ pub async fn update_project_settings( settings.ai_debug_chat_enabled, settings.ai_write_actions_enabled, settings.cross_project_trace_sharing, + settings.image_retention_hours, ) .await .map_err(Problem::from)?; diff --git a/crates/temps-projects/src/handlers/types.rs b/crates/temps-projects/src/handlers/types.rs index 73808ed1d..79474011a 100644 --- a/crates/temps-projects/src/handlers/types.rs +++ b/crates/temps-projects/src/handlers/types.rs @@ -312,6 +312,9 @@ pub struct ProjectResponse { /// OSS global-observability model where any OtelRead holder can query any /// project's telemetry). pub cross_project_trace_sharing: bool, + /// Hours to retain built Docker images before nightly cleanup. Null = system default (48 h). + #[serde(skip_serializing_if = "Option::is_none")] + pub image_retention_hours: Option, } #[derive(Debug, Serialize, Deserialize, ToSchema)] @@ -349,6 +352,7 @@ impl ProjectResponse { source_type: project.source_type, gitlab_webhook_id: project.gitlab_webhook_id, cross_project_trace_sharing: project.cross_project_trace_sharing, + image_retention_hours: project.image_retention_hours, deployment_config: DeploymentConfig { cpu_request: project .deployment_config @@ -572,6 +576,9 @@ pub struct UpdateProjectSettingsRequest { pub preview_envs_idle_timeout_seconds: Option, /// Wake timeout (seconds, 5..=120) for on-demand preview environments. pub preview_envs_wake_timeout_seconds: Option, + /// How long (hours) to retain built Docker images before nightly cleanup removes them. + /// Set to null to use the system default (48 hours). Valid range: 1โ€“8760. + pub image_retention_hours: Option, /// Preset-specific configuration (e.g., Dockerfile path for Docker preset) /// /// Example for Dockerfile preset: diff --git a/crates/temps-projects/src/services/project.rs b/crates/temps-projects/src/services/project.rs index 20cc7e6a8..1b603a89e 100644 --- a/crates/temps-projects/src/services/project.rs +++ b/crates/temps-projects/src/services/project.rs @@ -877,6 +877,7 @@ impl ProjectService { ai_debug_chat_enabled: Option, ai_write_actions_enabled: Option, cross_project_trace_sharing: Option, + image_retention_hours: Option, ) -> Result { // Validate preview env on-demand timeouts before touching the DB. // Mirrors DeploymentConfig::validate so the project-level defaults are @@ -897,6 +898,14 @@ impl ProjectService { ))); } } + if let Some(hours) = image_retention_hours { + if !(1..=8760).contains(&hours) { + return Err(ProjectError::InvalidInput(format!( + "image_retention_hours {} is not in valid range (1-8760)", + hours + ))); + } + } // Get the current project let mut project = projects::Entity::find_by_id(project_id) @@ -1047,11 +1056,12 @@ impl ProjectService { active_project.update(self.db.as_ref()).await?; } - // Update preview environment settings if any are provided + // Update preview environment settings and image retention if any are provided let needs_preview_update = enable_preview_environments.is_some() || preview_envs_on_demand.is_some() || preview_envs_idle_timeout_seconds.is_some() - || preview_envs_wake_timeout_seconds.is_some(); + || preview_envs_wake_timeout_seconds.is_some() + || image_retention_hours.is_some(); if needs_preview_update { // Reload project to ensure we have the latest state @@ -1077,6 +1087,9 @@ impl ProjectService { if let Some(wake) = preview_envs_wake_timeout_seconds { active_project.preview_envs_wake_timeout_seconds = Set(wake); } + if let Some(hours) = image_retention_hours { + active_project.image_retention_hours = Set(Some(hours)); + } active_project.update(self.db.as_ref()).await?; } @@ -2512,6 +2525,7 @@ impl ProjectService { source_type: db_project.source_type, gitlab_webhook_id: db_project.gitlab_webhook_id, cross_project_trace_sharing: db_project.cross_project_trace_sharing, + image_retention_hours: db_project.image_retention_hours, } } @@ -3050,6 +3064,7 @@ mod tests { None, None, None, // cross_project_trace_sharing + None, // image_retention_hours ) .await; diff --git a/crates/temps-projects/src/services/types.rs b/crates/temps-projects/src/services/types.rs index bbe32eb19..e5012cb40 100644 --- a/crates/temps-projects/src/services/types.rs +++ b/crates/temps-projects/src/services/types.rs @@ -82,6 +82,9 @@ pub struct Project { /// model). Operators can set false to suppress cross-project links to this /// project. pub cross_project_trace_sharing: bool, + /// How long (hours) to retain built Docker images before nightly cleanup. + /// None = use system default (48 h). + pub image_retention_hours: Option, } #[derive(Deserialize)] From f1ae755087a536b2ce4bc2e0e510c332628a6724 Mon Sep 17 00:00:00 2001 From: Ben Herila Date: Fri, 3 Jul 2026 21:32:36 +0000 Subject: [PATCH 2/7] fix(ai-chat): add missing image_retention_hours field to test project models The projects entity gained an image_retention_hours column; five test helper functions that construct projects::Model literals directly needed the new field added to keep compiling. --- crates/temps-ai-chat/src/handlers.rs | 1 + crates/temps-ai-chat/src/pending_actions.rs | 1 + crates/temps-ai-chat/src/providers/project.rs | 1 + crates/temps-ai-chat/src/providers/repo_tools.rs | 1 + crates/temps-ai-chat/src/service.rs | 1 + 5 files changed, 5 insertions(+) diff --git a/crates/temps-ai-chat/src/handlers.rs b/crates/temps-ai-chat/src/handlers.rs index 2d2e352f2..e8b5d4c43 100644 --- a/crates/temps-ai-chat/src/handlers.rs +++ b/crates/temps-ai-chat/src/handlers.rs @@ -1126,6 +1126,7 @@ mod tests { let now = chrono::Utc::now(); temps_entities::projects::Model { id, + image_retention_hours: None, name: "P".to_string(), repo_name: "r".to_string(), repo_owner: "o".to_string(), diff --git a/crates/temps-ai-chat/src/pending_actions.rs b/crates/temps-ai-chat/src/pending_actions.rs index a22d96f8c..21537709e 100644 --- a/crates/temps-ai-chat/src/pending_actions.rs +++ b/crates/temps-ai-chat/src/pending_actions.rs @@ -540,6 +540,7 @@ mod tests { let now = Utc::now(); temps_entities::projects::Model { id, + image_retention_hours: None, name: "test-project".to_string(), repo_name: "repo".to_string(), repo_owner: "owner".to_string(), diff --git a/crates/temps-ai-chat/src/providers/project.rs b/crates/temps-ai-chat/src/providers/project.rs index 9de19bfb7..906c94ee6 100644 --- a/crates/temps-ai-chat/src/providers/project.rs +++ b/crates/temps-ai-chat/src/providers/project.rs @@ -79,6 +79,7 @@ mod tests { let now = chrono::Utc::now(); projects::Model { id, + image_retention_hours: None, name: name.to_string(), repo_name: "repo".to_string(), repo_owner: "owner".to_string(), diff --git a/crates/temps-ai-chat/src/providers/repo_tools.rs b/crates/temps-ai-chat/src/providers/repo_tools.rs index 7db4f7528..67346007b 100644 --- a/crates/temps-ai-chat/src/providers/repo_tools.rs +++ b/crates/temps-ai-chat/src/providers/repo_tools.rs @@ -668,6 +668,7 @@ mod tests { let now = chrono::Utc::now(); projects::Model { id, + image_retention_hours: None, name: "test".to_string(), repo_name: "repo".to_string(), repo_owner: "owner".to_string(), diff --git a/crates/temps-ai-chat/src/service.rs b/crates/temps-ai-chat/src/service.rs index 3819b7ec8..6cf9003bd 100644 --- a/crates/temps-ai-chat/src/service.rs +++ b/crates/temps-ai-chat/src/service.rs @@ -1820,6 +1820,7 @@ mod tests { let now = Utc::now(); temps_entities::projects::Model { id, + image_retention_hours: None, name: name.to_string(), repo_name: "r".to_string(), repo_owner: "o".to_string(), From acf714bc9e9870011ac60b41cdecdd6b02d491de Mon Sep 17 00:00:00 2001 From: Ben Herila Date: Fri, 3 Jul 2026 23:10:22 +0000 Subject: [PATCH 3/7] fix(entities): add missing image_retention_hours field to remaining test project models Three more test helpers construct projects::Model literals directly and needed the new field: temps-agents (executor.rs, config_service.rs) and temps-notifications (vulnerability_notifications.rs). --- crates/temps-agents/src/services/config_service.rs | 1 + crates/temps-agents/src/services/executor.rs | 1 + crates/temps-notifications/src/vulnerability_notifications.rs | 1 + 3 files changed, 3 insertions(+) diff --git a/crates/temps-agents/src/services/config_service.rs b/crates/temps-agents/src/services/config_service.rs index 4b7deb8b4..678d31e5a 100644 --- a/crates/temps-agents/src/services/config_service.rs +++ b/crates/temps-agents/src/services/config_service.rs @@ -1003,6 +1003,7 @@ mod tests { fn make_project(id: i32, has_git: bool) -> projects::Model { projects::Model { id, + image_retention_hours: None, name: "test".into(), repo_name: "repo".into(), repo_owner: "owner".into(), diff --git a/crates/temps-agents/src/services/executor.rs b/crates/temps-agents/src/services/executor.rs index 62ffe88b5..a98cd38bd 100644 --- a/crates/temps-agents/src/services/executor.rs +++ b/crates/temps-agents/src/services/executor.rs @@ -4057,6 +4057,7 @@ mod tests { fn make_project(id: i32) -> projects::Model { projects::Model { id, + image_retention_hours: None, name: "test-app".into(), repo_name: "repo".into(), repo_owner: "testowner".into(), diff --git a/crates/temps-notifications/src/vulnerability_notifications.rs b/crates/temps-notifications/src/vulnerability_notifications.rs index 8be0c70df..b3368d51d 100644 --- a/crates/temps-notifications/src/vulnerability_notifications.rs +++ b/crates/temps-notifications/src/vulnerability_notifications.rs @@ -372,6 +372,7 @@ mod tests { let project = temps_entities::projects::Model { id: 1, + image_retention_hours: None, name: "My Project".to_string(), slug: "my-project".to_string(), repo_name: "my-repo".to_string(), From 1025ab43332da7124e205558b218494c22ec837f Mon Sep 17 00:00:00 2001 From: Ben Herila Date: Mon, 13 Jul 2026 11:03:37 +0000 Subject: [PATCH 4/7] fix(deployments): make image retention pruning safe --- .../src/services/docker_cleanup_service.rs | 181 ++++++++++++------ crates/temps-projects/src/handlers/types.rs | 32 +++- crates/temps-projects/src/services/project.rs | 6 +- 3 files changed, 157 insertions(+), 62 deletions(-) diff --git a/crates/temps-deployments/src/services/docker_cleanup_service.rs b/crates/temps-deployments/src/services/docker_cleanup_service.rs index a04c6e655..9a1b114e9 100644 --- a/crates/temps-deployments/src/services/docker_cleanup_service.rs +++ b/crates/temps-deployments/src/services/docker_cleanup_service.rs @@ -4,6 +4,7 @@ //! Runs as a background task scheduled at 2 AM UTC daily. use chrono::Timelike as _; +use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; use tokio::time::{sleep, Duration}; @@ -76,7 +77,7 @@ impl DockerClient for DefaultDockerClient { .remove_image( image_name, Some(bollard::query_parameters::RemoveImageOptions { - force: true, + force: false, ..Default::default() }), None, @@ -190,6 +191,27 @@ impl DockerCleanupService { self } + fn is_temps_managed_image(image_name: &str) -> bool { + image_name.starts_with("temps-") && !image_name.contains('/') + } + + fn record_image_retention_eligibility( + candidates: &mut HashMap, + image_name: &str, + referenced_at: chrono::DateTime, + cutoff: chrono::DateTime, + ) { + if !Self::is_temps_managed_image(image_name) { + return; + } + + let reference_is_expired = referenced_at < cutoff; + candidates + .entry(image_name.to_string()) + .and_modify(|eligible| *eligible &= reference_is_expired) + .or_insert(reference_is_expired); + } + /// Calculate seconds until the next scheduled cleanup fn seconds_until_next_cleanup(&self) -> u64 { let now = chrono::Utc::now(); @@ -242,93 +264,85 @@ impl DockerCleanupService { /// Remove deployment images that are older than their project's retention period. /// - /// Queries all projects and their successful deployments, then removes the Docker - /// image for any deployment whose `created_at` is older than - /// `project.image_retention_hours` (falling back to `self.default_image_retention_hours`). - /// Images currently referenced by a running container are skipped โ€” Docker will - /// return an error and we log a warning rather than failing the whole pass. + /// An image is eligible only when every deployment row that references it is older + /// than its owning project's retention period. This preserves images reused by a + /// newer rollback or promotion. Only Temps-managed local tags are considered; + /// registry images are left to Docker's normal cache policy. Docker removal is + /// non-forced, so images referenced by any container are retained. async fn prune_old_deployment_images(&self) { use sea_orm::{ColumnTrait, EntityTrait, QueryFilter}; use temps_entities::{deployments, projects}; - let projects_list = match projects::Entity::find() - .filter(projects::Column::IsDeleted.eq(false)) + let deployment_rows = match deployments::Entity::find() + .filter(deployments::Column::ImageName.is_not_null()) + .find_also_related(projects::Entity) .all(self.db.as_ref()) .await { - Ok(p) => p, + Ok(rows) => rows, Err(e) => { error!( - "Failed to query projects for image retention cleanup: {}", + "Failed to query deployment images for retention cleanup: {}", e ); return; } }; - let mut total_removed = 0u64; - let total_freed_mb = 0u64; - - for project in projects_list { + let now = chrono::Utc::now(); + let mut candidates = HashMap::new(); + for (deployment, project) in deployment_rows { + let Some(project) = project else { + warn!( + deployment_id = deployment.id, + "Skipping deployment image with no owning project" + ); + continue; + }; + let Some(image_name) = deployment.image_name.as_deref() else { + continue; + }; let retention_hours = project .image_retention_hours .map(|h| h as i64) .unwrap_or(self.default_image_retention_hours); + let cutoff = now - chrono::Duration::hours(retention_hours); - let cutoff = chrono::Utc::now() - chrono::Duration::hours(retention_hours); + Self::record_image_retention_eligibility( + &mut candidates, + image_name, + deployment.created_at, + cutoff, + ); + } - let old_deployments = match deployments::Entity::find() - .filter(deployments::Column::ProjectId.eq(project.id)) - .filter(deployments::Column::ImageName.is_not_null()) - .filter(deployments::Column::CreatedAt.lt(cutoff)) - .all(self.db.as_ref()) - .await - { - Ok(d) => d, + let mut total_removed = 0u64; + for (image_name, eligible) in candidates { + if !eligible { + continue; + } + + match self.docker_client.remove_image(&image_name).await { + Ok(()) => { + debug!(image_name = %image_name, "Removed expired deployment image"); + total_removed += 1; + } Err(e) => { - error!( - "Failed to query old deployments for project {}: {}", - project.id, e + // Image may already be gone or referenced by a container. + warn!( + image_name = %image_name, + error = %e, + "Could not remove expired deployment image" ); - continue; - } - }; - - for deployment in old_deployments { - let image_name = match deployment.image_name { - Some(ref name) => name.clone(), - None => continue, - }; - - match self.docker_client.remove_image(&image_name).await { - Ok(()) => { - debug!( - "Removed old deployment image '{}' (deployment {}, project {})", - image_name, deployment.id, project.id - ); - total_removed += 1; - } - Err(e) => { - // Image may already be gone or in use โ€” warn but don't fail - warn!( - "Could not remove deployment image '{}' (deployment {}, project {}): {}", - image_name, deployment.id, project.id, e - ); - } } } } if total_removed > 0 { - info!( - "โœ… Removed {} old deployment images, freed ~{} MB", - total_removed, total_freed_mb - ); + info!("โœ… Removed {} expired deployment images", total_removed); } else { - debug!("No old deployment images to remove"); + debug!("No expired deployment images to remove"); } - - let _ = total_freed_mb; // calculated per-image removal is non-trivial; reported as 0 for now } /// Perform the actual cleanup @@ -678,4 +692,55 @@ mod tests { .with_default_image_retention_hours(72); assert_eq!(service.default_image_retention_hours, 72); } + + #[test] + fn test_newer_reference_preserves_reused_image() { + let now = chrono::Utc::now(); + let cutoff = now - chrono::Duration::hours(48); + let mut candidates = HashMap::new(); + + DockerCleanupService::record_image_retention_eligibility( + &mut candidates, + "temps-demo:42", + now - chrono::Duration::hours(72), + cutoff, + ); + DockerCleanupService::record_image_retention_eligibility( + &mut candidates, + "temps-demo:42", + now - chrono::Duration::hours(1), + cutoff, + ); + + assert_eq!(candidates.get("temps-demo:42"), Some(&false)); + } + + #[test] + fn test_only_temps_managed_local_images_become_candidates() { + let now = chrono::Utc::now(); + let cutoff = now - chrono::Duration::hours(48); + let mut candidates = HashMap::new(); + + DockerCleanupService::record_image_retention_eligibility( + &mut candidates, + "ghcr.io/example/temps-demo:42", + now - chrono::Duration::hours(72), + cutoff, + ); + DockerCleanupService::record_image_retention_eligibility( + &mut candidates, + "nginx:latest", + now - chrono::Duration::hours(72), + cutoff, + ); + DockerCleanupService::record_image_retention_eligibility( + &mut candidates, + "temps-demo:42", + now - chrono::Duration::hours(72), + cutoff, + ); + + assert_eq!(candidates.len(), 1); + assert_eq!(candidates.get("temps-demo:42"), Some(&true)); + } } diff --git a/crates/temps-projects/src/handlers/types.rs b/crates/temps-projects/src/handlers/types.rs index 79474011a..cf549c0ae 100644 --- a/crates/temps-projects/src/handlers/types.rs +++ b/crates/temps-projects/src/handlers/types.rs @@ -551,6 +551,17 @@ pub struct UpdateDeploymentConfigRequest { pub security: Option, } +/// Deserialize a PATCH integer field while preserving the distinction between +/// an omitted key (`None`) and an explicit JSON null (`Some(None)`). +fn deserialize_optional_optional_i32<'de, D>( + deserializer: D, +) -> Result>, D::Error> +where + D: serde::Deserializer<'de>, +{ + Ok(Some(Option::::deserialize(deserializer)?)) +} + #[derive(Serialize, Deserialize, Clone, ToSchema)] pub struct UpdateProjectSettingsRequest { pub slug: Option, @@ -578,7 +589,8 @@ pub struct UpdateProjectSettingsRequest { pub preview_envs_wake_timeout_seconds: Option, /// How long (hours) to retain built Docker images before nightly cleanup removes them. /// Set to null to use the system default (48 hours). Valid range: 1โ€“8760. - pub image_retention_hours: Option, + #[serde(default, deserialize_with = "deserialize_optional_optional_i32")] + pub image_retention_hours: Option>, /// Preset-specific configuration (e.g., Dockerfile path for Docker preset) /// /// Example for Dockerfile preset: @@ -1020,3 +1032,21 @@ pub struct ReinstallWebhookResponse { /// Human-readable status message. pub message: String, } + +#[cfg(test)] +mod tests { + use super::UpdateProjectSettingsRequest; + + #[test] + fn image_retention_patch_distinguishes_omitted_null_and_value() { + let omitted: UpdateProjectSettingsRequest = serde_json::from_str("{}").unwrap(); + let cleared: UpdateProjectSettingsRequest = + serde_json::from_str(r#"{"image_retention_hours":null}"#).unwrap(); + let set: UpdateProjectSettingsRequest = + serde_json::from_str(r#"{"image_retention_hours":72}"#).unwrap(); + + assert_eq!(omitted.image_retention_hours, None); + assert_eq!(cleared.image_retention_hours, Some(None)); + assert_eq!(set.image_retention_hours, Some(Some(72))); + } +} diff --git a/crates/temps-projects/src/services/project.rs b/crates/temps-projects/src/services/project.rs index 1b603a89e..dc45ff13b 100644 --- a/crates/temps-projects/src/services/project.rs +++ b/crates/temps-projects/src/services/project.rs @@ -877,7 +877,7 @@ impl ProjectService { ai_debug_chat_enabled: Option, ai_write_actions_enabled: Option, cross_project_trace_sharing: Option, - image_retention_hours: Option, + image_retention_hours: Option>, ) -> Result { // Validate preview env on-demand timeouts before touching the DB. // Mirrors DeploymentConfig::validate so the project-level defaults are @@ -898,7 +898,7 @@ impl ProjectService { ))); } } - if let Some(hours) = image_retention_hours { + if let Some(Some(hours)) = image_retention_hours { if !(1..=8760).contains(&hours) { return Err(ProjectError::InvalidInput(format!( "image_retention_hours {} is not in valid range (1-8760)", @@ -1088,7 +1088,7 @@ impl ProjectService { active_project.preview_envs_wake_timeout_seconds = Set(wake); } if let Some(hours) = image_retention_hours { - active_project.image_retention_hours = Set(Some(hours)); + active_project.image_retention_hours = Set(hours); } active_project.update(self.db.as_ref()).await?; From 772e01d4679e049d786986327c726157a42b9983 Mon Sep 17 00:00:00 2001 From: Ben Herila Date: Mon, 13 Jul 2026 12:09:34 +0000 Subject: [PATCH 5/7] chore(web): regenerate image retention API types --- web/src/api/client/types.gen.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/web/src/api/client/types.gen.ts b/web/src/api/client/types.gen.ts index dc8b1c334..85189bcc1 100644 --- a/web/src/api/client/types.gen.ts +++ b/web/src/api/client/types.gen.ts @@ -11286,6 +11286,10 @@ export type ProjectResponse = { */ gitlab_webhook_id?: number | null; id: number; + /** + * Hours to retain built Docker images before nightly cleanup. Null = system default (48 h). + */ + image_retention_hours?: number | null; last_deployment?: number | null; main_branch: string; name: string; @@ -16152,6 +16156,10 @@ export type UpdateProjectSettingsRequest = { * Enable automatic preview environment creation for each branch */ enable_preview_environments?: boolean | null; + /** + * Hours to retain built Docker images. Omit = unchanged; null = use system default. + */ + image_retention_hours?: number | null; git_provider_connection_id?: number | null; main_branch?: string | null; preset?: string | null; From dfc9978ec2f1ea1cfc19ed2b3b910caf41d9fb46 Mon Sep 17 00:00:00 2001 From: Ben Herila Date: Wed, 15 Jul 2026 18:31:32 +0000 Subject: [PATCH 6/7] test(git): update project fixtures for image retention --- crates/temps-git/src/handlers/bitbucket.rs | 1 + crates/temps-git/src/handlers/generic.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/crates/temps-git/src/handlers/bitbucket.rs b/crates/temps-git/src/handlers/bitbucket.rs index d99c33a93..80140ba69 100644 --- a/crates/temps-git/src/handlers/bitbucket.rs +++ b/crates/temps-git/src/handlers/bitbucket.rs @@ -421,6 +421,7 @@ mod tests { ai_debug_chat_enabled: None, ai_write_actions_enabled: false, cross_project_trace_sharing: true, + image_retention_hours: None, enable_preview_environments: false, preview_envs_on_demand: false, preview_envs_idle_timeout_seconds: 300, diff --git a/crates/temps-git/src/handlers/generic.rs b/crates/temps-git/src/handlers/generic.rs index e187fc88c..83e83d2fc 100644 --- a/crates/temps-git/src/handlers/generic.rs +++ b/crates/temps-git/src/handlers/generic.rs @@ -376,6 +376,7 @@ mod tests { ai_debug_chat_enabled: None, ai_write_actions_enabled: false, cross_project_trace_sharing: true, + image_retention_hours: None, enable_preview_environments: false, preview_envs_on_demand: false, preview_envs_idle_timeout_seconds: 300, From 28101f8d2b742a6a8798cc55fd3183d23d2d4e9c Mon Sep 17 00:00:00 2001 From: David Viejo Date: Mon, 3 Aug 2026 21:46:34 +0200 Subject: [PATCH 7/7] fix(deployments): protect unrebuildable and active images from retention Addresses review findings on the image retention pass. Data-loss fixes: - Never prune images Temps cannot rebuild. Uploaded tarballs (temps-{slug}-{env}:upload-{ts}) and external registry pulls have no source to rebuild from, so removing one permanently breaks rollback and promotion. Matched on deployment provenance rather than tag text, since the upload endpoint accepts a caller-supplied tag. - Never prune the image an environment is currently serving (environments.current_deployment_id), regardless of age. - Never prune images whose containers live on a worker node; this pass only talks to the local Docker daemon. - Raise the default window from 48h to 336h (14 days). Rollback and promotion hard-fail once an image is gone, so this is a rollback window, not a cache TTL. A 48h default silently destroyed the rollback history of any project that did not deploy over a long weekend. - Abort the whole pass (rather than fail open) when the protection queries error. Scale: - Select only (id, project_id, image_name, created_at) instead of full deployment models joined to full project rows. The previous query materialised deployment_config, context_vars, commit_json and metadata for every deployment ever created. - Batch removals through one Docker connection instead of one per image. Operability: - Add AppSettings.image_retention (enabled + default_hours) so operators can change or disable the policy at runtime via the settings row, per the no-env-var-config rule. Out-of-range values are clamped. - Report removed vs retained counts separately; a run where every removal was refused previously logged "nothing to remove". - Audit image_retention_hours on project settings updates. - Add the setting to the project settings UI and to `temps projects settings` (--image-retention-hours / --reset-image-retention), with a warning below 48h. Also: redate the migration to 20260803 so it applies after the migrations already merged, drop the stale 48h references from docs, add skip_serializing_if to the double-Option PATCH field, and rename needs_preview_update to needs_project_row_update. Tests: 13 unit tests including protection-beats-expiry ordering, and a Docker-backed test asserting a real daemon removes an unreferenced image and refuses one a container still references. --- apps/temps-cli/src/api/types.gen.ts | 9 + apps/temps-cli/src/commands/projects/index.ts | 11 +- .../temps-cli/src/commands/projects/update.ts | 57 +- crates/temps-config/src/handler.rs | 12 +- crates/temps-core/src/app_settings.rs | 49 ++ crates/temps-core/src/lib.rs | 7 +- crates/temps-deployments/src/plugin.rs | 17 +- .../src/services/docker_cleanup_service.rs | 777 ++++++++++++++++-- ...60803_000001_add_image_retention_hours.rs} | 0 crates/temps-migrations/src/migration/mod.rs | 4 +- crates/temps-projects/src/handlers/audit.rs | 4 + .../temps-projects/src/handlers/handlers.rs | 1 + crates/temps-projects/src/handlers/types.rs | 16 +- crates/temps-projects/src/services/project.rs | 10 +- web/src/api/client/types.gen.ts | 25 + .../project/settings/GeneralSettings.tsx | 105 +++ 16 files changed, 1015 insertions(+), 89 deletions(-) rename crates/temps-migrations/src/migration/{m20260629_000001_add_image_retention_hours.rs => m20260803_000001_add_image_retention_hours.rs} (100%) diff --git a/apps/temps-cli/src/api/types.gen.ts b/apps/temps-cli/src/api/types.gen.ts index 7924f31bd..e14469979 100644 --- a/apps/temps-cli/src/api/types.gen.ts +++ b/apps/temps-cli/src/api/types.gen.ts @@ -11922,6 +11922,11 @@ export type ProjectResponse = { */ gitlab_webhook_id?: number | null; id: number; + /** + * Hours to retain built Docker images before nightly cleanup. Null = use the + * system-wide default from settings. + */ + image_retention_hours?: number | null; last_deployment?: number | null; main_branch: string; name: string; @@ -17194,6 +17199,10 @@ export type UpdateProjectSettingsRequest = { */ error_source_root?: string | null; git_provider_connection_id?: number | null; + /** + * Hours to retain built Docker images. Omit = unchanged; null = use system default. + */ + image_retention_hours?: number | null; main_branch?: string | null; preset?: string | null; preset_config?: null | PresetConfigSchema; diff --git a/apps/temps-cli/src/commands/projects/index.ts b/apps/temps-cli/src/commands/projects/index.ts index 7a9fc8117..eb3a5483a 100644 --- a/apps/temps-cli/src/commands/projects/index.ts +++ b/apps/temps-cli/src/commands/projects/index.ts @@ -66,13 +66,22 @@ export function registerProjectsCommands(program: Command): void { projects .command('settings') - .description('Update project settings (slug, attack mode, preview environments)') + .description('Update project settings (slug, attack mode, preview environments, image retention)') .option('-p, --project ', 'Project slug or ID') .option('--slug ', 'Project URL slug') .option('--attack-mode', 'Enable attack mode (CAPTCHA protection)') .option('--no-attack-mode', 'Disable attack mode') .option('--preview-envs', 'Enable preview environments') .option('--no-preview-envs', 'Disable preview environments') + .option( + '--image-retention-hours ', + 'Hours to keep built images before nightly cleanup removes them (1-8760). ' + + 'Images are needed to roll back, so this is the project rollback window' + ) + .option( + '--reset-image-retention', + 'Clear the per-project image retention override and use the system default' + ) .option('--json', 'Output in JSON format') .option('-y, --yes', 'Skip prompts (for automation)') .action(updateSettingsAction) diff --git a/apps/temps-cli/src/commands/projects/update.ts b/apps/temps-cli/src/commands/projects/update.ts index a97fd6ad1..99e869686 100644 --- a/apps/temps-cli/src/commands/projects/update.ts +++ b/apps/temps-cli/src/commands/projects/update.ts @@ -100,10 +100,36 @@ export async function updateSettingsAction( slug?: string attackMode?: boolean previewEnvs?: boolean + imageRetentionHours?: string + resetImageRetention?: boolean json?: boolean yes?: boolean } ): Promise { + // Validate arguments before authenticating or hitting the network, so a + // typo'd flag fails immediately instead of after a project lookup. + // + // Tri-state, matching the PATCH contract: `undefined` leaves the value + // unchanged, `null` clears the override back to the system default, and a + // number sets an explicit per-project window. + let imageRetentionHours: number | null | undefined + if (options.resetImageRetention && options.imageRetentionHours !== undefined) { + throw new Error( + 'Pass either --image-retention-hours or --reset-image-retention, not both' + ) + } + if (options.resetImageRetention) { + imageRetentionHours = null + } else if (options.imageRetentionHours !== undefined) { + const parsed = Number(options.imageRetentionHours) + if (!Number.isInteger(parsed) || parsed < 1 || parsed > 8760) { + throw new Error( + `--image-retention-hours must be a whole number between 1 and 8760 (got "${options.imageRetentionHours}")` + ) + } + imageRetentionHours = parsed + } + await requireAuth() await setupClient() @@ -147,7 +173,13 @@ export async function updateSettingsAction( let previewEnvs = options.previewEnvs // Only prompt if no flags provided AND not in automation mode - if (slug === undefined && attackMode === undefined && previewEnvs === undefined && !options.yes) { + if ( + slug === undefined && + attackMode === undefined && + previewEnvs === undefined && + imageRetentionHours === undefined && + !options.yes + ) { newline() header('Update Project Settings') info(`Current settings for "${project.name}"`) @@ -177,6 +209,12 @@ export async function updateSettingsAction( slug: slug ?? undefined, attack_mode: attackMode ?? undefined, enable_preview_environments: previewEnvs ?? undefined, + // Only include the key when the user actually asked to change it โ€” + // sending `null` unconditionally would silently reset every project + // to the system default. + ...(imageRetentionHours !== undefined + ? { image_retention_hours: imageRetentionHours } + : {}), }, }) if (error) { @@ -194,6 +232,23 @@ export async function updateSettingsAction( keyValue('Slug', slug ?? project.slug) keyValue('Attack Mode', attackMode ? colors.success('Enabled') : colors.muted('Disabled')) keyValue('Preview Environments', previewEnvs ? colors.success('Enabled') : colors.muted('Disabled')) + + const effectiveRetention = + imageRetentionHours !== undefined + ? imageRetentionHours + : (updated?.image_retention_hours ?? null) + keyValue( + 'Image Retention', + effectiveRetention === null + ? colors.muted('System default') + : `${effectiveRetention}h` + ) + if (effectiveRetention !== null && effectiveRetention < 48) { + warning( + `Rollback is only possible while a deployment's image still exists. ` + + `At ${effectiveRetention}h, deployments older than that can no longer be rolled back to.` + ) + } } export async function updateGitAction( diff --git a/crates/temps-config/src/handler.rs b/crates/temps-config/src/handler.rs index c46d12a92..fd27cd221 100644 --- a/crates/temps-config/src/handler.rs +++ b/crates/temps-config/src/handler.rs @@ -16,9 +16,10 @@ use temps_core::error_builder::ErrorBuilder; use temps_core::{ problemdetails::Problem, AiConfigSettings, AppSettings, AuditContext, AuditLogger, AuditOperation, BuildLimitsSettings, ClusterDnsSettings, ContainerLogSettings, - DiskSpaceAlertSettings, LetsEncryptSettings, MetricsStoreKind, MonitoringSettings, - ObservabilityCompressionSettings, ObservabilityRetentionSettings, PublicHostnameStrategy, - RateLimitSettings, RequestMetadata, ScreenshotSettings, SecurityHeadersSettings, + DiskSpaceAlertSettings, ImageRetentionSettings, LetsEncryptSettings, MetricsStoreKind, + MonitoringSettings, ObservabilityCompressionSettings, ObservabilityRetentionSettings, + PublicHostnameStrategy, RateLimitSettings, RequestMetadata, ScreenshotSettings, + SecurityHeadersSettings, }; use tracing::{error, info}; use utoipa::{OpenApi, ToSchema}; @@ -172,6 +173,10 @@ pub struct AppSettingsResponse { /// Build-time resource limits (control-plane only). No sensitive content, /// passed through as-is. pub build_limits: BuildLimitsSettings, + + /// Deployment-image retention policy. No sensitive content, passed through + /// as-is so the settings UI can show and edit the system-wide default. + pub image_retention: ImageRetentionSettings, } /// Monitoring settings with the ClickHouse DSN masked. @@ -383,6 +388,7 @@ impl From for AppSettingsResponse { require_mfa_for_admins: settings.require_mfa_for_admins, cluster_dns: settings.cluster_dns, build_limits: settings.build_limits, + image_retention: settings.image_retention, } } } diff --git a/crates/temps-core/src/app_settings.rs b/crates/temps-core/src/app_settings.rs index b23530ee1..20248256e 100644 --- a/crates/temps-core/src/app_settings.rs +++ b/crates/temps-core/src/app_settings.rs @@ -75,6 +75,12 @@ pub struct AppSettings { /// hardware that already has its own per-host headroom). pub build_limits: BuildLimitsSettings, + /// Retention policy for locally-built deployment images. Modeled as a + /// settings row (not an env var) per CLAUDE.md so an operator can change + /// the system-wide default at runtime without restarting the binary. + /// Individual projects override it via `projects.image_retention_hours`. + pub image_retention: ImageRetentionSettings, + /// Cluster-DNS resolver settings (ADR-024, experimental beta). Off by /// default โ€” see `ClusterDnsSettings` for the incident background and /// trade-offs. Must be explicitly enabled by operators who need @@ -194,6 +200,48 @@ pub struct BuildLimitsSettings { pub memory_limit_mb: u32, } +/// System-wide retention policy for locally-built deployment images. +/// +/// The nightly cleanup removes a Temps-built image only once *every* +/// deployment that references it is older than the owning project's retention +/// window. Deleting an image makes rollback/promotion to that deployment +/// impossible, so the default is deliberately generous: it is a rollback +/// window, not a cache TTL. +#[derive(Debug, Clone, Serialize, ToSchema, Deserialize)] +#[serde(default)] +pub struct ImageRetentionSettings { + /// Whether the nightly pass removes expired deployment images at all. + /// Disabling it keeps every built image forever (the pre-0.1 behaviour). + pub enabled: bool, + + /// Default hours to keep a built deployment image when the owning project + /// has no `image_retention_hours` override. Valid range 1..=8760. + #[schema(minimum = 1, maximum = 8760, example = 336)] + pub default_hours: i64, +} + +impl Default for ImageRetentionSettings { + fn default() -> Self { + Self { + enabled: true, + // 14 days. Long enough that a rollback is still possible after a + // quiet week; short enough to bound disk growth. A 48h default + // would silently destroy the rollback history of any project that + // did not deploy over a long weekend. + default_hours: 336, + } + } +} + +impl ImageRetentionSettings { + /// Clamp `default_hours` into the range the projects API accepts, so a + /// hand-edited settings row can never produce a cutoff that deletes images + /// the moment they are built (or one that never expires by accident). + pub fn effective_default_hours(&self) -> i64 { + self.default_hours.clamp(1, 8760) + } +} + impl Default for BuildLimitsSettings { fn default() -> Self { Self { @@ -825,6 +873,7 @@ impl Default for AppSettings { security_headers: SecurityHeadersSettings::default(), rate_limiting: RateLimitSettings::default(), docker_registry: DockerRegistrySettings::default(), + image_retention: ImageRetentionSettings::default(), disk_space_alert: DiskSpaceAlertSettings::default(), container_logs: ContainerLogSettings::default(), multi_node: MultiNodeSettings::default(), diff --git a/crates/temps-core/src/lib.rs b/crates/temps-core/src/lib.rs index 34d298428..2aeab732c 100644 --- a/crates/temps-core/src/lib.rs +++ b/crates/temps-core/src/lib.rs @@ -83,9 +83,10 @@ pub use anyhow; pub use app_settings::{ AgentSandboxSettings, AiConfigSettings, AppSettings, BuildLimitsSettings, ClusterDnsSettings, ContainerLogSettings, DiskSpaceAlertSettings, DnsProviderSettings, DockerRegistrySettings, - LetsEncryptSettings, MetricsStoreKind, MonitoringSettings, MultiNodeSettings, - ObservabilityCompressionSettings, ObservabilityRetentionSettings, PreviewGatewaySettings, - ProviderConfig, RateLimitSettings, ScreenshotSettings, SecurityHeadersSettings, + ImageRetentionSettings, LetsEncryptSettings, MetricsStoreKind, MonitoringSettings, + MultiNodeSettings, ObservabilityCompressionSettings, ObservabilityRetentionSettings, + PreviewGatewaySettings, ProviderConfig, RateLimitSettings, ScreenshotSettings, + SecurityHeadersSettings, }; pub use async_trait; pub use chrono; diff --git a/crates/temps-deployments/src/plugin.rs b/crates/temps-deployments/src/plugin.rs index af4ec2c14..ee7849d17 100644 --- a/crates/temps-deployments/src/plugin.rs +++ b/crates/temps-deployments/src/plugin.rs @@ -167,13 +167,28 @@ impl TempsPlugin for DeploymentsPlugin { let cas_dir = config_service.data_dir().join("cas"); let cleanup_file_store: Arc = Arc::new(temps_file_store::fs_store::FsFileStore::new(cas_dir)); + // Operator-configured image retention (settings row, not an env + // var). Falls back to the built-in default when settings cannot be + // read so a transient DB hiccup at boot cannot silently disable or + // over-aggressively enable image pruning. + let image_retention = match config_service.get_settings().await { + Ok(settings) => settings.image_retention, + Err(e) => { + tracing::warn!( + error = %e, + "Could not read image retention settings; using defaults" + ); + temps_core::ImageRetentionSettings::default() + } + }; let docker_cleanup = Arc::new( crate::services::DockerCleanupService::new( Arc::new(crate::services::DefaultDockerClient), db.clone(), cleanup_file_store, ) - .with_static_dir(config_service.static_dir()), + .with_static_dir(config_service.static_dir()) + .with_image_retention(&image_retention), ); tokio::spawn({ let cleanup_service = docker_cleanup.clone(); diff --git a/crates/temps-deployments/src/services/docker_cleanup_service.rs b/crates/temps-deployments/src/services/docker_cleanup_service.rs index 9a1b114e9..f11f80c77 100644 --- a/crates/temps-deployments/src/services/docker_cleanup_service.rs +++ b/crates/temps-deployments/src/services/docker_cleanup_service.rs @@ -19,8 +19,21 @@ pub trait DockerClient: Send + Sync { /// Remove unused Docker build cache async fn prune_builder_cache(&self, max_unused_days: i64) -> Result; - /// Remove a specific image by name (e.g. "careowner:211") - async fn remove_image(&self, image_name: &str) -> Result<(), String>; + /// Remove the named images, returning a per-image outcome in the same + /// order. Takes the whole batch (rather than one image per call) so the + /// implementation opens a single Docker connection for the entire nightly + /// pass instead of one per image. + async fn remove_images(&self, image_names: &[String]) -> Vec; +} + +/// Result of attempting to remove one image during the retention pass. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ImageRemovalOutcome { + pub image_name: String, + /// `None` on success, `Some(reason)` when Docker refused (most commonly + /// because a container still references the image, which is the intended + /// safety net rather than a real failure). + pub error: Option, } /// Statistics from Docker prune operations @@ -67,25 +80,49 @@ impl DockerClient for DefaultDockerClient { } } - async fn remove_image(&self, image_name: &str) -> Result<(), String> { + async fn remove_images(&self, image_names: &[String]) -> Vec { use bollard::Docker; - let docker = Docker::connect_with_unix_defaults() - .map_err(|e| format!("Failed to connect to Docker daemon: {}", e))?; + // One connection for the whole batch. + let docker = match Docker::connect_with_unix_defaults() { + Ok(docker) => docker, + Err(e) => { + let reason = format!("Failed to connect to Docker daemon: {}", e); + return image_names + .iter() + .map(|name| ImageRemovalOutcome { + image_name: name.clone(), + error: Some(reason.clone()), + }) + .collect(); + } + }; - docker - .remove_image( - image_name, - Some(bollard::query_parameters::RemoveImageOptions { - force: false, - ..Default::default() - }), - None, - ) - .await - .map_err(|e| format!("Failed to remove image '{}': {}", image_name, e))?; + let mut outcomes = Vec::with_capacity(image_names.len()); + for image_name in image_names { + // Non-forced: Docker refuses while any container (running or + // stopped) still references the image. That is deliberate โ€” it is + // the last line of defence behind the active-deployment guard. + let result = docker + .remove_image( + image_name, + Some(bollard::query_parameters::RemoveImageOptions { + force: false, + ..Default::default() + }), + None, + ) + .await; + + outcomes.push(ImageRemovalOutcome { + image_name: image_name.clone(), + error: result + .err() + .map(|e| format!("Failed to remove image '{}': {}", image_name, e)), + }); + } - Ok(()) + outcomes } async fn prune_builder_cache(&self, max_unused_days: i64) -> Result { @@ -143,8 +180,12 @@ pub struct DockerCleanupService { max_asset_cache_age_days: i64, /// System-wide default: how many hours to keep a built deployment image before /// it is eligible for removal. Projects can override this via their - /// `image_retention_hours` column. (default: 48) + /// `image_retention_hours` column. Sourced from + /// `AppSettings.image_retention.default_hours` at startup. default_image_retention_hours: i64, + /// When false, the retention pass is skipped entirely and built images are + /// kept forever. Sourced from `AppSettings.image_retention.enabled`. + image_retention_enabled: bool, } impl DockerCleanupService { @@ -162,7 +203,11 @@ impl DockerCleanupService { static_dir: None, max_chunk_age_hours: 24, max_asset_cache_age_days: 7, - default_image_retention_hours: 48, + // Mirrors ImageRetentionSettings::default(). Deleting an image + // makes rollback to that deployment impossible, so this is a + // rollback window (14 days), not a cache TTL. + default_image_retention_hours: 336, + image_retention_enabled: true, } } @@ -186,8 +231,10 @@ impl DockerCleanupService { self } - pub fn with_default_image_retention_hours(mut self, hours: i64) -> Self { - self.default_image_retention_hours = hours; + /// Apply the operator-configured retention policy from `AppSettings`. + pub fn with_image_retention(mut self, settings: &temps_core::ImageRetentionSettings) -> Self { + self.image_retention_enabled = settings.enabled; + self.default_image_retention_hours = settings.effective_default_hours(); self } @@ -212,6 +259,19 @@ impl DockerCleanupService { .or_insert(reference_is_expired); } + /// Mark an image as permanently protected, whatever its age. + /// + /// Used for images we cannot rebuild (uploaded tarballs, external + /// registry pulls) and for the image each environment is currently + /// serving. `insert` rather than `and_modify` so protection wins + /// regardless of the order references are visited in. + fn protect_image(candidates: &mut HashMap, image_name: &str) { + if !Self::is_temps_managed_image(image_name) { + return; + } + candidates.insert(image_name.to_string(), false); + } + /// Calculate seconds until the next scheduled cleanup fn seconds_until_next_cleanup(&self) -> u64 { let now = chrono::Utc::now(); @@ -264,87 +324,294 @@ impl DockerCleanupService { /// Remove deployment images that are older than their project's retention period. /// - /// An image is eligible only when every deployment row that references it is older - /// than its owning project's retention period. This preserves images reused by a - /// newer rollback or promotion. Only Temps-managed local tags are considered; - /// registry images are left to Docker's normal cache policy. Docker removal is - /// non-forced, so images referenced by any container are retained. + /// An image is eligible only when **every** deployment row that references it is + /// older than its owning project's retention period, so an image reused by a newer + /// rollback or promotion survives. On top of that expiry rule three categories are + /// protected unconditionally, whatever their age: + /// + /// 1. **Images we cannot rebuild** โ€” uploaded tarballs (`POST .../deployments/upload`) + /// and external registry pulls. There is no source to build them from again, so + /// removing one is data loss, not disk reclamation. + /// 2. **The image each environment is currently serving** + /// (`environments.current_deployment_id`), so a live service can never lose the + /// image it is running even if it has not been redeployed in months. + /// 3. **Images belonging to deployments on other nodes** โ€” this pass only talks to + /// the local Docker daemon, so it only considers deployments whose containers + /// live on the control plane. + /// + /// Only Temps-managed local tags are considered; registry images are left to + /// Docker's normal cache policy. Docker removal is non-forced, so an image still + /// referenced by any container is retained as a final safety net. async fn prune_old_deployment_images(&self) { - use sea_orm::{ColumnTrait, EntityTrait, QueryFilter}; - use temps_entities::{deployments, projects}; + use sea_orm::{ColumnTrait, EntityTrait, QueryFilter, QuerySelect}; + use temps_entities::{deployments, environments, projects}; + + if !self.image_retention_enabled { + debug!("Deployment image retention is disabled; skipping"); + return; + } - let deployment_rows = match deployments::Entity::find() - .filter(deployments::Column::ImageName.is_not_null()) - .find_also_related(projects::Entity) + // Per-project retention overrides. Small table, fetched once so the + // deployment scan below does not need to join (or carry) project rows. + let retention_by_project: HashMap = match projects::Entity::find() + .select_only() + .column(projects::Column::Id) + .column(projects::Column::ImageRetentionHours) + .into_tuple::<(i32, Option)>() .all(self.db.as_ref()) .await { - Ok(rows) => rows, + Ok(rows) => rows + .into_iter() + .map(|(id, hours)| { + ( + id, + hours + .map(i64::from) + .unwrap_or(self.default_image_retention_hours), + ) + }) + .collect(), Err(e) => { - error!( - "Failed to query deployment images for retention cleanup: {}", - e - ); + error!("Failed to query project retention overrides: {}", e); return; } }; - let now = chrono::Utc::now(); - let mut candidates = HashMap::new(); - for (deployment, project) in deployment_rows { - let Some(project) = project else { - warn!( - deployment_id = deployment.id, - "Skipping deployment image with no owning project" - ); - continue; + // Only the four columns the policy actually needs. Selecting the full + // model here would pull `deployment_config`, `context_vars`, + // `commit_json` and `metadata` for every deployment ever created. + let deployment_rows: Vec<(i32, i32, Option, chrono::DateTime)> = + match deployments::Entity::find() + .select_only() + .column(deployments::Column::Id) + .column(deployments::Column::ProjectId) + .column(deployments::Column::ImageName) + .column(deployments::Column::CreatedAt) + .filter(deployments::Column::ImageName.is_not_null()) + .into_tuple() + .all(self.db.as_ref()) + .await + { + Ok(rows) => rows, + Err(e) => { + error!( + "Failed to query deployment images for retention cleanup: {}", + e + ); + return; + } }; - let Some(image_name) = deployment.image_name.as_deref() else { + + if deployment_rows.is_empty() { + debug!("No deployment images recorded; nothing to prune"); + return; + } + + let now = chrono::Utc::now(); + let mut candidates: HashMap = HashMap::new(); + for (_, project_id, image_name, created_at) in &deployment_rows { + let Some(image_name) = image_name.as_deref() else { continue; }; - let retention_hours = project - .image_retention_hours - .map(|h| h as i64) + let retention_hours = retention_by_project + .get(project_id) + .copied() .unwrap_or(self.default_image_retention_hours); let cutoff = now - chrono::Duration::hours(retention_hours); Self::record_image_retention_eligibility( &mut candidates, image_name, - deployment.created_at, + *created_at, cutoff, ); } - let mut total_removed = 0u64; - for (image_name, eligible) in candidates { - if !eligible { - continue; + if candidates.is_empty() { + debug!("No Temps-managed deployment images to consider"); + return; + } + + // Protect images that cannot be rebuilt from source. + match self.unrebuildable_image_names().await { + Ok(names) => { + for name in &names { + Self::protect_image(&mut candidates, name); + } + debug!( + protected = names.len(), + "Protected non-rebuildable deployment images from retention" + ); + } + Err(e) => { + // Failing open here would delete uploaded images. Abort the + // whole pass instead and say so โ€” a skipped night costs disk, + // a wrong deletion costs the user their deployment. + error!( + error = %e, + "Could not determine which deployment images are rebuildable; \ + skipping image retention this run to avoid deleting an \ + unrecoverable image" + ); + return; } + } - match self.docker_client.remove_image(&image_name).await { - Ok(()) => { - debug!(image_name = %image_name, "Removed expired deployment image"); - total_removed += 1; + // Protect whatever each environment is currently serving. + match environments::Entity::find() + .select_only() + .column(environments::Column::CurrentDeploymentId) + .filter(environments::Column::CurrentDeploymentId.is_not_null()) + .into_tuple::>() + .all(self.db.as_ref()) + .await + { + Ok(current_ids) => { + let active: std::collections::HashSet = + current_ids.into_iter().flatten().collect(); + for (deployment_id, _, image_name, _) in &deployment_rows { + if !active.contains(deployment_id) { + continue; + } + if let Some(image_name) = image_name.as_deref() { + Self::protect_image(&mut candidates, image_name); + } } - Err(e) => { - // Image may already be gone or referenced by a container. - warn!( - image_name = %image_name, - error = %e, - "Could not remove expired deployment image" + } + Err(e) => { + error!( + error = %e, + "Could not determine active deployments; skipping image \ + retention this run to avoid removing a live image" + ); + return; + } + } + + // Protect images whose containers live on a worker node โ€” this pass + // only speaks to the local Docker daemon, so a remote image is not + // ours to remove and a failed removal here would be pure log noise. + match self.remote_node_image_names().await { + Ok(names) => { + for name in &names { + Self::protect_image(&mut candidates, name); + } + if !names.is_empty() { + debug!( + remote = names.len(), + "Skipping deployment images owned by worker nodes" ); } } + Err(e) => { + error!( + error = %e, + "Could not determine which deployment images are node-local; \ + skipping image retention this run" + ); + return; + } } - if total_removed > 0 { - info!("โœ… Removed {} expired deployment images", total_removed); - } else { + let expired: Vec = candidates + .into_iter() + .filter_map(|(name, eligible)| eligible.then_some(name)) + .collect(); + + if expired.is_empty() { debug!("No expired deployment images to remove"); + return; + } + + let outcomes = self.docker_client.remove_images(&expired).await; + let removed = outcomes.iter().filter(|o| o.error.is_none()).count(); + let failed = outcomes.len() - removed; + + for outcome in outcomes.iter().filter(|o| o.error.is_some()) { + // Most commonly "image is being used by container" โ€” the intended + // safety net rather than a real failure. Logged individually so an + // operator debugging disk usage can see exactly what was retained. + warn!( + image_name = %outcome.image_name, + error = %outcome.error.as_deref().unwrap_or_default(), + "Could not remove expired deployment image" + ); + } + + // Report retained-vs-removed explicitly. Reporting only successes made + // a run where every removal failed look identical to a run with + // nothing to do. + if failed > 0 { + info!( + "๐Ÿงน Deployment image retention: removed {}, retained {} (still referenced or already gone)", + removed, failed + ); + } else { + info!("โœ… Removed {} expired deployment images", removed); } } + /// Image names that must never be pruned because Temps cannot rebuild them: + /// tarballs uploaded via the image-upload API and external registry images. + /// + /// Matched on the deployment's own provenance rather than on the tag text, + /// because the upload endpoint lets the caller supply an arbitrary `tag`. + async fn unrebuildable_image_names(&self) -> Result, sea_orm::DbErr> { + use sea_orm::{ConnectionTrait, Statement}; + + // No user input is interpolated โ€” this is a fixed predicate over JSON + // columns that Sea-ORM's query builder cannot express directly. + let rows = self + .db + .as_ref() + .query_all(Statement::from_string( + sea_orm::DatabaseBackend::Postgres, + r#" + SELECT DISTINCT d.image_name + FROM deployments d + JOIN projects p ON p.id = d.project_id + WHERE d.image_name IS NOT NULL + AND ( + d.context_vars ->> 'trigger' = 'image_upload' + OR d.metadata ->> 'externalImageRef' IS NOT NULL + OR d.metadata ->> 'externalImageId' IS NOT NULL + OR p.source_type <> 'git' + ) + "#, + )) + .await?; + + rows.into_iter() + .map(|row| row.try_get::("", "image_name")) + .collect() + } + + /// Image names whose deployment containers are recorded against a worker + /// node rather than the control plane. + async fn remote_node_image_names(&self) -> Result, sea_orm::DbErr> { + use sea_orm::{ConnectionTrait, Statement}; + + let rows = self + .db + .as_ref() + .query_all(Statement::from_string( + sea_orm::DatabaseBackend::Postgres, + r#" + SELECT DISTINCT d.image_name + FROM deployments d + JOIN deployment_containers dc ON dc.deployment_id = d.id + WHERE d.image_name IS NOT NULL + AND dc.node_id IS NOT NULL + "#, + )) + .await?; + + rows.into_iter() + .map(|row| row.try_get::("", "image_name")) + .collect() + } + /// Perform the actual cleanup async fn perform_cleanup(&self) { info!("๐Ÿงน Starting nightly Docker cleanup"); @@ -634,8 +901,68 @@ mod tests { self.prune_cache_result.clone() } - async fn remove_image(&self, _image_name: &str) -> Result<(), String> { - Ok(()) + async fn remove_images(&self, image_names: &[String]) -> Vec { + image_names + .iter() + .map(|name| ImageRemovalOutcome { + image_name: name.clone(), + error: None, + }) + .collect() + } + } + + /// Docker client that records every image it was asked to remove, so tests + /// can assert on *which* images the policy selected rather than only that + /// the call did not panic. + #[derive(Default)] + struct RecordingDockerClient { + removed: std::sync::Mutex>, + /// Image names the fake daemon refuses to remove (simulating "image is + /// being used by container"). + refuse: Vec, + } + + impl RecordingDockerClient { + fn removed_sorted(&self) -> Vec { + let mut v = self + .removed + .lock() + .expect("recording mock mutex poisoned") + .clone(); + v.sort(); + v + } + } + + #[async_trait::async_trait] + impl DockerClient for RecordingDockerClient { + async fn prune_images(&self, _force: bool) -> Result { + Ok(PruneStats { + images_deleted: 0, + space_reclaimed_mb: 0, + }) + } + + async fn prune_builder_cache(&self, _max_unused_days: i64) -> Result { + Ok(String::new()) + } + + async fn remove_images(&self, image_names: &[String]) -> Vec { + self.removed + .lock() + .expect("recording mock mutex poisoned") + .extend(image_names.iter().cloned()); + image_names + .iter() + .map(|name| ImageRemovalOutcome { + image_name: name.clone(), + error: self + .refuse + .contains(name) + .then(|| format!("conflict: image {} is being used", name)), + }) + .collect() } } @@ -679,18 +1006,49 @@ mod tests { } #[test] - fn test_default_image_retention_hours() { + fn test_default_image_retention_hours_is_a_rollback_window() { let service = DockerCleanupService::new(Arc::new(DefaultDockerClient), mock_db(), mock_file_store()); - assert_eq!(service.default_image_retention_hours, 48); + // 14 days. A short default (e.g. 48h) silently destroys the rollback + // history of any project that does not deploy over a long weekend. + assert_eq!(service.default_image_retention_hours, 336); + assert!(service.image_retention_enabled); + assert_eq!( + service.default_image_retention_hours, + temps_core::ImageRetentionSettings::default().default_hours, + "service default must track the settings default" + ); } #[test] - fn test_custom_image_retention_hours() { + fn test_operator_settings_override_retention() { + let settings = temps_core::ImageRetentionSettings { + enabled: false, + default_hours: 72, + }; let service = DockerCleanupService::new(Arc::new(DefaultDockerClient), mock_db(), mock_file_store()) - .with_default_image_retention_hours(72); + .with_image_retention(&settings); + assert_eq!(service.default_image_retention_hours, 72); + assert!(!service.image_retention_enabled); + } + + #[test] + fn test_out_of_range_operator_setting_is_clamped() { + // A hand-edited settings row must not be able to produce a cutoff that + // deletes images the moment they are built. + let zero = temps_core::ImageRetentionSettings { + enabled: true, + default_hours: 0, + }; + let huge = temps_core::ImageRetentionSettings { + enabled: true, + default_hours: 999_999, + }; + + assert_eq!(zero.effective_default_hours(), 1); + assert_eq!(huge.effective_default_hours(), 8760); } #[test] @@ -743,4 +1101,277 @@ mod tests { assert_eq!(candidates.len(), 1); assert_eq!(candidates.get("temps-demo:42"), Some(&true)); } + + /// An uploaded image has no source to rebuild from. Pruning it is data + /// loss: rollback and promotion both hard-fail with "image no longer + /// exists locally" and there is no way to get it back. + #[test] + fn test_protection_beats_expiry_regardless_of_order() { + let now = chrono::Utc::now(); + let cutoff = now - chrono::Duration::hours(336); + let uploaded = "temps-demo-prod:upload-1750000000"; + + // Protect first, then record an expired reference. + let mut protect_first = HashMap::new(); + DockerCleanupService::protect_image(&mut protect_first, uploaded); + DockerCleanupService::record_image_retention_eligibility( + &mut protect_first, + uploaded, + now - chrono::Duration::days(400), + cutoff, + ); + assert_eq!(protect_first.get(uploaded), Some(&false)); + + // Record an expired reference first, then protect. + let mut record_first = HashMap::new(); + DockerCleanupService::record_image_retention_eligibility( + &mut record_first, + uploaded, + now - chrono::Duration::days(400), + cutoff, + ); + assert_eq!( + record_first.get(uploaded), + Some(&true), + "precondition: an old upload looks eligible on age alone" + ); + DockerCleanupService::protect_image(&mut record_first, uploaded); + assert_eq!( + record_first.get(uploaded), + Some(&false), + "protection must win over an already-recorded expiry" + ); + } + + #[test] + fn test_protect_ignores_non_temps_images() { + let mut candidates = HashMap::new(); + DockerCleanupService::protect_image(&mut candidates, "ghcr.io/example/app:1"); + DockerCleanupService::protect_image(&mut candidates, "nginx:latest"); + assert!( + candidates.is_empty(), + "external images are never candidates, so they need no protection entry" + ); + } + + #[tokio::test] + async fn test_retention_disabled_removes_nothing() { + let docker = Arc::new(RecordingDockerClient::default()); + let service = DockerCleanupService::new(docker.clone(), mock_db(), mock_file_store()) + .with_image_retention(&temps_core::ImageRetentionSettings { + enabled: false, + default_hours: 1, + }); + + service.prune_old_deployment_images().await; + + assert!( + docker.removed_sorted().is_empty(), + "no Docker call may be made when retention is disabled" + ); + } + + /// A run where every removal is refused must not report success. Before + /// this, `total_removed == 0` logged "nothing to remove", which reads to an + /// operator as "cleanup is healthy" when it is in fact doing nothing. + #[tokio::test] + async fn test_refused_removals_are_counted_separately() { + let docker = Arc::new(RecordingDockerClient { + removed: Default::default(), + refuse: vec!["temps-a:1".to_string()], + }); + + let outcomes = docker + .remove_images(&["temps-a:1".to_string(), "temps-b:1".to_string()]) + .await; + + let removed = outcomes.iter().filter(|o| o.error.is_none()).count(); + let failed = outcomes.len() - removed; + assert_eq!(removed, 1); + assert_eq!(failed, 1); + assert_eq!( + docker.removed_sorted(), + vec!["temps-a:1".to_string(), "temps-b:1".to_string()] + ); + } + + /// End-to-end proof against a real Docker daemon that + /// `DefaultDockerClient::remove_images` actually removes an unreferenced + /// image and actually *retains* one that a container still references. + /// + /// The retention rule is only as good as the non-forced removal underneath + /// it, and that behaviour lives in bollard rather than in our code โ€” so it + /// is worth asserting against the real daemon. Skips gracefully when Docker + /// is unavailable (per project policy, no `#[ignore]`). + /// + /// Both images are *built* rather than tagged from a shared base, because + /// `remove_image` on a tag that shares an image ID with another tag merely + /// untags it without consulting container references. Temps builds one + /// unique `temps-{slug}:{deployment_id}` tag per deployment, so the + /// single-tag case tested here is the shape that actually ships. + #[tokio::test] + async fn test_real_docker_removes_unused_and_retains_in_use_image() { + use bollard::query_parameters::{ + BuildImageOptionsBuilder, CreateContainerOptionsBuilder, CreateImageOptionsBuilder, + RemoveContainerOptions, RemoveImageOptions, + }; + use bollard::Docker; + use futures_util::StreamExt as _; + + let Ok(docker) = Docker::connect_with_unix_defaults() else { + println!("Docker not available, skipping"); + return; + }; + if docker.ping().await.is_err() { + println!("Docker not available, skipping"); + return; + } + + // Base layer for both test images. + let mut pull = docker.create_image( + Some( + CreateImageOptionsBuilder::default() + .from_image("busybox") + .tag("latest") + .build(), + ), + None, + None, + ); + while let Some(step) = pull.next().await { + if step.is_err() { + println!("Could not pull busybox, skipping"); + return; + } + } + + // Each image gets its own ID (distinct ENV) so its tag is the sole + // reference to it -- matching a real per-deployment build. + async fn build(docker: &Docker, tag: &str, marker: &str) -> bool { + let dockerfile = format!("FROM busybox:latest\nENV TEMPS_TEST_MARKER={}\n", marker); + let mut header = tar::Header::new_gnu(); + header.set_size(dockerfile.len() as u64); + header.set_mode(0o644); + header.set_cksum(); + + let mut builder = tar::Builder::new(Vec::new()); + if builder + .append_data(&mut header, "Dockerfile", dockerfile.as_bytes()) + .is_err() + { + return false; + } + let Ok(context) = builder.into_inner() else { + return false; + }; + + let mut build = docker.build_image( + BuildImageOptionsBuilder::default().t(tag).build(), + None, + Some(http_body_util::Either::Left(http_body_util::Full::new( + bytes::Bytes::from(context), + ))), + ); + while let Some(step) = build.next().await { + if step.is_err() { + return false; + } + } + true + } + + let unused = "temps-retention-test-unused:1"; + let in_use = "temps-retention-test-inuse:1"; + if !build(&docker, unused, "unused").await || !build(&docker, in_use, "inuse").await { + println!("Could not build test images, skipping"); + let opts = Some(RemoveImageOptions { + force: true, + ..Default::default() + }); + let _ = docker.remove_image(unused, opts.clone(), None).await; + let _ = docker.remove_image(in_use, opts, None).await; + return; + } + + // Hold `in_use` with a container, exactly like a deployed service does. + let container = docker + .create_container( + Some( + CreateContainerOptionsBuilder::new() + .name("temps-retention-test-holder") + .build(), + ), + bollard::models::ContainerCreateBody { + image: Some(in_use.to_string()), + cmd: Some(vec!["true".to_string()]), + ..Default::default() + }, + ) + .await; + let container_id = match container { + Ok(c) => c.id, + Err(e) => { + println!("Could not create holder container ({e}), skipping"); + let opts = Some(RemoveImageOptions { + force: true, + ..Default::default() + }); + let _ = docker.remove_image(unused, opts.clone(), None).await; + let _ = docker.remove_image(in_use, opts, None).await; + return; + } + }; + + let outcomes = DefaultDockerClient + .remove_images(&[unused.to_string(), in_use.to_string()]) + .await; + + let unused_outcome = outcomes + .iter() + .find(|o| o.image_name == unused) + .expect("outcome reported for the unused image") + .clone(); + let in_use_outcome = outcomes + .iter() + .find(|o| o.image_name == in_use) + .expect("outcome reported for the in-use image") + .clone(); + let unused_still_present = docker.inspect_image(unused).await.is_ok(); + + // Clean up before asserting so a failing assert cannot leak state. + let _ = docker + .remove_container( + &container_id, + Some(RemoveContainerOptions { + force: true, + ..Default::default() + }), + ) + .await; + let _ = docker + .remove_image( + in_use, + Some(RemoveImageOptions { + force: true, + ..Default::default() + }), + None, + ) + .await; + + assert!( + unused_outcome.error.is_none(), + "an unreferenced expired image must actually be removed, got: {:?}", + unused_outcome.error + ); + assert!( + !unused_still_present, + "the unused image must be gone from the daemon after removal" + ); + assert!( + in_use_outcome.error.is_some(), + "an image still referenced by a container must be retained, not untagged \ + underneath the service running it" + ); + } } diff --git a/crates/temps-migrations/src/migration/m20260629_000001_add_image_retention_hours.rs b/crates/temps-migrations/src/migration/m20260803_000001_add_image_retention_hours.rs similarity index 100% rename from crates/temps-migrations/src/migration/m20260629_000001_add_image_retention_hours.rs rename to crates/temps-migrations/src/migration/m20260803_000001_add_image_retention_hours.rs diff --git a/crates/temps-migrations/src/migration/mod.rs b/crates/temps-migrations/src/migration/mod.rs index 0da5a1ad3..b65d7104c 100644 --- a/crates/temps-migrations/src/migration/mod.rs +++ b/crates/temps-migrations/src/migration/mod.rs @@ -131,7 +131,6 @@ mod m20260627_000001_node_enrollment_tokens; mod m20260627_000002_create_ai_conversations; mod m20260628_000001_add_node_to_log_chunks; mod m20260628_000001_otel_spans_root_index; -mod m20260629_000001_add_image_retention_hours; mod m20260629_000001_otel_metrics_full_fidelity; mod m20260629_000002_add_provider_default_model; mod m20260630_000001_add_ai_pending_actions_and_write_toggle; @@ -167,6 +166,7 @@ mod m20260725_000001_sandboxes_agent_run_link; mod m20260728_000001_add_environment_id_to_metric_alert_rules; mod m20260730_000001_add_architecture_to_nodes; mod m20260802_000001_add_environment_force_https; +mod m20260803_000001_add_image_retention_hours; pub struct Migrator; @@ -301,7 +301,6 @@ impl MigratorTrait for Migrator { Box::new(m20260627_000002_create_ai_conversations::Migration), Box::new(m20260628_000001_add_node_to_log_chunks::Migration), Box::new(m20260628_000001_otel_spans_root_index::Migration), - Box::new(m20260629_000001_add_image_retention_hours::Migration), Box::new(m20260629_000001_otel_metrics_full_fidelity::Migration), Box::new(m20260629_000002_add_provider_default_model::Migration), Box::new(m20260630_000001_add_ai_pending_actions_and_write_toggle::Migration), @@ -341,6 +340,7 @@ impl MigratorTrait for Migrator { ), Box::new(m20260730_000001_add_architecture_to_nodes::Migration), Box::new(m20260802_000001_add_environment_force_https::Migration), + Box::new(m20260803_000001_add_image_retention_hours::Migration), ] } } diff --git a/crates/temps-projects/src/handlers/audit.rs b/crates/temps-projects/src/handlers/audit.rs index 1694af96f..6aa6cc6ae 100644 --- a/crates/temps-projects/src/handlers/audit.rs +++ b/crates/temps-projects/src/handlers/audit.rs @@ -104,6 +104,10 @@ pub struct ProjectSettingsUpdatedFields { pub memory_request: Option, pub memory_limit: Option, pub performance_metrics_enabled: Option, + /// New image-retention window, in hours. `Some(None)` records a reset back + /// to the system default. Audited because shortening retention permanently + /// destroys the project's ability to roll back to older deployments. + pub image_retention_hours: Option>, } impl AuditOperation for ProjectCreatedAudit { diff --git a/crates/temps-projects/src/handlers/handlers.rs b/crates/temps-projects/src/handlers/handlers.rs index 43bc3b0dd..3b100bead 100644 --- a/crates/temps-projects/src/handlers/handlers.rs +++ b/crates/temps-projects/src/handlers/handlers.rs @@ -663,6 +663,7 @@ pub async fn update_project_settings( memory_limit: None, performance_metrics_enabled: None, slug: settings.slug, + image_retention_hours: settings.image_retention_hours, }; let audit_event = ProjectSettingsUpdatedAudit { diff --git a/crates/temps-projects/src/handlers/types.rs b/crates/temps-projects/src/handlers/types.rs index 6cbbc37dd..91b2acd04 100644 --- a/crates/temps-projects/src/handlers/types.rs +++ b/crates/temps-projects/src/handlers/types.rs @@ -321,7 +321,8 @@ pub struct ProjectResponse { /// OSS global-observability model where any OtelRead holder can query any /// project's telemetry). pub cross_project_trace_sharing: bool, - /// Hours to retain built Docker images before nightly cleanup. Null = system default (48 h). + /// Hours to retain built Docker images before nightly cleanup. Null = use the + /// system-wide default from settings. #[serde(skip_serializing_if = "Option::is_none")] pub image_retention_hours: Option, } @@ -615,8 +616,17 @@ pub struct UpdateProjectSettingsRequest { /// Wake timeout (seconds, 5..=120) for on-demand preview environments. pub preview_envs_wake_timeout_seconds: Option, /// How long (hours) to retain built Docker images before nightly cleanup removes them. - /// Set to null to use the system default (48 hours). Valid range: 1โ€“8760. - #[serde(default, deserialize_with = "deserialize_optional_optional_i32")] + /// Set to null to use the system default. Valid range: 1โ€“8760. + /// + /// Omitting the key leaves the current value unchanged; sending an explicit + /// `null` clears the per-project override. `skip_serializing_if` keeps the + /// round-trip honest โ€” re-serializing a request that omitted the key must + /// not emit `"image_retention_hours": null`, which would mean "reset". + #[serde( + default, + deserialize_with = "deserialize_optional_optional_i32", + skip_serializing_if = "Option::is_none" + )] pub image_retention_hours: Option>, /// Preset-specific configuration (e.g., Dockerfile path for Docker preset) /// diff --git a/crates/temps-projects/src/services/project.rs b/crates/temps-projects/src/services/project.rs index b0e446e4e..b616fa894 100644 --- a/crates/temps-projects/src/services/project.rs +++ b/crates/temps-projects/src/services/project.rs @@ -1239,13 +1239,13 @@ impl ProjectService { } // Update preview environment settings and image retention if any are provided - let needs_preview_update = enable_preview_environments.is_some() + let needs_project_row_update = enable_preview_environments.is_some() || preview_envs_on_demand.is_some() || preview_envs_idle_timeout_seconds.is_some() || preview_envs_wake_timeout_seconds.is_some() || image_retention_hours.is_some(); - if needs_preview_update { + if needs_project_row_update { // Reload project to ensure we have the latest state let project = projects::Entity::find_by_id(project_id) .one(self.db.as_ref()) @@ -3702,6 +3702,7 @@ mod tests { None, None, None, + None, // image_retention_hours ) .await .expect("partial preset_config patch"); @@ -3765,6 +3766,7 @@ mod tests { None, None, None, + None, // image_retention_hours ) .await .expect("explicit empty providers"); @@ -3864,6 +3866,7 @@ mod tests { None, None, None, + None, // image_retention_hours ) .await; @@ -3962,6 +3965,7 @@ mod tests { None, None, None, + None, // image_retention_hours ) .await; @@ -4083,6 +4087,7 @@ mod tests { None, None, None, + None, // image_retention_hours ) .await .expect("update custom Dockerfile config"); @@ -4319,6 +4324,7 @@ mod tests { None, None, None, + None, // image_retention_hours ) .await .expect("update preset and config together"); diff --git a/web/src/api/client/types.gen.ts b/web/src/api/client/types.gen.ts index 9bded001c..78c296953 100644 --- a/web/src/api/client/types.gen.ts +++ b/web/src/api/client/types.gen.ts @@ -1005,6 +1005,13 @@ export type AppSettings = { */ edge_target?: string | null; external_url?: string | null; + /** + * Retention policy for locally-built deployment images. Modeled as a + * settings row (not an env var) per CLAUDE.md so an operator can change + * the system-wide default at runtime without restarting the binary. + * Individual projects override it via `projects.image_retention_hours`. + */ + image_retention?: ImageRetentionSettings; /** * Skip TLS certificate verification on outbound HTTP clients built by the * server (deployer, agent, remote service client). Strictly opt-in for @@ -1111,6 +1118,11 @@ export type AppSettingsResponse = { */ effective_observability_store: MetricsStoreKind; external_url?: string | null; + /** + * Deployment-image retention policy. No sensitive content, passed through + * as-is so the settings UI can show and edit the system-wide default. + */ + image_retention: ImageRetentionSettings; insecure_tls: boolean; internal_url?: string | null; letsencrypt: LetsEncryptSettings; @@ -8139,6 +8151,19 @@ export type HttpChallengeDebugResponse = { validation_url?: string | null; }; +export type ImageRetentionSettings = { + /** + * Default hours to keep a built deployment image when the owning project + * has no `image_retention_hours` override. Valid range 1..=8760. + */ + default_hours?: number; + /** + * Whether the nightly pass removes expired deployment images at all. + * Disabling it keeps every built image forever (the pre-0.1 behaviour). + */ + enabled?: boolean; +}; + /** * Platform-specific credentials for accessing the source system. * diff --git a/web/src/components/project/settings/GeneralSettings.tsx b/web/src/components/project/settings/GeneralSettings.tsx index 955c18975..967ef1299 100644 --- a/web/src/components/project/settings/GeneralSettings.tsx +++ b/web/src/components/project/settings/GeneralSettings.tsx @@ -98,6 +98,21 @@ const previewEnvironmentsSchema = z type PreviewEnvironmentsFormValues = z.infer +const imageRetentionSchema = z.object({ + // Empty string means "use the system default" and is sent as an explicit + // null, which clears the per-project override. + imageRetentionHours: z.string().refine( + (value) => { + if (value.trim() === '') return true + const parsed = Number(value) + return Number.isInteger(parsed) && parsed >= 1 && parsed <= 8760 + }, + { message: 'Must be a whole number of hours between 1 and 8760, or blank' } + ), +}) + +type ImageRetentionFormValues = z.infer + export function GeneralSettings({ project, refetch }: GeneralSettingsProps) { const navigate = useNavigate() @@ -159,8 +174,21 @@ export function GeneralSettings({ project, refetch }: GeneralSettingsProps) { }, }) + const imageRetentionForm = useForm({ + resolver: zodResolver(imageRetentionSchema), + defaultValues: { + imageRetentionHours: + project?.image_retention_hours != null + ? String(project.image_retention_hours) + : '', + }, + }) + const previewEnabled = previewForm.watch('enablePreviewEnvironments') const onDemandEnabled = previewForm.watch('previewEnvsOnDemand') + const retentionInput = imageRetentionForm.watch('imageRetentionHours') + const retentionHours = + retentionInput.trim() === '' ? null : Number(retentionInput) const handleSaveProject = async (values: ProjectFormValues) => { if (!project?.id) return @@ -229,6 +257,27 @@ export function GeneralSettings({ project, refetch }: GeneralSettingsProps) { refetch() } + const handleSaveImageRetention = async (values: ImageRetentionFormValues) => { + if (!project?.id) return + + const trimmed = values.imageRetentionHours.trim() + await toast.promise( + updateProjectSettings.mutateAsync({ + path: { project_id: project.id! }, + body: { + // Explicit null clears the override back to the system default. + image_retention_hours: trimmed === '' ? null : parseInt(trimmed, 10), + }, + }), + { + loading: 'Updating image retention...', + success: 'Image retention updated successfully', + error: 'Failed to update image retention', + } + ) + refetch() + } + const handleSavePreviewEnvironments = async ( values: PreviewEnvironmentsFormValues ) => { @@ -734,6 +783,62 @@ export function GeneralSettings({ project, refetch }: GeneralSettingsProps) { + {/* Image Retention Card */} +
+ + + + Built Image Retention + + How long this project's built Docker images are kept before the + nightly cleanup removes them. Rolling back or promoting a + deployment requires its image, so this is effectively the + project's rollback window. + + + + ( + + Retention (hours) + + + + + Leave blank to use the system-wide default configured in + Settings. Min 1 hour, max 8760 (one year). + + + )} + /> + {retentionHours !== null && retentionHours < 48 && ( +

+ At {retentionHours}h, deployments older than that can no + longer be rolled back to โ€” their images will already have been + deleted. +

+ )} +
+ + + +
+
+ + {/* Cross-Project Trace Sharing Card */}