Merge trunk and fix how nova-combined works with paste.deploy.

Refactor some of the bits of nova-api into nova/wsgi for working with paste,
for a little bit of de-duplication between nova-api and nova-combined.
Makes a cleaner interface for how paste configs can set flags.

10 files changed, 185 insertions, 98 deletions

diff --git a/bin/nova-api b/bin/nova-api
index b30c5ee71..7b4fbeab1 100755
--- a/bin/nova-api
+++ b/bin/nova-api
@@ -24,8 +24,6 @@ import gettext
 import os
 import sys
 
-from paste import deploy
-
 # If ../nova/__init__.py exists, add ../ to Python search path, so that
 # it will override what happens to be installed in /usr/(local/)lib/python...
 possible_topdir = os.path.normpath(os.path.join(os.path.abspath(sys.argv[0]),
@@ -42,57 +40,47 @@ from nova import wsgi
 
 logging.basicConfig()
 LOG = logging.getLogger('nova.api')
+LOG.setLevel(logging.DEBUG)
 
 FLAGS = flags.FLAGS
 
 API_ENDPOINTS = ['ec2', 'osapi']
 
 
-def load_configuration(paste_config, name):
-    """Load the paste configuration from the config file and return it."""
-    config = None
-    try:
-        config = deploy.appconfig("config:%s" % paste_config, name=name)
-        return config
-    except LookupError:
-        return None
-
-
 def run_app(paste_config_file):
-    LOG.debug(_("Using paste.deploy config at: %s"), configfile)
-    server = wsgi.Server()
+    LOG.debug(_("Using paste.deploy config at: %s"), paste_config_file)
     apps = []
     for api in API_ENDPOINTS:
-        config = load_configuration(paste_config_file, api)
+        config = wsgi.load_paste_configuration(paste_config_file, api)
         if config is None:
+            LOG.debug(_("No paste configuration for app: %s"), api)
             continue
-        if int(config.get('verbose', 0)) == 1:
-            FLAGS.verbose = True
-        host = config.get("%s_host" % api, config.get('host', '0.0.0.0'))
-        port = config.get("%s_port" % api, getattr(FLAGS, "%s_port" % api))
-        setattr(FLAGS, "%s_host" % api, host)
-        setattr(FLAGS, "%s_port" % api, port)
+        LOG.debug(_("App Config: %s\n%r"), api, config)
+        wsgi.paste_config_to_flags(config, {
+            "verbose": FLAGS.verbose,
+            "%s_host" % api: config.get('host', '0.0.0.0'),
+            "%s_port" % api: getattr(FLAGS, "%s_port" % api)})
         LOG.info(_("Running %s API"), api)
-        app = deploy.loadapp('config:%s' % paste_config_file, name=api)
-        apps.append((app, int(port), host))
+        app = wsgi.load_paste_app(paste_config_file, api)
+        apps.append((app, getattr(FLAGS, "%s_port" % api),
+                     getattr(FLAGS, "%s_host" % api)))
     if len(apps) == 0:
         LOG.error(_("No known API applications configured in %s."),
                   paste_config_file)
-    else:
-        for app in apps:
-            server.start(*app)
-        server.wait()
+        return
+
+    # NOTE(todd): redo logging config, verbose could be set in paste config
+    logging.basicConfig()
+    server = wsgi.Server()
+    for app in apps:
+        server.start(*app)
+    server.wait()
 
 
 if __name__ == '__main__':
     FLAGS(sys.argv)
-    configfiles = ['/etc/nova/nova-api.conf']
-    if os.path.exists(os.path.join(possible_topdir, 'nova', '__init__.py')):
-        configfiles.insert(0,
-                       os.path.join(possible_topdir, 'etc', 'nova-api.conf'))
-    for configfile in configfiles:
-        if os.path.exists(configfile):
-            run_app(configfile)
-            break
-        else:
-            LOG.debug(_("Skipping missing configuration: %s"), configfile)
+    conf = wsgi.paste_config_file('nova-api.conf')
+    if conf:
+        run_app(conf)
+    else:
+        LOG.error(_("No paste configuration found for: %s"), 'nova-api.conf')
diff --git a/bin/nova-combined b/bin/nova-combined
index f932fdfd5..913c866bf 100755
--- a/bin/nova-combined
+++ b/bin/nova-combined
@@ -36,22 +36,20 @@ if os.path.exists(os.path.join(possible_topdir, 'nova', '__init__.py')):
 
 gettext.install('nova', unicode=1)
 
-from nova import api
 from nova import flags
+from nova import log as logging
 from nova import service
 from nova import utils
 from nova import wsgi
 
 
 FLAGS = flags.FLAGS
-flags.DEFINE_string('osapi_host', '0.0.0.0', 'OpenStack API host')
-flags.DEFINE_integer('ec2api_port', 8773, 'EC2 API port')
-flags.DEFINE_string('ec2api_host', '0.0.0.0', 'EC2 API host')
 
 
 if __name__ == '__main__':
     utils.default_flagfile()
     FLAGS(sys.argv)
+    logging.basicConfig()
 
     compute = service.Service.create(binary='nova-compute')
     network = service.Service.create(binary='nova-network')
@@ -61,7 +59,22 @@ if __name__ == '__main__':
 
     service.serve(compute, network, volume, scheduler)
 
-    server = wsgi.Server()
-    server.start(api.API('os'), FLAGS.osapi_port, host=FLAGS.osapi_host)
-    server.start(api.API('ec2'), FLAGS.ec2api_port, host=FLAGS.ec2api_host)
-    server.wait()
+    apps = []
+    paste_config_file = wsgi.paste_config_file('nova-api.conf')
+    for api in ['osapi', 'ec2']:
+        config = wsgi.load_paste_configuration(paste_config_file, api)
+        if config is None:
+            continue
+        wsgi.paste_config_to_flags(config, {
+            "verbose": FLAGS.verbose,
+            "%s_host" % api: config.get('host', '0.0.0.0'),
+            "%s_port" % api: getattr(FLAGS, "%s_port" % api)})
+        app = wsgi.load_paste_app(paste_config_file, api)
+        apps.append((app, getattr(FLAGS, "%s_port" % api),
+                     getattr(FLAGS, "%s_host" % api)))
+    if len(apps) > 0:
+        logging.basicConfig()
+        server = wsgi.Server()
+        for app in apps:
+            server.start(*app)
+        server.wait()
diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index e5308ca87..a040e539a 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -73,17 +73,13 @@ def _gen_key(context, user_id, key_name):
 
 
 def ec2_id_to_id(ec2_id):
-    """Convert an ec2 ID (i-[base 36 number]) to an instance id (int)"""
-    return int(ec2_id[2:], 36)
+    """Convert an ec2 ID (i-[base 16 number]) to an instance id (int)"""
+    return int(ec2_id.split('-')[-1], 16)
 
 
-def id_to_ec2_id(instance_id):
-    """Convert an instance ID (int) to an ec2 ID (i-[base 36 number])"""
-    digits = []
-    while instance_id != 0:
-        instance_id, remainder = divmod(instance_id, 36)
-        digits.append('0123456789abcdefghijklmnopqrstuvwxyz'[remainder])
-    return "i-%s" % ''.join(reversed(digits))
+def id_to_ec2_id(instance_id, template='i-%08x'):
+    """Convert an instance ID (int) to an ec2 ID (i-[base 16 number])"""
+    return template % instance_id
 
 
 class CloudController(object):
@@ -541,6 +537,8 @@ class CloudController(object):
         return self.compute_api.get_ajax_console(context, internal_id)
 
     def describe_volumes(self, context, volume_id=None, **kwargs):
+        if volume_id:
+            volume_id = [ec2_id_to_id(x) for x in volume_id]
         volumes = self.volume_api.get_all(context)
         # NOTE(vish): volume_id is an optional list of volume ids to filter by.
         volumes = [self._format_volume(context, v) for v in volumes
@@ -556,7 +554,7 @@ class CloudController(object):
             instance_data = '%s[%s]' % (instance_ec2_id,
                                         volume['instance']['host'])
         v = {}
-        v['volumeId'] = volume['id']
+        v['volumeId'] = id_to_ec2_id(volume['id'], 'vol-%08x')
         v['status'] = volume['status']
         v['size'] = volume['size']
         v['availabilityZone'] = volume['availability_zone']
@@ -574,7 +572,8 @@ class CloudController(object):
                                    'device': volume['mountpoint'],
                                    'instanceId': instance_ec2_id,
                                    'status': 'attached',
-                                   'volume_id': volume['ec2_id']}]
+                                   'volumeId': id_to_ec2_id(volume['id'],
+                                                            'vol-%08x')}]
         else:
             v['attachmentSet'] = [{}]
 
