Skip to content

Commit 48213bd

Browse files
committed
fix(cli): retry sync during supervisor reconnect
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
1 parent e7c88cb commit 48213bd

3 files changed

Lines changed: 55 additions & 3 deletions

File tree

‎architecture/gateway.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,9 @@ stays correct throughout because a non-owner relays to the owner.
423423
File upload and download use tar-over-SSH through the same relay path. A gateway
424424
pod termination drops the active SSH proxy byte stream, so the CLI retries the
425425
whole sync operation with a fresh SSH session instead of attempting mid-stream
426-
resume.
426+
resume. After a transport interruption, the bounded retry also tolerates a
427+
brief sandbox-not-ready response while the supervisor reconnects. An initial
428+
sandbox-not-ready response still fails immediately.
427429

428430
Gateway peer RPCs authenticate with Kubernetes ServiceAccount identity rather
429431
than a shared secret. Helm mounts a projected, pod-bound token with audience

‎crates/openshell-cli/src/ssh.rs‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ const FORWARD_LISTENER_CONNECT_TIMEOUT: Duration = Duration::from_millis(200);
4444
/// command has already reported its terminal result.
4545
const TERMINAL_RELAY_REGISTRATION_TIMEOUT: Duration = Duration::from_secs(5);
4646
const TERMINAL_RELAY_REGISTRATION_INTERVAL: Duration = Duration::from_millis(50);
47-
const SYNC_RETRY_ATTEMPTS: usize = 4;
47+
const SYNC_RETRY_ATTEMPTS: usize = 8;
4848
const SYNC_RETRY_DELAY: Duration = Duration::from_secs(2);
4949

5050
#[derive(Clone, Copy, Debug)]
@@ -1288,10 +1288,15 @@ where
12881288
Fut: Future<Output = Result<()>>,
12891289
{
12901290
let mut attempt = 1;
1291+
let mut saw_transport_failure = false;
12911292
loop {
12921293
match run().await {
12931294
Ok(()) => return Ok(()),
1294-
Err(error) if attempt < SYNC_RETRY_ATTEMPTS && sync_error_is_retryable(&error) => {
1295+
Err(error)
1296+
if attempt < SYNC_RETRY_ATTEMPTS
1297+
&& sync_error_is_retryable_after_disruption(&error, saw_transport_failure) =>
1298+
{
1299+
saw_transport_failure |= sync_error_is_retryable(&error);
12951300
tracing::warn!(
12961301
operation,
12971302
attempt,
@@ -1307,6 +1312,16 @@ where
13071312
}
13081313
}
13091314

1315+
fn sync_error_is_not_ready(error: &Report) -> bool {
1316+
format!("{error:?}")
1317+
.to_ascii_lowercase()
1318+
.contains("sandbox is not ready")
1319+
}
1320+
1321+
fn sync_error_is_retryable_after_disruption(error: &Report, saw_transport_failure: bool) -> bool {
1322+
sync_error_is_retryable(error) || (saw_transport_failure && sync_error_is_not_ready(error))
1323+
}
1324+
13101325
fn sync_error_is_retryable(error: &Report) -> bool {
13111326
let message = format!("{error:?}").to_ascii_lowercase();
13121327
[
@@ -1868,6 +1883,36 @@ mod tests {
18681883
assert!(!sync_error_is_retryable(&error));
18691884
}
18701885

1886+
#[test]
1887+
fn sync_error_retry_filter_requires_prior_transport_failure_for_readiness() {
1888+
let error = miette::miette!("sandbox is not ready");
1889+
assert!(!sync_error_is_retryable_after_disruption(&error, false));
1890+
assert!(sync_error_is_retryable_after_disruption(&error, true));
1891+
let validation_error = miette::miette!("sandbox source path is invalid");
1892+
assert!(!sync_error_is_retryable_after_disruption(
1893+
&validation_error,
1894+
true
1895+
));
1896+
}
1897+
1898+
#[tokio::test]
1899+
async fn sync_retries_not_ready_after_transport_break() {
1900+
let mut attempts = 0;
1901+
retry_sandbox_sync("download", || {
1902+
attempts += 1;
1903+
async move {
1904+
match attempts {
1905+
1 => Err(miette::miette!("connection reset by peer")),
1906+
2 => Err(miette::miette!("sandbox is not ready")),
1907+
_ => Ok(()),
1908+
}
1909+
}
1910+
})
1911+
.await
1912+
.expect("sync should recover after supervisor readiness returns");
1913+
assert_eq!(attempts, 3);
1914+
}
1915+
18711916
#[test]
18721917
#[allow(unsafe_code)] // Test-only: env vars require unsafe in Rust 2024.
18731918
fn install_ssh_config_adds_include_once_and_updates_managed_file() {

‎docs/sandboxes/manage-sandboxes.mdx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,6 +764,11 @@ Download files from the sandbox to your host:
764764
openshell sandbox download my-sandbox output ./local
765765
```
766766
767+
If a gateway connection drops during a transfer, the CLI retries the whole
768+
upload or download with a fresh sandbox connection. It can also wait briefly
769+
for the sandbox to become ready again after a gateway restart. A sandbox that
770+
is already stopped still returns an error immediately.
771+
767772
When the sandbox-side source is a single file, the destination follows `cp`-style placement: if the destination already exists as a directory or ends with `/`, the file lands inside it as `<dest>/<basename>`; otherwise the file is written at the exact destination path.
768773
769774
The CLI discovers the sandbox's canonical working directory and only allows

0 commit comments

Comments
 (0)