fix(oabctl): wait for service drain on delete, retry on create (#1305)

* fix(oabctl): wait for service drain on delete, retry on create

delete: after force-deleting the ECS service, poll until it reaches
INACTIVE status (up to 60s) so a subsequent apply doesn't race
against the draining service.

apply: if create_service hits 'still Draining', retry with 5s backoff
(up to 12 attempts / 60s) instead of failing immediately.

* fix(review): address PR review findings

- delete.rs: treat DescribeServices errors as retry (not gone)
- delete.rs: increase polling interval from 1s to 5s (consistent with wait_for_stable)
- delete.rs: standardize progress output to stderr
- apply.rs: fix dead code — restructure loop so timeout path is reachable
- apply.rs: fix stdout/stderr mixing in retry progress output

* fix(review): use typed error matching, improve polling UX

- apply.rs: Replace fragile string matching on 'still Draining' with
  typed error detection via ProvideErrorMetadata (code == InvalidParameterException
  + case-insensitive 'draining' in message). Consistent with ingress.rs pattern.
- apply.rs: Add progress dots during retry sleep for better UX.
- delete.rs: Reorder poll loop to check-before-sleep, avoiding unnecessary
  initial 5s delay when service transitions quickly.
- delete.rs: Add progress dots during polling iterations.

Addresses review findings from PR group review.

---------

Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>

Author: chaodu-agent

operator/src/apply.rs

@@ -524,10 +524,50 @@ async fn apply_ecs(
             create_req = create_req.service_registries(registry.build());
         }
 
-        create_req
-            .send()
-            .await
-            .context("failed to create ECS service")?;
+        // Retry with backoff if ECS reports "still Draining" (race with a
+        // recent delete that hasn't fully completed yet).
+        // Match on the typed error code (InvalidParameterException) rather than
+        // raw message text to be resilient to SDK/API wording changes.
+        use aws_sdk_ecs::error::ProvideErrorMetadata;
+        let mut last_err = None;
+        for attempt in 0..12 {
+            match create_req.clone().send().await {
+                Ok(_) => {
+                    if attempt > 0 {
+                        eprintln!(" ok");
+                    }
+                    last_err = None;
+                    break;
+                }
+                Err(e) => {
+                    let is_draining = e.code() == Some("InvalidParameterException")
+                        && e.message()
+                            .unwrap_or_default()
+                            .to_lowercase()
+                            .contains("draining");
+                    if is_draining {
+                        if attempt == 0 {
+                            eprint!("  ⏳ Service still draining, retrying...");
+                        } else {
+                            eprint!(".");
+                        }
+                        last_err = Some(e);
+                        if attempt < 11 {
+                            tokio::time::sleep(std::time::Duration::from_secs(5)).await;
+                        }
+                    } else {
+                        if attempt > 0 {
+                            eprintln!(" failed");
+                        }
+                        return Err(e).context("failed to create ECS service");
+                    }
+                }
+            }
+        }
+        if let Some(e) = last_err {
+            eprintln!(" timed out");
+            return Err(anyhow::anyhow!(e)).context("failed to create ECS service after retries");
+        }
         println!(
             "  ✓ {} created ({}, {}cpu/{}mem{})",
             m.metadata.name,

operator/src/delete.rs

@@ -88,6 +88,47 @@ pub async fn run(
         .context("failed to delete ECS service")?;
     println!("  ✓ ECS service deleted");
 
+    // 2a. Wait for the service to fully drain (INACTIVE status) so that
+    // a subsequent `apply` doesn't hit "Unable to Start a service that
+    // is still Draining".
+    eprint!("  ⏳ Waiting for drain to complete...");
+    for i in 0..12 {
+        // Check status first, then sleep — avoids an unnecessary initial delay
+        // when the service transitions quickly.
+        let resp = ecs
+            .describe_services()
+            .cluster(cluster)
+            .services(&service_name)
+            .send()
+            .await;
+        let is_gone = match resp {
+            Ok(r) => r
+                .services()
+                .first()
+                .map(|s| s.status() == Some("INACTIVE"))
+                .unwrap_or(true),
+            Err(e) => {
+                eprintln!("\n  ⚠ describe_services error (retrying): {e}");
+                false
+            }
+        };
+        if is_gone {
+            if i == 0 {
+                eprintln!(" done (immediate)");
+            } else {
+                let elapsed = i * 5;
+                eprintln!(" done ({elapsed}s)");
+            }
+            break;
+        }
+        if i == 11 {
+            eprintln!(" timed out (service may still be draining)");
+        } else {
+            eprint!(".");
+            tokio::time::sleep(std::time::Duration::from_secs(5)).await;
+        }
+    }
+
     // 2b. Best-effort ingress teardown: Cloud Map service + this API's
     // routes/integration/stage. No-op for bots that never had ingress. Never
     // blocks deletion — failures are logged only.