@@ -590,13 +589,15 @@ class CloudController(object):
         # TODO(vish): Instance should be None at db layer instead of
         #             trying to lazy load, but for now we turn it into
         #             a dict to avoid an error.
-        return {'volumeSet': [self._format_volume(context, dict(volume_ref))]}
+        return {'volumeSet': [self._format_volume(context, dict(volume))]}
 
     def delete_volume(self, context, volume_id, **kwargs):
+        volume_id = ec2_id_to_id(volume_id)
         self.volume_api.delete(context, volume_id)
         return True
 
     def update_volume(self, context, volume_id, **kwargs):
+        volume_id = ec2_id_to_id(volume_id)
         updatable_fields = ['display_name', 'display_description']
         changes = {}
         for field in updatable_fields:
@@ -607,18 +608,21 @@ class CloudController(object):
         return True
 
     def attach_volume(self, context, volume_id, instance_id, device, **kwargs):
+        volume_id = ec2_id_to_id(volume_id)
+        instance_id = ec2_id_to_id(instance_id)
         LOG.audit(_("Attach volume %s to instacne %s at %s"), volume_id,
                   instance_id, device, context=context)
         self.compute_api.attach_volume(context, instance_id, volume_id, device)
         volume = self.volume_api.get(context, volume_id)
         return {'attachTime': volume['attach_time'],
                 'device': volume['mountpoint'],
-                'instanceId': instance_id,
+                'instanceId': id_to_ec2_id(instance_id),
                 'requestId': context.request_id,
                 'status': volume['attach_status'],
-                'volumeId': volume_id}
+                'volumeId': id_to_ec2_id(volume_id, 'vol-%08x')}
 
     def detach_volume(self, context, volume_id, **kwargs):
