Allow port queries by shard list

Adds ability to query ports by shard.

Example usage: /v1/ports?shard=lol,cats

Change-Id: Icacef7f71414d30f492a6d144bcc89dff4891784

api-ref/source/baremetal-api-v1-ports.inc

@@ -49,6 +49,10 @@ By default, this query will return the uuid and address for each Port.
 .. versionadded:: 1.53
   Added the ``is_smartnic`` field.
 
+.. versionadded:: 1.82
+  Added the ability to filter ports based on the shard of the node they are
+  associated with.
+
 Normal response code: 200
 
 Request
@@ -60,6 +64,7 @@ Request
     - node_uuid: r_port_node_uuid
     - portgroup: r_port_portgroup_ident
     - address: r_port_address
+    - shard: r_port_shard
     - fields: fields
     - limit: limit
     - marker: marker

api-ref/source/parameters.yaml

@@ -366,6 +366,13 @@ r_port_portgroup_ident:
   in: query
   required: false
   type: string
+r_port_shard:
+  description: |
+    Filter the list of returned Ports, and only return the ones associated
+    with nodes in this specific shard(s), or an empty set if not found.
+  in: query
+  required: false
+  type: array
 r_portgroup_address:
   description: |
     Filter the list of returned Portgroups, and only return the ones with the

ironic/api/controllers/v1/port.py

@@ -208,7 +208,7 @@ class PortsController(rest.RestController):
         self.parent_portgroup_ident = portgroup_ident
 
     def _get_ports_collection(self, node_ident, address, portgroup_ident,
-                              marker, limit, sort_key, sort_dir,
+                              shard, marker, limit, sort_key, sort_dir,
                               resource_url=None, fields=None, detail=None,
                               project=None):
         """Retrieve a collection of ports.
@@ -219,6 +219,8 @@ class PortsController(rest.RestController):
                         this MAC address.
         :param portgroup_ident: UUID or name of a portgroup, to get only ports
                                 for that portgroup.
+        :param shard: A comma-separated shard list, to get only ports for those
+                       shards
         :param marker: pagination marker for large data sets.
         :param limit: maximum number of resources to return in a single result.
                       This value cannot be larger than the value of max_limit
@@ -251,7 +253,11 @@ class PortsController(rest.RestController):
         node_ident = self.parent_node_ident or node_ident
         portgroup_ident = self.parent_portgroup_ident or portgroup_ident
 
-        if node_ident and portgroup_ident:
+        exclusive_filters = 0
+        for i in [node_ident, portgroup_ident, shard]:
+            if i:
+                exclusive_filters += 1
+            if exclusive_filters > 1:
                 raise exception.OperationNotPermitted()
 
         if portgroup_ident:
@@ -279,6 +285,11 @@ class PortsController(rest.RestController):
                                                  project=project)
         elif address:
             ports = self._get_ports_by_address(address, project=project)
+        elif shard:
+            ports = objects.Port.list_by_node_shards(api.request.context,
+                                                     shard, limit,
+                                                     marker_obj, sort_key,
+                                                     sort_dir, project=project)
         else:
             ports = objects.Port.list(api.request.context, limit,
                                       marker_obj, sort_key=sort_key,
@@ -349,10 +360,11 @@ class PortsController(rest.RestController):
                    address=args.mac_address, marker=args.uuid,
                    limit=args.integer, sort_key=args.string,
                    sort_dir=args.string, fields=args.string_list,
-                   portgroup=args.uuid_or_name, detail=args.boolean)
+                   portgroup=args.uuid_or_name, detail=args.boolean,
+                   shard=args.string_list)
     def get_all(self, node=None, node_uuid=None, address=None, marker=None,
                 limit=None, sort_key='id', sort_dir='asc', fields=None,
-                portgroup=None, detail=None):
+                portgroup=None, detail=None, shard=None):
         """Retrieve a list of ports.
 
         Note that the 'node_uuid' interface is deprecated in favour
