From 09e89c09d133a68b1172abca4f528e42066b9f15 Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <divius.inside@gmail.com>
Date: Fri, 22 Jun 2018 17:17:28 +0200
Subject: [PATCH] Remove the iDRAC classic drivers

This removes the pxe_drac and pxe_drac_inspector classic drivers.

Change-Id: I2837bc626425869008d9222523b05e2ce5175c19
---
 devstack/lib/ironic                           |  8 +-
 doc/source/admin/drivers/idrac.rst            | 15 +--
 doc/source/install/enabling-drivers.rst       |  2 +-
 ironic/drivers/drac.py                        | 60 ------------
 .../modules/drac/test_periodic_task.py        |  4 +-
 ironic/tests/unit/drivers/test_drac.py        | 92 ++++---------------
 .../no-classic-idrac-4fbf1ba66c35fb4a.yaml    |  6 ++
 setup.cfg                                     |  2 -
 8 files changed, 38 insertions(+), 151 deletions(-)
 create mode 100644 releasenotes/notes/no-classic-idrac-4fbf1ba66c35fb4a.yaml

diff --git a/devstack/lib/ironic b/devstack/lib/ironic
index def61dd6e8..38e29e0960 100644
--- a/devstack/lib/ironic
+++ b/devstack/lib/ironic
@@ -113,7 +113,7 @@ IRONIC_HW_ARCH=${IRONIC_HW_ARCH:-x86_64}
 # *_oneview:
 #   <Server Hardware URI> <Server Hardware Type URI> <Enclosure Group URI> <Server Profile Template URI> <MAC of primary connection> <Applied Server Profile URI>
 #
-# *_drac:
+# idrac:
 #   <BMC address> <MAC address> <BMC username> <BMC password>
 #
 # * irmc:
@@ -293,7 +293,7 @@ fi
 #
 # Additional valid choices if IRONIC_IS_HARDWARE == true are:
 # ``pxe_iscsi_cimc``, ``pxe_agent_cimc``, ``pxe_ucs``, ``pxe_cimc``,
-# ``*_pxe_oneview`` and ``pxe_drac``
+# ``*_pxe_oneview`` and ``idrac``
 IRONIC_DEPLOY_DRIVER=${IRONIC_DEPLOY_DRIVER:-pxe_ipmitool}
 
 # If the requested driver is not yet enable, enable it, if it is not it will fail anyway
@@ -639,7 +639,7 @@ function is_deployed_by_ilo {
 }
 
 function is_deployed_by_drac {
-    [[ -z "${IRONIC_DEPLOY_DRIVER##*_drac}" ]] && return 0
+    [[ "${IRONIC_DEPLOY_DRIVER}" == idrac ]] && return 0
     return 1
 }
 
@@ -659,7 +659,7 @@ function is_deployed_by_irmc {
 }
 
 function is_drac_enabled {
-    [[ -z "${IRONIC_ENABLED_DRIVERS##*_drac}" ]] && return 0
+    [[ -z "${IRONIC_ENABLED_HARDWARE_TYPES%%*idrac*}" ]] && return 0
     return 1
 }
 
diff --git a/doc/source/admin/drivers/idrac.rst b/doc/source/admin/drivers/idrac.rst
index f4126c5f54..fbe21f0c64 100644
--- a/doc/source/admin/drivers/idrac.rst
+++ b/doc/source/admin/drivers/idrac.rst
@@ -5,22 +5,17 @@ iDRAC drivers
 Overview
 ========
 
-iDRAC_ hardware is supported by the ``idrac`` hardware type and the following
-classic drivers:
-
-* ``pxe_drac`` (using out-of-band inspection)
-* ``pxe_drac_inspector`` (using in-band inspection via **ironic-inspector**)
-
-It is also supported by the standard ``ipmi`` hardware type, though with
-a smaller feature set.
+iDRAC_ hardware is supported by the ``idrac`` hardware type. It is also
+supported by the standard ``ipmi`` hardware type, though with a smaller
+feature set.
 
 .. TODO(dtantsur): supported hardware
 
 Enabling
 ========
 
