Merge lp:~anso/nova/rename into lp:~hudson-openstack/nova/trunk

[Todd Willey]

Add user-editable name & notes/description to volumes, instances, and images.

Add user-editable name & notes/description to volumes, instances, and images.

[Jesse Andrews (anotherjesse)]

merge conflict.

plus we should figure out how to add the support to the RS API as well? Maybe ask Gundlach?

[Todd Willey (xtoddx)]

fixed conflicts, invited ~gundlach to review so we can discuss here

[Michael Gundlach (gundlach)]

images are readonly in RS API till post-Austin.

i don't see why RS API won't be able to use your update_instance() as it stands.

[Michael Gundlach (gundlach)]

(and RS API has no concept of volumes, which i think is the third and last thing you're touching in this branch.)

[Devin Carlen (devcamcar)]

lgtm, RS API issues not withstanding

review: Approve

[Vish Ishaya (vishvananda)]

Fixed one test. Looks good.

review: Approve

[OpenStack Infra (hudson-openstack)]

Attempt to merge lp:~anso/nova/rename into lp:nova failed due to merge conflicts:

text conflict in nova/tests/cloud_unittest.py

[lp:~anso/nova/rename]

287. By Todd Willey on 2010-09-29

Merge trunk and fix test.

=== modified file 'nova/api/ec2/__init__.py'
--- nova/api/ec2/__init__.py	2010-09-23 21:06:23 +0000
+++ nova/api/ec2/__init__.py	2010-09-29 03:43:40 +0000
@@ -158,12 +158,14 @@
                 'RunInstances': ['projectmanager', 'sysadmin'],
                 'TerminateInstances': ['projectmanager', 'sysadmin'],
                 'RebootInstances': ['projectmanager', 'sysadmin'],
+                'UpdateInstance': ['projectmanager', 'sysadmin'],
                 'DeleteVolume': ['projectmanager', 'sysadmin'],
                 'DescribeImages': ['all'],
                 'DeregisterImage': ['projectmanager', 'sysadmin'],
                 'RegisterImage': ['projectmanager', 'sysadmin'],
                 'DescribeImageAttribute': ['all'],
                 'ModifyImageAttribute': ['projectmanager', 'sysadmin'],
+                'UpdateImage': ['projectmanager', 'sysadmin'],
             },
             'AdminController': {
                 # All actions have the same permission: ['none'] (the default)

=== modified file 'nova/api/ec2/cloud.py'
--- nova/api/ec2/cloud.py	2010-09-28 23:54:57 +0000
+++ nova/api/ec2/cloud.py	2010-09-29 03:43:40 +0000
@@ -285,6 +285,9 @@
                                    'volume_id': volume['ec2_id']}]
         else:
             v['attachmentSet'] = [{}]
+
+        v['display_name'] = volume['display_name']
+        v['display_description'] = volume['display_description']
         return v
 
     def create_volume(self, context, size, **kwargs):
@@ -302,6 +305,8 @@
         vol['availability_zone'] = FLAGS.storage_availability_zone
         vol['status'] = "creating"
         vol['attach_status'] = "detached"
+        vol['display_name'] = kwargs.get('display_name')
+        vol['display_description'] = kwargs.get('display_description')
         volume_ref = db.volume_create(context, vol)
 
         rpc.cast(FLAGS.scheduler_topic,
@@ -368,6 +373,16 @@
             lst = [lst]
         return [{label: x} for x in lst]
 
+    def update_volume(self, context, volume_id, **kwargs):
+        updatable_fields = ['display_name', 'display_description']
+        changes = {}
+        for field in updatable_fields:
+            if field in kwargs:
+                changes[field] = kwargs[field]
+        if changes:
+            db.volume_update(context, volume_id, kwargs)
+        return True
+
     def describe_instances(self, context, **kwargs):
         return self._format_describe_instances(context)
 
@@ -420,6 +435,8 @@
             i['instanceType'] = instance['instance_type']
             i['launchTime'] = instance['created_at']
             i['amiLaunchIndex'] = instance['launch_index']
+            i['displayName'] = instance['display_name']
+            i['displayDescription'] = instance['display_description']
             if not reservations.has_key(instance['reservation_id']):
                 r = {}
                 r['reservationId'] = instance['reservation_id']
@@ -577,6 +594,8 @@
         base_options['user_data'] = kwargs.get('user_data', '')
         base_options['security_group'] = security_group
         base_options['instance_type'] = instance_type
+        base_options['display_name'] = kwargs.get('display_name')
+        base_options['display_description'] = kwargs.get('display_description')
 
         type_data = INSTANCE_TYPES[instance_type]
         base_options['memory_mb'] = type_data['memory_mb']
@@ -673,6 +692,18 @@
                                "instance_id": instance_ref['id']}})
         return True
 
