Merge "Expose Asynchronous Fault entity in the OSAPI"

9 files changed, 326 insertions, 62 deletions

diff --git a/nova/api/openstack/v2/schemas/v1.1/server.rng b/nova/api/openstack/v2/schemas/v1.1/server.rng
index 2e86ccffe..07fa16daa 100644
--- a/nova/api/openstack/v2/schemas/v1.1/server.rng
+++ b/nova/api/openstack/v2/schemas/v1.1/server.rng
@@ -1,20 +1,20 @@
 <element name="server" ns="http://docs.openstack.org/compute/api/v1.1"
     xmlns="http://relaxng.org/ns/structure/1.0">
-  <attribute name="name">      <text/> </attribute>
-  <attribute name="userId">      <text/> </attribute>
-  <attribute name="tenantId">      <text/> </attribute>
-  <attribute name="id">      <text/> </attribute>
-  <attribute name="updated">   <text/> </attribute>
-  <attribute name="created">   <text/> </attribute>
-  <attribute name="hostId">   <text/> </attribute>
-  <attribute name="accessIPv4">   <text/> </attribute>
-  <attribute name="accessIPv6">   <text/> </attribute>
-  <attribute name="status">   <text/> </attribute>
+  <attribute name="name">       <text/> </attribute>
+  <attribute name="userId">     <text/> </attribute>
+  <attribute name="tenantId">   <text/> </attribute>
+  <attribute name="id">         <text/> </attribute>
+  <attribute name="updated">    <text/> </attribute>
+  <attribute name="created">    <text/> </attribute>
+  <attribute name="hostId">     <text/> </attribute>
+  <attribute name="accessIPv4"> <text/> </attribute>
+  <attribute name="accessIPv6"> <text/> </attribute>
+  <attribute name="status">     <text/> </attribute>
   <optional>
-    <attribute name="progress">   <text/> </attribute>
+    <attribute name="progress"> <text/> </attribute>
   </optional>
   <optional>
-    <attribute name="adminPass">   <text/> </attribute>
+    <attribute name="adminPass"><text/> </attribute>
   </optional>
   <element name="image">
     <attribute name="id"> <text/> </attribute>
@@ -24,6 +24,14 @@
     <attribute name="id"> <text/> </attribute>
     <externalRef href="../atom-link.rng"/>
   </element>
+  <optional>
+    <element name="fault">
+      <attribute name="code">    <text/> </attribute>
+      <attribute name="created"> <text/> </attribute>
+      <element name="message">   <text/> </element>
+      <element name="details">   <text/> </element>
+    </element>
+  </optional>
   <element name="metadata">
     <zeroOrMore>
       <element name="meta">
@@ -39,7 +47,7 @@
           <zeroOrMore>
             <element name="ip">
               <attribute name="version"> <text/> </attribute>
-              <attribute name="addr"> <text/> </attribute>
+              <attribute name="addr">    <text/> </attribute>
             </element>
           </zeroOrMore>
       </element>
diff --git a/nova/api/openstack/v2/servers.py b/nova/api/openstack/v2/servers.py
index b149c2cf9..049a17ef6 100644
--- a/nova/api/openstack/v2/servers.py
+++ b/nova/api/openstack/v2/servers.py
@@ -16,28 +16,21 @@
 
 import base64
 import os
-import traceback
 
-from lxml import etree
 from webob import exc
 import webob
 from xml.dom import minidom
 
 from nova.api.openstack import common
 from nova.api.openstack.v2 import ips
-from nova.api.openstack.v2.views import addresses as views_addresses
-from nova.api.openstack.v2.views import flavors as views_flavors
-from nova.api.openstack.v2.views import images as views_images
 from nova.api.openstack.v2.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.rpc import common as rpc_common
 from nova.scheduler import api as scheduler_api
@@ -92,6 +85,18 @@ class Controller(wsgi.Controller):
         """
         return None
 
+    def _add_instance_faults(self, ctxt, instances):
+        faults = self.compute_api.get_instance_faults(ctxt, instances)
+        if faults is not None:
+            for instance in instances:
+                faults_list = faults.get(instance['uuid'], [])
+                try:
+                    instance['fault'] = faults_list[0]
+                except IndexError:
+                    pass
+
+        return instances
+
     def _get_servers(self, req, is_detail):
         """Returns a list of servers, taking into account any search
         options specified.
@@ -142,6 +147,7 @@ class Controller(wsgi.Controller):
 
         limited_list = self._limit_items(instance_list, req)
         if is_detail:
+            self._add_instance_faults(context, limited_list)
             return self._view_builder.detail(req, limited_list)
         else:
             return self._view_builder.index(req, limited_list)
@@ -297,8 +303,9 @@ class Controller(wsgi.Controller):
     def show(self, req, id):
         """ Returns server details by server id """
         try:
-            instance = self.compute_api.routing_get(
-                req.environ['nova.context'], id)
+            context = req.environ['nova.context']
+            instance = self.compute_api.routing_get(context, id)
+            self._add_instance_faults(context, [instance])
             return self._view_builder.show(req, instance)
         except exception.NotFound:
             raise exc.HTTPNotFound()
@@ -503,6 +510,7 @@ class Controller(wsgi.Controller):
 
         instance.update(update_dict)
 
+        self._add_instance_faults(ctxt, [instance])
         return self._view_builder.show(req, instance)
 
     @exception.novaclient_converter
@@ -796,6 +804,7 @@ class Controller(wsgi.Controller):
             raise exc.HTTPNotFound(explanation=msg)
 
         instance = self._get_server(context, instance_id)
+        self._add_instance_faults(context, [instance])
         view = self._view_builder.show(request, instance)
         view['server']['adminPass'] = password
 
@@ -882,6 +891,16 @@ class SecurityGroupsTemplateElement(xmlutil.TemplateElement):
         return 'security_groups' in datum
 
 
+def make_fault(elem):
+    fault = xmlutil.SubTemplateElement(elem, 'fault', selector='fault')
+    fault.set('code')
+    fault.set('created')
+    msg = xmlutil.SubTemplateElement(fault, 'message')
+    msg.text = 'message'
+    det = xmlutil.SubTemplateElement(fault, 'details')
+    det.text = 'details'
+
+
 def make_server(elem, detailed=False):
     elem.set('name')
     elem.set('id')
@@ -907,6 +926,9 @@ def make_server(elem, detailed=False):
         flavor.set('id')
         xmlutil.make_links(flavor, 'links')
 
+        # Attach fault node
+        make_fault(elem)
+
         # Attach metadata node
         elem.append(common.MetadataTemplate())
 
diff --git a/nova/api/openstack/v2/views/servers.py b/nova/api/openstack/v2/views/servers.py
index 979be930f..859bd48ab 100644
--- a/nova/api/openstack/v2/views/servers.py
+++ b/nova/api/openstack/v2/views/servers.py
@@ -42,6 +42,10 @@ class ViewBuilder(common.ViewBuilder):
         "VERIFY_RESIZE",
     )
 
+    _fault_statuses = (
+        "ERROR",
+    )
+
     def __init__(self):
         """Initialize view builder."""
         super(ViewBuilder, self).__init__()
@@ -101,6 +105,9 @@ class ViewBuilder(common.ViewBuilder):
                 "links": self._get_links(request, instance["uuid"]),
             },
         }
+        _inst_fault = self._get_fault(request, instance)
+        if server["server"]["status"] in self._fault_statuses and _inst_fault:
+            server['server']['fault'] = _inst_fault
 
         if server["server"]["status"] in self._progress_statuses:
             server["server"]["progress"] = instance.get("progress", 0)
@@ -109,13 +116,11 @@ class ViewBuilder(common.ViewBuilder):
 
     def index(self, request, instances):
         """Show a list of servers without many details."""
-        list_func = self.basic
-        return self._list_view(list_func, request, instances)
+        return self._list_view(self.basic, request, instances)
 
     def detail(self, request, instances):
         """Detailed view of a list of instance."""
-        list_func = self.show
-        return self._list_view(list_func, request, instances)
+        return self._list_view(self.show, request, instances)
 
     def _list_view(self, func, request, servers):
         """Provide a view for a list of servers."""
@@ -173,3 +178,16 @@ class ViewBuilder(common.ViewBuilder):
                 "href": flavor_bookmark,
             }],
         }
+
+    def _get_fault(self, request, instance):
+        fault = instance.get("fault", None)
+
+        if not fault:
+            return None
+
+        return {
+            "code": fault["code"],
+            "created": utils.isotime(fault["created_at"]),
+            "message": fault["message"],
+            "details": fault["details"],
+        }
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 385d3d682..3b79e660d 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -1601,3 +1601,8 @@ class API(base.Base):
         self.db.instance_metadata_update(context, instance['id'],
                                          _metadata, True)
         return _metadata
+
+    def get_instance_faults(self, context, instances):
+        """Get all faults for a list of instance uuids."""
+        uuids = [instance['uuid'] for instance in instances]
+        return self.db.instance_fault_get_by_instance_uuids(context, uuids)
diff --git a/nova/db/api.py b/nova/db/api.py
index 8637b7fad..32b3f867d 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -1766,6 +1766,6 @@ def instance_fault_create(context, values):
     return IMPL.instance_fault_create(context, values)
 
 