+        volume_id = ec2_id_to_id(volume_id)
         LOG.audit(_("Detach volume %s"), volume_id, context=context)
         volume = self.volume_api.get(context, volume_id)
         instance = self.compute_api.detach_volume(context, volume_id)
@@ -627,7 +631,7 @@ class CloudController(object):
                 'instanceId': id_to_ec2_id(instance['id']),
                 'requestId': context.request_id,
                 'status': volume['attach_status'],
-                'volumeId': volume_id}
+                'volumeId': id_to_ec2_id(volume_id, 'vol-%08x')}
 
     def _convert_to_set(self, lst, label):
         if lst == None or lst == []:
diff --git a/nova/auth/manager.py b/nova/auth/manager.py
index ea7d07b1e..1652e24e1 100644
--- a/nova/auth/manager.py
+++ b/nova/auth/manager.py
@@ -721,7 +721,7 @@ class AuthManager(object):
         if project is None:
             project = user.id
         pid = Project.safe_id(project)
-        return self.__generate_rc(user.access, user.secret, pid, use_dmz)
+        return self.__generate_rc(user, pid, use_dmz)
 
     @staticmethod
     def __generate_rc(user, pid, use_dmz=True, host=None):
diff --git a/nova/db/api.py b/nova/db/api.py
index 1f81ef145..e57766b5c 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -42,6 +42,10 @@ flags.DEFINE_string('db_backend', 'sqlalchemy',
                     'The backend to use for db')
 flags.DEFINE_boolean('enable_new_services', True,
                      'Services to be added to the available pool on create')
