Fixes euca-describe-instances failing or not showing IPs

Fixes bug 855660

Makes EC2 call network manager to get IP information now, to match the
same change to OS API
Also refactors a bit of OS API's calls... moving some code into 'common'

Fixed tests.  Some tests for OS API v1.0 were not properly testing.

Fixed imports per HACKING in files touched.

Change-Id: I455637a9feb802291dfaf2ef694dabc2607784f9

5 files changed, 193 insertions, 150 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index a71ab8cf0..a6a10c767 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -238,14 +238,58 @@ class CloudController(object):
             utils.runthis(_("Generating root CA: %s"), "sh", genrootca_sh_path)
             os.chdir(start)
 
+    def _get_floaters_for_fixed_ip(self, context, fixed_ip):
+        """Return all floating IPs given a fixed IP"""
+        return self.network_api.get_floating_ips_by_fixed_address(context,
+                fixed_ip)
+
+    def _get_fixed_ips_for_instance(self, context, instance):
+        """Return a list of all fixed IPs for an instance"""
+
+        ret_ips = []
+        ret_ip6s = []
+        nw_info = self.network_api.get_instance_nw_info(context, instance)
+        for net, info in nw_info:
+            if not info:
+                continue
+            ips = info.get('ips', [])
+            for ip in ips:
+                try:
+                    ret_ips.append(ip['ip'])
+                except KeyError:
+                    pass
+            if FLAGS.use_ipv6:
+                ip6s = info.get('ip6s', [])
+                for ip6 in ip6s:
+                    try:
+                        ret_ip6s.append(ip6['ip'])
+                    except KeyError:
+                        pass
+        return (ret_ips, ret_ip6s)
+
+    def _get_floaters_for_instance(self, context, instance, return_all=True):
+        """Return all floating IPs for an instance"""
+
+        ret_floaters = []
+        fixed_ips = self._get_fixed_ips_for_instance(context, instance)
+        for ip in fixed_ips:
+            floaters = self._get_floaters_for_fixed_ip(context, ip)
+            # Allows a short circuit if we just need any floater.
+            if floaters and not return_all:
+                return floaters
+            ret_floaters.extend(floaters)
+            if floaters and only_one:
+                return ret_floaters
+        return ret_floaters
+
     def _get_mpi_data(self, context, project_id):
         result = {}
         search_opts = {'project_id': project_id}
         for instance in self.compute_api.get_all(context,
                 search_opts=search_opts):
-            if instance['fixed_ips']:
-                line = '%s slots=%d' % (instance['fixed_ips'][0]['address'],
-                                        instance['vcpus'])
+            fixed_ips = self._get_fixed_ips_for_instance(context, instance)[0]
+            if fixed_ips:
+                line = '%s slots=%d' % (fixed_ips[0], instance['vcpus'])
                 key = str(instance['key_name'])
                 if key in result:
                     result[key].append(line)
@@ -331,8 +375,11 @@ class CloudController(object):
         hostname = "%s.%s" % (instance_ref['hostname'], FLAGS.dhcp_domain)
         host = instance_ref['host']
         availability_zone = self._get_availability_zone_by_host(ctxt, host)
-        floating_ip = db.instance_get_floating_address(ctxt,
-                                                       instance_ref['id'])
+
+        floaters = self._get_floaters_for_instance(ctxt, instance_ref,
+                return_all=False)
+        floating_ip = floaters and floaters[0] or ''
+
         ec2_id = ec2utils.id_to_ec2_id(instance_ref['id'])
         image_ec2_id = self.image_ec2_id(instance_ref['image_ref'])
         security_groups = db.security_group_get_by_instance(ctxt,
@@ -354,7 +401,7 @@ class CloudController(object):
                 'local-ipv4': address,
                 'placement': {'availability-zone': availability_zone},
                 'public-hostname': hostname,
-                'public-ipv4': floating_ip or '',
+                'public-ipv4': floating_ip,
                 'reservation-id': instance_ref['reservation_id'],
                 'security-groups': security_groups,
                 'mpi': mpi}}
