Use neutronclient's port binding APIs

Take advantage of the neutronclient bindings for the port binding APIs
added in neutronclient 7.1.0 to avoid having to vendor this stuff
ourselves.

Change-Id: Icc284203fb53658abe304f24a62705217f90b22b
Signed-off-by: Stephen Finucane <[email protected]>

Author: stephenfin

lower-constraints.txt

@@ -122,7 +122,7 @@ python-glanceclient==2.8.0
 python-ironicclient==3.0.0
 python-keystoneclient==3.15.0
 python-mimeparse==1.6.0
-python-neutronclient==6.7.0
+python-neutronclient==7.1.0
 python-subunit==1.4.0
 pytz==2018.3
 PyYAML==5.1

nova/exception.py

@@ -816,13 +816,13 @@ class PortBindingFailed(Invalid):
 
 
 class PortBindingDeletionFailed(NovaException):
-    msg_fmt = _("Failed to delete binding for port %(port_id)s and host "
-                "%(host)s.")
+    msg_fmt = _("Failed to delete binding for port(s) %(port_id)s on host "
+                "%(host)s; please check neutron logs for more information")
 
 
 class PortBindingActivationFailed(NovaException):
-    msg_fmt = _("Failed to activate binding for port %(port_id)s and host "
-                "%(host)s.")
+    msg_fmt = _("Failed to activate binding for port %(port_id)s on host "
+                "%(host)s; please check neutron logs for more information")
 
 
 class PortUpdateFailed(Invalid):

nova/network/neutron.py

@@ -15,18 +15,14 @@
 import random
 
 import fixtures
-from keystoneauth1 import adapter as ksa_adap
-import mock
 from neutronclient.common import exceptions as neutron_client_exc
 import os_resource_classes as orc
-from oslo_serialization import jsonutils
 from oslo_utils import uuidutils
 
 from nova import exception
 from nova.network import constants as neutron_constants
 from nova.network import model as network_model
 from nova.tests.fixtures import nova as nova_fixtures
-from nova.tests.unit import fake_requests
 
 
 class _FakeNeutronClient:
@@ -665,25 +661,6 @@ def setUp(self):
             lambda *args, **kwargs: network_model.NetworkInfo.hydrate(
                 self.nw_info))
 
-        # Stub out port binding APIs which go through a KSA client Adapter
-        # rather than python-neutronclient.
-        self.test.stub_out(
-            'nova.network.neutron._get_ksa_client',
-            lambda *args, **kwargs: mock.Mock(
-                spec=ksa_adap.Adapter))
-        self.test.stub_out(
-            'nova.network.neutron.API._create_port_binding',
-            self.create_port_binding)
-        self.test.stub_out(
-            'nova.network.neutron.API._delete_port_binding',
-            self.delete_port_binding)
-        self.test.stub_out(
-            'nova.network.neutron.API._activate_port_binding',
-            self.activate_port_binding)
-        self.test.stub_out(
-            'nova.network.neutron.API._get_port_binding',
-            self.get_port_binding)
-
         self.test.stub_out(
             'nova.network.neutron.get_client', self._get_client)
 
@@ -692,13 +669,12 @@ def _get_client(self, context, admin=False):
         admin = admin or context.is_admin and not context.auth_token
         return _FakeNeutronClient(self, admin)
 
-    def create_port_binding(self, context, client, port_id, data):
+    def create_port_binding(self, port_id, body):
         if port_id not in self._ports:
-            return fake_requests.FakeResponse(
-                404, content='Port %s not found' % port_id)
+            raise neutron_client_exc.NeutronClientException(status_code=404)
 
         port = self._ports[port_id]
-        binding = copy.deepcopy(data['binding'])
+        binding = copy.deepcopy(body['binding'])
 
         # NOTE(stephenfin): We don't allow changing of backend
         binding['vif_type'] = port['binding:vif_type']
@@ -713,61 +689,36 @@ def create_port_binding(self, context, client, port_id, data):
 
         self._port_bindings[port_id][binding['host']] = binding
 
-        return fake_requests.FakeResponse(
-            200, content=jsonutils.dumps({'binding': binding}),
-        )
+        return {'binding': binding}
 
-    def _get_failure_response_if_port_or_binding_not_exists(
-        self, port_id, host,
-    ):
+    def _validate_port_binding(self, port_id, host_id):
         if port_id not in self._ports:
-            return fake_requests.FakeResponse(
-                404, content='Port %s not found' % port_id)
-        if host not in self._port_bindings[port_id]:
-            return fake_requests.FakeResponse(
-                404,
-                content='Binding for host %s for port %s not found'
-                        % (host, port_id))
-
-    def delete_port_binding(self, context, client, port_id, host):
-        failure = self._get_failure_response_if_port_or_binding_not_exists(
-            port_id, host)
-        if failure is not None:
-            return failure
+            raise neutron_client_exc.NeutronClientException(status_code=404)
 
