Merge "Fix cancel update for nova server with defined port" into stable/newton

heat/engine/resources/openstack/nova/server_network_mixin.py

@@ -13,6 +13,7 @@
 
 import itertools
 
+import eventlet
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import netutils
@@ -21,9 +22,7 @@ import retrying
 from heat.common import exception
 from heat.common.i18n import _
 from heat.common.i18n import _LI
-
 from heat.engine import resource
-
 from heat.engine.resources.openstack.neutron import port as neutron_port
 
 LOG = logging.getLogger(__name__)
@@ -413,6 +412,46 @@ class ServerNetworkMixin(object):
         elif not self.is_using_neutron():
             self._floating_ip_nova_associate(floating_ip)
 
+    @staticmethod
+    def get_all_ports(server):
+        return itertools.chain(
+            server._data_get_ports(),
+            server._data_get_ports('external_ports')
+        )
+
+    def detach_ports(self, server):
+        existing_server_id = server.resource_id
+        for port in self.get_all_ports(server):
+            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.')
+                             % {'port': port['id'],
+                                'server': existing_server_id})
+            except retrying.RetryError:
+                raise exception.InterfaceDetachFailed(
+                    port=port['id'], server=existing_server_id)
+
+    def attach_ports(self, server):
+        prev_server_id = server.resource_id
+
+        for port in self.get_all_ports(server):
+            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')
+                             % {'port': port['id'],
+                                'server': prev_server_id})
+            except retrying.RetryError:
+                raise exception.InterfaceAttachFailed(
+                    port=port['id'], server=prev_server_id)
+
     def prepare_ports_for_replace(self):
         if not self.is_using_neutron():
             return
@@ -426,21 +465,7 @@ class ServerNetworkMixin(object):
         for port_type, port in port_data:
             data[port_type].append({'id': port['id']})
 
-        # detach the ports from the server
+        self.detach_ports(self)
-        server_id = self.resource_id
-        for port_type, port in port_data:
-            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, convergence):
         if not self.is_using_neutron():
@@ -460,46 +485,23 @@ class ServerNetworkMixin(object):
         else:
             existing_server = self
 
-        port_data = itertools.chain(
+        # Wait until server will move to active state. We can't
-            existing_server._data_get_ports(),
+        # detach interfaces from server in BUILDING state.
-            existing_server._data_get_ports('external_ports')
+        # In case of convergence, the replacement resource may be
-        )
+        # created but never have been worked on because the rollback was
-
+        # trigerred or new update was trigerred.
-        existing_server_id = existing_server.resource_id
+        if existing_server.resource_id is not None:
-        for port in port_data:
-            # detach the ports from current resource
-            self.client_plugin().interface_detach(
-                existing_server_id, port['id'])
             try:
-                if self.client_plugin().check_interface_detach(
+                while True:
-                        existing_server_id, port['id']):
+                    active = self.client_plugin()._check_active(
-                    LOG.info(_LI('Detach interface %(port)s successful from '
+                        existing_server.resource_id)
-                                 'server %(server)s when restore after '
+                    if active:
-                                 'rollback.')
+                        break
-                             % {'port': port['id'],
+                    eventlet.sleep(1)
-                                'server': existing_server_id})
+            except exception.ResourceInError:
-            except retrying.RetryError:
+                pass
-                raise exception.InterfaceDetachFailed(
-                    port=port['id'], server=existing_server_id)
 
-        # attach the ports for old resource
+            self.store_external_ports()
-        prev_port_data = itertools.chain(
+            self.detach_ports(existing_server)
-            prev_server._data_get_ports(),
-            prev_server._data_get_ports('external_ports'))
 
-        prev_server_id = prev_server.resource_id
+        self.attach_ports(prev_server)
-
-        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)

heat/tests/openstack/nova/test_server.py

@@ -35,6 +35,7 @@ from heat.engine.clients.os import zaqar
 from heat.engine import environment
 from heat.engine import resource
 from heat.engine.resources.openstack.nova import server as servers
+from heat.engine.resources.openstack.nova import server_network_mixin
 from heat.engine.resources import scheduler_hints as sh
 from heat.engine import scheduler
 from heat.engine import stack as parser
@@ -4395,7 +4396,9 @@ class ServerInternalPortTest(common.HeatTestCase):
             mock.call('test_server', 3344),
             mock.call('test_server', 5566)])
 
-    def test_restore_ports_after_rollback(self):
+    @mock.patch.object(server_network_mixin.ServerNetworkMixin,
+                       'store_external_ports')
+    def test_restore_ports_after_rollback(self, store_ports):
         t, stack, server = self._return_template_stack_and_rsrc_defn(
             'test', tmpl_server_with_network_id)
         server.resource_id = 'existing_server'
@@ -4403,6 +4406,8 @@ class ServerInternalPortTest(common.HeatTestCase):
         external_port_ids = [{'id': 5566}]
         server._data = {"internal_ports": jsonutils.dumps(port_ids),
                         "external_ports": jsonutils.dumps(external_port_ids)}
