Skip to content

Commit a6bf681

Browse files
author
Andrey Cheptsov
committed
Reconnect gateway replica SSH tunnels after process exit
1 parent 9ebdd3f commit a6bf681

7 files changed

Lines changed: 1207 additions & 19 deletions

File tree

‎pyproject.toml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,8 @@ gateway = [
217217
"fastapi",
218218
"starlette>=0.26.0",
219219
"uvicorn",
220+
# Supervise long-lived SSH children without a waiting thread for each replica.
221+
"uvloop>=0.18.0; sys_platform != 'win32' and platform_python_implementation != 'PyPy'",
220222
"aiorwlock",
221223
"aiocache",
222224
"httpx>=0.28.0",

‎src/dstack/_internal/core/services/ssh/tunnel.py‎

Lines changed: 102 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22
import asyncio
33
import os
44
import shlex
5+
import signal
56
import subprocess
67
import tempfile
78
from dataclasses import dataclass
89
from typing import Dict, Iterable, List, Literal, NoReturn, Optional, Union
910

11+
from dstack._internal.compat import IS_WINDOWS
1012
from dstack._internal.core.errors import SSHError
1113
from dstack._internal.core.models.instances import SSHConnectionParams
1214
from dstack._internal.core.services.ssh import get_ssh_error
@@ -71,6 +73,7 @@ def __init__(
7173
port: Optional[int] = None,
7274
ssh_proxies: Iterable[tuple[SSHConnectionParams, Optional[FilePathOrContent]]] = (),
7375
batch_mode: bool = False,
76+
background: bool = True,
7477
):
7578
"""
7679
:param forwarded_sockets: Connections to the specified local sockets will be
@@ -91,6 +94,10 @@ def __init__(
9194
Control commands (`check`, `close`, `exec`) always run in batch mode, since they
9295
only talk to the local master and must not prompt if ssh falls back to a direct
9396
connection.
97+
:param background: If False, own a foreground SSH process instead of a daemon.
98+
Use only the async methods and a dedicated control socket. `aopen()` waits
99+
for startup readiness; `wait_closed()` waits for exit without polling;
100+
`aclose()` cleans up the process and its ProxyCommand children.
94101
"""
95102
self.destination = destination
96103
self.forwarded_sockets = list(forwarded_sockets)
@@ -114,6 +121,8 @@ def __init__(
114121
)
115122
self.ssh_proxies.append((proxy_params, proxy_identity_path))
116123
self.batch_mode = batch_mode
124+
self.background = background
125+
self._process: Optional[asyncio.subprocess.Process] = None
117126
self.log_path = normalize_path(os.path.join(temp_dir.name, "tunnel.log"))
118127
self.ssh_client_info = get_ssh_client_info()
119128
self.ssh_exec_path = str(self.ssh_client_info.path)
@@ -136,19 +145,21 @@ def open_command(self) -> List[str]:
136145
self.log_path,
137146
"-N", # do not run commands on remote
138147
]
139-
if self.ssh_client_info.supports_background_mode:
148+
if self.background:
149+
if not self.ssh_client_info.supports_background_mode:
150+
raise SSHError("Unsupported SSH client")
140151
command += ["-f"] # go to background after successful authentication
141152
else:
142-
raise SSHError("Unsupported SSH client")
153+
command += ["-o", "ForkAfterAuthentication=no", "-o", "ControlPersist=no"]
143154
if self.ssh_client_info.supports_control_socket:
144155
# It's safe to use ControlMaster even if the ssh client does not support multiplexing
145156
# as long as we don't allow more than one tunnel to the specific host to be running.
146157
# We use this feature for control only (see :meth:`close_command`).
147158
command += [
148-
# Not `-M`, which means `ControlMaster=yes`, to avoid spawning uncontrollable
149-
# ssh instances if more than one tunnel is started (precaution).
159+
# Background connections may reuse a master. Foreground connections must
160+
# own their process, rather than attach to another master and exit.
150161
"-o",
151-
"ControlMaster=auto",
162+
"ControlMaster=auto" if self.background else "ControlMaster=yes",
152163
"-S",
153164
self.control_sock_path,
154165
]
@@ -185,6 +196,8 @@ def exec_command(self) -> List[str]:
185196
return [*self._control_command_prefix(), self.destination]
186197

