Add support for fields in baremetal get_* resources

Omitting Driver since it does not support fields.

Fixes two issues that used to be hidden by ignored 'params' argument
to Resource.fetch:
* Finding/deleting firewall rules ignore filters
* Incorrect invocation of fetch in DNS

Change-Id: I1f0abaec823be327e0e12a090155f97bc59e3489

openstack/baremetal/v1/_proxy.py

@@ -25,6 +25,29 @@ class Proxy(proxy.Proxy):
 
     retriable_status_codes = _common.RETRIABLE_STATUS_CODES
 
+    def _get_with_fields(self, resource_type, value, fields=None):
+        """Fetch a bare metal resource.
+
+        :param resource_type: The type of resource to get.
+        :type resource_type: :class:`~openstack.resource.Resource`
+        :param value: The value to get. Can be either the ID of a
+                      resource or a :class:`~openstack.resource.Resource`
+                      subclass.
+        :param fields: Limit the resource fields to fetch.
+
+        :returns: The result of the ``fetch``
+        :rtype: :class:`~openstack.resource.Resource`
+        """
+        res = self._get_resource(resource_type, value)
+        kwargs = {}
+        if fields:
+            kwargs['fields'] = _common.comma_separated_list(fields)
+        return res.fetch(
+            self,
+            error_message="No {resource_type} found for {value}".format(
+                resource_type=resource_type.__name__, value=value),
+            **kwargs)
+
     def chassis(self, details=False, **query):
         """Retrieve a generator of chassis.
 
@@ -85,17 +108,18 @@ class Proxy(proxy.Proxy):
         return self._find(_chassis.Chassis, name_or_id,
                           ignore_missing=ignore_missing)
 
-    def get_chassis(self, chassis):
+    def get_chassis(self, chassis, fields=None):
         """Get a specific chassis.
 
         :param chassis: The value can be the ID of a chassis or a
             :class:`~openstack.baremetal.v1.chassis.Chassis` instance.
+        :param fields: Limit the resource fields to fetch.
 
         :returns: One :class:`~openstack.baremetal.v1.chassis.Chassis`
         :raises: :class:`~openstack.exceptions.ResourceNotFound` when no
             chassis matching the name or ID could be found.
         """
-        return self._get(_chassis.Chassis, chassis)
+        return self._get_with_fields(_chassis.Chassis, chassis, fields=fields)
 
     def update_chassis(self, chassis, **attrs):
         """Update a chassis.
@@ -239,17 +263,18 @@ class Proxy(proxy.Proxy):
         return self._find(_node.Node, name_or_id,
                           ignore_missing=ignore_missing)
 
-    def get_node(self, node):
+    def get_node(self, node, fields=None):
         """Get a specific node.
 
         :param node: The value can be the name or ID of a node or a
             :class:`~openstack.baremetal.v1.node.Node` instance.
+        :param fields: Limit the resource fields to fetch.
 
         :returns: One :class:`~openstack.baremetal.v1.node.Node`
         :raises: :class:`~openstack.exceptions.ResourceNotFound` when no
             node matching the name or ID could be found.
         """
-        return self._get(_node.Node, node)
+        return self._get_with_fields(_node.Node, node, fields=fields)
 
     def update_node(self, node, retry_on_conflict=True, **attrs):
         """Update a node.
@@ -552,23 +577,18 @@ class Proxy(proxy.Proxy):
         return self._find(_port.Port, name_or_id,
                           ignore_missing=ignore_missing)
 
-    def get_port(self, port, **query):
+    def get_port(self, port, fields=None):
         """Get a specific port.
 
         :param port: The value can be the ID of a port or a
             :class:`~openstack.baremetal.v1.port.Port` instance.
-        :param dict query: Optional query parameters to be sent to restrict
+        :param fields: Limit the resource fields to fetch.
-            the port properties returned. Available parameters include:
-
-            * ``fields``: A list containing one or more fields to be returned
-              in the response. This may lead to some performance gain
-              because other fields of the resource are not refreshed.
 
         :returns: One :class:`~openstack.baremetal.v1.port.Port`
         :raises: :class:`~openstack.exceptions.ResourceNotFound` when no
             port matching the name or ID could be found.
         """
-        return self._get(_port.Port, port, **query)
+        return self._get_with_fields(_port.Port, port, fields=fields)
 
     def update_port(self, port, **attrs):
         """Update a port.
@@ -675,23 +695,19 @@ class Proxy(proxy.Proxy):
         return self._find(_portgroup.PortGroup, name_or_id,
                           ignore_missing=ignore_missing)
 