@@ -1257,23 +1304,27 @@ class CloudController(object):
             i['instanceState'] = {
                 'code': instance['power_state'],
                 'name': state_description_from_vm_state(instance['vm_state'])}
-            fixed_addr = None
-            floating_addr = None
-            if instance['fixed_ips']:
-                fixed = instance['fixed_ips'][0]
-                fixed_addr = fixed['address']
-                if fixed['floating_ips']:
-                    floating_addr = fixed['floating_ips'][0]['address']
-                if fixed['network'] and use_v6:
-                    i['dnsNameV6'] = ipv6.to_global(
-                        fixed['network']['cidr_v6'],
-                        fixed['virtual_interface']['address'],
-                        instance['project_id'])
-
-            i['privateDnsName'] = fixed_addr
-            i['privateIpAddress'] = fixed_addr
-            i['publicDnsName'] = floating_addr
-            i['ipAddress'] = floating_addr or fixed_addr
+
+            fixed_ip = None
+            floating_ip = None
+            (fixed_ips, fixed_ip6s) = self._get_fixed_ips_for_instance(context,
+                    instance)
+            if fixed_ips:
+                fixed_ip = fixed_ips[0]
+                # Now look for a floater.
+                for ip in fixed_ips:
+                    floating_ips = self._get_floaters_for_fixed_ip(context, ip)
+                    # NOTE(comstud): Will it float?
+                    if floating_ips:
+                        floating_ip = floating_ips[0]
+                        # Got one, exit out.
+                        break
+            if fixed_ip6s:
+                i['dnsNameV6'] = fixed_ip6s[0]
+            i['privateDnsName'] = fixed_ip
+            i['privateIpAddress'] = fixed_ip
+            i['publicDnsName'] = floating_ip
+            i['ipAddress'] = floating_ip or fixed_ip
             i['dnsName'] = i['publicDnsName'] or i['privateDnsName']
             i['keyName'] = instance['key_name']
 
diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py
index 726ec4612..bf5d415d5 100644
--- a/nova/api/openstack/common.py
+++ b/nova/api/openstack/common.py
@@ -19,18 +19,18 @@ import functools
 from lxml import etree
 import re
 import urlparse
-from xml.dom import minidom
-
 import webob
+from xml.dom import minidom
 
-from nova import exception
-from nova import flags
-from nova import log as logging
-from nova import quota
 from nova.api.openstack import wsgi
 from nova.api.openstack import xmlutil
 from nova.compute import vm_states
 from nova.compute import task_states
+from nova import exception
+from nova import flags
+from nova import log as logging
+import nova.network
+from nova import quota
 
 
 LOG = logging.getLogger('nova.api.openstack.common')
@@ -272,6 +272,55 @@ def dict_to_query_str(params):
     return param_str.rstrip('&')
 
 
+def get_networks_for_instance(context, instance):
+    """Returns a prepared nw_info list for passing into the view
+    builders
+
+    We end up with a datastructure like:
+    {'public': {'ips': [{'addr': '10.0.0.1', 'version': 4},
+                        {'addr': '2001::1', 'version': 6}],
+                'floating_ips': [{'addr': '172.16.0.1', 'version': 4},
+                                 {'addr': '172.16.2.1', 'version': 4}]},
+     ...}
+    """
+
+    network_api = nova.network.API()
+
+    def _get_floats(ip):
+        return network_api.get_floating_ips_by_fixed_address(context, ip)
+
+    def _emit_addr(ip, version):
+        return {'addr': ip, 'version': version}
+
+    nw_info = network_api.get_instance_nw_info(context, instance)
+
+    networks = {}
+    for net, info in nw_info:
+        if not info:
+            continue
+        try:
+            network = {'ips': []}
+            network['floating_ips'] = []
+            for ip in info['ips']:
+                network['ips'].append(_emit_addr(ip['ip'], 4))
+                floats = [_emit_addr(addr, 4)
+                        for addr in _get_floats(ip['ip'])]
+                network['floating_ips'].extend(floats)
+            if FLAGS.use_ipv6 and 'ip6s' in info:
+                network['ips'].extend([_emit_addr(ip['ip'], 6)
+                        for ip in info['ip6s']])
+        # NOTE(comstud): These exception checks are for lp830817
+        # (Restoring them after a refactoring removed)
+        except TypeError:
+            raise
+            continue
+        except KeyError:
+            raise
+            continue
+        networks[info['label']] = network
+    return networks
+
+
 class MetadataXMLDeserializer(wsgi.XMLDeserializer):
 
     def extract_metadata(self, metadata_node):
