Remove python-ironicclient

After an epic battle with time, the python-ironicclient has been
driven from the land of shade, and there will be joy!

The patches doing this in shade have been squashed, as it was not
worth fixing transitive issues in the middle of the stack that
were only related to the merge.

Switch baremetal nics/ports tests over

Moved baremetal port tests to a separate file and
updated them to utilize the new testing method.

Additionally, normalized the output of the port lists as
noise is introduced that is not needed.

De-client-ify baremetal node_set_provision_state

de-client-ify baremetal get_machine

De-clientify baremetal create/delete

De-client-ify baremetal machine port list

De-client-ify machine patch operations

Remove version arg from updated ironic calls

Based upon discusison and feedback in change
I783fd47db368035d283b4984e4daacf9dc4ac4bd,
I am removing the ability for the caller to
specify the version, as it is not presently
needed.

Add helper to wait until baremetal locks clear

Add a halper to allow the methods to wait until locks
have cleared before proceeding.

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.

Fix and De-client-ify operator cloud get_nic_by_mac

Apparently, this method either never worked or was silently
broken at some point in the past, since we didn't have tests.

Added two tests and got the method back into working shape.

Additionally removed traces of ironic exceptions
being parsed in operatorcloud.py and removed legacy
tasks related to port lookups.

Also remove patch machine task wrapper as that appears
to have been migrated previously.

Change-Id: I8d6ca516250e0e10fe8b6edf235330b93535021b

extras/install-tips.sh

@@ -1,32 +0,0 @@
-#!/bin/bash
-# Copyright (c) 2017 Red Hat, Inc.
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#    http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
-# implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-for lib in \
-        python-keystoneclient \
-        python-ironicclient \
-        os-client-config \
-        keystoneauth
-do
-    egg=$(echo $lib | tr '-' '_' | sed 's/python-//')
-    if [ -d /opt/stack/new/$lib ] ; then
-        tip_location="git+file:///opt/stack/new/$lib#egg=$egg"
-        echo "$(which pip) install -U -e $tip_location"
-        pip uninstall -y $lib
-        pip install -U -e $tip_location
-    else
-        echo "$lib not found in /opt/stack/new/$lib"
-    fi
-done

openstack/cloud/operatorcloud.py

@@ -16,9 +16,7 @@ import jsonpatch
 import munch
 
 from openstack.cloud.exc import *  # noqa
-from openstack.cloud import meta
 from openstack.cloud import openstackcloud
-from openstack.cloud import _tasks
 from openstack.cloud import _utils
 
 
@@ -35,29 +33,42 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
     ironic_client = None
 
     def list_nics(self):
-        with _utils.shade_exceptions("Error fetching machine port list"):
+        msg = "Error fetching machine port list"
-            return meta.obj_list_to_munch(
+        return self._baremetal_client.get("/ports",
-                self.manager.submit_task(_tasks.MachinePortList(self)))
+                                          microversion="1.6",
+                                          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 meta.obj_list_to_munch(self.manager.submit_task(
+                     order to identify the machine.
-                _tasks.MachineNodePortList(self, node_id=uuid)))
+        :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):
-        """Get Machine by MAC address"""
+        try:
-        # TODO(shade) Finish porting ironic to REST/sdk
+            url = '/ports/detail?address=%s' % mac
-        # try:
+            data = self._baremetal_client.get(url)
-        #     return self.manager.submit_task(
+            if len(data['ports']) == 1:
-        #         _tasks.MachineNodePortGet(self, port_id=mac))
+                return data['ports'][0]
-        # except ironic_exceptions.ClientException:
+        except Exception:
-        #     return None
+            pass
+        return None
 
     def list_machines(self):
-        return self._normalize_machines(
+        msg = "Error fetching machine node list"
-            self.manager.submit_task(_tasks.MachineNodeList(self)))
+        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
@@ -70,13 +81,20 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
         :returns: ``munch.Munch`` representing the node found or None if no
                   nodes are found.
         """
-        # TODO(shade) Finish porting ironic to REST/sdk
+        # NOTE(TheJulia): This is the initial microversion shade support for
-        # try:
+        # ironic was created around. Ironic's default behavior for newer
-        #     return self._normalize_machine(
+        # versions is to expose the field, but with a value of None for
-        #         self.manager.submit_task(
+        # calls by a supported, yet older microversion.
-        #             _tasks.MachineNodeGet(node_id=name_or_id)))
+        # Consensus for moving forward with microversion handling in shade
-        # except ironic_exceptions.ClientException:
+        # seems to be to take the same approach, although ironic's API
-        #     return None
+        # 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:
+            return None
 
     def get_machine_by_mac(self, mac):
         """Get machine by port MAC address