-    def get_port_group(self, port_group, **query):
+    def get_port_group(self, port_group, fields=None):
         """Get a specific port group.
 
         :param port_group: The value can be the name or ID of a chassis or a
             :class:`~openstack.baremetal.v1.port_group.PortGroup` instance.
-        :param dict query: Optional query parameters to be sent to restrict
+        :param fields: Limit the resource fields to fetch.
-            the port group properties returned. Available parameters include:
-
-            * ``fields``: A list containing one or more fields to be returned
-              in the response. This may lead to some performance gain
-              because other fields of the resource are not refreshed.
 
         :returns: One :class:`~openstack.baremetal.v1.port_group.PortGroup`
         :raises: :class:`~openstack.exceptions.ResourceNotFound` when no
             port group matching the name or ID could be found.
         """
-        return self._get(_portgroup.PortGroup, port_group, **query)
+        return self._get_with_fields(_portgroup.PortGroup, port_group,
+                                     fields=fields)
 
     def update_port_group(self, port_group, **attrs):
         """Update a port group.
@@ -842,17 +858,19 @@ class Proxy(proxy.Proxy):
         """
         return self._create(_allocation.Allocation, **attrs)
 
-    def get_allocation(self, allocation):
+    def get_allocation(self, allocation, fields=None):
         """Get a specific allocation.
 
         :param allocation: The value can be the name or ID of an allocation or
             a :class:`~openstack.baremetal.v1.allocation.Allocation` instance.
+        :param fields: Limit the resource fields to fetch.
 
         :returns: One :class:`~openstack.baremetal.v1.allocation.Allocation`
         :raises: :class:`~openstack.exceptions.ResourceNotFound` when no
             allocation matching the name or ID could be found.
         """