-        del self._port_bindings[port_id][host]
+        if host_id not in self._port_bindings[port_id]:
+            raise neutron_client_exc.NeutronClientException(status_code=404)
 
-        return fake_requests.FakeResponse(204)
+    def delete_port_binding(self, port_id, host_id):
+        self._validate_port_binding(port_id, host_id)
+        del self._port_bindings[port_id][host_id]
 
-    def _activate_port_binding(self, port_id, host):
+    def _activate_port_binding(self, port_id, host_id):
         # It makes sure that only one binding is active for a port
-        for h, binding in self._port_bindings[port_id].items():
-            if h == host:
+        for host, binding in self._port_bindings[port_id].items():
+            if host == host_id:
                 # NOTE(gibi): neutron returns 409 if this binding is already
                 # active but nova does not depend on this behaviour yet.
                 binding['status'] = 'ACTIVE'
             else:
                 binding['status'] = 'INACTIVE'
 
-    def activate_port_binding(self, context, client, port_id, host):
-        failure = self._get_failure_response_if_port_or_binding_not_exists(
-            port_id, host)
-        if failure is not None:
-            return failure
-
-        self._activate_port_binding(port_id, host)
-
-        return fake_requests.FakeResponse(200)
-
-    def get_port_binding(self, context, client, port_id, host):
-        failure = self._get_failure_response_if_port_or_binding_not_exists(
-            port_id, host)
-        if failure is not None:
-            return failure
+    def activate_port_binding(self, port_id, host_id):
+        self._validate_port_binding(port_id, host_id)
+        self._activate_port_binding(port_id, host_id)
 
-        binding = {"binding": self._port_bindings[port_id][host]}
-        return fake_requests.FakeResponse(
-            200, content=jsonutils.dumps(binding))
+    def show_port_binding(self, port_id, host_id):
+        self._validate_port_binding(port_id, host_id)
+        return {'binding': self._port_bindings[port_id][host_id]}
 
     def _list_resource(self, resources, retrieve_all, **_params):
         # If 'fields' is passed we need to strip that out since it will mess

nova/tests/fixtures/neutron.py

@@ -12,6 +12,7 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
+
 import collections
 import copy
 import datetime
@@ -8383,32 +8384,35 @@ def test_cross_cell_migrate_server_with_qos_ports(self):
         server = self._create_server_with_ports_and_check_allocation(
             non_qos_normal_port, qos_normal_port, qos_sriov_port)
 
-        orig_create_binding = neutronapi.API._create_port_binding
+        orig_create_binding = self.neutron.create_port_binding
 
         hosts = {
-            'host1': self.compute1_rp_uuid, 'host2': self.compute2_rp_uuid}
+            'host1': self.compute1_rp_uuid, 'host2': self.compute2_rp_uuid,
+        }
 
         # Add an extra check to our neutron fixture. This check makes sure that
         # the RP sent in the binding corresponds to host of the binding. In a
         # real deployment this is checked by the Neutron server. As bug
         # 1907522 showed we fail this check for cross cell migration with qos
         # ports in a real deployment. So to reproduce that bug we need to have
         # the same check in our test env too.
-        def spy_on_create_binding(context, client, port_id, data):
+        def spy_on_create_binding(port_id, data):
             host_rp_uuid = hosts[data['binding']['host']]
             device_rp_uuid = data['binding']['profile'].get('allocation')
             if port_id == qos_normal_port['id']:
                 if device_rp_uuid != self.ovs_bridge_rp_per_host[host_rp_uuid]:
                     raise exception.PortBindingFailed(port_id=port_id)
             elif port_id == qos_sriov_port['id']:
-                if (device_rp_uuid not in
-                        self.sriov_dev_rp_per_host[host_rp_uuid].values()):
+                if (
+                    device_rp_uuid not in
+                    self.sriov_dev_rp_per_host[host_rp_uuid].values()
+                ):
                     raise exception.PortBindingFailed(port_id=port_id)
 
-            return orig_create_binding(context, client, port_id, data)
+            return orig_create_binding(port_id, data)
 
         with mock.patch(
-            'nova.network.neutron.API._create_port_binding',
+            'nova.tests.fixtures.NeutronFixture.create_port_binding',
             side_effect=spy_on_create_binding, autospec=True
         ):
             # We expect the migration to fail as the only available target
@@ -8440,8 +8444,8 @@ def spy_on_create_binding(context, client, port_id, data):
         self._create_networking_rp_tree('host3', self.compute3_rp_uuid)
 
         with mock.patch(
-                'nova.network.neutron.API._create_port_binding',
-                side_effect=spy_on_create_binding, autospec=True
+            'nova.tests.fixtures.NeutronFixture.create_port_binding',
+            side_effect=spy_on_create_binding, autospec=True
         ):
             server = self._migrate_server(server)
             self.assertEqual('host3', server['OS-EXT-SRV-ATTR:host'])