@@ -86,13 +104,14 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
         :returns: ``munch.Munch`` representing the node found or None
                   if the node is not found.
         """
-        # try:
+        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.manager.submit_task(
+            machine_url = '/nodes/{machine}'.format(
-        #         _tasks.MachineNodeGet(node_id=port.node_uuid))
+                machine=port['ports'][0]['node_uuid'])
-        # except ironic_exceptions.ClientException:
+            return self._baremetal_client.get(machine_url, microversion=1.6)
-        #     return None
+        except Exception:
+            return None
 
     def inspect_machine(self, name_or_id, wait=False, timeout=3600):
         """Inspect a Barmetal machine
@@ -204,15 +223,27 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
         :returns: Returns a ``munch.Munch`` representing the new
                   baremetal node.
         """
-        with _utils.shade_exceptions("Error registering machine with Ironic"):
+
-            machine = self.manager.submit_task(_tasks.MachineCreate(**kwargs))
+        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
+        # handle data across when the requestor is defining newer items
+        # with the older api.
+        machine = self._baremetal_client.post(url,
+                                              json=kwargs,
+                                              error_message=msg,
+                                              microversion="1.6")
 
         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:
@@ -221,14 +252,23 @@ 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:
-                self.manager.submit_task(
+                version = "1.6"
-                    _tasks.MachineDelete(node_id=machine['uuid']))
+                msg = "Baremetal machine failed to be deleted."
+                url = '/nodes/{node_id}'.format(
+                    node_id=machine['uuid'])
+                self._baremetal_client.delete(url,
+                                              error_message=msg,
+                                              microversion=version)
             raise OpenStackCloudException(
                 "Error registering NICs with the baremetal service: %s"
                 % str(e))
@@ -329,19 +369,44 @@ 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)):
-            self.manager.submit_task(
+
-                _tasks.MachineDelete(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)
+            self._baremetal_client.delete(url,
+                                          error_message=msg,
+                                          microversion=version)
+
             if wait:
                 for count in _utils._iterate_timeout(
                         timeout,
@@ -353,9 +418,7 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
         """Patch Machine Information
 
         This method allows for an interface to manipulate node entries
-        within Ironic.  Specifically, it is a pass-through for the
+        within Ironic.
-        ironicclient.nodes.update interface which allows the Ironic Node
-        properties to be updated.
 
         :param node_id: The server object to attach to.
         :param patch:
@@ -386,15 +449,13 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
         :returns: ``munch.Munch`` representing the newly updated node.
         """
 
-        with _utils.shade_exceptions(
+        msg = ("Error updating machine via patch operation on node "
-            "Error updating machine via patch operation on node "
+               "{node}".format(node=name_or_id))
-            "{node}".format(node=name_or_id)
+        url = '/nodes/{node_id}'.format(node_id=name_or_id)
-        ):
+        return self._normalize_machine(
-            return self._normalize_machine(
+            self._baremetal_client.patch(url,
-                self.manager.submit_task(
+                                         json=patch,
-                    _tasks.MachinePatch(node_id=name_or_id,
+                                         error_message=msg))
-                                        patch=patch,
-                                        http_method='PATCH')))
 
     def update_machine(self, name_or_id, chassis_uuid=None, driver=None,
                        driver_info=None, name=None, instance_info=None,
@@ -506,14 +567,20 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
                 )
 
     def validate_node(self, uuid):
-        with _utils.shade_exceptions():
+        # TODO(TheJulia): There are soooooo many other interfaces
-            ifaces = self.manager.submit_task(
+        # that we can support validating, while these are essential,
-                _tasks.MachineNodeValidate(node_uuid=uuid))
+        # we should support more.
+        # TODO(TheJulia): Add a doc string :(
+        msg = ("Failed to query the API for validation status of "
+               "node {node_id}").format(node_id=uuid)
+        url = '/nodes/{node_id}/validate'.format(node_id=uuid)
+        ifaces = self._baremetal_client.get(url, error_message=msg)
 
-        if not ifaces.deploy or not ifaces.power:
+        if not ifaces['deploy'] or not ifaces['power']:
             raise OpenStackCloudException(
                 "ironic node %s failed to validate. "
-                "(deploy: %s, power: %s)" % (ifaces.deploy, ifaces.power))
+                "(deploy: %s, power: %s)" % (ifaces['deploy'],
+                                             ifaces['power']))
 
     def node_set_provision_state(self,
                                  name_or_id,
@@ -549,36 +616,44 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
         :returns: ``munch.Munch`` representing the current state of the machine
                   upon exit of the method.
         """
