Merge lp:~rackspace-titan/nova/server-addresses-lp761652 into lp:~hudson-openstack/nova/trunk

[Brian Waldon]

- Present ip addresses in their actual networks, not just a static public/private
- Floating ip addresses are grouped into the networks with their associated fixed ips
- Add addresses attribute to server entities

[Brian Waldon (bcwaldon)]

Just a heads up, the images mapper was duplicated in the v1.0 api router. This branch removes the extra one.

[Matt Dietz (cerberus)]

366 +def require_instance_exists(f):

I'm a little bit concerned about this decorator, not because it isn'ta good idea (it is) but because I worry that people will get lazy with
enforcing an instance actually exists. I realize that it merged in a previous patch,
but regardless, I personally think the responsibility
of it should fall to the code interested in performing something with an instance,
not to the database to enforce.

Beyond that, it seems like it would introduce a bit of unnecessary overhead when someone
is being a good Nova citizen?

I realize you're only using it in a few places currently, but it would only be a matter of time
before it was used everywhere.

What do you think?

At the very least, wouldn't you want to set the name of the wrapper method ala:

399 - new_func.__name__ = func.__name__

With all that said, I won't hold up the review. I'm just genuinely curious

review: Needs Information

[Brian Waldon (bcwaldon)]

> 366 +def require_instance_exists(f):
>
> I'm a little bit concerned about this decorator, not because it isn'ta good
> idea (it is) but because I worry that people will get lazy with
> enforcing an instance actually exists. I realize that it merged in a previous
> patch,
> but regardless, I personally think the responsibility
> of it should fall to the code interested in performing something with an
> instance,
> not to the database to enforce.
>
> Beyond that, it seems like it would introduce a bit of unnecessary overhead
> when someone
> is being a good Nova citizen?

I can definitely agree that it could duplicate the existence check in some cases. However, I feel whomever is adding code to Nova should first see what is available to them. I originally wrote this decorator because I was duplicating code all over the place in the OSAPI. I felt this was a natural place to do this check, but if you think there is a better solution, I would love to hear your thoughts.

> At the very least, wouldn't you want to set the name of the wrapper method
> ala:
>
> 399 - new_func.__name__ = func.__name__

I removed that because none of the other decorators had it, but I think you're right, it should be added back.

[Dan Prince (dan-prince)]

I would side with Matt on this in that the require_instance_exists decorator might be a bit to convenient and risky in that it does the instance_get (which might be more expensive than someone expects because joins are involved, etc.)

The require_instance_exists is only used by the metadata methods in the DB API at the moment. Perhaps instead of using the decorator those functions could be updated to do their updates/deletes/or creates and then check to ensure a record was in fact updated. See the 'instance_type_destroy' as an example. If we did this we can probably avoid the extra SQL hit and get the check for free (after the requested update/create/or delete operation). In cases where this doesn't work maybe just inline the instance_get which is simple enough.

In any case I'm not sure nits on this issue belong in this merge prop. Just wanted to chime in on the conversation since we are having it.

[Matt Dietz (cerberus)]

"In any case I'm not sure nits on this issue belong in this merge prop. Just wanted to chime in on the conversation since we are having it."

You're right. It's not a good enough reason to hold this patch up, but I would like to consider alternatives in the future.

review: Approve

[Dan Prince (dan-prince)]

Looks good.

review: Approve

=== modified file 'nova/api/openstack/__init__.py'
--- nova/api/openstack/__init__.py	2011-06-24 14:37:43 +0000
+++ nova/api/openstack/__init__.py	2011-07-14 18:50:41 +0000
@@ -125,6 +125,10 @@
                         collection={'detail': 'GET'},
                         member=self.server_members)
 
+        mapper.resource("ip", "ips", controller=ips.create_resource(version),
+                        parent_resource=dict(member_name='server',
+                                             collection_name='servers'))
+
         mapper.resource("image", "images",
                         controller=images.create_resource(version),
                         collection={'detail': 'GET'})
@@ -144,9 +148,6 @@
 
     def _setup_routes(self, mapper):
         super(APIRouterV10, self)._setup_routes(mapper, '1.0')
-        mapper.resource("image", "images",
-                        controller=images.create_resource('1.0'),
-                        collection={'detail': 'GET'})
 
         mapper.resource("shared_ip_group", "shared_ip_groups",
                         collection={'detail': 'GET'},
@@ -157,11 +158,6 @@
                         parent_resource=dict(member_name='server',
                         collection_name='servers'))
 