+    def update_instance(self, context, instance_id, **kwargs):
+        updatable_fields = ['display_name', 'display_description']
+        changes = {}
+        for field in updatable_fields:
+            if field in kwargs:
+                changes[field] = kwargs[field]
+        if changes:
+            db_context = {}
+            inst = db.instance_get_by_ec2_id(db_context, instance_id)
+            db.instance_update(db_context, inst['id'], kwargs)
+        return True
+
     def delete_volume(self, context, volume_id, **kwargs):
         # TODO: return error if not authorized
         volume_ref = db.volume_get_by_ec2_id(context, volume_id)
@@ -728,3 +759,7 @@
         if not operation_type in ['add', 'remove']:
             raise exception.ApiError('operation_type must be add or remove')
         return images.modify(context, image_id, operation_type)
+
+    def update_image(self, context, image_id, **kwargs):
+        result = images.update(context, image_id, dict(kwargs))
+        return result

=== modified file 'nova/api/ec2/images.py'
--- nova/api/ec2/images.py	2010-09-16 15:33:42 +0000
+++ nova/api/ec2/images.py	2010-09-29 03:43:40 +0000
@@ -43,6 +43,14 @@
 
     return True
 
+def update(context, image_id, attributes):
+    """update an image's attributes / info.json"""
+    attributes.update({"image_id": image_id})
+    conn(context).make_request(
+        method='POST',
+        bucket='_images',
+        query_args=qs(attributes))
+    return True
 
 def register(context, image_location):
     """ rpc call to register a new image based from a manifest """

=== modified file 'nova/db/sqlalchemy/models.py'
--- nova/db/sqlalchemy/models.py	2010-09-29 02:38:15 +0000
+++ nova/db/sqlalchemy/models.py	2010-09-29 03:43:40 +0000
@@ -239,7 +239,6 @@
     vcpus = Column(Integer)
     local_gb = Column(Integer)
 
-
     hostname = Column(String(255))
     host = Column(String(255))  # , ForeignKey('hosts.id'))
 
@@ -253,6 +252,10 @@
     scheduled_at = Column(DateTime)
     launched_at = Column(DateTime)
     terminated_at = Column(DateTime)
+
+    display_name = Column(String(255))
+    display_description = Column(String(255))
+
     # TODO(vish): see Ewan's email about state improvements, probably
     #             should be in a driver base class or some such
     # vmstate_state = running, halted, suspended, paused
@@ -289,6 +292,10 @@
     launched_at = Column(DateTime)
     terminated_at = Column(DateTime)
 
+    display_name = Column(String(255))
+    display_description = Column(String(255))
+
+
 class Quota(BASE, NovaBase):
     """Represents quota overrides for a project"""
     __tablename__ = 'quotas'

=== modified file 'nova/objectstore/handler.py'
--- nova/objectstore/handler.py	2010-09-07 20:00:01 +0000
+++ nova/objectstore/handler.py	2010-09-29 03:43:40 +0000
@@ -352,6 +352,8 @@
                 m[u'imageType'] = m['type']
             elif 'imageType' in m:
                 m[u'type'] = m['imageType']
+            if 'displayName' not in m:
+                m[u'displayName'] = u''
             return m
 
         request.write(json.dumps([decorate(i.metadata) for i in images]))
@@ -382,16 +384,25 @@
     def render_POST(self, request): # pylint: disable-msg=R0201
         """Update image attributes: public/private"""
 
+        # image_id required for all requests
         image_id = get_argument(request, 'image_id', u'')
+        image_object = image.Image(image_id)
+        if not image_object.is_authorized(request.context):
+            logging.debug("not authorized for render_POST in images")
+            raise exception.NotAuthorized
+
         operation = get_argument(request, 'operation', u'')
-
-        image_object = image.Image(image_id)
-
-        if not image_object.is_authorized(request.context):
-            raise exception.NotAuthorized
-
-        image_object.set_public(operation=='add')
-
+        if operation:
+            # operation implies publicity toggle
+            logging.debug("handling publicity toggle")
+            image_object.set_public(operation=='add')
+        else:
+            # other attributes imply update
+            logging.debug("update user fields")
+            clean_args = {}
+            for arg in request.args.keys():
+                clean_args[arg] = request.args[arg][0]
+            image_object.update_user_editable_fields(clean_args)
         return ''
 
     def render_DELETE(self, request): # pylint: disable-msg=R0201

=== modified file 'nova/objectstore/image.py'
--- nova/objectstore/image.py	2010-08-18 21:14:24 +0000
+++ nova/objectstore/image.py	2010-09-29 03:43:40 +0000
@@ -82,6 +82,16 @@
         with open(os.path.join(self.path, 'info.json'), 'w') as f:
             json.dump(md, f)
 
+    def update_user_editable_fields(self, args):
+        """args is from the request parameters, so requires extra cleaning"""
+        fields = {'display_name': 'displayName', 'description': 'description'}
+        info = self.metadata
+        for field in fields.keys():
+            if field in args:
+                info[fields[field]] = args[field]
+        with open(os.path.join(self.path, 'info.json'), 'w') as f:
+            json.dump(info, f)
+
     @staticmethod
     def all():
         images = []

=== modified file 'nova/tests/cloud_unittest.py'
--- nova/tests/cloud_unittest.py	2010-09-25 02:25:12 +0000
+++ nova/tests/cloud_unittest.py	2010-09-29 03:43:40 +0000
@@ -16,10 +16,13 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import json
 import logging
 from M2Crypto import BIO
 from M2Crypto import RSA
