sts-sponsors team mailing list archive

[Igor Brovtsin <mp+436340@xxxxxxxxxxxxxxxxxx>]

Igor Brovtsin has proposed merging ~igor-brovtsin/maas:power-drivers-wrapper into maas:master.

Commit message:
Initial version of extracted power drivers layout

- Common wrapper for MAAS power drivers
- Minimal example of a concrete power driver (see `powerdrivers/abstract`)
- Tested `pex` with it, works fine

TBD:
- Integrate it into our Makefile
- Move some power drivers there
- Unit testing configuration for the drivers
- Actual unit tests for the wrapper

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~igor-brovtsin/maas/+git/maas/+merge/436340

I'm asking for a review of:
- the `powerdrivers` folder structure 
- wrapper code in `powerdrivers/common/maaspd`
- the layout of an example power driver project in `powerdrivers/abstract`
-- 
Your team MAAS Committers is subscribed to branch maas:master.

diff --git a/powerdrivers/abstract/README.md b/powerdrivers/abstract/README.md
new file mode 100644
index 0000000..ddc26a4
--- /dev/null
+++ b/powerdrivers/abstract/README.md
@@ -0,0 +1,6 @@
+Build it with
+`pex --inherit-path=fallback -f ../common/dist/ -c maas-powerdriver-abstract -o maas-powerdriver-abstract .`
+
+Run it with `./maas-powerdriver-abstract <action> empty_driver_params.py`
+
+These commands are not final, but will do for now.
\ No newline at end of file
diff --git a/powerdrivers/abstract/empty_driver_params.json b/powerdrivers/abstract/empty_driver_params.json
new file mode 100644
index 0000000..978b7a4
--- /dev/null
+++ b/powerdrivers/abstract/empty_driver_params.json
@@ -0,0 +1,5 @@
+{
+    "driverParameters": {
+    
+    }
+}
diff --git a/powerdrivers/abstract/pd_abstract/__init__.py b/powerdrivers/abstract/pd_abstract/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/powerdrivers/abstract/pd_abstract/__init__.py
diff --git a/powerdrivers/abstract/pd_abstract/driver.py b/powerdrivers/abstract/pd_abstract/driver.py
new file mode 100644
index 0000000..7a95453
--- /dev/null
+++ b/powerdrivers/abstract/pd_abstract/driver.py
@@ -0,0 +1,39 @@
+import sys
+
+from maaspd import pd_main
+from maaspd.classes import PowerDriver
+
+
+class ConcreteAbstractPowerDriver(PowerDriver):
+    @staticmethod
+    def get_description() -> str:
+        return "Concrete Abstract power driver"
+
+    @staticmethod
+    def get_schema() -> str:
+        return "{}"
+
+    @staticmethod
+    def supports_probe() -> bool:
+        return False
+
+    @staticmethod
+    def supports_boot_ordering() -> bool:
+        return False
+
+    def power_on(self) -> None:
+        pass
+
+    def power_off(self) -> None:
+        pass
+
+    def power_query(self) -> bool:
+        pass
+
+
+def run():
+    sys.exit(pd_main(ConcreteAbstractPowerDriver))
+
+
+if __name__ == "__main__":
+    run()
diff --git a/powerdrivers/abstract/pyproject.toml b/powerdrivers/abstract/pyproject.toml
new file mode 100644
index 0000000..9fb59cc
--- /dev/null
+++ b/powerdrivers/abstract/pyproject.toml
@@ -0,0 +1,28 @@
+[build-system]
+# With setuptools 50.0.0, 'make .ve' fails.
+requires = ["setuptools < 50.0.0", "wheel"]
+build-backend = "setuptools.build_meta"
+
+[tool.black]
+line-length = 79
+exclude = """
+/.egg
+/.git
+/.mypy_cache
+"""
+
+[tool.isort]
+from_first = false
+force_sort_within_sections = true
+profile = "black"
+line_length = 79
+known_first_party = """
+
+"""
+order_by_type = false
+
+[tool.pytest.ini_options]
+filterwarnings = "error::BytesWarning"
+testpaths = [
+  "src",
+]
diff --git a/powerdrivers/abstract/setup.cfg b/powerdrivers/abstract/setup.cfg
new file mode 100644
index 0000000..dfc96a5
--- /dev/null
+++ b/powerdrivers/abstract/setup.cfg
@@ -0,0 +1,63 @@
+[metadata]
+name = maas-powerdriver-abstract
+version = 1.0.0
+description = Concrete Abstract power driver of MAAS (Metal As A Service)
+url = https://maas.io/
+license = AGPLv3
+author = MAAS Developers
+author_email = maas-devel@xxxxxxxxxxxxxxxxxxx
+classifiers =
+  Development Status :: 5 - Production/Stable
+  Intended Audience :: Information Technology
+  Intended Audience :: System Administrators
+  License :: OSI Approved :: GNU Affero General Public License v3
+  Operating System :: POSIX :: Linux
+  Programming Language :: Python :: 3
+  Topic :: System :: Systems Administration
+
+[options]
+packages = find:
+install_requires =
+  maaspd
+
+[options.entry_points]
+console_scripts =
+  maas-powerdriver-abstract = pd_abstract.driver:run
+
+[globals]
+lint_files =
+  setup.py
+  src/
+
+cog_files =
+  pyproject.toml
+
+deps_lint =
+  black == 22.6.0
+  flake8 == 5.0.4
+  isort == 5.7.0
+  cogapp == 3.3.0
+  click == 8.1.3
+
+
+[flake8]
+ignore = E203, E266, E501, W503, W504
+
+[tox:tox]
+skipsdist = True
+envlist = format,lint
+
+[testenv:format]
+deps = {[globals]deps_lint}
+commands =
+  isort {[globals]lint_files}
+  black -q {[globals]lint_files}
+  cog -r --verbosity=1 {[globals]cog_files}
+
+[testenv:lint]
+deps = {[globals]deps_lint}
+commands =
+  isort --check-only --diff {[globals]lint_files}
+  black --check {[globals]lint_files}
+  flake8 {[globals]lint_files}
+  cog --check --verbosity=1 {[globals]cog_files}
diff --git a/powerdrivers/abstract/setup.py b/powerdrivers/abstract/setup.py
new file mode 100644
index 0000000..0bde62d
--- /dev/null
+++ b/powerdrivers/abstract/setup.py
@@ -0,0 +1,5 @@
+import os
+
+from setuptools import setup
+
+setup()
diff --git a/powerdrivers/common/README.md b/powerdrivers/common/README.md
new file mode 100644
index 0000000..c806dac
--- /dev/null
+++ b/powerdrivers/common/README.md
@@ -0,0 +1,53 @@
+# MAAS Power driver wrapper
+
+This package contains generic classes and functions useful for any power driver implementation.
+
+Specifically, it has a `PowerDriver` class that outlines the actions MAAS expects from a power driver.  It also provides 
+`pd_main` method that handles the argument parsing and result formatting according to power driver schema. Finally, it 
+includes `PowerDriverException` child classes that can be easily serialised to a dict that conforms to the MAAS schema.
+
+### Minimal example of a power driver
+```python
+import sys
+
+from maaspd import pd_main
+from maaspd.classes import PowerDriver
+
+
+class ConcreteAbstractPowerDriver(PowerDriver):
+    @staticmethod
+    def get_description() -> str:
+        return "Concrete Abstract power driver"
+
+    @staticmethod
+    def get_schema() -> str:
+        return "{}"
+
+    @staticmethod
+    def supports_probe() -> bool:
+        return False
+
+    @staticmethod
+    def supports_boot_ordering() -> bool:
+        return False
+
+    def power_on(self) -> None:
+        pass
+
+    def power_off(self) -> None:
+        pass
+
+    def power_query(self) -> bool:
+        pass
+
+
+def run():
+    sys.exit(pd_main(ConcreteAbstractPowerDriver))
+
+
+if __name__ == "__main__":
+    run()
+
+```
+
+In this example, `run` is an entry point for `maas-powerdriver-abstract` command.
\ No newline at end of file
diff --git a/powerdrivers/common/maaspd/__init__.py b/powerdrivers/common/maaspd/__init__.py
new file mode 100644
index 0000000..947c57f
--- /dev/null
+++ b/powerdrivers/common/maaspd/__init__.py
@@ -0,0 +1,3 @@
+from .main import pd_main
+
+__all__ = ["pd_main"]
diff --git a/powerdrivers/common/maaspd/classes.py b/powerdrivers/common/maaspd/classes.py
new file mode 100644
index 0000000..53b2ba0
--- /dev/null
+++ b/powerdrivers/common/maaspd/classes.py
@@ -0,0 +1,112 @@
+from abc import ABC, abstractmethod
+from dataclasses import dataclass
+from typing import Dict, Sequence
+
+
+@dataclass
+class PowerDriverProbedSystem:
+    """Dataclass for `probe` action results"""
+
+    hostname: str
+    mac: str
+    arch: str
+    driver_name: str
+    driver_parameters: Dict
+
+
+@dataclass
+class PowerDriverNetworkBootDevice:
+    """Network boot device object passed to `set_boot_order` action handler"""
+
+    # NIC MAC address
+    mac: str
+
+
+@dataclass
+class PowerDriverStorageBootDevice:
+    """Storage boot device object passed to `set_boot_order` action handler"""
+
+    # Storage serial number
+    serial: str
+
+
+class PowerDriver(ABC):
+    """
+    Abstract base class for MAAS power drivers.
+
+    Please note that driver parameters will be passed as kwargs
+    to the class initializer when using common `pd_main`.
+    """
+
+    @staticmethod
+    @abstractmethod
+    def get_description() -> str:
+        """Returns a description line `argparse` will use with a suffix"""
+
+    @staticmethod
+    @abstractmethod
+    def get_schema() -> str:
+        """Returns a valid JSON-string with the MAAS power driver schema"""
+
+    @staticmethod
+    @abstractmethod
+    def supports_probe() -> bool:
+        """Returns True when power driver supports `probe` action.
+
+        When using `pd_main`, the action will still be available,
+        but `PowerDriverUnsupportedAction` will be raised and printed
+        as a JSON
+        """
+        return False
+
+    @staticmethod
+    @abstractmethod
+    def supports_boot_ordering() -> bool:
+        """Returns True when power driver supports `set_boot_order` action.
+
+        When using `pd_main`, the action will still be available,
+        but `PowerDriverUnsupportedAction` will be raised and printed
+        as a JSON"""
+        return False
+
+    @abstractmethod
+    def power_on(self) -> None:
+        """A method that will be called for `power_on` action.
+
+        Should power machine on or throw PowerDriverError
+        child exception that reflects the failure reason.
+        """
+
+    @abstractmethod
+    def power_off(self) -> None:
+        """A method that will be called for `power_off` action.
+
+        Should power machine off or throw PowerDriverError
+        child exception that reflects the failure reason."""
+
+    @abstractmethod
+    def power_query(self) -> bool:
+        """A method that will be called for `power_on` action.
+
+        Should return 'True' when machine is on, 'False' when machine is off
+        or throw PowerDriverError child exception that reflects the failure reason.
+        """
+
+    def probe(self) -> Sequence[PowerDriverProbedSystem]:
+        """A method that will be called for `probe` action.
+
+        Should return an iterable (possibly empty) with PowerDriverProbedSystem
+        or throw PowerDriverError child exception that reflects the failure reason.
+        """
+        return []
+
+    def set_boot_order(
+        self,
+        devices: Sequence[
+            PowerDriverNetworkBootDevice | PowerDriverStorageBootDevice
+        ],
+    ) -> None:
+        """A method that will be called for `set_boot_order` action.
+
+        Should update boot order or throw PowerDriverError
+        child exception that reflects the failure reason."""
diff --git a/powerdrivers/common/maaspd/exc.py b/powerdrivers/common/maaspd/exc.py
new file mode 100644
index 0000000..6661a2a
--- /dev/null
+++ b/powerdrivers/common/maaspd/exc.py
@@ -0,0 +1,98 @@
+import enum
+
+
+class PowerDriverErrorCode(enum.Enum):
+    FATAL_ERROR = "fatalError"
+    SETTINGS_ERROR = "settingsError"
+    TOOL_ERROR = "toolError"
+    AUTH_ERROR = "authError"
+    CONNECTION_ERROR = "connError"
+    ACTION_ERROR = "actionError"
+    UNSUPPORTED_ACTION = "unsupportedActionError"
+
+
+class PowerDriverError(Exception):
+    """Base class for power driver exceptions"""
+
+    code: PowerDriverErrorCode = None
+
+    def __init__(self, message="No message provided"):
+        self.message = message
+        super().__init__(message)
+
+    def __json__(self) -> dict:
+        """Helper that returns dict representation of an exception
+        in a format described by PowerDriverError schema"""
+        return {"code": self.code.value, "message": self.message}
+
+
+class PowerDriverFatalError(PowerDriverError):
+    """Error that is raised when the power action should not continue to
+    retry at all.
+
+    This exception will cause the power action to fail instantly,
+    without retrying.
+    """
+
+    code = PowerDriverErrorCode.FATAL_ERROR
+
+
+class PowerDriverSettingsError(PowerDriverError):
+    """Error that is raised when the power type is missing argument
+    that is required to control the BMC.
+
+    This exception will cause the power action to fail instantly,
+    without retrying.
+    """
+
+    code = PowerDriverErrorCode.SETTINGS_ERROR
+
+
+class PowerDriverToolError(PowerDriverError):
+    """Error that is raised when the power tool is missing completely
+    for use.
+
+    This exception will cause the power action to fail instantly,
+    without retrying.
+    """
+
+    code = PowerDriverErrorCode.TOOL_ERROR
+
+
+class PowerDriverAuthError(PowerDriverError):
+    """Error raised when power driver fails to authenticate to BMC.
+
+    This exception will cause the power action to fail instantly,
+    without retrying.
+    """
+
+    code = PowerDriverErrorCode.AUTH_ERROR
+
+
+class PowerDriverConnError(PowerDriverError):
+    """Error raised when power driver fails to communicate to BMC.
+
+    This exception will cause a retry attempt later unless retries limit is reached.
+    """
+
+    code = PowerDriverErrorCode.CONNECTION_ERROR
+
+
+class PowerDriverActionError(PowerDriverError):
+    """Error when actually performing an action on the BMC, like `on`
+    or `off`.
+
+    This exception will cause a retry attempt later unless retries limit is reached.
+    """
+
+    code = PowerDriverErrorCode.ACTION_ERROR
+
+
+class PowerDriverUnsupportedActionError(PowerDriverError):
+    """Error raised when requested power driver action is unsupported.
+
+    This exception will cause the power action to fail instantly,
+    without retrying.
+    """
+
+    code = PowerDriverErrorCode.UNSUPPORTED_ACTION
diff --git a/powerdrivers/common/maaspd/main.py b/powerdrivers/common/maaspd/main.py
new file mode 100644
index 0000000..8961364
--- /dev/null
+++ b/powerdrivers/common/maaspd/main.py
@@ -0,0 +1,203 @@
+import argparse
+import enum
+import json
+from typing import Type, Dict, List
+
+from .classes import (
+    PowerDriverNetworkBootDevice,
+    PowerDriverStorageBootDevice,
+    PowerDriver,
+)
+from .exc import (
+    PowerDriverError,
+    PowerDriverUnsupportedActionError,
+    PowerDriverFatalError,
+)
+
+
+class _PowerAction(enum.Enum):
+    POWER_ON = "power_on"
+    POWER_OFF = "power_off"
+    POWER_QUERY = "power_query"
+    PROBE = "probe"
+    SET_BOOT_ORDER = "set_boot_order"
+
+
+def _prepare_set_boot_order_devices(
+    params: dict,
+) -> List[PowerDriverNetworkBootDevice | PowerDriverStorageBootDevice]:
+    """Converts the devices list of dicts to a matching list of dataclasses"""
+    result = []
+    for device in params["devices"]:
+        device_type = device["deviceType"]
+        if device_type == "NetworkInterface":
+            clazz = PowerDriverNetworkBootDevice
+        elif device_type == "StorageDevice":
+            clazz = PowerDriverStorageBootDevice
+        else:
+            raise PowerDriverFatalError(f'Unknown device type "{device_type}"')
+        result.append(clazz(**device["properties"]))
+    return result
+
+
+def _dispatch_action(power_driver: PowerDriver, action: str, parameters: dict):
+    """Calls the method of power_driver that performs the requested action"""
+    if action == _PowerAction.POWER_ON.value:
+        power_driver.power_on()
+    elif action == _PowerAction.POWER_OFF.value:
+        power_driver.power_off()
+    elif action == _PowerAction.POWER_QUERY.value:
+        return {
+            "status": "on" if power_driver.power_query() else "off"
+        }
+    elif action == _PowerAction.PROBE.value:
+        return {
+            "hosts": power_driver.probe()
+        }
+    elif action == _PowerAction.SET_BOOT_ORDER.value:
+        power_driver.set_boot_order(
+            _prepare_set_boot_order_devices(parameters)
+        )
+    else:
+        raise PowerDriverUnsupportedActionError(f"Unknown action {action}")
+
+
+def _handle_action(power_driver_cls: Type[PowerDriver], args) -> Dict:
+    """Instantiates a power driver from provided class and
+    calls the method matching the requested action. Returns a dict
+    that conforms to the MAAS power driver schema
+    """
+    action: str = args.action
+    # With MAAS power drivers, we expect that stdin contains a valid JSON with parameters
+    parameters: dict = json.load(args.input)
+    assert isinstance(
+        parameters, dict
+    ), f"Expected stdin parameters to be a dict, `{type(parameters)}` deserialised instead"
+
+    response = {"action_type": action, "error": None}
+
+    try:
+        # Since instantiating a power driver class
+        if (
+            action == _PowerAction.PROBE.value
+            and not power_driver_cls.supports_probe()
+        ) or (
+            action == _PowerAction.SET_BOOT_ORDER.value
+            and not power_driver_cls.supports_boot_ordering()
+        ):
+            raise PowerDriverUnsupportedActionError(
+                f'"{action}" action is not supported for this power driver'
+            )
+
+        instance = power_driver_cls(**parameters["driverParameters"])
+        call_result = _dispatch_action(instance, action, parameters)
+        if isinstance(call_result, dict):
+            response["data"] = call_result
+    except PowerDriverError as e:
+        # We don't want to handle all errors since python interpreter is smart enough
+        # to print unhandled exceptions tracebacks to stderr
+        response["error"] = e.__json__()
+
+    return response
+
+
+def _input_subparser(subparsers, action, help):
+    """Helper function to register a subparser with 'input' argument"""
+    result = subparsers.add_parser(
+        action,
+        help=help,
+        formatter_class=argparse.ArgumentDefaultsHelpFormatter,
+    )
+    result.add_argument(
+        "input",
+        type=argparse.FileType("r"),
+        nargs='?',
+        default='-',
+        help="File to read parameters from",
+    )
+    return result
+
+
+def _mark_unsupported(line: str, unsupported: bool):
+    """Adds an '(Not supported by this driver)' suffix to an argparse help line"""
+    suffix = " (Not supported by this driver)" if unsupported else ""
+    return line + suffix
+
+
+class _PowerDriverJSONEncoder(json.JSONEncoder):
+    def default(self, obj):
+        json_magic = obj.getattr("__json__", None)
+        if json_magic and callable(json_magic):
+            result = json_magic()
+            return result
+        return json.JSONEncoder.default(self, obj)
+
+
+def pd_main(power_driver_cls: Type[PowerDriver]) -> int:
+    """
+    Common main() function for all MAAS power drivers.
+
+    :param power_driver_cls: Power driver class inherited from `PowerDriver`.
+                             Note that the *class initializer must accept power
+                             driver parameters values as kwargs*.
+
+    :return exit code
+    :rtype: int
+    """
+    parser = argparse.ArgumentParser(
+        description=f"{power_driver_cls.get_description()} — A power driver used in MAAS",
+        formatter_class=argparse.ArgumentDefaultsHelpFormatter,
+    )
+
+    subparsers = parser.add_subparsers(dest="action")
+    subparsers.required = True
+
+    subparsers.add_parser("schema", help="Prints schema")
+
+    _input_subparser(
+        subparsers,
+        _PowerAction.POWER_ON.value,
+        help="Send power on command to the machine",
+    )
+    _input_subparser(
+        subparsers,
+        _PowerAction.POWER_OFF.value,
+        help="Send power off command to the machine",
+    )
+    _input_subparser(
+        subparsers,
+        _PowerAction.POWER_QUERY.value,
+        help="Query machine power state",
+    )
+    _input_subparser(
+        subparsers,
+        _PowerAction.PROBE.value,
+        help=_mark_unsupported(
+            "Probe chassis for a list of controlled machines",
+            not power_driver_cls.supports_probe(),
+        ),
+    )
+    _input_subparser(
+        subparsers,
+        _PowerAction.SET_BOOT_ORDER.value,
+        help=_mark_unsupported(
+            "Reorder boot sources",
+            not power_driver_cls.supports_boot_ordering(),
+        ),
+    )
+
+    args = parser.parse_args()
+
+    if args.action == "schema":
+        print(power_driver_cls.get_schema())
+    elif args.action in [e.value for e in _PowerAction]:
+        result = _handle_action(power_driver_cls, args)
+        print(
+            json.dumps(
+                result, indent=4, sort_keys=True, cls=_PowerDriverJSONEncoder
+            )
+        )
+    else:
+        raise NotImplementedError()
+
+    return 0
diff --git a/powerdrivers/common/pyproject.toml b/powerdrivers/common/pyproject.toml
new file mode 100644
index 0000000..9fb59cc
--- /dev/null
+++ b/powerdrivers/common/pyproject.toml
@@ -0,0 +1,28 @@
+[build-system]
+# With setuptools 50.0.0, 'make .ve' fails.
+requires = ["setuptools < 50.0.0", "wheel"]
+build-backend = "setuptools.build_meta"
+
+[tool.black]
+line-length = 79
+exclude = """
+/.egg
+/.git
+/.mypy_cache
+"""
+
+[tool.isort]
+from_first = false
+force_sort_within_sections = true
+profile = "black"
+line_length = 79
+known_first_party = """
+
+"""
+order_by_type = false
+
+[tool.pytest.ini_options]
+filterwarnings = "error::BytesWarning"
+testpaths = [
+  "src",
+]
diff --git a/powerdrivers/common/setup.cfg b/powerdrivers/common/setup.cfg
new file mode 100644
index 0000000..6fb9237
--- /dev/null
+++ b/powerdrivers/common/setup.cfg
@@ -0,0 +1,61 @@
+[metadata]
+name = maaspd
+version = 1.0.0
+description = Wrapper for power drivers of MAAS (Metal As A Service)
+url = https://maas.io/
+license = AGPLv3
+author = MAAS Developers
+author_email = maas-devel@xxxxxxxxxxxxxxxxxxx
+classifiers =
+  Development Status :: 5 - Production/Stable
+  Intended Audience :: Information Technology
+  Intended Audience :: System Administrators
+  License :: OSI Approved :: GNU Affero General Public License v3
+  Operating System :: POSIX :: Linux
+  Programming Language :: Python :: 3
+  Topic :: System :: Systems Administration
+
+[options]
+packages = find:
+
+[options.packages.find]
+exclude =
+  *.tests
+
+[globals]
+lint_files =
+  setup.py
+  maaspd/
+
+cog_files =
+  pyproject.toml
+
+deps_lint =
+  black == 22.6.0
+  flake8 == 5.0.4
+  isort == 5.7.0
+  cogapp == 3.3.0
+  click == 8.1.3
+
+
+[flake8]
+ignore = E203, E266, E501, W503, W504
+
+[tox:tox]
+skipsdist = True
+envlist = format,lint
+
+[testenv:format]
+deps = {[globals]deps_lint}
+commands =
+  isort {[globals]lint_files}
+  black -q {[globals]lint_files}
+  cog -r --verbosity=1 {[globals]cog_files}
+
+[testenv:lint]
+deps = {[globals]deps_lint}
+commands =
+  isort --check-only --diff {[globals]lint_files}
+  black --check {[globals]lint_files}
+  flake8 {[globals]lint_files}
+  cog --check --verbosity=1 {[globals]cog_files}
diff --git a/powerdrivers/common/setup.py b/powerdrivers/common/setup.py
new file mode 100644
index 0000000..6068493
--- /dev/null
+++ b/powerdrivers/common/setup.py
@@ -0,0 +1,3 @@
+from setuptools import setup
+
+setup()