Merge lp:~rackspace-titan/nova/changes-since-lp837405 into lp:~hudson-openstack/nova/trunk

[Brian Waldon]

- implements changes-since for servers resource
- default sort is now created_at desc for instances

[Brian Lamar (blamar)]

Love it, seems pretty straight forward to me.

review: Approve

[Vish Ishaya (vishvananda)]

lgtm

review: Approve

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~rackspace-titan/nova/changes-since-lp837405 into lp:nova failed. Below is the output from the failed tests.

CreateserverextTest
    test_create_instance_with_duplicate_networks OK 0.31
    test_create_instance_with_duplicate_networks_xml OK 0.42
    test_create_instance_with_network_empty_fixed_ip OK 0.29
    test_create_instance_with_network_empty_fixed_ip_xml OK 0.29
    test_create_instance_with_network_invalid_id OK 0.44
    test_create_instance_with_network_invalid_id_xml OK 0.28
    test_create_instance_with_network_no_fixed_ip OK 0.29
    test_create_instance_with_network_no_fixed_ip_xml OK 0.43
    test_create_instance_with_network_no_id OK 0.28
    test_create_instance_with_network_no_id_xml OK 0.42
    test_create_instance_with_network_non_string_fixed_ip OK 0.28
    test_create_instance_with_no_networks OK 0.29
    test_create_instance_with_no_networks_xml OK 0.47
    test_create_instance_with_one_network OK 0.30
    test_create_instance_with_one_network_xml OK 0.29
    test_create_instance_with_two_networks OK 0.46
    test_create_instance_with_two_networks_xml OK 0.30
    test_create_instance_with_userdata OK 0.29
    test_create_instance_with_userdata_none OK 0.46
    test_create_instance_with_userdata_with_non_b64_content OK 0.29
FloatingIpTest
    test_add_associated_floating_ip_to_instance OK 0.35
    test_add_floating_ip_to_instance OK 0.45
    test_associate_floating_ip_to_instance_no_project_id OK 0.34
    test_associate_floating_ip_to_instance_wrong_project_id OK 0.48
    test_bad_address_param_in_add_floating_ip OK 0.31
    test_bad_address_param_in_remove_floating_ip OK 0.30
    test_floating_ip_allocate OK 0.48
    test_floating_ip_allocate_no_free_ips OK 0.30
    test_floating_ip_release OK 0.30
    test_floating_ip_show OK 0.43
    test_floating_ips_list OK 0.30
    test_missing_dict_param_in_add_floating_ip OK 0.30
    test_missing_dict_param_in_remove_floating_ip OK 0.43
    test_remove_floating_ip_from_instance OK 0.29
    test_show_associated_floating_ip OK 0.43
    test_translate_floating_ip_view OK 0.04
    test_translate_floating_ip_view_dict OK 0.02
KeypairsTest
    test_keypair_create OK 0.44
    test_keypair_delete OK 0.31
    test_keypair_import OK 0.59
    test_keypair_list OK 0.32
FixedIpTest
    test_add_...

=== modified file 'nova/api/openstack/servers.py'
--- nova/api/openstack/servers.py	2011-09-01 04:55:12 +0000
+++ nova/api/openstack/servers.py	2011-09-01 15:23:36 +0000
@@ -106,6 +106,14 @@
                 raise exception.InvalidInput(reason=reason)
             search_opts['vm_state'] = state
 
+        if 'changes-since' in search_opts:
+            try:
+                parsed = utils.parse_isotime(search_opts['changes-since'])
+            except ValueError:
+                msg = _('Invalid changes-since value')
+                raise exc.HTTPBadRequest(explanation=msg)
+            search_opts['changes-since'] = parsed
+
         # By default, compute's get_all() will return deleted instances.
         # If an admin hasn't specified a 'deleted' search option, we need
         # to filter out deleted instances by setting the filter ourselves.
@@ -113,23 +121,17 @@
         # should return recently deleted images according to the API spec.
 
         if 'deleted' not in search_opts:
-            # Admin hasn't specified deleted filter
             if 'changes-since' not in search_opts:
-                # No 'changes-since', so we need to find non-deleted servers
+                # No 'changes-since', so we only want non-deleted servers
                 search_opts['deleted'] = False
-            else:
-                # This is the default, but just in case..
-                search_opts['deleted'] = True
-
-        instance_list = self.compute_api.get_all(
-                context, search_opts=search_opts)
-
-        # FIXME(comstud): 'changes-since' is not fully implemented.  Where
-        # should this be filtered?
+
+        instance_list = self.compute_api.get_all(context,
+                                                 search_opts=search_opts)
 
         limited_list = self._limit_items(instance_list, req)
         servers = [self._build_view(req, inst, is_detail)['server']
-                for inst in limited_list]
+                    for inst in limited_list]
+
         return dict(servers=servers)
 
     @scheduler_api.redirect_handler

