openstack/glance
Code Issues Proposed changes

Cleanup/fixes for Rick review

Browse Source
This commit is contained in:
jaypipes@gmail.com
2010-11-23 13:12:11 -05:00
parent 3516f14efb
commit 0e73f2e0cd
9 changed files with 201 additions and 157 deletions
Download Patch File Download Diff File Expand all files Collapse all files

174
glance/client.py
View File

@@ -32,6 +32,14 @@ from glance.common import exception
#TODO(jaypipes) Raise proper errors or OpenStack API faults
class UnsupportedProtocolError(Exception):
"""
Error resulting from a client connecting to a server
on an unsupported protocol
"""
pass
class ClientConnectionError(Exception):
"""Error resulting from a client connecting to a server"""
pass
@@ -62,20 +70,54 @@ class BaseClient(object):
self.port = kwargs.get('port', self.DEFAULT_PORT)
url = urlparse.urlparse(self.address)
self.netloc = url.netloc
self.protocol = url.scheme
self.connection = None
def do_request(self, method, action, body=None):
"""
Connects to the server and issues a request. Handles converting
any returned HTTP error status codes to OpenStack/Glance exceptions
and closing the server connection. Returns the result data, or
raises an appropriate exception.
:param method: HTTP method ("GET", "POST", "PUT", etc...)
:param action: part of URL after root netloc
:param headers: mapping of headers to send
:param data: string of data to send, or None (default)
"""
try:
self.connection_type = {'http': httplib.HTTPConnection,
'https': httplib.HTTPSConnection}\
[url.scheme]
# Do a quick ping to see if the server is even available...
c = self.connection_type(self.netloc, self.port)
c.connect()
c.close()
connection_type = {'http': httplib.HTTPConnection,
'https': httplib.HTTPSConnection}\
[self.protocol]
except KeyError:
raise ClientConnectionError("Unsupported protocol %s. Unable to "
"connect to server." % url.scheme)
raise UnsupportedProtocolError("Unsupported protocol %s. Unable "
" to connect to server."
% self.protocol)
try:
c = connection_type(self.netloc, self.port)
c.request(method, action, body)
res = c.getresponse()
status_code = self.get_status_code(res)
if status_code == httplib.OK:
return res
elif status_code == httplib.UNAUTHORIZED:
raise exception.NotAuthorized
elif status_code == httplib.FORBIDDEN:
raise exception.NotAuthorized
elif status_code == httplib.NOT_FOUND:
raise exception.NotFound
elif status_code == httplib.CONFLICT:
raise exception.Duplicate
elif status_code == httplib.BAD_REQUEST:
raise BadInputError
else:
raise Exception("Unknown error occurred! %d" % status_code)
except (socket.error, IOError), e:
raise ClientConnectionError("Unable to connect to "
"server. Got error: %s" % e)
finally:
c.close()
def get_status_code(self, response):
"""
@@ -125,113 +167,57 @@ class ParallaxClient(BaseClient):
"""
super(ParallaxClient, self).__init__(**kwargs)
def get_image_index(self):
def get_images(self):
"""
Returns a list of image id/name mappings from Parallax
"""
try:
c = self.connection_type(self.netloc, self.port)
c.request("GET", "images")
res = c.getresponse()
if self.get_status_code(res) == 200:
# Parallax returns a JSONified dict(images=image_list)
data = json.loads(res.read())['images']
return data
else:
logging.warn("Parallax returned HTTP error %d from "
"request for /images", res.status_int)
return []
except (socket.error, IOError), e:
raise ClientConnectionError("Unable to connect to Parallax "
"server. Got error: %s" % e)
finally:
c.close()
res = self.do_request("GET", "images")
data = json.loads(res.read())['images']
return data
def get_image_details(self):
def get_images_detailed(self):
"""
Returns a list of detailed image data mappings from Parallax
"""
try:
c = self.connection_type(self.netloc, self.port)
c.request("GET", "images/detail")
res = c.getresponse()
if self.get_status_code(res) == 200:
# Parallax returns a JSONified dict(images=image_list)
data = json.loads(res.read())['images']
return data
else:
logging.warn("Parallax returned HTTP error %d from "
"request for /images/detail", res.status_int)
return []
finally:
c.close()
res = self.do_request("GET", "images/detail")
data = json.loads(res.read())['images']
return data
def get_image_metadata(self, image_id):
def get_image(self, image_id):
"""
Returns a mapping of image metadata from Parallax
:raises exception.NotFound if image is not in registry
"""
try:
c = self.connection_type(self.netloc, self.port)
c.request("GET", "images/%s" % image_id)
res = c.getresponse()
status_code = self.get_status_code(res)
if status_code == 200:
# Parallax returns a JSONified dict(image=image_info)
data = json.loads(res.read())['image']
return data
elif status_code == 404:
raise exception.NotFound()
finally:
c.close()
res = self.do_request("GET", "images/%s" % image_id)
data = json.loads(res.read())['image']
return data
def add_image_metadata(self, image_metadata):
def add_image(self, image_metadata):
"""
Tells parallax about an image's metadata
"""
try:
c = self.connection_type(self.netloc, self.port)
if 'image' not in image_metadata.keys():
image_metadata = dict(image=image_metadata)
body = json.dumps(image_metadata)
c.request("POST", "images", body)
res = c.getresponse()
status_code = self.get_status_code(res)
if status_code == 200:
# Parallax returns a JSONified dict(image=image_info)
data = json.loads(res.read())
return data['image']['id']
elif status_code == 400:
raise BadInputError("Unable to add metadata to Parallax")
else:
raise RuntimeError("Unknown error code: %d" % status_code)
finally:
c.close()
if 'image' not in image_metadata.keys():
image_metadata = dict(image=image_metadata)
body = json.dumps(image_metadata)
res = self.do_request("POST", "images", body)
# Parallax returns a JSONified dict(image=image_info)
data = json.loads(res.read())
return data['image']['id']
def update_image_metadata(self, image_id, image_metadata):
def update_image(self, image_id, image_metadata):
"""
Updates Parallax's information about an image
"""
try:
if 'image' not in image_metadata.keys():
image_metadata = dict(image=image_metadata)
c = self.connection_type(self.netloc, self.port)
body = json.dumps(image_metadata)
c.request("PUT", "images/%s" % image_id, body)
res = c.getresponse()
return self.get_status_code(res) == 200
finally:
c.close()
if 'image' not in image_metadata.keys():
image_metadata = dict(image=image_metadata)
body = json.dumps(image_metadata)
self.do_request("PUT", "images/%s" % image_id, body)
return True
def delete_image_metadata(self, image_id):
def delete_image(self, image_id):
"""
Deletes Parallax's information about an image
"""
try:
c = self.connection_type(self.netloc, self.port)
c.request("DELETE", "images/%s" % image_id)
res = c.getresponse()
return self.get_status_code(res) == 200
finally:
c.close()
self.do_request("DELETE", "images/%s" % image_id)
return True