187198
def open(self) -> None:
199+
if not self.background:
200+
raise SSHError("Foreground SSH tunnels require aopen()")
188201
# We cannot use `stderr=subprocess.PIPE` here since the forked process (daemon) does not
189202
# close standard streams if ProxyJump is used, therefore we will wait EOF from the pipe
190203
# as long as the daemon exists.
@@ -206,6 +219,9 @@ def open(self) -> None:
206219
self._raise_ssh_error_from_log_output(log_output)
207220

208221
async def aopen(self) -> None:
222+
if not self.background:
223+
await self._aopen_foreground()
224+
return
209225
await run_async(self._remove_log_file)
210226
proc = await asyncio.create_subprocess_exec(
211227
*self.open_command(), stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL
@@ -222,6 +238,12 @@ async def aopen(self) -> None:
222238
log_output = await run_async(self._read_log_file)
223239
self._raise_ssh_error_from_log_output(log_output)
224240

241+
async def wait_closed(self) -> int:
242+
"""Wait for the foreground process to exit; retain it for `aclose()` cleanup."""
243+
if self._process is None:
244+
raise SSHError("No foreground SSH process to wait for")
245+
return await self._process.wait()
246+
225247
def close(self) -> None:
226248
if not os.path.exists(self.control_sock_path):
227249
logger.debug(
@@ -247,6 +269,16 @@ def close(self) -> None:
247269
)
248270

249271
async def aclose(self) -> None:
272+
if not self.background:
273+
if self._process is None:
274+
return
275+
cleanup = asyncio.create_task(self._close_foreground(self._process))
276+
try:
277+
await asyncio.shield(cleanup)
278+
except asyncio.CancelledError:
279+
await cleanup
280+
raise
281+
return
250282
if not os.path.exists(self.control_sock_path):
251283
logger.debug(
252284
"Control socket does not exist, it seems that ssh process has already exited"
@@ -330,6 +362,64 @@ def _control_command_prefix(self) -> List[str]:
330362
self.control_sock_path,
331363
]
332364

365+
async def _aopen_foreground(self) -> None:
366+
if self._process is not None:
367+
raise SSHError("Close the previous foreground SSH process before opening")
368+
await run_async(self._remove_log_file)
369+
creation = asyncio.create_task(
370+
asyncio.create_subprocess_exec(
371+
*self.open_command(),
372+
stdout=subprocess.DEVNULL,
373+
stderr=subprocess.DEVNULL,
374+
start_new_session=not IS_WINDOWS,
375+
)
376+
)
377+
try:
378+
# Retain the process handle even if cancellation arrives during its creation.
379+
proc = await asyncio.shield(creation)
380+
self._process = proc
381+
await asyncio.wait_for(self._wait_until_ready(proc), SSH_TIMEOUT)
382+
except BaseException as e:
383+
if self._process is None:
384+
try:
385+
self._process = await creation
386+
except Exception:
387+
# Preserve cancellation if creation also failed.
388+
pass
389+
await self.aclose()
390+
if isinstance(e, asyncio.TimeoutError):
391+
raise SSHError(
392+
f"SSH tunnel to {self.destination} did not open in {SSH_TIMEOUT} seconds"
393+
) from e
394+
raise
395+
396+
async def _wait_until_ready(self, proc: asyncio.subprocess.Process) -> None:
397+
while proc.returncode is None:
398+
if os.path.exists(self.control_sock_path) and await self.acheck():
399+
if proc.returncode is None:
400+
return
401+
break
402+
await asyncio.sleep(0.1)
403+
log_output = await run_async(self._read_log_file)
404+
self._raise_ssh_error_from_log_output(log_output)
405+
406+
async def _close_foreground(self, proc: asyncio.subprocess.Process) -> None:
407+
try:
408+
if IS_WINDOWS:
409+
proc.kill()
410+
else:
411+
# The launcher may have exited while a ProxyCommand child remains alive.
412+
os.killpg(proc.pid, signal.SIGKILL) # pyright: ignore[reportAttributeAccessIssue]
413+
except ProcessLookupError:
414+
pass
415+
await proc.wait()
416+
self._process = None
417+
# SIGKILL leaves the control socket behind. This path is owned by this tunnel.
418+
try:
419+
os.remove(self.control_sock_path)
420+
except FileNotFoundError:
421+
pass
422+
333423
def _get_proxy_command(self) -> Optional[str]:
334424
proxy_command: Optional[str] = None
335425
for params, identity_path in self.ssh_proxies:
@@ -410,16 +500,19 @@ def _get_identity_path(self, identity: FilePathOrContent, tmp_filename: str) ->
410500
async def _arun(command: List[str], timeout: float) -> tuple[int, bytes, bytes]:
411501
"""
412502
Runs `command` with stdin redirected from /dev/null and returns its exit status, stdout,
413-
and stderr. Kills the process and raises `asyncio.TimeoutError` if it does not exit in
414-
`timeout` seconds.
503+
and stderr. Kills and reaps the process on cancellation or if it does not exit in
504+
`timeout` seconds, preserving the cancellation or `asyncio.TimeoutError`.
415505
"""
416506
proc = await asyncio.create_subprocess_exec(
417507
*command, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.PIPE
418508
)
419509
try:
420510
stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout)
421-
except asyncio.TimeoutError:
422-
proc.kill()
511+
except (asyncio.CancelledError, asyncio.TimeoutError):
512+
try:
513+
proc.kill()
514+
except ProcessLookupError:
515+
pass
423516
await proc.wait()
424517
raise
425518
assert proc.returncode is not None