=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py	2011-08-31 23:07:29 +0000
+++ nova/db/sqlalchemy/api.py	2011-09-01 15:23:36 +0000
@@ -36,6 +36,7 @@
 from sqlalchemy.orm import joinedload
 from sqlalchemy.orm import joinedload_all
 from sqlalchemy.sql import func
+from sqlalchemy.sql.expression import desc
 from sqlalchemy.sql.expression import literal_column
 
 FLAGS = flags.FLAGS
@@ -1250,12 +1251,17 @@
                    options(joinedload_all('fixed_ips.network')).\
                    options(joinedload('metadata')).\
                    options(joinedload('instance_type')).\
-                   filter_by(deleted=can_read_deleted(context))
+                   order_by(desc(models.Instance.created_at))
 
     # Make a copy of the filters dictionary to use going forward, as we'll
     # be modifying it and we shouldn't affect the caller's use of it.
     filters = filters.copy()
 
+    if 'changes-since' in filters:
+        changes_since = filters['changes-since']
+        query_prefix = query_prefix.\
+                            filter(models.Instance.updated_at > changes_since)
+
     if not context.is_admin:
         # If we're not admin context, add appropriate filter..
         if context.project_id:
@@ -1277,9 +1283,7 @@
         query_prefix = _exact_match_filter(query_prefix, filter_name,
                 filters.pop(filter_name))
 
-    instances = query_prefix.\
-                    filter_by(deleted=can_read_deleted(context)).\
-                    all()
+    instances = query_prefix.all()
 
     if not instances:
         return []

=== modified file 'nova/tests/api/openstack/test_servers.py'
--- nova/tests/api/openstack/test_servers.py	2011-09-01 04:55:12 +0000
+++ nova/tests/api/openstack/test_servers.py	2011-09-01 15:23:36 +0000
@@ -1265,6 +1265,31 @@
         self.assertEqual(len(servers), 1)
         self.assertEqual(servers[0]['id'], 100)
 
+    def test_get_servers_allows_changes_since_v1_1(self):
+        def fake_get_all(compute_self, context, search_opts=None):
+            self.assertNotEqual(search_opts, None)
+            self.assertTrue('changes-since' in search_opts)
+            changes_since = datetime.datetime(2011, 1, 24, 17, 8, 1)
+            self.assertEqual(search_opts['changes-since'], changes_since)
+            self.assertTrue('deleted' not in search_opts)
+            return [stub_instance(100)]
+
+        self.stubs.Set(nova.compute.API, 'get_all', fake_get_all)
+
+        params = 'changes-since=2011-01-24T17:08:01Z'
+        req = webob.Request.blank('/v1.1/fake/servers?%s' % params)
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
+        servers = json.loads(res.body)['servers']
+        self.assertEqual(len(servers), 1)
+        self.assertEqual(servers[0]['id'], 100)
+
+    def test_get_servers_allows_changes_since_bad_value_v1_1(self):
+        params = 'changes-since=asdf'
+        req = webob.Request.blank('/v1.1/fake/servers?%s' % params)
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 400)
+
     def test_get_servers_unknown_or_admin_options1(self):
         """Test getting servers by admin-only or unknown options.
         This tests when admin_api is off.  Make sure the admin and

=== modified file 'nova/tests/test_cloud.py'
--- nova/tests/test_cloud.py	2011-09-01 14:02:02 +0000
+++ nova/tests/test_cloud.py	2011-09-01 15:23:36 +0000
@@ -486,8 +486,11 @@
         inst2 = db.instance_create(self.context, args2)
         db.instance_destroy(self.context, inst1.id)
         result = self.cloud.describe_instances(self.context)
-        result = result['reservationSet'][0]['instancesSet']
-        self.assertEqual(result[0]['instanceId'],
+        result1 = result['reservationSet'][0]['instancesSet']
+        self.assertEqual(result1[0]['instanceId'],
+                         ec2utils.id_to_ec2_id(inst1.id))
+        result2 = result['reservationSet'][1]['instancesSet']
+        self.assertEqual(result2[0]['instanceId'],
                          ec2utils.id_to_ec2_id(inst2.id))
 
     def _block_device_mapping_create(self, instance_id, mappings):

=== modified file 'nova/tests/test_db_api.py'
--- nova/tests/test_db_api.py	2011-08-18 21:57:52 +0000
+++ nova/tests/test_db_api.py	2011-09-01 15:23:36 +0000
@@ -91,5 +91,7 @@
         inst2 = db.instance_create(self.context, args2)
         db.instance_destroy(self.context, inst1.id)
         result = db.instance_get_all_by_filters(self.context.elevated(), {})
-        self.assertEqual(1, len(result))
+        self.assertEqual(2, len(result))
         self.assertEqual(result[0].id, inst2.id)
+        self.assertEqual(result[1].id, inst1.id)
+        self.assertTrue(result[1].deleted)