2
glance/common/exception.py
View File

@@ -83,5 +83,3 @@ def wrap_exception(f):
raise
_wrap.func_name = f.func_name
return _wrap

9
glance/parallax/controllers.py
View File

@@ -108,6 +108,8 @@ class ImageController(wsgi.Controller):
try:
new_image = db.image_create(context, image_data)
return dict(image=new_image)
except exception.Duplicate:
return exc.HTTPConflict()
except exception.Invalid:
return exc.HTTPBadRequest()
@@ -146,13 +148,14 @@ class ImageController(wsgi.Controller):
# TODO(sirp): should this be a dict, or a list of dicts?
# A plain dict is more convenient, but list of dicts would provide
# access to created_at, etc
metadata = dict((m['key'], m['value']) for m in image['metadata']
if not m['deleted'])
properties = dict((p['key'], p['value'])
for p in image['properties']
if not p['deleted'])
image_dict = _fetch_attrs(image, db.IMAGE_ATTRS)
image_dict['files'] = files
image_dict['metadata'] = metadata
image_dict['properties'] = properties
return image_dict

7
glance/parallax/db/api.py
View File

@@ -86,7 +86,6 @@ def image_file_create(context, values):
###################
def image_metadatum_create(context, values):
"""Create an image metadatum from the values dictionary."""
return IMPL.image_metadatum_create(context, values)
def image_property_create(context, values):
"""Create an image property from the values dictionary."""
return IMPL.image_property_create(context, values)

