Clean up some CLI arguments, make magic rollback optional

7 files changed, 141 insertions, 92 deletions

diff --git a/interface/deploy.json b/interface/deploy.json
index 19da486..93bacb8 100644
--- a/interface/deploy.json
+++ b/interface/deploy.json
@@ -23,6 +23,15 @@
                 },
                 "autoRollback": {
                     "type": "boolean"
+                },
+                "magicRollback": {
+                    "type": "boolean"
+                },
+                "confirmTimeout": {
+                    "type": "int"
+                },
+                "tempPath": {
+                    "type": "integer"
                 }
             }
         },
diff --git a/src/activate.rs b/src/activate.rs
index ce6b286..55ceb27 100644
--- a/src/activate.rs
+++ b/src/activate.rs
@@ -34,8 +34,18 @@ mod utils;
 struct Opts {
     profile_path: String,
     closure: String,
+
+    /// Temp path for any temporary files that may be needed during activation
+    #[clap(long)]
     temp_path: String,
-    max_time: u16,
+
+    /// Maximum time to wait for confirmation after activation
+    #[clap(long)]
+    confirm_timeout: u16,
+
+    /// Wait for confirmation after deployment and rollback if not confirmed
+    #[clap(long)]
+    magic_rollback: bool,
 
     /// Command for bootstrapping
     #[clap(long)]
@@ -139,7 +149,7 @@ async fn deactivate_on_err<A, B: core::fmt::Debug>(profile_path: &str, r: Result
 pub async fn activation_confirmation(
     profile_path: String,
     temp_path: String,
-    max_time: u16,
+    confirm_timeout: u16,
     closure: String,
 ) -> Result<(), Box<dyn std::error::Error>> {
     let lock_hash = &closure[11 /* /nix/store/ */ ..];
@@ -172,7 +182,8 @@ pub async fn activation_confirmation(
                             &profile_path,
                             deactivate_on_err(
                                 &profile_path,
-                                timeout(Duration::from_secs(max_time as u64), stream.next()).await,
+                                timeout(Duration::from_secs(confirm_timeout as u64), stream.next())
+                                    .await,
                             )
                             .await
                             .ok_or("Watcher ended prematurely"),
@@ -201,7 +212,8 @@ pub async fn activate(
     bootstrap_cmd: Option<String>,
     auto_rollback: bool,
     temp_path: String,
-    max_time: u16,
+    confirm_timeout: u16,
+    magic_rollback: bool,
 ) -> Result<(), Box<dyn std::error::Error>> {
     info!("Activating profile");
 
@@ -249,13 +261,17 @@ pub async fn activate(
         _ => (),
     }
 
-    info!("Activation succeeded, now performing post-activation checks");
+    info!("Activation succeeded!");
 
-    deactivate_on_err(
-        &profile_path,
-        activation_confirmation(profile_path.clone(), temp_path, max_time, closure).await,
-    )
-    .await;
+    if magic_rollback {
+        info!("Performing activation confirmation steps");
+        deactivate_on_err(
+            &profile_path,
+            activation_confirmation(profile_path.clone(), temp_path, confirm_timeout, closure)
+                .await,
+        )
+        .await;
+    }
 
     Ok(())
 }
@@ -276,7 +292,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
         opts.bootstrap_cmd,
         opts.auto_rollback,
         opts.temp_path,
-        opts.max_time,
+        opts.confirm_timeout,
+        opts.magic_rollback,
     )
     .await?;
 
diff --git a/src/main.rs b/src/main.rs
index b28a520..cedf684 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -51,9 +51,15 @@ struct Opts {
     /// Override hostname used for the node
     #[clap(long)]
     hostname: Option<String>,
-    /// Skip pushing step (useful for local testing)
-    #[clap(short, long)]
-    skip_push: bool,
+    /// Make activation wait for confirmation, or roll back after a period of time
+    #[clap(long)]
+    magic_rollback: Option<bool>,
+    /// How long activation should wait for confirmation (if using magic-rollback)
+    #[clap(long)]
+    confirm_timeout: Option<u16>,
+    /// Where to store temporary files (only used by magic-rollback)
+    #[clap(long)]
+    temp_path: Option<String>,
 }
 
 #[inline]
@@ -238,7 +244,6 @@ async fn run_deploy(
     data: utils::data::Data,
     supports_flakes: bool,
     check_sigs: bool,
-    skip_push: bool,
     cmd_overrides: utils::CmdOverrides,
 ) -> Result<(), Box<dyn std::error::Error>> {
     match (deploy_flake.node, deploy_flake.profile) {
@@ -263,16 +268,14 @@ async fn run_deploy(
 
             let deploy_defs = deploy_data.defs();
 
-            if !skip_push {
-                utils::push::push_profile(
-                    supports_flakes,
-                    check_sigs,
-                    deploy_flake.repo,
-                    &deploy_data,
-                    &deploy_defs,
-                )
-                .await?;
-            }
+            utils::push::push_profile(
+                supports_flakes,
+                check_sigs,
+                deploy_flake.repo,
+                &deploy_data,
+                &deploy_defs,
+            )
+            .await?;
 
             utils::deploy::deploy_profile(&deploy_data, &deploy_defs).await?;
         }
@@ -282,7 +285,23 @@ async fn run_deploy(
                 None => good_panic!("No node was found named `{}`", node_name),
             };
 
-            if !skip_push {
+            push_all_profiles(
+                node,
+                node_name,
+                supports_flakes,
+                deploy_flake.repo,
+                &data.generic_settings,
+                check_sigs,
+                &cmd_overrides,
+            )
+            .await?;
+
+            deploy_all_profiles(node, node_name, &data.generic_settings, &cmd_overrides).await?;
+        }
+        (None, None) => {
+            info!("Deploying all profiles on all nodes");
+
+            for (node_name, node) in &data.nodes {
                 push_all_profiles(
                     node,
                     node_name,
@@ -295,26 +314,6 @@ async fn run_deploy(
                 .await?;
             }
 
-            deploy_all_profiles(node, node_name, &data.generic_settings, &cmd_overrides).await?;
-        }
-        (None, None) => {
-            info!("Deploying all profiles on all nodes");
-
-            if !skip_push {
-                for (node_name, node) in &data.nodes {
-                    push_all_profiles(
-                        node,
-                        node_name,
-                        supports_flakes,
-                        deploy_flake.repo,
-                        &data.generic_settings,
-                        check_sigs,
-                        &cmd_overrides,
-                    )
-                    .await?;
-                }
-            }
-
             for (node_name, node) in &data.nodes {
                 deploy_all_profiles(node, node_name, &data.generic_settings, &cmd_overrides)
                     .await?;
@@ -347,6 +346,9 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
         fast_connection: opts.fast_connection,
         auto_rollback: opts.auto_rollback,
         hostname: opts.hostname,
+        magic_rollback: opts.magic_rollback,
+        temp_path: opts.temp_path,
+        confirm_timeout: opts.confirm_timeout,
     };
 
     let supports_flakes = test_flake_support().await?;
@@ -359,7 +361,6 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
         data,
         supports_flakes,
         opts.checksigs,
-        opts.skip_push,
         cmd_overrides,
     )
     .await?;
diff --git a/src/utils/data.rs b/src/utils/data.rs
index 351b9ae..5c58e3b 100644
--- a/src/utils/data.rs
+++ b/src/utils/data.rs
@@ -19,11 +19,15 @@ pub struct GenericSettings {
     #[merge(strategy = merge::vec::append)]
     pub ssh_opts: Vec<String>,
     #[serde(rename(deserialize = "fastConnection"), default)]
-    #[merge(strategy = merge::bool::overwrite_false)]
-    pub fast_connection: bool,
+    pub fast_connection: Option<bool>,
     #[serde(rename(deserialize = "autoRollback"), default)]
-    #[merge(strategy = merge::bool::overwrite_false)]
-    pub auto_rollback: bool,
+    pub auto_rollback: Option<bool>,
+    #[serde(rename(deserialize = "confirmTimeout"))]
+    pub confirm_timeout: Option<u16>,
+    #[serde(rename(deserialize = "tempPath"))]
+    pub temp_path: Option<String>,
+    #[serde(rename(deserialize = "magicRollback"))]
+    pub magic_rollback: Option<bool>,
 }
 
 #[derive(Deserialize, Debug, Clone)]
@@ -44,10 +48,6 @@ pub struct ProfileSettings {
     pub bootstrap: Option<String>,
     #[serde(rename(deserialize = "profilePath"))]
     pub profile_path: Option<String>,
-    #[serde(rename(deserialize = "maxTime"))]
-    pub max_time: Option<u16>,
-    #[serde(rename(deserialize = "tempPath"))]
-    pub temp_path: Option<String>,
 }
 
 #[derive(Deserialize, Debug, Clone)]
diff --git a/src/utils/deploy.rs b/src/utils/deploy.rs
index e3493ba..9b2f685 100644
--- a/src/utils/deploy.rs
+++ b/src/utils/deploy.rs
@@ -13,15 +13,20 @@ fn build_activate_command(
     bootstrap_cmd: &Option<String>,
     auto_rollback: bool,
     temp_path: &Cow<str>,
-    max_time: u16,
+    confirm_timeout: u16,
+    magic_rollback: bool,
 ) -> String {
     let mut self_activate_command = format!(
-        "{} '{}' '{}' {} {}",
-        activate_path_str, profile_path, closure, temp_path, max_time
+        "{} '{}' '{}' --temp-path {} --confirm-timeout {}",
+        activate_path_str, profile_path, closure, temp_path, confirm_timeout
     );
 
-    if let Some(sudo_cmd) = &sudo {
-        self_activate_command = format!("{} {}", sudo_cmd, self_activate_command);
+    if magic_rollback {
+        self_activate_command = format!("{} --magic-rollback", self_activate_command);
+    }
+
+    if auto_rollback {
+        self_activate_command = format!("{} --auto-rollback", self_activate_command);
     }
 
     if let Some(ref bootstrap_cmd) = bootstrap_cmd {
@@ -31,8 +36,8 @@ fn build_activate_command(
         );
     }
 
-    if auto_rollback {
-        self_activate_command = format!("{} --auto-rollback", self_activate_command);
+    if let Some(sudo_cmd) = &sudo {
+        self_activate_command = format!("{} {}", sudo_cmd, self_activate_command);
     }
 
     self_activate_command
@@ -47,7 +52,8 @@ fn test_activation_command_builder() {
     let bootstrap_cmd = None;
     let auto_rollback = true;
     let temp_path = &"/tmp/deploy-rs".into();
-    let max_time = 30;
+    let confirm_timeout = 30;
+    let magic_rollback = true;
 
     assert_eq!(
         build_activate_command(
@@ -58,9 +64,10 @@ fn test_activation_command_builder() {
             &bootstrap_cmd,
             auto_rollback,
             temp_path,
-            max_time
+            confirm_timeout,
+            magic_rollback
         ),
-        "sudo -u test /blah/bin/activate '/blah/profiles/test' '/blah/etc' /tmp/deploy-rs 30 --auto-rollback"
+        "sudo -u test /blah/bin/activate '/blah/profiles/test' '/blah/etc' --temp-path /tmp/deploy-rs --confirm-timeout 30 --magic-rollback --auto-rollback"
             .to_string(),
     );
 }
@@ -76,12 +83,16 @@ pub async fn deploy_profile(
 
     let activate_path_str = super::deploy_path_to_activate_path_str(&deploy_defs.current_exe)?;
 
-    let temp_path: Cow<str> = match &deploy_data.profile.profile_settings.temp_path {
+    let temp_path: Cow<str> = match &deploy_data.merged_settings.temp_path {
         Some(x) => x.into(),
         None => "/tmp/deploy-rs".into(),
     };
 
-    let max_time = deploy_data.profile.profile_settings.max_time.unwrap_or(30);
+    let confirm_timeout = deploy_data.merged_settings.confirm_timeout.unwrap_or(30);
+
+    let magic_rollback = deploy_data.merged_settings.magic_rollback.unwrap_or(false);
+
+    let auto_rollback = deploy_data.merged_settings.auto_rollback.unwrap_or(true);
 
     let self_activate_command = build_activate_command(
         activate_path_str,
@@ -89,9 +100,10 @@ pub async fn deploy_profile(
         &deploy_defs.profile_path,
         &deploy_data.profile.profile_settings.path,
         &deploy_data.profile.profile_settings.bootstrap,
-        deploy_data.merged_settings.auto_rollback,
+        auto_rollback,
         &temp_path,
-        max_time,
+        confirm_timeout,
+        magic_rollback,
     );
 
     let hostname = match deploy_data.cmd_overrides.hostname {
@@ -112,33 +124,37 @@ pub async fn deploy_profile(
         good_panic!("Activation over SSH failed");
     }
 
-    info!("Success, attempting to connect to the node to confirm deployment");
+    info!("Success activating!");
 
-    let mut c = Command::new("ssh");
-    let mut ssh_confirm_command = c.arg(format!("ssh://{}@{}", deploy_defs.ssh_user, hostname));
+    if magic_rollback {
+        info!("Attempting to confirm activation");
 
-    for ssh_opt in &deploy_data.merged_settings.ssh_opts {
-        ssh_confirm_command = ssh_confirm_command.arg(ssh_opt);
-    }
+        let mut c = Command::new("ssh");
+        let mut ssh_confirm_command = c.arg(format!("ssh://{}@{}", deploy_defs.ssh_user, hostname));
 
-    let lock_hash = &deploy_data.profile.profile_settings.path[11 /* /nix/store/ */ ..];
-    let lock_path = format!("{}/activating-{}", temp_path, lock_hash);
+        for ssh_opt in &deploy_data.merged_settings.ssh_opts {
+            ssh_confirm_command = ssh_confirm_command.arg(ssh_opt);
+        }
 
-    let mut confirm_command = format!("rm {}", lock_path);
-    if let Some(sudo_cmd) = &deploy_defs.sudo {
-        confirm_command = format!("{} {}", sudo_cmd, confirm_command);
-    }
+        let lock_hash = &deploy_data.profile.profile_settings.path[11 /* /nix/store/ */ ..];
+        let lock_path = format!("{}/activating-{}", temp_path, lock_hash);
 
-    let ssh_exit_status = ssh_confirm_command.arg(confirm_command).status().await?;
+        let mut confirm_command = format!("rm {}", lock_path);
+        if let Some(sudo_cmd) = &deploy_defs.sudo {
+            confirm_command = format!("{} {}", sudo_cmd, confirm_command);
+        }
 
-    if !ssh_exit_status.success() {
-        good_panic!(
-            "Failed to confirm deployment, the node will roll back in <{} seconds",
-            max_time
-        );
-    }
+        let ssh_exit_status = ssh_confirm_command.arg(confirm_command).status().await?;
 
-    info!("Deployment confirmed.");
+        if !ssh_exit_status.success() {
+            good_panic!(
+                "Failed to confirm deployment, the node will roll back in <{} seconds",
+                confirm_timeout
+            );
+        }
+
+        info!("Deployment confirmed.");
+    }
 
     Ok(())
 }
diff --git a/src/utils/mod.rs b/src/utils/mod.rs
index 672a9ba..a0e62e1 100644
--- a/src/utils/mod.rs
+++ b/src/utils/mod.rs
@@ -26,6 +26,9 @@ pub struct CmdOverrides {
     pub fast_connection: Option<bool>,
     pub auto_rollback: Option<bool>,
     pub hostname: Option<String>,
+    pub magic_rollback: Option<bool>,
+    pub temp_path: Option<String>,
+    pub confirm_timeout: Option<u16>,
 }
 
 #[derive(PartialEq, Debug)]
@@ -184,10 +187,13 @@ pub fn make_deploy_data<'a, 's>(
         merged_settings.ssh_opts = ssh_opts.split(' ').map(|x| x.to_owned()).collect();
     }
     if let Some(fast_connection) = cmd_overrides.fast_connection {
-        merged_settings.fast_connection = fast_connection;
+        merged_settings.fast_connection = Some(fast_connection);
     }
     if let Some(auto_rollback) = cmd_overrides.auto_rollback {
-        merged_settings.auto_rollback = auto_rollback;
+        merged_settings.auto_rollback = Some(auto_rollback);
+    }
+    if let Some(magic_rollback) = cmd_overrides.magic_rollback {
+        merged_settings.magic_rollback = Some(magic_rollback);
     }
 
     Ok(DeployData {
diff --git a/src/utils/push.rs b/src/utils/push.rs
index 3f48d68..f80f9f8 100644
--- a/src/utils/push.rs
+++ b/src/utils/push.rs
@@ -80,7 +80,7 @@ pub async fn push_profile(
     let mut copy_command_ = Command::new("nix");
     let mut copy_command = copy_command_.arg("copy");
 
-    if deploy_data.merged_settings.fast_connection {
+    if let Some(true) = deploy_data.merged_settings.fast_connection {
         copy_command = copy_command.arg("--substitute-on-destination");
     }