-All iDRAC drivers require the ``python-dracclient`` library to be installed,
-for example::
+The ``idrac`` hardware type requires the ``python-dracclient`` library to be
+installed, for example::
 
     sudo pip install 'python-dracclient>=1.3.0'
 
diff --git a/doc/source/install/enabling-drivers.rst b/doc/source/install/enabling-drivers.rst
index f2584df81c..9363ade15c 100644
--- a/doc/source/install/enabling-drivers.rst
+++ b/doc/source/install/enabling-drivers.rst
@@ -311,7 +311,7 @@ option, for example:
 .. code-block:: ini
 
     [DEFAULT]
-    enabled_drivers = pxe_ipmitool,pxe_drac
+    enabled_drivers = pxe_ipmitool
 
 The names in this comma-separated list are entry point names of the drivers.
 They have to be available at conductor start-up, and all dependencies must
diff --git a/ironic/drivers/drac.py b/ironic/drivers/drac.py
index 2b44c1e5f0..10fdc5c928 100644
--- a/ironic/drivers/drac.py
+++ b/ironic/drivers/drac.py
@@ -16,11 +16,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
-from ironic.common.i18n import _
-from ironic.drivers import base
 from ironic.drivers import generic
 from ironic.drivers.modules.drac import inspect as drac_inspect
 from ironic.drivers.modules.drac import management
@@ -28,9 +24,7 @@ from ironic.drivers.modules.drac import power
 from ironic.drivers.modules.drac import raid
 from ironic.drivers.modules.drac import vendor_passthru
 from ironic.drivers.modules import inspector
-from ironic.drivers.modules import iscsi_deploy
 from ironic.drivers.modules import noop
-from ironic.drivers.modules import pxe
 
 
 CONF = cfg.CONF
@@ -70,57 +64,3 @@ class IDRACHardware(generic.GenericHardware):
     def supported_vendor_interfaces(self):
         """List of supported vendor interfaces."""
         return [vendor_passthru.DracVendorPassthru, noop.NoVendor]
