diff --git a/shade/_tasks.py b/shade/_tasks.py index 90d4403fd..c7350977b 100644 --- a/shade/_tasks.py +++ b/shade/_tasks.py @@ -27,26 +27,6 @@ class MachinePortGetByAddress(task_manager.Task): return client.ironic_client.port.get_by_address(**self.args) -class MachinePortCreate(task_manager.Task): - def main(self, client): - return client.ironic_client.port.create(**self.args) - - -class MachinePortDelete(task_manager.Task): - def main(self, client): - return client.ironic_client.port.delete(**self.args) - - -class MachineNodeList(task_manager.Task): - def main(self, client): - return client.ironic_client.node.list(**self.args) - - -class MachineNodePortList(task_manager.Task): - def main(self, client): - return client.ironic_client.node.list_ports(**self.args) - - class MachineNodeUpdate(task_manager.Task): def main(self, client): return client.ironic_client.node.update(**self.args) @@ -55,13 +35,3 @@ class MachineNodeUpdate(task_manager.Task): class MachineNodeValidate(task_manager.Task): def main(self, client): return client.ironic_client.node.validate(**self.args) - - -class MachineSetMaintenance(task_manager.Task): - def main(self, client): - return client.ironic_client.node.set_maintenance(**self.args) - - -class MachineSetPower(task_manager.Task): - def main(self, client): - return client.ironic_client.node.set_power_state(**self.args) diff --git a/shade/operatorcloud.py b/shade/operatorcloud.py index 778e97775..3d5e52d09 100644 --- a/shade/operatorcloud.py +++ b/shade/operatorcloud.py @@ -39,14 +39,22 @@ class OperatorCloud(openstackcloud.OpenStackCloud): error_message=msg) def list_nics_for_machine(self, uuid): - with _utils.shade_exceptions( - "Error fetching port list for node {node_id}".format( - node_id=uuid)): - return self._normalize_machines( - self.manager.submit_task( - _tasks.MachineNodePortList(node_id=uuid))) + """Returns a list of ports present on the machine node. + + :param uuid: String representing machine UUID value in + order to identify the machine. + :returns: A dictionary containing containing a list of ports, + associated with the label "ports". + """ + msg = "Error fetching port list for node {node_id}".format( + node_id=uuid) + url = "/nodes/{node_id}/ports".format(node_id=uuid) + return self._baremetal_client.get(url, + microversion="1.6", + error_message=msg) def get_nic_by_mac(self, mac): + # TODO(TheJulia): Query /ports?address=mac when converting try: return self.manager.submit_task( _tasks.MachineNodePortGet(port_id=mac)) @@ -54,8 +62,11 @@ class OperatorCloud(openstackcloud.OpenStackCloud): return None def list_machines(self): - return self._normalize_machines( - self.manager.submit_task(_tasks.MachineNodeList())) + 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) def get_machine(self, name_or_id): """Get Machine by name or uuid @@ -92,10 +103,12 @@ class OperatorCloud(openstackcloud.OpenStackCloud): if the node is not found. """ try: - port = self.manager.submit_task( - _tasks.MachinePortGetByAddress(address=mac)) - return self.get_machine(port.node_uuid) - except ironic_exceptions.ClientException: + port_url = '/ports/detail?address={mac}'.format(mac=mac) + port = self._baremetal_client.get(port_url, microversion=1.6) + machine_url = '/nodes/{machine}'.format( + machine=port['ports'][0]['node_uuid']) + return self._baremetal_client.get(machine_url, microversion=1.6) + except Exception: return None def inspect_machine(self, name_or_id, wait=False, timeout=3600): @@ -209,7 +222,8 @@ class OperatorCloud(openstackcloud.OpenStackCloud): baremetal node. """ - msg = ("Baremetal machine node failed failed to be created.") + msg = ("Baremetal machine node failed to be created.") + port_msg = ("Baremetal machine port failed to be created.") url = '/nodes' # TODO(TheJulia): At some point we need to figure out how to @@ -223,9 +237,11 @@ class OperatorCloud(openstackcloud.OpenStackCloud): created_nics = [] try: for row in nics: - nic = self.manager.submit_task( - _tasks.MachinePortCreate(address=row['mac'], - node_uuid=machine['uuid'])) + payload = {'address': row['mac'], + 'node_uuid': machine['uuid']} + nic = self._baremetal_client.post('/ports', + json=payload, + error_message=port_msg) created_nics.append(nic['uuid']) except Exception as e: @@ -234,9 +250,13 @@ class OperatorCloud(openstackcloud.OpenStackCloud): try: for uuid in created_nics: try: - self.manager.submit_task( - _tasks.MachinePortDelete( - port_id=uuid)) + port_url = '/ports/{uuid}'.format(uuid=uuid) + # NOTE(TheJulia): Added in hope that it is logged. + port_msg = ('Failed to delete port {port} for node' + '{node}').format(port=uuid, + node=machine['uuid']) + self._baremetal_client.delete(port_url, + error_message=port_msg) except Exception: pass finally: @@ -347,14 +367,27 @@ class OperatorCloud(openstackcloud.OpenStackCloud): "Error unregistering node '%s' due to current provision " "state '%s'" % (uuid, machine['provision_state'])) + # NOTE(TheJulia) There is a high possibility of a lock being present + # if the machine was just moved through the state machine. This was + # 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) + except OpenStackCloudException as e: + raise OpenStackCloudException("Error unregistering node '%s': " + "Exception occured while waiting " + "to be able to proceed: %s" + % (machine['uuid'], e)) + for nic in nics: - with _utils.shade_exceptions( - "Error removing NIC {nic} from baremetal API for node " - "{uuid}".format(nic=nic, uuid=uuid)): - port = self.manager.submit_task( - _tasks.MachinePortGetByAddress(address=nic['mac'])) - self.manager.submit_task( - _tasks.MachinePortDelete(port_id=port.uuid)) + 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']) + self._baremetal_client.delete(port_url, error_message=port_msg) + with _utils.shade_exceptions( "Error unregistering machine {node_id} from the baremetal " "API".format(node_id=uuid)): @@ -639,25 +672,17 @@ class OperatorCloud(openstackcloud.OpenStackCloud): :returns: None """ - with _utils.shade_exceptions( - "Error setting machine maintenance state to {state} on node " - "{node}".format(state=state, node=name_or_id) - ): - if state: - result = self.manager.submit_task( - _tasks.MachineSetMaintenance(node_id=name_or_id, - state='true', - maint_reason=reason)) - else: - result = self.manager.submit_task( - _tasks.MachineSetMaintenance(node_id=name_or_id, - state='false')) - if result is not None: - raise OpenStackCloudException( - "Failed setting machine maintenance state to %s " - "on node %s. Received: %s" % ( - state, name_or_id, result)) - return None + msg = ("Error setting machine maintenance state to {state} on node " + "{node}").format(state=state, node=name_or_id) + url = '/nodes/{name_or_id}/maintenance'.format(name_or_id=name_or_id) + if state: + payload = {'reason': reason} + self._baremetal_client.put(url, + json=payload, + error_message=msg) + else: + self._baremetal_client.delete(url, error_message=msg) + return None def remove_machine_from_maintenance(self, name_or_id): """Remove Baremetal Machine from Maintenance State @@ -694,18 +719,19 @@ class OperatorCloud(openstackcloud.OpenStackCloud): :returns: None """ - with _utils.shade_exceptions( - "Error setting machine power state to {state} on node " - "{node}".format(state=state, node=name_or_id) - ): - power = self.manager.submit_task( - _tasks.MachineSetPower(node_id=name_or_id, - state=state)) - if power is not None: - raise OpenStackCloudException( - "Failed setting machine power state %s on node %s. " - "Received: %s" % (state, name_or_id, power)) - return None + msg = ("Error setting machine power state to {state} on node " + "{node}").format(state=state, node=name_or_id) + url = '/nodes/{name_or_id}/states/power'.format(name_or_id=name_or_id) + if 'reboot' in state: + desired_state = 'rebooting' + else: + desired_state = 'power {state}'.format(state=state) + payload = {'target': desired_state} + self._baremetal_client.put(url, + json=payload, + error_message=msg, + microversion="1.6") + return None def set_machine_power_on(self, name_or_id): """Activate baremetal machine power diff --git a/shade/task_manager.py b/shade/task_manager.py index 885df8be5..c618264d9 100644 --- a/shade/task_manager.py +++ b/shade/task_manager.py @@ -22,7 +22,6 @@ import time import types import keystoneauth1.exceptions -import simplejson import six from shade import _log @@ -162,7 +161,7 @@ class RequestTask(BaseTask): try: result_json = self._response.json() - except (simplejson.scanner.JSONDecodeError, ValueError) as e: + except Exception as e: result_json = self._response.text self._client.log.debug( 'Could not decode json in response: %(e)s', {'e': str(e)}) diff --git a/shade/tests/unit/test_baremetal_node.py b/shade/tests/unit/test_baremetal_node.py index 2ae3b1bd6..c6f759abc 100644 --- a/shade/tests/unit/test_baremetal_node.py +++ b/shade/tests/unit/test_baremetal_node.py @@ -1384,6 +1384,33 @@ class TestBaremetalNode(base.IronicTestCase): self.assert_calls() + def test_unregister_machine_locked_timeout(self): + mac_address = self.fake_baremetal_port['address'] + nics = [{'mac': mac_address}] + self.fake_baremetal_node['provision_state'] = 'available' + self.fake_baremetal_node['reservation'] = 'conductor99' + 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='nodes', + append=[self.fake_baremetal_node['uuid']]), + json=self.fake_baremetal_node), + ]) + self.assertRaises( + exc.OpenStackCloudException, + self.op_cloud.unregister_machine, + nics, + self.fake_baremetal_node['uuid'], + timeout=0.001) + self.assert_calls() + def test_unregister_machine_unavailable(self): # This is a list of invalid states that the method # should fail on. diff --git a/shade/tests/unit/test_baremetal_ports.py b/shade/tests/unit/test_baremetal_ports.py index c5b972bf6..3c7e914f8 100644 --- a/shade/tests/unit/test_baremetal_ports.py +++ b/shade/tests/unit/test_baremetal_ports.py @@ -64,8 +64,6 @@ class TestBaremetalPort(base.IronicTestCase): self.assert_calls() def test_list_nics_for_machine(self): - port_list = [self.fake_baremetal_port, - self.fake_baremetal_port2] self.register_uris([ dict(method='GET', uri=self.get_mock_url( @@ -77,8 +75,8 @@ class TestBaremetalPort(base.IronicTestCase): return_value = self.op_cloud.list_nics_for_machine( self.fake_baremetal_node['uuid']) - expected_value = port_list - self.assertEqual(expected_value, return_value) + self.assertEqual(2, len(return_value['ports'])) + self.assertEqual(self.fake_baremetal_port, return_value['ports'][0]) self.assert_calls() def test_list_nics_for_machine_failure(self): diff --git a/shade/tests/unit/test_operator_noauth.py b/shade/tests/unit/test_operator_noauth.py index e489d2e84..cc7d2f94e 100644 --- a/shade/tests/unit/test_operator_noauth.py +++ b/shade/tests/unit/test_operator_noauth.py @@ -12,8 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from keystoneauth1 import plugin - import shade from shade.tests.unit import base @@ -34,9 +32,6 @@ class TestShadeOperatorNoAuth(base.RequestsMockTestCase): # By clearing the URI registry, we remove all calls to a keystone # catalog or getting a token self._uri_registry.clear() - # TODO(mordred) Remove this if with next KSA release - if hasattr(plugin.BaseAuthPlugin, 'get_endpoint_data'): - self.use_ironic() self.register_uris([ dict(method='GET', uri=self.get_mock_url( @@ -50,9 +45,14 @@ class TestShadeOperatorNoAuth(base.RequestsMockTestCase): The new way of doing this is with the keystoneauth none plugin. """ + # NOTE(TheJulia): When we are using the python-ironicclient + # library, the library will automatically prepend the URI path + # with 'v1'. As such, since we are overriding the endpoint, + # we must explicitly do the same as we move away from the + # client library. self.cloud_noauth = shade.operator_cloud( auth_type='none', - baremetal_endpoint_override="https://bare-metal.example.com") + baremetal_endpoint_override="https://bare-metal.example.com/v1") self.cloud_noauth.list_machines() @@ -66,7 +66,7 @@ class TestShadeOperatorNoAuth(base.RequestsMockTestCase): self.cloud_noauth = shade.operator_cloud( auth_type='admin_token', auth=dict( - endpoint='https://bare-metal.example.com', + endpoint='https://bare-metal.example.com/v1', token='ignored')) self.cloud_noauth.list_machines()