-        return self._get(_allocation.Allocation, allocation)
+        return self._get_with_fields(_allocation.Allocation, allocation,
+                                     fields=fields)
 
     def update_allocation(self, allocation, **attrs):
         """Update an allocation.

openstack/dns/v2/_base.py

@@ -49,7 +49,7 @@ class Resource(resource.Resource):
                 id=name_or_id,
                 connection=session._get_connection(),
                 **params)
-            return match.fetch(session, **params)
+            return match.fetch(session)
         except exceptions.SDKException:
             # DNS may return 400 when we try to do GET with name
             pass

openstack/resource.py

@@ -1295,7 +1295,8 @@ class Resource(dict):
                                         base_path=base_path)
         session = self._get_session(session)
         microversion = self._get_microversion_for(session, 'fetch')
-        response = session.get(request.url, microversion=microversion)
+        response = session.get(request.url, microversion=microversion,
+                               params=params)
         kwargs = {}
         if error_message:
             kwargs['error_message'] = error_message

openstack/tests/functional/baremetal/test_baremetal_allocation.py

@@ -55,6 +55,11 @@ class TestBareMetalAllocation(Base):
         self.assertEqual(self.node.id, allocation.node_id)
         self.assertIsNone(allocation.last_error)
 
+        with_fields = self.conn.baremetal.get_allocation(
+            allocation.id, fields=['uuid', 'node_uuid'])
+        self.assertEqual(allocation.id, with_fields.id)
+        self.assertIsNone(with_fields.state)
+
         node = self.conn.baremetal.get_node(self.node.id)
         self.assertEqual(allocation.id, node.allocation_id)
 

openstack/tests/functional/baremetal/test_baremetal_node.py

@@ -34,6 +34,13 @@ class TestBareMetalNode(base.BaseBaremetalTest):
             self.assertEqual(node.id, found.id)
             self.assertEqual(node.name, found.name)
 
+        with_fields = self.conn.baremetal.get_node('node-name',
+                                                   fields=['uuid', 'driver'])
+        self.assertEqual(node.id, with_fields.id)
+        self.assertEqual(node.driver, with_fields.driver)
+        self.assertIsNone(with_fields.name)
+        self.assertIsNone(with_fields.provision_state)
+
         nodes = self.conn.baremetal.nodes()
         self.assertIn(node.id, [n.id for n in nodes])
 

openstack/tests/functional/baremetal/test_baremetal_port.py

@@ -31,6 +31,12 @@ class TestBareMetalPort(base.BaseBaremetalTest):
 
         loaded = self.conn.baremetal.get_port(port.id)
         self.assertEqual(loaded.id, port.id)
+        self.assertIsNotNone(loaded.address)
+
+        with_fields = self.conn.baremetal.get_port(port.id,
+                                                   fields=['uuid', 'extra'])
+        self.assertEqual(port.id, with_fields.id)
+        self.assertIsNone(with_fields.address)
 
         self.conn.baremetal.delete_port(port, ignore_missing=False)
         self.assertRaises(exceptions.ResourceNotFound,

openstack/tests/functional/baremetal/test_baremetal_port_group.py

@@ -28,6 +28,12 @@ class TestBareMetalPortGroup(base.BaseBaremetalTest):
 
         loaded = self.conn.baremetal.get_port_group(port_group.id)
         self.assertEqual(loaded.id, port_group.id)
+        self.assertIsNotNone(loaded.node_id)
+
+        with_fields = self.conn.baremetal.get_port_group(
+            port_group.id, fields=['uuid', 'extra'])
+        self.assertEqual(port_group.id, with_fields.id)
+        self.assertIsNone(with_fields.node_id)
 
         self.conn.baremetal.delete_port_group(port_group,
                                               ignore_missing=False)

openstack/tests/unit/baremetal/v1/test_proxy.py

@@ -24,6 +24,9 @@ from openstack.tests.unit import base
 from openstack.tests.unit import test_proxy_base
 
 
+_MOCK_METHOD = 'openstack.baremetal.v1._proxy.Proxy._get_with_fields'
+
+
 class TestBaremetalProxy(test_proxy_base.TestProxyBase):
 
     def setUp(self):
@@ -55,7 +58,9 @@ class TestBaremetalProxy(test_proxy_base.TestProxyBase):
         self.verify_find(self.proxy.find_chassis, chassis.Chassis)
 
     def test_get_chassis(self):
-        self.verify_get(self.proxy.get_chassis, chassis.Chassis)
+        self.verify_get(self.proxy.get_chassis, chassis.Chassis,
+                        mock_method=_MOCK_METHOD,
+                        expected_kwargs={'fields': None})
 
     def test_update_chassis(self):
         self.verify_update(self.proxy.update_chassis, chassis.Chassis)
@@ -85,7 +90,9 @@ class TestBaremetalProxy(test_proxy_base.TestProxyBase):
         self.verify_find(self.proxy.find_node, node.Node)
 
     def test_get_node(self):
-        self.verify_get(self.proxy.get_node, node.Node)
+        self.verify_get(self.proxy.get_node, node.Node,
+                        mock_method=_MOCK_METHOD,
+                        expected_kwargs={'fields': None})
 
     @mock.patch.object(node.Node, 'commit', autospec=True)
     def test_update_node(self, mock_commit):
@@ -127,7 +134,9 @@ class TestBaremetalProxy(test_proxy_base.TestProxyBase):
         self.verify_find(self.proxy.find_port, port.Port)
 
     def test_get_port(self):
-        self.verify_get(self.proxy.get_port, port.Port)
+        self.verify_get(self.proxy.get_port, port.Port,
+                        mock_method=_MOCK_METHOD,
+                        expected_kwargs={'fields': None})
 
     def test_update_port(self):
         self.verify_update(self.proxy.update_port, port.Port)
@@ -150,11 +159,18 @@ class TestBaremetalProxy(test_proxy_base.TestProxyBase):
         self.assertIs(result, mock_list.return_value)
         mock_list.assert_called_once_with(self.proxy, details=False, query=1)
 
+    def test_get_port_group(self):
+        self.verify_get(self.proxy.get_port_group, port_group.PortGroup,
+                        mock_method=_MOCK_METHOD,
+                        expected_kwargs={'fields': None})
+
     def test_create_allocation(self):
         self.verify_create(self.proxy.create_allocation, allocation.Allocation)
 
     def test_get_allocation(self):
-        self.verify_get(self.proxy.get_allocation, allocation.Allocation)
+        self.verify_get(self.proxy.get_allocation, allocation.Allocation,
+                        mock_method=_MOCK_METHOD,
+                        expected_kwargs={'fields': None})
 
     def test_delete_allocation(self):
         self.verify_delete(self.proxy.delete_allocation, allocation.Allocation,
@@ -164,6 +180,22 @@ class TestBaremetalProxy(test_proxy_base.TestProxyBase):
         self.verify_delete(self.proxy.delete_allocation, allocation.Allocation,
                            True)
 
+    @mock.patch.object(node.Node, 'fetch', autospec=True)
+    def test__get_with_fields_none(self, mock_fetch):
+        result = self.proxy._get_with_fields(node.Node, 'value')
+        self.assertIs(result, mock_fetch.return_value)
+        mock_fetch.assert_called_once_with(mock.ANY, self.proxy,
+                                           error_message=mock.ANY)
+
+    @mock.patch.object(node.Node, 'fetch', autospec=True)
+    def test__get_with_fields(self, mock_fetch):
+        result = self.proxy._get_with_fields(node.Node, 'value',
+                                             fields=['a', 'b'])
+        self.assertIs(result, mock_fetch.return_value)
+        mock_fetch.assert_called_once_with(mock.ANY, self.proxy,
+                                           error_message=mock.ANY,
+                                           fields='a,b')
+
 
 @mock.patch('time.sleep', lambda _sec: None)
 @mock.patch.object(_proxy.Proxy, 'get_node', autospec=True)

openstack/tests/unit/cloud/test_fwaas.py

@@ -107,7 +107,8 @@ class TestFirewallRule(FirewallTestCase):
         self.register_uris([
             dict(method='GET',  # short-circuit
                  uri=self._make_mock_url('firewall_rules',
-                                         self.firewall_rule_name),
+                                         self.firewall_rule_name,
+                                         **filters),
                  status_code=404),
             dict(method='GET',
                  uri=self._make_mock_url(
@@ -232,7 +233,7 @@ class TestFirewallRule(FirewallTestCase):
             dict(
                 method='GET',
                 uri=self._make_mock_url(
-                    'firewall_rules', self.firewall_rule_name),
+                    'firewall_rules', self.firewall_rule_name, **filters),
                 json={'firewall_rule': self._mock_firewall_rule_attrs}),
             dict(
                 method='PUT',

openstack/tests/unit/image/v2/test_image.py

@@ -355,7 +355,7 @@ class TestImage(base.TestCase):
         self.sess.get.assert_has_calls(
             [mock.call('images/IDENTIFIER/file',
                        stream=False),
-             mock.call('images/IDENTIFIER', microversion=None)])
+             mock.call('images/IDENTIFIER', microversion=None, params={})])
 
         self.assertEqual(rv, resp1)
 
@@ -384,7 +384,7 @@ class TestImage(base.TestCase):
         self.sess.get.assert_has_calls(
             [mock.call('images/IDENTIFIER/file',
                        stream=False),
-             mock.call('images/IDENTIFIER', microversion=None)])
+             mock.call('images/IDENTIFIER', microversion=None, params={})])
 
         self.assertEqual(rv, resp1)
 
@@ -483,7 +483,8 @@ class TestImage(base.TestCase):
         result = sot.find(self.sess, EXAMPLE['name'])
 
         self.sess.get.assert_has_calls([
-            mock.call('images/' + EXAMPLE['name'], microversion=None),
+            mock.call('images/' + EXAMPLE['name'], microversion=None,
+                      params={}),
             mock.call('/images', headers={'Accept': 'application/json'},
                       microversion=None, params={'name': EXAMPLE['name']}),
             mock.call('/images', headers={'Accept': 'application/json'},

openstack/tests/unit/test_resource.py

@@ -1445,7 +1445,19 @@ class TestResourceActions(base.TestCase):
         self.sot._prepare_request.assert_called_once_with(
             requires_id=True, base_path=None)
         self.session.get.assert_called_once_with(
-            self.request.url, microversion=None)
+            self.request.url, microversion=None, params={})
+
+        self.assertIsNone(self.sot.microversion)
+        self.sot._translate_response.assert_called_once_with(self.response)
+        self.assertEqual(result, self.sot)
+
+    def test_fetch_with_params(self):
+        result = self.sot.fetch(self.session, fields='a,b')
+
+        self.sot._prepare_request.assert_called_once_with(
+            requires_id=True, base_path=None)
+        self.session.get.assert_called_once_with(
+            self.request.url, microversion=None, params={'fields': 'a,b'})
 
         self.assertIsNone(self.sot.microversion)
         self.sot._translate_response.assert_called_once_with(self.response)
@@ -1467,7 +1479,7 @@ class TestResourceActions(base.TestCase):
         sot._prepare_request.assert_called_once_with(
             requires_id=True, base_path=None)
         self.session.get.assert_called_once_with(
-            self.request.url, microversion='1.42')
+            self.request.url, microversion='1.42', params={})
 
         self.assertEqual(sot.microversion, '1.42')
         sot._translate_response.assert_called_once_with(self.response)
@@ -1479,7 +1491,7 @@ class TestResourceActions(base.TestCase):
         self.sot._prepare_request.assert_called_once_with(
             requires_id=False, base_path=None)
         self.session.get.assert_called_once_with(
-            self.request.url, microversion=None)
+            self.request.url, microversion=None, params={})
 
         self.sot._translate_response.assert_called_once_with(self.response)
         self.assertEqual(result, self.sot)
@@ -1491,7 +1503,7 @@ class TestResourceActions(base.TestCase):
             requires_id=False,
             base_path='dummy')
         self.session.get.assert_called_once_with(
-            self.request.url, microversion=None)
+            self.request.url, microversion=None, params={})
 
         self.sot._translate_response.assert_called_once_with(self.response)
         self.assertEqual(result, self.sot)

releasenotes/notes/baremetal-fields-624546fa533a8287.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    Adds support for fetching specific fields when getting bare metal
+    `Node`, `Port`, `PortGroup`, `Chassis` and `Allocation` resources.