Add manage_boot parameter to introspection API

Adds a new node field "manage_boot" to store this value. When it is set
to False, neither boot device nor power state are touched for this node.
Instead, we expect a 3rd party to handle them.

We still manage the PXE filter because the node may need DHCP.

Change-Id: Id3585bd32138a069dfcfc0ab04ee4f5f10f0a5ea
Story: #1528920
Task: #11338
This commit is contained in:
Dmitry Tantsur 2016-05-16 14:44:51 +02:00 committed by Dmitry Tantsur
parent c4821fd1d5
commit e7c3218f71
16 changed files with 262 additions and 46 deletions

View File

@ -15,6 +15,11 @@ done prior to calling the endpoint.
Requires X-Auth-Token header with Keystone token for authentication.
Optional parameter:
* ``manage_boot`` boolean value, whether to manage boot (boot device, power
and firewall) for a node. Defaults to true.
Response:
* 202 - accepted introspection request
@ -390,3 +395,4 @@ Version History
* **1.11** adds invert&multiple fields into rules response data
* **1.12** this version indicates that support for setting IPMI credentials
was completely removed from API (all versions).
* **1.13** adds ``manage_boot`` parameter for the introspection API.

View File

@ -36,7 +36,7 @@ def get_transport():
def get_client():
target = messaging.Target(topic=TOPIC, server=SERVER_NAME,
version='1.0')
version='1.1')
transport = get_transport()
return messaging.RPCClient(transport, target)
@ -48,7 +48,7 @@ def get_server():
if _SERVER is None:
transport = get_transport()
target = messaging.Target(topic=TOPIC, server=SERVER_NAME,
version='1.0')
version='1.1')
mgr = manager.ConductorManager()
_SERVER = messaging.get_rpc_server(
transport, target, [mgr], executor='eventlet',

View File

@ -20,13 +20,14 @@ from ironic_inspector import utils
class ConductorManager(object):
"""ironic inspector conductor manager"""
RPC_API_VERSION = '1.0'
RPC_API_VERSION = '1.1'
target = messaging.Target(version=RPC_API_VERSION)
@messaging.expected_exceptions(utils.Error)
def do_introspection(self, context, node_id, token=None):
introspect.introspect(node_id, token=token)
def do_introspection(self, context, node_id, token=None,
manage_boot=True):
introspect.introspect(node_id, token=token, manage_boot=manage_boot)
@messaging.expected_exceptions(utils.Error)
def do_abort(self, context, node_id, token=None):

View File

@ -69,7 +69,13 @@ _OPTS = [
help=_('Path to the rootwrap configuration file to use for '
'running commands as root')),
cfg.IntOpt('api_max_limit', default=1000, min=1,
help=_('Limit the number of elements an API list-call returns'))
help=_('Limit the number of elements an API list-call '
'returns')),
cfg.BoolOpt('can_manage_boot', default=True,
help=_('Whether the current installation of ironic-inspector '
'can manage PXE booting of nodes. If set to False, '
'the API will reject introspection requests with '
'manage_boot missing or set to True.'))
]

View File

@ -57,6 +57,7 @@ class Node(Base):
started_at = Column(DateTime, nullable=True)
finished_at = Column(DateTime, nullable=True)
error = Column(Text, nullable=True)
manage_boot = Column(Boolean, nullable=True, default=True)
# version_id is being tracked in the NodeInfo object
# for the sake of consistency. See also SQLAlchemy docs:

View File