17
glance/parallax/db/sqlalchemy/api.py
View File

@@ -70,7 +70,7 @@ def image_get(context, image_id):
try:
return session.query(models.Image
).options(joinedload(models.Image.files)
).options(joinedload(models.Image.metadata)
).options(joinedload(models.Image.properties)
).filter_by(deleted=_deleted(context)
).filter_by(id=image_id
).one()
@@ -83,7 +83,7 @@ def image_get_all(context):
session = get_session()
return session.query(models.Image
).options(joinedload(models.Image.files)
).options(joinedload(models.Image.metadata)
).options(joinedload(models.Image.properties)
).filter_by(deleted=_deleted(context)
).all()
@@ -92,7 +92,7 @@ def image_get_all_public(context, public):
session = get_session()
return session.query(models.Image
).options(joinedload(models.Image.files)
).options(joinedload(models.Image.metadata)
).options(joinedload(models.Image.properties)
).filter_by(deleted=_deleted(context)
).filter_by(is_public=public
).all()
@@ -124,10 +124,9 @@ def image_file_create(_context, values):
###################
def image_metadatum_create(_context, values):
image_metadatum_ref = models.ImageMetadatum()
def image_property_create(_context, values):
image_properties_ref = models.ImageProperty()
for (key, value) in values.iteritems():
image_metadatum_ref[key] = value
image_metadatum_ref.save()
return image_metadatum_ref
image_properties_ref[key] = value
image_properties_ref.save()
return image_properties_ref

15
glance/parallax/db/sqlalchemy/models.py
View File

@@ -149,7 +149,7 @@ class Image(BASE, ModelBase):
raise exception.Invalid("Invalid status '%s' for image." % status)
return status
# TODO(sirp): should these be stored as metadata?
# TODO(sirp): should these be stored as properties?
#user_id = Column(String(255))
#project_id = Column(String(255))
#arch = Column(String(255))
@@ -179,22 +179,22 @@ class ImageFile(BASE, ModelBase):
size = Column(Integer)
class ImageMetadatum(BASE, ModelBase):
"""Represents an image metadata in the datastore"""
__tablename__ = 'image_metadata'
__prefix__ = 'img-meta'
class ImageProperty(BASE, ModelBase):
"""Represents an image properties in the datastore"""
__tablename__ = 'image_properties'
__prefix__ = 'img-prop'
__table_args__ = (UniqueConstraint('image_id', 'key'), {})
id = Column(Integer, primary_key=True)
image_id = Column(Integer, ForeignKey('images.id'), nullable=False)
image = relationship(Image, backref=backref('metadata'))
image = relationship(Image, backref=backref('properties'))
key = Column(String(255), index=True)
value = Column(Text)
def register_models():
"""Register Models and create metadata"""
"""Register Models and create properties"""
engine = get_engine()
BASE.metadata.create_all(engine)
@@ -203,4 +203,3 @@ def unregister_models():
"""Unregister Models, useful clearing out data before testing"""
engine = get_engine()
BASE.metadata.drop_all(engine)

19
tests/stubs.py
View File