+import os
 import StringIO
+import tempfile
 import time
 
 from twisted.internet import defer
@@ -36,15 +39,22 @@
 from nova.compute import power_state
 from nova.api.ec2 import context
 from nova.api.ec2 import cloud
+from nova.objectstore import image
 
 
 FLAGS = flags.FLAGS
 
 
+# Temp dirs for working with image attributes through the cloud controller
+# (stole this from objectstore_unittest.py)
+OSS_TEMPDIR = tempfile.mkdtemp(prefix='test_oss-')
+IMAGES_PATH = os.path.join(OSS_TEMPDIR, 'images')
+os.makedirs(IMAGES_PATH)
+
 class CloudTestCase(test.TrialTestCase):
     def setUp(self):
         super(CloudTestCase, self).setUp()
-        self.flags(connection_type='fake')
+        self.flags(connection_type='fake', images_path=IMAGES_PATH)
 
         self.conn = rpc.Connection.instance()
         logging.getLogger().setLevel(logging.DEBUG)
@@ -191,3 +201,67 @@
         #for i in xrange(4):
         #    data = self.cloud.get_metadata(instance(i)['private_dns_name'])
         #    self.assert_(data['meta-data']['ami-id'] == 'ami-%s' % i)
+
+    @staticmethod
+    def _fake_set_image_description(ctxt, image_id, description):
+        from nova.objectstore import handler
+        class req:
+            pass
+        request = req()
+        request.context = ctxt
+        request.args = {'image_id': [image_id],
+                        'description': [description]}
+
+        resource = handler.ImagesResource()
+        resource.render_POST(request)
+
+    def test_user_editable_image_endpoint(self):
+        pathdir = os.path.join(FLAGS.images_path, 'ami-testing')
+        os.mkdir(pathdir)
+        info = {'isPublic': False}
+        with open(os.path.join(pathdir, 'info.json'), 'w') as f:
+            json.dump(info, f)
+        img = image.Image('ami-testing')
+        # self.cloud.set_image_description(self.context, 'ami-testing',
+        #                                  'Foo Img')
+        # NOTE(vish): Above won't work unless we start objectstore or create
+        #             a fake version of api/ec2/images.py conn that can
+        #             call methods directly instead of going through boto.
+        #             for now, just cheat and call the method directly
+        self._fake_set_image_description(self.context, 'ami-testing',
+                                         'Foo Img')
+        self.assertEqual('Foo Img', img.metadata['description'])
+        self._fake_set_image_description(self.context, 'ami-testing', '')
+        self.assertEqual('', img.metadata['description'])
+
+    def test_update_of_instance_display_fields(self):
+        inst = db.instance_create({}, {})
+        self.cloud.update_instance(self.context, inst['ec2_id'],
+                                   display_name='c00l 1m4g3')
+        inst = db.instance_get({}, inst['id'])
+        self.assertEqual('c00l 1m4g3', inst['display_name'])
+        db.instance_destroy({}, inst['id'])
+
+    def test_update_of_instance_wont_update_private_fields(self):
+        inst = db.instance_create({}, {})
+        self.cloud.update_instance(self.context, inst['id'],
+                                   mac_address='DE:AD:BE:EF')
+        inst = db.instance_get({}, inst['id'])
+        self.assertEqual(None, inst['mac_address'])
+        db.instance_destroy({}, inst['id'])
+
+    def test_update_of_volume_display_fields(self):
+        vol = db.volume_create({}, {})
+        self.cloud.update_volume(self.context, vol['id'],
+                                 display_name='c00l v0lum3')
+        vol = db.volume_get({}, vol['id'])
+        self.assertEqual('c00l v0lum3', vol['display_name'])
+        db.volume_destroy({}, vol['id'])
+
+    def test_update_of_volume_wont_update_private_fields(self):
+        vol = db.volume_create({}, {})
+        self.cloud.update_volume(self.context, vol['id'],
+                                   mountpoint='/not/here')
+        vol = db.volume_get({}, vol['id'])
+        self.assertEqual(None, vol['mountpoint'])
+        db.volume_destroy({}, vol['id'])

=== modified file 'nova/tests/objectstore_unittest.py'
--- nova/tests/objectstore_unittest.py	2010-09-08 17:57:29 +0000
+++ nova/tests/objectstore_unittest.py	2010-09-29 03:43:40 +0000
@@ -164,6 +164,12 @@
         self.context.project = self.auth_manager.get_project('proj2')
         self.assertFalse(my_img.is_authorized(self.context))
 
+        # change user-editable fields
+        my_img.update_user_editable_fields({'display_name': 'my cool image'})
+        self.assertEqual('my cool image', my_img.metadata['displayName'])
+        my_img.update_user_editable_fields({'display_name': ''})
+        self.assert_(not my_img.metadata['displayName'])
+
 
 class TestHTTPChannel(http.HTTPChannel):
     """Dummy site required for twisted.web"""