Fix prepare_for_replace/restore_prev_rsrc handing for server

Now, we set 'fixed_ips' to [] for server ports when prepare
for server replacement, but the ports are still in-use if only
set 'fixed_ips' to []. So this patch will to detach the ports from
nova server to make sure same ports can be attached to new one in
prepare_for_replace(). Also, when restoring server, we need to detach
ports from existing server, and then to attach them to previous
server.

We check the interface attach/detach complete by list the
server.interfaces, this change will use 'retry' wrapper to re-poll
the server interfaces ten times, then will raise
exception if the attach/detach still not complete.

Closes-Bug: #1533076
(cherry picked from commit 163d46bdc8bbfa2e7da2989f5a5d608826de2dcc)

Change-Id: I7b322f9cf16c100dcd0365bc3091c289f00f0548

5 files changed, 203 insertions, 130 deletions

diff --git a/heat/common/exception.py b/heat/common/exception.py
index ee91d995b..026e19aaa 100644
--- a/heat/common/exception.py
+++ b/heat/common/exception.py
@@ -479,6 +479,16 @@ class ServiceNotFound(HeatException):
     msg_fmt = _("Service %(service_id)s not found")
 
 
+class InterfaceAttachFailed(HeatException):
+    msg_fmt = _("Failed to attach interface (%(port)s) "
+                "to server (%(server)s)")
+
+
+class InterfaceDetachFailed(HeatException):
+    msg_fmt = _("Failed to detach interface (%(port)s) "
+                "from server (%(server)s)")
+
+
 class UnsupportedObjectError(HeatException):
     msg_fmt = _('Unsupported object type %(objtype)s')
 
diff --git a/heat/engine/clients/client_plugin.py b/heat/engine/clients/client_plugin.py
index 4b0306ceb..e8c15a707 100644
--- a/heat/engine/clients/client_plugin.py
+++ b/heat/engine/clients/client_plugin.py
@@ -228,3 +228,7 @@ class ClientPlugin(object):
             return True
         except exceptions.EndpointNotFound:
             return False
+
+
+def retry_if_result_is_false(result):
+    return result is False
diff --git a/heat/engine/clients/os/nova.py b/heat/engine/clients/os/nova.py
index a83fa4347..a778c152b 100644
--- a/heat/engine/clients/os/nova.py
+++ b/heat/engine/clients/os/nova.py
@@ -25,6 +25,7 @@ from novaclient import exceptions
 from oslo_config import cfg
 from oslo_serialization import jsonutils
 from oslo_utils import uuidutils
+from retrying import retry
 import six
 from six.moves.urllib import parse as urlparse
 
@@ -656,6 +657,30 @@ echo -e '%s\tALL=(ALL)\tNOPASSWD: ALL' >> /etc/sudoers
         else:
             return False
 
+    @retry(stop_max_attempt_number=10,
+           wait_fixed=500,
+           retry_on_result=client_plugin.retry_if_result_is_false)
+    def check_interface_detach(self, server_id, port_id):
+        server = self.fetch_server(server_id)
+        if server:
+            interfaces = server.interface_list()
+            for iface in interfaces:
+                if iface.port_id == port_id:
+                    return False
+        return True
+
+    @retry(stop_max_attempt_number=10,
+           wait_fixed=500,
+           retry_on_result=client_plugin.retry_if_result_is_false)
+    def check_interface_attach(self, server_id, port_id):
+        server = self.fetch_server(server_id)
+        if server:
+            interfaces = server.interface_list()
+            for iface in interfaces:
+                if iface.port_id == port_id:
+                    return True
+        return False
+
     def has_extension(self, alias):
         """Check if extension is present."""
         extensions = self.client().list_extensions.show_all()
diff --git a/heat/engine/resources/openstack/nova/server_network_mixin.py b/heat/engine/resources/openstack/nova/server_network_mixin.py
index 49af5abc2..fe1693d62 100644
--- a/heat/engine/resources/openstack/nova/server_network_mixin.py
+++ b/heat/engine/resources/openstack/nova/server_network_mixin.py
@@ -16,6 +16,7 @@ import itertools
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import netutils
+import retrying
 
 from heat.common import exception
 from heat.common.i18n import _
@@ -190,7 +191,6 @@ class ServerNetworkMixin(object):
     def _build_nics(self, networks, security_groups=None):
         if not networks:
             return None
-
         nics = []
 
         for idx, net in enumerate(networks):
@@ -355,39 +355,72 @@ class ServerNetworkMixin(object):
             [('external_ports', port)
              for port in self._data_get_ports('external_ports')]))
         for port_type, port in port_data:
