Bugfix: Broken update operations for cores and ram

The bugfix for the #62 issue.
- Fixed node resize for cores and ram.
- More graceful treatment of errors handling for root volume creating
- New `get_volume` method for the driver

Signed-off-by: Anton Kremenetsky <anton.kremenetsky@gmail.com>

Author: akremenetsky

genesis_core/node/machine/pool/driver/base.py

@@ -15,6 +15,7 @@
 #    under the License.
 
 import abc
+import uuid as sys_uuid
 import typing as tp
 
 from genesis_core.node.dm import models
@@ -57,6 +58,12 @@ def list_volumes(
     ) -> tp.Iterable[models.MachineVolume]:
         """Return volume list from data plane."""
 
+    @abc.abstractmethod
+    def get_volume(
+        self, machine: sys_uuid.UUID, uuid: sys_uuid.UUID
+    ) -> models.MachineVolume:
+        """Get the machine volume by uuid."""
+
     @abc.abstractmethod
     def set_machine_cores(self, machine: models.Machine, cores: int) -> None:
         """Set machine cores."""
@@ -111,6 +118,11 @@ def list_volumes(
         """Return volume list from data plane."""
         return []
 
+    def get_volume(
+        self, machine: sys_uuid.UUID, uuid: sys_uuid.UUID
+    ) -> models.MachineVolume:
+        """Get the machine volume by uuid."""
+
     def set_machine_cores(self, machine: models.Machine, cores: int) -> None:
         """Set machine cores."""
 

genesis_core/node/machine/pool/driver/exceptions.py

@@ -26,3 +26,8 @@ class MachineAlreadyExistsError(exceptions.GCException):
 class VolumeAlreadyExistsError(exceptions.GCException):
     __template__ = "The volume {volume} already exists."
     volume: sys_uuid.UUID
+
+
+class VolumeNotFoundError(exceptions.GCException):
+    __template__ = "The volume {volume} not found."
+    volume: sys_uuid.UUID

genesis_core/node/machine/pool/driver/libvirt.py

@@ -15,7 +15,6 @@
 #    under the License.
 from __future__ import annotations
 
-import json
 import logging
 import time
 import typing as tp
@@ -24,11 +23,13 @@
 import contextlib as ctxlib
 
 import libvirt
+import netaddr
 
 from genesis_core.node.dm import models
 from genesis_core.common import constants as c
 from genesis_core.node import constants as nc
 from genesis_core.node.machine.pool.driver import base
+from genesis_core.node.machine.pool.driver import exceptions as pool_exc
 
 ImageFormatType = tp.Literal["raw", "qcow2"]
 NetworkType = tp.Literal["bridge", "network"]
@@ -474,6 +475,38 @@ def _vir_volume2machine_volume(
             project_id=c.SERVICE_PROJECT_ID,
         )
 
+    def _list_interfaces(self, machine: models.Machine) -> list[models.Port]:
+        """List all interfaces of the machine."""
+        # TODO(akremenetsky): The `Port` model is used to represent
+        # an interface. We need more appropriate model.
+        ports = []
+
+        with ctxlib.closing(self._connect()) as cn:
+            domain = cn.lookupByUUIDString(str(machine.uuid))
+            domain_xml = minidom.parseString(domain.XMLDesc())
+
+            for iface in domain_xml.getElementsByTagName("interface"):
+                mac_tags = iface.getElementsByTagName("mac")
+                if len(mac_tags) != 1 or not mac_tags[0].getAttribute(
+                    "address"
+                ):
+                    LOG.error("Unable to detect MAC address for %s", iface)
+                    continue
+
+                mac = mac_tags[0].getAttribute("address")
+                ports.append(
+                    models.Port(
+                        uuid=sys_uuid.UUID(
+                            "00000000-0000-0000-0000-000000000000"
+                        ),
+                        machine=machine.uuid,
+                        mac=mac,
+                        project_id=c.SERVICE_PROJECT_ID,
+                    )
+                )
+
+        return ports
+
     def list_volumes(
         self, machine: models.Machine
     ) -> tp.Iterable[models.MachineVolume]:
@@ -490,6 +523,40 @@ def list_volumes(
         LOG.debug("Volumes: %s", result)
         return result
 
+    def get_volume(
+        self, machine: sys_uuid.UUID, uuid: sys_uuid.UUID
+    ) -> models.MachineVolume:
+        target_volume = models.MachineVolume(
+            uuid=uuid,
+            machine=machine,
+            # These fields don't make sense in this case, just placeholders
+            size=1,
+            node=sys_uuid.uuid4(),
+            project_id=c.SERVICE_PROJECT_ID,
+        )
+        name = self._form_vir_volume_name(target_volume)
+
+        """Get the machine volume by uuid."""
+        with ctxlib.closing(self._connect()) as cn:
+            storage_pool = cn.storagePoolLookupByName(self._spec.storage_pool)
+
+            # We don't know which format is used for the volume so we try them all
+            for fmt in tp.get_args(ImageFormatType):
+                name_with_format = f"{name}.{fmt}"
+                try:
+                    volume = storage_pool.storageVolLookupByName(
+                        name_with_format
+                    )
+                    break
+                except libvirt.libvirtError as e:
+                    if e.get_error_code() == libvirt.VIR_ERR_NO_STORAGE_VOL:
+                        continue
+                    raise
+            else:
+                raise pool_exc.VolumeNotFoundError(volume=uuid)
+
+        return self._vir_volume2machine_volume(volume)
+
     def create_volume(
         self, volume: models.MachineVolume
     ) -> models.MachineVolume:
@@ -499,7 +566,13 @@ def create_volume(
             volume_xml = XMLLibvirtVolume.xml_from_base_template(
                 storage_pool, name, volume.size << 30
             )
-            virt_volume = storage_pool.createXML(volume_xml)
+
+            try:
+                virt_volume = storage_pool.createXML(volume_xml)
+            except libvirt.libvirtError as e:
+                if e.get_error_code() == libvirt.VIR_ERR_STORAGE_VOL_EXIST:
+                    raise pool_exc.VolumeAlreadyExistsError(volume=volume.uuid)
+                raise
 
         # TODO(akremenetsky): We shouldn't change the original object
         volume.path = virt_volume.path()
@@ -632,22 +705,24 @@ def delete_machine(
 
     def set_machine_cores(self, machine: models.Machine, cores: int) -> None:
         """Set machine cores."""
+        ports = self._list_interfaces(machine)
         volumes = self.list_volumes(machine)
         self.delete_machine(machine, delete_volumes=False)
 
         machine.cores = cores
-        self.create_machine(machine, volumes=volumes)
+        self.create_machine(machine, volumes=volumes, ports=ports)
         LOG.debug(
             "The domain %s was updated with cores %s", machine.uuid, cores
         )
 
     def set_machine_ram(self, machine: models.Machine, ram: int) -> None:
         """Set machine ram."""
+        ports = self._list_interfaces(machine)
         volumes = self.list_volumes(machine)
         self.delete_machine(machine, delete_volumes=False)
 
         machine.ram = ram
-        self.create_machine(machine, volumes=volumes)
+        self.create_machine(machine, volumes=volumes, ports=ports)
         LOG.debug("The domain %s was updated with ram %s", machine.uuid, ram)
 
     def reset_machine(self, machine: models.Machine) -> None:

genesis_core/node/machine/service.py

@@ -156,8 +156,8 @@ def _create_volumes(
             try:
                 driver.create_volume(v)
             except pool_exceptions.VolumeAlreadyExistsError:
-                # Do nothing the volume is already created
-                pass
+                # Copy the necessary fields
+                v.path = driver.get_volume(v.machine, v.uuid).path
 
     def _actualize_machine(
         self,
@@ -220,7 +220,7 @@ def _actualize_pool(
         self._actualize_pool_state(pool, actual_machines.values())
 
         # Delete any machines that are not in the target list
-        for uuid in set(actual_machines.keys()) - set(target_machines.keys()):
+        for uuid in actual_machines.keys() - target_machines.keys():
             machine = actual_machines[uuid]
             try:
                 driver.delete_machine(machine)
@@ -232,7 +232,7 @@ def _actualize_pool(
                 )
 
         # Create any machines that are not in the actual list
-        for uuid in set(target_machines.keys()) - set(actual_machines.keys()):
+        for uuid in target_machines.keys() - actual_machines.keys():
             machine = target_machines[uuid]
             # Skip machines that are not ready. They are building and
             # will be ready a little bit later.
@@ -278,7 +278,7 @@ def _actualize_pool(
                 )
 
         # Actualize any machines that are in both lists
-        for uuid in set(target_machines.keys()) & set(actual_machines.keys()):
+        for uuid in target_machines.keys() & actual_machines.keys():
             target_machine = target_machines[uuid]
             actual_machine = actual_machines[uuid]
             # Skip machines that are not ready. They are building and