Skip to content

Commit 2ec9d8e

Browse files
Anton Kremenetskyakremenetsky
authored andcommitted
fix(compute): fix volume create/delete in disk_spec reconciliation
Signed-off-by: Anton Kremenetsky <anton.kremenetsky@gmail.com>
1 parent 5f9d703 commit 2ec9d8e

5 files changed

Lines changed: 166 additions & 17 deletions

File tree

exordos_core/compute/builders/node.py

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -146,13 +146,19 @@ def _actualize_volumes(
146146

147147
# Create volumes
148148
for volume_uuid in target_map.keys() - actual_map.keys():
149-
volume = target_map[volume_uuid]
150-
volume.save()
149+
# Need to convert as they are different types (SDK vs DM)
150+
sdk_volume = target_map[volume_uuid]
151+
view = sdk_volume.dump_to_simple_view()
152+
volume = models.Volume.restore_from_simple_view(**view)
153+
volume.insert()
151154

152155
# Delete volumes
153-
for volume_uuid in actual_map.keys() - target_map.keys():
154-
volume = actual_map[volume_uuid]
155-
volume.delete()
156+
delete_uuids = actual_map.keys() - target_map.keys()
157+
if delete_uuids:
158+
for volume in models.Volume.objects.get_all(
159+
filters={"uuid": dm_filters.In(delete_uuids)}
160+
):
161+
volume.delete()
156162

157163
# Update volumes
158164
need_update = {}
@@ -194,7 +200,9 @@ def _update_volumes(self, target_node: Node, actual_node: Node) -> bool:
194200
return False
195201

196202
# Get volumes from disk specs
197-
target_volumes = target_disk_spec.volumes(target_node)
203+
target_volumes = target_disk_spec.volumes(
204+
target_node, project_id=target_node.volume_project_id
205+
)
198206
actual_volumes = actual_disk_spec.volumes(actual_node)
199207

200208
return self._actualize_volumes(target_volumes, actual_volumes)

exordos_core/compute/dm/models.py

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,21 @@ def volumes(self) -> tp.Collection[Volume]:
323323
"""Return the list of volumes for this node."""
324324
return self.disk_spec.volumes(self)
325325

326+
@property
327+
def volume_project_id(self) -> sys_uuid.UUID:
328+
"""Project ID to use for this node's volumes.
329+
330+
Handle a special case for EM. We cannot put volumes in the same
331+
project as the node because the volumes are created as children
332+
of the node and they aren't present in the manifest. So EM
333+
doesn't know about the volumes.
334+
"""
335+
return (
336+
self.project_id
337+
if self.project_id != cc.EM_PROJECT_ID
338+
else cc.EM_HIDDEN_PROJECT_ID
339+
)
340+
326341
def update_default_network(self, port: "Port") -> None:
327342
self.default_network = {
328343
"subnet": str(port.subnet),
@@ -363,18 +378,8 @@ def insert(self, session=None):
363378
)
364379
allocation.insert(session=session)
365380

366-
# Handle a special case for EM. We cannot put volumes in the same
367-
# project as the node because the volumes are created as children
368-
# of the node and they aren't present in the manifest. So EM
369-
# doesn't know about the volumes.
370-
volume_project_id = (
371-
self.project_id
372-
if self.project_id != cc.EM_PROJECT_ID
373-
else cc.EM_HIDDEN_PROJECT_ID
374-
)
375-
376381
# Update or create volumes for the node
377-
volumes = self.disk_spec.volumes(self, project_id=volume_project_id)
382+
volumes = self.disk_spec.volumes(self, project_id=self.volume_project_id)
378383
for sdk_volume in volumes:
379384
# Need to convert as they are different types (SDK vs DM)
380385
view = sdk_volume.dump_to_simple_view()

exordos_core/compute/scheduler/service.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,7 @@ def _schedule_volume_on_pools(self, pools: tp.List[base.MachinePoolBundle]) -> N
555555
try:
556556
machine_volume = self._place_volume_into_pool(volume, pool)
557557
machine_volume.pool = pool.pool.uuid
558+
machine_volume.machine = node_map[volume.node].uuid
558559
machine_volume.save()
559560

560561
volume.pool = pool.pool.uuid
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
# Copyright 2026 Genesis Corporation.
2+
#
3+
# All Rights Reserved.
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License"); you may
6+
# not use this file except in compliance with the License. You may obtain
7+
# a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
13+
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
14+
# License for the specific language governing permissions and limitations
15+
# under the License.
16+
17+
import typing as tp
18+
import uuid as sys_uuid
19+
20+
from gcl_sdk.infra.dm import models as sdk_models
21+
import pytest
22+
from restalchemy.dm import filters as dm_filters
23+
24+
from exordos_core.common import constants as c
25+
from exordos_core.compute.builders import node as node_builder
26+
from exordos_core.compute.dm import models
27+
28+
29+
class TestNodeBuilderService:
30+
def setup_method(self) -> None:
31+
self._service = node_builder.NodeBuilderService()
32+
33+
def _add_node(self, disks: tp.List[dict]) -> models.Node:
34+
node = models.Node(
35+
uuid=sys_uuid.uuid4(),
36+
name="foo-node",
37+
cores=1,
38+
ram=1024,
39+
disk_spec=sdk_models.DisksSpec(disks=disks),
40+
project_id=c.SERVICE_PROJECT_ID,
41+
)
42+
node.insert()
43+
return node
44+
45+
def _node_copy_with_disks(
46+
self, node: models.Node, disks: tp.List[dict]
47+
) -> models.Node:
48+
return models.Node(
49+
uuid=node.uuid,
50+
name=node.name,
51+
cores=node.cores,
52+
ram=node.ram,
53+
disk_spec=sdk_models.DisksSpec(disks=disks),
54+
project_id=node.project_id,
55+
)
56+
57+
def _node_volumes(self, node_uuid: sys_uuid.UUID) -> tp.List[models.Volume]:
58+
return list(
59+
models.Volume.objects.get_all(
60+
filters={"node": dm_filters.EQ(node_uuid)},
61+
)
62+
)
63+
64+
@pytest.mark.usefixtures("user_api_client", "auth_user_admin")
65+
def test_add_and_remove_extra_disk(self):
66+
root_disk = {"size": 8, "image": "ubuntu_24.04"}
67+
data_disk = {"size": 20, "label": "data"}
68+
69+
actual_node = self._add_node(disks=[root_disk])
70+
volumes = self._node_volumes(actual_node.uuid)
71+
assert len(volumes) == 1
72+
73+
# Add a data disk to the node
74+
target_with_data = self._node_copy_with_disks(
75+
actual_node, disks=[root_disk, data_disk]
76+
)
77+
self._service._update_volumes(target_with_data, actual_node)
78+
79+
volumes = self._node_volumes(actual_node.uuid)
80+
assert len(volumes) == 2
81+
added_volume = next(v for v in volumes if v.label == "data")
82+
assert added_volume.size == 20
83+
assert added_volume.project_id == actual_node.volume_project_id
84+
85+
# Remove the data disk from the node
86+
target_without_data = self._node_copy_with_disks(actual_node, disks=[root_disk])
87+
self._service._update_volumes(target_without_data, target_with_data)
88+
89+
volumes = self._node_volumes(actual_node.uuid)
90+
assert len(volumes) == 1
91+
assert volumes[0].label != "data"
92+
93+
volumes[0].delete()
94+
actual_node.delete()

exordos_core/tests/functional/service/test_scheduler.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import uuid as sys_uuid
2020

2121
from gcl_iam.tests.functional import clients as iam_clients
22+
from restalchemy.dm import filters as dm_filters
2223

2324
from exordos_core.compute.dm import models
2425
from exordos_core.compute.scheduler import service
@@ -87,6 +88,46 @@ def test_schedule_node(
8788
assert str(machines[0].node) == default_node["uuid"]
8889
assert volumes[0].machine == machines[0].uuid
8990

91+
def test_schedule_extra_volume_on_scheduled_node(
92+
self,
93+
default_pool: tp.Dict[str, tp.Any],
94+
default_node: tp.Dict[str, tp.Any],
95+
default_machine_agent: tp.Dict[str, tp.Any],
96+
default_pool_builder: tp.Dict[str, tp.Any],
97+
):
98+
# Schedule the node with its root volume first
99+
self._service._iteration()
100+
self._service._iteration()
101+
102+
machine = models.Machine.objects.get_one(
103+
filters={"node": dm_filters.EQ(sys_uuid.UUID(default_node["uuid"]))}
104+
)
105+
106+
# Simulate a disk added to the already-scheduled node, as done by
107+
# NodeBuilderService._actualize_volumes when the node's disk_spec
108+
# is extended with an extra disk.
109+
extra_volume = models.Volume(
110+
uuid=sys_uuid.uuid4(),
111+
name="data",
112+
label="data",
113+
node=sys_uuid.UUID(default_node["uuid"]),
114+
size=20,
115+
index=1,
116+
project_id=machine.project_id,
117+
status="NEW",
118+
)
119+
extra_volume.insert()
120+
121+
self._service._iteration()
122+
123+
machine_volumes = models.MachineVolume.objects.get_all(
124+
filters={"node_volume": dm_filters.EQ(extra_volume.uuid)}
125+
)
126+
assert len(machine_volumes) == 1
127+
assert machine_volumes[0].machine == machine.uuid
128+
129+
extra_volume.delete()
130+
90131
def test_schedule_node_no_builders(
91132
self,
92133
default_pool: tp.Dict[str, tp.Any],

0 commit comments

Comments
 (0)