From a263dfb4ee558e69ad4d3ad30e2afe7d15429ca4 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 13 Aug 2019 12:24:00 +0200 Subject: [PATCH] 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 | 62 ++++++++++++------- openstack/dns/v2/_base.py | 2 +- openstack/resource.py | 3 +- .../baremetal/test_baremetal_allocation.py | 5 ++ .../baremetal/test_baremetal_node.py | 7 +++ .../baremetal/test_baremetal_port.py | 6 ++ .../baremetal/test_baremetal_port_group.py | 6 ++ .../tests/unit/baremetal/v1/test_proxy.py | 40 ++++++++++-- openstack/tests/unit/cloud/test_fwaas.py | 5 +- openstack/tests/unit/image/v2/test_image.py | 7 ++- openstack/tests/unit/test_resource.py | 20 ++++-- .../baremetal-fields-624546fa533a8287.yaml | 5 ++ 12 files changed, 131 insertions(+), 37 deletions(-) create mode 100644 releasenotes/notes/baremetal-fields-624546fa533a8287.yaml diff --git a/openstack/baremetal/v1/_proxy.py b/openstack/baremetal/v1/_proxy.py index e15c367b6..8a000a535 100644 --- a/openstack/baremetal/v1/_proxy.py +++ b/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 - 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. + :param fields: Limit the resource fields to fetch. :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 - 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. + :param fields: Limit the resource fields to fetch. :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. diff --git a/openstack/dns/v2/_base.py b/openstack/dns/v2/_base.py index 40601a5f7..e2e3fa695 100644 --- a/openstack/dns/v2/_base.py +++ b/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 diff --git a/openstack/resource.py b/openstack/resource.py index a6ab26d06..28106c271 100644 --- a/openstack/resource.py +++ b/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 diff --git a/openstack/tests/functional/baremetal/test_baremetal_allocation.py b/openstack/tests/functional/baremetal/test_baremetal_allocation.py index 9ffc52560..2f93285b5 100644 --- a/openstack/tests/functional/baremetal/test_baremetal_allocation.py +++ b/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) diff --git a/openstack/tests/functional/baremetal/test_baremetal_node.py b/openstack/tests/functional/baremetal/test_baremetal_node.py index 92a93dc8f..a7a07be95 100644 --- a/openstack/tests/functional/baremetal/test_baremetal_node.py +++ b/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]) diff --git a/openstack/tests/functional/baremetal/test_baremetal_port.py b/openstack/tests/functional/baremetal/test_baremetal_port.py index 5c41c0e8e..2ac7fc050 100644 --- a/openstack/tests/functional/baremetal/test_baremetal_port.py +++ b/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, diff --git a/openstack/tests/functional/baremetal/test_baremetal_port_group.py b/openstack/tests/functional/baremetal/test_baremetal_port_group.py index bd74db5bd..3e5eeb0c8 100644 --- a/openstack/tests/functional/baremetal/test_baremetal_port_group.py +++ b/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) diff --git a/openstack/tests/unit/baremetal/v1/test_proxy.py b/openstack/tests/unit/baremetal/v1/test_proxy.py index cd6c65d53..f5902db97 100644 --- a/openstack/tests/unit/baremetal/v1/test_proxy.py +++ b/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) diff --git a/openstack/tests/unit/cloud/test_fwaas.py b/openstack/tests/unit/cloud/test_fwaas.py index 4abccb054..ccf7d78e9 100644 --- a/openstack/tests/unit/cloud/test_fwaas.py +++ b/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', diff --git a/openstack/tests/unit/image/v2/test_image.py b/openstack/tests/unit/image/v2/test_image.py index 8981db8f8..8d0cafed9 100644 --- a/openstack/tests/unit/image/v2/test_image.py +++ b/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'}, diff --git a/openstack/tests/unit/test_resource.py b/openstack/tests/unit/test_resource.py index d435a89ae..9a39fc21e 100644 --- a/openstack/tests/unit/test_resource.py +++ b/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) diff --git a/releasenotes/notes/baremetal-fields-624546fa533a8287.yaml b/releasenotes/notes/baremetal-fields-624546fa533a8287.yaml new file mode 100644 index 000000000..053140d58 --- /dev/null +++ b/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.