openstack/heat
Code Issues Proposed changes

Remove handling for client status races

Browse Source 
Now that we guarantee that resources are UPDATE_IN_PROGRESS before the
stack update call returns for legacy stacks (this was already true for
convergence stacks), there is no need to have special handling to check the
updated_time either in StackResources or in functional tests.

Change-Id: I5bf7ed6cb9ba6c77a77dd36eb173e1927065c53e
Story: #1669608
Task: 23176
This commit is contained in:
Zane Bitter 2018-07-31 19:35:08 -04:00 committed by David Peacock
parent 28ce9f3ad4
commit 6b7d64988c
4 changed files with 7 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
heat/engine/resources/stack_resource.py
View File

@ -425,26 +425,6 @@ class StackResource(resource.Resource):
if action != expected_action:
return False
# Has the action really started?
#
# The rpc call to update does not guarantee that the stack will be
# placed into IN_PROGRESS by the time it returns (it runs stack.update
# in a thread) so you could also have a situation where we get into
# this method and the update hasn't even started.
#
# So we are using a mixture of state (action+status) and updated_at
# to see if the action has actually progressed.
# - very fast updates (like something with one RandomString) we will
# probably miss the state change, but we should catch the updated_at.
# - very slow updates we won't see the updated_at for quite a while,
# but should see the state change.
if cookie is not None:
prev_state = cookie['previous']['state']
prev_updated_at = cookie['previous']['updated_at']
if (prev_updated_at == updated_time and
prev_state == (action, status)):
return False
if status == self.IN_PROGRESS:
return False
elif status == self.COMPLETE:
@ -534,9 +514,6 @@ class StackResource(resource.Resource):
action, status, status_reason, updated_time = status_data
kwargs = self._stack_kwargs(user_params, child_template)
cookie = {'previous': {
'updated_at': updated_time,
'state': (action, status)}}
kwargs.update({
'stack_identity': dict(self.nested_identifier()),
@ -550,7 +527,6 @@ class StackResource(resource.Resource):
with excutils.save_and_reraise_exception():
raw_template.RawTemplate.delete(self.context,
kwargs['template_id'])
return cookie
def check_update_complete(self, cookie=None):
if cookie is not None and 'target_action' in cookie:

17
heat/tests/test_stack_resource.py
View File

@ -841,23 +841,6 @@ class StackResourceCheckCompleteTest(StackResourceBaseTest):
self.mock_status.assert_called_once_with(
self.parent_resource.context, self.parent_resource.resource_id)
def test_update_not_started(self):
if self.action != 'update':
# only valid for updates at the moment.
return
self.status[1] = 'COMPLETE'
self.status[3] = 'test'
cookie = {'previous': {'state': ('UPDATE', 'COMPLETE'),
'updated_at': 'test'}}
complete = getattr(self.parent_resource,
'check_%s_complete' % self.action)
self.assertFalse(complete(cookie=cookie))
self.mock_status.assert_called_once_with(
self.parent_resource.context, self.parent_resource.resource_id)
def test_wrong_action(self):
self.status[0] = 'COMPLETE'
complete = getattr(self.parent_resource,

39
heat_integrationtests/common/test.py
View File

@ -94,7 +94,6 @@ class HeatIntegrationTest(testscenarios.WithScenarios,
'No password configured')
self.setup_clients(self.conf)
self.useFixture(fixtures.FakeLogger(format=_LOG_FORMAT))
self.updated_time = {}
if self.conf.disable_ssl_certificate_validation:
self.verify_cert = False
else:
@ -272,17 +271,7 @@ class HeatIntegrationTest(testscenarios.WithScenarios,
def _verify_status(self, stack, stack_identifier, status,
fail_regexp, is_action_cancelled=False):
if stack.stack_status == status:
# Handle UPDATE_COMPLETE/FAILED case: Make sure we don't
# wait for a stale UPDATE_COMPLETE/FAILED status.
if status in ('UPDATE_FAILED', 'UPDATE_COMPLETE'):
if is_action_cancelled:
return True
if self.updated_time.get(
stack_identifier) != stack.updated_time:
self.updated_time[stack_identifier] = stack.updated_time
return True
elif status == 'DELETE_COMPLETE' and stack.deletion_time is None:
if status == 'DELETE_COMPLETE' and stack.deletion_time is None:
# Wait for deleted_time to be filled, so that we have more
# confidence the operation is finished.
return False
@ -292,20 +281,12 @@ class HeatIntegrationTest(testscenarios.WithScenarios,
wait_for_action = status.split('_')[0]
if (stack.action == wait_for_action and
fail_regexp.search(stack.stack_status)):
# Handle UPDATE_COMPLETE/UPDATE_FAILED case.
if status in ('UPDATE_FAILED', 'UPDATE_COMPLETE'):
if self.updated_time.get(
stack_identifier) != stack.updated_time:
self.updated_time[stack_identifier] = stack.updated_time
raise exceptions.StackBuildErrorException(
stack_identifier=stack_identifier,
stack_status=stack.stack_status,
stack_status_reason=stack.stack_status_reason)
else:
raise exceptions.StackBuildErrorException(
stack_identifier=stack_identifier,
stack_status=stack.stack_status,
stack_status_reason=stack.stack_status_reason)
raise exceptions.StackBuildErrorException(
stack_identifier=stack_identifier,
stack_status=stack.stack_status,
stack_status_reason=stack.stack_status_reason)
return False
def _wait_for_stack_status(self, stack_identifier, status,
failure_pattern=None,
@ -395,9 +376,6 @@ class HeatIntegrationTest(testscenarios.WithScenarios,
env_files = files or {}
parameters = parameters or {}
self.updated_time[stack_identifier] = self.client.stacks.get(
stack_identifier, resolve_outputs=False).updated_time
self._handle_in_progress(
self.client.stacks.update,
stack_id=stack_identifier,
@ -423,9 +401,6 @@ class HeatIntegrationTest(testscenarios.WithScenarios,
stack_name = stack_identifier.split('/')[0]
self.updated_time[stack_identifier] = self.client.stacks.get(
stack_identifier, resolve_outputs=False).updated_time
if rollback:
self.client.actions.cancel_update(stack_name)
else:

10
heat_integrationtests/functional/test_update_restricted.py
View File

@ -11,7 +11,6 @@
# under the License.
import copy
import time
from heat_integrationtests.functional import functional_base
@ -74,9 +73,6 @@ class UpdateRestrictedStackTest(functional_base.FunctionalTestsBase):
self._check_for_restriction_reason(resource_events,
reason_update_restrict))
# Ensure the timestamp changes, since this will be very quick
time.sleep(1)
# check update succeeds - with only 'replace' restricted
self.update_stack(stack_identifier, update_template,
env_replace_restrict,
@ -112,9 +108,6 @@ class UpdateRestrictedStackTest(functional_base.FunctionalTestsBase):
self._check_for_restriction_reason(resource_events,
reason_replace_restrict))
# Ensure the timestamp changes, since this will be very quick
time.sleep(1)
# check replace fails - with only 'replace' restricted
self.update_stack(stack_identifier, update_template,
env_replace_restrict,
@ -149,9 +142,6 @@ class UpdateRestrictedStackTest(functional_base.FunctionalTestsBase):
self._check_for_restriction_reason(resource_events,
reason_replace_restrict))
# Ensure the timestamp changes, since this will be very quick
time.sleep(1)
# check replace fails - with only 'replace' restricted
self.update_stack(stack_identifier, update_template,
env_replace_restrict,