@ -34,10 +34,11 @@ _LAST_INTROSPECTION_TIME = 0
_LAST_INTROSPECTION_LOCK = semaphore.BoundedSemaphore()
def introspect(node_id, token=None):
def introspect(node_id, manage_boot=True, token=None):
"""Initiate hardware properties introspection for a given node.
:param node_id: node UUID or name
:param manage_boot: whether to manage boot for this node
:param token: authentication token
:raises: Error
"""
@ -45,15 +46,17 @@ def introspect(node_id, token=None):
node = ir_utils.get_node(node_id, ironic=ironic)
ir_utils.check_provision_state(node)
validation = ironic.node.validate(node.uuid)
if not validation.power['result']:
msg = _('Failed validation of power interface, reason: %s')
raise utils.Error(msg % validation.power['reason'],
node_info=node)
if manage_boot:
validation = ironic.node.validate(node.uuid)
if not validation.power['result']:
msg = _('Failed validation of power interface, reason: %s')
raise utils.Error(msg % validation.power['reason'],
node_info=node)
bmc_address = ir_utils.get_ipmi_address(node)
node_info = node_cache.start_introspection(node.uuid,
bmc_address=bmc_address,
manage_boot=manage_boot,
ironic=ironic)
utils.executor().submit(_background_introspect, node_info, ironic)
@ -98,21 +101,26 @@ def _background_introspect_locked(node_info, ironic):
LOG.info('The following attributes will be used for look up: %s',
attrs, node_info=node_info)
try:
ironic.node.set_boot_device(node_info.uuid, 'pxe',
persistent=False)
except Exception as exc:
LOG.warning('Failed to set boot device to PXE: %s',
exc, node_info=node_info)
if node_info.manage_boot:
try:
ironic.node.set_boot_device(node_info.uuid, 'pxe',
persistent=False)
except Exception as exc:
LOG.warning('Failed to set boot device to PXE: %s',
exc, node_info=node_info)
try:
ironic.node.set_power_state(node_info.uuid, 'reboot')
except Exception as exc:
raise utils.Error(_('Failed to power on the node, check it\'s '
'power management configuration: %s'),
exc, node_info=node_info)
LOG.info('Introspection started successfully',
node_info=node_info)
try:
ironic.node.set_power_state(node_info.uuid, 'reboot')
except Exception as exc:
raise utils.Error(_('Failed to power on the node, check it\'s '
'power management configuration: %s'),
exc, node_info=node_info)
LOG.info('Introspection started successfully',
node_info=node_info)
else:
LOG.info('Introspection environment is ready, external power on '
'is required within %d seconds', CONF.timeout,
node_info=node_info)
def abort(node_id, token=None):
@ -142,11 +150,12 @@ def _abort(node_info, ironic):
# runs in background
LOG.debug('Forcing power-off', node_info=node_info)
try:
ironic.node.set_power_state(node_info.uuid, 'off')
except Exception as exc:
LOG.warning('Failed to power off node: %s', exc,
node_info=node_info)
if node_info.manage_boot:
try:
ironic.node.set_power_state(node_info.uuid, 'off')
except Exception as exc:
LOG.warning('Failed to power off node: %s', exc,
node_info=node_info)
node_info.finished(istate.Events.abort_end,
error=_('Canceled by operator'))

View File

@ -15,6 +15,7 @@ import os
import re
import flask
from oslo_utils import strutils
from oslo_utils import uuidutils
import six
import werkzeug
@ -39,7 +40,7 @@ app = flask.Flask(__name__)
LOG = utils.getProcessingLogger(__name__)
MINIMUM_API_VERSION = (1, 0)
CURRENT_API_VERSION = (1, 12)
CURRENT_API_VERSION = (1, 13)
DEFAULT_API_VERSION = CURRENT_API_VERSION
_LOGGING_EXCLUDED_KEYS = ('logs',)
@ -239,8 +240,23 @@ def api_continue():
methods=['GET', 'POST'])
def api_introspection(node_id):
if flask.request.method == 'POST':
args = flask.request.args
manage_boot = args.get('manage_boot', 'True')
try:
manage_boot = strutils.bool_from_string(manage_boot, strict=True)
except ValueError:
raise utils.Error(_('Invalid boolean value for manage_boot: %s') %
manage_boot, code=400)
if manage_boot and not CONF.can_manage_boot:
raise utils.Error(_('Managed boot is requested, but this '
'installation cannot manage boot ('
'(can_manage_boot set to False)'),
code=400)
client = rpc.get_client()
client.call({}, 'do_introspection', node_id=node_id,
manage_boot=manage_boot,
token=flask.request.headers.get('X-Auth-Token'))
return '', 202
else:

