Merge "[placement] Fix incorrect exception import"

nova/api/openstack/placement/handlers/aggregate.py

@@ -17,12 +17,12 @@ from oslo_utils import encodeutils
 from oslo_utils import timeutils
 import webob
 
+from nova.api.openstack.placement import exception
 from nova.api.openstack.placement import microversion
 from nova.api.openstack.placement.objects import resource_provider as rp_obj
 from nova.api.openstack.placement.schemas import aggregate as schema
 from nova.api.openstack.placement import util
 from nova.api.openstack.placement import wsgi_wrapper
-from nova import exception
 from nova.i18n import _
 
 
@@ -53,6 +53,27 @@ def _serialize_aggregates(aggregate_uuids):
     return {'aggregates': aggregate_uuids}
 
 
+def _set_aggregates(resource_provider, aggregate_uuids,
+                    increment_generation=False):
+    """Set aggregates for the resource provider.
+
+    If increment generation is true, the resource provider generation
+    will be incremented if possible. If that fails (because something
+    else incremented the generation in another thread), a
+    ConcurrentUpdateDetected will be raised.
+    """
+    # NOTE(cdent): It's not clear what the DBDuplicateEntry handling
+    # is doing here, set_aggregates already handles that, but I'm leaving
+    # it here because it was already there.
+    try:
+        resource_provider.set_aggregates(
+            aggregate_uuids, increment_generation=increment_generation)
+    except (exception.ConcurrentUpdateDetected,
+            db_exc.DBDuplicateEntry) as exc:
+        raise webob.exc.HTTPConflict(
+            _('Update conflict: %(error)s') % {'error': exc})
+
+
 @wsgi_wrapper.PlacementWsgify
 @util.check_accept('application/json')
 @microversion.version_handler('1.1')
@@ -98,12 +119,7 @@ def set_aggregates(req):
         aggregate_uuids = data['aggregates']
     else:
         aggregate_uuids = data
-    try:
-        resource_provider.set_aggregates(
-            aggregate_uuids, increment_generation=consider_generation)
-    except (exception.ConcurrentUpdateDetected,
-            db_exc.DBDuplicateEntry) as exc:
-        raise webob.exc.HTTPConflict(
-            _('Update conflict: %(error)s') % {'error': exc})
+    _set_aggregates(resource_provider, aggregate_uuids,
+                    increment_generation=consider_generation)
 
     return _send_aggregates(req, resource_provider, aggregate_uuids)

nova/tests/unit/api/openstack/placement/handlers/test_aggregate.py

@@ -0,0 +1,37 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+"""Unit tests for code in the aggregate handler that gabbi isn't covering."""
+
+import mock
+import six
+import webob
+
+from nova.api.openstack.placement import exception
+from nova.api.openstack.placement.handlers import aggregate
+from nova.api.openstack.placement.objects import resource_provider
+from nova import test
+
+
+class TestAggregateHandlerErrors(test.NoDBTestCase):
+    """Tests that make sure errors hard to trigger by gabbi result in expected
+    exceptions.
+    """
+
+    def test_concurrent_exception_causes_409(self):
+        rp = resource_provider.ResourceProvider()
+        expected_message = ('Update conflict: Another thread concurrently '
+                            'updated the data')
+        with mock.patch.object(rp, "set_aggregates",
+                               side_effect=exception.ConcurrentUpdateDetected):
+            exc = self.assertRaises(webob.exc.HTTPConflict,
+                                    aggregate._set_aggregates, rp, [])
+        self.assertIn(expected_message, six.text_type(exc))