+flags.DEFINE_string('instance_name_template', 'instance-%08x',
+                    'Template string to be used to generate instance names')
+flags.DEFINE_string('volume_name_template', 'volume-%08x',
+                    'Template string to be used to generate instance names')
 
 
 IMPL = utils.LazyPluggable(FLAGS['db_backend'],
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index 1dc46fe78..bbc89e573 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -169,7 +169,7 @@ class Instance(BASE, NovaBase):
 
     @property
     def name(self):
-        return "instance-%08x" % self.id
+        return FLAGS.instance_name_template % self.id
 
     admin_pass = Column(String(255))
     user_id = Column(String(255))
@@ -256,7 +256,7 @@ class Volume(BASE, NovaBase):
 
     @property
     def name(self):
-        return "volume-%08x" % self.id
+        return FLAGS.volume_name_template % self.id
 
     user_id = Column(String(255))
     project_id = Column(String(255))
diff --git a/nova/log.py b/nova/log.py
index c1428c051..4997d3f28 100644
--- a/nova/log.py
+++ b/nova/log.py
@@ -116,6 +116,8 @@ def basicConfig():
         handler.setFormatter(_formatter)
     if FLAGS.verbose:
         logging.root.setLevel(logging.DEBUG)
+    else:
+        logging.root.setLevel(logging.INFO)
     if FLAGS.use_syslog:
         syslog = SysLogHandler(address='/dev/log')
         syslog.setFormatter(_formatter)
diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py
index fdacb04f6..2e350cd5a 100644
--- a/nova/tests/test_cloud.py
+++ b/nova/tests/test_cloud.py
@@ -126,10 +126,13 @@ class CloudTestCase(test.TestCase):
         vol2 = db.volume_create(self.context, {})
         result = self.cloud.describe_volumes(self.context)
         self.assertEqual(len(result['volumeSet']), 2)
+        volume_id = cloud.id_to_ec2_id(vol2['id'], 'vol-%08x')
         result = self.cloud.describe_volumes(self.context,
-                                             volume_id=[vol2['id']])
+                                             volume_id=[volume_id])
         self.assertEqual(len(result['volumeSet']), 1)
-        self.assertEqual(result['volumeSet'][0]['volumeId'], vol2['id'])
+        self.assertEqual(
+                cloud.ec2_id_to_id(result['volumeSet'][0]['volumeId']),
+                vol2['id'])
         db.volume_destroy(self.context, vol1['id'])
         db.volume_destroy(self.context, vol2['id'])
 
@@ -385,7 +388,8 @@ class CloudTestCase(test.TestCase):
 
     def test_update_of_volume_display_fields(self):
         vol = db.volume_create(self.context, {})