‎src/dstack/_internal/proxy/gateway/resources/systemd/start.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ else
88
version="blue"
99
echo "$version" > "$root/version"
1010
fi
11-
"$root/$version/bin/uvicorn" dstack._internal.proxy.gateway.main:app
11+
"$root/$version/bin/uvicorn" dstack._internal.proxy.gateway.main:app --loop uvloop

‎src/dstack/_internal/proxy/lib/services/service_connection.py‎

Lines changed: 93 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424

2525
logger = get_logger(__name__)
2626
OPEN_TUNNEL_TIMEOUT = 10
27+
# Bound SSH startup work during a shared outage, without limiting service requests.
28+
MAX_CONCURRENT_TUNNEL_RECONNECTS = 8
29+
MAX_TUNNEL_RECONNECT_DELAY = 30
2730

2831

2932
class ServiceClient(httpx.AsyncClient):
@@ -33,7 +36,19 @@ def build_request(self, *args, **kwargs) -> httpx.Request:
3336

3437

3538
class ServiceConnection:
36-
def __init__(self, project: Project, service: Service, replica: Replica) -> None:
39+
"""Forward a replica's HTTP traffic over SSH to a stable local Unix socket.
40+
41+
Gateways supervise and reconnect the SSH process so Nginx can keep
42+
using the same socket path. The in-server proxy opens connections on demand.
43+
"""
44+
45+
def __init__(
46+
self,
47+
project: Project,
48+
service: Service,
49+
replica: Replica,
50+
reconnect_semaphore: asyncio.Semaphore,
51+
) -> None:
3752
self._temp_dir = TemporaryDirectory()
3853
options = {
3954
**SSH_DEFAULT_OPTIONS,
@@ -66,6 +81,7 @@ def __init__(self, project: Project, service: Service, replica: Replica) -> None
6681
),
6782
],
6883
options=options,
84+
background=service.domain is None,
6985
)
7086
self._client = ServiceClient(
7187
transport=AsyncHTTPTransport(uds=str(self._app_socket_path)),
@@ -75,29 +91,93 @@ def __init__(self, project: Project, service: Service, replica: Replica) -> None
7591
timeout=service.read_timeout,
7692
)
7793
self._is_open = asyncio.locks.Event()
94+
self._lifecycle_lock = asyncio.Lock()
95+
self._closed = False
96+
self._monitor_task: Optional[asyncio.Task] = None
97+
self._auto_reconnect = service.domain is not None
98+
self._replica_id = replica.id
99+
self._reconnect_semaphore = reconnect_semaphore
78100

79101
@property
80102
def app_socket_path(self) -> Path:
81103
return self._app_socket_path
82104

