From 565a0ed6b940df2d45172065840700bbc3802c99 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Fri, 4 Nov 2016 17:40:42 +0100 Subject: [PATCH] Simplify base interfaces in ironic.drivers.base Move abstract definitions of get_properties and validate to BaseInterface and make all interfaces inherit it. This has a side effect of them getting clean steps methods, but they anyway won't be wired in due to a whitelist of interfaces in the conductor. The RAIDInterface contained a mistake: it didn't have an ABCMeta metaclass, so its @abstractmethod definitions didn't work. I've implemented get_properties for it to avoid breaking implementers. Also added interface_type to all interfaces for consistency and for future use in the driver composition work. Change-Id: Ia6708247a99ecdcf73d83e66350e8c309090b7d4 Partial-Bug: #1524745 --- ironic/drivers/base.py | 196 +++++-------------------- ironic/tests/unit/drivers/test_base.py | 13 +- 2 files changed, 46 insertions(+), 163 deletions(-) diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index d7b114bedc..b510fef163 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -171,6 +171,7 @@ class BareDriver(BaseDriver): self.core_interfaces.append('network') +@six.add_metaclass(abc.ABCMeta) class BaseInterface(object): """A base interface implementing common functions for Driver Interfaces.""" @@ -182,6 +183,30 @@ class BaseInterface(object): """ interface_type = 'base' + """Interface type, used for clean steps and logging.""" + + @abc.abstractmethod + def get_properties(self): + """Return the properties of the interface. + + :returns: dictionary of : entries. + """ + + @abc.abstractmethod + def validate(self, task): + """Validate the driver-specific Node deployment info. + + This method validates whether the 'driver_info' and/or 'instance_info' + properties of the task's node contains the required information for + this interface to function. + + This method is often executed synchronously in API requests, so it + should not conduct long-running checks. + + :param task: a TaskManager instance containing the node to act on. + :raises: InvalidParameterValue on malformed parameter(s) + :raises: MissingParameterValue on missing parameter(s) + """ def __new__(cls, *args, **kwargs): # Get the list of clean steps when the interface is initialized by @@ -252,32 +277,10 @@ class BaseInterface(object): return getattr(self, step['step'])(task) -@six.add_metaclass(abc.ABCMeta) class DeployInterface(BaseInterface): """Interface for deploy-related actions.""" interface_type = 'deploy' - @abc.abstractmethod - def get_properties(self): - """Return the properties of the interface. - - :returns: dictionary of : entries. - """ - - @abc.abstractmethod - def validate(self, task): - """Validate the driver-specific Node deployment info. - - This method validates whether the 'driver_info' property of the - task's node contains the required information for this driver to - deploy images to the node. If invalid, raises an exception; otherwise - returns None. - - :param task: a TaskManager instance containing the node to act on. - :raises: InvalidParameterValue - :raises: MissingParameterValue - """ - @abc.abstractmethod def deploy(self, task): """Perform a deployment to the task's node. @@ -402,30 +405,9 @@ class DeployInterface(BaseInterface): {'node': task.node.uuid, 'driver': task.node.driver}) -@six.add_metaclass(abc.ABCMeta) -class BootInterface(object): +class BootInterface(BaseInterface): """Interface for boot-related actions.""" - - @abc.abstractmethod - def get_properties(self): - """Return the properties of the interface. - - :returns: dictionary of : entries. - """ - - @abc.abstractmethod - def validate(self, task): - """Validate the driver-specific info for booting. - - This method validates the driver-specific info for booting the - ramdisk and instance on the node. If invalid, raises an - exception; otherwise returns None. - - :param task: a task from TaskManager. - :returns: None - :raises: InvalidParameterValue - :raises: MissingParameterValue - """ + interface_type = 'boot' @abc.abstractmethod def prepare_ramdisk(self, task, ramdisk_params): @@ -482,32 +464,10 @@ class BootInterface(object): """ -@six.add_metaclass(abc.ABCMeta) class PowerInterface(BaseInterface): """Interface for power-related actions.""" interface_type = 'power' - @abc.abstractmethod - def get_properties(self): - """Return the properties of the interface. - - :returns: dictionary of : entries. - """ - - @abc.abstractmethod - def validate(self, task): - """Validate the driver-specific Node power info. - - This method validates whether the 'driver_info' property of the - supplied node contains the required information for this driver to - manage the power state of the node. If invalid, raises an exception; - otherwise, returns None. - - :param task: a TaskManager instance containing the node to act on. - :raises: InvalidParameterValue - :raises: MissingParameterValue - """ - @abc.abstractmethod def get_power_state(self, task): """Return the power state of the task's node. @@ -538,30 +498,9 @@ class PowerInterface(BaseInterface): """ -@six.add_metaclass(abc.ABCMeta) -class ConsoleInterface(object): +class ConsoleInterface(BaseInterface): """Interface for console-related actions.""" - - @abc.abstractmethod - def get_properties(self): - """Return the properties of the interface. - - :returns: dictionary of : entries. - """ - - @abc.abstractmethod - def validate(self, task): - """Validate the driver-specific Node console info. - - This method validates whether the 'driver_info' property of the - supplied node contains the required information for this driver to - provide console access to the Node. If invalid, raises an exception; - otherwise returns None. - - :param task: a TaskManager instance containing the node to act on. - :raises: InvalidParameterValue - :raises: MissingParameterValue - """ + interface_type = "console" @abc.abstractmethod def start_console(self, task): @@ -591,27 +530,9 @@ class ConsoleInterface(object): """ -@six.add_metaclass(abc.ABCMeta) -class RescueInterface(object): +class RescueInterface(BaseInterface): """Interface for rescue-related actions.""" - - @abc.abstractmethod - def get_properties(self): - """Return the properties of the interface. - - :returns: dictionary of : entries. - """ - - @abc.abstractmethod - def validate(self, task): - """Validate the rescue info stored in the node' properties. - - If invalid, raises an exception; otherwise returns None. - - :param task: a TaskManager instance containing the node to act on. - :raises: InvalidParameterValue - :raises: MissingParameterValue - """ + interface_type = "rescue" @abc.abstractmethod def rescue(self, task): @@ -714,8 +635,7 @@ def driver_passthru(http_methods, method=None, async=True, description=None, description=description, attach=attach) -@six.add_metaclass(abc.ABCMeta) -class VendorInterface(object): +class VendorInterface(BaseInterface): """Interface for all vendor passthru functionality. Additional vendor- or driver-specific capabilities should be @@ -725,6 +645,7 @@ class VendorInterface(object): Methods decorated with @driver_passthru should be short-lived because it is a blocking call. """ + interface_type = "vendor" def __new__(cls, *args, **kwargs): super_new = super(VendorInterface, cls).__new__ @@ -752,13 +673,6 @@ class VendorInterface(object): return inst - @abc.abstractmethod - def get_properties(self): - """Return the properties of the interface. - - :returns: dictionary of : entries. - """ - @abc.abstractmethod def validate(self, task, method=None, **kwargs): """Validate vendor-specific actions. @@ -788,29 +702,10 @@ class VendorInterface(object): pass -@six.add_metaclass(abc.ABCMeta) class ManagementInterface(BaseInterface): """Interface for management related actions.""" interface_type = 'management' - @abc.abstractmethod - def get_properties(self): - """Return the properties of the interface. - - :returns: dictionary of : entries. - """ - - @abc.abstractmethod - def validate(self, task): - """Validate the driver-specific management information. - - If invalid, raises an exception; otherwise returns None. - - :param task: a task from TaskManager. - :raises: InvalidParameterValue - :raises: MissingParameterValue - """ - @abc.abstractmethod def get_supported_boot_devices(self, task): """Get a list of the supported boot devices. @@ -899,31 +794,13 @@ class ManagementInterface(BaseInterface): """ -@six.add_metaclass(abc.ABCMeta) -class InspectInterface(object): +class InspectInterface(BaseInterface): """Interface for inspection-related actions.""" + interface_type = 'inspect' ESSENTIAL_PROPERTIES = {'memory_mb', 'local_gb', 'cpus', 'cpu_arch'} """The properties required by scheduler/deploy.""" - @abc.abstractmethod - def get_properties(self): - """Return the properties of the interface. - - :returns: dictionary of : entries. - """ - - @abc.abstractmethod - def validate(self, task): - """Validate the driver-specific inspection information. - - If invalid, raises an exception; otherwise returns None. - - :param task: a task from TaskManager. - :raises: InvalidParameterValue - :raises: MissingParameterValue - """ - @abc.abstractmethod def inspect_hardware(self, task): """Inspect hardware. @@ -947,12 +824,12 @@ class RAIDInterface(BaseInterface): with open(RAID_CONFIG_SCHEMA, 'r') as raid_schema_fobj: self.raid_schema = json.load(raid_schema_fobj) - @abc.abstractmethod def get_properties(self): """Return the properties of the interface. :returns: dictionary of : entries. """ + return {} def validate(self, task): """Validates the RAID Interface. @@ -1035,7 +912,6 @@ class RAIDInterface(BaseInterface): return raid.get_logical_disk_properties(self.raid_schema) -@six.add_metaclass(abc.ABCMeta) class NetworkInterface(BaseInterface): """Base class for network interfaces.""" diff --git a/ironic/tests/unit/drivers/test_base.py b/ironic/tests/unit/drivers/test_base.py index d6b48250ea..cc9d4b0c95 100644 --- a/ironic/tests/unit/drivers/test_base.py +++ b/ironic/tests/unit/drivers/test_base.py @@ -226,7 +226,14 @@ class CleanStepTestCase(base.TestCase): method_args_mock = mock.MagicMock(spec_set=[]) task_mock = mock.MagicMock(spec_set=[]) - class TestClass(driver_base.BaseInterface): + class BaseTestClass(driver_base.BaseInterface): + def get_properties(self): + return {} + + def validate(self, task): + pass + + class TestClass(BaseTestClass): interface_type = 'test' @driver_base.clean_step(priority=0) @@ -240,7 +247,7 @@ class CleanStepTestCase(base.TestCase): def not_clean_method(self, task): pass - class TestClass2(driver_base.BaseInterface): + class TestClass2(BaseTestClass): interface_type = 'test2' @driver_base.clean_step(priority=0) @@ -254,7 +261,7 @@ class CleanStepTestCase(base.TestCase): def not_clean_method2(self, task): pass - class TestClass3(driver_base.BaseInterface): + class TestClass3(BaseTestClass): interface_type = 'test3' @driver_base.clean_step(priority=0, abortable=True, argsinfo={