-        self.cloud.update_volume(self.context, vol['id'],
+        self.cloud.update_volume(self.context,
+                                 cloud.id_to_ec2_id(vol['id'], 'vol-%08x'),
                                  display_name='c00l v0lum3')
         vol = db.volume_get(self.context, vol['id'])
         self.assertEqual('c00l v0lum3', vol['display_name'])
@@ -393,8 +397,9 @@ class CloudTestCase(test.TestCase):
 
     def test_update_of_volume_wont_update_private_fields(self):
         vol = db.volume_create(self.context, {})
-        self.cloud.update_volume(self.context, vol['id'],
-                                   mountpoint='/not/here')
+        self.cloud.update_volume(self.context,
+                                 cloud.id_to_ec2_id(vol['id'], 'vol-%08x'),
+                                 mountpoint='/not/here')
         vol = db.volume_get(self.context, vol['id'])
         self.assertEqual(None, vol['mountpoint'])
         db.volume_destroy(self.context, vol['id'])
diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index bd863b3a2..f75371a7b 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -197,40 +197,29 @@ class LibvirtConnection(object):
             pass
             # If the instance is already terminated, we're still happy
 
-        done = event.Event()
-
         # We'll save this for when we do shutdown,
         # instead of destroy - but destroy returns immediately
         timer = utils.LoopingCall(f=None)
 
-        def _wait_for_shutdown():
+        while True:
             try:
                 state = self.get_info(instance['name'])['state']
                 db.instance_set_state(context.get_admin_context(),
                                       instance['id'], state)
                 if state == power_state.SHUTDOWN:
-                    timer.stop()
+                    break
             except Exception:
                 db.instance_set_state(context.get_admin_context(),
                                       instance['id'],
                                       power_state.SHUTDOWN)
-                timer.stop()
+                break
 
-        timer.f = _wait_for_shutdown
-        timer_done = timer.start(interval=0.5, now=True)
+        self.firewall_driver.unfilter_instance(instance)
 
-        # NOTE(termie): this is strictly superfluous (we could put the
-        #               cleanup code in the timer), but this emulates the
-        #               previous model so I am keeping it around until
-        #               everything has been vetted a bit
-        def _wait_for_timer():
-            timer_done.wait()
-            if cleanup:
-                self._cleanup(instance)
-            done.send()
+        if cleanup:
+            self._cleanup(instance)
 
-        greenthread.spawn(_wait_for_timer)
-        return done
+        return True
 
     def _cleanup(self, instance):
         target = os.path.join(FLAGS.instances_path, instance['name'])
@@ -787,6 +776,10 @@ class FirewallDriver(object):
         At this point, the instance isn't running yet."""
         raise NotImplementedError()
 
+    def unfilter_instance(self, instance):
+        """Stop filtering instance"""
+        raise NotImplementedError()
+
     def apply_instance_filter(self, instance):
         """Apply instance filter.
 
@@ -977,6 +970,10 @@ class NWFilterFirewall(FirewallDriver):
         # execute in a native thread and block current greenthread until done
         tpool.execute(self._conn.nwfilterDefineXML, xml)
 
+    def unfilter_instance(self, instance):
+        # Nothing to do
+        pass
+
     def prepare_instance_filter(self, instance):
         """
         Creates an NWFilter for the given instance. In the process,
@@ -1058,17 +1055,25 @@ class NWFilterFirewall(FirewallDriver):
 class IptablesFirewallDriver(FirewallDriver):
     def __init__(self, execute=None):
         self.execute = execute or utils.execute
-        self.instances = set()
+        self.instances = {}
 
     def apply_instance_filter(self, instance):
         """No-op. Everything is done in prepare_instance_filter"""
         pass
 
     def remove_instance(self, instance):
-        self.instances.remove(instance)
+        if instance['id'] in self.instances:
+            del self.instances[instance['id']]
+        else:
+            LOG.info(_('Attempted to unfilter instance %s which is not '
+                       'filtered'), instance['id'])
 
     def add_instance(self, instance):
-        self.instances.add(instance)
+        self.instances[instance['id']] = instance
+
+    def unfilter_instance(self, instance):
+        self.remove_instance(instance)
+        self.apply_ruleset()
 
     def prepare_instance_filter(self, instance):
         self.add_instance(instance)
@@ -1101,10 +1106,11 @@ class IptablesFirewallDriver(FirewallDriver):
         our_chains += [':nova-local - [0:0]']
         our_rules += ['-A FORWARD -j nova-local']
 
-        security_groups = set()
+        security_groups = {}
         # Add our chains
         # First, we add instance chains and rules
-        for instance in self.instances:
+        for instance_id in self.instances:
+            instance = self.instances[instance_id]
             chain_name = self._instance_chain_name(instance)
             ip_address = self._ip_for_instance(instance)
 
@@ -1126,9 +1132,10 @@ class IptablesFirewallDriver(FirewallDriver):
             for security_group in \
                             db.security_group_get_by_instance(ctxt,
                                                               instance['id']):
-                security_groups.add(security_group)
+                security_groups[security_group['id']] = security_group
 
-                sg_chain_name = self._security_group_chain_name(security_group)
+                sg_chain_name = self._security_group_chain_name(
+                                                          security_group['id'])
 
                 our_rules += ['-A %s -j %s' % (chain_name, sg_chain_name)]
 
@@ -1141,13 +1148,13 @@ class IptablesFirewallDriver(FirewallDriver):
             our_rules += ['-A %s -j nova-ipv4-fallback' % (chain_name,)]
 
         # then, security group chains and rules
-        for security_group in security_groups:
-            chain_name = self._security_group_chain_name(security_group)
+        for security_group_id in security_groups:
+            chain_name = self._security_group_chain_name(security_group_id)
             our_chains += [':%s - [0:0]' % chain_name]
 
             rules = \
               db.security_group_rule_get_by_security_group(ctxt,
-                                                          security_group['id'])
+                                                          security_group_id)
 
             for rule in rules:
                 logging.info('%r', rule)
