diff --git a/glance/client.py b/glance/client.py index c685f51e81..5f09a0fa96 100644 --- a/glance/client.py +++ b/glance/client.py @@ -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 diff --git a/glance/common/exception.py b/glance/common/exception.py index b8894758ff..f00c27b34f 100644 --- a/glance/common/exception.py +++ b/glance/common/exception.py @@ -83,5 +83,3 @@ def wrap_exception(f): raise _wrap.func_name = f.func_name return _wrap - - diff --git a/glance/parallax/controllers.py b/glance/parallax/controllers.py index eb46f653a7..8dcfc52804 100644 --- a/glance/parallax/controllers.py +++ b/glance/parallax/controllers.py @@ -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 diff --git a/glance/parallax/db/api.py b/glance/parallax/db/api.py index 383dcb2184..82ee50a03a 100644 --- a/glance/parallax/db/api.py +++ b/glance/parallax/db/api.py @@ -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) diff --git a/glance/parallax/db/sqlalchemy/api.py b/glance/parallax/db/sqlalchemy/api.py index e13eddf1a9..5c658b51d7 100644 --- a/glance/parallax/db/sqlalchemy/api.py +++ b/glance/parallax/db/sqlalchemy/api.py @@ -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 diff --git a/glance/parallax/db/sqlalchemy/models.py b/glance/parallax/db/sqlalchemy/models.py index a5a10a4bb5..25e1141e8c 100644 --- a/glance/parallax/db/sqlalchemy/models.py +++ b/glance/parallax/db/sqlalchemy/models.py @@ -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) - diff --git a/tests/stubs.py b/tests/stubs.py index e9e7efda00..7cdff1639d 100644 --- a/tests/stubs.py +++ b/tests/stubs.py @@ -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) diff --git a/tests/test_clients.py b/tests/unit/test_clients.py similarity index 61% rename from tests/test_clients.py rename to tests/unit/test_clients.py index d95bb3ac74..347ca57e8d 100644 --- a/tests/test_clients.py +++ b/tests/unit/test_clients.py @@ -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) diff --git a/tests/unit/test_parallax_models.py b/tests/unit/test_parallax_models.py index 3439ad2aeb..4d7b476649 100644 --- a/tests/unit/test_parallax_models.py +++ b/tests/unit/test_parallax_models.py @@ -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