diff --git a/heat/engine/resources/openstack/nova/server_network_mixin.py b/heat/engine/resources/openstack/nova/server_network_mixin.py index 9def1a221a..87b5cb42b4 100644 --- a/heat/engine/resources/openstack/nova/server_network_mixin.py +++ b/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 - 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) + self.detach_ports(self) 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( - existing_server._data_get_ports(), - existing_server._data_get_ports('external_ports') - ) - - existing_server_id = existing_server.resource_id - for port in port_data: - # detach the ports from current resource - self.client_plugin().interface_detach( - existing_server_id, port['id']) + # Wait until server will move to active state. We can't + # detach interfaces from server in BUILDING state. + # 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. + if existing_server.resource_id is not None: 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) + while True: + active = self.client_plugin()._check_active( + existing_server.resource_id) + if active: + break + eventlet.sleep(1) + except exception.ResourceInError: + pass - # attach the ports for old resource - prev_port_data = itertools.chain( - prev_server._data_get_ports(), - prev_server._data_get_ports('external_ports')) + self.store_external_ports() + self.detach_ports(existing_server) - 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) + self.attach_ports(prev_server) diff --git a/heat/tests/openstack/nova/test_server.py b/heat/tests/openstack/nova/test_server.py index a0461df65d..97258f87e7 100644 --- a/heat/tests/openstack/nova/test_server.py +++ b/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'] diff --git a/heat_integrationtests/common/test.py b/heat_integrationtests/common/test.py index a7aec6c473..b9bb0362c5 100644 --- a/heat_integrationtests/common/test.py +++ b/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, diff --git a/heat_integrationtests/functional/test_cancel_update.py b/heat_integrationtests/functional/test_cancel_update.py new file mode 100644 index 0000000000..c40aeb0a06 --- /dev/null +++ b/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)