@@ -1195,8 +1202,8 @@ class IptablesFirewallDriver(FirewallDriver):
     def refresh_security_group_rules(self, security_group):
         self.apply_ruleset()
 
-    def _security_group_chain_name(self, security_group):
-        return 'nova-sg-%s' % (security_group['id'],)
+    def _security_group_chain_name(self, security_group_id):
+        return 'nova-sg-%s' % (security_group_id,)
 
     def _instance_chain_name(self, instance):
         return 'nova-inst-%s' % (instance['id'],)
diff --git a/nova/wsgi.py b/nova/wsgi.py
index e999f76a3..817cd0478 100644
--- a/nova/wsgi.py
+++ b/nova/wsgi.py
@@ -22,6 +22,7 @@ Utility methods for working with WSGI servers
 """
 
 import json
+import os
 import sys
 from xml.dom import minidom
 
@@ -34,9 +35,15 @@ import webob
 import webob.dec
 import webob.exc
 
+from paste import deploy
+
+from nova import flags
 from nova import log as logging
 
 
+FLAGS = flags.FLAGS
+
+
 class WritableLogger(object):
     """A thin wrapper that responds to `write` and logs."""
 
@@ -385,3 +392,60 @@ class Serializer(object):
             node = doc.createTextNode(str(data))
             result.appendChild(node)
         return result
+
+def paste_config_file(basename):
+    """Find the best location in the system for a paste config file.
+   
+    Search Order
+    ------------
+
+    The search for a paste config file honors `FLAGS.state_path`, which in a
+    version checked out from bzr will be the `nova` directory in the top level
+    of the checkout, and in an installation for a package for your distribution
+    will likely point to someplace like /etc/nova.
+
+    This method tries to load places likely to be used in development or
+    experimentation before falling back to the system-wide configuration
+    in `/etc/nova/`.
+
+    * Current working directory
+    * the `etc` directory under state_path, because when working on a checkout
+      from bzr this will point to the default
+    * top level of FLAGS.state_path, for distributions
+    * /etc/nova, which may not be diffrerent from state_path on your distro
+
+    """
+
+    configfiles = [basename,
+                   os.path.join(FLAGS.state_path, 'etc', basename),
+                   os.path.join(FLAGS.state_path, basename),
+                   '/etc/nova/%s' % basename]
+    for configfile in configfiles:
+        if os.path.exists(configfile):
+            return configfile
+
+def load_paste_configuration(filename, appname):
+    """Returns a paste configuration dict, or None."""
+    filename = os.path.abspath(filename)
+    config = None
+    try:
+        config = deploy.appconfig("config:%s" % filename, name=appname)
+    except LookupError:
+        pass
+    return config
+
+def load_paste_app(filename, appname):
+    """Builds a wsgi app from a paste config, None if app not configured."""
+    filename = os.path.abspath(filename)
+    app = None
+    try:
+        app = deploy.loadapp("config:%s" % filename, name=appname)
+    except LookupError:
+        pass
+    return app
+
+def paste_config_to_flags(config, mixins):
+    for k,v in mixins.iteritems():
+        value = config.get(k, v)
+        converted_value = FLAGS[k].parser.Parse(value)
+        setattr(FLAGS, k, converted_value)