openstack/heat
Code Issues Proposed changes

Fix cancel update for nova server with defined port

Browse Source 
This particular patch fixes a behaviour of cancel update for
nova server with defined port, so there are no ports manageable
by nova. We have these issues while restoring ports after rollback:
1) We doesn't detach any ports from current server, because we
doesn't save them to resoruce data. (we store this data after
succesfull create of the server)
2) Detaching an interface from current server will fail, if the server
will be in building state, so we need to wait until server will be
in active or in error state.
Refresh ports list to solve problem (1).
Wait until nova moves to active/error state to solve (2).
A functional test to prove the fix was added. Note, that this test is
skipped for convergence engine tests until cancel update will work
properly in convergence mode (see bug 1533176).
Partial-Bug: #1570908
Change-Id: If6fd916068a425eea6dc795192f286cb5ffcb794
This commit is contained in:
Oleksii Chuprykov 2016-04-29 17:03:17 +03:00 committed by Anant Patil
parent 99b055b423
commit 584efe3329
4 changed files with 159 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

116
heat/engine/resources/openstack/nova/server_network_mixin.py
View File

@ -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)

21
heat/tests/openstack/nova/test_server.py
View File

@ -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']

19
heat_integrationtests/common/test.py
View File

@ -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,

63
heat_integrationtests/functional/test_cancel_update.py Normal file
View File

@ -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)