From 5b77c108f14f2bcd42fecfcd060331e57a2e07dd Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Tue, 24 Mar 2015 07:49:41 -0700 Subject: [PATCH] Convert pci_device_pools dict to object before passing to scheduler This also fixes other issues in the resource tracker around inconsistent use of legacy pci_stats usage from when we were dealing with a db model compute node, to pci_device_pools to be in line with objects. Closes-Bug: #1435483 Change-Id: I56a9aa8b3e81a22d57e83f956bbcf1e42599a954 --- nova/compute/resource_tracker.py | 17 +++++++------- nova/scheduler/client/report.py | 16 +++++++++++++- .../unit/compute/test_resource_tracker.py | 10 ++++++--- nova/tests/unit/compute/test_tracker.py | 22 +++++++++---------- nova/tests/unit/scheduler/test_client.py | 15 ++++++++++++- 5 files changed, 55 insertions(+), 25 deletions(-) diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index 40f0f2e79364..b305e3e21ab2 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -387,9 +387,9 @@ class ResourceTracker(object): # from deleted instances. if self.pci_tracker: self.pci_tracker.clean_usage(instances, migrations, orphans) - resources['pci_stats'] = jsonutils.dumps(self.pci_tracker.stats) + resources['pci_device_pools'] = self.pci_tracker.stats else: - resources['pci_stats'] = jsonutils.dumps([]) + resources['pci_device_pools'] = [] self._report_final_resource_view(resources) @@ -532,7 +532,7 @@ class ResourceTracker(object): else: tcpu = 0 ucpu = 0 - pci_stats = resources.get('pci_stats') + pci_device_pools = resources.get('pci_device_pools') LOG.info(_LI("Final resource view: " "name=%(node)s " "phys_ram=%(phys_ram)sMB " @@ -549,7 +549,7 @@ class ResourceTracker(object): 'used_disk': resources['local_gb_used'], 'total_vcpus': tcpu, 'used_vcpus': ucpu, - 'pci_stats': pci_stats}) + 'pci_stats': pci_device_pools}) def _resource_change(self, resources): """Check to see if any resouces have changed.""" @@ -668,10 +668,9 @@ class ResourceTracker(object): self.pci_tracker.update_pci_for_migration(context, instance) self._update_usage(context, resources, usage) if self.pci_tracker: - resources['pci_stats'] = jsonutils.dumps( - self.pci_tracker.stats) + resources['pci_device_pools'] = self.pci_tracker.stats else: - resources['pci_stats'] = jsonutils.dumps([]) + resources['pci_device_pools'] = [] self.tracked_migrations[uuid] = (migration, itype) def _update_usage_from_migrations(self, context, resources, migrations): @@ -740,9 +739,9 @@ class ResourceTracker(object): resources['current_workload'] = self.stats.calculate_workload() if self.pci_tracker: - resources['pci_stats'] = jsonutils.dumps(self.pci_tracker.stats) + resources['pci_device_pools'] = self.pci_tracker.stats else: - resources['pci_stats'] = jsonutils.dumps([]) + resources['pci_device_pools'] = [] def _update_usage_from_instances(self, context, resources, instances): """Calculate resource usage based on instance utilization. This is diff --git a/nova/scheduler/client/report.py b/nova/scheduler/client/report.py index 61cb68aa0995..5cfc78b0748d 100644 --- a/nova/scheduler/client/report.py +++ b/nova/scheduler/client/report.py @@ -53,7 +53,21 @@ class SchedulerReportClient(object): id=compute_node_id) compute_node.obj_reset_changes() for k, v in updates.items(): - setattr(compute_node, k, v) + if k == 'pci_device_pools': + # NOTE(danms): Since the updates are actually the result of + # a obj_to_primitive() on some real objects, we need to convert + # back to a real object (not from_dict() or _from_db_object(), + # which expect a db-formatted object) but just an attr-based + # reconstruction. When we start getting a ComputeNode from + # scheduler this "bandage" can go away. + if v: + devpools = [objects.PciDevicePool.from_dict(x) for x in v] + else: + devpools = [] + compute_node.pci_device_pools = objects.PciDevicePoolList( + objects=devpools) + else: + setattr(compute_node, k, v) compute_node.save() LOG.info(_LI('Compute_service record updated for ' diff --git a/nova/tests/unit/compute/test_resource_tracker.py b/nova/tests/unit/compute/test_resource_tracker.py index 94837d14f8d5..06392ca2aa8d 100644 --- a/nova/tests/unit/compute/test_resource_tracker.py +++ b/nova/tests/unit/compute/test_resource_tracker.py @@ -721,7 +721,7 @@ class TrackerTestCase(BaseTrackerTestCase): self.assertFalse(self.tracker.disabled) self.assertEqual(0, self.tracker.compute_node['current_workload']) self.assertEqual(driver.pci_stats, - jsonutils.loads(self.tracker.compute_node['pci_stats'])) + self.tracker.compute_node['pci_device_pools']) class SchedulerClientTrackerTestCase(BaseTrackerTestCase): @@ -778,8 +778,12 @@ class TrackerPciStatsTestCase(BaseTrackerTestCase): self._assert(FAKE_VIRT_LOCAL_GB, 'free_disk_gb') self.assertFalse(self.tracker.disabled) self.assertEqual(0, self.tracker.compute_node['current_workload']) - self.assertEqual(driver.pci_stats, - jsonutils.loads(self.tracker.compute_node['pci_stats'])) + + # NOTE(danms): PciDeviceStats only supports iteration, so we have to + # listify it before we can examine the contents by index. + pools = list(self.tracker.compute_node['pci_device_pools']) + self.assertEqual(driver.pci_stats[0]['product_id'], + pools[0]['product_id']) def _driver(self): return FakeVirtDriver(pci_support=True) diff --git a/nova/tests/unit/compute/test_tracker.py b/nova/tests/unit/compute/test_tracker.py index 68a655f7c540..36e496ae329d 100644 --- a/nova/tests/unit/compute/test_tracker.py +++ b/nova/tests/unit/compute/test_tracker.py @@ -395,7 +395,7 @@ class TestUpdateAvailableResources(BaseTestCase): 'local_gb': 6, 'free_ram_mb': 512, 'memory_mb_used': 0, - 'pci_stats': '[]', + 'pci_device_pools': [], 'vcpus_used': 0, 'hypervisor_type': 'fake', 'local_gb_used': 0, @@ -431,7 +431,7 @@ class TestUpdateAvailableResources(BaseTestCase): 'local_gb': 6, 'free_ram_mb': 0, # 512MB avail - 512MB reserved 'memory_mb_used': 512, # 0MB used + 512MB reserved - 'pci_stats': '[]', + 'pci_device_pools': [], 'vcpus_used': 0, 'hypervisor_type': 'fake', 'local_gb_used': 1, # 0GB used + 1 GB reserved @@ -466,7 +466,7 @@ class TestUpdateAvailableResources(BaseTestCase): 'local_gb': 6, 'free_ram_mb': 384, # 512 - 128 used 'memory_mb_used': 128, - 'pci_stats': '[]', + 'pci_device_pools': [], # NOTE(jaypipes): Due to the design of the ERT, which now is used # track VCPUs, the actual used VCPUs isn't # "written" to the resources dictionary that is @@ -532,7 +532,7 @@ class TestUpdateAvailableResources(BaseTestCase): 'local_gb': 6, 'free_ram_mb': 448, # 512 - 64 orphaned usage 'memory_mb_used': 64, - 'pci_stats': '[]', + 'pci_device_pools': [], 'vcpus_used': 0, 'hypervisor_type': 'fake', 'local_gb_used': 0, @@ -586,7 +586,7 @@ class TestUpdateAvailableResources(BaseTestCase): 'local_gb': 6, 'free_ram_mb': 384, # 512 total - 128 for possible revert of orig 'memory_mb_used': 128, # 128 possible revert amount - 'pci_stats': '[]', + 'pci_device_pools': [], 'vcpus_used': 0, 'hypervisor_type': 'fake', 'local_gb_used': 1, @@ -635,7 +635,7 @@ class TestUpdateAvailableResources(BaseTestCase): 'local_gb': 6, 'free_ram_mb': 256, # 512 total - 256 for possible confirm of new 'memory_mb_used': 256, # 256 possible confirmed amount - 'pci_stats': '[]', + 'pci_device_pools': [], 'vcpus_used': 0, # See NOTE(jaypipes) above about why this is 0 'hypervisor_type': 'fake', 'local_gb_used': 5, @@ -688,7 +688,7 @@ class TestUpdateAvailableResources(BaseTestCase): # 512 total - 128 existing - 256 new flav - 128 old flav 'free_ram_mb': 0, 'memory_mb_used': 512, # 128 exist + 256 new flav + 128 old flav - 'pci_stats': '[]', + 'pci_device_pools': [], # See NOTE(jaypipes) above for reason why this isn't accurate until # _sync_compute_node() is called. 'vcpus_used': 0, @@ -743,7 +743,7 @@ class TestSyncComputeNode(BaseTestCase): 'local_gb': 6, 'free_ram_mb': 512, 'memory_mb_used': 0, - 'pci_stats': '[]', + 'pci_device_pools': [], 'vcpus_used': 0, 'hypervisor_type': 'fake', 'local_gb_used': 0, @@ -813,7 +813,7 @@ class TestSyncComputeNode(BaseTestCase): 'local_gb': 6, 'free_ram_mb': 512, 'memory_mb_used': 0, - 'pci_stats': '[]', + 'pci_device_pools': [], 'vcpus_used': 0, 'hypervisor_type': 'fake', 'local_gb_used': 0, @@ -862,7 +862,7 @@ class TestSyncComputeNode(BaseTestCase): 'local_gb': 6, 'free_ram_mb': 384, 'memory_mb_used': 128, - 'pci_stats': '[]', + 'pci_device_pools': [], 'vcpus_used': 2, 'hypervisor_type': 'fake', 'local_gb_used': 4, @@ -960,7 +960,7 @@ class TestInstanceClaim(BaseTestCase): "free_ram_mb": expected['memory_mb'] - self.instance.memory_mb, 'running_vms': 1, # 'vcpus_used': 0, # vcpus are not claimed - 'pci_stats': '[]', + 'pci_device_pools': [], }) with mock.patch.object(self.rt, '_update') as update_mock: self.rt.instance_claim(self.ctx, self.instance, None) diff --git a/nova/tests/unit/scheduler/test_client.py b/nova/tests/unit/scheduler/test_client.py index 2d919ff651ae..bafec79999fb 100644 --- a/nova/tests/unit/scheduler/test_client.py +++ b/nova/tests/unit/scheduler/test_client.py @@ -39,7 +39,11 @@ class SchedulerReportClientTestCase(test.NoDBTestCase): @mock.patch.object(objects.ComputeNode, '__new__') def test_update_compute_node_works(self, mock_cn): - stats = {"id": 1, "foo": "bar"} + stats = {"id": 1, "foo": "bar", + "pci_device_pools": [{"vendor_id": "foo", + "product_id": "foo", + "count": 1, + "a": "b"}]} self.client.update_resource_stats(self.context, ('fakehost', 'fakenode'), stats) @@ -48,6 +52,7 @@ class SchedulerReportClientTestCase(test.NoDBTestCase): id=1) cn = mock_cn() cn.obj_reset_changes.assert_called_once_with() + self.assertEqual("b", cn.pci_device_pools[0].tags["a"]) cn.save.assert_called_once_with() self.assertEqual('bar', cn.foo) @@ -57,6 +62,14 @@ class SchedulerReportClientTestCase(test.NoDBTestCase): self.client.update_resource_stats, self.context, ('fakehost', 'fakenode'), stats) + @mock.patch('nova.objects.ComputeNode.save') + def test_update_resource_stats_pci_device_pools_none(self, mock_save): + stats = {"id": 1, "foo": "bar", + "pci_device_pools": None} + self.client.update_resource_stats(self.context, + ('fakehost', 'fakenode'), + stats) + class SchedulerQueryClientTestCase(test.NoDBTestCase):