Expose node.conductor_group in the REST API

This adds microversion 1.46 to allow:

* returning conductor_group in the node object
* specifying conductor_group in the node create API
* specifying conductor_group in the node update API
* allow filtering nodes by conductor_group

Change-Id: I32e0333c78cfcb2d88dfd4f70f0be012dcfef407
Story: 2001795
Task: 22643

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)

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):

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)

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'],

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:

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)
+        self._test_node_field_hidden_in_lower_version('conductor_group',
-        data = self.get_json(
+                                                      '1.45', '1.46')
-            '/nodes/%s' % node.uuid,
-            headers={api_base.Version.string: '1.44'})
-        self.assertNotIn('conductor_group', data)
 
     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)

ironic/tests/unit/api/controllers/v1/test_utils.py

@@ -269,6 +269,22 @@ class TestApiUtils(base.TestCase):
         self.assertRaises(exception.NotAcceptable,
                           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
@@ -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())
 
 

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(),
@@ -187,6 +188,14 @@ class DbNodeTestCase(base.DbTestCase):
         res = self.dbapi.get_nodeinfo_list(filters={'resource_class': 'foo'})
         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']})
@@ -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])
 

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.