@@ -375,6 +387,8 @@ class PortsController(rest.RestController):
             of the resource to be returned.
         :param portgroup: UUID or name of a portgroup, to get only ports
                                    for that portgroup.
+        :param shard: Optional, a list of shard ids to filter by, only ports
+                      associated with nodes in these shards will be returned.
         :raises: NotAcceptable, HTTPNotFound
         """
         project = api_utils.check_port_list_policy(
@@ -394,6 +408,8 @@ class PortsController(rest.RestController):
         if portgroup and not api_utils.allow_portgroups_subcontrollers():
             raise exception.NotAcceptable()
 
+        api_utils.check_allow_filter_by_shard(shard)
+
         fields = api_utils.get_request_return_fields(fields, detail,
                                                      _DEFAULT_RETURN_FIELDS)
 
@@ -406,8 +422,9 @@ class PortsController(rest.RestController):
                 raise exception.NotAcceptable()
 
         return self._get_ports_collection(node_uuid or node, address,
-                                          portgroup, marker, limit, sort_key,
+                                          portgroup, shard, marker, limit,
-                                          sort_dir, resource_url='ports',
+                                          sort_key, sort_dir,
+                                          resource_url='ports',
                                           fields=fields, detail=detail,
                                           project=project)
 
@@ -416,10 +433,11 @@ class PortsController(rest.RestController):
     @args.validate(node=args.uuid_or_name, node_uuid=args.uuid,
                    address=args.mac_address, marker=args.uuid,
                    limit=args.integer, sort_key=args.string,
-                   sort_dir=args.string,
+                   sort_dir=args.string, portgroup=args.uuid_or_name,
-                   portgroup=args.uuid_or_name)
+                   shard=args.string_list)
     def detail(self, node=None, node_uuid=None, address=None, marker=None,
-               limit=None, sort_key='id', sort_dir='asc', portgroup=None):
+               limit=None, sort_key='id', sort_dir='asc', portgroup=None,
+               shard=None):
         """Retrieve a list of ports with detail.
 
         Note that the 'node_uuid' interface is deprecated in favour
@@ -433,6 +451,8 @@ class PortsController(rest.RestController):
                         this MAC address.
         :param portgroup: UUID or name of a portgroup, to get only ports
                            for that portgroup.
+        :param shard: comma separated list of shards, to only get ports
+                      associated with nodes in those shards.
         :param marker: pagination marker for large data sets.
         :param limit: maximum number of resources to return in a single result.
                       This value cannot be larger than the value of max_limit
@@ -450,6 +470,8 @@ class PortsController(rest.RestController):
         if portgroup and not api_utils.allow_portgroups_subcontrollers():
             raise exception.NotAcceptable()
 
+        api_utils.check_allow_filter_by_shard(shard)
+
         if not node_uuid and node:
             # We're invoking this interface using positional notation, or
             # explicitly using 'node'.  Try and determine which one.
@@ -464,8 +486,8 @@ class PortsController(rest.RestController):
             raise exception.HTTPNotFound()
 
         return self._get_ports_collection(node_uuid or node, address,
-                                          portgroup, marker, limit, sort_key,
+                                          portgroup, shard, marker, limit,
-                                          sort_dir,
+                                          sort_key, sort_dir,
                                           resource_url='ports/detail',
                                           project=project)
 

ironic/api/controllers/v1/utils.py

@@ -1069,7 +1069,7 @@ def check_allow_filter_by_lessee(lessee):
 def check_allow_filter_by_shard(shard):
     """Check if filtering nodes by shard is allowed.
 
-    Version 1.81 of the API allows filtering nodes by shard.
+    Version 1.82 of the API allows filtering nodes by shard.
     """
     if (shard is not None and api.request.version.minor
             < versions.MINOR_82_NODE_SHARD):

ironic/db/api.py