diff --git a/nova/api/openstack/ips.py b/nova/api/openstack/ips.py
index 49aa81958..db0b7107b 100644
--- a/nova/api/openstack/ips.py
+++ b/nova/api/openstack/ips.py
@@ -20,11 +20,12 @@ from lxml import etree
 from webob import exc
 
 import nova
-import nova.api.openstack.views.addresses
-from nova import log as logging
-from nova import flags
+from nova.api.openstack import common
+from nova.api.openstack.views import addresses as views_addresses
 from nova.api.openstack import wsgi
 from nova.api.openstack import xmlutil
+from nova import log as logging
+from nova import flags
 
 
 LOG = logging.getLogger('nova.api.openstack.ips')
@@ -36,7 +37,6 @@ class Controller(object):
 
     def __init__(self):
         self.compute_api = nova.compute.API()
-        self.network_api = nova.network.API()
 
     def _get_instance(self, context, server_id):
         try:
@@ -58,16 +58,14 @@ class ControllerV10(Controller):
     def index(self, req, server_id):
         context = req.environ['nova.context']
         instance = self._get_instance(context, server_id)
-        networks = _get_networks_for_instance(context, self.network_api,
-                                              instance)
-        builder = nova.api.openstack.views.addresses.ViewBuilderV10()
+        networks = common.get_networks_for_instance(context, instance)
+        builder = self._get_view_builder(req)
         return {'addresses': builder.build(networks)}
 
     def show(self, req, server_id, id):
         context = req.environ['nova.context']
         instance = self._get_instance(context, server_id)
-        networks = _get_networks_for_instance(context, self.network_api,
-                                              instance)
+        networks = common.get_networks_for_instance(context, instance)
         builder = self._get_view_builder(req)
         if id == 'private':
             view = builder.build_private_parts(networks)
@@ -80,7 +78,7 @@ class ControllerV10(Controller):
         return {id: view}
 
     def _get_view_builder(self, req):
-        return nova.api.openstack.views.addresses.ViewBuilderV10()
+        return views_addresses.ViewBuilderV10()
 
 
 class ControllerV11(Controller):
@@ -88,16 +86,13 @@ class ControllerV11(Controller):
     def index(self, req, server_id):
         context = req.environ['nova.context']
         instance = self._get_instance(context, server_id)
-        networks = _get_networks_for_instance(context, self.network_api,
-                                              instance)
+        networks = common.get_networks_for_instance(context, instance)
         return {'addresses': self._get_view_builder(req).build(networks)}
 
     def show(self, req, server_id, id):
         context = req.environ['nova.context']
         instance = self._get_instance(context, server_id)
-        networks = _get_networks_for_instance(context, self.network_api,
-                                              instance)
-
+        networks = common.get_networks_for_instance(context, instance)
         network = self._get_view_builder(req).build_network(networks, id)
 
         if network is None:
@@ -107,7 +102,7 @@ class ControllerV11(Controller):
         return network
 
     def _get_view_builder(self, req):
-        return nova.api.openstack.views.addresses.ViewBuilderV11()
+        return views_addresses.ViewBuilderV11()
 
 
 class IPXMLSerializer(wsgi.XMLDictSerializer):
