Skip to content

Commit 10934da

Browse files
committed
Fix SSHAttach.detach()
When error logging was added to SSHTunnel.close()[1], it was discovered that SSHAttach.detach() is called multiple times on detach. Although this is harmless as long as this method is idempotent (and it is), this patch ensures that detach() is no-op when called more than one time, removing ERROR level log noise. As a side-effect, detaching SSHAttach that reuses SSHTunnel no longer kills "master" SSHAttach, meaning that one can `dstack attach` multiple times and detach independently (detaching from the "master" attach still kicks out all other attaches). [1]: #3296
1 parent 1c07f64 commit 10934da

File tree

2 files changed

+15
-0
lines changed

2 files changed

+15
-0
lines changed

src/dstack/_internal/core/services/ssh/attach.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from dstack._internal.core.services.ssh.client import get_ssh_client_info
1313
from dstack._internal.core.services.ssh.ports import PortsLock
1414
from dstack._internal.core.services.ssh.tunnel import SSHTunnel, ports_to_forwarded_sockets
15+
from dstack._internal.utils.logging import get_logger
1516
from dstack._internal.utils.path import FilePath, PathLike
1617
from dstack._internal.utils.ssh import (
1718
default_ssh_config_path,
@@ -21,6 +22,8 @@
2122
update_ssh_config,
2223
)
2324

25+
logger = get_logger(__name__)
26+
2427
# ssh -L option format: [bind_address:]port:host:hostport
2528
_SSH_TUNNEL_REGEX = re.compile(r"(?:[\w.-]+:)?(?P<local_port>\d+):localhost:(?P<remote_port>\d+)")
2629

@@ -68,6 +71,7 @@ def __init__(
6871
local_backend: bool = False,
6972
bind_address: Optional[str] = None,
7073
):
74+
self._attached = False
7175
self._ports_lock = ports_lock
7276
self.ports = ports_lock.dict()
7377
self.run_name = run_name
@@ -209,6 +213,7 @@ def attach(self):
209213
for i in range(max_retries):
210214
try:
211215
self.tunnel.open()
216+
self._attached = True
212217
atexit.register(self.detach)
213218
break
214219
except SSHError:
@@ -219,9 +224,14 @@ def attach(self):
219224
raise SSHError("Can't connect to the remote host")
220225

221226
def detach(self):
227+
if not self._attached:
228+
logger.debug("Not attached")
229+
return
222230
self.tunnel.close()
223231
for host in self.hosts:
224232
update_ssh_config(self.ssh_config_path, host, {})
233+
self._attached = False
234+
logger.debug("Detached")
225235

226236
def __enter__(self):
227237
self.attach()

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,11 @@ def open(self) -> None:
174174
stdout=subprocess.DEVNULL,
175175
stderr=subprocess.DEVNULL,
176176
timeout=SSH_TIMEOUT,
177+
# We don't want the ssh process to receive SIGINT from the controlling terminal
178+
# (e.g., when SSHTunnel is used via CLI->Python API's SSHAttach and the dstack CLI
179+
# process is a foreground process group leader). Starting a new session ensures
180+
# that we don't have the controlling terminal at all.
181+
start_new_session=True,
177182
)
178183
except subprocess.TimeoutExpired as e:
179184
msg = f"SSH tunnel to {self.destination} did not open in {SSH_TIMEOUT} seconds"

0 commit comments

Comments
 (0)