openstack
/
ironic-inspector
Code Issues Proposed changes

Adds node state to the API response data 

This adds the node state when the GET /v1/introspection/<node uuid or
name> API is performed.

Change-Id: I81c6834933f789cb644a854313aacaf49a4856a7
Closes-Bug: #1665664
This commit is contained in:
yaojun 2017-02-21 19:04:00 +08:00
parent 34eb861913
commit fc2e029fa6
5 changed files with 61 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
doc/source/http-api.rst
View File

@ -45,6 +45,7 @@ Response body: JSON dictionary with keys:
* ``finished`` (boolean) whether introspection is finished * ``finished`` (boolean) whether introspection is finished
(``true`` on introspection completion or if it ends because of an error) (``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 * ``error`` error string or ``null``; ``Canceled by operator`` in
case introspection was aborted case introspection was aborted
* ``uuid`` node UUID * ``uuid`` node UUID
@ -76,7 +77,8 @@ Response body: a JSON object containing a list of status objects::
{ {
'introspection': [ 'introspection': [
{ {
'finished': true, 'finished': false,
'state': 'waiting',
'error': null, 'error': null,
... ...
}, },
@ -88,6 +90,7 @@ Each status object contains these keys:
* ``finished`` (boolean) whether introspection is finished * ``finished`` (boolean) whether introspection is finished
(``true`` on introspection completion or if it ends because of an error) (``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 * ``error`` error string or ``null``; ``Canceled by operator`` in
case introspection was aborted case introspection was aborted
* ``uuid`` node UUID * ``uuid`` node UUID
@ -392,3 +395,5 @@ Version History
* **1.8** support for listing all introspection statuses. * **1.8** support for listing all introspection statuses.
* **1.9** de-activate setting IPMI credentials, if IPMI credentials * **1.9** de-activate setting IPMI credentials, if IPMI credentials
are requested, API gets HTTP 400 response. are requested, API gets HTTP 400 response.
* **1.10** adds node state to the GET /v1/introspection/<Node ID> and
GET /v1/introspection API response data.

3
ironic_inspector/main.py
View File

@ -51,7 +51,7 @@ MINIMUM_API_VERSION = (1, 0)
# TODO(dtantsur): set to the current version as soon we move setting IPMI # TODO(dtantsur): set to the current version as soon we move setting IPMI
# credentials support completely. # credentials support completely.
DEFAULT_API_VERSION = (1, 8) DEFAULT_API_VERSION = (1, 8)
CURRENT_API_VERSION = (1, 9) CURRENT_API_VERSION = (1, 10)
_LOGGING_EXCLUDED_KEYS = ('logs',) _LOGGING_EXCLUDED_KEYS = ('logs',)
@ -149,6 +149,7 @@ def generate_introspection_status(node):
status = {} status = {}
status['uuid'] = node.uuid status['uuid'] = node.uuid
status['finished'] = bool(node.finished_at) status['finished'] = bool(node.finished_at)
status['state'] = node.state
status['started_at'] = started_at status['started_at'] = started_at
status['finished_at'] = finished_at status['finished_at'] = finished_at
status['error'] = node.error status['error'] = node.error

134
ironic_inspector/test/functional.py
View File

@ -202,18 +202,20 @@ class Base(base.NodeTest):
def call_get_rule(self, uuid, **kwargs): def call_get_rule(self, uuid, **kwargs):
return self.call('get', '/v1/rules/' + uuid, **kwargs).json() 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, started_at=mock.ANY, finished_at=mock.ANY,
links=mock.ANY): links=mock.ANY):
return {'uuid': self.uuid, 'finished': finished, 'error': error, 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, 'links': [{u'href': u'%s/v1/introspection/%s' % (self.ROOT_URL,
self.uuid), self.uuid),
u'rel': u'self'}]} u'rel': u'self'}]}
def check_status(self, status, finished, error=None): def check_status(self, status, finished, state, error=None):
self.assertEqual( self.assertEqual(
self._fake_status(finished=finished, self._fake_status(finished=finished,
state=state,
finished_at=finished and mock.ANY or None, finished_at=finished and mock.ANY or None,
error=error), error=error),
status status
@ -242,7 +244,7 @@ class Test(Base):
'reboot') 'reboot')
status = self.call_get_status(self.uuid) 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) res = self.call_continue(self.data)
self.assertEqual({'uuid': self.uuid}, res) 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={}) node_uuid=self.uuid, address='11:22:33:44:55:66', extra={})
status = self.call_get_status(self.uuid) 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): def test_bmc_with_client_id(self):
self.pxe_mac = self.macs[2] self.pxe_mac = self.macs[2]
@ -269,7 +271,7 @@ class Test(Base):
'reboot') 'reboot')
status = self.call_get_status(self.uuid) 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) res = self.call_continue(self.data)
self.assertEqual({'uuid': self.uuid}, res) self.assertEqual({'uuid': self.uuid}, res)
@ -282,7 +284,7 @@ class Test(Base):
extra={'client-id': self.client_id}) extra={'client-id': self.client_id})
status = self.call_get_status(self.uuid) 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): def test_setup_ipmi(self):
patch_credentials = [ patch_credentials = [
@ -298,7 +300,7 @@ class Test(Base):
self.assertFalse(self.cli.node.set_power_state.called) self.assertFalse(self.cli.node.set_power_state.called)
status = self.call_get_status(self.uuid) 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) res = self.call_continue(self.data)
self.assertEqual('admin', res['ipmi_username']) 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={}) node_uuid=self.uuid, address='11:22:33:44:55:66', extra={})
status = self.call_get_status(self.uuid) 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): def test_introspection_statuses(self):
self.call_introspect(self.uuid) self.call_introspect(self.uuid)
@ -339,7 +341,7 @@ class Test(Base):
eventlet.greenthread.sleep(DEFAULT_SLEEP) eventlet.greenthread.sleep(DEFAULT_SLEEP)
status = self.call_get_status(self.uuid) 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 # fetch all statuses and db nodes to assert pagination
statuses = self.call_get_statuses().get('introspection') statuses = self.call_get_statuses().get('introspection')
@ -504,7 +506,7 @@ class Test(Base):
'reboot') 'reboot')
status = self.call_get_status(self.uuid) 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) res = self.call_continue(self.data)
self.assertEqual({'uuid': self.uuid}, res) 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={}) node_uuid=self.uuid, address='11:22:33:44:55:66', extra={})
status = self.call_get_status(self.uuid) 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): def test_abort_introspection(self):
self.call_introspect(self.uuid) self.call_introspect(self.uuid)
@ -523,7 +525,7 @@ class Test(Base):
self.cli.node.set_power_state.assert_called_once_with(self.uuid, self.cli.node.set_power_state.assert_called_once_with(self.uuid,
'reboot') 'reboot')
status = self.call_get_status(self.uuid) 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) res = self.call_abort_introspect(self.uuid)
eventlet.greenthread.sleep(DEFAULT_SLEEP) eventlet.greenthread.sleep(DEFAULT_SLEEP)
@ -562,7 +564,7 @@ class Test(Base):
status = self.call_get_status(self.uuid) status = self.call_get_status(self.uuid)
inspect_started_at = timeutils.parse_isotime(status['started_at']) 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) res = self.call_reapply(self.uuid)
self.assertEqual(202, res.status_code) self.assertEqual(202, res.status_code)
@ -570,7 +572,7 @@ class Test(Base):
eventlet.greenthread.sleep(DEFAULT_SLEEP) eventlet.greenthread.sleep(DEFAULT_SLEEP)
status = self.call_get_status(self.uuid) 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 # checks the started_at updated in DB is correct
reapply_started_at = timeutils.parse_isotime(status['started_at']) reapply_started_at = timeutils.parse_isotime(status['started_at'])
@ -609,52 +611,6 @@ class Test(Base):
self.assertEqual(store_processing_call, self.assertEqual(store_processing_call,
store_mock.call_args_list[-1]) 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, 'store_introspection_data', autospec=True)
@mock.patch.object(swift, 'get_introspection_data', autospec=True) @mock.patch.object(swift, 'get_introspection_data', autospec=True)
def test_edge_state_transitions(self, get_mock, store_mock): 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)
self.call_introspect(self.uuid) self.call_introspect(self.uuid)
eventlet.greenthread.sleep(DEFAULT_SLEEP) eventlet.greenthread.sleep(DEFAULT_SLEEP)
# TODO(milan): switch to API once the introspection status status = self.call_get_status(self.uuid)
# endpoint exposes the state information self.check_status(status, finished=False, state=istate.States.waiting)
row = self.db_row()
self.assertEqual(istate.States.waiting, row.state)
# an error -start-> starting state transition is possible # an error -start-> starting state transition is possible
self.call_abort_introspect(self.uuid) self.call_abort_introspect(self.uuid)
self.call_introspect(self.uuid) self.call_introspect(self.uuid)
eventlet.greenthread.sleep(DEFAULT_SLEEP) eventlet.greenthread.sleep(DEFAULT_SLEEP)
row = self.db_row() status = self.call_get_status(self.uuid)
self.assertEqual(istate.States.waiting, row.state) self.check_status(status, finished=False, state=istate.States.waiting)
# double abort works # double abort works
self.call_abort_introspect(self.uuid) self.call_abort_introspect(self.uuid)
row = self.db_row() status = self.call_get_status(self.uuid)
version_id = row.version_id error = status['error']
error = row.error self.check_status(status, finished=True, state=istate.States.error,
self.assertEqual(istate.States.error, row.state) error=error)
self.call_abort_introspect(self.uuid) self.call_abort_introspect(self.uuid)
row = self.db_row() status = self.call_get_status(self.uuid)
self.assertEqual(istate.States.error, row.state) self.check_status(status, finished=True, state=istate.States.error,
# assert the error didn't change error=error)
self.assertEqual(error, row.error)
self.assertEqual(version_id, row.version_id)
# preventing stale data race condition # preventing stale data race condition
# waiting -> processing is a strict state transition # waiting -> processing is a strict state transition
@ -704,26 +656,24 @@ class Test(Base):
with db.ensure_transaction() as session: with db.ensure_transaction() as session:
row.save(session) row.save(session)
self.call_continue(self.data, expect_error=400) self.call_continue(self.data, expect_error=400)
row = self.db_row() status = self.call_get_status(self.uuid)
self.assertEqual(istate.States.error, row.state) self.check_status(status, finished=True, state=istate.States.error,
self.assertIn('no defined transition', row.error) error=mock.ANY)
self.assertIn('no defined transition', status['error'])
# multiple reapply calls # multiple reapply calls
self.call_introspect(self.uuid) self.call_introspect(self.uuid)
eventlet.greenthread.sleep(DEFAULT_SLEEP) eventlet.greenthread.sleep(DEFAULT_SLEEP)
self.call_continue(self.data) self.call_continue(self.data)
eventlet.greenthread.sleep(DEFAULT_SLEEP) eventlet.greenthread.sleep(DEFAULT_SLEEP)
self.call_reapply(self.uuid) self.call_reapply(self.uuid)
row = self.db_row() status = self.call_get_status(self.uuid)
version_id = row.version_id self.check_status(status, finished=True, state=istate.States.finished,
self.assertEqual(istate.States.finished, row.state) error=None)
self.assertIsNone(row.error)
self.call_reapply(self.uuid) self.call_reapply(self.uuid)
# assert an finished -reapply-> reapplying -> finished state transition # assert an finished -reapply-> reapplying -> finished state transition
row = self.db_row() status = self.call_get_status(self.uuid)
self.assertEqual(istate.States.finished, row.state) self.check_status(status, finished=True, state=istate.States.finished,
self.assertIsNone(row.error) error=None)
self.assertNotEqual(version_id, row.version_id)
def test_without_root_disk(self): def test_without_root_disk(self):
del self.data['root_disk'] del self.data['root_disk']
@ -737,7 +687,7 @@ class Test(Base):
'reboot') 'reboot')
status = self.call_get_status(self.uuid) 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) res = self.call_continue(self.data)
self.assertEqual({'uuid': self.uuid}, res) 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') node_uuid=self.uuid, extra={}, address='11:22:33:44:55:66')
status = self.call_get_status(self.uuid) 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, 'store_introspection_data', autospec=True)
@mock.patch.object(swift, 'get_introspection_data', autospec=True) @mock.patch.object(swift, 'get_introspection_data', autospec=True)
@ -765,14 +715,14 @@ class Test(Base):
'reboot') 'reboot')
status = self.call_get_status(self.uuid) 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) res = self.call_continue(self.data)
self.assertEqual({'uuid': self.uuid}, res) self.assertEqual({'uuid': self.uuid}, res)
eventlet.greenthread.sleep(DEFAULT_SLEEP) eventlet.greenthread.sleep(DEFAULT_SLEEP)
status = self.call_get_status(self.uuid) 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 # Verify that the lldp_processed data is written to swift
# as expected by the lldp plugin # as expected by the lldp plugin

