various improvemets

Makefile

@@ -5,7 +5,7 @@ DOCS_BUILDDIR = docs/build
 
 .PHONY: docs
 
-all: build
+all: docs build-deb
 
 requirements.txt:
 	poetry export -f requirements.txt -o requirements.txt

compute/__main__.py

@@ -15,7 +15,7 @@
 
 """Command line interface for compute module."""
 
-from compute.cli import main
+from compute.cli import control
 
 
-main.cli()
+control.cli()

compute/cli/control.py

@@ -30,7 +30,7 @@ import yaml
 from pydantic import ValidationError
 
 from compute import __version__
-from compute.exceptions import ComputeError, GuestAgentTimeoutExceededError
+from compute.exceptions import ComputeError, GuestAgentTimeoutError
 from compute.instance import GuestAgent
 from compute.session import Session
 from compute.utils import ids
@@ -116,7 +116,7 @@ def _exec_guest_agent_command(
             decode_output=True,
             poll=True,
         )
-    except GuestAgentTimeoutExceededError as e:
+    except GuestAgentTimeoutError as e:
         sys.exit(
             f'{e}. NOTE: command may still running in guest, '
             f'PID={ga.last_pid}'

compute/common.py

@@ -17,14 +17,26 @@
 
 from abc import ABC, abstractmethod
 
+from pydantic import BaseModel, Extra
+
+
+class EntityModel(BaseModel):
+    """Basic entity model."""
+
+    class Config:
+        """Do not allow extra fields."""
+
+        extra = Extra.forbid
+
 
 class EntityConfig(ABC):
     """An abstract entity XML config builder class."""
 
     @abstractmethod
     def to_xml(self) -> str:
-        """Return device XML config."""
+        """Return entity XML config."""
         raise NotImplementedError
 
 
-DeviceConfig = EntityConfig
+class DeviceConfig(EntityConfig):
+    """An abstract device XML config."""

compute/exceptions.py

@@ -36,12 +36,12 @@ class GuestAgentUnavailableError(GuestAgentError):
     """Guest agent is not connected or is unavailable."""
 
 
-class GuestAgentTimeoutExceededError(GuestAgentError):
+class GuestAgentTimeoutError(GuestAgentError):
     """QEMU timeout exceeded."""
 
-    def __init__(self, msg: int):
+    def __init__(self, seconds: int):
         """Initialise GuestAgentTimeoutExceededError."""
-        super().__init__(f'QEMU timeout ({msg} sec) exceeded')
+        super().__init__(f'QEMU timeout ({seconds} sec) exceeded')
 
 
 class GuestAgentCommandNotSupportedError(GuestAgentError):
@@ -78,3 +78,11 @@ class InstanceNotFoundError(InstanceError):
     def __init__(self, msg: str):
         """Initialise InstanceNotFoundError."""
         super().__init__(f"compute instance '{msg}' not found")
+
+
+class InvalidDataUnitError(ValueError, ComputeError):
+    """Data unit is not valid."""
+
+    def __init__(self, msg: str, units: list):
+        """Initialise InvalidDataUnitError."""
+        super().__init__(f'{msg}, valid units are: {", ".join(units)}')

compute/instance/guest_agent.py

@@ -27,7 +27,7 @@ import libvirt_qemu
 from compute.exceptions import (
     GuestAgentCommandNotSupportedError,
     GuestAgentError,
-    GuestAgentTimeoutExceededError,
+    GuestAgentTimeoutError,
     GuestAgentUnavailableError,
 )
 
@@ -199,7 +199,7 @@ class GuestAgent:
             sleep(poll_interval)
             now = time()
             if now - start_time > self.timeout:
-                raise GuestAgentTimeoutExceededError(self.timeout)
+                raise GuestAgentTimeoutError(self.timeout)
         log.debug(
             'Polling command pid=%s finished, time taken: %s seconds',
             pid,

compute/instance/instance.py

@@ -322,6 +322,8 @@ class Instance:
 
         :param method: Method used to shutdown instance
         """
+        if not self.is_running():
+            return
         methods = {
             'SOFT': libvirt.VIR_DOMAIN_SHUTDOWN_GUEST_AGENT,
             'NORMAL': libvirt.VIR_DOMAIN_SHUTDOWN_DEFAULT,
@@ -498,11 +500,6 @@ class Instance:
             msg = f'Cannot set memory for instance={self.name} {memory=}: {e}'
             raise InstanceError(msg) from e
 
-    def _get_disk_by_target(self, target: str) -> etree.Element:
-        xml = etree.fromstring(self.dump_xml())  # noqa: S320
-        child = xml.xpath(f'/domain/devices/disk/target[@dev="{target}"]')
-        return child[0].getparent() if child else None
-
     def attach_device(
         self, device: DeviceConfig, *, live: bool = False
     ) -> None:
@@ -520,7 +517,7 @@ class Instance:
         else:
             flags = libvirt.VIR_DOMAIN_AFFECT_CONFIG
         if isinstance(device, DiskConfig):  # noqa: SIM102
-            if self._get_disk_by_target(device.target):
+            if self.get_disk(device.target):
                 log.warning(
                     "Volume with target '%s' is already attached",
                     device.target,
@@ -545,7 +542,7 @@ class Instance:
         else:
             flags = libvirt.VIR_DOMAIN_AFFECT_CONFIG
         if isinstance(device, DiskConfig):  # noqa: SIM102
-            if self._get_disk_by_target(device.target) is None:
+            if self.get_disk(device.target) is None:
                 log.warning(
                     "Volume with target '%s' is already detached",
                     device.target,
@@ -553,6 +550,27 @@ class Instance:
                 return
         self.domain.detachDeviceFlags(device.to_xml(), flags=flags)
 
+    def get_disk(self, name: str) -> DiskConfig | None:
+        """
+        Return :class:`DiskConfig` by disk target name.
+
+        Return None if disk with specified target not found.
+
+        :param name: Disk name e.g. `vda`, `sda`, etc. This name may
+            not match the name of the disk inside the guest OS.
+        """
+        xml = etree.fromstring(self.dump_xml())
+        child = xml.xpath(f'/domain/devices/disk/target[@dev="{name}"]')
+        if len(child) == 0:
+            return None
+        return DiskConfig.from_xml(child[0].getparent())
+
+    def list_disks(self) -> list[DiskConfig]:
+        """Return list of attached disk devices."""
+        xml = etree.fromstring(self.dump_xml())
+        disks = xml.xpath('/domain/devices/disk')
+        return [DiskConfig.from_xml(disk) for disk in disks]
+
     def detach_disk(self, name: str) -> None:
         """
         Detach disk device by target name.
@@ -560,31 +578,17 @@ class Instance:
         There is no ``attach_disk()`` method. Use :func:`attach_device`
         with :class:`DiskConfig` as argument.
 
-        :param name: Disk name e.g. 'vda', 'sda', etc. This name may
+        :param name: Disk name e.g. `vda`, `sda`, etc. This name may
             not match the name of the disk inside the guest OS.
         """
-        xml = self._get_disk_by_target(name)
+        disk = self.get_disk(name)
-        if xml is None:
+        if disk is None:
             log.warning(
                 "Volume with target '%s' is already detached",
                 name,
             )
             return
-        disk_params = {
+        self.detach_device(disk, live=True)
-            'disk_type': xml.get('type'),
-            'source': xml.find('source').get('file'),
-            'target': xml.find('target').get('dev'),
-            'readonly': False if xml.find('readonly') is None else True,  # noqa: SIM211
-        }
-        for param in disk_params:
-            if disk_params[param] is None:
-                msg = (
-                    f"Cannot detach volume with target '{name}': "
-                    f"parameter '{param}' is not defined in libvirt XML "
-                    'config on host.'
-                )
-                raise InstanceError(msg)
-        self.detach_device(DiskConfig(**disk_params), live=True)
 
     def resize_disk(
         self, name: str, capacity: int, unit: units.DataUnit
@@ -592,7 +596,8 @@ class Instance:
         """
         Resize attached block device.
 
-        :param name: Disk device name e.g. `vda`, `sda`, etc.
+        :param name: Disk name e.g. `vda`, `sda`, etc. This name may
+            not match the name of the disk inside the guest OS.
         :param capacity: New capacity.
         :param unit: Capacity unit.
         """
@@ -602,10 +607,6 @@ class Instance:
             flags=libvirt.VIR_DOMAIN_BLOCK_RESIZE_BYTES,
         )
 
-    def get_disks(self) -> list[DiskConfig]:
-        """Return list of attached disks."""
-        raise NotImplementedError
-
     def pause(self) -> None:
         """Pause instance."""
         if not self.is_running():
@@ -616,31 +617,75 @@ class Instance:
         """Resume paused instance."""
         self.domain.resume()
 
-    def get_ssh_keys(self, user: str) -> list[str]:
+    def list_ssh_keys(self, user: str) -> list[str]:
         """
         Return list of SSH keys on guest for specific user.
 
         :param user: Username.
         """
-        raise NotImplementedError
+        self.guest_agent.raise_for_commands(['guest-ssh-get-authorized-keys'])
+        exc = self.guest_agent.guest_exec(
+            path='/bin/sh',
+            args=[
+                '-c',
+                (
+                    'su -c "'
+                    'if ! [ -f ~/.ssh/authorized_keys ]; then '
+                    'mkdir -p ~/.ssh && touch ~/.ssh/authorized_keys; '
+                    'fi" '
+                    f'{user}'
+                ),
+            ],
+            capture_output=True,
+            decode_output=True,
+            poll=True,
+        )
+        log.debug(exc)
+        try:
+            return self.domain.authorizedSSHKeysGet(user)
+        except libvirt.libvirtError as e:
+            raise InstanceError(e) from e
 
-    def set_ssh_keys(self, user: str, ssh_keys: list[str]) -> None:
+    def set_ssh_keys(
+        self,
+        user: str,
+        keys: list[str],
+        *,
+        remove: bool = False,
+        append: bool = False,
+    ) -> None:
         """
         Add SSH keys to guest for specific user.
 
         :param user: Username.
-        :param ssh_keys: List of public SSH keys.
+        :param keys: List of authorized SSH keys.
+        :param append: Append keys to authorized SSH keys instead of
+            overriding authorized_keys file.
+        :param remove: Remove authorized keys listed in `keys` parameter.
         """
-        raise NotImplementedError
+        qemu_ga_commands = ['guest-ssh-add-authorized-keys']
-
+        if remove and append:
-    def delete_ssh_keys(self, user: str, ssh_keys: list[str]) -> None:
+            raise InstanceError(
-        """
+                "'append' and 'remove' parameters is mutually exclusive"
-        Remove SSH keys from guest for specific user.
+            )
-
+        if not self.is_running():
-        :param user: Username.
+            raise InstanceError(
-        :param ssh_keys: List of public SSH keys.
+                'Cannot add authorized SSH keys to inactive instance'
-        """
+            )
-        raise NotImplementedError
+        if append:
+            flags = libvirt.VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND
+        elif remove:
+            flags = libvirt.VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_REMOVE
+            qemu_ga_commands = ['guest-ssh-remove-authorized-keys']
+        else:
+            flags = 0
+        if keys.sort() == self.list_ssh_keys().sort():
+            return
+        self.guest_agent.raise_for_commands(qemu_ga_commands)
+        try:
+            self.domain.authorizedSSHKeysSet(user, keys, flags=flags)
+        except libvirt.libvirtError as e:
+            raise InstanceError(e) from e
 
     def set_user_password(
         self, user: str, password: str, *, encrypted: bool = False
@@ -655,10 +700,6 @@ class Instance:
         :param encrypted: Set it to True if password is already encrypted.
             Right encryption method depends on guest OS.
         """
-        if not self.guest_agent.is_available():
-            raise InstanceError(
-                'Cannot change password: guest agent is unavailable'
-            )
         self.guest_agent.raise_for_commands(['guest-set-user-password'])
         flags = libvirt.VIR_DOMAIN_PASSWORD_ENCRYPTED if encrypted else 0
         self.domain.setUserPassword(user, password, flags=flags)
@@ -669,7 +710,10 @@ class Instance:
         return self.domain.XMLDesc(flags)
 
     def delete(self) -> None:
-        """Undefine instance."""
+        """Delete instance with local disks."""
-        # TODO @ge: delete local disks
         self.shutdown(method='HARD')
+        for disk in self.list_disks():
+            if disk.disk_type == 'file':
+                volume = self.connection.storageVolLookupByPath(disk.source)
+                volume.delete()
         self.domain.undefine()

compute/instance/schemas.py

@@ -19,20 +19,12 @@ import re
 from enum import StrEnum
 from pathlib import Path
 
-from pydantic import BaseModel, Extra, validator
+from pydantic import validator
 
+from compute.common import EntityModel
 from compute.utils.units import DataUnit
 
 
-class EntityModel(BaseModel):
-    """Basic entity model."""
-
-    class Config:
-        """Do not allow extra fields."""
-
-        extra = Extra.forbid
-
-
 class CPUEmulationMode(StrEnum):
     """CPU emulation mode enumerated."""
 

compute/session.py

@@ -154,7 +154,7 @@ class Session(AbstractContextManager):
         """Return capabilities e.g. arch, virt, emulator, etc."""
         prefix = '/domainCapabilities'
         hprefix = f'{prefix}/cpu/mode[@name="host-model"]'
-        caps = etree.fromstring(self.connection.getDomainCapabilities())  # noqa: S320
+        caps = etree.fromstring(self.connection.getDomainCapabilities())
         return Capabilities(
             arch=caps.xpath(f'{prefix}/arch/text()')[0],
             virt_type=caps.xpath(f'{prefix}/domain/text()')[0],

compute/storage/pool.py

@@ -49,12 +49,12 @@ class StoragePool:
 
     def _get_path(self) -> Path:
         """Return storage pool path."""
-        xml = etree.fromstring(self.pool.XMLDesc())  # noqa: S320
+        xml = etree.fromstring(self.pool.XMLDesc())
         return Path(xml.xpath('/pool/target/path/text()')[0])
 
     def get_usage_info(self) -> StoragePoolUsageInfo:
         """Return info about storage pool usage."""
-        xml = etree.fromstring(self.pool.XMLDesc())  # noqa: S320
+        xml = etree.fromstring(self.pool.XMLDesc())
         return StoragePoolUsageInfo(
             capacity=int(xml.xpath('/pool/capacity/text()')[0]),
             allocation=int(xml.xpath('/pool/allocation/text()')[0]),

compute/storage/volume.py

@@ -18,6 +18,7 @@
 from dataclasses import dataclass
 from pathlib import Path
 from time import time
+from typing import Union
 
 import libvirt
 from lxml import etree
@@ -88,6 +89,28 @@ class DiskConfig(DeviceConfig):
             xml.append(E.readonly())
         return etree.tostring(xml, encoding='unicode', pretty_print=True)
 
+    @classmethod
+    def from_xml(cls, xml: Union[str, etree.Element]) -> 'DiskConfig':  # noqa: UP007
+        """
+        Return :class:`DiskConfig` instance using existing XML config.
+
+        :param xml: Disk device XML configuration as :class:`str` or lxml
+            :class:`etree.Element` object.
+        """
+        if isinstance(xml, str):
+            xml = etree.fromstring(xml)
+        disk_params = {
+            'disk_type': xml.get('type'),
+            'source': xml.find('source').get('file'),
+            'target': xml.find('target').get('dev'),
+            'readonly': False if xml.find('readonly') is None else True,  # noqa: SIM211
+        }
+        for param in disk_params:
+            if disk_params[param] is None:
+                msg = f"Bad XML config: parameter '{param}' is not defined"
+                raise ValueError(msg)
+        return cls(**disk_params)
+
 
 class Volume:
     """Storage volume manipulating class."""

compute/utils/units.py

@@ -17,6 +17,8 @@
 
 from enum import StrEnum
 
+from compute.exceptions import InvalidDataUnitError
+
 
 class DataUnit(StrEnum):
     """Data units enumerated."""
@@ -28,22 +30,12 @@ class DataUnit(StrEnum):
     TIB = 'TiB'
 
 
-class InvalidDataUnitError(ValueError):
-    """Data unit is not valid."""
-
-    def __init__(self, msg: str):
-        """Initialise InvalidDataUnitError."""
-        super().__init__(
-            f'{msg}, valid units are: {", ".join(list(DataUnit))}'
-        )
-
-
 def to_bytes(value: int, unit: DataUnit = DataUnit.BYTES) -> int:
     """Convert value to bytes. See :class:`DataUnit`."""
     try:
         _ = DataUnit(unit)
     except ValueError as e:
-        raise InvalidDataUnitError(e) from e
+        raise InvalidDataUnitError(e, list(DataUnit)) from e
     powers = {
         DataUnit.BYTES: 0,
         DataUnit.KIB: 1,

docs/source/pyapi/index.rst

@@ -1,22 +1,17 @@
 Python API
 ==========
 
-The API allows you to perform actions on instances programmatically. Below is
+The API allows you to perform actions on instances programmatically.
-an example of changing parameters and launching the `myinstance` instance.
 
 .. code-block:: python
 
-    import logging
+    import compute
 
-    from compute import Session
+    with compute.Session() as session:
-
-    logging.basicConfig(level=logging.DEBUG)
-
-    with Session() as session:
         instance = session.get_instance('myinstance')
-        instance.set_vcpus(4)
+        info = instance.get_info()
-        instance.start()
+
-        instance.set_autostart(enabled=True)
+    print(info)
 
 
 :class:`Session` context manager provides an abstraction over :class:`libvirt.virConnect`

packaging/files/compute.bash-completion

@@ -18,16 +18,12 @@ _compute_root_cmd="
     status
     setvcpus
     setmem
-    setpasswd"
+    setpass"
 _compute_init_opts=""
-_compute_exec_opts="
+_compute_exec_opts="--timeout --executable --env --no-join-args"
-    --timeout
-    --executable
-    --env
-    --no-join-args"
 _compute_ls_opts=""
 _compute_start_opts=""
-_compute_shutdown_opts="--method"
+_compute_shutdown_opts="--soft --normal --hard --unsafe"
 _compute_reboot_opts=""
 _compute_reset_opts=""
 _compute_powrst_opts=""
@@ -36,13 +32,14 @@ _compute_resume_opts=""
 _compute_status_opts=""
 _compute_setvcpus_opts=""
 _compute_setmem_opts=""
-_compute_setpasswd_opts="--encrypted"
+_compute_setpass_opts="--encrypted"
 
 _compute_complete_instances()
 {
+    local base_name
     for file in /etc/libvirt/qemu/*.xml; do
-        nodir="${file##*/}"
+        base_name="${file##*/}"
-        printf '%s ' "${nodir//\.xml}"
+        printf '%s ' "${base_name//\.xml}"
     done
 }
 
@@ -80,7 +77,7 @@ _compute_complete()
                 status) _compute_compreply "$_compute_status_opts";;
                 setvcpus) _compute_compreply "$_compute_setvcpus_opts";;
                 setmem) _compute_compreply "$_compute_setmem_opts";;
-                setpasswd) _compute_compreply "$_compute_setpasswd_opts";;
+                setpass) _compute_compreply "$_compute_setpass_opts";;
                 *) COMPREPLY=()
             esac
             ;;

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = 'compute'
-version = '0.1.0-dev1'
+version = '0.1.0-dev2'
 description = 'Compute instances management library and tools'
 authors = ['ge <ge@nixhacks.net>']
 readme = 'README.md'
@@ -42,11 +42,19 @@ target-version = 'py311'
 [tool.ruff.lint]
 select = ['ALL']
 ignore = [
-    'Q000', 'Q003', 'D211', 'D212',
+    'Q000', 'Q003',
-    'ANN101', 'ISC001', 'COM812',
+    'D211', 'D212',
-    'D203', 'ANN204', 'T201',
+    'ANN101', 'ANN102', 'ANN204',
-    'EM102', 'TRY003', 'EM101',
+    'ISC001',
-    'TD003', 'TD006', 'FIX002',  # 'todo' strings linting
+    'COM812',
+    'D203',
+    'T201',
+    'S320',
+    'EM102',
+    'TRY003',
+    'EM101',
+    'TD003', 'TD006',
+    'FIX002',
 ]
 exclude = ['__init__.py']