Explorar el Código

stop: explicitly rm labeled stopped containers + 1s poll until no running labeled containers — closes the post-stop 'occupied' race that failed back-to-back serves

Alexander Korolev hace 3 semanas
padre
commit
a5b75d3014
Se han modificado 1 ficheros con 22 adiciones y 16 borrados
  1. 22 16
      src/docker/stop.rs

+ 22 - 16
src/docker/stop.rs

@@ -13,10 +13,13 @@ use tracing::info;
 
 /// Builds the per-node remote stop command.
 ///
-/// `wait = true` (default): after `docker stop`, poll every 2 s (30 times,
-/// 60 s max) until `docker ps` shows no container with the `sm121.recipe` /
-/// `sm121.cluster` labels in the running state; if any remain, exit 1 —
-/// `run_ssh` surfaces that as an error.
+/// `wait = true` (default): after `docker stop`, explicitly remove any
+/// stopped-but-still-present labeled containers (there is no `--rm`, so
+/// an Exited container lingers), then poll every 1 s (60 s max) until
+/// `docker ps` shows no labeled container running; if any remain, exit 1 —
+/// `run_ssh` surfaces that as an error. Without the explicit removal and
+/// the running-only polling, a graceful-shutdown straggler could keep the
+/// node reported as `occupied` for the immediately following serve.
 pub fn remote_command(recipe_name: &str, cluster_name: &str, rank: u32, wait: bool) -> String {
     let filter = format!(
         "--filter {} --filter {}",
@@ -24,16 +27,18 @@ pub fn remote_command(recipe_name: &str, cluster_name: &str, rank: u32, wait: bo
         shq(&format!("label=sm121.cluster={}", cluster_name)),
     );
     let wait_block = if wait {
-        "for _ in $(seq 1 30); do \
-            n=$(docker ps -q {filter}); \
-            [ -z \"$n\" ] && break; \
-            sleep 2; \
-        done; \
-        n=$(docker ps -q {filter}); \
-        if [ -n \"$n\" ]; then \
-            echo \"sm121 stop: containers still running after 60 s: $n\" >&2; \
-            exit 1; \
-        fi; "
+        "\
+docker ps -aq {filter} | xargs -r docker rm >/dev/null 2>&1; \
+for _ in $(seq 1 60); do \
+    n=$(docker ps -q {filter}); \
+    [ -z \"$n\" ] && break; \
+    sleep 1; \
+done; \
+n=$(docker ps -q {filter}); \
+if [ -n \"$n\" ]; then \
+    echo \"sm121 stop: containers still running after 60 s: $n\" >&2; \
+    exit 1; \
+fi; "
             .to_string()
     } else {
         String::new()
@@ -199,7 +204,8 @@ mod tests {
     fn default_includes_verification_loop() {
         let cmd = remote_command("r", "c", 0, true);
         assert!(cmd.contains("xargs -r docker stop"));
-        assert!(cmd.contains("seq 1 30"));
+        assert!(cmd.contains("xargs -r docker rm"));
+        assert!(cmd.contains("seq 1 60"));
         assert!(cmd.contains("exit 1"));
         assert!(cmd.ends_with("true"));
     }
@@ -207,7 +213,7 @@ mod tests {
     #[test]
     fn no_wait_omits_verification_loop() {
         let cmd = remote_command("r", "c", 1, false);
-        assert!(!cmd.contains("seq 1 30"));
+        assert!(!cmd.contains("seq 1 60"));
         assert!(!cmd.contains("exit 1"));
         assert!(cmd.contains("/tmp/sm121_r_1"));
     }