diff --git a/openstack/baremetal/v1/_common.py b/openstack/baremetal/v1/_common.py index 0c29e50ec..4217c2909 100644 --- a/openstack/baremetal/v1/_common.py +++ b/openstack/baremetal/v1/_common.py @@ -10,6 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. +from openstack import resource + RETRIABLE_STATUS_CODES = [ # HTTP Conflict - happens if a node is locked @@ -91,3 +93,16 @@ def comma_separated_list(value): return None else: return ','.join(value) + + +def fields_type(value, resource_type): + if value is None: + return None + + resource_mapping = { + key: value.name + for key, value in resource_type.__dict__.items() + if isinstance(value, resource.Body) + } + + return comma_separated_list(resource_mapping.get(x, x) for x in value) diff --git a/openstack/baremetal/v1/_proxy.py b/openstack/baremetal/v1/_proxy.py index 8a000a535..eea434227 100644 --- a/openstack/baremetal/v1/_proxy.py +++ b/openstack/baremetal/v1/_proxy.py @@ -41,7 +41,7 @@ class Proxy(proxy.Proxy): res = self._get_resource(resource_type, value) kwargs = {} if fields: - kwargs['fields'] = _common.comma_separated_list(fields) + kwargs['fields'] = _common.fields_type(fields, resource_type) return res.fetch( self, error_message="No {resource_type} found for {value}".format( diff --git a/openstack/baremetal/v1/allocation.py b/openstack/baremetal/v1/allocation.py index 40f57ee74..a1b8b728b 100644 --- a/openstack/baremetal/v1/allocation.py +++ b/openstack/baremetal/v1/allocation.py @@ -33,7 +33,7 @@ class Allocation(_common.ListMixin, resource.Resource): _query_mapping = resource.QueryParameters( 'node', 'resource_class', 'state', - fields={'name': 'fields', 'type': _common.comma_separated_list}, + fields={'type': _common.fields_type}, ) # Allocation update is available since 1.57 diff --git a/openstack/baremetal/v1/chassis.py b/openstack/baremetal/v1/chassis.py index 89ac5c571..635177e31 100644 --- a/openstack/baremetal/v1/chassis.py +++ b/openstack/baremetal/v1/chassis.py @@ -33,7 +33,7 @@ class Chassis(_common.ListMixin, resource.Resource): commit_jsonpatch = True _query_mapping = resource.QueryParameters( - fields={'name': 'fields', 'type': _common.comma_separated_list}, + fields={'type': _common.fields_type}, ) #: Timestamp at which the chassis was created. diff --git a/openstack/baremetal/v1/node.py b/openstack/baremetal/v1/node.py index e32c2a5fc..9b81937b6 100644 --- a/openstack/baremetal/v1/node.py +++ b/openstack/baremetal/v1/node.py @@ -48,7 +48,7 @@ class Node(_common.ListMixin, resource.Resource): _query_mapping = resource.QueryParameters( 'associated', 'conductor_group', 'driver', 'fault', 'provision_state', 'resource_class', - fields={'name': 'fields', 'type': _common.comma_separated_list}, + fields={'type': _common.fields_type}, instance_id='instance_uuid', is_maintenance='maintenance', ) diff --git a/openstack/baremetal/v1/port.py b/openstack/baremetal/v1/port.py index 338c29e01..a5a309526 100644 --- a/openstack/baremetal/v1/port.py +++ b/openstack/baremetal/v1/port.py @@ -31,7 +31,7 @@ class Port(_common.ListMixin, resource.Resource): _query_mapping = resource.QueryParameters( 'address', 'node', 'portgroup', - fields={'name': 'fields', 'type': _common.comma_separated_list}, + fields={'type': _common.fields_type}, node_id='node_uuid', ) diff --git a/openstack/baremetal/v1/port_group.py b/openstack/baremetal/v1/port_group.py index 939ad78bf..2fb55267b 100644 --- a/openstack/baremetal/v1/port_group.py +++ b/openstack/baremetal/v1/port_group.py @@ -31,7 +31,7 @@ class PortGroup(_common.ListMixin, resource.Resource): _query_mapping = resource.QueryParameters( 'node', 'address', - fields={'name': 'fields', 'type': _common.comma_separated_list}, + fields={'type': _common.fields_type}, ) # The mode and properties field introduced in 1.26. diff --git a/openstack/message/v2/message.py b/openstack/message/v2/message.py index 8b8d59385..e30d0f98d 100644 --- a/openstack/message/v2/message.py +++ b/openstack/message/v2/message.py @@ -86,7 +86,7 @@ class Message(resource.Resource): ) or session.get_project_id() } - query_params = cls._query_mapping._transpose(params) + query_params = cls._query_mapping._transpose(params, cls) while more_data: resp = session.get(uri, headers=headers, params=query_params) diff --git a/openstack/message/v2/queue.py b/openstack/message/v2/queue.py index 7d580e486..d6f60a8b9 100644 --- a/openstack/message/v2/queue.py +++ b/openstack/message/v2/queue.py @@ -74,7 +74,7 @@ class Queue(resource.Resource): and `X-PROJECT-ID` fields which are required by Zaqar v2 API. """ more_data = True - query_params = cls._query_mapping._transpose(params) + query_params = cls._query_mapping._transpose(params, cls) if base_path is None: base_path = cls.base_path diff --git a/openstack/message/v2/subscription.py b/openstack/message/v2/subscription.py index 3c6322869..d9b129c49 100644 --- a/openstack/message/v2/subscription.py +++ b/openstack/message/v2/subscription.py @@ -93,7 +93,7 @@ class Subscription(resource.Resource): ) or session.get_project_id() } - query_params = cls._query_mapping._transpose(params) + query_params = cls._query_mapping._transpose(params, cls) while more_data: resp = session.get(uri, headers=headers, params=query_params) diff --git a/openstack/resource.py b/openstack/resource.py index 3eb2da725..9ef70b941 100644 --- a/openstack/resource.py +++ b/openstack/resource.py @@ -32,6 +32,7 @@ and then returned to the caller. """ import collections +import inspect import itertools import jsonpatch @@ -303,8 +304,8 @@ class QueryParameters(object): """ expected_params = list(self._mapping.keys()) expected_params.extend( - value['name'] if isinstance(value, dict) else value - for value in self._mapping.values()) + value.get('name', key) if isinstance(value, dict) else value + for key, value in self._mapping.items()) if base_path: expected_params += utils.get_string_format_keys(base_path) @@ -323,7 +324,7 @@ class QueryParameters(object): set(expected_params)) return {k: query[k] for k in known_keys} - def _transpose(self, query): + def _transpose(self, query, resource_type): """Transpose the keys in query based on the mapping If a query is supplied with its server side name, we will still use @@ -332,16 +333,25 @@ class QueryParameters(object): :param dict query: Collection of key-value pairs where each key is the client-side parameter name to be transposed to its server side name. + :param resource_type: Class of a resource. """ result = {} for client_side, server_side in self._mapping.items(): if isinstance(server_side, dict): - name = server_side['name'] + name = server_side.get('name', client_side) type_ = server_side.get('type') else: name = server_side type_ = None + # NOTE(dtantsur): a small hack to be compatible with both + # single-argument (like int) and double-argument type functions. + try: + provide_resource_type = ( + len(inspect.getargspec(type_).args) > 1) + except TypeError: + provide_resource_type = False + if client_side in query: value = query[client_side] elif name in query: @@ -350,7 +360,10 @@ class QueryParameters(object): continue if type_ is not None: - result[name] = type_(value) + if provide_resource_type: + result[name] = type_(value, resource_type) + else: + result[name] = type_(value) else: result[name] = value return result @@ -1568,7 +1581,7 @@ class Resource(dict): params = cls._query_mapping._validate( params, base_path=base_path, allow_unknown_params=allow_unknown_params) - query_params = cls._query_mapping._transpose(params) + query_params = cls._query_mapping._transpose(params, cls) uri = base_path % params limit = query_params.get('limit') diff --git a/openstack/tests/functional/baremetal/test_baremetal_node.py b/openstack/tests/functional/baremetal/test_baremetal_node.py index a7a07be95..a08f8e1f3 100644 --- a/openstack/tests/functional/baremetal/test_baremetal_node.py +++ b/openstack/tests/functional/baremetal/test_baremetal_node.py @@ -34,8 +34,9 @@ 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']) + with_fields = self.conn.baremetal.get_node( + 'node-name', + fields=['uuid', 'driver', 'instance_id']) self.assertEqual(node.id, with_fields.id) self.assertEqual(node.driver, with_fields.driver) self.assertIsNone(with_fields.name) @@ -275,7 +276,8 @@ class TestBareMetalNodeFields(base.BaseBaremetalTest): def test_node_fields(self): self.create_node() - result = self.conn.baremetal.nodes(fields=['uuid', 'name']) + result = self.conn.baremetal.nodes( + fields=['uuid', 'name', 'instance_id']) for item in result: self.assertIsNotNone(item.id) self.assertIsNone(item.driver) diff --git a/openstack/tests/functional/baremetal/test_baremetal_port.py b/openstack/tests/functional/baremetal/test_baremetal_port.py index 2ac7fc050..ee2488f67 100644 --- a/openstack/tests/functional/baremetal/test_baremetal_port.py +++ b/openstack/tests/functional/baremetal/test_baremetal_port.py @@ -33,8 +33,8 @@ class TestBareMetalPort(base.BaseBaremetalTest): self.assertEqual(loaded.id, port.id) self.assertIsNotNone(loaded.address) - with_fields = self.conn.baremetal.get_port(port.id, - fields=['uuid', 'extra']) + with_fields = self.conn.baremetal.get_port( + port.id, fields=['uuid', 'extra', 'node_id']) self.assertEqual(port.id, with_fields.id) self.assertIsNone(with_fields.address) @@ -120,7 +120,7 @@ class TestBareMetalPortFields(base.BaseBaremetalTest): def test_port_fields(self): self.create_node() self.create_port(address='11:22:33:44:55:66') - result = self.conn.baremetal.ports(fields=['uuid']) + result = self.conn.baremetal.ports(fields=['uuid', 'node_id']) for item in result: self.assertIsNotNone(item.id) self.assertIsNone(item.address) diff --git a/openstack/tests/unit/baremetal/v1/test_proxy.py b/openstack/tests/unit/baremetal/v1/test_proxy.py index f5902db97..ee3600d88 100644 --- a/openstack/tests/unit/baremetal/v1/test_proxy.py +++ b/openstack/tests/unit/baremetal/v1/test_proxy.py @@ -188,13 +188,25 @@ class TestBaremetalProxy(test_proxy_base.TestProxyBase): 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']) + def test__get_with_fields_node(self, mock_fetch): + result = self.proxy._get_with_fields( + # Mix of server-side and client-side fields + node.Node, 'value', fields=['maintenance', 'id', 'instance_id']) 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_fetch.assert_called_once_with( + mock.ANY, self.proxy, error_message=mock.ANY, + # instance_id converted to server-side instance_uuid + fields='maintenance,uuid,instance_uuid') + + @mock.patch.object(port.Port, 'fetch', autospec=True) + def test__get_with_fields_port(self, mock_fetch): + result = self.proxy._get_with_fields( + port.Port, 'value', fields=['address', 'id', 'node_id']) + self.assertIs(result, mock_fetch.return_value) + mock_fetch.assert_called_once_with( + mock.ANY, self.proxy, error_message=mock.ANY, + # node_id converted to server-side node_uuid + fields='address,uuid,node_uuid') @mock.patch('time.sleep', lambda _sec: None) diff --git a/openstack/tests/unit/test_resource.py b/openstack/tests/unit/test_resource.py index 1f59cf7e8..fdfff8f23 100644 --- a/openstack/tests/unit/test_resource.py +++ b/openstack/tests/unit/test_resource.py @@ -377,21 +377,28 @@ class TestQueryParameters(base.TestCase): sot._mapping) def test_transpose_unmapped(self): + def _type(value, rtype): + self.assertIs(rtype, mock.sentinel.resource_type) + return value * 10 + location = "location" mapping = {"first_name": "first-name", "pet_name": {"name": "pet"}, - "answer": {"name": "answer", "type": int}} + "answer": {"name": "answer", "type": int}, + "complex": {"type": _type}} sot = resource.QueryParameters(location, **mapping) result = sot._transpose({"location": "Brooklyn", "first_name": "Brian", "pet_name": "Meow", "answer": "42", - "last_name": "Curtin"}) + "last_name": "Curtin", + "complex": 1}, + mock.sentinel.resource_type) # last_name isn't mapped and shouldn't be included self.assertEqual({"location": "Brooklyn", "first-name": "Brian", - "pet": "Meow", "answer": 42}, + "pet": "Meow", "answer": 42, "complex": 10}, result) def test_transpose_not_in_query(self): @@ -401,7 +408,8 @@ class TestQueryParameters(base.TestCase): "answer": {"name": "answer", "type": int}} sot = resource.QueryParameters(location, **mapping) - result = sot._transpose({"location": "Brooklyn"}) + result = sot._transpose({"location": "Brooklyn"}, + mock.sentinel.resource_type) # first_name not being in the query shouldn't affect results self.assertEqual({"location": "Brooklyn"}, diff --git a/releasenotes/notes/baremetal-fields-convert-857b8804327f1e86.yaml b/releasenotes/notes/baremetal-fields-convert-857b8804327f1e86.yaml new file mode 100644 index 000000000..07fa11f84 --- /dev/null +++ b/releasenotes/notes/baremetal-fields-convert-857b8804327f1e86.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes conversion of the bare metal ``fields`` argument from SDK to + server-side field names (e.g. ``instance_id`` to ``instance_uuid``).