Merge pull request #127 from trydirect/feature-redeploy-lock

Feature redeploy lock

Author: vsilent

src/cli/install_runner.rs

@@ -636,6 +636,15 @@ impl DeployStrategy for CloudDeploy {
                                     "server_id".to_string(),
                                     serde_json::Value::Number(sid.into()),
                                 );
+                                // When reusing an existing server, preserve
+                                // the user-chosen / looked-up name rather
+                                // than the auto-generated one.
+                                if let Some(srv_name) = &server_name {
+                                    obj.insert(
+                                        "name".to_string(),
+                                        serde_json::Value::String(srv_name.clone()),
+                                    );
+                                }
                             }
                         }
                     }

src/cli/stacker_client.rs

@@ -997,12 +997,21 @@ fn parse_port_mapping(port_str: &str) -> (String, String) {
 }
 
 /// Parse a volume mapping string like "./dist:/usr/share/nginx/html" or "data:/var/lib/db"
-/// into (host_path, container_path) tuple.
-fn parse_volume_mapping(vol_str: &str) -> (String, String) {
-    if let Some((host, container)) = vol_str.split_once(':') {
-        (host.to_string(), container.to_string())
-    } else {
-        (vol_str.to_string(), vol_str.to_string())
+/// into (host_path, container_path, read_only) tuple.
+/// Handles optional `:ro` / `:rw` suffix (e.g. "/var/run/docker.sock:/var/run/docker.sock:ro").
+fn parse_volume_mapping(vol_str: &str) -> (String, String, bool) {
+    let parts: Vec<&str> = vol_str.split(':').collect();
+    match parts.len() {
+        // "source:target:mode" (e.g. "/host:/container:ro")
+        3 => (
+            parts[0].to_string(),
+            parts[1].to_string(),
+            parts[2] == "ro",
+        ),
+        // "source:target"
+        2 => (parts[0].to_string(), parts[1].to_string(), false),
+        // bare path
+        _ => (vol_str.to_string(), vol_str.to_string(), false),
     }
 }
 
@@ -1028,10 +1037,11 @@ fn service_to_app_json(svc: &ServiceDefinition, network_ids: &[String]) -> serde
         .volumes
         .iter()
         .map(|v| {
-            let (host, container) = parse_volume_mapping(v);
+            let (host, container, read_only) = parse_volume_mapping(v);
             serde_json::json!({
                 "host_path": host,
                 "container_path": container,
+                "read_only": read_only,
             })
         })
         .collect();
@@ -1116,10 +1126,11 @@ fn app_source_to_app_json(
         .volumes
         .iter()
         .map(|v| {
-            let (host, container) = parse_volume_mapping(v);
+            let (host, container, read_only) = parse_volume_mapping(v);
             serde_json::json!({
                 "host_path": host,
                 "container_path": container,
+                "read_only": read_only,
             })
         })
         .collect();
@@ -1255,6 +1266,57 @@ pub fn build_project_body(config: &StackerConfig) -> serde_json::Value {
 
 /// Build the deploy form payload that matches the Stacker server's
 /// `forms::project::Deploy` structure.
+/// Generate a deterministic but unique server name from the project name.
+///
+/// Format: `{project}-{4hex}` where the hex suffix is derived from the current
+/// timestamp so each deploy gets a distinct name, e.g. `website-a3f1`.
+///
+/// The name is sanitised to satisfy the strictest provider rules (Hetzner):
+///   - only lowercase `a-z`, `0-9`, `-`
+///   - must start with a letter
+///   - must not end with `-`
+///   - max 63 characters total
+fn generate_server_name(project_name: &str) -> String {
+    use std::time::{SystemTime, UNIX_EPOCH};
+
+    // Sanitise project name: lowercase, replace non-alnum with hyphen, collapse runs
+    let sanitised: String = project_name
+        .to_lowercase()
+        .chars()
+        .map(|c| if c.is_ascii_alphanumeric() || c == '-' { c } else { '-' })
+        .collect::<String>()
+        .split('-')
+        .filter(|s| !s.is_empty())
+        .collect::<Vec<_>>()
+        .join("-");
+
+    // Ensure it starts with a letter (Hetzner requirement)
+    let base = if sanitised.is_empty() {
+        "srv".to_string()
+    } else if !sanitised.starts_with(|c: char| c.is_ascii_lowercase()) {
+        format!("srv-{}", sanitised)
+    } else {
+        sanitised
+    };
+
+    // 4-char hex suffix from current timestamp (unique per ~65k deploys within any second)
+    let ts = SystemTime::now()
+        .duration_since(UNIX_EPOCH)
+        .unwrap_or_default()
+        .as_millis();
+    let suffix = format!("{:04x}", (ts & 0xFFFF) as u16);
+
+    // Truncate base so total stays within 63 chars: base + '-' + 4-char suffix = base ≤ 58
+    let max_base = 63 - 1 - suffix.len(); // 58
+    let truncated = if base.len() > max_base {
+        base[..max_base].trim_end_matches('-').to_string()
+    } else {
+        base
+    };
+
+    format!("{}-{}", truncated, suffix)
+}
+
 pub fn build_deploy_form(config: &StackerConfig) -> serde_json::Value {
     let cloud = config.deploy.cloud.as_ref();
     let provider = cloud
@@ -1267,6 +1329,11 @@ pub fn build_deploy_form(config: &StackerConfig) -> serde_json::Value {
         _ => "ubuntu-22.04",
     };
 
+    // Auto-generate a server name from the project name so every
+    // provisioned server gets a recognisable label in `stacker list servers`.
+    let project_name = config.project.identity.clone().unwrap_or_else(|| config.name.clone());
+    let server_name = generate_server_name(&project_name);
+
     let mut form = serde_json::json!({
         "cloud": {
             "provider": provider,
@@ -1276,6 +1343,7 @@ pub fn build_deploy_form(config: &StackerConfig) -> serde_json::Value {
             "region": region,
             "server": server_size,
             "os": os,
+            "name": server_name,
         },
         "stack": {
             "stack_code": config.project.identity.clone().unwrap_or_else(|| config.name.clone()),
@@ -1374,6 +1442,10 @@ mod tests {
         assert_eq!(form["server"]["region"], "fsn1");
         assert_eq!(form["server"]["server"], "cpx11");
         assert_eq!(form["stack"]["stack_code"], "myproject");
+        // Auto-generated server name should start with the project name
+        let name = form["server"]["name"].as_str().unwrap();
+        assert!(name.starts_with("myproject-"), "server name should start with project name, got: {}", name);
+        assert_eq!(name.len(), "myproject-".len() + 4, "suffix should be 4 hex chars");
     }
 
     #[test]
@@ -1513,4 +1585,49 @@ mod tests {
         let features = body["custom"]["feature"].as_array().unwrap();
         assert!(features.is_empty(), "feature array should be empty when no proxy configured");
     }
+
+    #[test]
+    fn test_generate_server_name_basic() {
+        let name = generate_server_name("website");
+        assert!(name.starts_with("website-"), "got: {}", name);
+        // 4 hex chars suffix
+        let suffix = &name["website-".len()..];
+        assert_eq!(suffix.len(), 4);
+        assert!(suffix.chars().all(|c| c.is_ascii_hexdigit()), "suffix should be hex, got: {}", suffix);
+    }
+
+    #[test]
+    fn test_generate_server_name_sanitises() {
+        let name = generate_server_name("My Cool App!");
+        assert!(name.starts_with("my-cool-app-"), "got: {}", name);
+    }
+
+    #[test]
+    fn test_generate_server_name_empty() {
+        let name = generate_server_name("");
+        assert!(name.starts_with("srv-"), "empty input should fallback to 'srv', got: {}", name);
+    }
+
+    #[test]
+    fn test_generate_server_name_special_chars() {
+        let name = generate_server_name("app___v2..beta");
+        assert!(name.starts_with("app-v2-beta-"), "consecutive separators collapsed, got: {}", name);
+    }
+
+    #[test]
+    fn test_generate_server_name_numeric_start() {
+        // Hetzner requires name to start with a letter
+        let name = generate_server_name("123app");
+        assert!(name.starts_with("srv-123app-"), "numeric start should get 'srv-' prefix, got: {}", name);
+    }
+
+    #[test]
+    fn test_generate_server_name_max_length() {
+        let long = "a".repeat(100);
+        let name = generate_server_name(&long);
+        assert!(name.len() <= 63, "name must be ≤63 chars (Hetzner), got {} chars: {}", name.len(), name);
+        assert!(name.starts_with("aaa"), "got: {}", name);
+        // Must not end with hyphen
+        assert!(!name.ends_with('-'), "must not end with hyphen, got: {}", name);
+    }
 }

src/console/commands/cli/deploy.rs

@@ -755,6 +755,8 @@ pub fn run_deploy(
     //     If the server is reachable, automatically switch to Server target.
     //     If not, show diagnostics and abort so the user can fix or remove the section.
     //     Skipped when --force-new is set (user explicitly wants a fresh cloud provision).
+    //     When a lockfile exists, auto-inject the server name so the API reuses the server.
+    let mut lock_server_name: Option<String> = None;
     if deploy_target == DeployTarget::Cloud && !force_new {
         if let Some(ref server_cfg) = config.deploy.server {
             eprintln!("  Found deploy.server section (host={}). Checking SSH connectivity...", server_cfg.host);
@@ -811,12 +813,16 @@ pub fn run_deploy(
             }
         } else if DeploymentLock::exists(project_dir) {
             // No deploy.server in config, but a lockfile exists from a prior deploy.
-            // Inform the user without auto-switching — they must opt in.
+            // Auto-inject the server name so the cloud deploy API reuses the same server.
             if let Ok(Some(lock)) = DeploymentLock::load(project_dir) {
-                if let Some(ref ip) = lock.server_ip {
+                if let Some(ref name) = lock.server_name {
+                    eprintln!("  ℹ Found previous deployment (server='{}') — reusing server", name);
+                    eprintln!("    To provision a new server instead: stacker deploy --force-new");
+                    lock_server_name = Some(name.clone());
+                } else if let Some(ref ip) = lock.server_ip {
                     if ip != "127.0.0.1" {
                         eprintln!("  ℹ Found previous deployment to {} (from .stacker/deployment.lock)", ip);
-                        eprintln!("    To redeploy to the same server, run: stacker config lock");
+                        eprintln!("    Server name unknown — cannot auto-reuse. Run: stacker config lock");
                         eprintln!("    To provision a new server instead:   stacker deploy --force-new");
                     }
                 }
@@ -924,7 +930,10 @@ pub fn run_deploy(
         project_name_override: remote_overrides.project_name.clone(),
         key_name_override: remote_overrides.key_name.clone(),
         key_id_override: remote_overrides.key_id,
-        server_name_override: remote_overrides.server_name.clone(),
+        server_name_override: remote_overrides
+            .server_name
+            .clone()
+            .or(lock_server_name),
     };
 
     let result = strategy.deploy(&config, &context, executor)?;
@@ -1046,6 +1055,23 @@ impl DeployCommand {
                 let mut l = DeploymentLock::from_result(result)
                     .with_project_name(self.project_name.clone());
 
+                // If no --project flag, try to get the project name from config
+                if l.project_name.is_none() {
+                    let config_path = match &self.file {
+                        Some(f) => project_dir.join(f),
+                        None => project_dir.join(DEFAULT_CONFIG_FILE),
+                    };
+                    if let Ok(config) = StackerConfig::from_file(&config_path) {
+                        // Prefer project.identity as the registered name, fall back to config name
+                        let name = config
+                            .project
+                            .identity
+                            .filter(|s| !s.is_empty())
+                            .unwrap_or(config.name);
+                        l = l.with_project_name(Some(name));
+                    }
+                }
+
                 // Try to fetch provisioned server details from the Stacker API
                 if let Some(project_id) = result.project_id {
                     match fetch_server_for_project(project_id as i32) {
@@ -1136,9 +1162,15 @@ impl DeployCommand {
 
 /// After a cloud deploy completes, look up the provisioned server's details
 /// (IP, SSH user, port, name) from the Stacker server API.
+///
+/// Retries up to 3 times with a 10-second delay between attempts, because the
+/// server IP may not yet be assigned right after the deployment status becomes
+/// "completed".
 fn fetch_server_for_project(
     project_id: i32,
 ) -> Result<Option<stacker_client::ServerInfo>, Box<dyn std::error::Error>> {
+    use std::time::Duration;
+
     let cred_manager = CredentialsManager::with_default_store();
     let creds = cred_manager.require_valid_token("server lookup")?;
 
@@ -1152,14 +1184,56 @@ fn fetch_server_for_project(
 
     rt.block_on(async {
         let client = StackerClient::new(&base_url, &creds.access_token);
-        let servers = client.list_servers().await?;
 
-        // Find the server linked to this project
-        let server = servers
-            .into_iter()
-            .find(|s| s.project_id == project_id && s.srv_ip.is_some());
+        let max_retries = 3;
+        let retry_delay = Duration::from_secs(10);
+
+        for attempt in 0..max_retries {
+            let servers = client.list_servers().await?;
+
+            // Find the server linked to this project
+            let server = servers
+                .into_iter()
+                .find(|s| s.project_id == project_id);
+
+            match server {
+                Some(ref s) if s.srv_ip.is_some() => {
+                    // Server found with IP — done
+                    return Ok(server);
+                }
+                Some(_) if attempt < max_retries - 1 => {
+                    // Server found but no IP yet — wait and retry
+                    eprintln!(
+                        "  Server found but IP not yet assigned (attempt {}/{}), retrying in {}s...",
+                        attempt + 1,
+                        max_retries,
+                        retry_delay.as_secs(),
+                    );
+                    tokio::time::sleep(retry_delay).await;
+                }
+                Some(s) => {
+                    // Final attempt — return server even without IP so we capture
+                    // name, cloud_id, etc.
+                    return Ok(Some(s));
+                }
+                None if attempt < max_retries - 1 => {
+                    // No server yet — wait and retry
+                    eprintln!(
+                        "  No server found for project {} (attempt {}/{}), retrying in {}s...",
+                        project_id,
+                        attempt + 1,
+                        max_retries,
+                        retry_delay.as_secs(),
+                    );
+                    tokio::time::sleep(retry_delay).await;
+                }
+                None => {
+                    return Ok(None);
+                }
+            }
+        }
 
-        Ok(server)
+        Ok(None)
     })
 }
 

src/forms/project/volume.rs

@@ -27,11 +27,23 @@ impl TryInto<dctypes::AdvancedVolumes> for &Volume {
     type Error = String;
     fn try_into(self) -> Result<dctypes::AdvancedVolumes, Self::Error> {
         let source = self.host_path.clone();
-        let target = self.container_path.clone();
+        let raw_target = self.container_path.clone().unwrap_or_default();
+
+        // Strip `:ro` / `:rw` suffix from container_path and extract read_only flag.
+        // Data may arrive with the mode embedded (e.g. "/var/run/docker.sock:ro").
+        let (target, read_only) = if raw_target.ends_with(":ro") {
+            (raw_target.trim_end_matches(":ro").to_string(), true)
+        } else if raw_target.ends_with(":rw") {
+            (raw_target.trim_end_matches(":rw").to_string(), false)
+        } else {
+            (raw_target, false)
+        };
+
         tracing::debug!(
-            "Volume conversion result: source: {:?} target: {:?}",
+            "Volume conversion result: source: {:?} target: {:?} read_only: {}",
             source,
-            target
+            target,
+            read_only
         );
 
         let _type = if self.is_named_docker_volume() {
@@ -42,9 +54,9 @@ impl TryInto<dctypes::AdvancedVolumes> for &Volume {
 
         Ok(dctypes::AdvancedVolumes {
             source: source,
-            target: target.unwrap_or("".to_string()),
+            target: target,
             _type: _type.to_string(),
-            read_only: false,
+            read_only,
             bind: None,
             volume: None,
             tmpfs: None,