+        self.patchobject(nova.NovaClientPlugin, '_check_active')
+        nova.NovaClientPlugin._check_active.side_effect = [False, True]
 
         # add data to old server in backup stack
         old_server = mock.Mock()
@@ -4420,6 +4425,8 @@ class ServerInternalPortTest(common.HeatTestCase):
 
         server.restore_prev_rsrc()
 
+        self.assertEqual(2, nova.NovaClientPlugin._check_active.call_count)
+
         # check, that ports were detached from new server
         nova.NovaClientPlugin.interface_detach.assert_has_calls([
             mock.call('existing_server', 1122),
@@ -4432,12 +4439,16 @@ class ServerInternalPortTest(common.HeatTestCase):
             mock.call('old_server', 3344),
             mock.call('old_server', 5566)])
 
-    def test_restore_ports_after_rollback_attach_failed(self):
+    @mock.patch.object(server_network_mixin.ServerNetworkMixin,
+                       'store_external_ports')
+    def test_restore_ports_after_rollback_attach_failed(self, store_ports):
         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)}
+        self.patchobject(nova.NovaClientPlugin, '_check_active')
+        nova.NovaClientPlugin._check_active.return_value = True
 
         # add data to old server in backup stack
         old_server = mock.Mock()
@@ -4465,10 +4476,14 @@ class ServerInternalPortTest(common.HeatTestCase):
                       '(old_server)',
                       six.text_type(exc))
 
-    def test_restore_ports_after_rollback_convergence(self):
+    @mock.patch.object(server_network_mixin.ServerNetworkMixin,
+                       'store_external_ports')
+    def test_restore_ports_after_rollback_convergence(self, store_ports):
         t = template_format.parse(tmpl_server_with_network_id)
         stack = utils.parse_stack(t)
         stack.store()
+        self.patchobject(nova.NovaClientPlugin, '_check_active')
+        nova.NovaClientPlugin._check_active.return_value = True
 
         # mock resource from previous template
         prev_rsrc = stack['server']

heat_integrationtests/common/test.py

@@ -411,6 +411,25 @@ class HeatIntegrationTest(testscenarios.WithScenarios,
 
         self._wait_for_stack_status(**kwargs)
 
+    def cancel_update_stack(self, stack_identifier,
+                            expected_status='ROLLBACK_COMPLETE'):
+
+        stack_name = stack_identifier.split('/')[0]
+
+        self.updated_time[stack_identifier] = self.client.stacks.get(
+            stack_identifier, resolve_outputs=False).updated_time
+
+        self.client.actions.cancel_update(stack_name)
+
+        kwargs = {'stack_identifier': stack_identifier,
+                  'status': expected_status}
+        if expected_status in ['ROLLBACK_COMPLETE']:
+            # To trigger rollback you would intentionally fail the stack
+            # Hence check for rollback failures
+            kwargs['failure_pattern'] = '^ROLLBACK_FAILED$'
+
+        self._wait_for_stack_status(**kwargs)
+
     def preview_update_stack(self, stack_identifier, template,
                              environment=None, files=None, parameters=None,
                              tags=None, disable_rollback=True,

heat_integrationtests/functional/test_cancel_update.py

@@ -0,0 +1,63 @@
+#    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.
+
+from heat_integrationtests.functional import functional_base
+
+
+class CancelUpdateTest(functional_base.FunctionalTestsBase):
+
+    template = '''
+heat_template_version: '2013-05-23'
+parameters:
+ InstanceType:
+   type: string
+ ImageId:
+   type: string
+ network:
+   type: string
+resources:
+ port:
+   type: OS::Neutron::Port
+   properties:
+     network: {get_param: network}
+ Server:
+   type: OS::Nova::Server
+   properties:
+     flavor_update_policy: REPLACE
+     image: {get_param: ImageId}
+     flavor: {get_param: InstanceType}
+     networks:
+       - port: {get_resource: port}
+'''
+
+    def setUp(self):
+        super(CancelUpdateTest, self).setUp()
+        if not self.conf.image_ref:
+            raise self.skipException("No image configured to test.")
+        if not self.conf.instance_type:
+            raise self.skipException("No flavor configured to test.")
+        if not self.conf.minimal_instance_type:
+            raise self.skipException("No minimal flavor configured to test.")
+
+    def test_cancel_update_server_with_port(self):
+        parameters = {'InstanceType': self.conf.minimal_instance_type,
+                      'ImageId': self.conf.image_ref,
+                      'network': self.conf.fixed_network_name}
+
+        stack_identifier = self.stack_create(template=self.template,
+                                             parameters=parameters)
+        parameters['InstanceType'] = 'm1.large'
+        self.update_stack(stack_identifier, self.template,
+                          parameters=parameters,
+                          expected_status='UPDATE_IN_PROGRESS')
+
+        self.cancel_update_stack(stack_identifier)