-        with _utils.shade_exceptions(
+        # NOTE(TheJulia): Default microversion for this call is 1.6.
-            "Baremetal machine node failed change provision state to "
+        # Setting locally until we have determined our master plan regarding
-            "{state}".format(state=state)
+        # microversion handling.
-        ):
+        version = "1.6"
-            machine = self.manager.submit_task(
+        msg = ("Baremetal machine node failed change provision state to "
-                _tasks.MachineSetProvision(node_uuid=name_or_id,
+               "{state}".format(state=state))
-                                           state=state,
-                                           configdrive=configdrive))
 
-            if wait:
+        url = '/nodes/{node_id}/states/provision'.format(
-                for count in _utils._iterate_timeout(
+            node_id=name_or_id)
-                        timeout,
+        payload = {'target': state}
-                        "Timeout waiting for node transition to "
+        if configdrive:
-                        "target state of '%s'" % state):
+            payload['configdrive'] = configdrive
-                    machine = self.get_machine(name_or_id)
+
-                    if 'failed' in machine['provision_state']:
+        machine = self._baremetal_client.put(url,
-                        raise OpenStackCloudException(
+                                             json=payload,
-                            "Machine encountered a failure.")
+                                             error_message=msg,
-                    # NOTE(TheJulia): This performs matching if the requested
+                                             microversion=version)
-                    # end state matches the state the node has reached.
+        if wait:
-                    if state in machine['provision_state']:
+            for count in _utils._iterate_timeout(
-                        break
+                    timeout,
-                    # NOTE(TheJulia): This performs matching for cases where
+                    "Timeout waiting for node transition to "
-                    # the reqeusted state action ends in available state.
+                    "target state of '%s'" % state):
-                    if ("available" in machine['provision_state'] and
-                            state in ["provide", "deleted"]):
-                        break
-            else:
                 machine = self.get_machine(name_or_id)
-            return machine
+                if 'failed' in machine['provision_state']:
+                    raise OpenStackCloudException(
+                        "Machine encountered a failure.")
+                # NOTE(TheJulia): This performs matching if the requested
+                # end state matches the state the node has reached.
+                if state in machine['provision_state']:
+                    break
+                # NOTE(TheJulia): This performs matching for cases where
+                # the reqeusted state action ends in available state.
+                if ("available" in machine['provision_state'] and
+                        state in ["provide", "deleted"]):
+                    break
+        else:
+            machine = self.get_machine(name_or_id)
+        return machine
 
     def set_machine_maintenance_state(
             self,
@@ -603,25 +678,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
@@ -658,18 +725,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
@@ -729,16 +797,69 @@ class OperatorCloud(openstackcloud.OpenStackCloud):
             uuid, 'deleted', wait=wait, timeout=timeout)
 
     def set_node_instance_info(self, uuid, patch):
-        with _utils.shade_exceptions():
+        msg = ("Error updating machine via patch operation on node "
-            return self.manager.submit_task(
+               "{node}".format(node=uuid))
-                _tasks.MachineNodeUpdate(node_id=uuid, patch=patch))
+        url = '/nodes/{node_id}'.format(node_id=uuid)
+        return self._baremetal_client.patch(url,
+                                            json=patch,
+                                            error_message=msg)
 
     def purge_node_instance_info(self, uuid):
         patch = []
         patch.append({'op': 'remove', 'path': '/instance_info'})
