diff --git a/doc/source/http-api.rst b/doc/source/http-api.rst index d67636962..3199cec6f 100644 --- a/doc/source/http-api.rst +++ b/doc/source/http-api.rst @@ -51,7 +51,25 @@ Response body: JSON dictionary with keys: * ``finished`` (boolean) whether introspection is finished (``true`` on introspection completion or if it ends because of an error) -* ``error`` error string or ``null`` +* ``error`` error string or ``null``; ``Canceled by operator`` in + case introspection was aborted + + +Abort Running Introspection +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +``POST /v1/introspection//abort`` abort running introspection. + +Requires X-Auth-Token header with Keystone token for authentication. + +Response: + +* 202 - accepted +* 400 - bad request +* 401, 403 - missing or invalid authentication +* 404 - node cannot be found +* 409 - inspector has locked this node for processing + Get Introspection Data ~~~~~~~~~~~~~~~~~~~~~~ @@ -295,3 +313,4 @@ Version History * **1.0** version of API at the moment of introducing versioning. * **1.1** adds endpoint to retrieve stored introspection data. * **1.2** endpoints for manipulating introspection rules. +* **1.3** endpoint for canceling running introspection diff --git a/ironic_inspector/introspect.py b/ironic_inspector/introspect.py index 16601514c..fedef3466 100644 --- a/ironic_inspector/introspect.py +++ b/ironic_inspector/introspect.py @@ -174,3 +174,54 @@ def _background_introspect_locked(ironic, node_info): LOG.info(_LI('Introspection environment is ready, manual power on is ' 'required within %d seconds'), CONF.timeout, node_info=node_info) + + +def abort(uuid, token=None): + """Abort running introspection. + + :param uuid: node uuid + :param token: authentication token + :raises: Error + """ + LOG.debug('Aborting introspection for node %s', uuid) + ironic = utils.get_client(token) + node_info = node_cache.get_node(uuid, ironic=ironic, locked=False) + + # check pending operations + locked = node_info.acquire_lock(blocking=False) + if not locked: + # Node busy --- cannot abort atm + raise utils.Error(_('Node is locked, please, retry later'), + node_info=node_info, code=409) + + utils.spawn_n(_abort, node_info, ironic) + + +def _abort(node_info, ironic): + # runs in background + if node_info.finished_at is not None: + # introspection already finished; nothing to do + LOG.info(_LI('Cannot abort introspection as it is already ' + 'finished'), node_info=node_info) + node_info.release_lock() + return + + # block this node from PXE Booting the introspection image + try: + firewall.update_filters(ironic) + except Exception as exc: + # Note(mkovacik): this will be retried in firewall update + # periodic task; we continue aborting + LOG.warning(_LW('Failed to update firewall filters: %s'), exc, + node_info=node_info) + + # finish the introspection + 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(_LW('Failed to power off node: %s'), exc, + node_info=node_info) + + node_info.finished(error=_('Canceled by operator')) + LOG.info(_LI('Introspection aborted'), node_info=node_info) diff --git a/ironic_inspector/main.py b/ironic_inspector/main.py index df2dae811..c1c399d96 100644 --- a/ironic_inspector/main.py +++ b/ironic_inspector/main.py @@ -45,7 +45,7 @@ app = flask.Flask(__name__) LOG = utils.getProcessingLogger(__name__) MINIMUM_API_VERSION = (1, 0) -CURRENT_API_VERSION = (1, 2) +CURRENT_API_VERSION = (1, 3) _MIN_VERSION_HEADER = 'X-OpenStack-Ironic-Inspector-API-Minimum-Version' _MAX_VERSION_HEADER = 'X-OpenStack-Ironic-Inspector-API-Maximum-Version' _VERSION_HEADER = 'X-OpenStack-Ironic-Inspector-API-Version' @@ -209,6 +209,18 @@ def api_introspection(uuid): error=node_info.error or None) +@app.route('/v1/introspection//abort', methods=['POST']) +@convert_exceptions +def api_introspection_abort(uuid): + utils.check_auth(flask.request) + + if not uuidutils.is_uuid_like(uuid): + raise utils.Error(_('Invalid UUID value'), code=400) + + introspect.abort(uuid, token=flask.request.headers.get('X-Auth-Token')) + return '', 202 + + @app.route('/v1/introspection//data', methods=['GET']) @convert_exceptions def api_introspection_data(uuid): diff --git a/ironic_inspector/process.py b/ironic_inspector/process.py index 1447a0d62..ab22ddf3c 100644 --- a/ironic_inspector/process.py +++ b/ironic_inspector/process.py @@ -104,6 +104,12 @@ def process(introspection_data): LOG.info(_LI('Matching node is %s'), node_info.uuid, node_info=node_info, data=introspection_data) + if node_info.finished_at is not None: + # race condition or introspection canceled + raise utils.Error(_('Node processing already finished with ' + 'error: %s') % node_info.error, + node_info=node_info, code=400) + try: node = node_info.node() except exceptions.NotFound: diff --git a/ironic_inspector/test/functional.py b/ironic_inspector/test/functional.py index 7bc7597ca..821bd3bcd 100644 --- a/ironic_inspector/test/functional.py +++ b/ironic_inspector/test/functional.py @@ -159,6 +159,9 @@ class Base(base.NodeTest): def call_get_status(self, uuid): return self.call('get', '/v1/introspection/%s' % uuid).json() + def call_abort_introspect(self, uuid): + return self.call('post', '/v1/introspection/%s/abort' % uuid) + def call_continue(self, data): return self.call('post', '/v1/continue', data=data).json() @@ -361,6 +364,30 @@ class Test(Base): status = self.call_get_status(self.uuid) self.assertEqual({'finished': True, 'error': None}, status) + def test_abort_introspection(self): + self.call_introspect(self.uuid) + eventlet.greenthread.sleep(DEFAULT_SLEEP) + self.cli.node.set_power_state.assert_called_once_with(self.uuid, + 'reboot') + status = self.call_get_status(self.uuid) + self.assertEqual({'finished': False, 'error': None}, status) + + res = self.call_abort_introspect(self.uuid) + eventlet.greenthread.sleep(DEFAULT_SLEEP) + + self.assertEqual(res.status_code, 202) + status = self.call_get_status(self.uuid) + self.assertTrue(status['finished']) + self.assertEqual('Canceled by operator', status['error']) + + # Note(mkovacik): we're checking just this doesn't pass OK as + # there might be either a race condition (hard to test) that + # yields a 'Node already finished.' or an attribute-based + # look-up error from some pre-processing hooks because + # node_info.finished() deletes the look-up attributes only + # after releasing the node lock + self.call('post', '/v1/continue', self.data, expect_error=400) + @contextlib.contextmanager def mocked_server(): diff --git a/ironic_inspector/test/test_introspect.py b/ironic_inspector/test/test_introspect.py index dcbcba4e0..5875da378 100644 --- a/ironic_inspector/test/test_introspect.py +++ b/ironic_inspector/test/test_introspect.py @@ -416,3 +416,109 @@ class TestSetIpmiCredentials(BaseTest): self.assertRaises(utils.Error, introspect.introspect, self.uuid, new_ipmi_credentials=self.new_creds) + + +@mock.patch.object(utils, 'spawn_n', + lambda f, *a, **kw: f(*a, **kw) and None) +@mock.patch.object(firewall, 'update_filters', autospec=True) +@mock.patch.object(node_cache, 'get_node', autospec=True) +@mock.patch.object(utils, 'get_client', autospec=True) +class TestAbort(BaseTest): + def setUp(self): + super(TestAbort, self).setUp() + self.node_info.started_at = None + self.node_info.finished_at = None + + def test_ok(self, client_mock, get_mock, filters_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 + + 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) + filters_mock.assert_called_once_with(cli) + cli.node.set_power_state.assert_called_once_with(self.uuid, 'off') + self.node_info.finished.assert_called_once_with(error='Canceled ' + 'by operator') + + def test_node_not_found(self, client_mock, get_mock, filters_mock): + cli = self._prepare(client_mock) + exc = utils.Error('Not found.', code=404) + get_mock.side_effect = iter([exc]) + + self.assertRaisesRegexp(utils.Error, str(exc), + introspect.abort, self.uuid) + + self.assertEqual(0, filters_mock.call_count) + self.assertEqual(0, cli.node.set_power_state.call_count) + self.assertEqual(0, self.node_info.finished.call_count) + + def test_node_locked(self, client_mock, get_mock, filters_mock): + cli = self._prepare(client_mock) + get_mock.return_value = self.node_info + self.node_info.acquire_lock.return_value = False + self.node_info.started_at = time.time() + + self.assertRaisesRegexp(utils.Error, 'Node is locked, please, ' + 'retry later', introspect.abort, self.uuid) + + self.assertEqual(0, filters_mock.call_count) + self.assertEqual(0, cli.node.set_power_state.call_count) + self.assertEqual(0, self.node_info.finshed.call_count) + + def test_introspection_already_finished(self, client_mock, + get_mock, filters_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 = time.time() + + introspect.abort(self.uuid) + + self.assertEqual(0, filters_mock.call_count) + self.assertEqual(0, cli.node.set_power_state.call_count) + self.assertEqual(0, self.node_info.finshed.call_count) + + def test_firewall_update_exception(self, client_mock, get_mock, + filters_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 + filters_mock.side_effect = iter([Exception('Boom')]) + + introspect.abort(self.uuid) + + get_mock.assert_called_once_with(self.uuid, ironic=cli, + locked=False) + self.node_info.acquire_lock.assert_called_once_with(blocking=False) + filters_mock.assert_called_once_with(cli) + cli.node.set_power_state.assert_called_once_with(self.uuid, 'off') + self.node_info.finished.assert_called_once_with(error='Canceled ' + 'by operator') + + def test_node_power_off_exception(self, client_mock, get_mock, + filters_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 + cli.node.set_power_state.side_effect = iter([Exception('BadaBoom')]) + + introspect.abort(self.uuid) + + get_mock.assert_called_once_with(self.uuid, ironic=cli, + locked=False) + self.node_info.acquire_lock.assert_called_once_with(blocking=False) + filters_mock.assert_called_once_with(cli) + cli.node.set_power_state.assert_called_once_with(self.uuid, 'off') + self.node_info.finished.assert_called_once_with(error='Canceled ' + 'by operator') diff --git a/ironic_inspector/test/test_main.py b/ironic_inspector/test/test_main.py index 9b9c924b0..a6728f0a5 100644 --- a/ironic_inspector/test/test_main.py +++ b/ironic_inspector/test/test_main.py @@ -135,6 +135,50 @@ class TestApiContinue(BaseAPITest): self.assertFalse(process_mock.called) +@mock.patch.object(introspect, 'abort', autospec=True) +class TestApiAbort(BaseAPITest): + def test_ok(self, abort_mock): + abort_mock.return_value = '', 202 + + res = self.app.post('/v1/introspection/%s/abort' % self.uuid, + headers={'X-Auth-Token': 'token'}) + + abort_mock.assert_called_once_with(self.uuid, token='token') + self.assertEqual(202, res.status_code) + self.assertEqual(b'', res.data) + + def test_no_authentication(self, abort_mock): + abort_mock.return_value = b'', 202 + + res = self.app.post('/v1/introspection/%s/abort' % self.uuid) + + abort_mock.assert_called_once_with(self.uuid, token=None) + self.assertEqual(202, res.status_code) + self.assertEqual(b'', res.data) + + def test_node_not_found(self, abort_mock): + exc = utils.Error("Not Found.", code=404) + abort_mock.side_effect = iter([exc]) + + res = self.app.post('/v1/introspection/%s/abort' % self.uuid) + + abort_mock.assert_called_once_with(self.uuid, token=None) + self.assertEqual(404, res.status_code) + data = json.loads(str(res.data.decode())) + self.assertEqual(str(exc), data['error']['message']) + + def test_abort_failed(self, abort_mock): + exc = utils.Error("Locked.", code=409) + abort_mock.side_effect = iter([exc]) + + res = self.app.post('/v1/introspection/%s/abort' % self.uuid) + + abort_mock.assert_called_once_with(self.uuid, token=None) + self.assertEqual(409, res.status_code) + data = json.loads(res.data.decode()) + self.assertEqual(str(exc), data['error']['message']) + + class TestApiGetStatus(BaseAPITest): @mock.patch.object(node_cache, 'get_node', autospec=True) def test_get_introspection_in_progress(self, get_mock): diff --git a/ironic_inspector/test/test_process.py b/ironic_inspector/test/test_process.py index 245a25547..163c65789 100644 --- a/ironic_inspector/test/test_process.py +++ b/ironic_inspector/test/test_process.py @@ -127,6 +127,18 @@ class TestProcess(BaseTest): self.assertFalse(process_mock.called) pop_mock.return_value.finished.assert_called_once_with(error=mock.ANY) + @prepare_mocks + def test_already_finished(self, cli, pop_mock, process_mock): + old_finished_at = pop_mock.return_value.finished_at + pop_mock.return_value.finished_at = time.time() + try: + self.assertRaisesRegexp(utils.Error, 'already finished', + process.process, self.data) + self.assertFalse(process_mock.called) + self.assertFalse(pop_mock.return_value.finished.called) + finally: + pop_mock.return_value.finished_at = old_finished_at + @prepare_mocks def test_expected_exception(self, cli, pop_mock, process_mock): process_mock.side_effect = iter([utils.Error('boom')]) diff --git a/releasenotes/notes/abort-introspection-ae5cb5a9fbacd2ac.yaml b/releasenotes/notes/abort-introspection-ae5cb5a9fbacd2ac.yaml new file mode 100644 index 000000000..8800269fa --- /dev/null +++ b/releasenotes/notes/abort-introspection-ae5cb5a9fbacd2ac.yaml @@ -0,0 +1,4 @@ +--- +features: + - Introduced API "POST /v1/introspection//abort" for aborting + the introspection process.