@@ -226,7 +226,7 @@ def stub_out_parallax_db_image_api(stubs):
'deleted_at': None,
'deleted': False,
'files': [],
'metadata': []},
'properties': []},
{'id': 2,
'name': 'fake image #2',
'status': 'available',
@@ -237,7 +237,7 @@ def stub_out_parallax_db_image_api(stubs):
'deleted_at': None,
'deleted': False,
'files': [],
'metadata': []}]
'properties': []}]
VALID_STATUSES = ('available', 'disabled', 'pending')
@@ -246,7 +246,12 @@ def stub_out_parallax_db_image_api(stubs):
self.next_id = 3
def image_create(self, _context, values):
values['id'] = self.next_id
values['id'] = values.get('id', self.next_id)
if values['id'] in [image['id'] for image in self.images]:
raise exception.Duplicate("Duplicate image id: %s" %
values['id'])
if 'status' not in values.keys():
values['status'] = 'available'
@@ -257,10 +262,16 @@ def stub_out_parallax_db_image_api(stubs):
values['deleted'] = False
values['files'] = values.get('files', [])
values['metadata'] = values.get('metadata', [])
values['properties'] = values.get('properties', [])
values['created_at'] = datetime.datetime.utcnow()
values['updated_at'] = datetime.datetime.utcnow()
values['deleted_at'] = None
for p in values['properties']:
p['deleted'] = False
p['created_at'] = datetime.datetime.utcnow()
p['updated_at'] = datetime.datetime.utcnow()
p['deleted_at'] = None
self.next_id += 1
self.images.append(values)

99
tests/test_clients.py → tests/unit/test_clients.py
View File

@@ -30,15 +30,19 @@ class TestBadClients(unittest.TestCase):
"""Test exceptions raised for bad clients"""
def test_bad_protocol(self):
"""Test unsupported protocol raised"""
c = client.ParallaxClient(address="hdsa://127.012..1./")
self.assertRaises(client.UnsupportedProtocolError,
c.get_image,
1)
def test_bad_address(self):
"""Test bad connection credentials produces exception"""
"""Test unsupported protocol raised"""
c = client.ParallaxClient(address="http://127.999.1.1/")
self.assertRaises(client.ClientConnectionError,
client.ParallaxClient,
address="hdsa://127.012..1./")
self.assertRaises(client.ClientConnectionError,
client.ParallaxClient,
address="http://127.0.0.1/",
port="")
c.get_image,
1)
class TestParallaxClient(unittest.TestCase):
@@ -60,7 +64,7 @@ class TestParallaxClient(unittest.TestCase):
"""Test correct set of public image returned"""
fixture = {'id': 2,
'name': 'fake image #2'}
images = self.client.get_image_index()
images = self.client.get_images()
self.assertEquals(len(images), 1)
for k,v in fixture.iteritems():
@@ -75,7 +79,7 @@ class TestParallaxClient(unittest.TestCase):
'status': 'available'
}
images = self.client.get_image_details()
images = self.client.get_images_detailed()
self.assertEquals(len(images), 1)
for k,v in fixture.iteritems():
@@ -90,32 +94,32 @@ class TestParallaxClient(unittest.TestCase):
'status': 'available'
}
data = self.client.get_image_metadata(2)
data = self.client.get_image(2)
for k,v in fixture.iteritems():
self.assertEquals(v, data[k])
def test_get_image_metadata_non_existing(self):
def test_get_image_non_existing(self):
"""Tests that NotFound is raised when getting a non-existing image"""
self.assertRaises(exception.NotFound,
self.client.get_image_metadata,
self.client.get_image,
42)
def test_add_image_metadata(self):
def test_add_image_metadata_basic(self):
"""Tests that we can add image metadata and returns the new id"""
fixture = {'name': 'fake public image',
'is_public': True,
'image_type': 'kernel'
}
new_id = self.client.add_image_metadata(fixture)
new_id = self.client.add_image(fixture)
# Test ID auto-assigned properly
self.assertEquals(3, new_id)
# Test all other attributes set
data = self.client.get_image_metadata(3)
data = self.client.get_image(3)
for k,v in fixture.iteritems():
self.assertEquals(v, data[k])
@@ -124,7 +128,49 @@ class TestParallaxClient(unittest.TestCase):
self.assertTrue('status' in data.keys())
self.assertEquals('available', data['status'])
def test_create_image_with_bad_status(self):
def test_add_image_metadata_with_properties(self):
"""Tests that we can add image metadata with properties"""
fixture = {'name': 'fake public image',
'is_public': True,
'image_type': 'kernel',
'properties': [{'key':'disco',
'value': 'baby'}]
}
expected = {'name': 'fake public image',
'is_public': True,
'image_type': 'kernel',
'properties': {'disco': 'baby'}
}
new_id = self.client.add_image(fixture)
# Test ID auto-assigned properly
self.assertEquals(3, new_id)
# Test all other attributes set
data = self.client.get_image(3)
for k,v in expected.iteritems():
self.assertEquals(v, data[k])
# Test status was updated properly
self.assertTrue('status' in data.keys())
self.assertEquals('available', data['status'])
def test_add_image_already_exists(self):
"""Tests proper exception is raised if image with ID already exists"""
fixture = {'id': 2,
'name': 'fake public image',
'is_public': True,
'image_type': 'kernel',
'status': 'bad status'
}
self.assertRaises(exception.Duplicate,
self.client.add_image,
fixture)
def test_add_image_with_bad_status(self):
"""Tests proper exception is raised if a bad status is set"""
fixture = {'id': 3,
'name': 'fake public image',
@@ -134,7 +180,7 @@ class TestParallaxClient(unittest.TestCase):
}
self.assertRaises(client.BadInputError,
self.client.add_image_metadata,
self.client.add_image,
fixture)
def test_update_image(self):
@@ -143,10 +189,10 @@ class TestParallaxClient(unittest.TestCase):
'image_type': 'ramdisk'
}
self.assertTrue(self.client.update_image_metadata(2, fixture))
self.assertTrue(self.client.update_image(2, fixture))
# Test all other attributes set
data = self.client.get_image_metadata(2)
data = self.client.get_image(2)
for k,v in fixture.iteritems():
self.assertEquals(v, data[k])
@@ -160,23 +206,28 @@ class TestParallaxClient(unittest.TestCase):
'status': 'bad status'
}
self.assertFalse(self.client.update_image_metadata(3, fixture))
self.assertRaises(exception.NotFound,
self.client.update_image,
3,
fixture)
def test_delete_image(self):
"""Tests that image metadata is deleted properly"""
# Grab the original number of images
orig_num_images = len(self.client.get_image_index())
orig_num_images = len(self.client.get_images())
# Delete image #2
self.assertTrue(self.client.delete_image_metadata(2))
self.assertTrue(self.client.delete_image(2))
# Verify one less image
new_num_images = len(self.client.get_image_index())
new_num_images = len(self.client.get_images())
self.assertEquals(new_num_images, orig_num_images - 1)
def test_delete_image_not_existing(self):
"""Tests cannot delete non-existing image"""
self.assertFalse(self.client.delete_image_metadata(3))
self.assertRaises(exception.NotFound,
self.client.delete_image,
3)

