From aeeee3c1e3e9bfc38462cb315b6e19ee9fe6db70 Mon Sep 17 00:00:00 2001 From: Roman Melnikov Date: Wed, 1 Nov 2023 12:42:30 +0100 Subject: [PATCH 1/2] [Chore] Make activation wait timeout configurable Problem: Currently profile activation waiting timeout is hardcoded to 240 seconds, see https://github.com/serokell/deploy-rs/pull/48. In some cases, this timeout can be exceeded (e.g. activation performs a heavy DB migration and waits for it to finish before considering the profile activation succesful). Solution: Make this timeout configurable via 'activationTimeout' deploy attribute or corresponding '--activation-timeout' CLI option. For the sake of backward compatibility, the new 'wait' subcommand '--activation-timeout' option is made optional and defaults to 240 seconds if it wasn't provided. --- interface.json | 3 +++ src/bin/activate.rs | 10 +++++++--- src/cli.rs | 4 ++++ src/data.rs | 2 ++ src/deploy.rs | 11 ++++++++++- src/lib.rs | 4 ++++ 6 files changed, 30 insertions(+), 4 deletions(-) diff --git a/interface.json b/interface.json index a9471f4f..c733f255 100644 --- a/interface.json +++ b/interface.json @@ -30,6 +30,9 @@ "confirmTimeout": { "type": "integer" }, + "activationTimeout": { + "type": "integer" + }, "tempPath": { "type": "string" } diff --git a/src/bin/activate.rs b/src/bin/activate.rs index 4a2760b6..40175105 100644 --- a/src/bin/activate.rs +++ b/src/bin/activate.rs @@ -101,6 +101,10 @@ struct WaitOpts { /// Path for any temporary files that may be needed during activation #[clap(long)] temp_path: PathBuf, + + /// Timeout to wait for activation + #[clap(long)] + activation_timeout: Option, } /// Revoke profile activation @@ -319,7 +323,7 @@ pub enum WaitError { #[error("Error waiting for activation: {0}")] Waiting(#[from] DangerZoneError), } -pub async fn wait(temp_path: PathBuf, closure: String) -> Result<(), WaitError> { +pub async fn wait(temp_path: PathBuf, closure: String, activation_timeout: Option) -> Result<(), WaitError> { let lock_path = deploy::make_lock_path(&temp_path, &closure); let (created, done) = mpsc::channel(1); @@ -359,7 +363,7 @@ pub async fn wait(temp_path: PathBuf, closure: String) -> Result<(), WaitError> return Ok(()); } - danger_zone(done, 240).await?; + danger_zone(done, activation_timeout.unwrap_or(240)).await?; info!("Found canary file, done waiting!"); @@ -575,7 +579,7 @@ async fn main() -> Result<(), Box> { .await .map_err(|x| Box::new(x) as Box), - SubCommand::Wait(wait_opts) => wait(wait_opts.temp_path, wait_opts.closure) + SubCommand::Wait(wait_opts) => wait(wait_opts.temp_path, wait_opts.closure, wait_opts.activation_timeout) .await .map_err(|x| Box::new(x) as Box), diff --git a/src/cli.rs b/src/cli.rs index cc17ea90..b25aed20 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -85,6 +85,9 @@ pub struct Opts { /// How long activation should wait for confirmation (if using magic-rollback) #[clap(long)] confirm_timeout: Option, + /// How long we should wait for profile activation (if using magic-rollback) + #[clap(long)] + activation_timeout: Option, /// Where to store temporary files (only used by magic-rollback) #[clap(long)] temp_path: Option, @@ -658,6 +661,7 @@ pub async fn run(args: Option<&ArgMatches>) -> Result<(), RunError> { magic_rollback: opts.magic_rollback, temp_path: opts.temp_path, confirm_timeout: opts.confirm_timeout, + activation_timeout: opts.activation_timeout, dry_activate: opts.dry_activate, remote_build: opts.remote_build, sudo: opts.sudo, diff --git a/src/data.rs b/src/data.rs index 3b3e2c92..c507a31a 100644 --- a/src/data.rs +++ b/src/data.rs @@ -25,6 +25,8 @@ pub struct GenericSettings { pub auto_rollback: Option, #[serde(rename(deserialize = "confirmTimeout"))] pub confirm_timeout: Option, + #[serde(rename(deserialize = "activationTimeout"))] + pub activation_timeout: Option, #[serde(rename(deserialize = "tempPath"))] pub temp_path: Option, #[serde(rename(deserialize = "magicRollback"))] diff --git a/src/deploy.rs b/src/deploy.rs index 41cd58ba..a371c181 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -121,6 +121,7 @@ struct WaitCommandData<'a> { sudo: &'a Option, closure: &'a str, temp_path: &'a Path, + activation_timeout: Option, debug_logs: bool, log_dir: Option<&'a str>, } @@ -142,6 +143,9 @@ fn build_wait_command(data: &WaitCommandData) -> String { data.closure, data.temp_path.display(), ); + if let Some(activation_timeout) = data.activation_timeout { + self_activate_command = format!("{} --activation-timeout {}", self_activate_command, activation_timeout); + } if let Some(sudo_cmd) = &data.sudo { self_activate_command = format!("{} {}", sudo_cmd, self_activate_command); @@ -155,6 +159,7 @@ fn test_wait_command_builder() { let sudo = Some("sudo -u test".to_string()); let closure = "/nix/store/blah/etc"; let temp_path = Path::new("/tmp"); + let activation_timeout = Some(600); let debug_logs = true; let log_dir = Some("/tmp/something.txt"); @@ -163,10 +168,11 @@ fn test_wait_command_builder() { sudo: &sudo, closure, temp_path, + activation_timeout, debug_logs, log_dir }), - "sudo -u test /nix/store/blah/etc/activate-rs --debug-logs --log-dir /tmp/something.txt wait '/nix/store/blah/etc' --temp-path '/tmp'" + "sudo -u test /nix/store/blah/etc/activate-rs --debug-logs --log-dir /tmp/something.txt wait '/nix/store/blah/etc' --temp-path '/tmp' --activation-timeout 600" .to_string(), ); } @@ -328,6 +334,8 @@ pub async fn deploy_profile( let confirm_timeout = deploy_data.merged_settings.confirm_timeout.unwrap_or(30); + let activation_timeout = deploy_data.merged_settings.activation_timeout; + let magic_rollback = deploy_data.merged_settings.magic_rollback.unwrap_or(true); let auto_rollback = deploy_data.merged_settings.auto_rollback.unwrap_or(true); @@ -386,6 +394,7 @@ pub async fn deploy_profile( sudo: &deploy_defs.sudo, closure: &deploy_data.profile.profile_settings.path, temp_path: temp_path, + activation_timeout: activation_timeout, debug_logs: deploy_data.debug_logs, log_dir: deploy_data.log_dir, }); diff --git a/src/lib.rs b/src/lib.rs index 0e5d817b..856514f7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -163,6 +163,7 @@ pub struct CmdOverrides { pub magic_rollback: Option, pub temp_path: Option, pub confirm_timeout: Option, + pub activation_timeout: Option, pub sudo: Option, pub dry_activate: bool, pub remote_build: bool, @@ -444,6 +445,9 @@ pub fn make_deploy_data<'a, 's>( if let Some(confirm_timeout) = cmd_overrides.confirm_timeout { merged_settings.confirm_timeout = Some(confirm_timeout); } + if let Some(activation_timeout) = cmd_overrides.activation_timeout { + merged_settings.confirm_timeout = Some(activation_timeout); + } DeployData { node_name, From 50d640f4032c32d5bddab31af493670ba1773518 Mon Sep 17 00:00:00 2001 From: Roman Melnikov Date: Wed, 1 Nov 2023 17:00:45 +0100 Subject: [PATCH 2/2] fixup! [Chore] Make activation wait timeout configurable --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 856514f7..663e26eb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -446,7 +446,7 @@ pub fn make_deploy_data<'a, 's>( merged_settings.confirm_timeout = Some(confirm_timeout); } if let Some(activation_timeout) = cmd_overrides.activation_timeout { - merged_settings.confirm_timeout = Some(activation_timeout); + merged_settings.activation_timeout = Some(activation_timeout); } DeployData {