-            # store port fixed_ips for restoring after failed update
-            port_details = self.client('neutron').show_port(port['id'])['port']
-            fixed_ips = port_details.get('fixed_ips', [])
-            data[port_type].append({'id': port['id'], 'fixed_ips': fixed_ips})
-
-        if data.get('internal_ports'):
-            self.data_set('internal_ports',
-                          jsonutils.dumps(data['internal_ports']))
-        if data.get('external_ports'):
-            self.data_set('external_ports',
-                          jsonutils.dumps(data['external_ports']))
-        # reset fixed_ips for these ports by setting for each of them
-        # fixed_ips to []
+            data[port_type].append({'id': port['id']})
+
+        # detach the ports from the server
+        server_id = self.resource_id
         for port_type, port in port_data:
-            self.client('neutron').update_port(
-                port['id'], {'port': {'fixed_ips': []}})
+            self.client_plugin().interface_detach(server_id, port['id'])
+            try:
+                if self.client_plugin().check_interface_detach(
+                        server_id, port['id']):
+                    LOG.info(_LI('Detach interface %(port)s successful '
+                                 'from server %(server)s when prepare '
+                                 'for replace.')
+                             % {'port': port['id'],
+                                'server': server_id})
+            except retrying.RetryError:
+                raise exception.InterfaceDetachFailed(
+                    port=port['id'], server=server_id)
 
     def restore_ports_after_rollback(self):
         if not self.is_using_neutron():
             return
 
-        old_server = self.stack._backup_stack().resources.get(self.name)
+        backup_res = self.stack._backup_stack().resources.get(self.name)
+        prev_server = backup_res
+        existing_server = self
+
+        port_data = itertools.chain(
+            existing_server._data_get_ports(),
+            existing_server._data_get_ports('external_ports')
+        )
 
-        port_data = itertools.chain(self._data_get_ports(),
-                                    self._data_get_ports('external_ports'))
+        existing_server_id = existing_server.resource_id
         for port in port_data:
-            self.client('neutron').update_port(port['id'],
-                                               {'port': {'fixed_ips': []}})
-
-        old_port_data = itertools.chain(
-            old_server._data_get_ports(),
-            old_server._data_get_ports('external_ports'))
-        for port in old_port_data:
-            fixed_ips = port['fixed_ips']
-            self.client('neutron').update_port(
-                port['id'], {'port': {'fixed_ips': fixed_ips}})
+            # detach the ports from current resource
+            self.client_plugin().interface_detach(
+                existing_server_id, port['id'])
+            try:
+                if self.client_plugin().check_interface_detach(
+                        existing_server_id, port['id']):
+                    LOG.info(_LI('Detach interface %(port)s successful from '
+                                 'server %(server)s when restore after '
+                                 'rollback.')
+                             % {'port': port['id'],
+                                'server': existing_server_id})
+            except retrying.RetryError:
+                raise exception.InterfaceDetachFailed(
+                    port=port['id'], server=existing_server_id)
+
+        # attach the ports for old resource
+        prev_port_data = itertools.chain(
+            prev_server._data_get_ports(),
+            prev_server._data_get_ports('external_ports'))
+
+        prev_server_id = prev_server.resource_id
+
+        for port in prev_port_data:
+            self.client_plugin().interface_attach(prev_server_id,
+                                                  port['id'])
+            try:
+                if self.client_plugin().check_interface_attach(
+                        prev_server_id, port['id']):
+                    LOG.info(_LI('Attach interface %(port)s successful to '
+                                 'server %(server)s when restore after '
+                                 'rollback.')
+                             % {'port': port['id'],
+                                'server': prev_server_id})
+            except retrying.RetryError:
+                raise exception.InterfaceAttachFailed(
+                    port=port['id'], server=prev_server_id)
diff --git a/heat/tests/nova/test_server.py b/heat/tests/nova/test_server.py
index a123e5a34..23eda8e08 100644
--- a/heat/tests/nova/test_server.py
+++ b/heat/tests/nova/test_server.py
@@ -113,6 +113,18 @@ resources:
       network: 12345
 '''
 
+tmpl_server_with_network_id = """
+heat_template_version: 2015-10-15
+resources:
+  server:
+    type: OS::Nova::Server
+    properties:
+      flavor: m1.small
+      image: F17-x86_64-gold
+      networks:
+        - network: 4321
+"""
+
 tmpl_server_with_sub_secu_group = """
 heat_template_version: 2015-10-15
 resources:
@@ -4087,8 +4099,6 @@ class ServerInternalPortTest(common.HeatTestCase):
                                             'delete_port')
         self.port_show = self.patchobject(neutronclient.Client,
                                           'show_port')
-        self.port_update = self.patchobject(neutronclient.Client,
-                                            'update_port')
 
     def _return_template_stack_and_rsrc_defn(self, stack_name, temp):
         templ = template.Template(template_format.parse(temp),
@@ -4444,127 +4454,118 @@ class ServerInternalPortTest(common.HeatTestCase):
         self.assertEqual({'port_type': 'external_ports'},
                          update_data.call_args_list[1][1])
 
+    def test_prepare_ports_for_replace_detach_failed(self):
+        t, stack, server = self._return_template_stack_and_rsrc_defn(
+            'test', tmpl_server_with_network_id)
+
+        class Fake(object):
+            def interface_list(self):
+                return [iface(1122)]
+        iface = collections.namedtuple('iface', ['port_id'])
+
+        server.resource_id = 'ser-11'
+        port_ids = [{'id': 1122}]
+
+        server._data = {"internal_ports": jsonutils.dumps(port_ids)}
+        self.patchobject(nova.NovaClientPlugin, 'interface_detach')
+        self.patchobject(nova.NovaClientPlugin, 'fetch_server')
+        nova.NovaClientPlugin.fetch_server.side_effect = [Fake()] * 10
+
+        exc = self.assertRaises(exception.InterfaceDetachFailed,
+                                server.prepare_for_replace)
+        self.assertIn('Failed to detach interface (1122) from server '
+                      '(ser-11)',
+                      six.text_type(exc))
+
     def test_prepare_ports_for_replace(self):
-        tmpl = """
-        heat_template_version: 2015-10-15
-        resources:
-          server:
-            type: OS::Nova::Server
-            properties:
-              flavor: m1.small
-              image: F17-x86_64-gold
-              networks:
-                - network: 4321
-        """
-        t, stack, server = self._return_template_stack_and_rsrc_defn('test',
-                                                                     tmpl)
+        t, stack, server = self._return_template_stack_and_rsrc_defn(
+            'test', tmpl_server_with_network_id)
+        server.resource_id = 'test_server'
         port_ids = [{'id': 1122}, {'id': 3344}]
         external_port_ids = [{'id': 5566}]
         server._data = {"internal_ports": jsonutils.dumps(port_ids),
                         "external_ports": jsonutils.dumps(external_port_ids)}
-        data_set = self.patchobject(server, 'data_set')
-
-        port1_fixed_ip = {
-            'fixed_ips': {
-                'subnet_id': 'test_subnet1',
-                'ip_address': '41.41.41.41'
-            }
-        }
-        port2_fixed_ip = {
-            'fixed_ips': {
-                'subnet_id': 'test_subnet2',
-                'ip_address': '42.42.42.42'
-            }
-        }
-        port3_fixed_ip = {
-            'fixed_ips': {
-                'subnet_id': 'test_subnet3',
-                'ip_address': '43.43.43.43'
-            }
-        }
-        self.port_show.side_effect = [{'port': port1_fixed_ip},
-                                      {'port': port2_fixed_ip},
-                                      {'port': port3_fixed_ip}]
+        self.patchobject(nova.NovaClientPlugin, 'interface_detach')
+        self.patchobject(nova.NovaClientPlugin, 'check_interface_detach',
+                         return_value=True)
 
         server.prepare_for_replace()
 
-        # check, that data was updated
-        port_ids[0].update(port1_fixed_ip)
-        port_ids[1].update(port2_fixed_ip)
-        external_port_ids[0].update(port3_fixed_ip)
-
-        expected_data = jsonutils.dumps(port_ids)
-        expected_external_data = jsonutils.dumps(external_port_ids)
-        data_set.assert_has_calls([
-            mock.call('internal_ports', expected_data),
-            mock.call('external_ports', expected_external_data)])
-
-        # check, that all ip were removed from ports
-        empty_fixed_ips = {'port': {'fixed_ips': []}}
-        self.port_update.assert_has_calls([
-            mock.call(1122, empty_fixed_ips),
-            mock.call(3344, empty_fixed_ips),
-            mock.call(5566, empty_fixed_ips)])
+        # check, that the ports were detached from server
+        nova.NovaClientPlugin.interface_detach.assert_has_calls([
+            mock.call('test_server', 1122),
+            mock.call('test_server', 3344),
+            mock.call('test_server', 5566)])
 
     def test_restore_ports_after_rollback(self):
-        tmpl = """
-        heat_template_version: 2015-10-15
-        resources:
-          server:
-            type: OS::Nova::Server
-            properties:
-              flavor: m1.small
-              image: F17-x86_64-gold
-              networks:
-                - network: 4321
-        """
-        t, stack, server = self._return_template_stack_and_rsrc_defn('test',
-                                                                     tmpl)
+        t, stack, server = self._return_template_stack_and_rsrc_defn(
+            'test', tmpl_server_with_network_id)
+        server.resource_id = 'existing_server'
         port_ids = [{'id': 1122}, {'id': 3344}]
         external_port_ids = [{'id': 5566}]
         server._data = {"internal_ports": jsonutils.dumps(port_ids),
                         "external_ports": jsonutils.dumps(external_port_ids)}
-        port1_fixed_ip = {
-            'fixed_ips': {
-                'subnet_id': 'test_subnet1',
-                'ip_address': '41.41.41.41'
-            }
-        }
-        port2_fixed_ip = {
-            'fixed_ips': {
-                'subnet_id': 'test_subnet2',
-                'ip_address': '42.42.42.42'
-            }
-        }
-        port3_fixed_ip = {
-            'fixed_ips': {
-                'subnet_id': 'test_subnet3',
-                'ip_address': '43.43.43.43'
-            }
-        }
-        port_ids[0].update(port1_fixed_ip)
-        port_ids[1].update(port2_fixed_ip)
-        external_port_ids[0].update(port3_fixed_ip)
+
         # add data to old server in backup stack
         old_server = mock.Mock()
+        old_server.resource_id = 'old_server'
         stack._backup_stack = mock.Mock()
         stack._backup_stack().resources.get.return_value = old_server
         old_server._data_get_ports.side_effect = [port_ids, external_port_ids]
 
+        self.patchobject(nova.NovaClientPlugin, 'interface_detach')
+        self.patchobject(nova.NovaClientPlugin, 'check_interface_detach',
+                         return_value=True)
+        self.patchobject(nova.NovaClientPlugin, 'interface_attach')
+        self.patchobject(nova.NovaClientPlugin, 'check_interface_attach',
+                         return_value=True)
+
         server.restore_after_rollback()
 
-        # check, that all ip were removed from new_ports
-        empty_fixed_ips = {'port': {'fixed_ips': []}}
-        self.port_update.assert_has_calls([
-            mock.call(1122, empty_fixed_ips),
-            mock.call(3344, empty_fixed_ips),
-            mock.call(5566, empty_fixed_ips)])
-
-        # check, that all ip were restored for old_ports
-        self.port_update.assert_has_calls([
-            mock.call(1122, {'port': port1_fixed_ip}),
-            mock.call(3344, {'port': port2_fixed_ip}),
-            mock.call(5566, {'port': port3_fixed_ip})])
+        # check, that ports were detached from new server
+        nova.NovaClientPlugin.interface_detach.assert_has_calls([
+            mock.call('existing_server', 1122),
+            mock.call('existing_server', 3344),
+            mock.call('existing_server', 5566)])
+
+        # check, that ports were attached to old server
+        nova.NovaClientPlugin.interface_attach.assert_has_calls([
+            mock.call('old_server', 1122),
+            mock.call('old_server', 3344),
+            mock.call('old_server', 5566)])
+
+    def test_restore_ports_after_rollback_attach_failed(self):
+        t, stack, server = self._return_template_stack_and_rsrc_defn(
+            'test', tmpl_server_with_network_id)
+        server.resource_id = 'existing_server'
+        port_ids = [{'id': 1122}, {'id': 3344}]
+        server._data = {"internal_ports": jsonutils.dumps(port_ids)}
+
+        # add data to old server in backup stack
+        old_server = mock.Mock()
+        old_server.resource_id = 'old_server'
+        stack._backup_stack = mock.Mock()
+        stack._backup_stack().resources.get.return_value = old_server
+        old_server._data_get_ports.side_effect = [port_ids, []]
+
+        class Fake(object):
+            def interface_list(self):
+                return [iface(1122)]
+        iface = collections.namedtuple('iface', ['port_id'])
+
+        self.patchobject(nova.NovaClientPlugin, 'interface_detach')
+        self.patchobject(nova.NovaClientPlugin, 'check_interface_detach',
+                         return_value=True)
+        self.patchobject(nova.NovaClientPlugin, 'interface_attach')
+        self.patchobject(nova.NovaClientPlugin, 'fetch_server')
+        # need to mock 11 times: 1 for port 1122, 10 for port 3344
+        nova.NovaClientPlugin.fetch_server.side_effect = [Fake()] * 11
+
+        exc = self.assertRaises(exception.InterfaceAttachFailed,
+                                server.restore_after_rollback)
+        self.assertIn('Failed to attach interface (3344) to server '
+                      '(old_server)',
+                      six.text_type(exc))
 
     def test_store_external_ports_os_interface_not_installed(self):
         tmpl = """