16
tests/unit/test_parallax_models.py
View File

@@ -40,20 +40,20 @@ class TestModels(unittest.TestCase):
same key
"""
self._make_metadatum(self.image, key="spam", value="eggs")
self._make_property(self.image, key="spam", value="eggs")
second_image = self._make_image(id=3, name='fake image #3')
self._make_metadatum(second_image, key="spam", value="eggs")
self._make_property(second_image, key="spam", value="eggs")
def test_metadata_key_constraint_bad(self):
"""The same image cannot have two distinct pieces of metadata with the
same key.
"""
self._make_metadatum(self.image, key="spam", value="eggs")
self._make_property(self.image, key="spam", value="eggs")
self.assertRaises(sa_exc.IntegrityError,
self._make_metadatum, self.image, key="spam", value="eggs")
self._make_property, self.image, key="spam", value="eggs")
def _make_image(self, id, name):
"""Convenience method to create an image with a given name and id"""
@@ -67,11 +67,9 @@ class TestModels(unittest.TestCase):
image = db.api.image_create(context, fixture)
return image
def _make_metadatum(self, image, key, value):
def _make_property(self, image, key, value):
"""Convenience method to create metadata attached to an image"""
metadata = {'image_id': image['id'], 'key': key, 'value': value}
context = None
metadatum = db.api.image_metadatum_create(context, metadata)
return metadatum
property = db.api.image_property_create(context, metadata)
return property