-def instance_fault_get_by_instance(context, instance_uuid):
-    """Get first instance fault with the given instance uuid."""
-    return IMPL.instance_fault_get_by_instance(context, instance_uuid)
+def instance_fault_get_by_instance_uuids(context, instance_uuids):
+    """Get all instance faults for the provided instance_uuids."""
+    return IMPL.instance_fault_get_by_instance_uuids(context, instance_uuids)
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 4e1910e48..116e74b60 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -4021,16 +4021,27 @@ def sm_volume_get_all(context):
 
 
 def instance_fault_create(context, values):
-    """Create a new Instance Fault."""
+    """Create a new InstanceFault."""
     fault_ref = models.InstanceFault()
     fault_ref.update(values)
     fault_ref.save()
-    return fault_ref
+    return dict(fault_ref.iteritems())
 
 
-def instance_fault_get_by_instance(context, instance_uuid):
-    """Get first instance fault with the given instance uuid."""
-    return model_query(context, models.InstanceFault, read_deleted='no').\
-                filter_by(instance_uuid=instance_uuid).\
-                order_by(desc("created_at")).\
-                first()
+def instance_fault_get_by_instance_uuids(context, instance_uuids):
+    """Get all instance faults for the provided instance_uuids."""
+    rows = model_query(context, models.InstanceFault, read_deleted='no').\
+                       filter(models.InstanceFault.instance_uuid.in_(
+                           instance_uuids)).\
+                       order_by(desc("created_at")).\
+                       all()
+
+    output = {}
+    for instance_uuid in instance_uuids:
+        output[instance_uuid] = []
+
+    for row in rows:
+        data = dict(row.iteritems())
+        output[row['instance_uuid']].append(data)
+
+    return output
diff --git a/nova/tests/api/openstack/v2/test_servers.py b/nova/tests/api/openstack/v2/test_servers.py
index 243438ca5..23621fd7f 100644
--- a/nova/tests/api/openstack/v2/test_servers.py
+++ b/nova/tests/api/openstack/v2/test_servers.py
@@ -2447,6 +2447,158 @@ class ServersViewBuilderTest(test.TestCase):
         output = self.view_builder.show(self.request, self.instance)
         self.assertDictMatch(output, expected_server)
 