9
ironic_inspector/test/unit/test_main.py
View File

@ -25,6 +25,7 @@ from ironic_inspector import conf
from ironic_inspector import db from ironic_inspector import db
from ironic_inspector import firewall from ironic_inspector import firewall
from ironic_inspector import introspect from ironic_inspector import introspect
from ironic_inspector import introspection_state as istate
from ironic_inspector import main from ironic_inspector import main
from ironic_inspector import node_cache from ironic_inspector import node_cache
from ironic_inspector.plugins import base as plugins_base from ironic_inspector.plugins import base as plugins_base
@ -193,7 +194,8 @@ class GetStatusAPIBaseTest(BaseAPITest):
uuid=self.uuid, uuid=self.uuid,
started_at=datetime.datetime(1, 1, 1), started_at=datetime.datetime(1, 1, 1),
finished_at=datetime.datetime(1, 1, 2), finished_at=datetime.datetime(1, 1, 2),
error='boom') error='boom',
state=istate.States.error)
self.finished_node.links = [ self.finished_node.links = [
{u'href': u'http://localhost/v1/introspection/%s' % {u'href': u'http://localhost/v1/introspection/%s' %
self.finished_node.uuid, self.finished_node.uuid,
@ -201,6 +203,7 @@ class GetStatusAPIBaseTest(BaseAPITest):
] ]
self.finished_node.status = { self.finished_node.status = {
'finished': True, 'finished': True,
'state': self.finished_node._state,
'started_at': self.finished_node.started_at.isoformat(), 'started_at': self.finished_node.started_at.isoformat(),
'finished_at': self.finished_node.finished_at.isoformat(), 'finished_at': self.finished_node.finished_at.isoformat(),
'error': self.finished_node.error, 'error': self.finished_node.error,
@ -210,7 +213,8 @@ class GetStatusAPIBaseTest(BaseAPITest):
self.unfinished_node = node_cache.NodeInfo( self.unfinished_node = node_cache.NodeInfo(
uuid=self.uuid2, 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 = [ self.unfinished_node.links = [
{u'href': u'http://localhost/v1/introspection/%s' % {u'href': u'http://localhost/v1/introspection/%s' %
self.unfinished_node.uuid, self.unfinished_node.uuid,
@ -220,6 +224,7 @@ class GetStatusAPIBaseTest(BaseAPITest):
if self.unfinished_node.finished_at else None) if self.unfinished_node.finished_at else None)
self.unfinished_node.status = { self.unfinished_node.status = {
'finished': False, 'finished': False,
'state': self.unfinished_node._state,
'started_at': self.unfinished_node.started_at.isoformat(), 'started_at': self.unfinished_node.started_at.isoformat(),
'finished_at': finished_at, 'finished_at': finished_at,
'error': None, 'error': None,

4
releasenotes/notes/add-node-state-to-introspection-api-response-85fb7f4e72ae386a.yaml Normal file
View File

@ -0,0 +1,4 @@
---
features:
- Adds node state to the GET /v1/introspection/<node UUID or name> and
GET /v1/introspection API response data.