83105
async def open(self) -> None:
84-
await self._tunnel.aopen()
85-
self._is_open.set()
106+
async with self._lifecycle_lock:
107+
if self._closed:
108+
raise UnexpectedProxyError("Cannot open a closed service connection")
109+
if self._is_open.is_set():
110+
return
111+
await self._tunnel.aopen()
112+
if self._closed:
113+
# Removal may have started while SSH was connecting.
114+
raise UnexpectedProxyError("Service connection was removed while opening")
115+
self._is_open.set()
116+
if self._auto_reconnect:
117+
self._monitor_task = asyncio.create_task(self._monitor_tunnel())
86118

87119
async def close(self) -> None:
88-
self._is_open.clear()
89-
await self._client.aclose()
90-
await self._tunnel.aclose()
120+
self._closed = True
121+
# Removal must finish cleaning up even if its caller is cancelled.
122+
cleanup = asyncio.create_task(self._close())
123+
cancelled = None
124+
while not cleanup.done():
125+
try:
126+
await asyncio.shield(cleanup)
127+
except asyncio.CancelledError as e:
128+
cancelled = e
129+
cleanup.result()
130+
if cancelled is not None:
131+
raise cancelled
91132

92133
async def client(self) -> ServiceClient:
93134
await asyncio.wait_for(self._is_open.wait(), timeout=OPEN_TUNNEL_TIMEOUT)
94135
return self._client
95136

137+
async def _close(self) -> None:
138+
async with self._lifecycle_lock:
139+
if self._monitor_task is not None:
140+
self._monitor_task.cancel()
141+
await asyncio.gather(self._monitor_task, return_exceptions=True)
142+
self._monitor_task = None
143+
self._is_open.clear()
144+
try:
145+
await self._client.aclose()
146+
finally:
147+
await self._tunnel.aclose()
148+
149+
async def _monitor_tunnel(self) -> None:
150+
loop = asyncio.get_running_loop()
151+
retry_delay = 0
152+
while True:
153+
started_at = loop.time()
154+
await self._tunnel.wait_closed()
155+
if loop.time() - started_at >= MAX_TUNNEL_RECONNECT_DELAY:
156+
retry_delay = 0
157+
logger.warning("SSH tunnel to replica %s exited, reconnecting", self._replica_id)
158+
# Reap any surviving ProxyCommand children before opening a replacement.
159+
await self._tunnel.aclose()
160+
while True:
161+
if retry_delay:
162+
await asyncio.sleep(random.uniform(retry_delay / 2, retry_delay))
163+
# Back off failed starts and tunnels that repeatedly exit just after startup.
164+
retry_delay = min(max(1, retry_delay * 2), MAX_TUNNEL_RECONNECT_DELAY)
165+
try:
166+
async with self._reconnect_semaphore:
167+
# Keep the socket path configured in Nginx. SSH replaces stale sockets.
168+
await self._tunnel.aopen()
169+
except Exception as e:
170+
logger.warning("Could not reconnect to replica %s: %s", self._replica_id, e)
171+
else:
172+
logger.info("SSH tunnel to replica %s reconnected", self._replica_id)
173+
break
174+
96175

97176
class ServiceConnectionPool:
98177
def __init__(self) -> None:
99178
# TODO(#2238): remove connections to stopped replicas in-server
100179
self.connections: Dict[str, ServiceConnection] = {}
180+
self._reconnect_semaphore = asyncio.Semaphore(MAX_CONCURRENT_TUNNEL_RECONNECTS)
101181

102182
async def get(self, replica_id: str) -> Optional[ServiceConnection]:
103183
return self.connections.get(replica_id)
@@ -108,12 +188,17 @@ async def get_or_add(
108188
connection = self.connections.get(replica.id)
109189
if connection is not None:
110190
return connection
111-
connection = ServiceConnection(project, service, replica)
191+
connection = ServiceConnection(project, service, replica, self._reconnect_semaphore)
112192
self.connections[replica.id] = connection
113193
try:
114194
await connection.open()
115195
except BaseException:
116-
self.connections.pop(replica.id, None)
196+
if self.connections.get(replica.id) is connection:
197+
self.connections.pop(replica.id)
198+
try:
199+
await connection.close()
200+
except Exception:
201+
logger.exception("Error closing failed connection to replica %s", replica.id)
117202
raise
118203
return connection
119204

0 commit comments

Comments
 (0)