Skip to content

Commit 90c0748

Browse files
authored
Optimize terminating Verda instances (#3811)
- Replace multiple `DELETE /sshkeys/:id` API calls with a single `DELETE /sshkeys` call. - Drop phantom error handling for missing SSH keys or startup scripts. The Verda API does not return any errors if a key or a script is missing.
1 parent 4c6dac1 commit 90c0748

2 files changed

Lines changed: 7 additions & 178 deletions

File tree

src/dstack/_internal/core/backends/verda/compute.py

Lines changed: 2 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -261,65 +261,13 @@ def _create_startup_script(client: VerdaClient, name: str, script: str) -> str:
261261
def _delete_startup_script(client: VerdaClient, startup_script_id: Optional[str]) -> None:
262262
if startup_script_id is None:
263263
return
264-
try:
265-
client.startup_scripts.delete_by_id(startup_script_id)
266-
except APIException as e:
267-
if _is_startup_script_not_found_error(e):
268-
logger.debug(
269-
"Skipping startup script %s deletion. Startup script not found.",
270-
startup_script_id,
271-
)
272-
return
273-
raise
264+
client.startup_scripts.delete_by_id(startup_script_id)
274265

275266

276267
def _delete_ssh_keys(client: VerdaClient, ssh_key_ids: Optional[List[str]]) -> None:
277268
if not ssh_key_ids:
278269
return
279-
for ssh_key_id in ssh_key_ids:
280-
_delete_ssh_key(client, ssh_key_id)
281-
282-
283-
def _delete_ssh_key(client: VerdaClient, ssh_key_id: str) -> None:
284-
try:
285-
client.ssh_keys.delete_by_id(ssh_key_id)
286-
except APIException as e:
287-
if _is_ssh_key_not_found_error(e):
288-
logger.debug("Skipping ssh key %s deletion. SSH key not found.", ssh_key_id)
289-
return
290-
raise
291-
292-
293-
def _is_ssh_key_not_found_error(error: APIException) -> bool:
294-
code = (error.code or "").lower()
295-
message = (error.message or "").lower()
296-
if code == "not_found":
297-
return True
298-
if code not in {"", "invalid_request"}:
299-
return False
300-
return (
301-
message == "invalid ssh-key id"
302-
or message == "invalid ssh key id"
303-
or message == "not found"
304-
or ("ssh-key id" in message and "invalid" in message)
305-
or ("ssh key id" in message and "invalid" in message)
306-
)
307-
308-
309-
def _is_startup_script_not_found_error(error: APIException) -> bool:
310-
code = (error.code or "").lower()
311-
message = (error.message or "").lower()
312-
if code == "not_found":
313-
return True
314-
if code not in {"", "invalid_request"}:
315-
return False
316-
return (
317-
message == "invalid startup script id"
318-
or message == "invalid script id"
319-
or message == "not found"
320-
or ("startup script id" in message and "invalid" in message)
321-
or ("script id" in message and "invalid" in message)
322-
)
270+
client.ssh_keys.delete(ssh_key_ids)
323271

324272

325273
def _get_instance_by_id(

src/tests/_internal/core/backends/verda/test_compute.py

Lines changed: 5 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99
VerdaInstanceBackendData,
1010
_create_ssh_key,
1111
_create_startup_script,
12-
_is_ssh_key_not_found_error,
13-
_is_startup_script_not_found_error,
1412
)
1513
from dstack._internal.core.errors import BackendError, NoCapacityError
1614

@@ -297,7 +295,7 @@ def test_terminate_instance_without_backend_data(self):
297295

298296
_assert_terminate_call(compute.client.instances.action)
299297
compute.client.startup_scripts.delete_by_id.assert_not_called()
300-
compute.client.ssh_keys.delete_by_id.assert_not_called()
298+
compute.client.ssh_keys.delete.assert_not_called()
301299

302300
def test_terminate_instance_deletes_startup_script(self):
303301
compute = VerdaCompute.__new__(VerdaCompute)
@@ -311,7 +309,7 @@ def test_terminate_instance_deletes_startup_script(self):
311309

312310
_assert_terminate_call(compute.client.instances.action)
313311
compute.client.startup_scripts.delete_by_id.assert_called_once_with("script-id")
314-
assert compute.client.ssh_keys.delete_by_id.call_count == 2
312+
compute.client.ssh_keys.delete.assert_called_once_with(["ssh-key-id-1", "ssh-key-id-2"])
315313

316314
def test_terminate_instance_still_deletes_script_when_instance_is_missing(self):
317315
compute = VerdaCompute.__new__(VerdaCompute)
@@ -325,41 +323,7 @@ def test_terminate_instance_still_deletes_script_when_instance_is_missing(self):
325323
compute.terminate_instance("instance-id", "FIN-01", backend_data)
326324

327325
compute.client.startup_scripts.delete_by_id.assert_called_once_with("script-id")
328-
compute.client.ssh_keys.delete_by_id.assert_called_once_with("ssh-key-id-1")
329-
330-
def test_terminate_instance_ignores_missing_startup_script(self):
331-
compute = VerdaCompute.__new__(VerdaCompute)
332-
compute.client = MagicMock()
333-
compute.client.startup_scripts.delete_by_id.side_effect = APIException(
334-
"",
335-
"Invalid startup script id",
336-
)
337-
backend_data = VerdaInstanceBackendData(
338-
startup_script_id="script-id",
339-
ssh_key_ids=["ssh-key-id-1"],
340-
).json()
341-
342-
compute.terminate_instance("instance-id", "FIN-01", backend_data)
343-
344-
_assert_terminate_call(compute.client.instances.action)
345-
compute.client.ssh_keys.delete_by_id.assert_called_once_with("ssh-key-id-1")
346-
347-
def test_terminate_instance_ignores_missing_startup_script_invalid_script_id(self):
348-
compute = VerdaCompute.__new__(VerdaCompute)
349-
compute.client = MagicMock()
350-
compute.client.startup_scripts.delete_by_id.side_effect = APIException(
351-
"invalid_request",
352-
"Invalid script ID",
353-
)
354-
backend_data = VerdaInstanceBackendData(
355-
startup_script_id="script-id",
356-
ssh_key_ids=["ssh-key-id-1"],
357-
).json()
358-
359-
compute.terminate_instance("instance-id", "FIN-01", backend_data)
360-
361-
_assert_terminate_call(compute.client.instances.action)
362-
compute.client.ssh_keys.delete_by_id.assert_called_once_with("ssh-key-id-1")
326+
compute.client.ssh_keys.delete.assert_called_once_with(["ssh-key-id-1"])
363327

364328
def test_terminate_instance_retries_on_script_delete_error(self):
365329
compute = VerdaCompute.__new__(VerdaCompute)
@@ -375,99 +339,16 @@ def test_terminate_instance_retries_on_script_delete_error(self):
375339
with pytest.raises(APIException):
376340
compute.terminate_instance("instance-id", "FIN-01", backend_data)
377341

378-
compute.client.ssh_keys.delete_by_id.assert_not_called()
379-
380-
def test_terminate_instance_ignores_missing_ssh_key(self):
381-
compute = VerdaCompute.__new__(VerdaCompute)
382-
compute.client = MagicMock()
383-
compute.client.ssh_keys.delete_by_id.side_effect = APIException(
384-
"invalid_request",
385-
"Invalid ssh-key ID",
386-
)
387-
backend_data = VerdaInstanceBackendData(
388-
startup_script_id="script-id",
389-
ssh_key_ids=["ssh-key-id-1"],
390-
).json()
391-
392-
compute.terminate_instance("instance-id", "FIN-01", backend_data)
393-
394-
_assert_terminate_call(compute.client.instances.action)
395-
compute.client.startup_scripts.delete_by_id.assert_called_once_with("script-id")
396-
compute.client.ssh_keys.delete_by_id.assert_called_once_with("ssh-key-id-1")
397-
398-
def test_terminate_instance_deletes_remaining_ssh_keys_when_one_missing(self):
399-
compute = VerdaCompute.__new__(VerdaCompute)
400-
compute.client = MagicMock()
401-
compute.client.ssh_keys.delete_by_id.side_effect = [
402-
APIException("invalid_request", "Invalid ssh-key ID"),
403-
None,
404-
]
405-
backend_data = VerdaInstanceBackendData(
406-
startup_script_id="script-id",
407-
ssh_key_ids=["ssh-key-id-1", "ssh-key-id-2"],
408-
).json()
409-
410-
compute.terminate_instance("instance-id", "FIN-01", backend_data)
411-
412-
compute.client.startup_scripts.delete_by_id.assert_called_once_with("script-id")
413-
compute.client.ssh_keys.delete_by_id.assert_any_call("ssh-key-id-1")
414-
compute.client.ssh_keys.delete_by_id.assert_any_call("ssh-key-id-2")
415-
assert compute.client.ssh_keys.delete_by_id.call_count == 2
342+
compute.client.ssh_keys.delete.assert_not_called()
416343

417344
def test_terminate_instance_retries_on_ssh_key_delete_error(self):
418345
compute = VerdaCompute.__new__(VerdaCompute)
419346
compute.client = MagicMock()
420-
compute.client.ssh_keys.delete_by_id.side_effect = APIException("", "Random API error")
347+
compute.client.ssh_keys.delete.side_effect = APIException("", "Random API error")
421348
backend_data = VerdaInstanceBackendData(
422349
startup_script_id="script-id",
423350
ssh_key_ids=["ssh-key-id-1"],
424351
).json()
425352

426353
with pytest.raises(APIException):
427354
compute.terminate_instance("instance-id", "FIN-01", backend_data)
428-
429-
430-
class TestIsStartupScriptNotFoundError:
431-
def test_returns_true_for_not_found_code_even_with_custom_message(self):
432-
assert _is_startup_script_not_found_error(
433-
APIException("not_found", "Startup script does not exist anymore")
434-
)
435-
436-
def test_returns_true_for_invalid_script_id(self):
437-
assert _is_startup_script_not_found_error(
438-
APIException("invalid_request", "Invalid script ID")
439-
)
440-
441-
def test_returns_true_for_not_found(self):
442-
assert _is_startup_script_not_found_error(APIException("not_found", "Not Found"))
443-
444-
def test_returns_false_for_unrelated_error(self):
445-
assert not _is_startup_script_not_found_error(
446-
APIException("forbidden", "Permission denied")
447-
)
448-
449-
def test_returns_false_for_unrelated_invalid_request(self):
450-
assert not _is_startup_script_not_found_error(
451-
APIException("invalid_request", "Some other invalid request")
452-
)
453-
454-
455-
class TestIsSSHKeyNotFoundError:
456-
def test_returns_true_for_not_found_code_even_with_custom_message(self):
457-
assert _is_ssh_key_not_found_error(
458-
APIException("not_found", "SSH key does not exist anymore")
459-
)
460-
461-
def test_returns_true_for_invalid_ssh_key_id(self):
462-
assert _is_ssh_key_not_found_error(APIException("invalid_request", "Invalid ssh-key ID"))
463-
464-
def test_returns_true_for_not_found(self):
465-
assert _is_ssh_key_not_found_error(APIException("not_found", "Not Found"))
466-
467-
def test_returns_false_for_unrelated_error(self):
468-
assert not _is_ssh_key_not_found_error(APIException("forbidden", "Permission denied"))
469-
470-
def test_returns_false_for_unrelated_invalid_request(self):
471-
assert not _is_ssh_key_not_found_error(
472-
APIException("invalid_request", "Some other invalid request")
473-
)

0 commit comments

Comments
 (0)