-
-
-class PXEDracDriver(base.BaseDriver):
-    """DRAC driver using PXE for deploy."""
-
-    def __init__(self):
-        if not importutils.try_import('dracclient'):
-            raise exception.DriverLoadError(
-                driver=self.__class__.__name__,
-                reason=_('Unable to import python-dracclient library'))
-
-        self.power = power.DracPower()
-        self.boot = pxe.PXEBoot()
-        self.deploy = iscsi_deploy.ISCSIDeploy()
-        self.management = management.DracManagement()
-        self.raid = raid.DracRAID()
-        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."""
-
-    def __init__(self):
-        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/tests/unit/drivers/modules/drac/test_periodic_task.py b/ironic/tests/unit/drivers/modules/drac/test_periodic_task.py
index 7577e4b417..6cb3d80b04 100644
--- a/ironic/tests/unit/drivers/modules/drac/test_periodic_task.py
+++ b/ironic/tests/unit/drivers/modules/drac/test_periodic_task.py
@@ -66,7 +66,7 @@ class DracPeriodicTaskTestCase(db_base.DbTestCase):
         self.node.save()
         # mock manager
         mock_manager = mock.Mock()
-        node_list = [(self.node.uuid, 'pxe_drac',
+        node_list = [(self.node.uuid, 'idrac',
                       {'raid_config_job_ids': ['42']})]
         mock_manager.iter_nodes.return_value = node_list
         # mock task_manager.acquire
@@ -85,7 +85,7 @@ class DracPeriodicTaskTestCase(db_base.DbTestCase):
     def test__query_raid_config_job_status_no_config_jobs(self, mock_acquire):
         # mock manager
         mock_manager = mock.Mock()
-        node_list = [(self.node.uuid, 'pxe_drac', {})]
+        node_list = [(self.node.uuid, 'idrac', {})]
         mock_manager.iter_nodes.return_value = node_list
         # mock task_manager.acquire
         task = mock.Mock(node=self.node, driver=mock.Mock(raid=self.raid))
diff --git a/ironic/tests/unit/drivers/test_drac.py b/ironic/tests/unit/drivers/test_drac.py
index 9ec609ab48..dc08566c63 100644
--- a/ironic/tests/unit/drivers/test_drac.py
+++ b/ironic/tests/unit/drivers/test_drac.py
@@ -12,14 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import inspect
-
-import mock
-from oslo_utils import importutils
-
-from ironic.common import exception
 from ironic.conductor import task_manager
-from ironic.drivers import drac as drac_drivers
 from ironic.drivers.modules import agent
 from ironic.drivers.modules import drac
 from ironic.drivers.modules import inspector
@@ -32,10 +25,18 @@ from ironic.tests.unit.db import base as db_base
 from ironic.tests.unit.objects import utils as obj_utils
 
 
-class BaseIDRACTestCase(db_base.DbTestCase):
+class IDRACHardwareTestCase(db_base.DbTestCase):
 
     def setUp(self):
-        super(BaseIDRACTestCase, self).setUp()
+        super(IDRACHardwareTestCase, self).setUp()
+        self.config(enabled_hardware_types=['idrac'],
+                    enabled_management_interfaces=['idrac'],
+                    enabled_power_interfaces=['idrac'],
+                    enabled_inspect_interfaces=[
+                        'idrac', 'inspector', 'no-inspect'],
+                    enabled_network_interfaces=['flat', 'neutron', 'noop'],
+                    enabled_raid_interfaces=['idrac', 'no-raid'],
+                    enabled_vendor_interfaces=['idrac', 'no-vendor'])
 
     def _validate_interfaces(self, driver, **kwargs):
         self.assertIsInstance(
@@ -51,64 +52,40 @@ class BaseIDRACTestCase(db_base.DbTestCase):
             driver.power,
             kwargs.get('power', drac.power.DracPower))
 
-        # Console interface of iDRAC classic drivers is None.
-        console_interface = kwargs.get('console', None)
-
-        # None is not a class or type.
-        if inspect.isclass(console_interface):
-            self.assertIsInstance(driver.console, console_interface)
-        else:
-            self.assertIs(driver.console, console_interface)
+        self.assertIsInstance(
+            driver.console,
+            kwargs.get('console', noop.NoConsole))
 
         self.assertIsInstance(
             driver.inspect,
             kwargs.get('inspect', drac.inspect.DracInspect))
 
-        # iDRAC classic drivers do not have a network interface.
-        if 'network' in driver.all_interfaces:
-            self.assertIsInstance(
-                driver.network,
-                kwargs.get('network', flat_net.FlatNetwork))
+        self.assertIsInstance(
+            driver.network,
+            kwargs.get('network', flat_net.FlatNetwork))
 
         self.assertIsInstance(
             driver.raid,
             kwargs.get('raid', drac.raid.DracRAID))
 
-        # iDRAC classic drivers do not have a storage interface.
-        if 'storage' in driver.all_interfaces:
-            self.assertIsInstance(
-                driver.storage,
-                kwargs.get('storage', noop_storage.NoopStorage))
+        self.assertIsInstance(
+            driver.storage,
+            kwargs.get('storage', noop_storage.NoopStorage))
 
         self.assertIsInstance(
             driver.vendor,
             kwargs.get('vendor', drac.vendor_passthru.DracVendorPassthru))
 
-
-class IDRACHardwareTestCase(BaseIDRACTestCase):
-
-    def setUp(self):
-        super(IDRACHardwareTestCase, self).setUp()
-        self.config(enabled_hardware_types=['idrac'],
-                    enabled_management_interfaces=['idrac'],
-                    enabled_power_interfaces=['idrac'],
-                    enabled_inspect_interfaces=[
-                        'idrac', 'inspector', 'no-inspect'],
-                    enabled_network_interfaces=['flat', 'neutron', 'noop'],
-                    enabled_raid_interfaces=['idrac', 'no-raid'],
-                    enabled_vendor_interfaces=['idrac', 'no-vendor'])
-
     def test_default_interfaces(self):
         node = obj_utils.create_test_node(self.context, driver='idrac')
         with task_manager.acquire(self.context, node.id) as task:
-            self._validate_interfaces(task.driver, console=noop.NoConsole)
+            self._validate_interfaces(task.driver)
 
     def test_override_with_inspector(self):
         node = obj_utils.create_test_node(self.context, driver='idrac',
                                           inspect_interface='inspector')
         with task_manager.acquire(self.context, node.id) as task:
             self._validate_interfaces(task.driver,
-                                      console=noop.NoConsole,
                                       inspect=inspector.Inspector)
 
     def test_override_with_agent(self):
@@ -117,7 +94,6 @@ class IDRACHardwareTestCase(BaseIDRACTestCase):
                                           inspect_interface='inspector')
         with task_manager.acquire(self.context, node.id) as task:
             self._validate_interfaces(task.driver,
-                                      console=noop.NoConsole,
                                       deploy=agent.AgentDeploy,
                                       inspect=inspector.Inspector)
 
@@ -126,7 +102,6 @@ class IDRACHardwareTestCase(BaseIDRACTestCase):
                                           raid_interface='no-raid')
         with task_manager.acquire(self.context, node.id) as task:
             self._validate_interfaces(task.driver,
-                                      console=noop.NoConsole,
                                       raid=noop.NoRAID)
 
     def test_override_no_vendor(self):
@@ -134,31 +109,4 @@ class IDRACHardwareTestCase(BaseIDRACTestCase):
                                           vendor_interface='no-vendor')
         with task_manager.acquire(self.context, node.id) as task:
             self._validate_interfaces(task.driver,
-                                      console=noop.NoConsole,
                                       vendor=noop.NoVendor)
-
-
-@mock.patch.object(importutils, 'try_import', spec_set=True, autospec=True)
-class DracClassicDriversTestCase(BaseIDRACTestCase):
-
-    def setUp(self):
-        super(DracClassicDriversTestCase, self).setUp()
-
-    def test_pxe_drac_driver(self, mock_try_import):
-        mock_try_import.return_value = True
-
-        driver = drac_drivers.PXEDracDriver()
-        self._validate_interfaces(driver)
-
-    def test___init___try_import_dracclient_failure(self, mock_try_import):
-        mock_try_import.return_value = False
-
-        self.assertRaises(exception.DriverLoadError,
-                          drac_drivers.PXEDracDriver)
-
-    def test_pxe_drac_inspector_driver(self, mock_try_import):
-        self.config(enabled=True, group='inspector')
-        mock_try_import.return_value = True
-
-        driver = drac_drivers.PXEDracInspectorDriver()
-        self._validate_interfaces(driver, inspect=inspector.Inspector)
diff --git a/releasenotes/notes/no-classic-idrac-4fbf1ba66c35fb4a.yaml b/releasenotes/notes/no-classic-idrac-4fbf1ba66c35fb4a.yaml
new file mode 100644
index 0000000000..06c8e66145
--- /dev/null
+++ b/releasenotes/notes/no-classic-idrac-4fbf1ba66c35fb4a.yaml
@@ -0,0 +1,6 @@
+---
+upgrade:
+  - |
+    The deprecated iDRAC classic drivers ``pxe_drac`` and
+    ``pxe_drac_inspector`` have been removed. Please use the ``idrac``
+    hardware type.
diff --git a/setup.cfg b/setup.cfg
index b0807682e6..7bd10eeab2 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -62,8 +62,6 @@ ironic.drivers =
     iscsi_pxe_oneview = ironic.drivers.oneview:ISCSIPXEOneViewDriver
     pxe_ipmitool = ironic.drivers.ipmi:PXEAndIPMIToolDriver
     pxe_ipmitool_socat = ironic.drivers.ipmi:PXEAndIPMIToolAndSocatDriver
-    pxe_drac = ironic.drivers.drac:PXEDracDriver
-    pxe_drac_inspector = ironic.drivers.drac:PXEDracInspectorDriver
     pxe_snmp = ironic.drivers.pxe:PXEAndSNMPDriver
     pxe_irmc = ironic.drivers.pxe:PXEAndIRMCDriver
     pxe_ucs = ironic.drivers.pxe:PXEAndUcsDriver