-        with _utils.shade_exceptions():
+        msg = ("Error updating machine via patch operation on node "
-            return self.manager.submit_task(
+               "{node}".format(node=uuid))
-                _tasks.MachineNodeUpdate(node_id=uuid, patch=patch))
+        url = '/nodes/{node_id}'.format(node_id=uuid)
+        return self._baremetal_client.patch(url,
+                                            json=patch,
+                                            error_message=msg)
+
+    def wait_for_baremetal_node_lock(self, node, timeout=30):
+        """Wait for a baremetal node to have no lock.
+
+        Baremetal nodes in ironic have a reservation lock that
+        is used to represent that a conductor has locked the node
+        while performing some sort of action, such as changing
+        configuration as a result of a machine state change.
+
+        This lock can occur during power syncronization, and prevents
+        updates to objects attached to the node, such as ports.
+
+        In the vast majority of cases, locks should clear in a few
+        seconds, and as such this method will only wait for 30 seconds.
+        The default wait is two seconds between checking if the lock
+        has cleared.
+
+        This method is intended for use by methods that need to
+        gracefully block without genreating errors, however this
+        method does prevent another client or a timer from
+        triggering a lock immediately after we see the lock as
+        having cleared.
+
+        :param node: The json representation of the node,
+                     specificially looking for the node
+                     'uuid' and 'reservation' fields.
+        :param timeout: Integer in seconds to wait for the
+                        lock to clear. Default: 30
+
+        :raises: OpenStackCloudException upon client failure.
+        :returns: None
+        """
+        # TODO(TheJulia): This _can_ still fail with a race
+        # condition in that between us checking the status,
+        # a conductor where the conductor could still obtain
+        # a lock before we are able to obtain a lock.
+        # This means we should handle this with such conections
+
+        if node['reservation'] is None:
+            return
+        else:
+            msg = 'Waiting for lock to be released for node {node}'.format(
+                node=node['uuid'])
+            for count in _utils._iterate_timeout(timeout, msg, 2):
+                current_node = self.get_machine(node['uuid'])
+                if current_node['reservation'] is None:
+                    return
 
     @_utils.valid_kwargs('type', 'service_type', 'description')
     def create_service(self, name, enabled=True, **kwargs):

openstack/tests/fakes.py

@@ -358,7 +358,8 @@ class FakeVolumeSnapshot(object):
 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):
+                 properties=None, reservation=None, last_error=None,
+                 provision_state=None):
         self.uuid = id
         self.name = name
         self.driver = driver
@@ -369,6 +370,7 @@ class FakeMachine(object):
         self.properties = properties
         self.reservation = reservation
         self.last_error = last_error
+        self.provision_state = provision_state
 
 
 class FakeMachinePort(object):

openstack/tests/unit/cloud/test_baremetal_node.py

@@ -672,6 +672,7 @@ class TestBaremetalNode(base.IronicTestCase):
 
     def test_node_set_provision_state_wait_timeout_fails(self):
         # Intentionally time out.