View File

@ -0,0 +1,33 @@
# 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.
"""Add manage_boot to nodes
Revision ID: 2970d2d44edc
Revises: e169a4a81d88
Create Date: 2016-05-16 14:03:02.861672
"""
from alembic import op
import sqlalchemy as sa
# revision identifiers, used by Alembic.
revision = '2970d2d44edc'
down_revision = '18440d0834af'
branch_labels = None
depends_on = None
def upgrade():
op.add_column('nodes', sa.Column('manage_boot', sa.Boolean(),
nullable=True, default=True))

View File

@ -71,7 +71,7 @@ class NodeInfo(object):
def __init__(self, uuid, version_id=None, state=None, started_at=None,
finished_at=None, error=None, node=None, ports=None,
ironic=None, lock=None):
ironic=None, lock=None, manage_boot=True):
self.uuid = uuid
self.started_at = started_at
self.finished_at = finished_at
@ -85,6 +85,9 @@ class NodeInfo(object):
self._ports = ports
self._attributes = None
self._ironic = ironic
# On upgrade existing records will have manage_boot=NULL, which is
# equivalent to True actually.
self._manage_boot = manage_boot if manage_boot is not None else True
# This is a lock on a node UUID, not on a NodeInfo object
self._lock = lock if lock is not None else _get_lock(uuid)
# Whether lock was acquired using this NodeInfo object
@ -262,6 +265,11 @@ class NodeInfo(object):
self._ironic = ir_utils.get_client()
return self._ironic
@property
def manage_boot(self):
"""Whether to manage boot for this node."""
return self._manage_boot
def set_option(self, name, value):
"""Set an option for a node."""
encoded = json.dumps(value)
@ -315,7 +323,7 @@ class NodeInfo(object):
"""Construct NodeInfo from a database row."""
fields = {key: row[key]
for key in ('uuid', 'version_id', 'state', 'started_at',
'finished_at', 'error')}
'finished_at', 'error', 'manage_boot')}
return cls(ironic=ironic, lock=lock, node=node, **fields)
def invalidate_cache(self):
@ -672,7 +680,7 @@ def start_introspection(uuid, **kwargs):
return add_node(uuid, state, **kwargs)
def add_node(uuid, state, **attributes):
def add_node(uuid, state, manage_boot=True, **attributes):
"""Store information about a node under introspection.
All existing information about this node is dropped.
@ -680,6 +688,7 @@ def add_node(uuid, state, **attributes):
:param uuid: Ironic node UUID
:param state: The initial state of the node
:param manage_boot: whether to manage boot for this node
:param attributes: attributes known about this node (like macs, BMC etc);
also ironic client instance may be passed under 'ironic'
:returns: NodeInfo
@ -689,10 +698,10 @@ def add_node(uuid, state, **attributes):
_delete_node(uuid)
version_id = uuidutils.generate_uuid()
db.Node(uuid=uuid, state=state, version_id=version_id,
started_at=started_at).save(session)
started_at=started_at, manage_boot=manage_boot).save(session)
node_info = NodeInfo(uuid=uuid, state=state, started_at=started_at,
version_id=version_id,
version_id=version_id, manage_boot=manage_boot,
ironic=attributes.pop('ironic', None))
for (name, value) in attributes.items():
if not value:
@ -738,8 +747,9 @@ def introspection_active():
def active_macs():
"""List all MAC's that are on introspection right now."""
return ({x.value for x in db.model_query(db.Attribute.value).
filter_by(name=MACS_ATTRIBUTE)})
query = (db.model_query(db.Attribute.value).join(db.Node)
.filter(db.Attribute.name == MACS_ATTRIBUTE))
return {x.value for x in query}
def _list_node_uuids():