-        mapper.resource("ip", "ips", controller=ips.create_resource(),
-                        collection=dict(public='GET', private='GET'),
-                        parent_resource=dict(member_name='server',
-                                             collection_name='servers'))
-
 
 class APIRouterV11(APIRouter):
     """Define routes specific to OpenStack API V1.1."""
 
=== modified file 'nova/api/openstack/ips.py'
--- nova/api/openstack/ips.py	2011-07-06 20:28:10 +0000
+++ nova/api/openstack/ips.py	2011-07-14 18:50:41 +0000
@@ -23,6 +23,7 @@
 from nova.api.openstack import faults
 import nova.api.openstack.views.addresses
 from nova.api.openstack import wsgi
+from nova import db
 
 
 class Controller(object):
@@ -30,7 +31,6 @@
 
     def __init__(self):
         self.compute_api = nova.compute.API()
-        self.builder = nova.api.openstack.views.addresses.ViewBuilderV10()
 
     def _get_instance(self, req, server_id):
         try:
@@ -40,21 +40,6 @@
             return faults.Fault(exc.HTTPNotFound())
         return instance
 
-    def index(self, req, server_id):
-        instance = self._get_instance(req, server_id)
-        return {'addresses': self.builder.build(instance)}
-
-    def public(self, req, server_id):
-        instance = self._get_instance(req, server_id)
-        return {'public': self.builder.build_public_parts(instance)}
-
-    def private(self, req, server_id):
-        instance = self._get_instance(req, server_id)
-        return {'private': self.builder.build_private_parts(instance)}
-
-    def show(self, req, server_id, id):
-        return faults.Fault(exc.HTTPNotImplemented())
-
     def create(self, req, server_id, body):
         return faults.Fault(exc.HTTPNotImplemented())
 
@@ -62,7 +47,71 @@
         return faults.Fault(exc.HTTPNotImplemented())
 
 
-def create_resource():
+class ControllerV10(Controller):
+
+    def index(self, req, server_id):
+        instance = self._get_instance(req, server_id)
+        builder = nova.api.openstack.views.addresses.ViewBuilderV10()
+        return {'addresses': builder.build(instance)}
+
+    def show(self, req, server_id, id):
+        instance = self._get_instance(req, server_id)
+        builder = self._get_view_builder(req)
+        if id == 'private':
+            view = builder.build_private_parts(instance)
+        elif id == 'public':
+            view = builder.build_public_parts(instance)
+        else:
+            msg = _("Only private and public networks available")
+            return faults.Fault(exc.HTTPNotFound(explanation=msg))
+
+        return {id: view}
+
+    def _get_view_builder(self, req):
+        return nova.api.openstack.views.addresses.ViewBuilderV10()
+
+
+class ControllerV11(Controller):
+
+    def index(self, req, server_id):
+        context = req.environ['nova.context']
+        interfaces = self._get_virtual_interfaces(context, server_id)
+        networks = self._get_view_builder(req).build(interfaces)
+        return {'addresses': networks}
+
+    def show(self, req, server_id, id):
+        context = req.environ['nova.context']
+        interfaces = self._get_virtual_interfaces(context, server_id)
+        network = self._get_view_builder(req).build_network(interfaces, id)
+
+        if network is None:
+            msg = _("Instance is not a member of specified network")
+            return faults.Fault(exc.HTTPNotFound(explanation=msg))
+
+        return network
+
+    def _get_virtual_interfaces(self, context, server_id):
+        try:
+            return db.api.virtual_interface_get_by_instance(context, server_id)
+        except nova.exception.InstanceNotFound:
+            msg = _("Instance does not exist")
+            raise exc.HTTPNotFound(explanation=msg)
+
+    def _get_view_builder(self, req):
+        return nova.api.openstack.views.addresses.ViewBuilderV11()
+
+
+def create_resource(version):
+    controller = {
+        '1.0': ControllerV10,
+        '1.1': ControllerV11,
+    }[version]()
+
+    xmlns = {
+        '1.0': wsgi.XMLNS_V10,
+        '1.1': wsgi.XMLNS_V11,
+    }[version]
+
     metadata = {
         'list_collections': {
             'public':  {'item_name': 'ip', 'item_key': 'addr'},
@@ -72,8 +121,8 @@
 
     body_serializers = {
         'application/xml': wsgi.XMLDictSerializer(metadata=metadata,
-                                                  xmlns=wsgi.XMLNS_V10),
+                                                  xmlns=xmlns),
     }
     serializer = wsgi.ResponseSerializer(body_serializers)
 
-    return wsgi.Resource(Controller(), serializer=serializer)
+    return wsgi.Resource(controller, serializer=serializer)
 
=== modified file 'nova/api/openstack/servers.py'
--- nova/api/openstack/servers.py	2011-07-12 16:13:02 +0000
+++ nova/api/openstack/servers.py	2011-07-14 18:50:41 +0000
@@ -19,6 +19,7 @@
 from webob import exc
 
 from nova import compute
+from nova import db
 from nova import exception
 from nova import flags
 from nova import log as logging
@@ -62,7 +63,7 @@
             return exc.HTTPBadRequest(explanation=str(err))
         return servers
 
-    def _get_view_builder(self, req):
+    def _build_view(self, req, instance, is_detail=False):
         raise NotImplementedError()
 
     def _limit_items(self, items, req):
@@ -88,8 +89,7 @@
                 fixed_ip=fixed_ip,
                 recurse_zones=recurse_zones)
         limited_list = self._limit_items(instance_list, req)