+        self.fake_baremetal_node['provision_state'] = 'deploy wait'
         self.register_uris([
             dict(
                 method='PUT',
@@ -780,6 +781,55 @@ class TestBaremetalNode(base.IronicTestCase):
         self.assertEqual(available_node, return_value)
         self.assert_calls()
 
+    def test_wait_for_baremetal_node_lock_locked(self):
+        self.fake_baremetal_node['reservation'] = 'conductor0'
+        unlocked_node = self.fake_baremetal_node.copy()
+        unlocked_node['reservation'] = None
+        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=unlocked_node),
+        ])
+        self.assertIsNone(
+            self.op_cloud.wait_for_baremetal_node_lock(
+                self.fake_baremetal_node,
+                timeout=1))
+
+        self.assert_calls()
+
+    def test_wait_for_baremetal_node_lock_not_locked(self):
+        self.fake_baremetal_node['reservation'] = None
+        self.assertIsNone(
+            self.op_cloud.wait_for_baremetal_node_lock(
+                self.fake_baremetal_node,
+                timeout=1))
+
+        self.assertEqual(0, len(self.adapter.request_history))
+
+    def test_wait_for_baremetal_node_lock_timeout(self):
+        self.fake_baremetal_node['reservation'] = 'conductor0'
+        self.register_uris([
+            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.wait_for_baremetal_node_lock,
+            self.fake_baremetal_node,
+            timeout=0.001)
+
+        self.assert_calls()
+
     def test_activate_node(self):
         self.fake_baremetal_node['provision_state'] = 'active'
         self.register_uris([
@@ -834,9 +884,7 @@ class TestBaremetalNode(base.IronicTestCase):
         node_uuid = self.fake_baremetal_node['uuid']
         # TODO(TheJulia): There is a lot of duplication
         # in testing creation. Surely this hsould be a helper
-        # or something. We should fix this, after we have
+        # or something. We should fix this.
-        # ironicclient removed, as in the mean time visibility
-        # will be helpful.
         node_to_post = {
             'chassis_uuid': None,
             'driver': None,
@@ -867,11 +915,10 @@ class TestBaremetalNode(base.IronicTestCase):
         self.assertDictEqual(self.fake_baremetal_node, return_value)
         self.assert_calls()
 
-    # TODO(TheJulia): After we remove ironicclient,
+    # TODO(TheJulia): We need to de-duplicate these tests.
-    # we need to de-duplicate these tests. Possibly
+    # Possibly a dedicated class, although we should do it
-    # a dedicated class, although we should do it then
+    # then as we may find differences that need to be
-    # as we may find differences that need to be accounted
+    # accounted for newer microversions.
-    # for newer microversions.
     def test_register_machine_enroll(self):
         mac_address = '00:01:02:03:04:05'
         nics = [{'mac': mac_address}]
@@ -1336,6 +1383,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.

openstack/tests/unit/cloud/test_baremetal_ports.py

@@ -0,0 +1,121 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+"""
+test_baremetal_ports
+----------------------------------
+
+Tests for baremetal port related operations
+"""
+
+from testscenarios import load_tests_apply_scenarios as load_tests  # noqa
+
+from openstack.cloud import exc
+from openstack.tests import fakes
+from openstack.tests.unit import base
+
+
+class TestBaremetalPort(base.IronicTestCase):
+
+    def setUp(self):
+        super(TestBaremetalPort, self).setUp()
+        self.fake_baremetal_node = fakes.make_fake_machine(
+            self.name, self.uuid)
+        # TODO(TheJulia): Some tests below have fake ports,
+        # since they are required in some processes. Lets refactor
+        # them at some point to use self.fake_baremetal_port.
+        self.fake_baremetal_port = fakes.make_fake_port(
+            '00:01:02:03:04:05',
+            node_id=self.uuid)
+        self.fake_baremetal_port2 = fakes.make_fake_port(
+            '0a:0b:0c:0d:0e:0f',
+            node_id=self.uuid)
+
+    def test_list_nics(self):
+        self.register_uris([
+            dict(method='GET',
+                 uri=self.get_mock_url(resource='ports'),
+                 json={'ports': [self.fake_baremetal_port,
+                                 self.fake_baremetal_port2]}),
+        ])
+
+        return_value = self.op_cloud.list_nics()
+        self.assertEqual(2, len(return_value['ports']))
+        self.assertEqual(self.fake_baremetal_port, return_value['ports'][0])
+        self.assert_calls()
+
+    def test_list_nics_failure(self):
+        self.register_uris([
+            dict(method='GET',
+                 uri=self.get_mock_url(resource='ports'),
+                 status_code=400)
+        ])
+        self.assertRaises(exc.OpenStackCloudException,
+                          self.op_cloud.list_nics)
+        self.assert_calls()
+
+    def test_list_nics_for_machine(self):
+        self.register_uris([
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     resource='nodes',
+                     append=[self.fake_baremetal_node['uuid'], 'ports']),
+                 json={'ports': [self.fake_baremetal_port,
+                                 self.fake_baremetal_port2]}),
+        ])
+
+        return_value = self.op_cloud.list_nics_for_machine(
+            self.fake_baremetal_node['uuid'])
+        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):
+        self.register_uris([
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     resource='nodes',
+                     append=[self.fake_baremetal_node['uuid'], 'ports']),
+                 status_code=400)
+        ])
+
+        self.assertRaises(exc.OpenStackCloudException,
+                          self.op_cloud.list_nics_for_machine,
+                          self.fake_baremetal_node['uuid'])
+        self.assert_calls()
+
+    def test_get_nic_by_mac(self):
+        mac = self.fake_baremetal_port['address']
+        query = 'detail?address=%s' % mac
+        self.register_uris([
+            dict(method='GET',
+                 uri=self.get_mock_url(resource='ports', append=[query]),
+                 json={'ports': [self.fake_baremetal_port]}),
+        ])
+
+        return_value = self.op_cloud.get_nic_by_mac(mac)
+
+        self.assertEqual(self.fake_baremetal_port, return_value)
+        self.assert_calls()
+
+    def test_get_nic_by_mac_failure(self):
+        mac = self.fake_baremetal_port['address']
+        query = 'detail?address=%s' % mac
+        self.register_uris([
+            dict(method='GET',
+                 uri=self.get_mock_url(resource='ports', append=[query]),
+                 json={'ports': []}),
+        ])
+
+        self.assertIsNone(self.op_cloud.get_nic_by_mac(mac))
+
+        self.assert_calls()

