openstack
/
heat
Code Issues Proposed changes

Pass data from handle_create() to check_active() 

All resources that implement check_active() will require some state to be
retained from the call to handle_create(). Saving this as state in the
Resource object results in repeated, ugly, and potentially error-prone
code.

Instead, allow a subclass-defined state object returned from
handle_create() to be passed to check_active(). This ensures that the state
is limited in scope to where it is meaningful (during the create
operation), and that it will be garbage-collected at the appropriate time,
even if an unexpected exception occurs e.g. because a thread is cancelled.

Change-Id: I9d690b44a066aaf33970562a2b9a55c633a7d4e8
This commit is contained in:
Zane Bitter 2013-04-09 20:01:23 +02:00
parent 092222de42
commit bfdee58d2e
6 changed files with 25 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
heat/engine/resource.py
View File

@ -316,9 +316,10 @@ class Resource(object):
try:
self.properties.validate()
self.state_set(self.CREATE_IN_PROGRESS)
create_data = None
if callable(getattr(self, 'handle_create', None)):
self.handle_create()
while not self.check_active():
create_data = self.handle_create()
while not self.check_active(create_data):
eventlet.sleep(1)
except greenlet.GreenletExit:
raise
@ -329,12 +330,13 @@ class Resource(object):
else:
self.state_set(self.CREATE_COMPLETE)
def check_active(self):
def check_active(self, create_data):
'''
Check if the resource is active (ready to move to the CREATE_COMPLETE
state). By default this happens as soon as the handle_create() method
has completed successfully, but subclasses may customise this by
overriding this function.
overriding this function. The return value of handle_create() is
passed in to this function each time it is called.
'''
return True

6
heat/engine/resources/autoscaling.py
View File

@ -82,7 +82,7 @@ class InstanceGroup(resource.Resource):
def handle_create(self):
self.resize(int(self.properties['Size']), raise_on_error=True)
def check_active(self):
def check_active(self, create_data=None):
active = all(i.check_active(override=False) for i in self._activating)
if active:
self._activating = []
@ -143,14 +143,14 @@ class InstanceGroup(resource.Resource):
def state_set(self, new_state, reason="state changed"):
self._store_or_update(new_state, reason)
def check_active(self, override=True):
def check_active(self, create_data=None, override=True):
'''
By default, report that the instance is active so that we
won't wait for it in create().
'''
if override:
return True
return super(GroupedInstance, self).check_active()
return super(GroupedInstance, self).check_active(create_data)
conf = self.properties['LaunchConfigurationName']
instance_definition = self.stack.t['Resources'][conf]

2
heat/engine/resources/instance.py
View File

@ -313,7 +313,7 @@ class Instance(resource.Resource):
self._server_status = server.status
def check_active(self):
def check_active(self, create_data=None):
if self._server_status == 'ACTIVE':
return True

8
heat/tests/test_autoscaling.py
View File

@ -86,13 +86,13 @@ class AutoScalingTest(unittest.TestCase):
def _stub_create(self, num):
self.m.StubOutWithMock(eventlet, 'sleep')
self.m.StubOutWithMock(instance.Instance, 'create')
self.m.StubOutWithMock(instance.Instance, 'handle_create')
self.m.StubOutWithMock(instance.Instance, 'check_active')
for x in range(num):
instance.Instance.create().AndReturn(None)
instance.Instance.check_active().AndReturn(False)
instance.Instance.handle_create().AndReturn(None)
instance.Instance.check_active(None).AndReturn(False)
eventlet.sleep(mox.IsA(int)).AndReturn(None)
instance.Instance.check_active().MultipleTimes().AndReturn(True)
instance.Instance.check_active(None).MultipleTimes().AndReturn(True)
def _stub_lb_reload(self, expected_list, unset=True):
if unset:

16
heat/tests/test_instance_group.py
View File

@ -23,6 +23,7 @@ from nose.plugins.attrib import attr
from heat.tests.v1_1 import fakes
from heat.common import context
from heat.common import exception
from heat.common import template_format
from heat.engine.resources import autoscaling as asc
from heat.engine.resources import instance
@ -65,13 +66,13 @@ class InstanceGroupTest(unittest.TestCase):
def _stub_create(self, num):
self.m.StubOutWithMock(eventlet, 'sleep')
self.m.StubOutWithMock(instance.Instance, 'create')
self.m.StubOutWithMock(instance.Instance, 'handle_create')
self.m.StubOutWithMock(instance.Instance, 'check_active')
for x in range(num):
instance.Instance.create().AndReturn(None)
instance.Instance.check_active().AndReturn(False)
instance.Instance.handle_create().AndReturn(None)
instance.Instance.check_active(None).AndReturn(False)
eventlet.sleep(mox.IsA(int)).AndReturn(None)
instance.Instance.check_active().MultipleTimes().AndReturn(True)
instance.Instance.check_active(None).MultipleTimes().AndReturn(True)
def create_instance_group(self, t, stack, resource_name):
resource = asc.InstanceGroup(resource_name,
@ -113,12 +114,13 @@ class InstanceGroupTest(unittest.TestCase):
t['Resources']['JobServerGroup'],
stack)
self.m.StubOutWithMock(instance.Instance, 'create')
instance.Instance.create().AndReturn('ImageNotFound: bla')
self.m.StubOutWithMock(instance.Instance, 'handle_create')
not_found = exception.ImageNotFound(image_name='bla')
instance.Instance.handle_create().AndRaise(not_found)
self.m.ReplayAll()
self.assertEqual(resource.create(), 'ImageNotFound: bla')
self.assertNotEqual(resource.create(), None)
self.assertEqual(asc.InstanceGroup.CREATE_FAILED, resource.state)
self.m.VerifyAll()

4
heat/tests/test_metadata_refresh.py
View File

@ -152,7 +152,7 @@ class MetadataRefreshTest(unittest.TestCase):
self.m.StubOutWithMock(instance.Instance, 'handle_create')
self.m.StubOutWithMock(instance.Instance, 'check_active')
instance.Instance.handle_create().AndReturn(None)
instance.Instance.check_active().AndReturn(True)
instance.Instance.check_active(None).AndReturn(True)
self.m.StubOutWithMock(instance.Instance, 'FnGetAtt')
return stack
@ -214,7 +214,7 @@ class WaitCondMetadataUpdateTest(unittest.TestCase):
self.m.StubOutWithMock(instance.Instance, 'handle_create')
self.m.StubOutWithMock(instance.Instance, 'check_active')
instance.Instance.handle_create().MultipleTimes().AndReturn(None)
instance.Instance.check_active().AndReturn(True)
instance.Instance.check_active(None).AndReturn(True)
self.m.StubOutWithMock(wc.WaitConditionHandle, 'keystone')
wc.WaitConditionHandle.keystone().MultipleTimes().AndReturn(self.fc)