openstack/ironic
Code Issues Proposed changes

ironic-discoverd is being renamed to ironic-inspector

Browse Source 
This patch changes module names, allows importing new module
name for inspector (will be later changed to ironic_inspector_client).

We need this patch to unbreak devstack support.

Change-Id: I34cc4336dcd7e8d44aebecf5c85b52c83f0c717b
This commit is contained in:
Dmitry Tantsur 2015-05-29 16:28:33 +02:00
parent d1b586cbf6
commit 06c8fc9ad1
9 changed files with 133 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
etc/ironic/ironic.conf.sample
View File

@ -713,26 +713,6 @@
#dhcp_provider=neutron
[discoverd]
#
# Options defined in ironic.drivers.modules.discoverd
#
# whether to enable inspection using ironic-discoverd (boolean
# value)
#enabled=false
# ironic-discoverd HTTP endpoint. If this is not set, the
# ironic-discoverd client default (http://127.0.0.1:5050) will
# be used. (string value)
#service_url=<None>
# period (in seconds) to check status of nodes on inspection
# (integer value)
#status_check_period=60
[disk_partitioner]
#
@ -934,6 +914,26 @@
#power_wait=2
[inspector]
#
# Options defined in ironic.drivers.modules.inspector
#
# whether to enable inspection using ironic-inspector (boolean
# value)
#enabled=false
# ironic-inspector HTTP endpoint. If this is not set, the
# ironic-inspector client default (http://127.0.0.1:5050) will
# be used. (string value)
#service_url=<None>
# period (in seconds) to check status of nodes on inspection
# (integer value)
#status_check_period=60
[ipmi]
#

4
ironic/drivers/drac.py
View File

