openstack/openstacksdk
Code Issues Proposed changes

Have delete_server use the timed server list cache

Browse Source 
Waiting for server deletion is just as costly as waiting for creation.
Make sure that delete honors the 5-second cache if it's in use by using
get_server() instead of get_server_by_id().

Using the get_server() call identified a bug in the volume cache
invalidation that is done in the code immediately after the call, so we
fix that here too, and add a new cache_enabled attribute to allow us
to avoid an unnecessary API call.

Co-Authored-By: David Shrewsbury <shrewsbury.dave@gmail.com>

Change-Id: I70ccfffe6cbb1f46049b3525bba35c612a572ef0
This commit is contained in:
Monty Taylor 2016-04-19 10:42:20 -05:00 committed by David Shrewsbury
parent 59a2afd188
commit 04fd7dbd73
2 changed files with 19 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
shade/openstackcloud.py
View File

@ -195,6 +195,7 @@ class OpenStackCloud(object):
cache_arguments = cloud_config.get_cache_arguments()
if cache_class != 'dogpile.cache.null':
self.cache_enabled = True
self._cache = cache.make_region(
function_key_generator=self._make_cache_key
).configure(
@ -204,6 +205,8 @@ class OpenStackCloud(object):
self._SERVER_AGE = DEFAULT_SERVER_AGE
self._PORT_AGE = DEFAULT_PORT_AGE
else:
self.cache_enabled = False
def _fake_invalidate(unused):
pass
@ -4045,28 +4048,27 @@ class OpenStackCloud(object):
if not wait:
return True
# If the server has volume attachments, or if it has booted
# from volume, deleting it will change volume state so we will
# need to invalidate the cache. Avoid the extra API call if
# caching is not enabled.
reset_volume_cache = False
if (self.cache_enabled
and self.has_service('volume')
and self.get_volumes(server)):
reset_volume_cache = True
for count in _utils._iterate_timeout(
timeout,
"Timed out waiting for server to get deleted.",
wait=self._SERVER_AGE):
try:
server = self.get_server_by_id(server['id'])
with _utils.shade_exceptions("Error in deleting server"):
server = self.get_server(server['id'])
if not server:
break
except nova_exceptions.NotFound:
break
except OpenStackCloudException:
raise
except Exception as e:
raise OpenStackCloudException(
"Error in deleting server: {0}".format(e))
if self.has_service('volume'):
# If the server has volume attachments, or if it has booted
# from volume, deleting it will change volume state
if (not server['image'] or not server['image']['id']
or self.get_volume(server)):
self.list_volumes.invalidate(self)
if reset_volume_cache:
self.list_volumes.invalidate(self)
# Reset the list servers cache time so that the next list server
# call gets a new list

4
shade/tests/unit/test_delete_server.py
View File

@ -123,10 +123,10 @@ class TestDeleteServer(base.TestCase):
'ACTIVE')]
for fail in self.novaclient_exceptions:
def _raise_fail(server):
def _raise_fail():
raise fail(code=fail.http_status)
nova_mock.servers.get.side_effect = _raise_fail
nova_mock.servers.list.side_effect = _raise_fail
exc = self.assertRaises(shade_exc.OpenStackCloudException,
self.cloud.delete_server, 'yosemite',
wait=True)