openstack
/
openstacksdk
Code Issues Proposed changes

baremetal: Add 'details' parameter to various 'find' proxy methods 

In change I13f32e6ca9be9ed4eb42398aace47e3c5205a81f, we introduced
support for a 'details' parameter to various proxy methods that
supported it. Unfortunately there was a bug in the baremetal proxy layer
changes, which were using a mixin to modify the 'Resource.list' class
method to allow listing detailed responses easily via the resource
layer. This necessitated a revert, change
Icc70bbdc06b5f32722a93775aee2da4d7b7ca4ae.

Take another go at this by reverting the revert and making the necessary
changes to the mixin and proxy layer to make things compatible. We also
fix a few bugs, where certain resources has the mixin applied, resulting
in them attempting to use legacy path-based mechanism to retrieve
detailed resources (GET /resource/detail) even though they didn't
support this.

Change-Id: I6acbcb4d9af35e68c04bb86e50c8844487bd7d6c
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane 2023-08-29 19:18:29 +01:00
parent 30d5753bcf
commit 147b66169d
10 changed files with 308 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

133
openstack/baremetal/v1/_common.py
View File

@ -90,10 +90,39 @@ CHANGE_BOOT_MODE_VERSION = '1.76'
class Resource(resource.Resource):
"""A subclass for resources that use the path to request a detailed view.
Two patterns exist for fetching the detailed view when listing resources.
- As part of the path. For example:
GET /v1/ports/detail
- As a query parameter. For example:
GET /v1/conductors?detail=True
This handles resources that use the former pattern, namely:
- chassis
- nodes
- ports
- portgroups
"""
base_path: str
@classmethod
def list(cls, session, details=False, **params):
def list(
cls,
session,
paginated=True,
base_path=None,
allow_unknown_params=False,
*,
microversion=None,
details=False,
**params,
):
"""This method is a generator which yields resource objects.
This resource object list generator handles pagination and takes query
@ -101,22 +130,102 @@ class Resource(resource.Resource):
:param session: The session to use for making this request.
:type session: :class:`~keystoneauth1.adapter.Adapter`
:param bool details: Whether to return detailed node records
:param bool paginated: ``True`` if a GET to this resource returns
a paginated series of responses, or ``False`` if a GET returns only
one page of data. **When paginated is False only one page of data
will be returned regardless of the API's support of pagination.**
:param str base_path: Base part of the URI for listing resources, if
different from :data:`~openstack.resource.Resource.base_path`.
:param bool allow_unknown_params: ``True`` to accept, but discard
unknown query parameters. This allows getting list of 'filters' and
passing everything known to the server. ``False`` will result in
validation exception when unknown query parameters are passed.
:param str microversion: API version to override the negotiated one.
:param bool details: Whether to return detailed resource records.
:param dict params: These keyword arguments are passed through the
:meth:`~openstack.resource.QueryParameter._transpose` method
to find if any of them match expected query parameters to be
sent in the *params* argument to
:meth:`~keystoneauth1.adapter.Adapter.get`.
:meth:`~openstack.resource.QueryParamter._transpose` method
to find if any of them match expected query parameters to be sent
in the *params* argument to
:meth:`~keystoneauth1.adapter.Adapter.get`. They are additionally
checked against the :data:`~openstack.resource.Resource.base_path`
format string to see if any path fragments need to be filled in by
the contents of this argument.
Parameters supported as filters by the server side are passed in
the API call, remaining parameters are applied as filters to the
retrieved results.
:return: A generator of :class:`openstack.resource.Resource` objects.
:return: A generator of :class:`Resource` objects.
:raises: :exc:`~openstack.exceptions.MethodNotSupported` if
:data:`Resource.allow_list` is not set to ``True``.
:raises: :exc:`~openstack.exceptions.InvalidResourceQuery` if query
contains invalid params.
contains invalid params.
"""
base_path = cls.base_path
if details:
base_path += '/detail'
if not base_path:
base_path = cls.base_path
if details:
base_path += '/detail'
return super().list(
session, paginated=True, base_path=base_path, **params
session,
paginated=paginated,
base_path=base_path,
allow_unknown_params=allow_unknown_params,
microversion=microversion,
**params,
)
@classmethod
def find(
cls,
session,
name_or_id,
ignore_missing=True,
list_base_path=None,
*,
microversion=None,
all_projects=None,
details=False,
**params,
):
"""Find a resource by its name or id.
:param session: The session to use for making this request.
:type session: :class:`~keystoneauth1.adapter.Adapter`
:param name_or_id: This resource's identifier, if needed by
the request. The default is ``None``.
:param bool ignore_missing: When set to ``False``
:class:`~openstack.exceptions.ResourceNotFound` will be raised when
the resource does not exist. When set to ``True``, None will be
returned when attempting to find a nonexistent resource.
:param str list_base_path: base_path to be used when need listing
resources.
:param str microversion: API version to override the negotiated one.
:param bool details: Whether to return detailed resource records.
:param dict params: Any additional parameters to be passed into
underlying methods, such as to
:meth:`~openstack.resource.Resource.existing` in order to pass on
URI parameters.
:return: The :class:`Resource` object matching the given name or id
or None if nothing matches.
:raises: :class:`openstack.exceptions.DuplicateResource` if more
than one resource is found for this request.
:raises: :class:`openstack.exceptions.ResourceNotFound` if nothing
is found and ignore_missing is ``False``.
"""
if not list_base_path:
list_base_path = cls.base_path
if details:
list_base_path += '/detail'
return super().find(
session,
name_or_id,
ignore_missing=ignore_missing,
list_base_path=list_base_path,
microversion=microversion,
all_projects=all_projects,
**params,
)

