diff --git a/api-ref/source/introspection-api-v1-introspection.inc b/api-ref/source/introspection-api-v1-introspection.inc index adf58efa3..a271fa5cc 100644 --- a/api-ref/source/introspection-api-v1-introspection.inc +++ b/api-ref/source/introspection-api-v1-introspection.inc @@ -57,6 +57,7 @@ Status list may be paginated with these query string fields: - marker: marker - limit: limit + - state: state Response diff --git a/api-ref/source/introspection-api-versions.inc b/api-ref/source/introspection-api-versions.inc index 1363d9edb..86b9ea4de 100644 --- a/api-ref/source/introspection-api-versions.inc +++ b/api-ref/source/introspection-api-versions.inc @@ -107,3 +107,4 @@ Version History * **1.15** allows reapply with provided introspection data from request. * **1.16** adds ``scope`` field to introspection rule. * **1.17** adds ``GET /v1/introspection//data/unprocessed``. +* **1.18** adds state selector ``GET /v1/introspection?state=starting,...``. diff --git a/ironic_inspector/api_tools.py b/ironic_inspector/api_tools.py index f277c5493..c5384e2bf 100644 --- a/ironic_inspector/api_tools.py +++ b/ironic_inspector/api_tools.py @@ -19,6 +19,7 @@ from oslo_config import cfg from oslo_utils import uuidutils from ironic_inspector.common.i18n import _ +from ironic_inspector import introspection_state as istate from ironic_inspector import utils CONF = cfg.CONF @@ -82,3 +83,18 @@ def limit_field(value): assert value >= 0, _('Limit cannot be negative') assert value <= CONF.api_max_limit, _('Limit over %s') % CONF.api_max_limit return value + + +@request_field('state') +@raises_coercion_exceptions +def state_field(value): + """Fetch the pagination state field from flask.request.args. + + :returns: list of the state(s) + """ + states = istate.States.all() + value = value.split(',') + invalid_states = [state for state in value if state not in states] + assert not invalid_states, \ + _('State(s) "%s" are not valid') % ', '.join(invalid_states) + return value diff --git a/ironic_inspector/main.py b/ironic_inspector/main.py index fa137d00c..b3a21747f 100644 --- a/ironic_inspector/main.py +++ b/ironic_inspector/main.py @@ -44,7 +44,7 @@ _wsgi_app = _app.wsgi_app LOG = utils.getProcessingLogger(__name__) MINIMUM_API_VERSION = (1, 0) -CURRENT_API_VERSION = (1, 17) +CURRENT_API_VERSION = (1, 18) DEFAULT_API_VERSION = CURRENT_API_VERSION _LOGGING_EXCLUDED_KEYS = ('logs',) @@ -378,7 +378,8 @@ def api_introspection(node_id): def api_introspection_statuses(): nodes = node_cache.get_node_list( marker=api_tools.marker_field(), - limit=api_tools.limit_field(default=CONF.api_max_limit) + limit=api_tools.limit_field(default=CONF.api_max_limit), + state=api_tools.state_field() ) data = { 'introspection': [generate_introspection_status(node) diff --git a/ironic_inspector/node_cache.py b/ironic_inspector/node_cache.py index 3b40862ee..99e21def1 100644 --- a/ironic_inspector/node_cache.py +++ b/ironic_inspector/node_cache.py @@ -976,7 +976,7 @@ def record_node(ironic=None, bmc_addresses=None, macs=None): manage_boot=False, mac=macs, bmc_address=bmc_addresses) -def get_node_list(ironic=None, marker=None, limit=None): +def get_node_list(ironic=None, marker=None, limit=None, state=None): """Get node list from the cache. The list of the nodes is ordered based on the (started_at, uuid) @@ -985,6 +985,7 @@ def get_node_list(ironic=None, marker=None, limit=None): :param ironic: optional ironic client instance :param marker: pagination marker (an UUID or None) :param limit: pagination limit; None for default CONF.api_max_limit + :param state: list of states for the filter; None for no state filter :returns: a list of NodeInfo instances. """ if marker is not None: @@ -995,6 +996,8 @@ def get_node_list(ironic=None, marker=None, limit=None): code=404) rows = db.model_query(db.Node) + if state: + rows = rows.filter(db.Node.state.in_(state)) # ordered based on (started_at, uuid); newer first rows = db_utils.paginate_query(rows, db.Node, limit, ('started_at', 'uuid'), diff --git a/ironic_inspector/test/unit/test_api_tools.py b/ironic_inspector/test/unit/test_api_tools.py index 7587e1ba0..65c228f64 100644 --- a/ironic_inspector/test/unit/test_api_tools.py +++ b/ironic_inspector/test/unit/test_api_tools.py @@ -140,3 +140,22 @@ class LimitFieldTestCase(test_base.BaseTest): def test_limit_invalid_value(self, get_mock): self.assertRaisesRegex(utils.Error, 'Bad request', api_tools.limit_field) + + +class StateFieldTestCase(test_base.BaseTest): + @mock_test_field(return_value='error') + def test_single_state(self, get_mock): + value = api_tools.state_field() + self.assertEqual(get_mock.return_value.split(','), value) + + @mock_test_field(return_value='error,finished') + def test_multiple_state(self, get_mock): + value = api_tools.state_field() + self.assertEqual(get_mock.return_value.split(','), value) + + @mock_test_field(return_value='error,invalid') + def test_invalid_state(self, get_mock): + self.assertRaisesRegex(utils.Error, + r'.*(State\(s\) "%s" are not valid)' + % 'invalid', + api_tools.state_field) diff --git a/ironic_inspector/test/unit/test_main.py b/ironic_inspector/test/unit/test_main.py index e40b79226..60566c5f3 100644 --- a/ironic_inspector/test/unit/test_main.py +++ b/ironic_inspector/test/unit/test_main.py @@ -343,19 +343,50 @@ class TestApiListStatus(GetStatusAPIBaseTest): self.assertEqual([self.finished_node.status, self.unfinished_node.status], statuses) list_mock.assert_called_once_with(marker=None, - limit=CONF.api_max_limit) + limit=CONF.api_max_limit, state=None) def test_list_introspection_limit(self, list_mock): res = self.app.get('/v1/introspection?limit=1000') self.assertEqual(200, res.status_code) - list_mock.assert_called_once_with(marker=None, limit=1000) + list_mock.assert_called_once_with(marker=None, limit=1000, state=None) def test_list_introspection_makrer(self, list_mock): res = self.app.get('/v1/introspection?marker=%s' % self.finished_node.uuid) self.assertEqual(200, res.status_code) list_mock.assert_called_once_with(marker=self.finished_node.uuid, - limit=CONF.api_max_limit) + limit=CONF.api_max_limit, state=None) + + def test_list_introspection_state(self, list_mock): + state = 'error' + list_mock.return_value = [self.finished_node] + res = self.app.get('/v1/introspection?state=%s' % state) + self.assertEqual(200, res.status_code) + statuses = json.loads(res.data.decode('utf-8')).get('introspection') + + self.assertEqual([self.finished_node.status], statuses) + list_mock.assert_called_once_with(marker=None, + limit=CONF.api_max_limit, + state=state.split(',')) + + def test_list_introspection_multiple_state(self, list_mock): + state = 'error,processing' + list_mock.return_value = [self.finished_node, + self.unfinished_node] + res = self.app.get('/v1/introspection?state=%s' % state) + self.assertEqual(200, res.status_code) + statuses = json.loads(res.data.decode('utf-8')).get('introspection') + + self.assertEqual([self.finished_node.status, + self.unfinished_node.status], statuses) + list_mock.assert_called_once_with(marker=None, + limit=CONF.api_max_limit, + state=state.split(',')) + + def test_list_introspection_invalid_state(self, list_mock): + state = 'error,invalid' + res = self.app.get('/v1/introspection?state=%s' % state) + self.assertEqual(400, res.status_code) class TestApiGetData(BaseAPITest): diff --git a/ironic_inspector/test/unit/test_node_cache.py b/ironic_inspector/test/unit/test_node_cache.py index 58a0f0614..79a286818 100644 --- a/ironic_inspector/test/unit/test_node_cache.py +++ b/ironic_inspector/test/unit/test_node_cache.py @@ -975,12 +975,15 @@ class TestNodeCacheListNode(test_base.NodeTest): def setUp(self): super(TestNodeCacheListNode, self).setUp() self.uuid2 = uuidutils.generate_uuid() + self.uuid3 = uuidutils.generate_uuid() session = db.get_writer_session() with session.begin(): db.Node(uuid=self.uuid, started_at=datetime.datetime(1, 1, 2)).save(session) db.Node(uuid=self.uuid2, started_at=datetime.datetime(1, 1, 1), finished_at=datetime.datetime(1, 1, 3)).save(session) + db.Node(uuid=self.uuid3, started_at=datetime.datetime(1, 1, 3), + state='error').save(session) # mind please node(self.uuid).started_at > node(self.uuid2).started_at # and the result ordering is strict in node_cache.get_node_list newer first @@ -988,12 +991,12 @@ class TestNodeCacheListNode(test_base.NodeTest): def test_list_node(self): nodes = node_cache.get_node_list() - self.assertEqual([self.uuid, self.uuid2], + self.assertEqual([self.uuid3, self.uuid, self.uuid2], [node.uuid for node in nodes]) def test_list_node_limit(self): nodes = node_cache.get_node_list(limit=1) - self.assertEqual([self.uuid], [node.uuid for node in nodes]) + self.assertEqual([self.uuid3], [node.uuid for node in nodes]) def test_list_node_marker(self): # get nodes started_at after node(self.uuid) @@ -1004,6 +1007,15 @@ class TestNodeCacheListNode(test_base.NodeTest): self.assertRaises(utils.Error, node_cache.get_node_list, marker='foo-bar') + def test_list_node_state(self): + nodes = node_cache.get_node_list(state=['error']) + self.assertEqual([self.uuid3], [node.uuid for node in nodes]) + + def test_list_node_state_multiple(self): + nodes = node_cache.get_node_list(state=['error', 'finished']) + self.assertEqual([self.uuid3, self.uuid, self.uuid2], + [node.uuid for node in nodes]) + class TestNodeInfoVersionId(test_base.NodeStateTest): def test_get(self): diff --git a/releasenotes/notes/add-list-introspection-state-selector-3bbb37dd08e35d09.yaml b/releasenotes/notes/add-list-introspection-state-selector-3bbb37dd08e35d09.yaml new file mode 100644 index 000000000..c7adc550e --- /dev/null +++ b/releasenotes/notes/add-list-introspection-state-selector-3bbb37dd08e35d09.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + Adds support for filter by state in the list introspection API. + See `story 1625183 + `_. + + * ``GET /v1/introspection?state=starting,...``