From e35f230e1a9374cebd6aa43761da2a49e7d5c204 Mon Sep 17 00:00:00 2001
From: Vasyl Saienko <vsaienko@mirantis.com>
Date: Tue, 22 Aug 2017 17:30:36 +0300
Subject: [PATCH] Rework update_port_address logic

Some neutron backends like contrail require port is unbound before one
can change its mac_address or vnic_type is set to baremeteal.
Rework update_port_address for Neutron case to use separate steps for:
  1. Unbind port
  2. Update mac address
  3. Bind port

Change-Id: I9938fa565eef7e0f1403f6a7c55922115128158c
Closes-Bug: #1712359
---
 ironic/common/neutron.py                          | 15 ++++++++++++---
 ironic/drivers/modules/network/common.py          |  2 +-
 ironic/tests/unit/common/test_neutron.py          | 10 ++++++----
 ...ess-update-with-contrail-b1e1b725cc0829c2.yaml |  5 +++++
 4 files changed, 24 insertions(+), 8 deletions(-)
 create mode 100644 releasenotes/notes/fix-mac-address-update-with-contrail-b1e1b725cc0829c2.yaml

diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py
index 58b03dc6a2..1cb0b0d3d2 100644
--- a/ironic/common/neutron.py
+++ b/ironic/common/neutron.py
@@ -26,6 +26,8 @@ DEFAULT_NEUTRON_URL = 'http://%s:9696' % CONF.my_ip
 
 _NEUTRON_SESSION = None
 
+VNIC_BAREMETAL = 'baremetal'
+
 PHYSNET_PARAM_NAME = 'provider:physical_network'
 """Name of the neutron network API physical network parameter."""
 
@@ -132,11 +134,18 @@ def update_port_address(port_id, address):
                      "Neutron port %s, while updating its MAC "
                      "address.") % port_id)
             unbind_neutron_port(port_id, client=client)
-            port_req_body['port']['binding:host_id'] = binding_host_id
-            port_req_body['port']['binding:profile'] = binding_profile
 
         msg = (_("Failed to update MAC address on Neutron port %s.") % port_id)
         client.update_port(port_id, port_req_body)
+
+        # Restore original binding:profile and host_id
+        if binding_host_id:
+            msg = (_("Failed to update binding:host_id and profile on Neutron "
+                     "port %s.") % port_id)
+            port_req_body = {'port': {'binding:host_id': binding_host_id,
+                                      'binding:profile': binding_profile}}
+
+            client.update_port(port_id, port_req_body)
     except (neutron_exceptions.NeutronClientException, exception.NetworkError):
         LOG.exception(msg)
         raise exception.FailedToUpdateMacOnPort(port_id=port_id)
@@ -200,7 +209,7 @@ def add_ports_to_network(task, network_uuid, security_groups=None):
         'port': {
             'network_id': network_uuid,
             'admin_state_up': True,
-            'binding:vnic_type': 'baremetal',
+            'binding:vnic_type': VNIC_BAREMETAL,
             'device_owner': 'baremetal:none',
             'binding:host_id': node.uuid,
         }
diff --git a/ironic/drivers/modules/network/common.py b/ironic/drivers/modules/network/common.py
index 7fe3527314..6b4350bc84 100644
--- a/ironic/drivers/modules/network/common.py
+++ b/ironic/drivers/modules/network/common.py
@@ -252,7 +252,7 @@ def plug_port_to_tenant_network(task, port_like_obj, client=None):
     # nova.
     body = {
         'port': {
-            'binding:vnic_type': 'baremetal',
+            'binding:vnic_type': neutron.VNIC_BAREMETAL,
             'binding:host_id': node.uuid,
         }
     }
diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py
index 9b277a7ada..6fa0c14e43 100644
--- a/ironic/tests/unit/common/test_neutron.py
+++ b/ironic/tests/unit/common/test_neutron.py
@@ -550,16 +550,18 @@ class TestUpdatePortAddress(base.TestCase):
     def test_update_port_address_with_binding(self, mock_unp, mock_client):
         address = 'fe:54:00:77:07:d9'
         port_id = 'fake-port-id'
-        expected = {'port': {'mac_address': address,
-                             'binding:host_id': 'host',
-                             'binding:profile': 'foo'}}
+
         mock_client.return_value.show_port.return_value = {
             'port': {'binding:host_id': 'host',
                      'binding:profile': 'foo'}}
 
+        calls = [mock.call(port_id, {'port': {'mac_address': address}}),
+                 mock.call(port_id, {'port': {'binding:host_id': 'host',
+                                     'binding:profile': 'foo'}})]
+
         neutron.update_port_address(port_id, address)
         mock_unp.assert_called_once_with(port_id, client=mock_client())
-        mock_client.return_value.update_port.assert_any_call(port_id, expected)
+        mock_client.return_value.update_port.assert_has_calls(calls)
 
     @mock.patch.object(neutron, 'unbind_neutron_port', autospec=True)
     def test_update_port_address_without_binding(self, mock_unp, mock_client):
diff --git a/releasenotes/notes/fix-mac-address-update-with-contrail-b1e1b725cc0829c2.yaml b/releasenotes/notes/fix-mac-address-update-with-contrail-b1e1b725cc0829c2.yaml
new file mode 100644
index 0000000000..019d876ef2
--- /dev/null
+++ b/releasenotes/notes/fix-mac-address-update-with-contrail-b1e1b725cc0829c2.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - Fixed an issue when mac address update was called in the same
+    request with bind port which is not handled by some neutron
+    backends like contrail correctly.