+    def test_build_server_detail_with_fault(self):
+        self.instance['vm_state'] = vm_states.ERROR
+        self.instance['fault'] = {
+            'code': 404,
+            'instance_uuid': self.uuid,
+            'message': "HTTPNotFound",
+            'details': "Stock details for test",
+            'created_at': datetime.datetime(2010, 10, 10, 12, 0, 0),
+        }
+
+        image_bookmark = "http://localhost/fake/images/5"
+        flavor_bookmark = "http://localhost/fake/flavors/1"
+        self_link = "http://localhost/v2/fake/servers/%s" % self.uuid
+        bookmark_link = "http://localhost/fake/servers/%s" % self.uuid
+        expected_server = {
+            "server": {
+                "id": self.uuid,
+                "user_id": "fake",
+                "tenant_id": "fake",
+                "updated": "2010-11-11T11:00:00Z",
+                "created": "2010-10-10T12:00:00Z",
+                "name": "test_server",
+                "status": "ERROR",
+                "accessIPv4": "",
+                "accessIPv6": "",
+                "hostId": '',
+                "key_name": '',
+                "image": {
+                    "id": "5",
+                    "links": [
+                        {
+                            "rel": "bookmark",
+                            "href": image_bookmark,
+                        },
+                    ],
+                },
+                "flavor": {
+                    "id": "1",
+                  "links": [
+                                            {
+                          "rel": "bookmark",
+                          "href": flavor_bookmark,
+                      },
+                  ],
+                },
+                "addresses": {
+                    'private': [
+                        {'version': 4, 'addr': '172.19.0.1'}
+                    ],
+                    'public': [
+                        {'version': 6, 'addr': 'b33f::fdee:ddff:fecc:bbaa'},
+                        {'version': 4, 'addr': '192.168.0.3'},
+                    ],
+                },
+                "metadata": {},
+                "config_drive": None,
+                "links": [
+                    {
+                        "rel": "self",
+                        "href": self_link,
+                    },
+                    {
+                        "rel": "bookmark",
+                        "href": bookmark_link,
+                    },
+                ],
+                "fault": {
+                    "code": 404,
+                    "created": "2010-10-10T12:00:00Z",
+                    "message": "HTTPNotFound",
+                    "details": "Stock details for test",
+                },
+            }
+        }
+
+        output = self.view_builder.show(self.request, self.instance)
+        self.assertDictMatch(output, expected_server)
+
+    def test_build_server_detail_with_fault_but_active(self):
+        self.instance['vm_state'] = vm_states.ACTIVE
+        self.instance['progress'] = 100
+        self.instance['fault'] = {
+            'code': 404,
+            'instance_uuid': self.uuid,
+            'message': "HTTPNotFound",
+            'details': "Stock details for test",
+            'created_at': datetime.datetime(2010, 10, 10, 12, 0, 0),
+        }
+
+        image_bookmark = "http://localhost/fake/images/5"
+        flavor_bookmark = "http://localhost/fake/flavors/1"
+        self_link = "http://localhost/v2/fake/servers/%s" % self.uuid
+        bookmark_link = "http://localhost/fake/servers/%s" % self.uuid
+        expected_server = {
+            "server": {
+                "id": self.uuid,
+                "user_id": "fake",
+                "tenant_id": "fake",
+                "updated": "2010-11-11T11:00:00Z",
+                "created": "2010-10-10T12:00:00Z",
+                "progress": 100,
+                "name": "test_server",
+                "status": "ACTIVE",
+                "accessIPv4": "",
+                "accessIPv6": "",
+                "hostId": '',
+                "key_name": '',
+                "image": {
+                    "id": "5",
+                    "links": [
+                        {
+                            "rel": "bookmark",
+                            "href": image_bookmark,
+                        },
+                    ],
+                },
+                "flavor": {
+                    "id": "1",
+                  "links": [
+                                            {
+                          "rel": "bookmark",
+                          "href": flavor_bookmark,
+                      },
+                  ],
+                },
+                "addresses": {
+                    'private': [
+                        {'version': 4, 'addr': '172.19.0.1'}
+                    ],
+                    'public': [
+                        {'version': 6, 'addr': 'b33f::fdee:ddff:fecc:bbaa'},
+                        {'version': 4, 'addr': '192.168.0.3'},
+                    ],
+                },
+                "metadata": {},
+                "config_drive": None,
+                "links": [
+                    {
+                        "rel": "self",
+                        "href": self_link,
+                    },
+                    {
+                        "rel": "bookmark",
+                        "href": bookmark_link,
+                    },
+                ],
+            }
+        }
+
+        output = self.view_builder.show(self.request, self.instance)
+        self.assertDictMatch(output, expected_server)
+
     def test_build_server_detail_active_status(self):
         #set the power state of the instance to running
         self.instance['vm_state'] = vm_states.ACTIVE
@@ -3457,6 +3609,12 @@ class ServerXMLSerializationTest(test.TestCase):
                         'rel': 'bookmark',
                     },
                 ],
+                "fault": {
+                    "code": 500,
+                    "created": self.TIMESTAMP,
+                    "message": "Error Message",
+                    "details": "Fault details",
+                }
             }
         }
 
@@ -3517,6 +3675,15 @@ class ServerXMLSerializationTest(test.TestCase):
                 self.assertEqual(str(ip_elem.get('addr')),
                                  str(ip['addr']))
 
+        fault_root = root.find('{0}fault'.format(NS))
+        fault_dict = server_dict['fault']
+        self.assertEqual(fault_root.get("code"), str(fault_dict["code"]))
+        self.assertEqual(fault_root.get("created"), fault_dict["created"])
+        msg_elem = fault_root.find('{0}message'.format(NS))
+        self.assertEqual(msg_elem.text, fault_dict["message"])
+        det_elem = fault_root.find('{0}details'.format(NS))
+        self.assertEqual(det_elem.text, fault_dict["details"])
+
     def test_action(self):
         serializer = servers.ServerXMLSerializer()
 
diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py
index 883ee0173..b70f444e9 100644
--- a/nova/tests/test_compute.py
+++ b/nova/tests/test_compute.py
@@ -20,6 +20,7 @@
 Tests For Compute
 """
 from copy import copy
+import datetime
 from webob import exc
 
 import mox
@@ -2262,6 +2263,32 @@ class ComputeAPITestCase(BaseTestCase):
 
         db.instance_destroy(_context, instance['uuid'])
 
+    def test_get_instance_faults(self):
+        """Get an instances latest fault"""
+        instance = self._create_fake_instance()
+
+        fault_fixture = {
+                'code': 404,
+                'instance_uuid': instance['uuid'],
+                'message': "HTTPNotFound",
+                'details': "Stock details for test",
+                'created_at': datetime.datetime(2010, 10, 10, 12, 0, 0),
+            }
+
+        def return_fault(_ctxt, instance_uuids):
+            return dict.fromkeys(instance_uuids, [fault_fixture])
+
+        self.stubs.Set(nova.db,
+                       'instance_fault_get_by_instance_uuids',
+                       return_fault)
+
+        _context = context.get_admin_context()
+        output = self.compute_api.get_instance_faults(_context, [instance])
+        expected = {instance['uuid']: [fault_fixture]}
+        self.assertEqual(output, expected)
+
+        db.instance_destroy(_context, instance['uuid'])
+
     @staticmethod
     def _parse_db_block_device_mapping(bdm_ref):
         attr_list = ('delete_on_termination', 'device_name', 'no_device',
diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py
index 36b5eced4..314a9f276 100644
--- a/nova/tests/test_db_api.py
+++ b/nova/tests/test_db_api.py
@@ -209,58 +209,64 @@ class DbApiTestCase(test.TestCase):
         db.instance_fault_create(ctxt, fault_values)
 
         # Retrieve the fault to ensure it was successfully added
-        instance_fault = db.instance_fault_get_by_instance(ctxt, uuid)
-        self.assertEqual(404, instance_fault['code'])
+        faults = db.instance_fault_get_by_instance_uuids(ctxt, [uuid])
+        self.assertEqual(404, faults[uuid][0]['code'])
 
     def test_instance_fault_get_by_instance(self):
         """ ensure we can retrieve an instance fault by  instance UUID """
         ctxt = context.get_admin_context()
+        instance1 = db.instance_create(ctxt, {})
+        instance2 = db.instance_create(ctxt, {})
+        uuids = [instance1['uuid'], instance2['uuid']]
 
         # Create faults
-        uuid = str(utils.gen_uuid())
         fault_values = {
             'message': 'message',
             'details': 'detail',
-            'instance_uuid': uuid,
+            'instance_uuid': uuids[0],
             'code': 404,
         }
-        db.instance_fault_create(ctxt, fault_values)
+        fault1 = db.instance_fault_create(ctxt, fault_values)
 
-        uuid2 = str(utils.gen_uuid())
         fault_values = {
             'message': 'message',
             'details': 'detail',
-            'instance_uuid': uuid2,
+            'instance_uuid': uuids[0],
             'code': 500,
         }
-        db.instance_fault_create(ctxt, fault_values)
-
-        # Retrieve the fault to ensure it was successfully added
-        instance_fault = db.instance_fault_get_by_instance(ctxt, uuid2)
-        self.assertEqual(500, instance_fault['code'])
-
-    def test_instance_fault_get_by_instance_first_fault(self):
-        """Instance_fault_get_by_instance should return the latest fault """
-        ctxt = context.get_admin_context()
+        fault2 = db.instance_fault_create(ctxt, fault_values)
 
-        # Create faults
-        uuid = str(utils.gen_uuid())
         fault_values = {
             'message': 'message',
             'details': 'detail',
-            'instance_uuid': uuid,
+            'instance_uuid': uuids[1],
             'code': 404,
         }
-        db.instance_fault_create(ctxt, fault_values)
+        fault3 = db.instance_fault_create(ctxt, fault_values)
 
         fault_values = {
             'message': 'message',
             'details': 'detail',
-            'instance_uuid': uuid,
+            'instance_uuid': uuids[1],
             'code': 500,
         }
-        db.instance_fault_create(ctxt, fault_values)
+        fault4 = db.instance_fault_create(ctxt, fault_values)
 
-        # Retrieve the fault to ensure it was successfully added
-        instance_fault = db.instance_fault_get_by_instance(ctxt, uuid)
-        self.assertEqual(500, instance_fault['code'])
+        instance_faults = db.instance_fault_get_by_instance_uuids(ctxt, uuids)
+
+        expected = {
+                uuids[0]: [fault2, fault1],
+                uuids[1]: [fault4, fault3],
+        }
+
+        self.assertEqual(instance_faults, expected)
+
+    def test_instance_faults_get_by_instance_uuids_no_faults(self):
+        """None should be returned when no faults exist"""
+        ctxt = context.get_admin_context()
+        instance1 = db.instance_create(ctxt, {})
+        instance2 = db.instance_create(ctxt, {})
+        uuids = [instance1['uuid'], instance2['uuid']]
+        instance_faults = db.instance_fault_get_by_instance_uuids(ctxt, uuids)
+        expected = {uuids[0]: [], uuids[1]: []}
+        self.assertEqual(expected, instance_faults)