Skip to content

Commit 5bb7e6f

Browse files
authored
Fix SSHAttach.detach() (#3306)
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 5bb7e6f

File tree

2 files changed

+20
-0
lines changed

2 files changed

+20
-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: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,11 @@ async def aopen(self) -> None:
204204
raise get_ssh_error(stderr)
205205

206206
def close(self) -> None:
207+
if not os.path.exists(self.control_sock_path):
208+
logger.debug(
209+
"Control socket does not exist, it seems that ssh process has already exited"
210+
)
211+
return
207212
proc = subprocess.run(
208213
self.close_command(), stdout=subprocess.PIPE, stderr=subprocess.STDOUT
209214
)
@@ -215,6 +220,11 @@ def close(self) -> None:
215220
)
216221

217222
async def aclose(self) -> None:
223+
if not os.path.exists(self.control_sock_path):
224+
logger.debug(
225+
"Control socket does not exist, it seems that ssh process has already exited"
226+
)
227+
return
218228
proc = await asyncio.create_subprocess_exec(
219229
*self.close_command(), stdout=subprocess.PIPE, stderr=subprocess.STDOUT
220230
)

0 commit comments

Comments
 (0)