@@ -296,6 +296,14 @@ class Connection(object, metaclass=abc.ABCMeta):
                          (asc, desc)
         """
 
+    @abc.abstractmethod
+    def get_ports_by_shards(self, shards, limit=None, marker=None,
+                            sort_key=None, sort_dir=None):
+        """Return a list of ports contained in the provided shards.
+
+        :param shard_ids: A list of shards to filter ports by.
+        """
+
     @abc.abstractmethod
     def get_ports_by_node_id(self, node_id, limit=None, marker=None,
                              sort_key=None, sort_dir=None):

ironic/db/sqlalchemy/api.py

@@ -965,6 +965,18 @@ class Connection(api.Connection):
         return _paginate_query(models.Port, limit, marker,
                                sort_key, sort_dir, query)
 
+    def get_ports_by_shards(self, shards, limit=None, marker=None,
+                            sort_key=None, sort_dir=None):
+        shard_node_ids = sa.select(models.Node) \
+            .where(models.Node.shard.in_(shards)) \
+            .with_only_columns(models.Node.id)
+        with _session_for_read() as session:
+            query = session.query(models.Port).filter(
+                models.Port.node_id.in_(shard_node_ids))
+            ports = _paginate_query(
+                models.Port, limit, marker, sort_key, sort_dir, query)
+        return ports
+
     def get_ports_by_node_id(self, node_id, limit=None, marker=None,
                              sort_key=None, sort_dir=None, owner=None,
                              project=None):

ironic/objects/port.py

@@ -299,6 +299,27 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
                                            project=project)
         return cls._from_db_object_list(context, db_ports)
 
+    @classmethod
+    def list_by_node_shards(cls, context, shards, limit=None, marker=None,
+                            sort_key=None, sort_dir=None, project=None):
+        """Return a list of Port objects associated with nodes in shards
+
+        :param context: Security context.
+        :param shards: a list of shards
+        :param limit: maximum number of resources to return in a single result.
+        :param marker: pagination marker for large data sets.
+        :param sort_key: column to sort results by.
+        :param sort_dir: direction to sort. "asc" or "desc".
+        :param project: a node owner or lessee to match against
+        :returns: a list of :class:`Port` object.
+
+        """
+        db_ports = cls.dbapi.get_ports_by_shards(shards, limit=limit,
+                                                 marker=marker,
+                                                 sort_key=sort_key,
+                                                 sort_dir=sort_dir)
+        return cls._from_db_object_list(context, db_ports)
+
     # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable
     # methods can be used in the future to replace current explicit RPC calls.
     # Implications of calling new remote procedures should be thought through.

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

@@ -194,7 +194,7 @@ class TestPortsController__GetPortsCollection(base.TestCase):
         mock_request.context = 'fake-context'
         mock_list.return_value = []
         self.controller._get_ports_collection(None, None, None, None, None,
-                                              None, 'asc',
+                                              None, None, 'asc',
                                               resource_url='ports')
         mock_list.assert_called_once_with('fake-context', 1000, None,
                                           project=None, sort_dir='asc',
@@ -1100,6 +1100,44 @@ class TestListPorts(test_api_base.BaseApiTest):
                       response.json['error_message'])
 
 
+class TestListPortsByShard(test_api_base.BaseApiTest):
+    def setUp(self):
+        super(TestListPortsByShard, self).setUp()
+        self.headers = {
+            api_base.Version.string: '1.%s' % versions.MINOR_82_NODE_SHARD
+        }
+
+    def _create_port_with_shard(self, shard, address):
+        node = obj_utils.create_test_node(self.context, owner='12345',
+                                          shard=shard,
+                                          uuid=uuidutils.generate_uuid())
+        return obj_utils.create_test_port(self.context, name='port_%s' % shard,
+                                          node_id=node.id, address=address,
+                                          uuid=uuidutils.generate_uuid())
+
+    def test_get_by_shard_single_fail_api_version(self):
+        self._create_port_with_shard('test_shard', 'aa:bb:cc:dd:ee:ff')
+        data = self.get_json('/ports?shard=test_shard', expect_errors=True)
+        self.assertEqual(406, data.status_int)
+
+    def test_get_by_shard_single(self):
+        port = self._create_port_with_shard('test_shard', 'aa:bb:cc:dd:ee:ff')
+        data = self.get_json('/ports?shard=test_shard', headers=self.headers)
+        self.assertEqual(port.uuid, data['ports'][0]["uuid"])
+
+    def test_get_by_shard_multi(self):
+        bad_shard_address = 'ee:ee:ee:ee:ee:ee'
+        self._create_port_with_shard('shard1', 'aa:bb:cc:dd:ee:ff')
+        self._create_port_with_shard('shard2', 'ab:bb:cc:dd:ee:ff')
+        self._create_port_with_shard('shard3', bad_shard_address)
+
+        res = self.get_json('/ports?shard=shard1,shard2', headers=self.headers)
+        self.assertEqual(2, len(res['ports']))
+        print(res['ports'][0])
+        self.assertNotEqual(res['ports'][0]['address'], bad_shard_address)
+        self.assertNotEqual(res['ports'][1]['address'], bad_shard_address)
+
+
 @mock.patch.object(rpcapi.ConductorAPI, 'update_port', autospec=True,
                    side_effect=_rpcapi_update_port)
 class TestPatch(test_api_base.BaseApiTest):

ironic/tests/unit/db/test_ports.py

@@ -22,6 +22,23 @@ from ironic.tests.unit.db import base
 from ironic.tests.unit.db import utils as db_utils
 
 
+def _create_test_port_with_shard(shard, address):
+    node = db_utils.create_test_node(
+        uuid=uuidutils.generate_uuid(),
+        owner='12345', lessee='54321', shard=shard)
+    pg = db_utils.create_test_portgroup(
+        name='pg-%s' % shard,
+        uuid=uuidutils.generate_uuid(),
+        node_id=node.id,
+        address=address)
+    return db_utils.create_test_port(
+        name='port-%s' % shard,
+        uuid=uuidutils.generate_uuid(),
+        node_id=node.id,
+        address=address,
+        portgroup_id=pg.id)
+
+
 class DbPortTestCase(base.DbTestCase):
 
     def setUp(self):
@@ -202,6 +219,28 @@ class DbPortTestCase(base.DbTestCase):
     def test_get_ports_by_portgroup_id_that_does_not_exist(self):
         self.assertEqual([], self.dbapi.get_ports_by_portgroup_id(99))
 
+    def test_get_ports_by_shard_no_match(self):
+        res = self.dbapi.get_ports_by_shards(['shard1', 'shard2'])
+        self.assertEqual([], res)
+
+    def test_get_ports_by_shard_with_match_single(self):
+        _create_test_port_with_shard('shard1', 'aa:bb:cc:dd:ee:ff')
+
+        res = self.dbapi.get_ports_by_shards(['shard1'])
+        self.assertEqual(1, len(res))
+        self.assertEqual('port-shard1', res[0].name)
+
+    def test_get_ports_by_shard_with_match_multi(self):
+        _create_test_port_with_shard('shard1', 'aa:bb:cc:dd:ee:ff')
+        _create_test_port_with_shard('shard2', 'ab:bb:cc:dd:ee:ff')
+        _create_test_port_with_shard('shard3', 'ac:bb:cc:dd:ee:ff')
+
+        res = self.dbapi.get_ports_by_shards(['shard1', 'shard2'])
+        self.assertEqual(2, len(res))
+        # note(JayF): We do not query for shard3; ensure we don't get it.
+        self.assertNotEqual('port-shard3', res[0].name)
+        self.assertNotEqual('port-shard3', res[1].name)
+
     def test_destroy_port(self):
         self.dbapi.destroy_port(self.port.id)
         self.assertRaises(exception.PortNotFound,