@@ -143,45 +138,6 @@ class IPXMLSerializer(wsgi.XMLDictSerializer):
         return self._to_xml(addresses)
 
 
-def _get_networks_for_instance(context, network_api, instance):
-    """Returns a prepared nw_info list for passing into the view
-    builders
-
-    We end up with a datastructure like:
-    {'public': {'ips': [{'addr': '10.0.0.1', 'version': 4},
-                        {'addr': '2001::1', 'version': 6}],
-                'floating_ips': [{'addr': '172.16.0.1', 'version': 4},
-                                 {'addr': '172.16.2.1', 'version': 4}]},
-     ...}
-    """
-    def _get_floats(ip):
-        return network_api.get_floating_ips_by_fixed_address(context, ip)
-
-    def _emit_addr(ip, version):
-        return {'addr': ip, 'version': version}
-
-    if FLAGS.stub_network:
-        return {}
-
-    nw_info = network_api.get_instance_nw_info(context, instance)
-
-    networks = {}
-    for net, info in nw_info:
-        network = {'ips': []}
-        network['floating_ips'] = []
-        if 'ip6s' in info:
-            network['ips'].extend([_emit_addr(ip['ip'],
-                                              6) for ip in info['ip6s']])
-
-        for ip in info['ips']:
-            network['ips'].append(_emit_addr(ip['ip'], 4))
-            floats = [_emit_addr(addr,
-                                 4) for addr in _get_floats(ip['ip'])]
-            network['floating_ips'].extend(floats)
-        networks[info['label']] = network
-    return networks
-
-
 def create_resource(version):
     controller = {
         '1.0': ControllerV10,
diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py
index de081343e..673294e1a 100644
--- a/nova/api/openstack/servers.py
+++ b/nova/api/openstack/servers.py
@@ -24,27 +24,27 @@ from webob import exc
 import webob
 from xml.dom import minidom
 
+import nova.api.openstack
+from nova.api.openstack import common
+from nova.api.openstack import ips
+from nova.api.openstack.views import addresses as views_addresses
+from nova.api.openstack.views import flavors as views_flavors
+from nova.api.openstack.views import images as views_images
+from nova.api.openstack.views import servers as views_servers
+from nova.api.openstack import wsgi
+from nova.api.openstack import xmlutil
 from nova import compute
+from nova.compute import instance_types
 from nova import network
 from nova import db
 from nova import exception
 from nova import flags
 from nova import image
 from nova import log as logging
-from nova import utils
 from nova import quota
-from nova.api.openstack import common
-from nova.api.openstack import ips
-from nova.api.openstack import wsgi
-from nova.compute import instance_types
-from nova.scheduler import api as scheduler_api
-import nova.api.openstack
-import nova.api.openstack.views.addresses
-import nova.api.openstack.views.flavors
-import nova.api.openstack.views.images
-import nova.api.openstack.views.servers
-from nova.api.openstack import xmlutil
 from nova.rpc import common as rpc_common
+from nova.scheduler import api as scheduler_api
+from nova import utils
 
 
 LOG = logging.getLogger('nova.api.openstack.servers')
@@ -117,11 +117,6 @@ class Controller(object):
         """
         return None
 
-    def _get_networks_for_instance(self, req, instance):
-        return ips._get_networks_for_instance(req.environ['nova.context'],
-                                              self.network_api,
-                                              instance)
-
     def _get_block_device_mapping(self, data):
         """Get block_device_mapping from 'server' dictionary.
         Overidden by volumes controller.
@@ -984,17 +979,16 @@ class ControllerV10(Controller):
         return data['server']['flavorId']
 
     def _build_view(self, req, instance, is_detail=False):
-        addresses = nova.api.openstack.views.addresses.ViewBuilderV10()
-        builder = nova.api.openstack.views.servers.ViewBuilderV10(addresses)
-        networks = self._get_networks_for_instance(req, instance)
-        return builder.build(instance, networks, is_detail=is_detail)
+        context = req.environ['nova.context']
+        addresses = views_addresses.ViewBuilderV10()
+        builder = views_servers.ViewBuilderV10(context, addresses)
+        return builder.build(instance, is_detail=is_detail)
 
     def _build_list(self, req, instances, is_detail=False):
-        addresses = nova.api.openstack.views.addresses.ViewBuilderV10()
-        builder = nova.api.openstack.views.servers.ViewBuilderV10(addresses)
-        get_nw = self._get_networks_for_instance
-        inst_data = [(inst, get_nw(req, inst)) for inst in instances]
-        return builder.build_list(inst_data, is_detail=is_detail)
+        context = req.environ['nova.context']
+        addresses = views_addresses.ViewBuilderV10()
+        builder = views_servers.ViewBuilderV10(context, addresses)
+        return builder.build_list(instances, is_detail=is_detail)
 
     def _limit_items(self, items, req):
         return common.limited(items, req)
@@ -1081,18 +1075,15 @@ class ControllerV11(Controller):
         return common.get_id_from_href(flavor_ref)
 
     def _build_view(self, req, instance, is_detail=False):
-        project_id = getattr(req.environ['nova.context'], 'project_id', '')
+        context = req.environ['nova.context']
+        project_id = getattr(context, 'project_id', '')
         base_url = req.application_url
-        flavor_builder = nova.api.openstack.views.flavors.ViewBuilderV11(
-            base_url, project_id)
-        image_builder = nova.api.openstack.views.images.ViewBuilderV11(
-            base_url, project_id)
-        addresses_builder = nova.api.openstack.views.addresses.ViewBuilderV11()
-        builder = nova.api.openstack.views.servers.ViewBuilderV11(
-            addresses_builder, flavor_builder, image_builder,
-            base_url, project_id)
-        networks = self._get_networks_for_instance(req, instance)
-        return builder.build(instance, networks, is_detail=is_detail)
+        flavor_builder = views_flavors.ViewBuilderV11(base_url, project_id)
+        image_builder = views_images.ViewBuilderV11(base_url, project_id)
+        addresses_builder = views_addresses.ViewBuilderV11()
+        builder = views_servers.ViewBuilderV11(context, addresses_builder,
+                flavor_builder, image_builder, base_url, project_id)
+        return builder.build(instance, is_detail=is_detail)
 
     def _build_list(self, req, instances, is_detail=False):
         params = req.GET.copy()
@@ -1101,19 +1092,15 @@ class ControllerV11(Controller):
         for key, val in pagination_params.iteritems():
             params[key] = val
 
-        project_id = getattr(req.environ['nova.context'], 'project_id', '')
+        context = req.environ['nova.context']
+        project_id = getattr(context, 'project_id', '')
         base_url = req.application_url
-        flavor_builder = nova.api.openstack.views.flavors.ViewBuilderV11(
-            base_url, project_id)
-        image_builder = nova.api.openstack.views.images.ViewBuilderV11(
-            base_url, project_id)
-        addresses_builder = nova.api.openstack.views.addresses.ViewBuilderV11()
-        builder = nova.api.openstack.views.servers.ViewBuilderV11(
-            addresses_builder, flavor_builder, image_builder,
-            base_url, project_id)
-        get_nw = self._get_networks_for_instance
-        inst_data = [(inst, get_nw(req, inst)) for inst in instances]
-        return builder.build_list(inst_data, is_detail=is_detail, **params)
+        flavor_builder = views_flavors.ViewBuilderV11(base_url, project_id)
+        image_builder = views_images.ViewBuilderV11(base_url, project_id)
+        addresses_builder = views_addresses.ViewBuilderV11()
+        builder = views_servers.ViewBuilderV11(context, addresses_builder,
+                flavor_builder, image_builder, base_url, project_id)
+        return builder.build_list(instances, is_detail=is_detail, **params)
 
     def _action_change_password(self, input_dict, req, id):
         context = req.environ['nova.context']
diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py
index 127a50c78..646cf12ca 100644
--- a/nova/api/openstack/views/servers.py
+++ b/nova/api/openstack/views/servers.py
@@ -19,11 +19,11 @@
 import hashlib
 import os
 
+from nova.api.openstack import common
+from nova.compute import vm_states
 from nova import exception
 from nova import log as logging
 from nova import utils
-from nova.api.openstack import common
-from nova.compute import vm_states
 
 
 LOG = logging.getLogger('nova.api.openstack.views.servers')
@@ -37,16 +37,17 @@ class ViewBuilder(object):
 
     """
 
-    def __init__(self, addresses_builder):
+    def __init__(self, context, addresses_builder):
+        self.context = context
         self.addresses_builder = addresses_builder
 
-    def build(self, inst, networks, is_detail=False):
+    def build(self, inst, is_detail=False):
         """Return a dict that represenst a server."""
         if inst.get('_is_precooked', False):
             server = dict(server=inst)
         else:
             if is_detail:
-                server = self._build_detail(inst, networks)
+                server = self._build_detail(inst)
             else:
                 server = self._build_simple(inst)
 
@@ -59,9 +60,8 @@ class ViewBuilder(object):
         servers = []
         servers_links = []
 
-        for server_obj, networks in server_objs:
-            servers.append(self.build(server_obj, networks,
-                                      is_detail)['server'])
+        for server_obj in server_objs:
+            servers.append(self.build(server_obj, is_detail)['server'])
 
         return dict(servers=servers)
 
@@ -69,7 +69,7 @@ class ViewBuilder(object):
         """Return a simple model of a server."""
         return dict(server=dict(id=inst['id'], name=inst['display_name']))
 
-    def _build_detail(self, inst, networks):
+    def _build_detail(self, inst):
         """Returns a detailed model of a server."""
         vm_state = inst.get('vm_state', vm_states.BUILDING)
         task_state = inst.get('task_state')
@@ -93,6 +93,7 @@ class ViewBuilder(object):
 
         self._build_image(inst_dict, inst)
         self._build_flavor(inst_dict, inst)
+        networks = common.get_networks_for_instance(self.context, inst)
         self._build_addresses(inst_dict, networks)
 
         return dict(server=inst_dict)
@@ -133,16 +134,16 @@ class ViewBuilderV10(ViewBuilder):
 
 class ViewBuilderV11(ViewBuilder):
     """Model an Openstack API V1.0 server response."""
-    def __init__(self, addresses_builder, flavor_builder, image_builder,
-                 base_url, project_id=""):
-        ViewBuilder.__init__(self, addresses_builder)
+    def __init__(self, context, addresses_builder, flavor_builder,
+            image_builder, base_url, project_id=""):
+        super(ViewBuilderV11, self).__init__(context, addresses_builder)
         self.flavor_builder = flavor_builder
         self.image_builder = image_builder
         self.base_url = base_url
         self.project_id = project_id
 
-    def _build_detail(self, inst, network):
-        response = super(ViewBuilderV11, self)._build_detail(inst, network)
+    def _build_detail(self, inst):
+        response = super(ViewBuilderV11, self)._build_detail(inst)
         response['server']['created'] = utils.isotime(inst['created_at'])
         response['server']['updated'] = utils.isotime(inst['updated_at'])
 
@@ -214,9 +215,8 @@ class ViewBuilderV11(ViewBuilder):
         servers = []
         servers_links = []
 
-        for server_obj, networks in server_objs:
-            servers.append(self.build(server_obj, networks,
-                                      is_detail)['server'])
+        for server_obj in server_objs:
+            servers.append(self.build(server_obj, is_detail)['server'])
 
         if (len(servers) and limit) and (limit == len(servers)):
             next_link = self.generate_next_link(servers[-1]['id'],