From e0080bff6259f409f029c27fa8932d6e8ed04907 Mon Sep 17 00:00:00 2001
From: Marc Koderer <marc@koderer.com>
Date: Wed, 13 Jul 2016 09:14:35 +0200
Subject: [PATCH] Make port_binding_extension mandatory if host_id is specified

In case create_port is called with host_id it's better to fail if
port_binding_extension is not available. Otherwise there won't
be any binding and no logging that the port will be inactive.

Change-Id: I65595308bc71ae78c992fd97e3f26fd718b249ec
Closes-bug: #1602525
---
 manila/network/neutron/api.py                 |  7 ++++-
 .../tests/network/neutron/test_neutron_api.py | 27 +++++++++----------
 ...rt_binding_mandatory-2aaba0fa72b82676.yaml |  4 +++
 3 files changed, 22 insertions(+), 16 deletions(-)
 create mode 100644 releasenotes/notes/bug-1602525-port_binding_mandatory-2aaba0fa72b82676.yaml

diff --git a/manila/network/neutron/api.py b/manila/network/neutron/api.py
index af15fb7566..b3d49547af 100644
--- a/manila/network/neutron/api.py
+++ b/manila/network/neutron/api.py
@@ -175,7 +175,12 @@ class API(object):
                 port_req_body['port']['security_groups'] = security_group_ids
             if mac_address:
                 port_req_body['port']['mac_address'] = mac_address
-            if self._has_port_binding_extension() and host_id:
+            if host_id:
+                if not self._has_port_binding_extension():
+                    msg = ("host_id (%(host_id)s) specified but neutron "
+                           "doesn't support port binding. Please activate the "
+                           "extension accordingly." % {"host_id": host_id})
+                    raise exception.NetworkException(message=msg)
                 port_req_body['port']['binding:host_id'] = host_id
             if dhcp_opts is not None:
                 port_req_body['port']['extra_dhcp_opts'] = dhcp_opts
diff --git a/manila/tests/network/neutron/test_neutron_api.py b/manila/tests/network/neutron/test_neutron_api.py
index ef3ace5428..0214280265 100644
--- a/manila/tests/network/neutron/test_neutron_api.py
+++ b/manila/tests/network/neutron/test_neutron_api.py
@@ -150,8 +150,6 @@ class NeutronApiTest(test.TestCase):
 
     def test_create_port_with_required_args(self):
         # Set up test data
-        self.mock_object(self.neutron_api, '_has_port_binding_extension',
-                         mock.Mock(return_value=True))
         port_args = {'tenant_id': 'test tenant', 'network_id': 'test net'}
 
         # Execute method 'create_port'
@@ -161,13 +159,10 @@ class NeutronApiTest(test.TestCase):
         self.assertEqual(port_args['tenant_id'], port['tenant_id'])
         self.assertEqual(port_args['network_id'],
                          port['network_id'])
-        self.neutron_api._has_port_binding_extension.assert_called_once_with()
         self.assertTrue(clientv20.Client.called)
 
     def test_create_port_with_additional_kwargs(self):
         # Set up test data
-        self.mock_object(self.neutron_api, '_has_port_binding_extension',
-                         mock.Mock(return_value=True))
         port_args = {'tenant_id': 'test tenant', 'network_id': 'test net',
                      'binding_arg': 'foo'}
 
@@ -180,15 +175,22 @@ class NeutronApiTest(test.TestCase):
                          port['network_id'])
         self.assertEqual(port_args['binding_arg'],
                          port['binding_arg'])
-        self.neutron_api._has_port_binding_extension.assert_called_once_with()
         self.assertTrue(clientv20.Client.called)
 
+    def test_create_port_with_host_id_no_binding_ext(self):
+        self.mock_object(self.neutron_api, '_has_port_binding_extension',
+                         mock.Mock(return_value=False))
+        port_args = {
+            'tenant_id': 'test tenant',
+            'network_id': 'test net',
+            'host_id': 'foohost'
+        }
+
+        self.assertRaises(exception.NetworkException,
+                          self.neutron_api.create_port, **port_args)
+
     @mock.patch.object(neutron_api.LOG, 'exception', mock.Mock())
     def test_create_port_exception(self):
-        # Set up test data
-        self.mock_object(
-            self.neutron_api, '_has_port_binding_extension',
-            mock.Mock(return_value=True))
         self.mock_object(
             self.neutron_api.client, 'create_port',
             mock.Mock(side_effect=neutron_client_exc.NeutronClientException))
@@ -200,7 +202,6 @@ class NeutronApiTest(test.TestCase):
                           **port_args)
 
         # Verify results
-        self.neutron_api._has_port_binding_extension.assert_called_once_with()
         self.assertTrue(neutron_api.LOG.exception.called)
         self.assertTrue(clientv20.Client.called)
         self.assertTrue(self.neutron_api.client.create_port.called)
@@ -208,9 +209,6 @@ class NeutronApiTest(test.TestCase):
     @mock.patch.object(neutron_api.LOG, 'exception', mock.Mock())
     def test_create_port_exception_status_409(self):
         # Set up test data
-        self.mock_object(
-            self.neutron_api, '_has_port_binding_extension',
-            mock.Mock(return_value=True))
         self.mock_object(
             self.neutron_api.client, 'create_port',
             mock.Mock(side_effect=neutron_client_exc.NeutronClientException(
@@ -223,7 +221,6 @@ class NeutronApiTest(test.TestCase):
                           **port_args)
 
         # Verify results
-        self.neutron_api._has_port_binding_extension.assert_called_once_with()
         self.assertTrue(neutron_api.LOG.exception.called)
         self.assertTrue(clientv20.Client.called)
         self.assertTrue(self.neutron_api.client.create_port.called)
diff --git a/releasenotes/notes/bug-1602525-port_binding_mandatory-2aaba0fa72b82676.yaml b/releasenotes/notes/bug-1602525-port_binding_mandatory-2aaba0fa72b82676.yaml
new file mode 100644
index 0000000000..e36318a697
--- /dev/null
+++ b/releasenotes/notes/bug-1602525-port_binding_mandatory-2aaba0fa72b82676.yaml
@@ -0,0 +1,4 @@
+---
+fixes:
+  - Raises an exception in case the host_id is specified when creating a neutron port
+    but the port_binding extension is not activated.