Don't metadata_update all resources for deployment signals
This fixes a race when a call to
SoftwareConfigService._push_metadata_software_deployments is in
progress at the same time as other deployments are signalling heat.
The signal triggers a metadata_update() on the server resource
which results in rsrc_metadata being updated without the resource
lock. This can overwrite the rsrc_metadata being written by
SoftwareConfigService._push_metadata_software_deployments resulting in
stale metadata.
Note: heat/tests/engine/test_stack_snapshot.py was not getting tested
as there was no __init__.py. This test file seems to require code
that is not in stable/kilo.
Change-Id: I081bc154ed7e79f4a4258c846857b3f39cc7887c
Closes-Bug: #1488366
(cherry picked from commit b8f3820414)
This commit is contained in:
Angus Salkeld
Rabi Mishra
parent
d156095bff
commit
2bffb1273c
|
|
@ -120,6 +120,10 @@ class Resource(object):
|
|||
# no signal actions
|
||||
no_signal_actions = (SUSPEND, DELETE)
|
||||
|
||||
# Whether all other resources need a metadata_update() after
|
||||
# a signal to this resource
|
||||
signal_needs_metadata_updates = True
|
||||
|
||||
def __new__(cls, name, definition, stack):
|
||||
'''Create a new Resource of the appropriate class for its type.'''
|
||||
|
||||
|
|
|
|||
|
|
@ -179,6 +179,10 @@ class SoftwareDeployment(signal_responder.SignalResponder):
|
|||
|
||||
no_signal_actions = ()
|
||||
|
||||
# No need to make metadata_update() calls since deployments have a
|
||||
# dedicated API for changing state on signals
|
||||
signal_needs_metadata_updates = False
|
||||
|
||||
def _signal_transport_cfn(self):
|
||||
return self.properties.get(
|
||||
self.SIGNAL_TRANSPORT) == self.CFN_SIGNAL
|
||||
|
|
|
|||
|
|
@ -1126,6 +1126,9 @@ class EngineService(service.Service):
|
|||
LOG.debug("signaling resource %s:%s" % (stack.name, rsrc.name))
|
||||
rsrc.signal(details)
|
||||
|
||||
if not rsrc.signal_needs_metadata_updates:
|
||||
return
|
||||
|
||||
# Refresh the metadata for all other resources, since signals can
|
||||
# update metadata which is used by other resources, e.g
|
||||
# when signalling a WaitConditionHandle resource, and other
|
||||
|
|
|
|||
|
|
@ -1,260 +0,0 @@
|
|||
# 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.
|
||||
|
||||
import uuid
|
||||
|
||||
import mock
|
||||
from oslo_messaging.rpc import dispatcher
|
||||
import six
|
||||
|
||||
from heat.common import exception
|
||||
from heat.common import template_format
|
||||
from heat.engine import service
|
||||
from heat.engine import stack
|
||||
from heat.objects import snapshot as snapshot_objects
|
||||
from heat.tests import common
|
||||
from heat.tests.engine import tools
|
||||
from heat.tests import utils
|
||||
|
||||
|
||||
class SnapshotServiceTest(common.HeatTestCase):
|
||||
# TODO(Qiming): Rework this test to handle OS::Nova::Server which
|
||||
# has a real snapshot support.
|
||||
def setUp(self):
|
||||
super(SnapshotServiceTest, self).setUp()
|
||||
self.ctx = utils.dummy_context()
|
||||
|
||||
self.engine = service.EngineService('a-host', 'a-topic')
|
||||
self.engine.create_periodic_tasks()
|
||||
utils.setup_dummy_db()
|
||||
|
||||
def _create_stack(self, stack_name, files=None):
|
||||
t = template_format.parse(tools.wp_template)
|
||||
stk = utils.parse_stack(t, stack_name=stack_name, files=files)
|
||||
stk.state_set(stk.CREATE, stk.COMPLETE, 'mock completion')
|
||||
|
||||
return stk
|
||||
|
||||
def test_show_snapshot_not_found(self):
|
||||
stk = self._create_stack('stack_snapshot_not_found')
|
||||
snapshot_id = str(uuid.uuid4())
|
||||
ex = self.assertRaises(dispatcher.ExpectedException,
|
||||
self.engine.show_snapshot,
|
||||
self.ctx, stk.identifier(),
|
||||
snapshot_id)
|
||||
expected = 'Snapshot with id %s not found' % snapshot_id
|
||||
self.assertEqual(exception.NotFound, ex.exc_info[0])
|
||||
self.assertIn(expected, six.text_type(ex.exc_info[1]))
|
||||
|
||||
def test_show_snapshot_not_belong_to_stack(self):
|
||||
stk1 = self._create_stack('stack_snaphot_not_belong_to_stack_1')
|
||||
snapshot1 = self.engine.stack_snapshot(
|
||||
self.ctx, stk1.identifier(), 'snap1')
|
||||
self.engine.thread_group_mgr.groups[stk1.id].wait()
|
||||
snapshot_id = snapshot1['id']
|
||||
|
||||
stk2 = self._create_stack('stack_snaphot_not_belong_to_stack_2')
|
||||
ex = self.assertRaises(dispatcher.ExpectedException,
|
||||
self.engine.show_snapshot,
|
||||
self.ctx, stk2.identifier(),
|
||||
snapshot_id)
|
||||
expected = ('The Snapshot (%(snapshot)s) for Stack (%(stack)s) '
|
||||
'could not be found') % {'snapshot': snapshot_id,
|
||||
'stack': stk2.name}
|
||||
self.assertEqual(exception.SnapshotNotFound, ex.exc_info[0])
|
||||
self.assertIn(expected, six.text_type(ex.exc_info[1]))
|
||||
|
||||
@mock.patch.object(stack.Stack, 'load')
|
||||
def test_create_snapshot(self, mock_load):
|
||||
files = {'a_file': 'the contents'}
|
||||
stk = self._create_stack('stack_snapshot_create', files=files)
|
||||
mock_load.return_value = stk
|
||||
|
||||
snapshot = self.engine.stack_snapshot(
|
||||
self.ctx, stk.identifier(), 'snap1')
|
||||
self.assertIsNotNone(snapshot['id'])
|
||||
self.assertIsNotNone(snapshot['creation_time'])
|
||||
self.assertEqual('snap1', snapshot['name'])
|
||||
self.assertEqual("IN_PROGRESS", snapshot['status'])
|
||||
self.engine.thread_group_mgr.groups[stk.id].wait()
|
||||
snapshot = self.engine.show_snapshot(
|
||||
self.ctx, stk.identifier(), snapshot['id'])
|
||||
self.assertEqual("COMPLETE", snapshot['status'])
|
||||
self.assertEqual("SNAPSHOT", snapshot['data']['action'])
|
||||
self.assertEqual("COMPLETE", snapshot['data']['status'])
|
||||
self.assertEqual(files, snapshot['data']['files'])
|
||||
self.assertEqual(stk.id, snapshot['data']['id'])
|
||||
self.assertIsNotNone(stk.updated_time)
|
||||
self.assertIsNotNone(snapshot['creation_time'])
|
||||
mock_load.assert_called_once_with(self.ctx, stack=mock.ANY)
|
||||
|
||||
@mock.patch.object(stack.Stack, 'load')
|
||||
def test_create_snapshot_action_in_progress(self, mock_load):
|
||||
stack_name = 'stack_snapshot_action_in_progress'
|
||||
stk = self._create_stack(stack_name)
|
||||
mock_load.return_value = stk
|
||||
|
||||
stk.state_set(stk.UPDATE, stk.IN_PROGRESS, 'test_override')
|
||||
ex = self.assertRaises(dispatcher.ExpectedException,
|
||||
self.engine.stack_snapshot,
|
||||
self.ctx, stk.identifier(), 'snap_none')
|
||||
self.assertEqual(exception.ActionInProgress, ex.exc_info[0])
|
||||
msg = ("Stack %(stack)s already has an action (%(action)s) "
|
||||
"in progress.") % {'stack': stack_name, 'action': stk.action}
|
||||
self.assertEqual(msg, six.text_type(ex.exc_info[1]))
|
||||
|
||||
mock_load.assert_called_once_with(self.ctx, stack=mock.ANY)
|
||||
|
||||
@mock.patch.object(stack.Stack, 'load')
|
||||
def test_delete_snapshot_not_found(self, mock_load):
|
||||
stk = self._create_stack('stack_snapshot_delete_not_found')
|
||||
mock_load.return_value = stk
|
||||
|
||||
snapshot_id = str(uuid.uuid4())
|
||||
ex = self.assertRaises(dispatcher.ExpectedException,
|
||||
self.engine.delete_snapshot,
|
||||
self.ctx, stk.identifier(), snapshot_id)
|
||||
self.assertEqual(exception.NotFound, ex.exc_info[0])
|
||||
|
||||
mock_load.assert_called_once_with(self.ctx, stack=mock.ANY)
|
||||
|
||||
@mock.patch.object(stack.Stack, 'load')
|
||||
def test_delete_snapshot_not_belong_to_stack(self, mock_load):
|
||||
stk1 = self._create_stack('stack_snapshot_delete_not_belong_1')
|
||||
mock_load.return_value = stk1
|
||||
|
||||
snapshot1 = self.engine.stack_snapshot(
|
||||
self.ctx, stk1.identifier(), 'snap1')
|
||||
self.engine.thread_group_mgr.groups[stk1.id].wait()
|
||||
snapshot_id = snapshot1['id']
|
||||
|
||||
mock_load.assert_called_once_with(self.ctx, stack=mock.ANY)
|
||||
mock_load.reset_mock()
|
||||
|
||||
stk2 = self._create_stack('stack_snapshot_delete_not_belong_2')
|
||||
mock_load.return_value = stk2
|
||||
|
||||
ex = self.assertRaises(dispatcher.ExpectedException,
|
||||
self.engine.delete_snapshot,
|
||||
self.ctx,
|
||||
stk2.identifier(),
|
||||
snapshot_id)
|
||||
expected = ('The Snapshot (%(snapshot)s) for Stack (%(stack)s) '
|
||||
'could not be found') % {'snapshot': snapshot_id,
|
||||
'stack': stk2.name}
|
||||
self.assertEqual(exception.SnapshotNotFound, ex.exc_info[0])
|
||||
self.assertIn(expected, six.text_type(ex.exc_info[1]))
|
||||
|
||||
mock_load.assert_called_once_with(self.ctx, stack=mock.ANY)
|
||||
mock_load.reset_mock()
|
||||
|
||||
@mock.patch.object(stack.Stack, 'load')
|
||||
def test_delete_snapshot_in_progress(self, mock_load):
|
||||
# can not delete the snapshot in snapshotting
|
||||
stk = self._create_stack('test_delete_snapshot_in_progress')
|
||||
mock_load.return_value = stk
|
||||
snapshot = mock.Mock()
|
||||
snapshot.id = str(uuid.uuid4())
|
||||
snapshot.status = 'IN_PROGRESS'
|
||||
self.patchobject(snapshot_objects.Snapshot,
|
||||
'get_snapshot_by_stack').return_value = snapshot
|
||||
ex = self.assertRaises(dispatcher.ExpectedException,
|
||||
self.engine.delete_snapshot,
|
||||
self.ctx, stk.identifier(), snapshot.id)
|
||||
msg = 'Deleting in-progress snapshot is not supported'
|
||||
self.assertIn(msg, six.text_type(ex.exc_info[1]))
|
||||
self.assertEqual(exception.NotSupported, ex.exc_info[0])
|
||||
|
||||
@mock.patch.object(stack.Stack, 'load')
|
||||
def test_delete_snapshot(self, mock_load):
|
||||
stk = self._create_stack('stack_snapshot_delete_normal')
|
||||
mock_load.return_value = stk
|
||||
|
||||
snapshot = self.engine.stack_snapshot(
|
||||
self.ctx, stk.identifier(), 'snap1')
|
||||
self.engine.thread_group_mgr.groups[stk.id].wait()
|
||||
snapshot_id = snapshot['id']
|
||||
self.engine.delete_snapshot(self.ctx, stk.identifier(), snapshot_id)
|
||||
self.engine.thread_group_mgr.groups[stk.id].wait()
|
||||
|
||||
ex = self.assertRaises(dispatcher.ExpectedException,
|
||||
self.engine.show_snapshot, self.ctx,
|
||||
stk.identifier(), snapshot_id)
|
||||
self.assertEqual(exception.NotFound, ex.exc_info[0])
|
||||
|
||||
self.assertTrue(2, mock_load.call_count)
|
||||
|
||||
@mock.patch.object(stack.Stack, 'load')
|
||||
def test_list_snapshots(self, mock_load):
|
||||
stk = self._create_stack('stack_snapshot_list')
|
||||
mock_load.return_value = stk
|
||||
|
||||
snapshot = self.engine.stack_snapshot(
|
||||
self.ctx, stk.identifier(), 'snap1')
|
||||
self.assertIsNotNone(snapshot['id'])
|
||||
self.assertEqual("IN_PROGRESS", snapshot['status'])
|
||||
self.engine.thread_group_mgr.groups[stk.id].wait()
|
||||
|
||||
snapshots = self.engine.stack_list_snapshots(
|
||||
self.ctx, stk.identifier())
|
||||
expected = {
|
||||
"id": snapshot["id"],
|
||||
"name": "snap1",
|
||||
"status": "COMPLETE",
|
||||
"status_reason": "Stack SNAPSHOT completed successfully",
|
||||
"data": stk.prepare_abandon(),
|
||||
"creation_time": snapshot['creation_time']}
|
||||
|
||||
self.assertEqual([expected], snapshots)
|
||||
mock_load.assert_called_once_with(self.ctx, stack=mock.ANY)
|
||||
|
||||
@mock.patch.object(stack.Stack, 'load')
|
||||
def test_restore_snapshot(self, mock_load):
|
||||
stk = self._create_stack('stack_snapshot_restore_normal')
|
||||
mock_load.return_value = stk
|
||||
|
||||
snapshot = self.engine.stack_snapshot(
|
||||
self.ctx, stk.identifier(), 'snap1')
|
||||
self.engine.thread_group_mgr.groups[stk.id].wait()
|
||||
snapshot_id = snapshot['id']
|
||||
self.engine.stack_restore(self.ctx, stk.identifier(), snapshot_id)
|
||||
self.engine.thread_group_mgr.groups[stk.id].wait()
|
||||
self.assertEqual((stk.RESTORE, stk.COMPLETE), stk.state)
|
||||
self.assertEqual(2, mock_load.call_count)
|
||||
|
||||
@mock.patch.object(stack.Stack, 'load')
|
||||
def test_restore_snapshot_other_stack(self, mock_load):
|
||||
stk1 = self._create_stack('stack_snapshot_restore_other_stack_1')
|
||||
mock_load.return_value = stk1
|
||||
|
||||
snapshot1 = self.engine.stack_snapshot(
|
||||
self.ctx, stk1.identifier(), 'snap1')
|
||||
self.engine.thread_group_mgr.groups[stk1.id].wait()
|
||||
snapshot_id = snapshot1['id']
|
||||
|
||||
mock_load.assert_called_once_with(self.ctx, stack=mock.ANY)
|
||||
mock_load.reset_mock()
|
||||
|
||||
stk2 = self._create_stack('stack_snapshot_restore_other_stack_1')
|
||||
mock_load.return_value = stk2
|
||||
|
||||
ex = self.assertRaises(dispatcher.ExpectedException,
|
||||
self.engine.stack_restore,
|
||||
self.ctx, stk2.identifier(),
|
||||
snapshot_id)
|
||||
expected = ('The Snapshot (%(snapshot)s) for Stack (%(stack)s) '
|
||||
'could not be found') % {'snapshot': snapshot_id,
|
||||
'stack': stk2.name}
|
||||
self.assertEqual(exception.SnapshotNotFound, ex.exc_info[0])
|
||||
self.assertIn(expected, six.text_type(ex.exc_info[1]))
|
||||
|
||||
mock_load.assert_called_once_with(self.ctx, stack=mock.ANY)
|
||||
|
|
@ -0,0 +1,59 @@
|
|||
# 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.common import template_format
|
||||
from heat.engine.clients.os import keystone
|
||||
from heat.engine import environment
|
||||
from heat.engine import stack as parser
|
||||
from heat.engine import template as templatem
|
||||
from heat.tests import fakes as test_fakes
|
||||
|
||||
wp_template = '''
|
||||
heat_template_version: 2014-10-16
|
||||
description: WordPress
|
||||
parameters:
|
||||
KeyName:
|
||||
description: KeyName
|
||||
type: string
|
||||
default: test
|
||||
resources:
|
||||
WebServer:
|
||||
type: AWS::EC2::Instance
|
||||
properties:
|
||||
ImageId: F17-x86_64-gold
|
||||
InstanceType: m1.large
|
||||
KeyName: test
|
||||
UserData: wordpress
|
||||
'''
|
||||
|
||||
|
||||
def get_stack(stack_name, ctx, template=None, with_params=True,
|
||||
convergence=False):
|
||||
if template is None:
|
||||
t = template_format.parse(wp_template)
|
||||
if with_params:
|
||||
env = environment.Environment({'KeyName': 'test'})
|
||||
tmpl = templatem.Template(t, env=env)
|
||||
else:
|
||||
tmpl = templatem.Template(t)
|
||||
else:
|
||||
t = template_format.parse(template)
|
||||
tmpl = templatem.Template(t)
|
||||
stack = parser.Stack(ctx, stack_name, tmpl, convergence=convergence)
|
||||
return stack
|
||||
|
||||
|
||||
def setup_keystone_mocks(mocks, stack):
|
||||
fkc = test_fakes.FakeKeystoneClient()
|
||||
|
||||
mocks.StubOutWithMock(keystone.KeystoneClientPlugin, '_create')
|
||||
keystone.KeystoneClientPlugin._create().AndReturn(fkc)
|
||||
|
|
@ -131,6 +131,9 @@ policy_template = '''
|
|||
"Cooldown" : "60",
|
||||
"ScalingAdjustment" : "-1"
|
||||
}
|
||||
},
|
||||
"Random" : {
|
||||
"Type" : "OS::Heat::RandomString"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -3095,6 +3098,62 @@ class StackServiceTest(common.HeatTestCase):
|
|||
self.assertEqual(exception.WatchRuleNotFound, ex.exc_info[0])
|
||||
self.m.VerifyAll()
|
||||
|
||||
def test_signal_calls_metadata_update(self):
|
||||
stack = get_stack('signal_reception', self.ctx, policy_template)
|
||||
self.stack = stack
|
||||
setup_keystone_mocks(self.m, stack)
|
||||
self.m.ReplayAll()
|
||||
stack.store()
|
||||
stack.create()
|
||||
|
||||
self.m.StubOutWithMock(service.EngineService, '_get_stack')
|
||||
s = stack_object.Stack.get_by_id(self.ctx, self.stack.id)
|
||||
service.EngineService._get_stack(self.ctx,
|
||||
self.stack.identifier()).AndReturn(s)
|
||||
|
||||
self.m.StubOutWithMock(res.Resource, 'signal')
|
||||
res.Resource.signal(mox.IgnoreArg()).AndReturn(None)
|
||||
self.m.StubOutWithMock(res.Resource, 'metadata_update')
|
||||
# this will be called once for the Random resource
|
||||
res.Resource.metadata_update().AndReturn(None)
|
||||
self.m.ReplayAll()
|
||||
|
||||
self.eng.resource_signal(self.ctx,
|
||||
dict(self.stack.identifier()),
|
||||
'WebServerScaleDownPolicy', None,
|
||||
sync_call=True)
|
||||
self.m.VerifyAll()
|
||||
self.stack.delete()
|
||||
|
||||
def test_signal_no_calls_metadata_update(self):
|
||||
stack = get_stack('signal_reception', self.ctx, policy_template)
|
||||
self.stack = stack
|
||||
setup_keystone_mocks(self.m, stack)
|
||||
self.m.ReplayAll()
|
||||
stack.store()
|
||||
stack.create()
|
||||
|
||||
res.Resource.signal_needs_metadata_updates = False
|
||||
|
||||
self.m.StubOutWithMock(service.EngineService, '_get_stack')
|
||||
s = stack_object.Stack.get_by_id(self.ctx, self.stack.id)
|
||||
service.EngineService._get_stack(self.ctx,
|
||||
self.stack.identifier()).AndReturn(s)
|
||||
|
||||
self.m.StubOutWithMock(res.Resource, 'signal')
|
||||
res.Resource.signal(mox.IgnoreArg()).AndReturn(None)
|
||||
# this will never be called
|
||||
self.m.StubOutWithMock(res.Resource, 'metadata_update')
|
||||
self.m.ReplayAll()
|
||||
|
||||
self.eng.resource_signal(self.ctx,
|
||||
dict(self.stack.identifier()),
|
||||
'WebServerScaleDownPolicy', None,
|
||||
sync_call=True)
|
||||
self.m.VerifyAll()
|
||||
self.stack.delete()
|
||||
res.Resource.signal_needs_metadata_updates = True
|
||||
|
||||
def test_stack_list_all_empty(self):
|
||||
sl = self.eng.list_stacks(self.ctx)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue