diff options
| -rw-r--r-- | ironic/api/controllers/v1/node.py | 27 | ||||
| -rw-r--r-- | ironic/api/controllers/v1/utils.py | 19 | ||||
| -rw-r--r-- | ironic/api/controllers/v1/versions.py | 4 | ||||
| -rw-r--r-- | ironic/common/release_mappings.py | 2 | ||||
| -rw-r--r-- | ironic/db/sqlalchemy/api.py | 5 | ||||
| -rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_node.py | 75 | ||||
| -rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_utils.py | 22 | ||||
| -rw-r--r-- | ironic/tests/unit/db/test_nodes.py | 20 | ||||
| -rw-r--r-- | releasenotes/notes/conductor-groups-c22c17e276e63bed.yaml | 17 |
9 files changed, 168 insertions, 23 deletions
diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index fc91a9475..1808a4f94 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1249,7 +1249,7 @@ class Node(base.APIBase): management_interface=None, power_interface=None, raid_interface=None, vendor_interface=None, storage_interface=None, traits=[], rescue_interface=None, - bios_interface=None) + bios_interface=None, conductor_group="") # NOTE(matty_dubs): The chassis_uuid getter() is based on the # _chassis_uuid variable: sample._chassis_uuid = 'edcad704-b2da-41d5-96d9-afd580ecfa12' @@ -1552,7 +1552,8 @@ class NodesController(rest.RestController): maintenance, provision_state, marker, limit, sort_key, sort_dir, driver=None, resource_class=None, resource_url=None, - fields=None, fault=None, detail=None): + fields=None, fault=None, conductor_group=None, + detail=None): if self.from_chassis and not chassis_uuid: raise exception.MissingParameterValue( _("Chassis id not specified.")) @@ -1600,6 +1601,8 @@ class NodesController(rest.RestController): filters['resource_class'] = resource_class if fault is not None: filters['fault'] = fault + if conductor_group is not None: + filters['conductor_group'] = conductor_group nodes = objects.Node.list(pecan.request.context, limit, marker_obj, sort_key=sort_key, sort_dir=sort_dir, @@ -1707,11 +1710,12 @@ class NodesController(rest.RestController): @expose.expose(NodeCollection, types.uuid, types.uuid, types.boolean, types.boolean, wtypes.text, types.uuid, int, wtypes.text, wtypes.text, wtypes.text, types.listtype, wtypes.text, - wtypes.text, types.boolean) + wtypes.text, wtypes.text, types.boolean) def get_all(self, chassis_uuid=None, instance_uuid=None, associated=None, maintenance=None, provision_state=None, marker=None, limit=None, sort_key='id', sort_dir='asc', driver=None, - fields=None, resource_class=None, fault=None, detail=None): + fields=None, resource_class=None, fault=None, + conductor_group=None, detail=None): """Retrieve a list of nodes. :param chassis_uuid: Optional UUID of a chassis, to get only nodes for @@ -1737,6 +1741,8 @@ class NodesController(rest.RestController): driver. :param resource_class: Optional string value to get only nodes with that resource_class. + :param conductor_group: Optional string value to get only nodes with + that conductor_group. :param fields: Optional, a list with a specified set of fields of the resource to be returned. :param fault: Optional string value to get only nodes with that fault. @@ -1751,6 +1757,7 @@ class NodesController(rest.RestController): api_utils.check_allow_specify_driver(driver) api_utils.check_allow_specify_resource_class(resource_class) api_utils.check_allow_filter_by_fault(fault) + api_utils.check_allow_filter_by_conductor_group(conductor_group) fields = api_utils.get_request_return_fields(fields, detail, _DEFAULT_RETURN_FIELDS) @@ -1762,16 +1769,18 @@ class NodesController(rest.RestController): driver=driver, resource_class=resource_class, fields=fields, fault=fault, + conductor_group=conductor_group, detail=detail) @METRICS.timer('NodesController.detail') @expose.expose(NodeCollection, types.uuid, types.uuid, types.boolean, types.boolean, wtypes.text, types.uuid, int, wtypes.text, - wtypes.text, wtypes.text, wtypes.text, wtypes.text) + wtypes.text, wtypes.text, wtypes.text, wtypes.text, + wtypes.text) def detail(self, chassis_uuid=None, instance_uuid=None, associated=None, maintenance=None, provision_state=None, marker=None, limit=None, sort_key='id', sort_dir='asc', driver=None, - resource_class=None, fault=None): + resource_class=None, fault=None, conductor_group=None): """Retrieve a list of nodes with detail. :param chassis_uuid: Optional UUID of a chassis, to get only nodes for @@ -1798,6 +1807,8 @@ class NodesController(rest.RestController): :param resource_class: Optional string value to get only nodes with that resource_class. :param fault: Optional string value to get only nodes with that fault. + :param conductor_group: Optional string value to get only nodes with + that conductor_group. """ cdict = pecan.request.context.to_policy_values() policy.authorize('baremetal:node:get', cdict, cdict) @@ -1806,6 +1817,7 @@ class NodesController(rest.RestController): api_utils.check_allow_specify_driver(driver) api_utils.check_allow_specify_resource_class(resource_class) api_utils.check_allow_filter_by_fault(fault) + api_utils.check_allow_filter_by_conductor_group(conductor_group) api_utils.check_allowed_fields([sort_key]) # /detail should only work against collections parent = pecan.request.path.split('/')[:-1][-1] @@ -1820,7 +1832,8 @@ class NodesController(rest.RestController): driver=driver, resource_class=resource_class, resource_url=resource_url, - fault=fault) + fault=fault, + conductor_group=conductor_group) @METRICS.timer('NodesController.validate') @expose.expose(wtypes.text, types.uuid_or_name, types.uuid) diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 56c2db697..ff9980f8b 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -502,6 +502,20 @@ def check_allow_filter_by_fault(fault): msg, status_code=http_client.BAD_REQUEST) +def check_allow_filter_by_conductor_group(conductor_group): + """Check if filtering nodes by conductor_group is allowed. + + Version 1.46 of the API allows filtering nodes by conductor_group. + """ + if (conductor_group is not None and pecan.request.version.minor + < versions.MINOR_46_NODE_CONDUCTOR_GROUP): + raise exception.NotAcceptable(_( + "Request not acceptable. The minimal required API version " + "should be %(base)s.%(opr)s") % + {'base': versions.BASE_VERSION, + 'opr': versions.MINOR_46_NODE_CONDUCTOR_GROUP}) + + def initial_node_provision_state(): """Return node state to use by default when creating new nodes. @@ -873,9 +887,10 @@ def allow_reset_interfaces(): def allow_conductor_group(): """Check if passing a conductor_group for a node is allowed. - There is no version yet that allows this. + Version 1.46 exposes this field. """ - return False + return (pecan.request.version.minor >= + versions.MINOR_46_NODE_CONDUCTOR_GROUP) def get_request_return_fields(fields, detail, default_fields): diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index b0d336f76..650e8be78 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -83,6 +83,7 @@ BASE_VERSION = 1 # v1.43: Add detail=True flag to all API endpoints # v1.44: Add node deploy_step field # v1.45: reset_interfaces parameter to node's PATCH +# v1.46: Add conductor_group to the node object. MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -130,6 +131,7 @@ MINOR_42_FAULT = 42 MINOR_43_ENABLE_DETAIL_QUERY = 43 MINOR_44_NODE_DEPLOY_STEP = 44 MINOR_45_RESET_INTERFACES = 45 +MINOR_46_NODE_CONDUCTOR_GROUP = 46 # When adding another version, update: # - MINOR_MAX_VERSION @@ -137,7 +139,7 @@ MINOR_45_RESET_INTERFACES = 45 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_45_RESET_INTERFACES +MINOR_MAX_VERSION = MINOR_46_NODE_CONDUCTOR_GROUP # String representations of the minor and maximum versions _MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 81712818c..855702b42 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -115,7 +115,7 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.45', + 'api': '1.46', 'rpc': '1.47', 'objects': { 'Node': ['1.26', '1.27'], diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index 9e3c9918c..c7fcaf1f7 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -223,7 +223,8 @@ class Connection(api.Connection): 'resource_class', 'provision_state', 'uuid', 'id', 'chassis_uuid', 'associated', 'reserved', 'reserved_by_any_of', 'provisioned_before', - 'inspection_started_before', 'fault'} + 'inspection_started_before', 'fault', + 'conductor_group'} unsupported_filters = set(filters).difference(supported_filters) if unsupported_filters: msg = _("SqlAlchemy API does not support " @@ -231,7 +232,7 @@ class Connection(api.Connection): raise ValueError(msg) for field in ['console_enabled', 'maintenance', 'driver', 'resource_class', 'provision_state', 'uuid', 'id', - 'fault']: + 'fault', 'conductor_group']: if field in filters: query = query.filter_by(**{field: filters[field]}) if 'chassis_uuid' in filters: diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 992f91bb6..26a047bf7 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -123,6 +123,7 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertNotIn('chassis_id', data['nodes'][0]) self.assertNotIn('bios_interface', data['nodes'][0]) self.assertNotIn('deploy_step', data['nodes'][0]) + self.assertNotIn('conductor_group', data['nodes'][0]) def test_get_one(self): node = obj_utils.create_test_node(self.context, @@ -160,6 +161,7 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertNotIn('chassis_id', data) self.assertIn('bios_interface', data) self.assertIn('deploy_step', data) + self.assertIn('conductor_group', data) def test_get_one_with_json(self): # Test backward compatibility with guess_content_type_from_ext @@ -257,11 +259,8 @@ class TestListNodes(test_api_base.BaseApiTest): '1.43', '1.44') def test_node_conductor_group_hidden_in_lower_version(self): - node = obj_utils.create_test_node(self.context) - data = self.get_json( - '/nodes/%s' % node.uuid, - headers={api_base.Version.string: '1.44'}) - self.assertNotIn('conductor_group', data) + self._test_node_field_hidden_in_lower_version('conductor_group', + '1.45', '1.46') def test_get_one_custom_fields(self): node = obj_utils.create_test_node(self.context, @@ -406,10 +405,19 @@ class TestListNodes(test_api_base.BaseApiTest): fields = 'conductor_group' response = self.get_json( '/nodes/%s?fields=%s' % (node.uuid, fields), - headers={api_base.Version.string: '1.43'}, + headers={api_base.Version.string: '1.44'}, expect_errors=True) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + def test_get_conductor_group_fields(self): + node = obj_utils.create_test_node(self.context, + chassis_id=self.chassis.id) + fields = 'conductor_group' + response = self.get_json( + '/nodes/%s?fields=%s' % (node.uuid, fields), + headers={api_base.Version.string: '1.46'}) + self.assertIn('conductor_group', response) + def test_detail(self): node = obj_utils.create_test_node(self.context, chassis_id=self.chassis.id) @@ -439,6 +447,7 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertIn(field, data['nodes'][0]) self.assertIn('storage_interface', data['nodes'][0]) self.assertIn('traits', data['nodes'][0]) + self.assertIn('conductor_group', data['nodes'][0]) # never expose the chassis_id self.assertNotIn('chassis_id', data['nodes'][0]) @@ -467,6 +476,7 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertIn('target_raid_config', data['nodes'][0]) self.assertIn('network_interface', data['nodes'][0]) self.assertIn('resource_class', data['nodes'][0]) + self.assertIn('conductor_group', data['nodes'][0]) for field in api_utils.V31_FIELDS: self.assertIn(field, data['nodes'][0]) # never expose the chassis_id @@ -1411,6 +1421,36 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) self.assertTrue(response.json['error_message']) + def test_get_nodes_by_conductor_group(self): + node1 = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + conductor_group='group1') + node2 = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + conductor_group='group2') + + for base_url in ('/nodes', '/nodes/detail'): + data = self.get_json(base_url + '?conductor_group=group1', + headers={api_base.Version.string: "1.46"}) + uuids = [n['uuid'] for n in data['nodes']] + self.assertIn(node1.uuid, uuids) + self.assertNotIn(node2.uuid, uuids) + data = self.get_json(base_url + '?conductor_group=group2', + headers={api_base.Version.string: "1.46"}) + uuids = [n['uuid'] for n in data['nodes']] + self.assertIn(node2.uuid, uuids) + self.assertNotIn(node1.uuid, uuids) + + def test_get_nodes_by_conductor_group_not_allowed(self): + for url in ('/nodes?conductor_group=group1', + '/nodes/detail?conductor_group=group1'): + response = self.get_json( + url, headers={api_base.Version.string: "1.44"}, + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) + self.assertTrue(response.json['error_message']) + def test_get_console_information(self): node = obj_utils.create_test_node(self.context) expected_console_info = {'test': 'test-data'} @@ -2491,6 +2531,19 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(http_client.BAD_REQUEST, response.status_code) self.assertTrue(response.json['error_message']) + def test_update_conductor_group(self): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + self.mock_update_node.return_value = node + headers = {api_base.Version.string: '1.46'} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/conductor_group', + 'value': 'foogroup', + 'op': 'add'}], + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + def test_update_conductor_group_old_api(self): node = obj_utils.create_test_node(self.context, uuid=uuidutils.generate_uuid()) @@ -2728,8 +2781,16 @@ class TestPost(test_api_base.BaseApiTest): # Check that 'id' is not in first arg of positional args self.assertNotIn('id', cn_mock.call_args[0][0]) + def test_create_node_specify_conductor_group(self): + headers = {api_base.Version.string: '1.46'} + ndict = test_api_utils.post_get_test_node(conductor_group='foo') + self.post_json('/nodes', ndict, headers=headers) + + result = self.get_json('/nodes/%s' % ndict['uuid'], headers=headers) + self.assertEqual('foo', result['conductor_group']) + def test_create_node_specify_conductor_group_bad_version(self): - headers = {api_base.Version.string: '1.43'} + headers = {api_base.Version.string: '1.44'} ndict = test_api_utils.post_get_test_node(conductor_group='foo') response = self.post_json('/nodes', ndict, headers=headers, expect_errors=True) diff --git a/ironic/tests/unit/api/controllers/v1/test_utils.py b/ironic/tests/unit/api/controllers/v1/test_utils.py index e2b236e84..af2a3fe09 100644 --- a/ironic/tests/unit/api/controllers/v1/test_utils.py +++ b/ironic/tests/unit/api/controllers/v1/test_utils.py @@ -270,6 +270,22 @@ class TestApiUtils(base.TestCase): utils.check_allow_filter_driver_type, 'classic') @mock.patch.object(pecan, 'request', spec_set=['version']) + def test_check_allow_filter_by_conductor_group(self, mock_request): + mock_request.version.minor = 46 + self.assertIsNone(utils.check_allow_filter_by_conductor_group('foo')) + + @mock.patch.object(pecan, 'request', spec_set=['version']) + def test_check_allow_filter_by_conductor_group_none(self, mock_request): + mock_request.version.minor = 46 + self.assertIsNone(utils.check_allow_filter_by_conductor_group(None)) + + @mock.patch.object(pecan, 'request', spec_set=['version']) + def test_check_allow_filter_by_conductor_group_fail(self, mock_request): + mock_request.version.minor = 44 + self.assertRaises(exception.NotAcceptable, + utils.check_allow_filter_by_conductor_group, 'foo') + + @mock.patch.object(pecan, 'request', spec_set=['version']) def test_check_allow_driver_detail(self, mock_request): mock_request.version.minor = 30 self.assertIsNone(utils.check_allow_driver_detail(True)) @@ -521,7 +537,11 @@ class TestApiUtils(base.TestCase): mock_request.version.minor = 40 self.assertFalse(utils.allow_inspect_abort()) - def test_allow_conductor_group(self): + @mock.patch.object(pecan, 'request', spec_set=['version']) + def test_allow_conductor_group(self, mock_request): + mock_request.version.minor = 46 + self.assertTrue(utils.allow_conductor_group()) + mock_request.version.minor = 45 self.assertFalse(utils.allow_conductor_group()) diff --git a/ironic/tests/unit/db/test_nodes.py b/ironic/tests/unit/db/test_nodes.py index 590b22420..45b770370 100644 --- a/ironic/tests/unit/db/test_nodes.py +++ b/ironic/tests/unit/db/test_nodes.py @@ -144,7 +144,8 @@ class DbNodeTestCase(base.DbTestCase): uuid=uuidutils.generate_uuid(), maintenance=True, fault='boom', - resource_class='foo') + resource_class='foo', + conductor_group='group1') node3 = utils.create_test_node( driver='driver-one', uuid=uuidutils.generate_uuid(), @@ -188,6 +189,14 @@ class DbNodeTestCase(base.DbTestCase): self.assertEqual([node2.id], [r.id for r in res]) res = self.dbapi.get_nodeinfo_list( + filters={'conductor_group': 'group1'}) + self.assertEqual([node2.id], [r.id for r in res]) + + res = self.dbapi.get_nodeinfo_list( + filters={'conductor_group': 'group2'}) + self.assertEqual([], [r.id for r in res]) + + res = self.dbapi.get_nodeinfo_list( filters={'reserved_by_any_of': ['fake-host', 'another-fake-host']}) self.assertEqual(sorted([node1.id, node3.id]), @@ -292,7 +301,8 @@ class DbNodeTestCase(base.DbTestCase): chassis_id=ch2['id'], maintenance=True, fault='boom', - resource_class='foo') + resource_class='foo', + conductor_group='group1') res = self.dbapi.get_node_list(filters={'chassis_uuid': ch1['uuid']}) self.assertEqual([node1.id], [r.id for r in res]) @@ -333,6 +343,12 @@ class DbNodeTestCase(base.DbTestCase): res = self.dbapi.get_node_list(filters={'resource_class': 'foo'}) self.assertEqual([node2.id], [r.id for r in res]) + res = self.dbapi.get_node_list(filters={'conductor_group': 'group1'}) + self.assertEqual([node2.id], [r.id for r in res]) + + res = self.dbapi.get_node_list(filters={'conductor_group': 'group2'}) + self.assertEqual([], [r.id for r in res]) + res = self.dbapi.get_node_list(filters={'id': node1.id}) self.assertEqual([node1.id], [r.id for r in res]) diff --git a/releasenotes/notes/conductor-groups-c22c17e276e63bed.yaml b/releasenotes/notes/conductor-groups-c22c17e276e63bed.yaml new file mode 100644 index 000000000..8a1857e2f --- /dev/null +++ b/releasenotes/notes/conductor-groups-c22c17e276e63bed.yaml @@ -0,0 +1,17 @@ +--- +features: + - | + Conductors and nodes may be arbitrarily grouped to provide a basic level + of affinity between conductors and nodes. Conductors use the + ``[conductor]/conductor_group`` configuration option to set the group + which they belong to. The same value may be set on one or more nodes + in the ``conductor_group`` field (available in API version 1.46), and + these will be matched such that only conductors with a given group will + manage nodes with the same group. + + A group name may be up to 255 characters containing ``a-z``, ``0-9``, + ``_``, ``-``, and ``.``. The group is case-insensitive. The default group + is the empty string (``""``). + + The "node list" API endpoint (``GET /v1/nodes``) may also be filtered by + conductor group in API version 1.46. |