diff --git a/doc/source/http-api.rst b/doc/source/http-api.rst index 44a294f2d..82d38a429 100644 --- a/doc/source/http-api.rst +++ b/doc/source/http-api.rst @@ -45,6 +45,7 @@ Response body: JSON dictionary with keys: * ``finished`` (boolean) whether introspection is finished (``true`` on introspection completion or if it ends because of an error) +* ``state`` state of the introspection * ``error`` error string or ``null``; ``Canceled by operator`` in case introspection was aborted * ``uuid`` node UUID @@ -76,7 +77,8 @@ Response body: a JSON object containing a list of status objects:: { 'introspection': [ { - 'finished': true, + 'finished': false, + 'state': 'waiting', 'error': null, ... }, @@ -88,6 +90,7 @@ Each status object contains these keys: * ``finished`` (boolean) whether introspection is finished (``true`` on introspection completion or if it ends because of an error) +* ``state`` state of the introspection * ``error`` error string or ``null``; ``Canceled by operator`` in case introspection was aborted * ``uuid`` node UUID @@ -392,3 +395,5 @@ Version History * **1.8** support for listing all introspection statuses. * **1.9** de-activate setting IPMI credentials, if IPMI credentials are requested, API gets HTTP 400 response. +* **1.10** adds node state to the GET /v1/introspection/ and + GET /v1/introspection API response data. diff --git a/ironic_inspector/main.py b/ironic_inspector/main.py index 85b34ae73..b2fa657c7 100644 --- a/ironic_inspector/main.py +++ b/ironic_inspector/main.py @@ -51,7 +51,7 @@ MINIMUM_API_VERSION = (1, 0) # TODO(dtantsur): set to the current version as soon we move setting IPMI # credentials support completely. DEFAULT_API_VERSION = (1, 8) -CURRENT_API_VERSION = (1, 9) +CURRENT_API_VERSION = (1, 10) _LOGGING_EXCLUDED_KEYS = ('logs',) @@ -149,6 +149,7 @@ def generate_introspection_status(node): status = {} status['uuid'] = node.uuid status['finished'] = bool(node.finished_at) + status['state'] = node.state status['started_at'] = started_at status['finished_at'] = finished_at status['error'] = node.error diff --git a/ironic_inspector/test/functional.py b/ironic_inspector/test/functional.py index 1fadea27c..0110e2a7e 100644 --- a/ironic_inspector/test/functional.py +++ b/ironic_inspector/test/functional.py @@ -202,18 +202,20 @@ class Base(base.NodeTest): def call_get_rule(self, uuid, **kwargs): return self.call('get', '/v1/rules/' + uuid, **kwargs).json() - def _fake_status(self, finished=mock.ANY, error=mock.ANY, + def _fake_status(self, finished=mock.ANY, state=mock.ANY, error=mock.ANY, started_at=mock.ANY, finished_at=mock.ANY, links=mock.ANY): return {'uuid': self.uuid, 'finished': finished, 'error': error, - 'finished_at': finished_at, 'started_at': started_at, + 'state': state, 'finished_at': finished_at, + 'started_at': started_at, 'links': [{u'href': u'%s/v1/introspection/%s' % (self.ROOT_URL, self.uuid), u'rel': u'self'}]} - def check_status(self, status, finished, error=None): + def check_status(self, status, finished, state, error=None): self.assertEqual( self._fake_status(finished=finished, + state=state, finished_at=finished and mock.ANY or None, error=error), status @@ -242,7 +244,7 @@ class Test(Base): 'reboot') status = self.call_get_status(self.uuid) - self.check_status(status, finished=False) + self.check_status(status, finished=False, state=istate.States.waiting) res = self.call_continue(self.data) self.assertEqual({'uuid': self.uuid}, res) @@ -254,7 +256,7 @@ class Test(Base): node_uuid=self.uuid, address='11:22:33:44:55:66', extra={}) status = self.call_get_status(self.uuid) - self.check_status(status, finished=True) + self.check_status(status, finished=True, state=istate.States.finished) def test_bmc_with_client_id(self): self.pxe_mac = self.macs[2] @@ -269,7 +271,7 @@ class Test(Base): 'reboot') status = self.call_get_status(self.uuid) - self.check_status(status, finished=False) + self.check_status(status, finished=False, state=istate.States.waiting) res = self.call_continue(self.data) self.assertEqual({'uuid': self.uuid}, res) @@ -282,7 +284,7 @@ class Test(Base): extra={'client-id': self.client_id}) status = self.call_get_status(self.uuid) - self.check_status(status, finished=True) + self.check_status(status, finished=True, state=istate.States.finished) def test_setup_ipmi(self): patch_credentials = [ @@ -298,7 +300,7 @@ class Test(Base): self.assertFalse(self.cli.node.set_power_state.called) status = self.call_get_status(self.uuid) - self.check_status(status, finished=False) + self.check_status(status, finished=False, state=istate.States.waiting) res = self.call_continue(self.data) self.assertEqual('admin', res['ipmi_username']) @@ -312,7 +314,7 @@ class Test(Base): node_uuid=self.uuid, address='11:22:33:44:55:66', extra={}) status = self.call_get_status(self.uuid) - self.check_status(status, finished=True) + self.check_status(status, finished=True, state=istate.States.finished) def test_introspection_statuses(self): self.call_introspect(self.uuid) @@ -339,7 +341,7 @@ class Test(Base): eventlet.greenthread.sleep(DEFAULT_SLEEP) status = self.call_get_status(self.uuid) - self.check_status(status, finished=True) + self.check_status(status, finished=True, state=istate.States.finished) # fetch all statuses and db nodes to assert pagination statuses = self.call_get_statuses().get('introspection') @@ -504,7 +506,7 @@ class Test(Base): 'reboot') status = self.call_get_status(self.uuid) - self.check_status(status, finished=False) + self.check_status(status, finished=False, state=istate.States.waiting) res = self.call_continue(self.data) self.assertEqual({'uuid': self.uuid}, res) @@ -515,7 +517,7 @@ class Test(Base): node_uuid=self.uuid, address='11:22:33:44:55:66', extra={}) status = self.call_get_status(self.uuid) - self.check_status(status, finished=True) + self.check_status(status, finished=True, state=istate.States.finished) def test_abort_introspection(self): self.call_introspect(self.uuid) @@ -523,7 +525,7 @@ class Test(Base): self.cli.node.set_power_state.assert_called_once_with(self.uuid, 'reboot') status = self.call_get_status(self.uuid) - self.check_status(status, finished=False) + self.check_status(status, finished=False, state=istate.States.waiting) res = self.call_abort_introspect(self.uuid) eventlet.greenthread.sleep(DEFAULT_SLEEP) @@ -562,7 +564,7 @@ class Test(Base): status = self.call_get_status(self.uuid) inspect_started_at = timeutils.parse_isotime(status['started_at']) - self.check_status(status, finished=True) + self.check_status(status, finished=True, state=istate.States.finished) res = self.call_reapply(self.uuid) self.assertEqual(202, res.status_code) @@ -570,7 +572,7 @@ class Test(Base): eventlet.greenthread.sleep(DEFAULT_SLEEP) status = self.call_get_status(self.uuid) - self.check_status(status, finished=True) + self.check_status(status, finished=True, state=istate.States.finished) # checks the started_at updated in DB is correct reapply_started_at = timeutils.parse_isotime(status['started_at']) @@ -609,52 +611,6 @@ class Test(Base): self.assertEqual(store_processing_call, store_mock.call_args_list[-1]) - # TODO(milan): remove the test case in favor of other tests once - # the introspection status endpoint exposes the state information - @mock.patch.object(swift, 'store_introspection_data', autospec=True) - @mock.patch.object(swift, 'get_introspection_data', autospec=True) - def test_state_transitions(self, get_mock, store_mock): - """Assert state transitions work as expected.""" - cfg.CONF.set_override('store_data', 'swift', 'processing') - - # ramdisk data copy - # please mind the data is changed during processing - ramdisk_data = json.dumps(copy.deepcopy(self.data)) - get_mock.return_value = ramdisk_data - - self.call_introspect(self.uuid) - reboot_call = mock.call(self.uuid, 'reboot') - self.cli.node.set_power_state.assert_has_calls([reboot_call]) - - eventlet.greenthread.sleep(DEFAULT_SLEEP) - row = self.db_row() - self.assertEqual(istate.States.waiting, row.state) - - self.call_continue(self.data) - eventlet.greenthread.sleep(DEFAULT_SLEEP) - - row = self.db_row() - self.assertEqual(istate.States.finished, row.state) - self.assertIsNone(row.error) - version_id = row.version_id - - self.call_reapply(self.uuid) - eventlet.greenthread.sleep(DEFAULT_SLEEP) - row = self.db_row() - self.assertEqual(istate.States.finished, row.state) - self.assertIsNone(row.error) - # the finished state was visited from the reapplying state - self.assertNotEqual(version_id, row.version_id) - - self.call_introspect(self.uuid) - eventlet.greenthread.sleep(DEFAULT_SLEEP) - row = self.db_row() - self.assertEqual(istate.States.waiting, row.state) - self.call_abort_introspect(self.uuid) - row = self.db_row() - self.assertEqual(istate.States.error, row.state) - self.assertEqual('Canceled by operator', row.error) - @mock.patch.object(swift, 'store_introspection_data', autospec=True) @mock.patch.object(swift, 'get_introspection_data', autospec=True) def test_edge_state_transitions(self, get_mock, store_mock): @@ -670,30 +626,26 @@ class Test(Base): self.call_introspect(self.uuid) self.call_introspect(self.uuid) eventlet.greenthread.sleep(DEFAULT_SLEEP) - # TODO(milan): switch to API once the introspection status - # endpoint exposes the state information - row = self.db_row() - self.assertEqual(istate.States.waiting, row.state) + status = self.call_get_status(self.uuid) + self.check_status(status, finished=False, state=istate.States.waiting) # an error -start-> starting state transition is possible self.call_abort_introspect(self.uuid) self.call_introspect(self.uuid) eventlet.greenthread.sleep(DEFAULT_SLEEP) - row = self.db_row() - self.assertEqual(istate.States.waiting, row.state) + status = self.call_get_status(self.uuid) + self.check_status(status, finished=False, state=istate.States.waiting) # double abort works self.call_abort_introspect(self.uuid) - row = self.db_row() - version_id = row.version_id - error = row.error - self.assertEqual(istate.States.error, row.state) + status = self.call_get_status(self.uuid) + error = status['error'] + self.check_status(status, finished=True, state=istate.States.error, + error=error) self.call_abort_introspect(self.uuid) - row = self.db_row() - self.assertEqual(istate.States.error, row.state) - # assert the error didn't change - self.assertEqual(error, row.error) - self.assertEqual(version_id, row.version_id) + status = self.call_get_status(self.uuid) + self.check_status(status, finished=True, state=istate.States.error, + error=error) # preventing stale data race condition # waiting -> processing is a strict state transition @@ -704,26 +656,24 @@ class Test(Base): with db.ensure_transaction() as session: row.save(session) self.call_continue(self.data, expect_error=400) - row = self.db_row() - self.assertEqual(istate.States.error, row.state) - self.assertIn('no defined transition', row.error) - + status = self.call_get_status(self.uuid) + self.check_status(status, finished=True, state=istate.States.error, + error=mock.ANY) + self.assertIn('no defined transition', status['error']) # multiple reapply calls self.call_introspect(self.uuid) eventlet.greenthread.sleep(DEFAULT_SLEEP) self.call_continue(self.data) eventlet.greenthread.sleep(DEFAULT_SLEEP) self.call_reapply(self.uuid) - row = self.db_row() - version_id = row.version_id - self.assertEqual(istate.States.finished, row.state) - self.assertIsNone(row.error) + status = self.call_get_status(self.uuid) + self.check_status(status, finished=True, state=istate.States.finished, + error=None) self.call_reapply(self.uuid) # assert an finished -reapply-> reapplying -> finished state transition - row = self.db_row() - self.assertEqual(istate.States.finished, row.state) - self.assertIsNone(row.error) - self.assertNotEqual(version_id, row.version_id) + status = self.call_get_status(self.uuid) + self.check_status(status, finished=True, state=istate.States.finished, + error=None) def test_without_root_disk(self): del self.data['root_disk'] @@ -737,7 +687,7 @@ class Test(Base): 'reboot') status = self.call_get_status(self.uuid) - self.check_status(status, finished=False) + self.check_status(status, finished=False, state=istate.States.waiting) res = self.call_continue(self.data) self.assertEqual({'uuid': self.uuid}, res) @@ -749,7 +699,7 @@ class Test(Base): node_uuid=self.uuid, extra={}, address='11:22:33:44:55:66') status = self.call_get_status(self.uuid) - self.check_status(status, finished=True) + self.check_status(status, finished=True, state=istate.States.finished) @mock.patch.object(swift, 'store_introspection_data', autospec=True) @mock.patch.object(swift, 'get_introspection_data', autospec=True) @@ -765,14 +715,14 @@ class Test(Base): 'reboot') status = self.call_get_status(self.uuid) - self.check_status(status, finished=False) + 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) status = self.call_get_status(self.uuid) - self.check_status(status, finished=True) + self.check_status(status, finished=True, state=istate.States.finished) # Verify that the lldp_processed data is written to swift # as expected by the lldp plugin diff --git a/ironic_inspector/test/unit/test_main.py b/ironic_inspector/test/unit/test_main.py index c0c780eff..9bd6b4b27 100644 --- a/ironic_inspector/test/unit/test_main.py +++ b/ironic_inspector/test/unit/test_main.py @@ -25,6 +25,7 @@ from ironic_inspector import conf from ironic_inspector import db from ironic_inspector import firewall from ironic_inspector import introspect +from ironic_inspector import introspection_state as istate from ironic_inspector import main from ironic_inspector import node_cache from ironic_inspector.plugins import base as plugins_base @@ -193,7 +194,8 @@ class GetStatusAPIBaseTest(BaseAPITest): uuid=self.uuid, started_at=datetime.datetime(1, 1, 1), finished_at=datetime.datetime(1, 1, 2), - error='boom') + error='boom', + state=istate.States.error) self.finished_node.links = [ {u'href': u'http://localhost/v1/introspection/%s' % self.finished_node.uuid, @@ -201,6 +203,7 @@ class GetStatusAPIBaseTest(BaseAPITest): ] self.finished_node.status = { 'finished': True, + 'state': self.finished_node._state, 'started_at': self.finished_node.started_at.isoformat(), 'finished_at': self.finished_node.finished_at.isoformat(), 'error': self.finished_node.error, @@ -210,7 +213,8 @@ class GetStatusAPIBaseTest(BaseAPITest): self.unfinished_node = node_cache.NodeInfo( uuid=self.uuid2, - started_at=datetime.datetime(1, 1, 1)) + started_at=datetime.datetime(1, 1, 1), + state=istate.States.processing) self.unfinished_node.links = [ {u'href': u'http://localhost/v1/introspection/%s' % self.unfinished_node.uuid, @@ -220,6 +224,7 @@ class GetStatusAPIBaseTest(BaseAPITest): if self.unfinished_node.finished_at else None) self.unfinished_node.status = { 'finished': False, + 'state': self.unfinished_node._state, 'started_at': self.unfinished_node.started_at.isoformat(), 'finished_at': finished_at, 'error': None, diff --git a/releasenotes/notes/add-node-state-to-introspection-api-response-85fb7f4e72ae386a.yaml b/releasenotes/notes/add-node-state-to-introspection-api-response-85fb7f4e72ae386a.yaml new file mode 100644 index 000000000..92c51f159 --- /dev/null +++ b/releasenotes/notes/add-node-state-to-introspection-api-response-85fb7f4e72ae386a.yaml @@ -0,0 +1,4 @@ +--- +features: + - Adds node state to the GET /v1/introspection/ and + GET /v1/introspection API response data.