De-client-ify many baremetal calls

Update calls for numerous baremetal methods to utilize
the _baremetal_client calls instead of python-ironicclient.

Also corrected a minor baremetal port test and fixed the
base noauth test endpoint override as python-ironicclient
was previously injecting that for us.

Change-Id: I358a63a52067f7d48d71f0b23652491b2497fe43

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)

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(
+        """Returns a list of ports present on the machine node.
-                "Error fetching port list for node {node_id}".format(
+
-                node_id=uuid)):
+        :param uuid: String representing machine UUID value in
-            return self._normalize_machines(
+                     order to identify the machine.
-                self.manager.submit_task(
+        :returns: A dictionary containing containing a list of ports,
-                    _tasks.MachineNodePortList(node_id=uuid)))
+                  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(
+        msg = "Error fetching machine node list"
-            self.manager.submit_task(_tasks.MachineNodeList()))
+        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(
+            port_url = '/ports/detail?address={mac}'.format(mac=mac)
-                _tasks.MachinePortGetByAddress(address=mac))
+            port = self._baremetal_client.get(port_url, microversion=1.6)
-            return self.get_machine(port.node_uuid)
+            machine_url = '/nodes/{machine}'.format(
-        except ironic_exceptions.ClientException:
+                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(
+                payload = {'address': row['mac'],
-                    _tasks.MachinePortCreate(address=row['mac'],
+                           'node_uuid': machine['uuid']}
-                                             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(
+                        port_url = '/ports/{uuid}'.format(uuid=uuid)
-                            _tasks.MachinePortDelete(
+                        # NOTE(TheJulia): Added in hope that it is logged.
-                                port_id=uuid))
+                        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(
+            port_msg = ("Error removing NIC {nic} from baremetal API for "
-                    "Error removing NIC {nic} from baremetal API for node "
+                        "node {uuid}").format(nic=nic, uuid=uuid)
-                    "{uuid}".format(nic=nic, uuid=uuid)):
+            port_url = '/ports/detail?address={mac}'.format(mac=nic['mac'])
-                port = self.manager.submit_task(
+            port = self._baremetal_client.get(port_url, microversion=1.6,
-                    _tasks.MachinePortGetByAddress(address=nic['mac']))
+                                              error_message=port_msg)
-                self.manager.submit_task(
+            port_url = '/ports/{uuid}'.format(uuid=port['ports'][0]['uuid'])
-                    _tasks.MachinePortDelete(port_id=port.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(
+        msg = ("Error setting machine maintenance state to {state} on node "
-            "Error setting machine maintenance state to {state} on node "
+               "{node}").format(state=state, node=name_or_id)
-            "{node}".format(state=state, node=name_or_id)
+        url = '/nodes/{name_or_id}/maintenance'.format(name_or_id=name_or_id)
-        ):
+        if state:
-            if state:
+            payload = {'reason': reason}
-                result = self.manager.submit_task(
+            self._baremetal_client.put(url,
-                    _tasks.MachineSetMaintenance(node_id=name_or_id,
+                                       json=payload,
-                                                 state='true',
+                                       error_message=msg)
-                                                 maint_reason=reason))
+        else:
-            else:
+            self._baremetal_client.delete(url, error_message=msg)
-                result = self.manager.submit_task(
+        return None
-                    _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
 
     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(
+        msg = ("Error setting machine power state to {state} on node "
-            "Error setting machine power state to {state} on node "
+               "{node}").format(state=state, node=name_or_id)
-            "{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:
-            power = self.manager.submit_task(
+            desired_state = 'rebooting'
-                _tasks.MachineSetPower(node_id=name_or_id,
+        else:
-                                       state=state))
+            desired_state = 'power {state}'.format(state=state)
-            if power is not None:
+        payload = {'target': desired_state}
-                raise OpenStackCloudException(
+        self._baremetal_client.put(url,
-                    "Failed setting machine power state %s on node %s. "
+                                   json=payload,
-                    "Received: %s" % (state, name_or_id, power))
+                                   error_message=msg,
-            return None
+                                   microversion="1.6")
+        return None
 
     def set_machine_power_on(self, name_or_id):
         """Activate baremetal machine power

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)})

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.

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(2, len(return_value['ports']))
-        self.assertEqual(expected_value, return_value)
+        self.assertEqual(self.fake_baremetal_port, return_value['ports'][0])
         self.assert_calls()
 
     def test_list_nics_for_machine_failure(self):

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()