Fix update inventory for multiple providers

DB call method get_all_by_resource_provider_uuid queries all
Inventories and filters them by provider's uuid from ResourceProvider,
but query list is not valid (contains inventories for all providers),
the correct way to load ResourceProvider is to use query join. Replace
joinedload with contains_eager to avoid redundant joins.

As result last updated provider overwrites all provider's inventories
with wrong data.

Closes-Bug: #1572555
Change-Id: I49fb1bf63400280635c202dea8f870d727a91e81

nova/db/sqlalchemy/api_models.py

@@ -308,8 +308,8 @@ class Inventory(API_BASE):
     allocation_ratio = Column(Float, nullable=False)
     resource_provider = orm.relationship(
         "ResourceProvider",
-        primaryjoin=('and_(Inventory.resource_provider_id == '
-                     'ResourceProvider.id)'),
+        primaryjoin=('Inventory.resource_provider_id == '
+                     'ResourceProvider.id'),
         foreign_keys=resource_provider_id)
 
 

nova/objects/resource_provider.py

@@ -11,7 +11,7 @@
 #    under the License.
 
 import six
-from sqlalchemy.orm import joinedload
+from sqlalchemy.orm import contains_eager
 
 from nova.db.sqlalchemy import api as db_api
 from nova.db.sqlalchemy import api_models as models
@@ -223,9 +223,10 @@ class InventoryList(base.ObjectListBase, base.NovaObject):
     @staticmethod
     @db_api.api_context_manager.reader
     def _get_all_by_resource_provider(context, rp_uuid):
-        return context.session.query(models.Inventory).\
-            options(joinedload('resource_provider')).\
-            filter(models.ResourceProvider.uuid == rp_uuid).all()
+        return context.session.query(db_api.models.Inventory).\
+            join(db_api.models.Inventory.resource_provider).\
+            options(contains_eager('resource_provider')).\
+            filter(db_api.models.ResourceProvider.uuid == rp_uuid).all()
 
     @base.remotable_classmethod
     def get_all_by_resource_provider_uuid(cls, context, rp_uuid):

nova/tests/unit/objects/test_resource_provider.py

@@ -10,6 +10,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import uuid
+
 import mock
 
 from nova import exception
@@ -177,18 +179,23 @@ class TestRemoteInventoryNoDB(test_objects._RemoteTest,
 
 class TestInventory(test_objects._LocalTest):
 
-    def _make_inventory(self):
+    def _make_inventory(self, rp_uuid=None):
+        uuid = rp_uuid or uuids.inventory_resource_provider
+        name = uuid
         db_rp = objects.ResourceProvider(
-            context=self.context, uuid=uuids.inventory_resource_provider,
-            name=_RESOURCE_PROVIDER_NAME)
+            context=self.context, uuid=uuid, name=name)
         db_rp.create()
-        updates = dict(_INVENTORY_DB,
-                       resource_provider_id=db_rp.id)
-        updates.pop('id')
-        db_inventory = objects.Inventory._create_in_db(
-            self.context, updates)
+        db_inventory = self._create_inventory_in_db(db_rp.id)
         return db_rp, db_inventory
 
+    def _create_inventory_in_db(self, rp_id, **records):
+        updates = dict(_INVENTORY_DB,
+                       resource_provider_id=rp_id)
+        updates.pop('id')
+        updates.update(records)
+        return objects.Inventory._create_in_db(
+            self.context, updates)
+
     def test_create_in_db(self):
         updates = dict(_INVENTORY_DB)
         updates.pop('id')
@@ -229,6 +236,60 @@ class TestInventory(test_objects._LocalTest):
         )
         self.assertEqual(0, len(retrieved_inventories))
 
+    def test_get_all_by_resource_provider_multiple_providers(self):
+        # TODO(cdent): This and other nearby tests are functional
+        # and should be moved.
+        # Create 2 resource providers with DISK_GB resources. And
+        # update total value for second one.
+        db_rp1, db_inv1 = self._make_inventory(str(uuid.uuid4()))
+        db_rp2, db_inv2 = self._make_inventory(str(uuid.uuid4()))
+
+        objects.Inventory._update_in_db(self.context,
+                                        db_inv2.id,
+                                        {'total': 32})
+
+        # Create IPV4_ADDRESS resources for each provider.
+        IPV4_ADDRESS_ID = objects.fields.ResourceClass.index(
+            objects.fields.ResourceClass.IPV4_ADDRESS)
+
+        self._create_inventory_in_db(db_rp1.id,
+                                     resource_provider_id=db_rp1.id,
+                                     resource_class_id=IPV4_ADDRESS_ID,
+                                     total=2)
+        self._create_inventory_in_db(db_rp2.id,
+                                     resource_provider_id=db_rp2.id,
+                                     resource_class_id=IPV4_ADDRESS_ID,
+                                     total=4)
+
+        expected_inv1 = {
+            _RESOURCE_CLASS_ID: _INVENTORY_DB['total'],
+            IPV4_ADDRESS_ID: 2}
+        expected_inv2 = {
+            _RESOURCE_CLASS_ID: 32,
+            IPV4_ADDRESS_ID: 4}
+
+        # Get inventories for each resource provider and validate
+        # that the inventory records for that resource provider uuid
+        # and match expected total value.
+        retrieved_inv = (
+            objects.InventoryList._get_all_by_resource_provider(
+                self.context, db_rp1.uuid)
+        )
+        for inv in retrieved_inv:
+            self.assertEqual(db_rp1.id, inv.resource_provider_id)
+            self.assertEqual(expected_inv1[inv.resource_class_id],
+                             inv.total)
+
+        retrieved_inv = (
+            objects.InventoryList._get_all_by_resource_provider(
+                self.context, db_rp2.uuid)
+        )
+
+        for inv in retrieved_inv:
+            self.assertEqual(db_rp2.id, inv.resource_provider_id)
+            self.assertEqual(expected_inv2[inv.resource_class_id],
+                             inv.total)
+
     def test_create_requires_resource_provider(self):
         inventory_dict = dict(_INVENTORY_DB)
         inventory_dict.pop('id')