From 7ad72328e19ca7ff0c4c095bad065816d4f87e84 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 12 Nov 2025 16:57:27 -0800 Subject: [PATCH] add config for enabling tuf_repo_pruner task --- .../bin/omdb/nexus/reconfigurator_config.rs | 6 + dev-tools/omdb/src/bin/omdb/reconfigurator.rs | 3 + dev-tools/omdb/tests/successes.out | 10 +- nexus-config/src/nexus_config.rs | 2 + nexus/db-model/src/reconfigurator_config.rs | 3 + nexus/db-model/src/schema_versions.rs | 3 +- .../src/db/datastore/reconfigurator_config.rs | 37 +++- nexus/db-schema/src/schema.rs | 1 + nexus/src/app/background/init.rs | 5 +- .../app/background/tasks/blueprint_planner.rs | 7 +- .../background/tasks/reconfigurator_config.rs | 168 ++++++++++++++++++ .../app/background/tasks/tuf_repo_pruner.rs | 53 +++++- nexus/test-utils/src/lib.rs | 8 + .../tuf_repo_pruner_status_disabled.out | 2 + ...out => tuf_repo_pruner_status_enabled.out} | 1 + .../src/deployment/reconfigurator_config.rs | 27 ++- nexus/types/src/internal_api/background.rs | 57 +++++- openapi/nexus-lockstep.json | 6 +- schema/crdb/dbinit.sql | 7 +- schema/crdb/disable-tuf-repo-pruner/up1.sql | 2 + schema/crdb/disable-tuf-repo-pruner/up2.sql | 2 + 21 files changed, 371 insertions(+), 39 deletions(-) create mode 100644 nexus/types/output/tuf_repo_pruner_status_disabled.out rename nexus/types/output/{tuf_repo_pruner_status.out => tuf_repo_pruner_status_enabled.out} (96%) create mode 100644 schema/crdb/disable-tuf-repo-pruner/up1.sql create mode 100644 schema/crdb/disable-tuf-repo-pruner/up2.sql diff --git a/dev-tools/omdb/src/bin/omdb/nexus/reconfigurator_config.rs b/dev-tools/omdb/src/bin/omdb/nexus/reconfigurator_config.rs index c033e12bbdf..b293250baf8 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus/reconfigurator_config.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus/reconfigurator_config.rs @@ -52,6 +52,9 @@ pub struct ReconfiguratorConfigOpts { #[clap(long, action = ArgAction::Set)] add_zones_with_mupdate_override: Option, + + #[clap(long, action = ArgAction::Set)] + tuf_repo_pruner_enabled: Option, } impl ReconfiguratorConfigOpts { @@ -69,6 +72,9 @@ impl ReconfiguratorConfigOpts { current.planner_config.add_zones_with_mupdate_override, ), }, + tuf_repo_pruner_enabled: self + .tuf_repo_pruner_enabled + .unwrap_or(current.tuf_repo_pruner_enabled), } } diff --git a/dev-tools/omdb/src/bin/omdb/reconfigurator.rs b/dev-tools/omdb/src/bin/omdb/reconfigurator.rs index 9a361d794e6..78c3400960a 100644 --- a/dev-tools/omdb/src/bin/omdb/reconfigurator.rs +++ b/dev-tools/omdb/src/bin/omdb/reconfigurator.rs @@ -428,6 +428,7 @@ async fn cmd_reconfigurator_config_history( version: String, planner_enabled: String, add_zones_with_mupdate_override: String, + tuf_repo_pruner_enabled: String, time_modified: String, } @@ -441,6 +442,7 @@ async fn cmd_reconfigurator_config_history( planner_enabled, planner_config: PlannerConfig { add_zones_with_mupdate_override }, + tuf_repo_pruner_enabled, }, time_modified, } = s; @@ -449,6 +451,7 @@ async fn cmd_reconfigurator_config_history( planner_enabled: planner_enabled.to_string(), add_zones_with_mupdate_override: add_zones_with_mupdate_override.to_string(), + tuf_repo_pruner_enabled: tuf_repo_pruner_enabled.to_string(), time_modified: time_modified.to_string(), } }) diff --git a/dev-tools/omdb/tests/successes.out b/dev-tools/omdb/tests/successes.out index 116286f4b6c..05981f6085d 100644 --- a/dev-tools/omdb/tests/successes.out +++ b/dev-tools/omdb/tests/successes.out @@ -876,6 +876,7 @@ task: "tuf_repo_pruner" configured period: every m last completed activation: , triggered by started at (s ago) and ran for ms + status: enabled configuration: nkeep_recent_releases: 3 nkeep_recent_uploads: 3 @@ -1437,6 +1438,7 @@ task: "tuf_repo_pruner" configured period: every m last completed activation: , triggered by started at (s ago) and ran for ms + status: enabled configuration: nkeep_recent_releases: 3 nkeep_recent_uploads: 3 @@ -1816,6 +1818,7 @@ stdout: Reconfigurator config: version: 1 modified time: + tuf repo pruner enabled: true planner enabled: false planner config: add zones with mupdate override: false @@ -1896,7 +1899,8 @@ termination: Exited(0) --------------------------------------------- stdout: reconfigurator config updated to version 2: -* planner enabled: false -> true + tuf repo pruner enabled: true (unchanged) +* planner enabled::::::::: false -> true planner config: add zones with mupdate override: false (unchanged) --------------------------------------------- @@ -1908,7 +1912,8 @@ termination: Exited(0) --------------------------------------------- stdout: reconfigurator config updated to version 3: - planner enabled: true (unchanged) + tuf repo pruner enabled: true (unchanged) + planner enabled::::::::: true (unchanged) planner config: * add zones with mupdate override: false -> true --------------------------------------------- @@ -1922,6 +1927,7 @@ stdout: Reconfigurator config: version: 3 modified time: + tuf repo pruner enabled: true planner enabled: true planner config: add zones with mupdate override: true diff --git a/nexus-config/src/nexus_config.rs b/nexus-config/src/nexus_config.rs index 2b1a32a17c5..fe3c391fb81 100644 --- a/nexus-config/src/nexus_config.rs +++ b/nexus-config/src/nexus_config.rs @@ -1154,6 +1154,7 @@ mod test { [initial_reconfigurator_config] planner_enabled = true planner_config.add_zones_with_mupdate_override = true + tuf_repo_pruner_enabled = false [background_tasks] dns_internal.period_secs_config = 1 dns_internal.period_secs_servers = 2 @@ -1314,6 +1315,7 @@ mod test { planner_config: PlannerConfig { add_zones_with_mupdate_override: true, }, + tuf_repo_pruner_enabled: false, }), background_tasks: BackgroundTaskConfig { dns_internal: DnsTasksConfig { diff --git a/nexus/db-model/src/reconfigurator_config.rs b/nexus/db-model/src/reconfigurator_config.rs index 36aaf0d966d..801bdb41369 100644 --- a/nexus/db-model/src/reconfigurator_config.rs +++ b/nexus/db-model/src/reconfigurator_config.rs @@ -16,6 +16,7 @@ pub struct ReconfiguratorConfig { pub planner_enabled: bool, pub time_modified: DateTime, pub add_zones_with_mupdate_override: bool, + pub tuf_repo_pruner_enabled: bool, } impl From for ReconfiguratorConfig { @@ -28,6 +29,7 @@ impl From for ReconfiguratorConfig { .config .planner_config .add_zones_with_mupdate_override, + tuf_repo_pruner_enabled: value.config.tuf_repo_pruner_enabled, } } } @@ -42,6 +44,7 @@ impl From for deployment::ReconfiguratorConfigView { add_zones_with_mupdate_override: value .add_zones_with_mupdate_override, }, + tuf_repo_pruner_enabled: value.tuf_repo_pruner_enabled, }, time_modified: value.time_modified, } diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 9c8ef7f0fdd..d8305b885f1 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock}; /// /// This must be updated when you change the database schema. Refer to /// schema/crdb/README.adoc in the root of this repository for details. -pub const SCHEMA_VERSION: Version = Version::new(207, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(208, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock> = LazyLock::new(|| { // | leaving the first copy as an example for the next person. // v // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"), + KnownVersion::new(208, "disable-tuf-repo-pruner"), KnownVersion::new(207, "disk-types"), KnownVersion::new(206, "fm-sitreps-by-parent-id-index"), KnownVersion::new(205, "fm-sitrep"), diff --git a/nexus/db-queries/src/db/datastore/reconfigurator_config.rs b/nexus/db-queries/src/db/datastore/reconfigurator_config.rs index 5e63ba8ba3e..df34d2ce883 100644 --- a/nexus/db-queries/src/db/datastore/reconfigurator_config.rs +++ b/nexus/db-queries/src/db/datastore/reconfigurator_config.rs @@ -20,6 +20,8 @@ use nexus_db_errors::public_error_from_diesel; use nexus_db_lookup::DbConnection; use nexus_db_model::ReconfiguratorConfig as DbReconfiguratorConfig; use nexus_db_model::SqlU32; +use nexus_types::deployment::PlannerConfig; +use nexus_types::deployment::ReconfiguratorConfig; use nexus_types::deployment::ReconfiguratorConfigParam; use nexus_types::deployment::ReconfiguratorConfigView; use omicron_common::api::external::DataPageParams; @@ -142,22 +144,38 @@ impl DataStore { )); } + // This statement exists just to trigger compilation errors when the + // `ReconfiguratorConfigView` type changes so that people are prompted + // to fix this query. If you get a compiler error here, you probably + // added or removed a field to this type, and you will need to adjust + // the query below accordingly. + let ReconfiguratorConfigView { + version, + config: + ReconfiguratorConfig { + planner_enabled, + planner_config: + PlannerConfig { add_zones_with_mupdate_override }, + tuf_repo_pruner_enabled, + }, + time_modified, + } = *switches; + sql_query( r"INSERT INTO reconfigurator_config (version, planner_enabled, time_modified, - add_zones_with_mupdate_override) - SELECT $1, $2, $3, $4 + add_zones_with_mupdate_override, tuf_repo_pruner_enabled) + SELECT $1, $2, $3, $4, $5 WHERE $1 - 1 IN ( SELECT COALESCE(MAX(version), 0) FROM reconfigurator_config )", ) - .bind::(switches.version.into()) - .bind::(switches.config.planner_enabled) - .bind::(switches.time_modified) - .bind::( - switches.config.planner_config.add_zones_with_mupdate_override, - ) + .bind::(version.into()) + .bind::(planner_enabled) + .bind::(time_modified) + .bind::(add_zones_with_mupdate_override) + .bind::(tuf_repo_pruner_enabled) .execute_async(conn) .await .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) @@ -186,12 +204,13 @@ mod tests { .is_empty() ); - // Fail to insert a swtiches with version 0 + // Fail to insert version 0 let mut switches = ReconfiguratorConfigParam { version: 0, config: ReconfiguratorConfig { planner_enabled: false, planner_config: PlannerConfig::default(), + tuf_repo_pruner_enabled: true, }, }; diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index da4ebf243ed..7804d119af3 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -1967,6 +1967,7 @@ table! { planner_enabled -> Bool, time_modified -> Timestamptz, add_zones_with_mupdate_override -> Bool, + tuf_repo_pruner_enabled -> Bool, } } diff --git a/nexus/src/app/background/init.rs b/nexus/src/app/background/init.rs index c526351126f..3ed5ff471b2 100644 --- a/nexus/src/app/background/init.rs +++ b/nexus/src/app/background/init.rs @@ -568,7 +568,7 @@ impl BackgroundTasksInitializer { watchers: vec![ Box::new(inventory_load_watcher.clone()), Box::new(rx_blueprint.clone()), - Box::new(reconfigurator_config_watcher), + Box::new(reconfigurator_config_watcher.clone()), ], activator: task_blueprint_planner, }); @@ -981,9 +981,10 @@ impl BackgroundTasksInitializer { task_impl: Box::new(tuf_repo_pruner::TufRepoPruner::new( datastore.clone(), config.tuf_repo_pruner.clone(), + reconfigurator_config_watcher.clone(), )), opctx: opctx.child(BTreeMap::new()), - watchers: vec![], + watchers: vec![Box::new(reconfigurator_config_watcher)], activator: task_tuf_repo_pruner, }); diff --git a/nexus/src/app/background/tasks/blueprint_planner.rs b/nexus/src/app/background/tasks/blueprint_planner.rs index b99b8d685c0..4d9d87c0212 100644 --- a/nexus/src/app/background/tasks/blueprint_planner.rs +++ b/nexus/src/app/background/tasks/blueprint_planner.rs @@ -429,8 +429,7 @@ mod test { use nexus_test_utils::db::TestDatabase; use nexus_test_utils_macros::nexus_test; use nexus_types::deployment::{ - PendingMgsUpdates, PlannerConfig, ReconfiguratorConfig, - ReconfiguratorConfigView, + PendingMgsUpdates, ReconfiguratorConfig, ReconfiguratorConfigView, }; use omicron_test_utils::dev; use omicron_uuid_kinds::OmicronZoneUuid; @@ -488,7 +487,7 @@ mod test { version: 1, config: ReconfiguratorConfig { planner_enabled: true, - planner_config: PlannerConfig::default(), + ..ReconfiguratorConfig::default() }, time_modified: now_db_precision(), }), @@ -660,7 +659,7 @@ mod test { version: 1, config: ReconfiguratorConfig { planner_enabled: true, - planner_config: PlannerConfig::default(), + ..ReconfiguratorConfig::default() }, time_modified: now_db_precision(), }), diff --git a/nexus/src/app/background/tasks/reconfigurator_config.rs b/nexus/src/app/background/tasks/reconfigurator_config.rs index 01a3aa8b4ff..d6a4ee8c40f 100644 --- a/nexus/src/app/background/tasks/reconfigurator_config.rs +++ b/nexus/src/app/background/tasks/reconfigurator_config.rs @@ -85,11 +85,18 @@ impl BackgroundTask for ReconfiguratorConfigLoader { #[cfg(test)] mod test { use super::*; + use crate::app::background::BackgroundTask; use async_bb8_diesel::AsyncRunQueryDsl; + use nexus_lockstep_client::types::LastResult; use nexus_test_utils_macros::nexus_test; use nexus_types::deployment::{ PlannerConfig, ReconfiguratorConfig, ReconfiguratorConfigParam, }; + use nexus_types::internal_api::background::BlueprintPlannerStatus; + use nexus_types::internal_api::background::TufRepoPrunerStatus; + use omicron_test_utils::dev::poll::{CondCheckError, wait_for_condition}; + use serde::de::DeserializeOwned; + use std::time::Duration; type ControlPlaneTestContext = nexus_test_utils::ControlPlaneTestContext; @@ -142,6 +149,7 @@ mod test { let expected_switches = ReconfiguratorConfig { planner_enabled: !default_switches.config.planner_enabled, planner_config: PlannerConfig::default(), + tuf_repo_pruner_enabled: true, }; let switches = ReconfiguratorConfigParam { version: 1, config: expected_switches }; @@ -172,6 +180,7 @@ mod test { let expected_switches = ReconfiguratorConfig { planner_enabled: !expected_switches.planner_enabled, planner_config: PlannerConfig::default(), + tuf_repo_pruner_enabled: true, }; let switches = ReconfiguratorConfigParam { version: 2, config: expected_switches }; @@ -193,4 +202,163 @@ mod test { assert_eq!(view.config, expected_switches); } } + + /// Tests the actual behavior of enabling and disabling background tasks + #[nexus_test(server = crate::Server)] + async fn test_background_tasks_enable_disable( + cptestctx: &ControlPlaneTestContext, + ) { + // This test uses both the Progenitor lockstep client (because that's + // preferred in modern code) as well as the one hanging directly off the + // `cptestctx` (which uses ClientTestContext, which predates Progenitor + // altogether, when calling existing code that uses that client). + let lockstep_client = cptestctx.lockstep_client(); + + // As of this writing, the initial state in the test suite is that the + // planner is disabled but the TUF repo pruner is enabled. But we don't + // want to depend on that. So the basic plan here will be: + // + // - disable both tasks: verify they're disabled + // - enable both tasks: verify they're enabled + // - disable both tasks again: verify they're disabled + // + // This last step seems redundant but it exercises the transition from + // enabled -> disabled, which may not have happened if one of the tasks + // was disabled when we started. + + // Disable both tasks. + let initial_config_version = lockstep_client + .reconfigurator_config_show_current() + .await + .expect("failed to get current config") + .version; + let config_disabled = ReconfiguratorConfig { + planner_enabled: false, + planner_config: PlannerConfig::default(), + tuf_repo_pruner_enabled: false, + }; + let switches = ReconfiguratorConfigParam { + version: initial_config_version + 1, + config: config_disabled, + }; + lockstep_client + .reconfigurator_config_set(&switches) + .await + .expect("failed to set enabled config"); + eprintln!("disabled both tasks"); + + // Wait for both tasks to report being disabled. + wait_for_task_status( + &lockstep_client, + "blueprint_planner", + &|planner_status: &BlueprintPlannerStatus| { + matches!(planner_status, BlueprintPlannerStatus::Disabled) + }, + ) + .await; + wait_for_task_status( + &lockstep_client, + "tuf_repo_pruner", + &|pruner_status: &TufRepoPrunerStatus| { + matches!(pruner_status, TufRepoPrunerStatus::Disabled { .. }) + }, + ) + .await; + + // Enable both tasks. + let config_enabled = ReconfiguratorConfig { + planner_enabled: true, + planner_config: PlannerConfig::default(), + tuf_repo_pruner_enabled: true, + }; + let switches = ReconfiguratorConfigParam { + version: initial_config_version + 2, + config: config_enabled, + }; + lockstep_client + .reconfigurator_config_set(&switches) + .await + .expect("failed to set enabled config"); + eprintln!("enabled both tasks"); + + // Wait for both tasks to report being enabled. + wait_for_task_status( + &lockstep_client, + "blueprint_planner", + &|planner_status: &BlueprintPlannerStatus| { + !matches!(planner_status, BlueprintPlannerStatus::Disabled) + }, + ) + .await; + wait_for_task_status( + &lockstep_client, + "tuf_repo_pruner", + &|pruner_status: &TufRepoPrunerStatus| { + !matches!(pruner_status, TufRepoPrunerStatus::Disabled { .. }) + }, + ) + .await; + + // Disable both tasks again. + let switches = ReconfiguratorConfigParam { + version: initial_config_version + 3, + config: config_disabled, + }; + lockstep_client + .reconfigurator_config_set(&switches) + .await + .expect("failed to set enabled config"); + eprintln!("disabled both tasks again"); + + // Wait for both tasks to report being disabled. + wait_for_task_status( + &lockstep_client, + "blueprint_planner", + &|planner_status: &BlueprintPlannerStatus| { + matches!(planner_status, BlueprintPlannerStatus::Disabled) + }, + ) + .await; + wait_for_task_status( + &lockstep_client, + "tuf_repo_pruner", + &|pruner_status: &TufRepoPrunerStatus| { + matches!(pruner_status, TufRepoPrunerStatus::Disabled { .. }) + }, + ) + .await; + } + + async fn wait_for_task_status( + lockstep_client: &nexus_lockstep_client::Client, + task_name: &'static str, + check: &dyn Fn(&T) -> bool, + ) { + eprintln!("waiting for {task_name} status"); + wait_for_condition( + || async { + let task_status = lockstep_client + .bgtask_view(task_name) + .await + .expect("fetching task status") + .into_inner(); + eprintln!("task {} status: {:#?}", task_name, task_status); + let LastResult::Completed(completed) = task_status.last else { + return Err(CondCheckError::<()>::NotYet); + }; + + let status: T = serde_json::from_value(completed.details) + .expect("failed to parse task status as JSON"); + if check(&status) { + Ok(()) + } else { + Err(CondCheckError::NotYet) + } + }, + &Duration::from_millis(100), + &Duration::from_secs(30), + ) + .await + .expect("timed out waiting for task status") + } } diff --git a/nexus/src/app/background/tasks/tuf_repo_pruner.rs b/nexus/src/app/background/tasks/tuf_repo_pruner.rs index 2f9003baa0f..8b1cbc485f2 100644 --- a/nexus/src/app/background/tasks/tuf_repo_pruner.rs +++ b/nexus/src/app/background/tasks/tuf_repo_pruner.rs @@ -4,6 +4,7 @@ //! Background task for determining when to prune artifacts from TUF repos +use super::reconfigurator_config::ReconfiguratorConfigLoaderState; use crate::app::background::BackgroundTask; use anyhow::Context; use futures::future::BoxFuture; @@ -12,6 +13,7 @@ use nexus_config::TufRepoPrunerConfig; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::DataStore; use nexus_types::internal_api::background::TufRepoInfo; +use nexus_types::internal_api::background::TufRepoPrunerDetails; use nexus_types::internal_api::background::TufRepoPrunerStatus; use omicron_uuid_kinds::TufRepoUuid; use serde_json::json; @@ -19,6 +21,7 @@ use slog_error_chain::InlineErrorChain; use std::cmp::Reverse; use std::collections::BTreeSet; use std::sync::Arc; +use tokio::sync::watch::Receiver; /// number of recent distinct target releases that we always keep, regardless of /// configuration @@ -39,11 +42,16 @@ const NKEEP_RECENT_UPLOADS_ALWAYS: u8 = 1; pub struct TufRepoPruner { datastore: Arc, config: TufRepoPrunerConfig, + rx_config: Receiver, } impl TufRepoPruner { - pub fn new(datastore: Arc, config: TufRepoPrunerConfig) -> Self { - Self { datastore, config } + pub fn new( + datastore: Arc, + config: TufRepoPrunerConfig, + rx_config: Receiver, + ) -> Self { + Self { datastore, config, rx_config } } } @@ -53,13 +61,20 @@ impl BackgroundTask for TufRepoPruner { opctx: &'a OpContext, ) -> BoxFuture<'a, serde_json::Value> { Box::pin(async move { - match tuf_repos_prune(opctx, &self.datastore, &self.config).await { + match tuf_repos_prune( + opctx, + &self.datastore, + &self.config, + &self.rx_config, + ) + .await + { Ok(status) => match serde_json::to_value(status) { Ok(val) => val, Err(err) => json!({ "error": format!( "could not serialize task status: {}", - InlineErrorChain::new(&err) + InlineErrorChain::new(&err) ), }), }, @@ -75,7 +90,31 @@ async fn tuf_repos_prune( opctx: &OpContext, datastore: &DataStore, config: &TufRepoPrunerConfig, + rx_config: &Receiver, ) -> Result { + // Refuse to run if we haven't had a chance to load our config from + // the database yet. (There might not be a config, which is fine! + // But the loading task needs to have a chance to check.) + match &*rx_config.borrow() { + ReconfiguratorConfigLoaderState::NotYetLoaded => { + info!( + opctx.log, + "reconfigurator config not yet loaded; doing nothing" + ); + return Ok(TufRepoPrunerStatus::Disabled { + reason: "reconfigurator config not yet loaded".to_string(), + }); + } + ReconfiguratorConfigLoaderState::Loaded(config) => { + if !config.config.tuf_repo_pruner_enabled { + info!(&opctx.log, "TUF repo pruner disabled; doing nothing"); + return Ok(TufRepoPrunerStatus::Disabled { + reason: String::from("explicitly disabled"), + }); + } + } + }; + // Compute configuration. let nkeep_recent_releases = NKEEP_RECENT_TARGET_RELEASES_ALWAYS .saturating_add(config.nkeep_extra_target_releases); @@ -148,7 +187,7 @@ async fn tuf_repos_prune( } } - Ok(status) + Ok(TufRepoPrunerStatus::Enabled(status)) } /// Arguments to `decide_prune()` @@ -165,7 +204,7 @@ struct TufRepoPrune<'a> { /// Given the complete list of TUF repos and a set of recent releases that we /// definitely want to keep, decide what to prune and what to keep. -fn decide_prune(args: TufRepoPrune) -> TufRepoPrunerStatus { +fn decide_prune(args: TufRepoPrune) -> TufRepoPrunerDetails { let TufRepoPrune { nkeep_recent_releases, nkeep_recent_uploads, @@ -221,7 +260,7 @@ fn decide_prune(args: TufRepoPrune) -> TufRepoPrunerStatus { } } - TufRepoPrunerStatus { + TufRepoPrunerDetails { nkeep_recent_releases, nkeep_recent_uploads, repos_keep_target_release, diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index 6fb1b59cb1d..b7a9e776565 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -278,6 +278,13 @@ impl ControlPlaneTestContext { ) } + pub fn lockstep_client(&self) -> nexus_lockstep_client::Client { + nexus_lockstep_client::Client::new( + &format!("http://{}", self.lockstep_client.bind_address), + self.lockstep_client.client_log.clone(), + ) + } + pub async fn teardown(mut self) { self.server.close().await; self.database.cleanup().await.unwrap(); @@ -848,6 +855,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { Some(ReconfiguratorConfig { planner_enabled: false, planner_config: PlannerConfig::default(), + tuf_repo_pruner_enabled: true, }); self.config.deployment.internal_dns = InternalDns::FromAddress { address: self diff --git a/nexus/types/output/tuf_repo_pruner_status_disabled.out b/nexus/types/output/tuf_repo_pruner_status_disabled.out new file mode 100644 index 00000000000..3bbc238153a --- /dev/null +++ b/nexus/types/output/tuf_repo_pruner_status_disabled.out @@ -0,0 +1,2 @@ + status: disabled + reason: disabled in this test diff --git a/nexus/types/output/tuf_repo_pruner_status.out b/nexus/types/output/tuf_repo_pruner_status_enabled.out similarity index 96% rename from nexus/types/output/tuf_repo_pruner_status.out rename to nexus/types/output/tuf_repo_pruner_status_enabled.out index 5e43e5db4a2..9f2a483b90c 100644 --- a/nexus/types/output/tuf_repo_pruner_status.out +++ b/nexus/types/output/tuf_repo_pruner_status_enabled.out @@ -1,3 +1,4 @@ + status: enabled configuration: nkeep_recent_releases: 1 nkeep_recent_uploads: 1 diff --git a/nexus/types/src/deployment/reconfigurator_config.rs b/nexus/types/src/deployment/reconfigurator_config.rs index 1e1441b9587..41f461bc2e8 100644 --- a/nexus/types/src/deployment/reconfigurator_config.rs +++ b/nexus/types/src/deployment/reconfigurator_config.rs @@ -117,6 +117,7 @@ impl fmt::Display for ReconfiguratorConfigViewDisplay<'_> { pub struct ReconfiguratorConfig { pub planner_enabled: bool, pub planner_config: PlannerConfig, + pub tuf_repo_pruner_enabled: bool, } impl ReconfiguratorConfig { @@ -127,7 +128,11 @@ impl ReconfiguratorConfig { impl Default for ReconfiguratorConfig { fn default() -> Self { - Self { planner_enabled: true, planner_config: PlannerConfig::default() } + Self { + planner_enabled: true, + planner_config: PlannerConfig::default(), + tuf_repo_pruner_enabled: true, + } } } @@ -139,8 +144,14 @@ pub struct ReconfiguratorConfigDisplay<'a> { impl fmt::Display for ReconfiguratorConfigDisplay<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let Self { - config: ReconfiguratorConfig { planner_enabled, planner_config }, + config: + ReconfiguratorConfig { + planner_enabled, + planner_config, + tuf_repo_pruner_enabled, + }, } = self; + writeln!(f, "tuf repo pruner enabled: {}", tuf_repo_pruner_enabled)?; writeln!(f, "planner enabled: {}", planner_enabled)?; writeln!(f, "planner config:")?; // planner_config does its own indentation, so it's not necessary to @@ -164,12 +175,18 @@ pub struct ReconfiguratorConfigDiffDisplay<'a, 'b> { impl fmt::Display for ReconfiguratorConfigDiffDisplay<'_, '_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let ReconfiguratorConfigDiff { planner_enabled, planner_config } = - self.diff; + let ReconfiguratorConfigDiff { + planner_enabled, + planner_config, + tuf_repo_pruner_enabled, + } = self.diff; let list = KvList::new( None, - vec![diff_row!(planner_enabled, "planner enabled")], + vec![ + diff_row!(tuf_repo_pruner_enabled, "tuf repo pruner enabled"), + diff_row!(planner_enabled, "planner enabled"), + ], ); // No need for writeln! here because KvList adds its own newlines. write!(f, "{list}")?; diff --git a/nexus/types/src/internal_api/background.rs b/nexus/types/src/internal_api/background.rs index 715d7e08cbc..f9bc3e0aacc 100644 --- a/nexus/types/src/internal_api/background.rs +++ b/nexus/types/src/internal_api/background.rs @@ -389,8 +389,23 @@ pub enum TufArtifactReplicationOperation { Copy { hash: ArtifactHash, source_sled: SledUuid }, } -#[derive(Debug, Deserialize, Serialize)] -pub struct TufRepoPrunerStatus { +/// High-level status of the TUF repo pruner background task. +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(tag = "status", rename_all = "snake_case")] +#[allow(clippy::large_enum_variant)] +pub enum TufRepoPrunerStatus { + /// The TUF repo pruner is disabled. + Disabled { + /// The reason why the pruner is disabled. + reason: String, + }, + /// The TUF repo pruner is enabled and ran successfully. + Enabled(TufRepoPrunerDetails), +} + +/// Details about a TUF repo pruner run when the task is enabled. +#[derive(Clone, Debug, Deserialize, Serialize)] +pub struct TufRepoPrunerDetails { // Input /// how many recent releases we're configured to keep pub nkeep_recent_releases: u8, @@ -410,7 +425,7 @@ pub struct TufRepoPrunerStatus { pub warnings: Vec, } -impl std::fmt::Display for TufRepoPrunerStatus { +impl std::fmt::Display for TufRepoPrunerDetails { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { fn print_collection(c: &IdOrdMap) -> String { if c.is_empty() { @@ -477,6 +492,22 @@ impl std::fmt::Display for TufRepoPrunerStatus { } } +impl std::fmt::Display for TufRepoPrunerStatus { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + TufRepoPrunerStatus::Disabled { reason } => { + writeln!(f, " status: disabled")?; + writeln!(f, " reason: {}", reason)?; + Ok(()) + } + TufRepoPrunerStatus::Enabled(details) => { + writeln!(f, " status: enabled")?; + details.fmt(f) + } + } + } +} + #[derive(Clone, Debug, Deserialize, Serialize)] pub struct TufRepoInfo { pub id: TufRepoUuid, @@ -811,12 +842,13 @@ pub struct ProbeDistributorStatus { #[cfg(test)] mod test { use super::TufRepoInfo; + use super::TufRepoPrunerDetails; use super::TufRepoPrunerStatus; use expectorate::assert_contents; use iddqd::IdOrdMap; #[test] - fn test_display_tuf_repo_pruner_status() { + fn test_display_tuf_repo_pruner_status_enabled() { let repo1 = TufRepoInfo { id: "4e8a87a0-3102-4014-99d3-e1bf486685bd".parse().unwrap(), system_version: "1.2.3".parse().unwrap(), @@ -829,7 +861,7 @@ mod test { }; let repo_map: IdOrdMap<_> = std::iter::once(repo1.clone()).collect(); - let status = TufRepoPrunerStatus { + let details = TufRepoPrunerDetails { nkeep_recent_releases: 1, nkeep_recent_uploads: 2, repos_keep_target_release: repo_map, @@ -840,9 +872,22 @@ mod test { .collect(), warnings: vec![String::from("fake-oh problem-oh")], }; + let status = TufRepoPrunerStatus::Enabled(details); + + assert_contents( + "output/tuf_repo_pruner_status_enabled.out", + &status.to_string(), + ); + } + + #[test] + fn test_display_tuf_repo_pruner_status_disabled() { + let status = TufRepoPrunerStatus::Disabled { + reason: "disabled in this test".to_string(), + }; assert_contents( - "output/tuf_repo_pruner_status.out", + "output/tuf_repo_pruner_status_disabled.out", &status.to_string(), ); } diff --git a/openapi/nexus-lockstep.json b/openapi/nexus-lockstep.json index c51f905390f..55523c1b9b9 100644 --- a/openapi/nexus-lockstep.json +++ b/openapi/nexus-lockstep.json @@ -7756,11 +7756,15 @@ }, "planner_enabled": { "type": "boolean" + }, + "tuf_repo_pruner_enabled": { + "type": "boolean" } }, "required": [ "planner_config", - "planner_enabled" + "planner_enabled", + "tuf_repo_pruner_enabled" ] }, "ReconfiguratorConfigParam": { diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index fbd29859307..35d8d162ec3 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -4635,7 +4635,10 @@ CREATE TABLE IF NOT EXISTS omicron.public.reconfigurator_config ( time_modified TIMESTAMPTZ NOT NULL, -- Whether to add zones while the system has detected a mupdate override. - add_zones_with_mupdate_override BOOL NOT NULL + add_zones_with_mupdate_override BOOL NOT NULL, + + -- Enable the TUF repo pruner background task + tuf_repo_pruner_enabled BOOL NOT NULL ); /* @@ -7031,7 +7034,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '207.0.0', NULL) + (TRUE, NOW(), NOW(), '208.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/schema/crdb/disable-tuf-repo-pruner/up1.sql b/schema/crdb/disable-tuf-repo-pruner/up1.sql new file mode 100644 index 00000000000..8366f2157f6 --- /dev/null +++ b/schema/crdb/disable-tuf-repo-pruner/up1.sql @@ -0,0 +1,2 @@ +ALTER TABLE omicron.public.reconfigurator_config + ADD COLUMN IF NOT EXISTS tuf_repo_pruner_enabled BOOL NOT NULL DEFAULT TRUE; diff --git a/schema/crdb/disable-tuf-repo-pruner/up2.sql b/schema/crdb/disable-tuf-repo-pruner/up2.sql new file mode 100644 index 00000000000..8e86bfa47cb --- /dev/null +++ b/schema/crdb/disable-tuf-repo-pruner/up2.sql @@ -0,0 +1,2 @@ +ALTER TABLE omicron.public.reconfigurator_config + ALTER COLUMN tuf_repo_pruner_enabled DROP DEFAULT;