110
openstack/baremetal/v1/_proxy.py
View File

@ -64,7 +64,7 @@ class Proxy(proxy.Proxy):
error_message="No {resource_type} found for {value}".format(
resource_type=resource_type.__name__, value=value
),
**kwargs
**kwargs,
)
def chassis(self, details=False, **query):
@ -113,7 +113,9 @@ class Proxy(proxy.Proxy):
"""
return self._create(_chassis.Chassis, **attrs)
def find_chassis(self, name_or_id, ignore_missing=True):
# TODO(stephenfin): Delete this. You can't lookup a chassis by name so this
# is identical to get_chassis
def find_chassis(self, name_or_id, ignore_missing=True, *, details=True):
"""Find a single chassis.
:param str name_or_id: The ID of a chassis.
@ -121,11 +123,17 @@ class Proxy(proxy.Proxy):
:class:`~openstack.exceptions.ResourceNotFound` will be raised
when the chassis does not exist. When set to `True``, None will
be returned when attempting to find a nonexistent chassis.
:param details: A boolean indicating whether the detailed information
for the chassis should be returned.
:returns: One :class:`~openstack.baremetal.v1.chassis.Chassis` object
or None.
"""
return self._find(
_chassis.Chassis, name_or_id, ignore_missing=ignore_missing
_chassis.Chassis,
name_or_id,
ignore_missing=ignore_missing,
details=details,
)
def get_chassis(self, chassis, fields=None):
@ -305,7 +313,7 @@ class Proxy(proxy.Proxy):
"""
return self._create(_node.Node, **attrs)
def find_node(self, name_or_id, ignore_missing=True):
def find_node(self, name_or_id, ignore_missing=True, *, details=True):
"""Find a single node.
:param str name_or_id: The name or ID of a node.
@ -313,11 +321,16 @@ class Proxy(proxy.Proxy):
:class:`~openstack.exceptions.ResourceNotFound` will be raised
when the node does not exist. When set to `True``, None will
be returned when attempting to find a nonexistent node.
:param details: A boolean indicating whether the detailed information
for the node should be returned.
:returns: One :class:`~openstack.baremetal.v1.node.Node` object
or None.
"""
return self._find(
_node.Node, name_or_id, ignore_missing=ignore_missing
_node.Node,
name_or_id,
ignore_missing=ignore_missing,
details=details,
)
def get_node(self, node, fields=None):
@ -768,7 +781,9 @@ class Proxy(proxy.Proxy):
"""
return self._create(_port.Port, **attrs)
def find_port(self, name_or_id, ignore_missing=True):
# TODO(stephenfin): Delete this. You can't lookup a port by name so this is
# identical to get_port
def find_port(self, name_or_id, ignore_missing=True, *, details=True):
"""Find a single port.
:param str name_or_id: The ID of a port.
@ -776,11 +791,16 @@ class Proxy(proxy.Proxy):
:class:`~openstack.exceptions.ResourceNotFound` will be raised
when the port does not exist. When set to `True``, None will
be returned when attempting to find a nonexistent port.
:param details: A boolean indicating whether the detailed information
for every port should be returned.
:returns: One :class:`~openstack.baremetal.v1.port.Port` object
or None.
"""
return self._find(
_port.Port, name_or_id, ignore_missing=ignore_missing
_port.Port,
name_or_id,
ignore_missing=ignore_missing,
details=details,
)
def get_port(self, port, fields=None):
@ -887,7 +907,13 @@ class Proxy(proxy.Proxy):
"""
return self._create(_portgroup.PortGroup, **attrs)
def find_port_group(self, name_or_id, ignore_missing=True):
def find_port_group(
self,
name_or_id,
ignore_missing=True,
*,
details=True,
):
"""Find a single port group.
:param str name_or_id: The name or ID of a portgroup.
@ -895,11 +921,16 @@ class Proxy(proxy.Proxy):
:class:`~openstack.exceptions.ResourceNotFound` will be raised
when the port group does not exist. When set to `True``, None will
be returned when attempting to find a nonexistent port group.
:param details: A boolean indicating whether the detailed information
for the port group should be returned.
:returns: One :class:`~openstack.baremetal.v1.port_group.PortGroup`
object or None.
"""
return self._find(
_portgroup.PortGroup, name_or_id, ignore_missing=ignore_missing
_portgroup.PortGroup,
name_or_id,
ignore_missing=ignore_missing,
details=details,
)
def get_port_group(self, port_group, fields=None):
@ -1294,16 +1325,26 @@ class Proxy(proxy.Proxy):
"""
return self._create(_volumeconnector.VolumeConnector, **attrs)
def find_volume_connector(self, vc_id, ignore_missing=True):
# TODO(stephenfin): Delete this. You can't lookup a volume connector by
# name so this is identical to get_volume_connector
def find_volume_connector(
self,
vc_id,
ignore_missing=True,
*,
details=True,
):
"""Find a single volume connector.
:param str vc_id: The ID of a volume connector.
:param bool ignore_missing: When set to ``False``, an exception of
:class:`~openstack.exceptions.ResourceNotFound` will be raised
when the volume connector does not exist. When set to `True``,
None will be returned when attempting to find a nonexistent
volume connector.
:param details: A boolean indicating whether the detailed information
for the volume connector should be returned.
:returns: One
:class:`~openstack.baremetal.v1.volumeconnector.VolumeConnector`
object or None.
@ -1312,6 +1353,7 @@ class Proxy(proxy.Proxy):
_volumeconnector.VolumeConnector,
vc_id,
ignore_missing=ignore_missing,
details=details,
)
def get_volume_connector(self, volume_connector, fields=None):
@ -1441,22 +1483,29 @@ class Proxy(proxy.Proxy):
"""
return self._create(_volumetarget.VolumeTarget, **attrs)
def find_volume_target(self, vt_id, ignore_missing=True):
# TODO(stephenfin): Delete this. You can't lookup a volume target by
# name so this is identical to get_volume_connector
def find_volume_target(self, vt_id, ignore_missing=True, *, details=True):
"""Find a single volume target.
:param str vt_id: The ID of a volume target.
:param bool ignore_missing: When set to ``False``, an exception of
:class:`~openstack.exceptions.ResourceNotFound` will be raised
when the volume connector does not exist. When set to `True``,
None will be returned when attempting to find a nonexistent
volume target.
:param details: A boolean indicating whether the detailed information
for the volume target should be returned.
:returns: One
:class:`~openstack.baremetal.v1.volumetarget.VolumeTarget`
object or None.
"""
return self._find(
_volumetarget.VolumeTarget, vt_id, ignore_missing=ignore_missing
_volumetarget.VolumeTarget,
vt_id,
ignore_missing=ignore_missing,
details=details,
)
def get_volume_target(self, volume_target, fields=None):
@ -1624,6 +1673,35 @@ class Proxy(proxy.Proxy):
_deploytemplates.DeployTemplate, deploy_template, fields=fields
)
def find_deploy_template(
self,
name_or_id,
ignore_missing=True,
*,
details=True,
):
"""Find a single deployment template.
:param str name_or_id: The name or ID of a deployment template.
:param bool ignore_missing: When set to ``False``, an exception of
:class:`~openstack.exceptions.ResourceNotFound` will be raised
when the deployment template does not exist. When set to `True``,
None will be returned when attempting to find a nonexistent
deployment template.
:param details: A boolean indicating whether the detailed information
for the deployment template should be returned.
:returns: One
:class:`~openstack.baremetal.v1.deploy_templates.DeployTemplate` or
None.
"""
return self._find(
_deploytemplates.DeployTemplate,
name_or_id,
ignore_missing=ignore_missing,
details=details,
)
def patch_deploy_template(self, deploy_template, patch):
"""Apply a JSON patch to the deploy_templates.
@ -1655,6 +1733,10 @@ class Proxy(proxy.Proxy):
query['details'] = True
return _conductor.Conductor.list(self, **query)
# NOTE(stephenfin): There is no 'find_conductor' since conductors are
# identified by the host name, not an arbitrary UUID, meaning
# 'find_conductor' would be identical to 'get_conductor'
def get_conductor(self, conductor, fields=None):
"""Get a specific conductor.

2
openstack/baremetal/v1/allocation.py
View File

@ -16,7 +16,7 @@ from openstack import resource
from openstack import utils
class Allocation(_common.Resource):
class Allocation(resource.Resource):
resources_key = 'allocations'
base_path = '/allocations'

2
openstack/baremetal/v1/conductor.py
View File

@ -14,7 +14,7 @@ from openstack.baremetal.v1 import _common
from openstack import resource
class Conductor(_common.Resource):
class Conductor(resource.Resource):
resources_key = 'conductors'
base_path = '/conductors'

2
openstack/baremetal/v1/deploy_templates.py
View File

@ -14,7 +14,7 @@ from openstack.baremetal.v1 import _common
from openstack import resource
class DeployTemplate(_common.Resource):
class DeployTemplate(resource.Resource):
resources_key = 'deploy_templates'
base_path = '/deploy_templates'

2
openstack/baremetal/v1/volume_connector.py
View File

@ -14,7 +14,7 @@ from openstack.baremetal.v1 import _common
from openstack import resource
class VolumeConnector(_common.Resource):
class VolumeConnector(resource.Resource):
resources_key = 'connectors'
base_path = '/volume/connectors'

2
openstack/baremetal/v1/volume_target.py
View File

@ -14,7 +14,7 @@ from openstack.baremetal.v1 import _common
from openstack import resource
class VolumeTarget(_common.Resource):
class VolumeTarget(resource.Resource):
resources_key = 'targets'
base_path = '/volume/targets'

3
openstack/baremetal_introspection/v1/introspection_rule.py
View File

@ -10,11 +10,10 @@
# License for the specific language governing permissions and limitations
# under the License.
from openstack.baremetal.v1 import _common
from openstack import resource
class IntrospectionRule(_common.Resource):
class IntrospectionRule(resource.Resource):
resources_key = 'rules'
base_path = '/rules'

76
openstack/tests/unit/baremetal/v1/test_proxy.py
View File

@ -15,6 +15,7 @@ from unittest import mock
from openstack.baremetal.v1 import _proxy
from openstack.baremetal.v1 import allocation
from openstack.baremetal.v1 import chassis
from openstack.baremetal.v1 import deploy_templates
from openstack.baremetal.v1 import driver
from openstack.baremetal.v1 import node
from openstack.baremetal.v1 import port
@ -60,7 +61,11 @@ class TestChassis(TestBaremetalProxy):
self.verify_create(self.proxy.create_chassis, chassis.Chassis)
def test_find_chassis(self):
self.verify_find(self.proxy.find_chassis, chassis.Chassis)
self.verify_find(
self.proxy.find_chassis,
chassis.Chassis,
expected_kwargs={'details': True},
)
def test_get_chassis(self):
self.verify_get(
@ -110,7 +115,11 @@ class TestNode(TestBaremetalProxy):
self.verify_create(self.proxy.create_node, node.Node)
def test_find_node(self):
self.verify_find(self.proxy.find_node, node.Node)
self.verify_find(
self.proxy.find_node,
node.Node,
expected_kwargs={'details': True},
)
def test_get_node(self):
self.verify_get(
@ -162,7 +171,11 @@ class TestPort(TestBaremetalProxy):
self.verify_create(self.proxy.create_port, port.Port)
def test_find_port(self):
self.verify_find(self.proxy.find_port, port.Port)
self.verify_find(
self.proxy.find_port,
port.Port,
expected_kwargs={'details': True},
)
def test_get_port(self):
self.verify_get(
@ -236,7 +249,9 @@ class TestVolumeConnector(TestBaremetalProxy):
def test_find_volume_connector(self):
self.verify_find(
self.proxy.find_volume_connector, volume_connector.VolumeConnector
self.proxy.find_volume_connector,
volume_connector.VolumeConnector,
expected_kwargs={'details': True},
)
def test_get_volume_connector(self):
@ -282,7 +297,9 @@ class TestVolumeTarget(TestBaremetalProxy):
def test_find_volume_target(self):
self.verify_find(
self.proxy.find_volume_target, volume_target.VolumeTarget
self.proxy.find_volume_target,
volume_target.VolumeTarget,
expected_kwargs={'details': True},
)
def test_get_volume_target(self):
@ -304,6 +321,55 @@ class TestVolumeTarget(TestBaremetalProxy):
)
class TestDeployTemplate(TestBaremetalProxy):
@mock.patch.object(deploy_templates.DeployTemplate, 'list')
def test_deploy_templates_detailed(self, mock_list):
result = self.proxy.deploy_templates(details=True, query=1)
self.assertIs(result, mock_list.return_value)
mock_list.assert_called_once_with(self.proxy, detail=True, query=1)
@mock.patch.object(deploy_templates.DeployTemplate, 'list')
def test_deploy_templates_not_detailed(self, mock_list):
result = self.proxy.deploy_templates(query=1)
self.assertIs(result, mock_list.return_value)
mock_list.assert_called_once_with(self.proxy, query=1)
def test_create_deploy_template(self):
self.verify_create(
self.proxy.create_deploy_template,
deploy_templates.DeployTemplate,
)
def test_find_deploy_template(self):
self.verify_find(
self.proxy.find_deploy_template,
deploy_templates.DeployTemplate,
expected_kwargs={'details': True},
)
def test_get_deploy_template(self):
self.verify_get(
self.proxy.get_deploy_template,
deploy_templates.DeployTemplate,
mock_method=_MOCK_METHOD,
expected_kwargs={'fields': None},
)
def test_delete_deploy_template(self):
self.verify_delete(
self.proxy.delete_deploy_template,
deploy_templates.DeployTemplate,
False,
)
def test_delete_deploy_template_ignore(self):
self.verify_delete(
self.proxy.delete_deploy_template,
deploy_templates.DeployTemplate,
True,
)
class TestMisc(TestBaremetalProxy):
@mock.patch.object(node.Node, 'fetch', autospec=True)
def test__get_with_fields_none(self, mock_fetch):

14
releasenotes/notes/retrieve-detailed-view-for-find-proxy-methods-10ecdff59f5c6913.yaml Normal file
View File

@ -0,0 +1,14 @@
---
features:
- |
The following proxy ``find_*`` operations will now retrieve a detailed
resource by default when retrieving by name:
* Bare metal (v1)
* ``find_chassis``
* ``find_node``
* ``find_port``
* ``find_port_group``
* ``find_volume_connector``
* ``find_volume_target``