@ -19,9 +19,9 @@ from oslo_utils import importutils
from ironic.common import exception
from ironic.common.i18n import _
from ironic.drivers import base
from ironic.drivers.modules import discoverd
from ironic.drivers.modules.drac import management
from ironic.drivers.modules.drac import power
from ironic.drivers.modules import inspector
from ironic.drivers.modules import pxe
@ -38,5 +38,5 @@ class PXEDracDriver(base.BaseDriver):
self.deploy = pxe.PXEDeploy()
self.management = management.DracManagement()
self.vendor = pxe.VendorPassthru()
self.inspect = discoverd.DiscoverdInspect.create_if_enabled(
self.inspect = inspector.Inspector.create_if_enabled(
'PXEDracDriver')

12
ironic/drivers/fake.py
View File

@ -25,7 +25,6 @@ from ironic.drivers import base
from ironic.drivers.modules import agent
from ironic.drivers.modules.amt import management as amt_mgmt
from ironic.drivers.modules.amt import power as amt_power
from ironic.drivers.modules import discoverd
from ironic.drivers.modules.drac import management as drac_mgmt
from ironic.drivers.modules.drac import power as drac_power
from ironic.drivers.modules import fake
@ -33,6 +32,7 @@ from ironic.drivers.modules import iboot
from ironic.drivers.modules.ilo import inspect as ilo_inspect
from ironic.drivers.modules.ilo import management as ilo_management
from ironic.drivers.modules.ilo import power as ilo_power
from ironic.drivers.modules import inspector
from ironic.drivers.modules import ipminative
from ironic.drivers.modules import ipmitool
from ironic.drivers.modules.irmc import management as irmc_management
@ -210,8 +210,8 @@ class FakeVirtualBoxDriver(base.BaseDriver):
self.management = virtualbox.VirtualBoxManagement()
class FakeIPMIToolDiscoverdDriver(base.BaseDriver):
"""Fake Discoverd driver."""
class FakeIPMIToolInspectorDriver(base.BaseDriver):
"""Fake Inspector driver."""
def __init__(self):
self.power = ipmitool.IPMIPower()
@ -219,10 +219,10 @@ class FakeIPMIToolDiscoverdDriver(base.BaseDriver):
self.deploy = fake.FakeDeploy()
self.vendor = ipmitool.VendorPassthru()
self.management = ipmitool.IPMIManagement()
# NOTE(dtantsur): unlike other uses of DiscoverdInspect, this one is
# unconditional, as this driver is designed for testing discoverd
# NOTE(dtantsur): unlike other uses of Inspector, this one is
# unconditional, as this driver is designed for testing inspector
# integration.
self.inspect = discoverd.DiscoverdInspect()
self.inspect = inspector.Inspector()
class FakeAMTDriver(base.BaseDriver):

93
ironic/drivers/modules/discoverd.py → ironic/drivers/modules/inspector.py
View File

@ -11,7 +11,7 @@
# under the License.
"""
Modules required to work with ironic_discoverd:
Modules required to work with ironic_inspector:
https://pypi.python.org/pypi/ironic-discoverd
"""
@ -33,59 +33,66 @@ from ironic.drivers import base
LOG = logging.getLogger(__name__)
discoverd_opts = [
inspector_opts = [
cfg.BoolOpt('enabled', default=False,
help='whether to enable inspection using ironic-discoverd'),
help='whether to enable inspection using ironic-inspector',
deprecated_group='discoverd'),
cfg.StrOpt('service_url',
help='ironic-discoverd HTTP endpoint. If this is not set, the '
'ironic-discoverd client default (http://127.0.0.1:5050) '
'will be used.'),
help='ironic-inspector HTTP endpoint. If this is not set, the '
'ironic-inspector client default (http://127.0.0.1:5050) '
'will be used.',
deprecated_group='discoverd'),
cfg.IntOpt('status_check_period', default=60,
help='period (in seconds) to check status of nodes '
'on inspection')
'on inspection',
deprecated_group='discoverd'),
]
CONF = cfg.CONF
CONF.register_opts(discoverd_opts, group='discoverd')
CONF.register_opts(inspector_opts, group='inspector')
ironic_discoverd = importutils.try_import('ironic_discoverd')
if ironic_discoverd:
from ironic_discoverd import client
# TODO(dtantsur): change this to ironic_inspector_client once it's available
ironic_inspector = importutils.try_import('ironic_inspector')
if not ironic_inspector:
# NOTE(dtantsur): old name for ironic-inspector
ironic_inspector = importutils.try_import('ironic_discoverd')
if ironic_inspector:
from ironic_inspector import client
class DiscoverdInspect(base.InspectInterface):
"""In-band inspection via ironic-discoverd project."""
class Inspector(base.InspectInterface):
"""In-band inspection via ironic-inspector project."""
@classmethod
def create_if_enabled(cls, driver_name):
"""Create instance of DiscoverdInspect if it's enabled.
"""Create instance of Inspector if it's enabled.
Reports log warning with given driver_name if it's not.
:return: DiscoverdInspect instance or None
:return: Inspector instance or None
"""
if CONF.discoverd.enabled:
if CONF.inspector.enabled:
return cls()
else:
LOG.info(_LI("Inspection via ironic-discoverd is disabled in "
LOG.info(_LI("Inspection via ironic-inspector is disabled in "
"configuration for driver %s. To enable, change "
"[discoverd] enabled = True."), driver_name)
"[inspector] enabled = True."), driver_name)
def __init__(self):
if not CONF.discoverd.enabled:
if not CONF.inspector.enabled:
raise exception.DriverLoadError(
_('ironic-discoverd support is disabled'))
_('ironic-inspector support is disabled'))
if not ironic_discoverd:
if not ironic_inspector:
raise exception.DriverLoadError(
_('ironic-discoverd Python module not found'))
_('ironic-inspector Python module not found'))
# NOTE(dtantsur): __version_info__ attribute appeared in 1.0.0
version = getattr(ironic_discoverd, '__version_info__', (0, 2))
version = getattr(ironic_inspector, '__version_info__', (0, 2))
if version < (1, 0):
raise exception.DriverLoadError(
_('ironic-discoverd version is too old: required >= 1.0.0, '
_('ironic-inspector version is too old: required >= 1.0.0, '
'got %s') % '.'.join(str(x) for x in version))
def get_properties(self):
@ -102,7 +109,7 @@ class DiscoverdInspect(base.InspectInterface):
:param task: a task from TaskManager.
"""
# NOTE(deva): this is not callable if discoverd is disabled
# NOTE(deva): this is not callable if inspector is disabled
# so don't raise an exception -- just pass.
pass
@ -110,24 +117,24 @@ class DiscoverdInspect(base.InspectInterface):
"""Inspect hardware to obtain the hardware properties.
This particular implementation only starts inspection using
ironic-discoverd. Results will be checked in a periodic task.
ironic-inspector. Results will be checked in a periodic task.
:param task: a task from TaskManager.
:returns: states.INSPECTING
"""
LOG.debug('Starting inspection for node %(uuid)s using '
'ironic-discoverd client %(version)s',
'ironic-inspector client %(version)s',
{'uuid': task.node.uuid, 'version':
ironic_discoverd.__version__})
ironic_inspector.__version__})
# NOTE(dtantsur): we're spawning a short-living green thread so that
# we can release a lock as soon as possible and allow ironic-discoverd
# we can release a lock as soon as possible and allow ironic-inspector
# to operate on a node.
eventlet.spawn_n(_start_inspection, task.node.uuid, task.context)
return states.INSPECTING
@base.driver_periodic_task(spacing=CONF.discoverd.status_check_period,
enabled=CONF.discoverd.enabled)
@base.driver_periodic_task(spacing=CONF.inspector.status_check_period,
enabled=CONF.inspector.enabled)
def _periodic_check_result(self, manager, context):
"""Periodic task checking results of inspection."""
filters = {'provision_state': states.INSPECTING}
@ -143,21 +150,21 @@ class DiscoverdInspect(base.InspectInterface):
continue
def _call_discoverd(func, uuid, context):
"""Wrapper around calls to discoverd."""
def _call_inspector(func, uuid, context):
"""Wrapper around calls to inspector."""
# NOTE(dtantsur): due to bug #1428652 None is not accepted for base_url.
kwargs = {}
if CONF.discoverd.service_url:
kwargs['base_url'] = CONF.discoverd.service_url
if CONF.inspector.service_url:
kwargs['base_url'] = CONF.inspector.service_url
return func(uuid, auth_token=context.auth_token, **kwargs)
def _start_inspection(node_uuid, context):
"""Call to discoverd to start inspection."""
"""Call to inspector to start inspection."""
try:
_call_discoverd(client.introspect, node_uuid, context)
_call_inspector(client.introspect, node_uuid, context)
except Exception as exc:
LOG.exception(_LE('Exception during contacting ironic-discoverd '
LOG.exception(_LE('Exception during contacting ironic-inspector '
'for inspection of node %(node)s: %(err)s'),
{'node': node_uuid, 'err': exc})
# NOTE(dtantsur): if acquire fails our last option is to rely on
@ -166,7 +173,7 @@ def _start_inspection(node_uuid, context):
task.node.last_error = _('Failed to start inspection: %s') % exc
task.process_event('fail')
else:
LOG.info(_LI('Node %s was sent to inspection to ironic-discoverd'),
LOG.info(_LI('Node %s was sent to inspection to ironic-inspector'),
node_uuid)
@ -175,16 +182,16 @@ def _check_status(task):
node = task.node
if node.provision_state != states.INSPECTING:
return
if not isinstance(task.driver.inspect, DiscoverdInspect):
if not isinstance(task.driver.inspect, Inspector):
return
LOG.debug('Calling to discoverd to check status of node %s',
LOG.debug('Calling to inspector to check status of node %s',
task.node.uuid)
# NOTE(dtantsur): periodic tasks do not have proper tokens in context
task.context.auth_token = keystone.get_admin_auth_token()
try:
status = _call_discoverd(client.get_status, node.uuid, task.context)
status = _call_inspector(client.get_status, node.uuid, task.context)
except Exception:
# NOTE(dtantsur): get_status should not normally raise
# let's assume it's a transient failure and retry later
@ -197,7 +204,7 @@ def _check_status(task):
LOG.error(_LE('Inspection failed for node %(uuid)s '
'with error: %(err)s'),
{'uuid': node.uuid, 'err': status['error']})
node.last_error = (_('ironic-discoverd inspection failed: %s')
node.last_error = (_('ironic-inspector inspection failed: %s')
% status['error'])
task.process_event('fail')
elif status.get('finished'):

8
ironic/drivers/pxe.py
View File

@ -25,12 +25,12 @@ from ironic.drivers import base
from ironic.drivers.modules.amt import management as amt_management
from ironic.drivers.modules.amt import power as amt_power
from ironic.drivers.modules.amt import vendor as amt_vendor
from ironic.drivers.modules import discoverd
from ironic.drivers.modules import iboot
from ironic.drivers.modules.ilo import deploy as ilo_deploy
from ironic.drivers.modules.ilo import inspect as ilo_inspect
from ironic.drivers.modules.ilo import management as ilo_management
from ironic.drivers.modules.ilo import power as ilo_power
from ironic.drivers.modules import inspector
from ironic.drivers.modules import ipminative
from ironic.drivers.modules import ipmitool
from ironic.drivers.modules.irmc import management as irmc_management
@ -60,7 +60,7 @@ class PXEAndIPMIToolDriver(base.BaseDriver):
self.deploy = pxe.PXEDeploy()
self.management = ipmitool.IPMIManagement()
self.vendor = pxe.VendorPassthru()
self.inspect = discoverd.DiscoverdInspect.create_if_enabled(
self.inspect = inspector.Inspector.create_if_enabled(
'PXEAndIPMIToolDriver')
@ -81,7 +81,7 @@ class PXEAndSSHDriver(base.BaseDriver):
self.deploy = pxe.PXEDeploy()
self.management = ssh.SSHManagement()
self.vendor = pxe.VendorPassthru()
self.inspect = discoverd.DiscoverdInspect.create_if_enabled(
self.inspect = inspector.Inspector.create_if_enabled(
'PXEAndSSHDriver')
@ -106,7 +106,7 @@ class PXEAndIPMINativeDriver(base.BaseDriver):
self.deploy = pxe.PXEDeploy()
self.management = ipminative.NativeIPMIManagement()
self.vendor = pxe.VendorPassthru()
self.inspect = discoverd.DiscoverdInspect.create_if_enabled(
self.inspect = inspector.Inspector.create_if_enabled(
'PXEAndIPMINativeDriver')

76
ironic/tests/drivers/test_discoverd.py → ironic/tests/drivers/test_inspector.py
View File

@ -11,8 +11,8 @@
# under the License.
import eventlet
import ironic_discoverd
from ironic_discoverd import client
import ironic_inspector
from ironic_inspector import client
import mock
from ironic.common import driver_factory
@ -20,7 +20,7 @@ from ironic.common import exception
from ironic.common import keystone
from ironic.common import states
from ironic.conductor import task_manager
from ironic.drivers.modules import discoverd
from ironic.drivers.modules import inspector
from ironic.tests.conductor import utils as mgr_utils
from ironic.tests.db import base as db_base
from ironic.tests.objects import utils as obj_utils
@ -36,43 +36,43 @@ class DisabledTestCase(db_base.DbTestCase):
self.driver = driver_factory.get_driver("pxe_ssh")
def test_disabled(self):
self.config(enabled=False, group='discoverd')
self.config(enabled=False, group='inspector')
self._do_mock()
self.assertIsNone(self.driver.inspect)
# NOTE(dtantsur): it's expected that fake_discoverd fails to load
# NOTE(dtantsur): it's expected that fake_inspector fails to load
# in this case
self.assertRaises(exception.DriverLoadError,
mgr_utils.mock_the_extension_manager,
"fake_discoverd")
"fake_inspector")
def test_enabled(self):
self.config(enabled=True, group='discoverd')
self.config(enabled=True, group='inspector')
self._do_mock()
self.assertIsNotNone(self.driver.inspect)
@mock.patch.object(discoverd, 'ironic_discoverd', None)
def test_init_discoverd_not_imported(self):
@mock.patch.object(inspector, 'ironic_inspector', None)
def test_init_inspector_not_imported(self):
self.assertRaises(exception.DriverLoadError,
discoverd.DiscoverdInspect)
inspector.Inspector)
@mock.patch.object(ironic_discoverd, '__version_info__', (1, 0, 0))
@mock.patch.object(ironic_inspector, '__version_info__', (1, 0, 0))
def test_init_ok(self):
self.config(enabled=True, group='discoverd')
discoverd.DiscoverdInspect()
self.config(enabled=True, group='inspector')
inspector.Inspector()
@mock.patch.object(ironic_discoverd, '__version_info__', (0, 2, 2))
@mock.patch.object(ironic_inspector, '__version_info__', (0, 2, 2))
def test_init_old_version(self):
self.config(enabled=True, group='discoverd')
self.config(enabled=True, group='inspector')
self.assertRaises(exception.DriverLoadError,
discoverd.DiscoverdInspect)
inspector.Inspector)
class BaseTestCase(db_base.DbTestCase):
def setUp(self):
super(BaseTestCase, self).setUp()
self.config(enabled=True, group='discoverd')
mgr_utils.mock_the_extension_manager("fake_discoverd")
self.driver = driver_factory.get_driver("fake_discoverd")
self.config(enabled=True, group='inspector')
mgr_utils.mock_the_extension_manager("fake_inspector")
self.driver = driver_factory.get_driver("fake_inspector")
self.node = obj_utils.get_test_node(self.context)
self.task = mock.MagicMock(spec=task_manager.TaskManager)
self.task.context = mock.MagicMock(spec_set=['auth_token'])
@ -90,13 +90,13 @@ class CommonFunctionsTestCase(BaseTestCase):
self.assertEqual({}, res)
def test_create_if_enabled(self):
res = discoverd.DiscoverdInspect.create_if_enabled('driver')
self.assertIsInstance(res, discoverd.DiscoverdInspect)
res = inspector.Inspector.create_if_enabled('driver')
self.assertIsInstance(res, inspector.Inspector)
@mock.patch.object(discoverd.LOG, 'info', autospec=True)
@mock.patch.object(inspector.LOG, 'info', autospec=True)
def test_create_if_enabled_disabled(self, warn_mock):
self.config(enabled=False, group='discoverd')
res = discoverd.DiscoverdInspect.create_if_enabled('driver')
self.config(enabled=False, group='inspector')
res = inspector.Inspector.create_if_enabled('driver')
self.assertIsNone(res)
self.assertTrue(warn_mock.called)
@ -112,7 +112,7 @@ class InspectHardwareTestCase(BaseTestCase):
auth_token=self.task.context.auth_token)
def test_url(self, mock_introspect):
self.config(service_url='meow', group='discoverd')
self.config(service_url='meow', group='inspector')
self.assertEqual(states.INSPECTING,
self.driver.inspect.inspect_hardware(self.task))
mock_introspect.assert_called_once_with(
@ -141,47 +141,47 @@ class CheckStatusTestCase(BaseTestCase):
def test_not_inspecting(self, mock_get):
self.node.provision_state = states.MANAGEABLE
discoverd._check_status(self.task)
inspector._check_status(self.task)
self.assertFalse(mock_get.called)
def test_not_discoverd(self, mock_get):
def test_not_inspector(self, mock_get):
self.task.driver.inspect = object()
discoverd._check_status(self.task)
inspector._check_status(self.task)
self.assertFalse(mock_get.called)
def test_not_finished(self, mock_get):
mock_get.return_value = {}
discoverd._check_status(self.task)
inspector._check_status(self.task)
mock_get.assert_called_once_with(self.node.uuid,
auth_token='the token')
self.assertFalse(self.task.process_event.called)
def test_exception_ignored(self, mock_get):
mock_get.side_effect = RuntimeError('boom')
discoverd._check_status(self.task)
inspector._check_status(self.task)
mock_get.assert_called_once_with(self.node.uuid,
auth_token='the token')
self.assertFalse(self.task.process_event.called)
def test_status_ok(self, mock_get):
mock_get.return_value = {'finished': True}
discoverd._check_status(self.task)
inspector._check_status(self.task)
mock_get.assert_called_once_with(self.node.uuid,
auth_token='the token')
self.task.process_event.assert_called_once_with('done')
def test_status_error(self, mock_get):
mock_get.return_value = {'error': 'boom'}
discoverd._check_status(self.task)
inspector._check_status(self.task)
mock_get.assert_called_once_with(self.node.uuid,
auth_token='the token')
self.task.process_event.assert_called_once_with('fail')
self.assertIn('boom', self.node.last_error)
def test_service_url(self, mock_get):
self.config(service_url='meow', group='discoverd')
self.config(service_url='meow', group='inspector')
mock_get.return_value = {'finished': True}
discoverd._check_status(self.task)
inspector._check_status(self.task)
mock_get.assert_called_once_with(self.node.uuid,
auth_token='the token',
base_url='meow')
@ -190,9 +190,9 @@ class CheckStatusTestCase(BaseTestCase):
@mock.patch.object(eventlet.greenthread, 'spawn_n',
lambda f, *a, **kw: f(*a, **kw))
@mock.patch.object(ironic_discoverd, '__version_info__', (1, 0, 0))
@mock.patch.object(ironic_inspector, '__version_info__', (1, 0, 0))
@mock.patch.object(task_manager, 'acquire', autospec=True)
@mock.patch.object(discoverd, '_check_status', autospec=True)
@mock.patch.object(inspector, '_check_status', autospec=True)
class PeriodicTaskTestCase(BaseTestCase):
def test_ok(self, mock_check, mock_acquire):
mgr = mock.MagicMock(spec=['iter_nodes'])
@ -202,7 +202,7 @@ class PeriodicTaskTestCase(BaseTestCase):
mock.MagicMock(__enter__=mock.MagicMock(return_value=task))
for task in tasks
)
discoverd.DiscoverdInspect()._periodic_check_result(
inspector.Inspector()._periodic_check_result(
mgr, mock.sentinel.context)
mock_check.assert_any_call(tasks[0])
mock_check.assert_any_call(tasks[1])
@ -212,7 +212,7 @@ class PeriodicTaskTestCase(BaseTestCase):
mgr = mock.MagicMock(spec=['iter_nodes'])
mgr.iter_nodes.return_value = [('1', 'd1'), ('2', 'd2')]
mock_acquire.side_effect = exception.NodeLocked("boom")
discoverd.DiscoverdInspect()._periodic_check_result(
inspector.Inspector()._periodic_check_result(
mgr, mock.sentinel.context)
self.assertFalse(mock_check.called)
self.assertEqual(2, mock_acquire.call_count)

4
ironic/tests/drivers/third_party_driver_mock_specs.py
View File

@ -21,8 +21,8 @@ IBOOT_SPEC = (
'iBootInterface',
)
# ironic_discoverd
IRONIC_DISCOVERD_SPEC = (
# ironic_inspector
IRONIC_INSPECTOR_SPEC = (
'__version__',
'__version_info__',
'client',

20
ironic/tests/drivers/third_party_driver_mocks.py
View File

@ -186,14 +186,14 @@ if not pyremotevbox:
sys.modules['ironic.drivers.modules.virtualbox'])
ironic_discoverd = importutils.try_import('ironic_discoverd')
if not ironic_discoverd:
ironic_discoverd = mock.MagicMock(
spec_set=mock_specs.IRONIC_DISCOVERD_SPEC)
ironic_discoverd.__version_info__ = (1, 0, 0)
ironic_discoverd.__version__ = "1.0.0"
sys.modules['ironic_discoverd'] = ironic_discoverd
sys.modules['ironic_discoverd.client'] = ironic_discoverd.client
if 'ironic.drivers.modules.discoverd' in sys.modules:
ironic_inspector = importutils.try_import('ironic_inspector')
if not ironic_inspector:
ironic_inspector = mock.MagicMock(
spec_set=mock_specs.IRONIC_INSPECTOR_SPEC)
ironic_inspector.__version_info__ = (1, 0, 0)
ironic_inspector.__version__ = "1.0.0"
sys.modules['ironic_inspector'] = ironic_inspector
sys.modules['ironic_inspector.client'] = ironic_inspector.client
if 'ironic.drivers.modules.inspector' in sys.modules:
six.moves.reload_module(
sys.modules['ironic.drivers.modules.discoverd'])
sys.modules['ironic.drivers.modules.inspector'])

2
setup.cfg
View File

@ -40,7 +40,7 @@ ironic.drivers =
agent_vbox = ironic.drivers.agent:AgentAndVirtualBoxDriver
fake = ironic.drivers.fake:FakeDriver
fake_agent = ironic.drivers.fake:FakeAgentDriver
fake_discoverd = ironic.drivers.fake:FakeIPMIToolDiscoverdDriver
fake_inspector = ironic.drivers.fake:FakeIPMIToolInspectorDriver
fake_ipmitool = ironic.drivers.fake:FakeIPMIToolDriver
fake_ipminative = ironic.drivers.fake:FakeIPMINativeDriver
fake_ssh = ironic.drivers.fake:FakeSSHDriver