View File

@ -167,9 +167,11 @@ class Base(base.NodeTest):
raise AssertionError(msg)
return res
def call_introspect(self, uuid, **kwargs):
def call_introspect(self, uuid, manage_boot=True, **kwargs):
endpoint = '/v1/introspection/%s' % uuid
return self.call('post', endpoint, **kwargs)
if manage_boot is not None:
endpoint = '%s?manage_boot=%s' % (endpoint, manage_boot)
return self.call('post', endpoint)
def call_get_status(self, uuid, **kwargs):
return self.call('get', '/v1/introspection/%s' % uuid, **kwargs).json()
@ -257,6 +259,7 @@ class Test(Base):
self.cli.port.create.assert_called_once_with(
node_uuid=self.uuid, address='11:22:33:44:55:66', extra={},
pxe_enabled=True)
self.assertTrue(self.cli.node.set_boot_device.called)
status = self.call_get_status(self.uuid)
self.check_status(status, finished=True, state=istate.States.finished)
@ -358,6 +361,24 @@ class Test(Base):
status_.get('links')[0].get('href')).path).json()
for status_ in statuses])
def test_manage_boot(self):
self.call_introspect(self.uuid, manage_boot=False)
eventlet.greenthread.sleep(DEFAULT_SLEEP)
self.assertFalse(self.cli.node.set_power_state.called)
status = self.call_get_status(self.uuid)
self.check_status(status, finished=False, state=istate.States.waiting)
res = self.call_continue(self.data)
self.assertEqual({'uuid': self.uuid}, res)
eventlet.greenthread.sleep(DEFAULT_SLEEP)
self.cli.node.update.assert_called_with(self.uuid, mock.ANY)
self.assertFalse(self.cli.node.set_boot_device.called)
status = self.call_get_status(self.uuid)
self.check_status(status, finished=True, state=istate.States.finished)
def test_rules_api(self):
res = self.call_list_rules()
self.assertEqual([], res)

View File