openstack/tests/unit/cloud/test_operator.py

@@ -15,7 +15,6 @@ import testtools
 
 import openstack
 from openstack.cloud import exc
-from openstack.cloud import meta
 from openstack.config import cloud_config
 from openstack.tests import fakes
 from openstack.tests.unit import base
@@ -26,37 +25,6 @@ class TestOperatorCloud(base.RequestsMockTestCase):
     def test_operator_cloud(self):
         self.assertIsInstance(self.op_cloud, openstack.OperatorCloud)
 
-    @mock.patch.object(openstack.OperatorCloud, 'ironic_client')
-    def test_list_nics(self, mock_client):
-        port1 = fakes.FakeMachinePort(1, "aa:bb:cc:dd", "node1")
-        port2 = fakes.FakeMachinePort(2, "dd:cc:bb:aa", "node2")
-        port_list = [port1, port2]
-        port_dict_list = meta.obj_list_to_munch(port_list)
-
-        mock_client.port.list.return_value = port_list
-        nics = self.op_cloud.list_nics()
-
-        self.assertTrue(mock_client.port.list.called)
-        self.assertEqual(port_dict_list, nics)
-
-    @mock.patch.object(openstack.OperatorCloud, 'ironic_client')
-    def test_list_nics_failure(self, mock_client):
-        mock_client.port.list.side_effect = Exception()
-        self.assertRaises(exc.OpenStackCloudException,
-                          self.op_cloud.list_nics)
-
-    @mock.patch.object(openstack.OperatorCloud, 'ironic_client')
-    def test_list_nics_for_machine(self, mock_client):
-        mock_client.node.list_ports.return_value = []
-        self.op_cloud.list_nics_for_machine("123")
-        mock_client.node.list_ports.assert_called_with(node_id="123")
-
-    @mock.patch.object(openstack.OperatorCloud, 'ironic_client')
-    def test_list_nics_for_machine_failure(self, mock_client):
-        mock_client.node.list_ports.side_effect = Exception()
-        self.assertRaises(exc.OpenStackCloudException,
-                          self.op_cloud.list_nics_for_machine, None)
-
     @mock.patch.object(cloud_config.CloudConfig, 'get_endpoint')
     def test_get_session_endpoint_provided(self, fake_get_endpoint):
         fake_get_endpoint.return_value = 'http://fake.url'

openstack/tests/unit/cloud/test_operator_noauth.py

@@ -12,11 +12,63 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# TODO(shade) Port this content back in from shade repo as tests don't have
+import openstack
-# references to ironic_client.
-
 from openstack.tests.unit import base
 
 
-class TestShadeOperatorNoAuth(base.RequestsMockTestCase):
+class TestOpenStackCloudOperatorNoAuth(base.RequestsMockTestCase):
-    pass
+    def setUp(self):
+        """Setup Noauth OperatorCloud tests
+
+        Setup the test to utilize no authentication and an endpoint
+        URL in the auth data.  This is permits testing of the basic
+        mechanism that enables Ironic noauth mode to be utilized with
+        Shade.
+
+        Uses base.RequestsMockTestCase instead of IronicTestCase because
+        we need to do completely different things with discovery.
+        """
+        super(TestOpenStackCloudOperatorNoAuth, self).setUp()
+        # By clearing the URI registry, we remove all calls to a keystone
+        # 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',
+                     resource='nodes'),
+                 json={'nodes': []}),
+        ])
+
+    def test_ironic_noauth_none_auth_type(self):
+        """Test noauth selection for Ironic in OperatorCloud
+
+        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 = openstack.operator_cloud(
+            auth_type='none',
+            baremetal_endpoint_override="https://bare-metal.example.com/v1")
+
+        self.cloud_noauth.list_machines()
+
+        self.assert_calls()
+
+    def test_ironic_noauth_admin_token_auth_type(self):
+        """Test noauth selection for Ironic in OperatorCloud
+
+        The old way of doing this was to abuse admin_token.
+        """
+        self.cloud_noauth = openstack.operator_cloud(
+            auth_type='admin_token',
+            auth=dict(
+                endpoint='https://bare-metal.example.com/v1',
+                token='ignored'))
+
+        self.cloud_noauth.list_machines()
+
+        self.assert_calls()