diff --git a/openstack/baremetal/v1/node.py b/openstack/baremetal/v1/node.py index fafdfd5d2..d60758f8c 100644 --- a/openstack/baremetal/v1/node.py +++ b/openstack/baremetal/v1/node.py @@ -124,6 +124,13 @@ class Node(resource.Resource): #: Timestamp at which the node was last updated. updated_at = resource.Body("updated_at") + def _consume_body_attrs(self, attrs): + if 'provision_state' in attrs and attrs['provision_state'] is None: + # API version 1.1 uses None instead of "available". Make it + # consistent. + attrs['provision_state'] = 'available' + return super(Node, self)._consume_body_attrs(attrs) + def create(self, session, *args, **kwargs): """Create a remote resource based on this instance. diff --git a/openstack/cloud/openstackcloud.py b/openstack/cloud/openstackcloud.py index 0144d71e0..77ffe775c 100755 --- a/openstack/cloud/openstackcloud.py +++ b/openstack/cloud/openstackcloud.py @@ -9309,11 +9309,12 @@ class OpenStackCloud(_normalize.Normalizer): return None def list_machines(self): - msg = "Error fetching machine node list" - data = self._baremetal_client.get("/nodes", - microversion="1.6", - error_message=msg) - return self._get_and_munchify('nodes', data) + """List Machines. + + :returns: list of ``munch.Munch`` representing machines. + """ + return [self._normalize_machine(node._to_munch()) + for node in self.baremetal.nodes()] def get_machine(self, name_or_id): """Get Machine by name or uuid @@ -9326,19 +9327,10 @@ class OpenStackCloud(_normalize.Normalizer): :returns: ``munch.Munch`` representing the node found or None if no nodes are found. """ - # NOTE(TheJulia): This is the initial microversion shade support for - # ironic was created around. Ironic's default behavior for newer - # versions is to expose the field, but with a value of None for - # calls by a supported, yet older microversion. - # Consensus for moving forward with microversion handling in shade - # seems to be to take the same approach, although ironic's API - # does it for the user. - version = "1.6" try: - url = '/nodes/{node_id}'.format(node_id=name_or_id) return self._normalize_machine( - self._baremetal_client.get(url, microversion=version)) - except Exception: + self.baremetal.get_node(name_or_id)._to_munch()) + except exc.OpenStackCloudResourceNotFound: return None def get_machine_by_mac(self, mac): @@ -9675,7 +9667,7 @@ class OpenStackCloud(_normalize.Normalizer): This method allows for an interface to manipulate node entries within Ironic. - :param node_id: The server object to attach to. + :param string name_or_id: A machine name or UUID to be updated. :param patch: The JSON Patch document is a list of dictonary objects that comply with RFC 6902 which can be found at @@ -9703,44 +9695,26 @@ class OpenStackCloud(_normalize.Normalizer): :returns: ``munch.Munch`` representing the newly updated node. """ - + node = self.baremetal.get_node(name_or_id) + microversion = node._get_microversion_for(self._baremetal_client, + 'commit') msg = ("Error updating machine via patch operation on node " "{node}".format(node=name_or_id)) - url = '/nodes/{node_id}'.format(node_id=name_or_id) + url = '/nodes/{node_id}'.format(node_id=node.id) return self._normalize_machine( self._baremetal_client.patch(url, json=patch, + microversion=microversion, error_message=msg)) - def update_machine(self, name_or_id, chassis_uuid=None, driver=None, - driver_info=None, name=None, instance_info=None, - instance_uuid=None, properties=None): + def update_machine(self, name_or_id, **attrs): """Update a machine with new configuration information A user-friendly method to perform updates of a machine, in whole or part. :param string name_or_id: A machine name or UUID to be updated. - :param string chassis_uuid: Assign a chassis UUID to the machine. - NOTE: As of the Kilo release, this value - cannot be changed once set. If a user - attempts to change this value, then the - Ironic API, as of Kilo, will reject the - request. - :param string driver: The driver name for controlling the machine. - :param dict driver_info: The dictonary defining the configuration - that the driver will utilize to control - the machine. Permutations of this are - dependent upon the specific driver utilized. - :param string name: A human relatable name to represent the machine. - :param dict instance_info: A dictonary of configuration information - that conveys to the driver how the host - is to be configured when deployed. - be deployed to the machine. - :param string instance_uuid: A UUID value representing the instance - that the deployed machine represents. - :param dict properties: A dictonary defining the properties of a - machine. + :param attrs: Attributes to updated on the machine. :raises: OpenStackCloudException on operation error. @@ -9754,72 +9728,28 @@ class OpenStackCloud(_normalize.Normalizer): raise exc.OpenStackCloudException( "Machine update failed to find Machine: %s. " % name_or_id) - machine_config = {} - new_config = {} + new_config = dict(machine, **attrs) try: - if chassis_uuid: - machine_config['chassis_uuid'] = machine['chassis_uuid'] - new_config['chassis_uuid'] = chassis_uuid - - if driver: - machine_config['driver'] = machine['driver'] - new_config['driver'] = driver - - if driver_info: - machine_config['driver_info'] = machine['driver_info'] - new_config['driver_info'] = driver_info - - if name: - machine_config['name'] = machine['name'] - new_config['name'] = name - - if instance_info: - machine_config['instance_info'] = machine['instance_info'] - new_config['instance_info'] = instance_info - - if instance_uuid: - machine_config['instance_uuid'] = machine['instance_uuid'] - new_config['instance_uuid'] = instance_uuid - - if properties: - machine_config['properties'] = machine['properties'] - new_config['properties'] = properties - except KeyError as e: - self.log.debug( - "Unexpected machine response missing key %s [%s]", - e.args[0], name_or_id) - raise exc.OpenStackCloudException( - "Machine update failed - machine [%s] missing key %s. " - "Potential API issue." - % (name_or_id, e.args[0])) - - try: - patch = jsonpatch.JsonPatch.from_diff(machine_config, new_config) + patch = jsonpatch.JsonPatch.from_diff(machine, new_config) except Exception as e: raise exc.OpenStackCloudException( "Machine update failed - Error generating JSON patch object " "for submission to the API. Machine: %s Error: %s" - % (name_or_id, str(e))) + % (name_or_id, e)) - with _utils.shade_exceptions( - "Machine update failed - patch operation failed on Machine " - "{node}".format(node=name_or_id) - ): - if not patch: - return dict( - node=machine, - changes=None - ) - else: - machine = self.patch_machine(machine['uuid'], list(patch)) - change_list = [] - for change in list(patch): - change_list.append(change['path']) - return dict( - node=machine, - changes=change_list - ) + if not patch: + return dict( + node=machine, + changes=None + ) + + change_list = [change['path'] for change in patch] + node = self.baremetal.update_node(machine, **attrs) + return dict( + node=self._normalize_machine(node._to_munch()), + changes=change_list + ) def attach_port_to_machine(self, name_or_id, port_name_or_id): """Attach a virtual port to the bare metal machine. diff --git a/openstack/tests/fakes.py b/openstack/tests/fakes.py index beb4dfeb0..d224811da 100644 --- a/openstack/tests/fakes.py +++ b/openstack/tests/fakes.py @@ -359,7 +359,7 @@ class FakeMachine(object): def __init__(self, id, name=None, driver=None, driver_info=None, chassis_uuid=None, instance_info=None, instance_uuid=None, properties=None, reservation=None, last_error=None, - provision_state=None): + provision_state='available'): self.uuid = id self.name = name self.driver = driver diff --git a/openstack/tests/unit/baremetal/v1/test_node.py b/openstack/tests/unit/baremetal/v1/test_node.py index e965590f9..063414779 100644 --- a/openstack/tests/unit/baremetal/v1/test_node.py +++ b/openstack/tests/unit/baremetal/v1/test_node.py @@ -148,6 +148,11 @@ class TestNode(base.TestCase): self.assertEqual(FAKE['target_raid_config'], sot.target_raid_config) self.assertEqual(FAKE['updated_at'], sot.updated_at) + def test_normalize_provision_state(self): + attrs = dict(FAKE, provision_state=None) + sot = node.Node(**attrs) + self.assertEqual('available', sot.provision_state) + class TestNodeDetail(base.TestCase): diff --git a/openstack/tests/unit/cloud/test_baremetal_node.py b/openstack/tests/unit/cloud/test_baremetal_node.py index da1ae81c1..982fe64ce 100644 --- a/openstack/tests/unit/cloud/test_baremetal_node.py +++ b/openstack/tests/unit/cloud/test_baremetal_node.py @@ -50,7 +50,8 @@ class TestBaremetalNode(base.IronicTestCase): machines = self.cloud.list_machines() self.assertEqual(2, len(machines)) - self.assertEqual(self.fake_baremetal_node, machines[0]) + self.assertSubdict(self.fake_baremetal_node, machines[0]) + self.assertSubdict(fake_baremetal_two, machines[1]) self.assert_calls() def test_get_machine(self): @@ -156,6 +157,11 @@ class TestBaremetalNode(base.IronicTestCase): 'path': '/instance_info'}] self.fake_baremetal_node['instance_info'] = {} self.register_uris([ + dict(method='GET', + uri=self.get_mock_url( + resource='nodes', + append=[self.fake_baremetal_node['uuid']]), + json=self.fake_baremetal_node), dict(method='PATCH', uri=self.get_mock_url( resource='nodes', @@ -1093,7 +1099,7 @@ class TestBaremetalNode(base.IronicTestCase): # this code is never used in the current code path. return_value = self.cloud.register_machine(nics, **node_to_post) - self.assertDictEqual(available_node, return_value) + self.assertSubdict(available_node, return_value) self.assert_calls() def test_register_machine_enroll_wait(self): @@ -1174,7 +1180,7 @@ class TestBaremetalNode(base.IronicTestCase): return_value = self.cloud.register_machine( nics, wait=True, **node_to_post) - self.assertDictEqual(available_node, return_value) + self.assertSubdict(available_node, return_value) self.assert_calls() def test_register_machine_enroll_failure(self): @@ -1605,7 +1611,7 @@ class TestBaremetalNode(base.IronicTestCase): update_dict = self.cloud.update_machine( self.fake_baremetal_node['uuid']) self.assertIsNone(update_dict['changes']) - self.assertDictEqual(self.fake_baremetal_node, update_dict['node']) + self.assertSubdict(self.fake_baremetal_node, update_dict['node']) self.assert_calls() @@ -1711,7 +1717,7 @@ class TestUpdateMachinePatch(base.IronicTestCase): self.fake_baremetal_node[self.field_name] = None value_to_send = self.fake_baremetal_node[self.field_name] if self.changed: - value_to_send = 'meow' + value_to_send = self.new_value uris = [dict( method='GET', uri=self.get_mock_url( @@ -1723,7 +1729,7 @@ class TestUpdateMachinePatch(base.IronicTestCase): test_patch = [{ 'op': 'replace', 'path': '/' + self.field_name, - 'value': 'meow'}] + 'value': value_to_send}] uris.append( dict( method='PATCH', @@ -1739,28 +1745,37 @@ class TestUpdateMachinePatch(base.IronicTestCase): update_dict = self.cloud.update_machine( self.fake_baremetal_node['uuid'], **call_args) - if not self.changed: + if self.changed: + self.assertEqual(['/' + self.field_name], update_dict['changes']) + else: self.assertIsNone(update_dict['changes']) - self.assertDictEqual(self.fake_baremetal_node, update_dict['node']) + self.assertSubdict(self.fake_baremetal_node, update_dict['node']) self.assert_calls() scenarios = [ ('chassis_uuid', dict(field_name='chassis_uuid', changed=False)), ('chassis_uuid_changed', - dict(field_name='chassis_uuid', changed=True)), + dict(field_name='chassis_uuid', changed=True, + new_value='meow')), ('driver', dict(field_name='driver', changed=False)), - ('driver_changed', dict(field_name='driver', changed=True)), + ('driver_changed', dict(field_name='driver', changed=True, + new_value='meow')), ('driver_info', dict(field_name='driver_info', changed=False)), - ('driver_info_changed', dict(field_name='driver_info', changed=True)), + ('driver_info_changed', dict(field_name='driver_info', changed=True, + new_value={'cat': 'meow'})), ('instance_info', dict(field_name='instance_info', changed=False)), ('instance_info_changed', - dict(field_name='instance_info', changed=True)), + dict(field_name='instance_info', changed=True, + new_value={'cat': 'meow'})), ('instance_uuid', dict(field_name='instance_uuid', changed=False)), ('instance_uuid_changed', - dict(field_name='instance_uuid', changed=True)), + dict(field_name='instance_uuid', changed=True, + new_value='meow')), ('name', dict(field_name='name', changed=False)), - ('name_changed', dict(field_name='name', changed=True)), + ('name_changed', dict(field_name='name', changed=True, + new_value='meow')), ('properties', dict(field_name='properties', changed=False)), - ('properties_changed', dict(field_name='properties', changed=True)) + ('properties_changed', dict(field_name='properties', changed=True, + new_value={'cat': 'meow'})) ] diff --git a/openstack/tests/unit/cloud/test_operator_noauth.py b/openstack/tests/unit/cloud/test_operator_noauth.py index 4462bbf71..f62c44488 100644 --- a/openstack/tests/unit/cloud/test_operator_noauth.py +++ b/openstack/tests/unit/cloud/test_operator_noauth.py @@ -33,6 +33,12 @@ class TestOpenStackCloudOperatorNoAuth(base.TestCase): # catalog or getting a token self._uri_registry.clear() self.register_uris([ + dict(method='GET', + uri=self.get_mock_url( + service_type='baremetal', base_url_append='v1'), + json={'id': 'v1', + 'links': [{"href": "https://bare-metal.example.com/v1/", + "rel": "self"}]}), dict(method='GET', uri=self.get_mock_url( service_type='baremetal', base_url_append='v1', diff --git a/releasenotes/notes/machine-get-update-microversions-4b910e63cebd65e2.yaml b/releasenotes/notes/machine-get-update-microversions-4b910e63cebd65e2.yaml new file mode 100644 index 000000000..7c0f47749 --- /dev/null +++ b/releasenotes/notes/machine-get-update-microversions-4b910e63cebd65e2.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + The ``get_machine``, ``update_machine`` and ``patch_machine`` calls now + support all Bare Metal API microversions supported by the SDK. Previously + they used 1.6 unconditionally. +upgrade: + - | + The baremetal API now returns ``available`` as provision state for nodes + available for deployment. Previously, ``None`` could be returned for API + version 1.1 (early Kilo) and older.