-        builder = self._get_view_builder(req)
-        servers = [builder.build(inst, is_detail)['server']
+        servers = [self._build_view(req, inst, is_detail)['server']
                 for inst in limited_list]
         return dict(servers=servers)
 
@@ -99,8 +99,7 @@
         try:
             instance = self.compute_api.routing_get(
                 req.environ['nova.context'], id)
-            builder = self._get_view_builder(req)
-            return builder.build(instance, is_detail=True)
+            return self._build_view(req, instance, is_detail=True)
         except exception.NotFound:
             return faults.Fault(exc.HTTPNotFound())
 
@@ -121,8 +120,7 @@
         for key in ['instance_type', 'image_ref']:
             inst[key] = extra_values[key]
 
-        builder = self._get_view_builder(req)
-        server = builder.build(inst, is_detail=True)
+        server = self._build_view(req, inst, is_detail=True)
         server['server']['adminPass'] = extra_values['password']
         return server
 
@@ -426,10 +424,10 @@
     def _flavor_id_from_req_data(self, data):
         return data['server']['flavorId']
 
-    def _get_view_builder(self, req):
-        addresses_builder = nova.api.openstack.views.addresses.ViewBuilderV10()
-        return nova.api.openstack.views.servers.ViewBuilderV10(
-            addresses_builder)
+    def _build_view(self, req, instance, is_detail=False):
+        addresses = nova.api.openstack.views.addresses.ViewBuilderV10()
+        builder = nova.api.openstack.views.servers.ViewBuilderV10(addresses)
+        return builder.build(instance, is_detail=is_detail)
 
     def _limit_items(self, items, req):
         return common.limited(items, req)
@@ -498,16 +496,18 @@
         href = data['server']['flavorRef']
         return common.get_id_from_href(href)
 
-    def _get_view_builder(self, req):
+    def _build_view(self, req, instance, is_detail=False):
         base_url = req.application_url
         flavor_builder = nova.api.openstack.views.flavors.ViewBuilderV11(
             base_url)
         image_builder = nova.api.openstack.views.images.ViewBuilderV11(
             base_url)
         addresses_builder = nova.api.openstack.views.addresses.ViewBuilderV11()
-        return nova.api.openstack.views.servers.ViewBuilderV11(
+        builder = nova.api.openstack.views.servers.ViewBuilderV11(
             addresses_builder, flavor_builder, image_builder, base_url)
 
+        return builder.build(instance, is_detail=is_detail)
+
     def _action_change_password(self, input_dict, req, id):
         context = req.environ['nova.context']
         if (not 'changePassword' in input_dict
 
=== modified file 'nova/api/openstack/views/addresses.py'
--- nova/api/openstack/views/addresses.py	2011-06-30 19:42:51 +0000
+++ nova/api/openstack/views/addresses.py	2011-07-14 18:50:41 +0000
@@ -20,13 +20,14 @@
 
 
 class ViewBuilder(object):
-    ''' Models a server addresses response as a python dictionary.'''
+    """Models a server addresses response as a python dictionary."""
 
     def build(self, inst):
         raise NotImplementedError()
 
 
 class ViewBuilderV10(ViewBuilder):
+
     def build(self, inst):
         private_ips = self.build_private_parts(inst)
         public_ips = self.build_public_parts(inst)
@@ -40,11 +41,31 @@
 
 
 class ViewBuilderV11(ViewBuilder):
-    def build(self, inst):
-        # TODO(tr3buchet) - this shouldn't be hard coded to 4...
-        private_ips = utils.get_from_path(inst, 'fixed_ips/address')
-        private_ips = [dict(version=4, addr=a) for a in private_ips]
-        public_ips = utils.get_from_path(inst,
-                                         'fixed_ips/floating_ips/address')
-        public_ips = [dict(version=4, addr=a) for a in public_ips]
-        return dict(public=public_ips, private=private_ips)
+
+    def build(self, interfaces):
+        networks = {}
+        for interface in interfaces:
+            network_label = interface['network']['label']
+
+            if network_label not in networks:
+                networks[network_label] = []
+
+            networks[network_label].extend(self._extract_ipv4(interface))
+
+        return networks
+
+    def build_network(self, interfaces, network_label):
+        for interface in interfaces:
+            if interface['network']['label'] == network_label:
+                ips = self._extract_ipv4(interface)
+                return {network_label: list(ips)}
+        return None
+
+    def _extract_ipv4(self, interface):
+        for fixed_ip in interface['fixed_ips']:
+            yield self._build_ip_entity(fixed_ip['address'], 4)
+            for floating_ip in fixed_ip.get('floating_ips', []):
+                yield self._build_ip_entity(floating_ip['address'], 4)
+
+    def _build_ip_entity(self, address, version):
+        return {'addr': address, 'version': version}
 
=== modified file 'nova/api/openstack/views/servers.py'
--- nova/api/openstack/views/servers.py	2011-07-07 13:42:13 +0000
+++ nova/api/openstack/views/servers.py	2011-07-14 18:50:41 +0000
@@ -77,7 +77,6 @@
         inst_dict = {
             'id': inst['id'],
             'name': inst['display_name'],
-            'addresses': self.addresses_builder.build(inst),
             'status': power_mapping[inst.get('state')]}
 
         ctxt = nova.context.get_admin_context()
@@ -98,10 +97,15 @@
 
         self._build_image(inst_dict, inst)
         self._build_flavor(inst_dict, inst)
+        self._build_addresses(inst_dict, inst)
 
         inst_dict['uuid'] = inst['uuid']
         return dict(server=inst_dict)
 
+    def _build_addresses(self, response, inst):
+        """Return the addresses sub-resource of a server."""
+        raise NotImplementedError()
+
     def _build_image(self, response, inst):
         """Return the image sub-resource of a server."""
         raise NotImplementedError()
@@ -128,6 +132,9 @@
         if 'instance_type' in dict(inst):
             response['flavorId'] = inst['instance_type']['flavorid']
 
+    def _build_addresses(self, response, inst):
+        response['addresses'] = self.addresses_builder.build(inst)
+
 
 class ViewBuilderV11(ViewBuilder):
     """Model an Openstack API V1.0 server response."""
@@ -151,6 +158,10 @@
             flavor_ref = self.flavor_builder.generate_href(flavor_id)
             response["flavorRef"] = flavor_ref
 
+    def _build_addresses(self, response, inst):
+        interfaces = inst.get('virtual_interfaces', [])
+        response['addresses'] = self.addresses_builder.build(interfaces)
+
     def _build_extra(self, response, inst):
         self._build_links(response, inst)
 
 
=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py	2011-07-01 15:07:08 +0000
+++ nova/db/sqlalchemy/api.py	2011-07-14 18:50:41 +0000
@@ -118,8 +118,23 @@
     return wrapper
 
 
+def require_instance_exists(f):
+    """Decorator to require the specified instance to exist.
+
+    Requres the wrapped function to use context and instance_id as
+    their first two arguments.
+    """
+
+    def wrapper(context, instance_id, *args, **kwargs):
+        db.api.instance_get(context, instance_id)
+        return f(context, instance_id, *args, **kwargs)
+    wrapper.__name__ = f.__name__
+    return wrapper
+
+
 ###################
 
+
 @require_admin_context
 def service_destroy(context, service_id):
     session = get_session()
@@ -921,6 +936,7 @@
 
 
 @require_context
+@require_instance_exists
 def virtual_interface_get_by_instance(context, instance_id):
     """Gets all virtual interfaces for instance.
 
@@ -3071,14 +3087,6 @@
 ####################
 
 
-def require_instance_exists(func):
-    def new_func(context, instance_id, *args, **kwargs):
-        db.api.instance_get(context, instance_id)
-        return func(context, instance_id, *args, **kwargs)
-    new_func.__name__ = func.__name__
-    return new_func
-
-
 @require_context
 @require_instance_exists
 def instance_metadata_get(context, instance_id):
 
=== modified file 'nova/tests/api/openstack/test_servers.py'
--- nova/tests/api/openstack/test_servers.py	2011-07-11 21:26:46 +0000
+++ nova/tests/api/openstack/test_servers.py	2011-07-14 18:50:41 +0000
@@ -65,6 +65,18 @@
     return stub_instance(id, uuid=uuid)
 
 
+def return_virtual_interface_by_instance(interfaces):
+    def _return_virtual_interface_by_instance(context, instance_id):
+        return interfaces
+    return _return_virtual_interface_by_instance
+
+
+def return_virtual_interface_instance_nonexistant(interfaces):
+    def _return_virtual_interface_by_instance(context, instance_id):
+        raise exception.InstanceNotFound(instance_id=instance_id)
+    return _return_virtual_interface_by_instance
+
+
 def return_server_with_addresses(private, public):
     def _return_server(context, id):
         return stub_instance(id, private_address=private,
@@ -72,6 +84,12 @@
     return _return_server
 
 
+def return_server_with_interfaces(interfaces):
+    def _return_server(context, id):
+        return stub_instance(id, interfaces=interfaces)
+    return _return_server
+
+
 def return_server_with_power_state(power_state):
     def _return_server(context, id):
         return stub_instance(id, power_state=power_state)
@@ -124,10 +142,13 @@
 
 def stub_instance(id, user_id=1, private_address=None, public_addresses=None,
                   host=None, power_state=0, reservation_id="",
-                  uuid=FAKE_UUID):
+                  uuid=FAKE_UUID, interfaces=None):
     metadata = []
     metadata.append(InstanceMetadata(key='seq', value=id))
 
+    if interfaces is None:
+        interfaces = []
+
     inst_type = instance_types.get_instance_type_by_flavor_id(1)
 
     if public_addresses is None:
@@ -171,7 +192,8 @@
         "display_description": "",
         "locked": False,
         "metadata": metadata,
-        "uuid": uuid}
+        "uuid": uuid,
+        "virtual_interfaces": interfaces}
 
     instance["fixed_ips"] = {
         "address": private_address,
@@ -411,23 +433,152 @@
         self.assertEquals(ip.getAttribute('addr'), private)
 
     def test_get_server_by_id_with_addresses_v1_1(self):
-        private = "192.168.0.3"
-        public = ["1.2.3.4"]
-        new_return_server = return_server_with_addresses(private, public)
+        interfaces = [
+            {
+                'network': {'label': 'network_1'},
+                'fixed_ips': [
+                    {'address': '192.168.0.3'},
+                    {'address': '192.168.0.4'},
+                ],
+            },
+            {
+                'network': {'label': 'network_2'},
+                'fixed_ips': [
+                    {'address': '172.19.0.1'},
+                    {'address': '172.19.0.2'},
+                ],
+            },
+        ]
+        new_return_server = return_server_with_interfaces(interfaces)
         self.stubs.Set(nova.db.api, 'instance_get', new_return_server)
+
         req = webob.Request.blank('/v1.1/servers/1')
         res = req.get_response(fakes.wsgi_app())
+
         res_dict = json.loads(res.body)
         self.assertEqual(res_dict['server']['id'], 1)
         self.assertEqual(res_dict['server']['name'], 'server1')
         addresses = res_dict['server']['addresses']
-        # RM(4047): Figure otu what is up with the 1.1 api and multi-nic
-        #self.assertEqual(len(addresses["public"]), len(public))
-        #self.assertEqual(addresses["public"][0],
-        #    {"version": 4, "addr": public[0]})
-        #self.assertEqual(len(addresses["private"]), 1)
-        #self.assertEqual(addresses["private"][0],
-        #    {"version": 4, "addr": private})
+        expected = {
+            'network_1': [
+                {'addr': '192.168.0.3', 'version': 4},
+                {'addr': '192.168.0.4', 'version': 4},
+            ],
+            'network_2': [
+                {'addr': '172.19.0.1', 'version': 4},
+                {'addr': '172.19.0.2', 'version': 4},
+            ],
+        }
+
+        self.assertEqual(addresses, expected)
+
+    def test_get_server_addresses_v1_1(self):
+        interfaces = [
+            {
+                'network': {'label': 'network_1'},
+                'fixed_ips': [
+                    {'address': '192.168.0.3'},
+                    {'address': '192.168.0.4'},
+                ],
+            },
+            {
+                'network': {'label': 'network_2'},
+                'fixed_ips': [
+                    {
+                        'address': '172.19.0.1',
+                        'floating_ips': [
+                            {'address': '1.2.3.4'},
+                        ],
+                    },
+                    {'address': '172.19.0.2'},
+                ],
+            },
+        ]
+
+        _return_vifs = return_virtual_interface_by_instance(interfaces)
+        self.stubs.Set(nova.db.api,
+                       'virtual_interface_get_by_instance',
+                       _return_vifs)
+
+        req = webob.Request.blank('/v1.1/servers/1/ips')
+        res = req.get_response(fakes.wsgi_app())
+        res_dict = json.loads(res.body)
+
+        expected = {
+            'addresses': {
+                'network_1': [
+                    {'version': 4, 'addr': '192.168.0.3'},
+                    {'version': 4, 'addr': '192.168.0.4'},
+                ],
+                'network_2': [
+                    {'version': 4, 'addr': '172.19.0.1'},
+                    {'version': 4, 'addr': '1.2.3.4'},
+                    {'version': 4, 'addr': '172.19.0.2'},
+                ],
+            },
+        }
+
+        self.assertEqual(res_dict, expected)
+
+    def test_get_server_addresses_single_network_v1_1(self):
+        interfaces = [
+            {
+                'network': {'label': 'network_1'},
+                'fixed_ips': [
+                    {'address': '192.168.0.3'},
+                    {'address': '192.168.0.4'},
+                ],
+            },
+            {
+                'network': {'label': 'network_2'},
+                'fixed_ips': [
+                    {
+                        'address': '172.19.0.1',
+                        'floating_ips': [
+                            {'address': '1.2.3.4'},
+                        ],
+                    },
+                    {'address': '172.19.0.2'},
+                ],
+            },
+        ]
+        _return_vifs = return_virtual_interface_by_instance(interfaces)
+        self.stubs.Set(nova.db.api,
+                       'virtual_interface_get_by_instance',
+                       _return_vifs)
+
+        req = webob.Request.blank('/v1.1/servers/1/ips/network_2')
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
+        res_dict = json.loads(res.body)
+        expected = {
+            'network_2': [
+                {'version': 4, 'addr': '172.19.0.1'},
+                {'version': 4, 'addr': '1.2.3.4'},
+                {'version': 4, 'addr': '172.19.0.2'},
+            ],
+        }
+        self.assertEqual(res_dict, expected)
+
+    def test_get_server_addresses_nonexistant_network_v1_1(self):
+        _return_vifs = return_virtual_interface_by_instance([])
+        self.stubs.Set(nova.db.api,
+                       'virtual_interface_get_by_instance',
+                       _return_vifs)
+
+        req = webob.Request.blank('/v1.1/servers/1/ips/network_0')
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 404)
+
+    def test_get_server_addresses_nonexistant_server_v1_1(self):
+        _return_vifs = return_virtual_interface_instance_nonexistant([])
+        self.stubs.Set(nova.db.api,
+                       'virtual_interface_get_by_instance',
+                       _return_vifs)
+
+        req = webob.Request.blank('/v1.1/servers/600/ips')
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 404)
 
     def test_get_server_list(self):
         req = webob.Request.blank('/v1.0/servers')
@@ -787,13 +938,13 @@
 
         res = req.get_response(fakes.wsgi_app())
 
+        self.assertEqual(res.status_int, 200)
         server = json.loads(res.body)['server']
         self.assertEqual(16, len(server['adminPass']))
         self.assertEqual('server_test', server['name'])
         self.assertEqual(1, server['id'])
         self.assertEqual(flavor_ref, server['flavorRef'])
         self.assertEqual(image_href, server['imageRef'])
-        self.assertEqual(res.status_int, 200)
 
     def test_create_instance_v1_1_bad_href(self):
         self._setup_for_create_instance()