more fixes from code review

4 files changed, 54 insertions, 58 deletions

diff --git a/nova/endpoint/admin.py b/nova/endpoint/admin.py
index 3d91c66dc..c6dcb5320 100644
--- a/nova/endpoint/admin.py
+++ b/nova/endpoint/admin.py
@@ -53,7 +53,6 @@ def project_dict(project):
 def host_dict(host):
     """Convert a host model object to a result dict"""
     if host:
-        # FIXME(vish)
         return host.state
     else:
         return {}
@@ -195,6 +194,8 @@ class AdminController(object):
             raise exception.ApiError('operation must be add or remove')
         return True
 
+    # FIXME(vish): these host commands don't work yet, perhaps some of the
+    #              required data can be retrieved from service objects?
     @admin_only
     def describe_hosts(self, _context, **_kwargs):
         """Returns status info for all nodes. Includes:
diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py
index 932d42de4..709c967bb 100644
--- a/nova/endpoint/cloud.py
+++ b/nova/endpoint/cloud.py
@@ -78,7 +78,7 @@ class CloudController(object):
         if not os.path.exists(root_ca_path):
             start = os.getcwd()
             os.chdir(FLAGS.ca_path)
-            # TODO: Do this with M2Crypto instead
+            # TODO(vish): Do this with M2Crypto instead
             utils.runthis("Generating root CA: %s", "sh genrootca.sh")
             os.chdir(start)
 
@@ -93,28 +93,30 @@ class CloudController(object):
                 result[instance['key_name']] = [line]
         return result
 
-    def get_metadata(self, ipaddress):
-        i = db.fixed_ip_get_instance(ipaddress)
-        if i is None:
+    def get_metadata(self, address):
+        instance_ref = db.fixed_ip_get_instance(None, address)
+        if instance_ref is None:
             return None
-        mpi = self._get_mpi_data(i['project_id'])
-        if i['key_name']:
+        mpi = self._get_mpi_data(instance_ref['project_id'])
+        if instance_ref['key_name']:
             keys = {
                 '0': {
-                    '_name': i['key_name'],
-                    'openssh-key': i['key_data']
+                    '_name': instance_ref['key_name'],
+                    'openssh-key': instance_ref['key_data']
                 }
             }
         else:
             keys = ''
-        hostname = i['hostname']
+        hostname = instance_ref['hostname']
+        floating_ip = db.instance_get_floating_ip_address(None,
+                                                          instance_ref['id'])
         data = {
-            'user-data': base64.b64decode(i['user_data']),
+            'user-data': base64.b64decode(instance_ref['user_data']),
             'meta-data': {
-                'ami-id': i['image_id'],
-                'ami-launch-index': i['ami_launch_index'],
-                'ami-manifest-path': 'FIXME', # image property
-                'block-device-mapping': { # TODO: replace with real data
+                'ami-id': instance_ref['image_id'],
+                'ami-launch-index': instance_ref['ami_launch_index'],
+                'ami-manifest-path': 'FIXME',
+                'block-device-mapping': {  # TODO(vish): replace with real data
                     'ami': 'sda1',
                     'ephemeral0': 'sda2',
                     'root': '/dev/sda1',
@@ -122,27 +124,27 @@ class CloudController(object):
                 },
                 'hostname': hostname,
                 'instance-action': 'none',
-                'instance-id': i['instance_id'],
-                'instance-type': i.get('instance_type', ''),
+                'instance-id': instance_ref['str_id'],
+                'instance-type': instance_ref['instance_type'],
                 'local-hostname': hostname,
-                'local-ipv4': i['private_dns_name'], # TODO: switch to IP
-                'kernel-id': i.get('kernel_id', ''),
+                'local-ipv4': address,
+                'kernel-id': instance_ref['kernel_id'],
                 'placement': {
-                    'availaibility-zone': i.get('availability_zone', 'nova'),
+                    'availaibility-zone': instance_ref['availability_zone'],
                 },
                 'public-hostname': hostname,
-                'public-ipv4': i.get('dns_name', ''), # TODO: switch to IP
+                'public-ipv4': floating_ip or '',
                 'public-keys': keys,
-                'ramdisk-id': i.get('ramdisk_id', ''),
-                'reservation-id': i['reservation_id'],
-                'security-groups': i.get('groups', ''),
+                'ramdisk-id': instance_ref['ramdisk_id'],
+                'reservation-id': instance_ref['reservation_id'],
+                'security-groups': '',
                 'mpi': mpi
             }
         }
-        if False: # TODO: store ancestor ids
+        if False:  # TODO(vish): store ancestor ids
             data['ancestor-ami-ids'] = []
-        if i.get('product_codes', None):
-            data['product-codes'] = i['product_codes']
+        if False:  # TODO(vish): store product codes
+            data['product-codes'] = []
         return data
 
     @rbac.allow('all')
@@ -253,7 +255,7 @@ class CloudController(object):
         v['status'] = volume['status']
         v['size'] = volume['size']
         v['availabilityZone'] = volume['availability_zone']
-        # v['createTime'] = volume['create_time']
+        v['createTime'] = volume['created_at']
         if context.user.is_admin():
             v['status'] = '%s (%s, %s, %s, %s)' % (
                 volume['status'],
@@ -296,7 +298,6 @@ class CloudController(object):
         # TODO(vish): abstract status checking?
         if volume_ref['attach_status'] == "attached":
             raise exception.ApiError("Volume is already attached")
-        #volume.start_attach(instance_id, device)
         instance_ref = db.instance_get_by_str(context, instance_id)
         host = db.instance_get_host(context, instance_ref['id'])
         rpc.cast(db.queue_get_for(context, FLAGS.compute_topic, host),
@@ -322,7 +323,6 @@ class CloudController(object):
         if volume_ref['status'] == "available":
             raise exception.Error("Volume is already detached")
         try:
-            #volume.start_detach()
             host = db.instance_get_host(context, instance_ref['id'])
             rpc.cast(db.queue_get_for(context, FLAGS.compute_topic, host),
                                 {"method": "detach_volume",
@@ -422,15 +422,12 @@ class CloudController(object):
         for floating_ip_ref in iterator:
             address = floating_ip_ref['id_str']
             instance_ref = db.floating_ip_get_instance(address)
-            address_rv = {
-                'public_ip': address,
-                'instance_id': instance_ref['id_str']
-            }
+            address_rv = {'public_ip': address,
+                          'instance_id': instance_ref['id_str']}
             if context.user.is_admin():
-                address_rv['instance_id'] = "%s (%s)" % (
-                    address_rv['instance_id'],
-                    floating_ip_ref['project_id'],
-                )
+                details = "%s (%s)" % (address_rv['instance_id'],
+                                       floating_ip_ref['project_id'])
+                address_rv['instance_id'] = details
             addresses.append(address_rv)
         return {'addressesSet': addresses}
 
@@ -579,7 +576,6 @@ class CloudController(object):
     @defer.inlineCallbacks
     def terminate_instances(self, context, instance_id, **kwargs):
         logging.debug("Going to start terminating instances")
-        # network_topic = yield self._get_network_topic(context)
         for id_str in instance_id:
             logging.debug("Going to try and terminate %s" % id_str)
             try:
diff --git a/nova/flags.py b/nova/flags.py
index ebbfe3ff8..7b0c95a3c 100644
--- a/nova/flags.py
+++ b/nova/flags.py
@@ -203,12 +203,6 @@ DEFINE_string('vpn_key_suffix',
 
 DEFINE_integer('auth_token_ttl', 3600, 'Seconds for auth tokens to linger')
 
-# UNUSED
-DEFINE_string('node_availability_zone', 'nova',
-              'availability zone of this node')
-DEFINE_string('host', socket.gethostname(),
-              'name of this node')
-
 DEFINE_string('sql_connection',
               'sqlite:///%s/nova.sqlite' % os.path.abspath("./"),
               'connection string for sql database')
@@ -220,4 +214,9 @@ DEFINE_string('network_manager', 'nova.network.manager.VlanManager',
 DEFINE_string('volume_manager', 'nova.volume.manager.AOEManager',
               'Manager for volume')
 
+DEFINE_string('host', socket.gethostname(),
+              'name of this node')
 
+# UNUSED
+DEFINE_string('node_availability_zone', 'nova',
+              'availability zone of this node')
diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 1506e85ad..41aeb5da7 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -41,6 +41,9 @@ flags.DEFINE_string('bridge_dev', 'eth0',
                         'network device for bridges')
 
 
+DEFAULT_PORTS = [("tcp", 80), ("tcp", 22), ("udp", 1194), ("tcp", 443)]
+
+
 def bind_floating_ip(floating_ip):
     """Bind ip to public interface"""
     _execute("sudo ip addr add %s dev %s" % (floating_ip,
@@ -61,9 +64,6 @@ def ensure_vlan_forward(public_ip, port, private_ip):
             % (public_ip, port, private_ip))
 
 
-DEFAULT_PORTS = [("tcp", 80), ("tcp", 22), ("udp", 1194), ("tcp", 443)]
-
-
 def ensure_floating_forward(floating_ip, fixed_ip):
     """Ensure floating ip forwarding rule"""
     _confirm_rule("PREROUTING -t nat -d %s -j DNAT --to %s"
@@ -208,16 +208,16 @@ def _remove_rule(cmd):
 def _dnsmasq_cmd(net):
     """Builds dnsmasq command"""
     cmd = ['sudo -E dnsmasq',
-        ' --strict-order',
-        ' --bind-interfaces',
-        ' --conf-file=',
-        ' --pid-file=%s' % _dhcp_file(net['vlan'], 'pid'),
-        ' --listen-address=%s' % net['gateway'],
-        ' --except-interface=lo',
-        ' --dhcp-range=%s,static,120s' % net['dhcp_start'],
-        ' --dhcp-hostsfile=%s' % _dhcp_file(net['vlan'], 'conf'),
-        ' --dhcp-script=%s' % _bin_file('nova-dhcpbridge'),
-        ' --leasefile-ro']
+           ' --strict-order',
+           ' --bind-interfaces',
+           ' --conf-file=',
+           ' --pid-file=%s' % _dhcp_file(net['vlan'], 'pid'),
+           ' --listen-address=%s' % net['gateway'],
+           ' --except-interface=lo',
+           ' --dhcp-range=%s,static,120s' % net['dhcp_start'],
+           ' --dhcp-hostsfile=%s' % _dhcp_file(net['vlan'], 'conf'),
+           ' --dhcp-script=%s' % _bin_file('nova-dhcpbridge'),
+           ' --leasefile-ro']
     return ''.join(cmd)