@ -67,6 +67,7 @@ class TestIntrospect(BaseTest):
start_mock.assert_called_once_with(self.uuid,
bmc_address=self.bmc_address,
manage_boot=True,
ironic=cli)
self.node_info.ports.assert_called_once_with()
self.node_info.add_attribute.assert_called_once_with('mac',
@ -92,6 +93,7 @@ class TestIntrospect(BaseTest):
start_mock.assert_called_once_with(self.uuid,
bmc_address=None,
manage_boot=True,
ironic=cli)
self.node_info.ports.assert_called_once_with()
self.node_info.add_attribute.assert_called_once_with('mac',
@ -115,6 +117,7 @@ class TestIntrospect(BaseTest):
start_mock.assert_called_with(self.uuid,
bmc_address=self.bmc_address,
manage_boot=True,
ironic=cli)
def test_power_failure(self, client_mock, start_mock):
@ -129,6 +132,7 @@ class TestIntrospect(BaseTest):
start_mock.assert_called_once_with(self.uuid,
bmc_address=self.bmc_address,
manage_boot=True,
ironic=cli)
cli.node.set_boot_device.assert_called_once_with(self.uuid,
'pxe',
@ -151,6 +155,7 @@ class TestIntrospect(BaseTest):
start_mock.assert_called_once_with(self.uuid,
bmc_address=self.bmc_address,
manage_boot=True,
ironic=cli)
self.assertFalse(cli.node.set_boot_device.called)
start_mock.return_value.finished.assert_called_once_with(
@ -169,6 +174,7 @@ class TestIntrospect(BaseTest):
start_mock.assert_called_once_with(self.uuid,
bmc_address=self.bmc_address,
manage_boot=True,
ironic=cli)
self.assertFalse(self.node_info.add_attribute.called)
self.assertFalse(self.sync_filter_mock.called)
@ -315,6 +321,27 @@ class TestIntrospect(BaseTest):
# updated to the current time.time()
self.assertEqual(100, introspect._LAST_INTROSPECTION_TIME)
def test_no_manage_boot(self, client_mock, add_mock):
cli = self._prepare(client_mock)
self.node_info.manage_boot = False
add_mock.return_value = self.node_info
introspect.introspect(self.node.uuid, manage_boot=False)
cli.node.get.assert_called_once_with(self.uuid)
add_mock.assert_called_once_with(self.uuid,
bmc_address=self.bmc_address,
manage_boot=False,
ironic=cli)
self.node_info.ports.assert_called_once_with()
self.node_info.add_attribute.assert_called_once_with('mac',
self.macs)
self.sync_filter_mock.assert_called_with(cli)
self.assertFalse(cli.node.validate.called)
self.assertFalse(cli.node.set_boot_device.called)
self.assertFalse(cli.node.set_power_state.called)
@mock.patch.object(node_cache, 'get_node', autospec=True)
@mock.patch.object(ir_utils, 'get_client', autospec=True)
@ -346,6 +373,25 @@ class TestAbort(BaseTest):
introspect.istate.Events.abort_end, error='Canceled by operator')
self.node_info.fsm_event.assert_has_calls(self.fsm_calls)
def test_no_manage_boot(self, client_mock, get_mock):
cli = self._prepare(client_mock)
get_mock.return_value = self.node_info
self.node_info.acquire_lock.return_value = True
self.node_info.started_at = time.time()
self.node_info.finished_at = None
self.node_info.manage_boot = False
introspect.abort(self.node.uuid)
get_mock.assert_called_once_with(self.uuid, ironic=cli,
locked=False)
self.node_info.acquire_lock.assert_called_once_with(blocking=False)
self.sync_filter_mock.assert_called_once_with(cli)
self.assertFalse(cli.node.set_power_state.called)
self.node_info.finished.assert_called_once_with(
introspect.istate.Events.abort_end, error='Canceled by operator')
self.node_info.fsm_event.assert_has_calls(self.fsm_calls)
def test_node_not_found(self, client_mock, get_mock):
cli = self._prepare(client_mock)
exc = utils.Error('Not found.', code=404)

View File

@ -66,11 +66,11 @@ class TestApiIntrospect(BaseAPITest):
self.assertEqual(202, res.status_code)
self.client_mock.call.assert_called_once_with({}, 'do_introspection',
node_id=self.uuid,
manage_boot=True,
token=None)
def test_intospect_failed(self):
self.client_mock.call.side_effect = utils.Error("boom")
res = self.app.post('/v1/introspection/%s' % self.uuid)
self.assertEqual(400, res.status_code)
@ -79,8 +79,40 @@ class TestApiIntrospect(BaseAPITest):
json.loads(res.data.decode('utf-8'))['error']['message'])
self.client_mock.call.assert_called_once_with({}, 'do_introspection',
node_id=self.uuid,
manage_boot=True,
token=None)
def test_introspect_no_manage_boot(self):
res = self.app.post('/v1/introspection/%s?manage_boot=0' % self.uuid)
self.assertEqual(202, res.status_code)
self.client_mock.call.assert_called_once_with({}, 'do_introspection',
node_id=self.uuid,
manage_boot=False,
token=None)
def test_introspect_can_manage_boot_false(self):
CONF.set_override('can_manage_boot', False)
res = self.app.post('/v1/introspection/%s?manage_boot=0' % self.uuid)
self.assertEqual(202, res.status_code)
self.client_mock.call.assert_called_once_with({}, 'do_introspection',
node_id=self.uuid,
manage_boot=False,
token=None)
def test_introspect_can_manage_boot_false_failed(self):
CONF.set_override('can_manage_boot', False)
res = self.app.post('/v1/introspection/%s' % self.uuid)
self.assertEqual(400, res.status_code)
self.assertFalse(self.client_mock.call.called)
def test_introspect_wrong_manage_boot(self):
res = self.app.post('/v1/introspection/%s?manage_boot=foo' % self.uuid)
self.assertEqual(400, res.status_code)
self.assertFalse(self.client_mock.call.called)
self.assertEqual(
'Invalid boolean value for manage_boot: foo',
json.loads(res.data.decode('utf-8'))['error']['message'])
@mock.patch.object(utils, 'check_auth', autospec=True)
def test_introspect_failed_authentication(self, auth_mock):
CONF.set_override('auth_strategy', 'keystone')

View File

@ -37,7 +37,16 @@ class TestManagerIntrospect(BaseManagerTest):
def test_do_introspect(self, introspect_mock):
self.manager.do_introspection(self.context, self.uuid, self.token)
introspect_mock.assert_called_once_with(self.uuid, self.token)
introspect_mock.assert_called_once_with(self.uuid, token=self.token,
manage_boot=True)
@mock.patch.object(introspect, 'introspect', autospec=True)
def test_do_introspect_with_manage_boot(self, introspect_mock):
self.manager.do_introspection(self.context, self.uuid, self.token,
False)
introspect_mock.assert_called_once_with(self.uuid, token=self.token,
manage_boot=False)
@mock.patch.object(introspect, 'introspect', autospec=True)
def test_introspect_failed(self, introspect_mock):
@ -48,7 +57,8 @@ class TestManagerIntrospect(BaseManagerTest):
self.context, self.uuid, self.token)
self.assertEqual(utils.Error, exc.exc_info[0])
introspect_mock.assert_called_once_with(self.uuid, token=None)
introspect_mock.assert_called_once_with(self.uuid, token=None,
manage_boot=True)
class TestManagerAbort(BaseManagerTest):

View File

@ -433,6 +433,14 @@ class MigrationCheckersMixin(object):
self.assertEqual('foo', row.name)
self.assertEqual('bar', row.value)
def _check_2970d2d44edc(self, engine, data):
nodes = db_utils.get_table(engine, 'nodes')
data = {'uuid': 'abcd'}
nodes.insert().execute(data)
n = nodes.select(nodes.c.uuid == 'abcd').execute().first()
self.assertIsNone(n['manage_boot'])
def test_upgrade_and_version(self):
with patch_with_engine(self.engine):
self.migration_ext.upgrade('head')

View File

@ -120,15 +120,22 @@ class TestNodeCache(test_base.NodeTest):
def test_active_macs(self):
session = db.get_writer_session()
uuid2 = uuidutils.generate_uuid()
with session.begin():
db.Node(uuid=self.node.uuid,
state=istate.States.starting).save(session)
db.Node(uuid=uuid2,
state=istate.States.starting,
manage_boot=False).save(session)
values = [('mac', '11:22:11:22:11:22', self.uuid),
('mac', '22:11:22:11:22:11', self.uuid)]
('mac', '22:11:22:11:22:11', self.uuid),
('mac', 'aa:bb:cc:dd:ee:ff', uuid2)]
for value in values:
db.Attribute(uuid=uuidutils.generate_uuid(), name=value[0],
value=value[1], node_uuid=value[2]).save(session)
self.assertEqual({'11:22:11:22:11:22', '22:11:22:11:22:11'},
self.assertEqual({'11:22:11:22:11:22', '22:11:22:11:22:11',
# We still need to serve DHCP to unmanaged nodes
'aa:bb:cc:dd:ee:ff'},
node_cache.active_macs())
def test__list_node_uuids(self):

View File

@ -0,0 +1,10 @@
---
features:
- |
Adds new parameter ``manage_boot`` to the introspection API to allow
disabling boot management (setting the boot device and rebooting)
for a specific node. If it is set to ``False``, the boot is supposed
to be managed by a 3rd party.
If the new option ``can_manage_boot`` is set to ``False`` (the default is
``True), then ``manage_boot`` must be explicitly set to ``False``.