Skip to content

Commit 89d2098

Browse files
(PTFE-3027) Add support for block storage
1 parent 689956d commit 89d2098

2 files changed

Lines changed: 176 additions & 9 deletions

File tree

runner_manager/backend/scaleway.py

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from pydantic import Field
99
from redis_om import NotFoundError
1010
from scaleway import Client
11+
from scaleway.block.v1alpha1 import BlockV1Alpha1API
1112
from scaleway.instance.v1 import (
1213
Image,
1314
Server,
@@ -56,6 +57,26 @@ def client(self) -> InstanceUtilsV1API:
5657
)
5758
return InstanceUtilsV1API(scw_client)
5859

60+
@property
61+
def block_client(self) -> BlockV1Alpha1API:
62+
"""Returns a Scaleway Block Storage API client."""
63+
access_key = self.config.access_key or os.getenv("SCW_ACCESS_KEY")
64+
secret_key = self.config.secret_key or os.getenv("SCW_SECRET_KEY")
65+
66+
if not access_key or not secret_key:
67+
raise ValueError(
68+
"Scaleway credentials not found. Set SCW_ACCESS_KEY and SCW_SECRET_KEY."
69+
)
70+
71+
scw_client = Client(
72+
access_key=access_key,
73+
secret_key=secret_key,
74+
default_project_id=self.config.project_id,
75+
default_zone=self.config.zone,
76+
default_region=self.config.region,
77+
)
78+
return BlockV1Alpha1API(scw_client)
79+
5980
def sanitize_tags(self, tags: List[str]) -> List[str]:
6081
"""Sanitize tags to comply with Scaleway requirements.
6182
@@ -438,20 +459,41 @@ def delete(self, runner: Runner) -> int:
438459

439460
# Delete associated volumes
440461
# Note: The behavior differs by volume type:
441-
# - l_ssd (local storage): Usually auto-deleted with the server
442-
# - sbs_volume (block storage): Persists after server deletion, must be deleted manually
443-
# The Instance API manages both types, but sbs volumes need explicit cleanup
462+
# - l_ssd (local storage): Usually auto-deleted with the server, use Instance API
463+
# - sbs_volume (block storage): Persists after server deletion, must be deleted manually using Block API
444464
for volume_id in volume_ids:
445465
try:
446-
self.client.delete_volume(
447-
zone=self.config.zone,
448-
volume_id=volume_id,
449-
)
450-
log.info(f"Volume {volume_id} deleted successfully")
466+
# First, try to delete using Block Storage API (for sbs_volume)
467+
try:
468+
self.block_client.delete_volume(
469+
zone=self.config.zone,
470+
volume_id=volume_id,
471+
)
472+
log.info(
473+
f"Block storage volume {volume_id} deleted successfully"
474+
)
475+
except Exception as block_error:
476+
block_error_msg = str(block_error)
477+
# If volume not found in Block API, try Instance API (for l_ssd volumes)
478+
if (
479+
"404" in block_error_msg
480+
or "not_found" in block_error_msg.lower()
481+
):
482+
log.debug(
483+
f"Volume {volume_id} not found in Block API, trying Instance API"
484+
)
485+
self.client.delete_volume(
486+
zone=self.config.zone,
487+
volume_id=volume_id,
488+
)
489+
log.info(
490+
f"Instance volume {volume_id} deleted successfully"
491+
)
492+
else:
493+
raise block_error
451494
except Exception as vol_error:
452495
error_msg = str(vol_error)
453496
# Volume may already be deleted automatically (especially l_ssd volumes)
454-
# or might not be found if searching in wrong scope
455497
if "404" in error_msg or "not_found" in error_msg.lower():
456498
log.info(
457499
f"Volume {volume_id} not found - may have been auto-deleted with server or already cleaned up"

tests/unit/backend/test_scaleway.py

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,21 @@ def fake_scaleway_group(scaleway_group, monkeypatch):
7272
mock_client.server_action.return_value = None
7373
mock_client.update_server.return_value = None
7474
mock_client.delete_server.return_value = None
75+
mock_client.delete_volume.return_value = None # For fallback from Block API
76+
77+
# Mock Block Storage API client
78+
# By default, simulate that volumes are not found in Block API (like l_ssd volumes)
79+
# This will trigger fallback to Instance API, maintaining compatibility with existing tests
80+
mock_block_client = MagicMock()
81+
mock_block_client.delete_volume.side_effect = Exception(
82+
"404 not_found: Volume not in Block API"
83+
)
7584

7685
# Patch the client property
7786
monkeypatch.setattr(ScalewayBackend, "client", property(lambda self: mock_client))
87+
monkeypatch.setattr(
88+
ScalewayBackend, "block_client", property(lambda self: mock_block_client)
89+
)
7890

7991
# Mock wait_for_server_state to return immediately
8092
def mock_wait(self, server_id, target_state, timeout=300):
@@ -549,6 +561,119 @@ def test_delete_not_found(scaleway_runner, fake_scaleway_group, caplog):
549561
assert result == 1
550562

551563

564+
def test_delete_with_block_storage_volume(
565+
scaleway_runner, fake_scaleway_group, caplog, monkeypatch
566+
):
567+
"""Test instance deletion with block storage (sbs_volume) using Block API."""
568+
backend = fake_scaleway_group.backend
569+
scaleway_runner.instance_id = "test-server-id"
570+
scaleway_runner.save()
571+
572+
# Mock server with block storage volume
573+
mock_volume = MagicMock()
574+
mock_volume.id = "test-block-volume-id"
575+
mock_server = MagicMock()
576+
mock_server.id = "test-server-id"
577+
mock_server.state = ServerState.RUNNING
578+
mock_server.volumes = {"0": mock_volume}
579+
580+
# Mock Instance API client
581+
mock_client = MagicMock()
582+
mock_client.get_server.return_value = MagicMock(server=mock_server)
583+
mock_client.delete_server.return_value = None
584+
mock_client.delete_volume.return_value = None
585+
mock_client.server_action.return_value = None
586+
587+
# Mock Block Storage API client
588+
mock_block_client = MagicMock()
589+
mock_block_client.delete_volume.return_value = None
590+
591+
# Patch both clients
592+
monkeypatch.setattr(ScalewayBackend, "client", property(lambda self: mock_client))
593+
monkeypatch.setattr(
594+
ScalewayBackend, "block_client", property(lambda self: mock_block_client)
595+
)
596+
597+
# Mock wait_for_server_state
598+
def mock_wait(self, server_id, target_state, timeout=300):
599+
return mock_server
600+
601+
monkeypatch.setattr(ScalewayBackend, "wait_for_server_state", mock_wait)
602+
603+
result = backend.delete(scaleway_runner)
604+
605+
# Verify Block Storage API was called first
606+
mock_block_client.delete_volume.assert_called_once_with(
607+
zone=backend.config.zone,
608+
volume_id="test-block-volume-id",
609+
)
610+
# Verify Instance API delete_volume was NOT called
611+
mock_client.delete_volume.assert_not_called()
612+
# Verify successful deletion was logged
613+
assert (
614+
"Block storage volume test-block-volume-id deleted successfully" in caplog.text
615+
)
616+
assert result == 1
617+
618+
619+
def test_delete_with_volume_fallback_to_instance_api(
620+
scaleway_runner, fake_scaleway_group, caplog, monkeypatch
621+
):
622+
"""Test volume deletion fallback from Block API to Instance API for l_ssd volumes."""
623+
backend = fake_scaleway_group.backend
624+
scaleway_runner.instance_id = "test-server-id"
625+
scaleway_runner.save()
626+
627+
# Mock server with l_ssd volume
628+
mock_volume = MagicMock()
629+
mock_volume.id = "test-lssd-volume-id"
630+
mock_server = MagicMock()
631+
mock_server.id = "test-server-id"
632+
mock_server.state = ServerState.RUNNING
633+
mock_server.volumes = {"0": mock_volume}
634+
635+
# Mock Instance API client
636+
mock_client = MagicMock()
637+
mock_client.get_server.return_value = MagicMock(server=mock_server)
638+
mock_client.delete_server.return_value = None
639+
mock_client.delete_volume.return_value = None
640+
mock_client.server_action.return_value = None
641+
642+
# Mock Block Storage API client to return 404 (volume not found in Block API)
643+
mock_block_client = MagicMock()
644+
mock_block_client.delete_volume.side_effect = Exception(
645+
"404 not_found: Volume not found in Block API"
646+
)
647+
648+
# Patch both clients
649+
monkeypatch.setattr(ScalewayBackend, "client", property(lambda self: mock_client))
650+
monkeypatch.setattr(
651+
ScalewayBackend, "block_client", property(lambda self: mock_block_client)
652+
)
653+
654+
# Mock wait_for_server_state
655+
def mock_wait(self, server_id, target_state, timeout=300):
656+
return mock_server
657+
658+
monkeypatch.setattr(ScalewayBackend, "wait_for_server_state", mock_wait)
659+
660+
result = backend.delete(scaleway_runner)
661+
662+
# Verify Block Storage API was called first
663+
mock_block_client.delete_volume.assert_called_once_with(
664+
zone=backend.config.zone,
665+
volume_id="test-lssd-volume-id",
666+
)
667+
# Verify Instance API was called as fallback
668+
mock_client.delete_volume.assert_called_once_with(
669+
zone=backend.config.zone,
670+
volume_id="test-lssd-volume-id",
671+
)
672+
# Verify successful deletion through Instance API was logged
673+
assert "Instance volume test-lssd-volume-id deleted successfully" in caplog.text
674+
assert result == 1
675+
676+
552677
def test_list_with_auto_create(fake_scaleway_group, monkeypatch):
553678
"""Test list() creates runners for servers not in database."""
554679
backend = fake_scaleway_group.backend

0 commit comments

Comments
 (0)