diff --git a/ironic/drivers/drac.py b/ironic/drivers/drac.py index 811a5da74f..ea0abb0654 100644 --- a/ironic/drivers/drac.py +++ b/ironic/drivers/drac.py @@ -20,13 +20,13 @@ from oslo_utils import importutils from ironic.common import exception from ironic.common.i18n import _ from ironic.drivers import base -from ironic.drivers.modules.drac import deploy from ironic.drivers.modules.drac import inspect as drac_inspect from ironic.drivers.modules.drac import management 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 pxe @@ -41,7 +41,7 @@ class PXEDracDriver(base.BaseDriver): self.power = power.DracPower() self.boot = pxe.PXEBoot() - self.deploy = deploy.DracDeploy() + self.deploy = iscsi_deploy.ISCSIDeploy() self.management = management.DracManagement() self.raid = raid.DracRAID() self.vendor = vendor_passthru.DracVendorPassthru() diff --git a/ironic/drivers/fake.py b/ironic/drivers/fake.py index e926aaa8f7..df1b112f2b 100644 --- a/ironic/drivers/fake.py +++ b/ironic/drivers/fake.py @@ -25,7 +25,6 @@ from ironic.drivers import base from ironic.drivers.modules import agent from ironic.drivers.modules.cimc import management as cimc_mgmt from ironic.drivers.modules.cimc import power as cimc_power -from ironic.drivers.modules.drac import deploy as drac_deploy from ironic.drivers.modules.drac import inspect as drac_inspect from ironic.drivers.modules.drac import management as drac_mgmt from ironic.drivers.modules.drac import power as drac_power @@ -176,7 +175,7 @@ class FakeDracDriver(base.BaseDriver): reason=_('Unable to import python-dracclient library')) self.power = drac_power.DracPower() - self.deploy = drac_deploy.DracDeploy() + self.deploy = iscsi_deploy.ISCSIDeploy() self.management = drac_mgmt.DracManagement() self.raid = drac_raid.DracRAID() self.vendor = drac_vendor.DracVendorPassthru() diff --git a/ironic/drivers/modules/drac/deploy.py b/ironic/drivers/modules/drac/deploy.py deleted file mode 100644 index 722fa944e4..0000000000 --- a/ironic/drivers/modules/drac/deploy.py +++ /dev/null @@ -1,54 +0,0 @@ -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -""" -DRAC deploy interface -""" - -from ironic_lib import metrics_utils - -from ironic.drivers.modules import deploy_utils -from ironic.drivers.modules import iscsi_deploy - -_OOB_CLEAN_STEPS = [ - {'interface': 'raid', 'step': 'create_configuration'}, - {'interface': 'raid', 'step': 'delete_configuration'} -] - -METRICS = metrics_utils.get_metrics_logger(__name__) - - -class DracDeploy(iscsi_deploy.ISCSIDeploy): - - @METRICS.timer('DracDeploy.prepare_cleaning') - def prepare_cleaning(self, task): - """Prepare environment for cleaning - - Boot into the agent to prepare for cleaning if in-band cleaning step - is requested. - - :param task: a TaskManager instance containing the node to act on. - :returns: states.CLEANWAIT if there is any in-band clean step to - signify an asynchronous prepare. - """ - node = task.node - - inband_steps = [step for step - in node.driver_internal_info.get('clean_steps', []) - if {'interface': step['interface'], - 'step': step['step']} not in _OOB_CLEAN_STEPS] - - if ('agent_cached_clean_steps' not in node.driver_internal_info or - inband_steps): - return deploy_utils.prepare_inband_cleaning(task, - manage_boot=True) diff --git a/ironic/tests/unit/drivers/modules/drac/test_deploy.py b/ironic/tests/unit/drivers/modules/drac/test_deploy.py deleted file mode 100644 index 03bec356e8..0000000000 --- a/ironic/tests/unit/drivers/modules/drac/test_deploy.py +++ /dev/null @@ -1,101 +0,0 @@ -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -""" -Test class for DRAC deploy interface -""" - -import mock - -from ironic.common import states -from ironic.conductor import task_manager -from ironic.drivers.modules import deploy_utils -from ironic.tests.unit.conductor import mgr_utils -from ironic.tests.unit.db import base as db_base -from ironic.tests.unit.db import utils as db_utils -from ironic.tests.unit.objects import utils as obj_utils - -INFO_DICT = db_utils.get_test_drac_info() - - -class DracDeployTestCase(db_base.DbTestCase): - - def setUp(self): - super(DracDeployTestCase, self).setUp() - mgr_utils.mock_the_extension_manager(driver='fake_drac') - self.node = obj_utils.create_test_node(self.context, - driver='fake_drac', - driver_info=INFO_DICT) - - @mock.patch.object(deploy_utils, 'prepare_inband_cleaning', spec_set=True, - autospec=True) - def test_prepare_cleaning_with_no_clean_step( - self, mock_prepare_inband_cleaning): - mock_prepare_inband_cleaning.return_value = states.CLEANWAIT - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - res = task.driver.deploy.prepare_cleaning(task) - self.assertEqual(states.CLEANWAIT, res) - - mock_prepare_inband_cleaning.assert_called_once_with( - task, manage_boot=True) - - @mock.patch.object(deploy_utils, 'prepare_inband_cleaning', spec_set=True, - autospec=True) - def test_prepare_cleaning_with_inband_clean_step( - self, mock_prepare_inband_cleaning): - self.node.driver_internal_info['clean_steps'] = [ - {'step': 'erase_disks', 'priority': 20, 'interface': 'deploy'}] - mock_prepare_inband_cleaning.return_value = states.CLEANWAIT - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - res = task.driver.deploy.prepare_cleaning(task) - self.assertEqual(states.CLEANWAIT, res) - - mock_prepare_inband_cleaning.assert_called_once_with( - task, manage_boot=True) - - @mock.patch.object(deploy_utils, 'prepare_inband_cleaning', spec_set=True, - autospec=True) - def test_prepare_cleaning_with_oob_clean_step_with_no_agent_cached_steps( - self, mock_prepare_inband_cleaning): - self.node.driver_internal_info['clean_steps'] = [ - {'interface': 'raid', 'step': 'create_configuration'}] - mock_prepare_inband_cleaning.return_value = states.CLEANWAIT - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - res = task.driver.deploy.prepare_cleaning(task) - self.assertEqual(states.CLEANWAIT, res) - - mock_prepare_inband_cleaning.assert_called_once_with( - task, manage_boot=True) - - @mock.patch.object(deploy_utils, 'prepare_inband_cleaning', spec_set=True, - autospec=True) - def test_prepare_cleaning_with_oob_clean_step_with_agent_cached_steps( - self, mock_prepare_inband_cleaning): - self.node.driver_internal_info['agent_cached_clean_steps'] = [] - self.node.driver_internal_info['clean_steps'] = [ - {'interface': 'raid', 'step': 'create_configuration'}] - mock_prepare_inband_cleaning.return_value = states.CLEANWAIT - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - res = task.driver.deploy.prepare_cleaning(task) - self.assertEqual(states.CLEANWAIT, res) - - mock_prepare_inband_cleaning.assert_called_once_with( - task, manage_boot=True) diff --git a/ironic/tests/unit/drivers/test_drac.py b/ironic/tests/unit/drivers/test_drac.py new file mode 100644 index 0000000000..4d37947be4 --- /dev/null +++ b/ironic/tests/unit/drivers/test_drac.py @@ -0,0 +1,108 @@ +# Copyright (c) 2017 Dell Inc. or its subsidiaries. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# 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.drivers import drac as drac_drivers +from ironic.drivers.modules import drac +from ironic.drivers.modules import inspector +from ironic.drivers.modules import iscsi_deploy +from ironic.drivers.modules.network import flat as flat_net +from ironic.drivers.modules import pxe +from ironic.drivers.modules.storage import noop as noop_storage +from ironic.tests.unit.db import base as db_base + + +class BaseIDRACTestCase(db_base.DbTestCase): + + def setUp(self): + super(BaseIDRACTestCase, self).setUp() + + def _validate_interfaces(self, driver, **kwargs): + self.assertIsInstance( + driver.boot, + kwargs.get('boot', pxe.PXEBoot)) + self.assertIsInstance( + driver.deploy, + kwargs.get('deploy', iscsi_deploy.ISCSIDeploy)) + self.assertIsInstance( + driver.management, + kwargs.get('management', drac.management.DracManagement)) + self.assertIsInstance( + 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.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.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.vendor, + kwargs.get('vendor', drac.vendor_passthru.DracVendorPassthru)) + + +@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/drac-fix-double-manage-provide-cycle-6ac8a427068f87fe.yaml b/releasenotes/notes/drac-fix-double-manage-provide-cycle-6ac8a427068f87fe.yaml new file mode 100644 index 0000000000..20d13f2db4 --- /dev/null +++ b/releasenotes/notes/drac-fix-double-manage-provide-cycle-6ac8a427068f87fe.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - Fixes an issue that caused a node using a Dell EMC integrated Dell Remote + Access Controller (iDRAC) *classic driver*, ``pxe_drac`` or + ``pxe_drac_inspector``, to be placed in the ``clean failed`` state after a + double ``manage``/``provide`` cycle, instead of the ``available`` state. + For more information, see `bug 1676387 + `_.