From b1ec4174e76d5876e17749a6dabb07cc4e919c32 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 27 May 2020 17:57:16 +0200 Subject: [PATCH] baremetal: use proxy methods in unregister_machine Has a nice side effect of no longer failing when a machine or any port do not exist. Also stop using a deprecated method. The wait argument is deprecated since it never had any effect: node deletion is synchronous. Change-Id: I63ea929540f22c2b73faf4a1f767e30ecc1dd5dd --- openstack/cloud/_baremetal.py | 51 +++++------------ .../tests/unit/cloud/test_baremetal_node.py | 55 +------------------ 2 files changed, 15 insertions(+), 91 deletions(-) diff --git a/openstack/cloud/_baremetal.py b/openstack/cloud/_baremetal.py index b3921c55a..8e8986cce 100644 --- a/openstack/cloud/_baremetal.py +++ b/openstack/cloud/_baremetal.py @@ -325,7 +325,7 @@ class BaremetalCloudMixin(_normalize.Normalizer): else: return machine - def unregister_machine(self, nics, uuid, wait=False, timeout=600): + def unregister_machine(self, nics, uuid, wait=None, timeout=600): """Unregister Baremetal from Ironic Removes entries for Network Interfaces and baremetal nodes @@ -335,15 +335,17 @@ class BaremetalCloudMixin(_normalize.Normalizer): to be removed. :param string uuid: The UUID of the node to be deleted. - :param wait: Boolean value, defaults to false, if to block the method - upon the final step of unregistering the machine. + :param wait: DEPRECATED, do not use. :param timeout: Integer value, representing seconds with a default value of 600, which controls the maximum amount of - time to block the method's completion on. + time to block until a lock is released on machine. :raises: OpenStackCloudException on operation failure. """ + if wait is not None: + warnings.warn("wait argument is deprecated and has no effect", + DeprecationWarning) machine = self.get_machine(uuid) invalid_states = ['active', 'cleaning', 'clean wait', 'clean failed'] @@ -357,47 +359,20 @@ class BaremetalCloudMixin(_normalize.Normalizer): # previously concealed by exception retry logic that detected the # failure, and resubitted the request in python-ironicclient. try: - self.wait_for_baremetal_node_lock(machine, timeout=timeout) + self.baremetal.wait_for_node_reservation(machine, timeout) except exc.OpenStackCloudException as e: raise exc.OpenStackCloudException( "Error unregistering node '%s': Exception occured while" " waiting to be able to proceed: %s" % (machine['uuid'], e)) for nic in nics: - port_msg = ("Error removing NIC {nic} from baremetal API for " - "node {uuid}").format(nic=nic, uuid=uuid) - port_url = '/ports/detail?address={mac}'.format(mac=nic['mac']) - port = self._baremetal_client.get(port_url, microversion=1.6, - error_message=port_msg) - port_url = '/ports/{uuid}'.format(uuid=port['ports'][0]['uuid']) - _utils._call_client_and_retry(self._baremetal_client.delete, - port_url, retry_on=[409, 503], - error_message=port_msg) + try: + port = next(self.baremetal.ports(address=nic['mac'])) + except StopIteration: + continue + self.baremetal.delete_port(port.id) - with _utils.shade_exceptions( - "Error unregistering machine {node_id} from the baremetal " - "API".format(node_id=uuid)): - - # NOTE(TheJulia): While this should not matter microversion wise, - # ironic assumes all calls without an explicit microversion to be - # version 1.0. Ironic expects to deprecate support for older - # microversions in future releases, as such, we explicitly set - # the version to what we have been using with the client library.. - version = "1.6" - msg = "Baremetal machine failed to be deleted" - url = '/nodes/{node_id}'.format( - node_id=uuid) - _utils._call_client_and_retry(self._baremetal_client.delete, - url, retry_on=[409, 503], - error_message=msg, - microversion=version) - - if wait: - for count in utils.iterate_timeout( - timeout, - "Timeout waiting for machine to be deleted"): - if not self.get_machine(uuid): - break + self.baremetal.delete_node(uuid) def patch_machine(self, name_or_id, patch): """Patch Machine Information diff --git a/openstack/tests/unit/cloud/test_baremetal_node.py b/openstack/tests/unit/cloud/test_baremetal_node.py index 6c7905eed..d18c940bf 100644 --- a/openstack/tests/unit/cloud/test_baremetal_node.py +++ b/openstack/tests/unit/cloud/test_baremetal_node.py @@ -1482,7 +1482,6 @@ class TestBaremetalNode(base.IronicTestCase): port_uuid = self.fake_baremetal_port['uuid'] # NOTE(TheJulia): The two values below should be the same. port_node_uuid = self.fake_baremetal_port['node_uuid'] - port_url_address = 'detail?address=%s' % mac_address self.fake_baremetal_node['provision_state'] = 'available' self.register_uris([ dict( @@ -1495,7 +1494,7 @@ class TestBaremetalNode(base.IronicTestCase): method='GET', uri=self.get_mock_url( resource='ports', - append=[port_url_address]), + qs_elements=['address=%s' % mac_address]), json={'ports': [{'address': mac_address, 'node_uuid': port_node_uuid, 'uuid': port_uuid}]}), @@ -1516,55 +1515,6 @@ class TestBaremetalNode(base.IronicTestCase): self.assert_calls() - def test_unregister_machine_timeout(self): - mac_address = self.fake_baremetal_port['address'] - nics = [{'mac': mac_address}] - port_uuid = self.fake_baremetal_port['uuid'] - port_node_uuid = self.fake_baremetal_port['node_uuid'] - port_url_address = 'detail?address=%s' % mac_address - self.fake_baremetal_node['provision_state'] = 'available' - 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='GET', - uri=self.get_mock_url( - resource='ports', - append=[port_url_address]), - json={'ports': [{'address': mac_address, - 'node_uuid': port_node_uuid, - 'uuid': port_uuid}]}), - dict( - method='DELETE', - uri=self.get_mock_url( - resource='ports', - append=[self.fake_baremetal_port['uuid']])), - dict( - method='DELETE', - uri=self.get_mock_url( - resource='nodes', - append=[self.fake_baremetal_node['uuid']])), - dict( - method='GET', - uri=self.get_mock_url( - resource='nodes', - append=[self.fake_baremetal_node['uuid']]), - json=self.fake_baremetal_node), - ]) - self.assertRaises( - exc.OpenStackCloudException, - self.cloud.unregister_machine, - nics, - self.fake_baremetal_node['uuid'], - wait=True, - timeout=0.001) - - self.assert_calls() - def test_unregister_machine_locked_timeout(self): mac_address = self.fake_baremetal_port['address'] nics = [{'mac': mac_address}] @@ -1598,7 +1548,6 @@ class TestBaremetalNode(base.IronicTestCase): port_uuid = self.fake_baremetal_port['uuid'] # NOTE(TheJulia): The two values below should be the same. port_node_uuid = self.fake_baremetal_port['node_uuid'] - port_url_address = 'detail?address=%s' % mac_address self.fake_baremetal_node['provision_state'] = 'available' self.register_uris([ dict( @@ -1611,7 +1560,7 @@ class TestBaremetalNode(base.IronicTestCase): method='GET', uri=self.get_mock_url( resource='ports', - append=[port_url_address]), + qs_elements=['address=%s' % mac_address]), json={'ports': [{'address': mac_address, 'node_uuid': port_node_uuid, 'uuid': port_uuid}]}),