From d1062cfbdd3fdacc4e4bd807f1508f90557bef65 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 31 Jan 2018 16:40:23 +0100 Subject: [PATCH] Migrate the remaining classic drivers to hardware types I've tried my best to provide a correct matching for these, but there are two exceptions: * fake_drac uses iscsi deploy with None boot, which cannot work, so changing both to fake * some fake drivers are migrated to the 'fake' power interface, which is changed to include soft power as well. The latter means that all fake classic drivers now support fake soft power actions, while previously only fake_soft_power did. Now fake_soft_power is identical to fake and is left for backward compatibility Also wrote more tests to check correctness of this migration. Change-Id: I00c9c6ed698b10f035e65428e1a20d733c7e544a Partial-Bug: #1690185 --- ironic/drivers/agent.py | 34 +++++++++ ironic/drivers/drac.py | 31 ++++++++ ironic/drivers/fake.py | 72 ++++++++++++++++-- ironic/drivers/ilo.py | 21 ++++++ ironic/drivers/irmc.py | 18 +++++ ironic/drivers/modules/fake.py | 15 +--- ironic/drivers/oneview.py | 35 +++++++++ ironic/drivers/pxe.py | 54 ++++++++++++++ ironic/tests/unit/drivers/test_base.py | 73 ++++++++++++++++++- .../fake_soft_power-32683a848a989fc2.yaml | 5 ++ 10 files changed, 338 insertions(+), 20 deletions(-) create mode 100644 releasenotes/notes/fake_soft_power-32683a848a989fc2.yaml diff --git a/ironic/drivers/agent.py b/ironic/drivers/agent.py index 75502fd5dd..163ae796eb 100644 --- a/ironic/drivers/agent.py +++ b/ironic/drivers/agent.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from oslo_config import cfg from oslo_utils import importutils from ironic.common import exception @@ -27,6 +28,9 @@ from ironic.drivers.modules.ucs import management as ucs_mgmt from ironic.drivers.modules.ucs import power as ucs_power +CONF = cfg.CONF + + # For backward compatibility AgentAndIPMIToolDriver = ipmi.AgentAndIPMIToolDriver AgentAndIPMIToolAndSocatDriver = ipmi.AgentAndIPMIToolAndSocatDriver @@ -55,6 +59,21 @@ class AgentAndUcsDriver(base.BaseDriver): self.inspect = inspector.Inspector.create_if_enabled( 'AgentAndUcsDriver') + @classmethod + def to_hardware_type(cls): + # NOTE(dtantsur): classic drivers are not affected by the + # enabled_inspect_interfaces configuration option. + if CONF.inspector.enabled: + inspect_interface = 'inspector' + else: + inspect_interface = 'no-inspect' + + return 'cisco-ucs-managed', {'boot': 'pxe', + 'deploy': 'direct', + 'inspect': inspect_interface, + 'management': 'ucsm', + 'power': 'ucsm'} + class AgentAndCIMCDriver(base.BaseDriver): """Agent + Cisco CIMC driver. @@ -78,3 +97,18 @@ class AgentAndCIMCDriver(base.BaseDriver): self.management = cimc_mgmt.CIMCManagement() self.inspect = inspector.Inspector.create_if_enabled( 'AgentAndCIMCDriver') + + @classmethod + def to_hardware_type(cls): + # NOTE(dtantsur): classic drivers are not affected by the + # enabled_inspect_interfaces configuration option. + if CONF.inspector.enabled: + inspect_interface = 'inspector' + else: + inspect_interface = 'no-inspect' + + return 'cisco-ucs-standalone', {'boot': 'pxe', + 'deploy': 'direct', + 'inspect': inspect_interface, + 'management': 'cimc', + 'power': 'cimc'} diff --git a/ironic/drivers/drac.py b/ironic/drivers/drac.py index 7fedaee273..2b44c1e5f0 100644 --- a/ironic/drivers/drac.py +++ b/ironic/drivers/drac.py @@ -15,6 +15,7 @@ DRAC Driver for remote system management using Dell Remote Access Card. """ +from oslo_config import cfg from oslo_utils import importutils from ironic.common import exception @@ -32,6 +33,9 @@ from ironic.drivers.modules import noop from ironic.drivers.modules import pxe +CONF = cfg.CONF + + class IDRACHardware(generic.GenericHardware): """integrated Dell Remote Access Controller hardware type""" @@ -85,6 +89,16 @@ class PXEDracDriver(base.BaseDriver): self.vendor = vendor_passthru.DracVendorPassthru() self.inspect = drac_inspect.DracInspect() + @classmethod + def to_hardware_type(cls): + return 'idrac', {'boot': 'pxe', + 'deploy': 'iscsi', + 'inspect': 'idrac', + 'management': 'idrac', + 'power': 'idrac', + 'raid': 'idrac', + 'vendor': 'idrac'} + class PXEDracInspectorDriver(PXEDracDriver): """Drac driver using PXE for deploy and OOB inspection interface.""" @@ -93,3 +107,20 @@ class PXEDracInspectorDriver(PXEDracDriver): super(PXEDracInspectorDriver, self).__init__() self.inspect = inspector.Inspector.create_if_enabled( 'PXEDracInspectorDriver') + + @classmethod + def to_hardware_type(cls): + # NOTE(dtantsur): classic drivers are not affected by the + # enabled_inspect_interfaces configuration option. + if CONF.inspector.enabled: + inspect_interface = 'inspector' + else: + inspect_interface = 'no-inspect' + + return 'idrac', {'boot': 'pxe', + 'deploy': 'iscsi', + 'inspect': inspect_interface, + 'management': 'idrac', + 'power': 'idrac', + 'raid': 'idrac', + 'vendor': 'idrac'} diff --git a/ironic/drivers/fake.py b/ironic/drivers/fake.py index 2592828090..219692f9fb 100644 --- a/ironic/drivers/fake.py +++ b/ironic/drivers/fake.py @@ -80,10 +80,8 @@ class FakeDriver(base.BaseDriver): class FakeSoftPowerDriver(FakeDriver): """Example implementation of a Driver.""" - - def __init__(self): - super(FakeSoftPowerDriver, self).__init__() - self.power = fake.FakeSoftPower() + # NOTE(dtantsur): identical to FakeDriver now, will be removed with other + # classic drivers class FakeIPMIToolDriver(base.BaseDriver): @@ -181,6 +179,16 @@ class FakeIloDriver(base.BaseDriver): self.management = ilo_management.IloManagement() self.inspect = ilo_inspect.IloInspect() + @classmethod + def to_hardware_type(cls): + return 'fake-hardware', { + 'boot': 'fake', + 'deploy': 'fake', + 'inspect': 'ilo', + 'management': 'ilo', + 'power': 'ilo' + } + class FakeDracDriver(base.BaseDriver): """Fake Drac driver.""" @@ -198,6 +206,21 @@ class FakeDracDriver(base.BaseDriver): self.vendor = drac_vendor.DracVendorPassthru() self.inspect = drac_inspect.DracInspect() + @classmethod + def to_hardware_type(cls): + return 'fake-hardware', { + 'boot': 'fake', + # NOTE(dtantsur): the classic driver uses boot=None and + # deploy=iscsi. This cannot work, so correcting it based on the + # intended purpose of these fake drivers. + 'deploy': 'fake', + 'inspect': 'idrac', + 'management': 'idrac', + 'power': 'idrac', + 'raid': 'idrac', + 'vendor': 'idrac' + } + class FakeSNMPDriver(base.BaseDriver): """Fake SNMP driver.""" @@ -212,7 +235,7 @@ class FakeSNMPDriver(base.BaseDriver): @classmethod def to_hardware_type(cls): - return 'snmp', { + return 'fake-hardware', { 'boot': 'fake', 'deploy': 'fake', 'management': 'fake', @@ -233,6 +256,16 @@ class FakeIRMCDriver(base.BaseDriver): self.management = irmc_management.IRMCManagement() self.inspect = irmc_inspect.IRMCInspect() + @classmethod + def to_hardware_type(cls): + return 'fake-hardware', { + 'boot': 'fake', + 'deploy': 'fake', + 'inspect': 'irmc', + 'management': 'irmc', + 'power': 'irmc' + } + class FakeIPMIToolInspectorDriver(base.BaseDriver): """Fake Inspector driver.""" @@ -257,6 +290,7 @@ class FakeIPMIToolInspectorDriver(base.BaseDriver): 'inspect': 'inspector', 'management': 'ipmitool', 'power': 'ipmitool', + 'vendor': 'ipmitool', } @@ -272,6 +306,15 @@ class FakeUcsDriver(base.BaseDriver): self.deploy = fake.FakeDeploy() self.management = ucs_mgmt.UcsManagement() + @classmethod + def to_hardware_type(cls): + return 'fake-hardware', { + 'boot': 'fake', + 'deploy': 'fake', + 'management': 'ucsm', + 'power': 'ucsm' + } + class FakeCIMCDriver(base.BaseDriver): """Fake CIMC driver.""" @@ -285,6 +328,15 @@ class FakeCIMCDriver(base.BaseDriver): self.deploy = fake.FakeDeploy() self.management = cimc_mgmt.CIMCManagement() + @classmethod + def to_hardware_type(cls): + return 'fake-hardware', { + 'boot': 'fake', + 'deploy': 'fake', + 'management': 'cimc', + 'power': 'cimc' + } + class FakeOneViewDriver(base.BaseDriver): """Fake OneView driver. For testing purposes.""" @@ -300,3 +352,13 @@ class FakeOneViewDriver(base.BaseDriver): self.boot = fake.FakeBoot() self.deploy = fake.FakeDeploy() self.inspect = fake.FakeInspect() + + @classmethod + def to_hardware_type(cls): + return 'fake-hardware', { + 'boot': 'fake', + 'deploy': 'fake', + 'inspect': 'fake', + 'management': 'oneview', + 'power': 'oneview' + } diff --git a/ironic/drivers/ilo.py b/ironic/drivers/ilo.py index 9d51ad50b5..09b53cef34 100644 --- a/ironic/drivers/ilo.py +++ b/ironic/drivers/ilo.py @@ -97,6 +97,17 @@ class IloVirtualMediaIscsiDriver(base.BaseDriver): self.inspect = inspect.IloInspect() self.raid = agent.AgentRAID() + @classmethod + def to_hardware_type(cls): + return 'ilo', {'boot': 'ilo-virtual-media', + 'console': 'ilo', + 'deploy': 'iscsi', + 'inspect': 'ilo', + 'management': 'ilo', + 'power': 'ilo', + 'raid': 'agent', + 'vendor': 'ilo'} + class IloVirtualMediaAgentDriver(base.BaseDriver): """IloDriver using IloClient interface. @@ -121,3 +132,13 @@ class IloVirtualMediaAgentDriver(base.BaseDriver): self.management = management.IloManagement() self.inspect = inspect.IloInspect() self.raid = agent.AgentRAID() + + @classmethod + def to_hardware_type(cls): + return 'ilo', {'boot': 'ilo-virtual-media', + 'console': 'ilo', + 'deploy': 'direct', + 'inspect': 'ilo', + 'management': 'ilo', + 'power': 'ilo', + 'raid': 'agent'} diff --git a/ironic/drivers/irmc.py b/ironic/drivers/irmc.py index 10fe664a62..4cf9315045 100644 --- a/ironic/drivers/irmc.py +++ b/ironic/drivers/irmc.py @@ -56,6 +56,15 @@ class IRMCVirtualMediaIscsiDriver(base.BaseDriver): self.management = management.IRMCManagement() self.inspect = inspect.IRMCInspect() + @classmethod + def to_hardware_type(cls): + return 'irmc', {'boot': 'irmc-virtual-media', + 'console': 'ipmitool-shellinabox', + 'deploy': 'iscsi', + 'inspect': 'irmc', + 'management': 'irmc', + 'power': 'irmc'} + class IRMCVirtualMediaAgentDriver(base.BaseDriver): """iRMC Driver using SCCI. @@ -80,6 +89,15 @@ class IRMCVirtualMediaAgentDriver(base.BaseDriver): self.management = management.IRMCManagement() self.inspect = inspect.IRMCInspect() + @classmethod + def to_hardware_type(cls): + return 'irmc', {'boot': 'irmc-virtual-media', + 'console': 'ipmitool-shellinabox', + 'deploy': 'direct', + 'inspect': 'irmc', + 'management': 'irmc', + 'power': 'irmc'} + class IRMCHardware(generic.GenericHardware): """iRMC hardware type. diff --git a/ironic/drivers/modules/fake.py b/ironic/drivers/modules/fake.py index 816cc730e4..dc37ef1e42 100644 --- a/ironic/drivers/modules/fake.py +++ b/ironic/drivers/modules/fake.py @@ -43,20 +43,9 @@ class FakePower(base.PowerInterface): def get_power_state(self, task): return task.node.power_state - def set_power_state(self, task, power_state, timeout=None): - if power_state not in [states.POWER_ON, states.POWER_OFF]: - raise exception.InvalidParameterValue( - _("set_power_state called with an invalid power " - "state: %s.") % power_state) - task.node.power_state = power_state - def reboot(self, task, timeout=None): pass - -class FakeSoftPower(FakePower): - """Example implementation of a simple soft power operations.""" - def set_power_state(self, task, power_state, timeout=None): if power_state not in [states.POWER_ON, states.POWER_OFF, states.SOFT_REBOOT, states.SOFT_POWER_OFF]: @@ -70,6 +59,10 @@ class FakeSoftPower(FakePower): states.SOFT_REBOOT, states.SOFT_POWER_OFF] +# NOTE(dtantsur): for backward compatibility +FakeSoftPower = FakePower + + class FakeBoot(base.BootInterface): """Example implementation of a simple boot interface.""" diff --git a/ironic/drivers/oneview.py b/ironic/drivers/oneview.py index 562d10146c..11819ec2a4 100644 --- a/ironic/drivers/oneview.py +++ b/ironic/drivers/oneview.py @@ -16,6 +16,8 @@ """ OneView Driver and supporting meta-classes. """ + +from oslo_config import cfg from oslo_utils import importutils from ironic.common import exception @@ -30,6 +32,9 @@ from ironic.drivers.modules.oneview import power from ironic.drivers.modules import pxe +CONF = cfg.CONF + + class OneViewHardware(generic.GenericHardware): """OneView hardware type. @@ -84,6 +89,21 @@ class AgentPXEOneViewDriver(base.BaseDriver): self.inspect = inspect.OneViewInspect.create_if_enabled( 'AgentPXEOneViewDriver') + @classmethod + def to_hardware_type(cls): + # NOTE(dtantsur): classic drivers are not affected by the + # enabled_inspect_interfaces configuration option. + if CONF.inspector.enabled: + inspect_interface = 'oneview' + else: + inspect_interface = 'no-inspect' + + return 'oneview', {'boot': 'pxe', + 'deploy': 'oneview-direct', + 'inspect': inspect_interface, + 'management': 'oneview', + 'power': 'oneview'} + class ISCSIPXEOneViewDriver(base.BaseDriver): """OneViewDriver using OneViewClient interface. @@ -111,3 +131,18 @@ class ISCSIPXEOneViewDriver(base.BaseDriver): self.deploy = deploy.OneViewIscsiDeploy() self.inspect = inspect.OneViewInspect.create_if_enabled( 'ISCSIPXEOneViewDriver') + + @classmethod + def to_hardware_type(cls): + # NOTE(dtantsur): classic drivers are not affected by the + # enabled_inspect_interfaces configuration option. + if CONF.inspector.enabled: + inspect_interface = 'oneview' + else: + inspect_interface = 'no-inspect' + + return 'oneview', {'boot': 'pxe', + 'deploy': 'oneview-iscsi', + 'inspect': inspect_interface, + 'management': 'oneview', + 'power': 'oneview'} diff --git a/ironic/drivers/pxe.py b/ironic/drivers/pxe.py index 4cba28b7fa..0c2e025110 100644 --- a/ironic/drivers/pxe.py +++ b/ironic/drivers/pxe.py @@ -17,6 +17,7 @@ PXE Driver and supporting meta-classes. """ +from oslo_config import cfg from oslo_utils import importutils from ironic.common import exception @@ -45,6 +46,9 @@ from ironic.drivers.modules.ucs import management as ucs_mgmt from ironic.drivers.modules.ucs import power as ucs_power +CONF = cfg.CONF + + # For backward compatibility PXEAndIPMIToolDriver = ipmi.PXEAndIPMIToolDriver PXEAndIPMIToolAndSocatDriver = ipmi.PXEAndIPMIToolAndSocatDriver @@ -75,6 +79,17 @@ class PXEAndIloDriver(base.BaseDriver): self.inspect = ilo_inspect.IloInspect() self.raid = agent.AgentRAID() + @classmethod + def to_hardware_type(cls): + return 'ilo', {'boot': 'ilo-pxe', + 'console': 'ilo', + 'deploy': 'iscsi', + 'inspect': 'ilo', + 'management': 'ilo', + 'power': 'ilo', + 'raid': 'agent', + 'vendor': 'ilo'} + class PXEAndSNMPDriver(base.BaseDriver): """PXE + SNMP driver. @@ -130,6 +145,15 @@ class PXEAndIRMCDriver(base.BaseDriver): self.management = irmc_management.IRMCManagement() self.inspect = irmc_inspect.IRMCInspect() + @classmethod + def to_hardware_type(cls): + return 'irmc', {'boot': 'irmc-pxe', + 'console': 'ipmitool-shellinabox', + 'deploy': 'iscsi', + 'inspect': 'irmc', + 'management': 'irmc', + 'power': 'irmc'} + class PXEAndUcsDriver(base.BaseDriver): """PXE + Cisco UCSM driver. @@ -153,6 +177,21 @@ class PXEAndUcsDriver(base.BaseDriver): self.inspect = inspector.Inspector.create_if_enabled( 'PXEAndUcsDriver') + @classmethod + def to_hardware_type(cls): + # NOTE(dtantsur): classic drivers are not affected by the + # enabled_inspect_interfaces configuration option. + if CONF.inspector.enabled: + inspect_interface = 'inspector' + else: + inspect_interface = 'no-inspect' + + return 'cisco-ucs-managed', {'boot': 'pxe', + 'deploy': 'iscsi', + 'inspect': inspect_interface, + 'management': 'ucsm', + 'power': 'ucsm'} + class PXEAndCIMCDriver(base.BaseDriver): """PXE + Cisco IMC driver. @@ -175,3 +214,18 @@ class PXEAndCIMCDriver(base.BaseDriver): self.management = cimc_mgmt.CIMCManagement() self.inspect = inspector.Inspector.create_if_enabled( 'PXEAndCIMCDriver') + + @classmethod + def to_hardware_type(cls): + # NOTE(dtantsur): classic drivers are not affected by the + # enabled_inspect_interfaces configuration option. + if CONF.inspector.enabled: + inspect_interface = 'inspector' + else: + inspect_interface = 'no-inspect' + + return 'cisco-ucs-standalone', {'boot': 'pxe', + 'deploy': 'iscsi', + 'inspect': inspect_interface, + 'management': 'cimc', + 'power': 'cimc'} diff --git a/ironic/tests/unit/drivers/test_base.py b/ironic/tests/unit/drivers/test_base.py index ebf08ccf42..d0185c3c8b 100644 --- a/ironic/tests/unit/drivers/test_base.py +++ b/ironic/tests/unit/drivers/test_base.py @@ -476,18 +476,39 @@ class TestToHardwareType(base.TestCase): hw_type = driver.to_hardware_type()[0] except NotImplementedError: continue + except KeyError: + self.fail('%s does not return a tuple' % driver) + self.assertIn(hw_type, self.hardware_types, '%s returns unknown hardware type %s' % (driver, hw_type)) def test_to_hardware_type_returns_existing_interfaces(self): + self.config(enabled=False, group='inspector') # Check that all defined implementations of to_hardware_type # contain only existing interface types for driver in self.driver_classes: try: delta = driver.to_hardware_type()[1] - except NotImplementedError: - continue + except Exception: + continue # covered by other tests + for iface, value in delta.items(): + self.assertIn(iface, self.existing_ifaces, + '%s returns unknown interface %s' % + (driver, iface)) + self.assertIn(value, self.existing_ifaces[iface], + '%s returns unknown %s interface %s' % + (driver, iface, value)) + + def test_to_hardware_type_returns_existing_interfaces_inspector(self): + self.config(enabled=True, group='inspector') + # Check that all defined implementations of to_hardware_type + # contain only existing interface types + for driver in self.driver_classes: + try: + delta = driver.to_hardware_type()[1] + except Exception: + continue # covered by other tests for iface, value in delta.items(): self.assertIn(iface, self.existing_ifaces, '%s returns unknown interface %s' % @@ -500,9 +521,53 @@ class TestToHardwareType(base.TestCase): for driver in self.driver_classes: try: delta = driver.to_hardware_type()[1] - except NotImplementedError: - continue + except Exception: + continue # covered by other tests for iface in self.mandatory_interfaces: self.assertIn(iface, delta, '%s does not return mandatory interface %s' % (driver, iface)) + + def test_to_hardware_type_for_all_in_tree_drivers(self): + missing = set() + for driver in self.driver_classes: + # We don't want to test out-of-tree drivers installed locally + if not driver.__module__.startswith('ironic.'): + continue + try: + driver.to_hardware_type() + except NotImplementedError: + missing.add(driver.__name__) + + if missing: + self.fail('to_hardware_type not implemented for %s' % + ', '.join(missing)) + + def test_to_hardware_type_boot_deploy(self): + for driver in self.driver_classes: + # We don't want to test out-of-tree drivers installed locally + if not driver.__module__.startswith('ironic.'): + continue + + try: + delta = driver.to_hardware_type()[1] + boot = delta['boot'] + deploy = delta['deploy'] + except NotImplementedError: + continue # covered by other tests + + name = driver.__name__.lower() + # Try to guess the correct values for boot and deploy based on our + # naming schema + if 'pxe' in name: + self.assertIn('pxe', boot, + 'boot interface should be based on pxe for %s' % + driver) + if 'agent' in name: + self.assertIn('direct', deploy, + 'deploy interface should be direct for %s' % + driver) + elif 'iscsi' in name or 'pxe' in name: + self.assertIn('iscsi', deploy, + 'deploy interface should be iscsi for %s' % + driver) diff --git a/releasenotes/notes/fake_soft_power-32683a848a989fc2.yaml b/releasenotes/notes/fake_soft_power-32683a848a989fc2.yaml new file mode 100644 index 0000000000..a393a9ecaf --- /dev/null +++ b/releasenotes/notes/fake_soft_power-32683a848a989fc2.yaml @@ -0,0 +1,5 @@ +--- +other: + - | + All ``fake`` classic drivers now implement fake soft power actions. + The ``fake_soft_power`` driver is now identical to ``fake``.